Skip to content

fix(examples): add resource limits to Helm workload samples - #1889

Merged
sbaum1994 merged 1 commit into
mainfrom
fix/helm-task-sample-resources
Sep 15, 2026
Merged

sbaum1994 merged 1 commit into
mainfrom
fix/helm-task-sample-resources

Conversation

@sbaum1994

@sbaum1994 sbaum1994 commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

TL;DR

Add default CPU and memory requests and limits to the Helm task and function
samples so current self-managed admission validation accepts both workloads.

Additional Details

The self-managed compute-plane stack enables Helm task and function resource
enforcement by default. Both public sample charts omitted container resources,
so users could create the API objects but the compute plane rejected the
rendered workloads during admission.

This change:

  • adds configurable resources values with small CPU and memory defaults to
    both charts
  • renders the values on the task and inference containers
  • bumps both chart versions to 0.1.1
  • documents why resources are required and how to override them

No third-party dependencies were added. No license or NOTICE changes are
required.

For the Reviewer

Please review the default resource sizing and the template indentation in the
task Job and function Deployment.

For QA

Validated with:

  • helm lint examples/task-samples/task-helmchart-sample/task-helmchart-test
  • helm template task-helmchart-test examples/task-samples/task-helmchart-sample/task-helmchart-test
  • helm lint examples/function-samples/helmchart-samples/inference-test-sample/inference-test
  • helm template inference-test examples/function-samples/helmchart-samples/inference-test-sample/inference-test
  • git diff --check

QA is needed after publishing the corrected chart versions. The BDD coverage
in #1890 verifies that charts without resources fail while the compliant task
and function charts complete successfully.

Issues

Closes #1887

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features

    • Added configurable CPU and memory requests and limits to the sample Helm charts.
    • Enabled resource settings to be overridden through a values file.
  • Documentation

    • Documented resource configuration and self-managed validation requirements.
    • Updated Helm usage examples to reference the selected chart version.
  • Chores

    • Updated the sample Helm chart version to 0.1.1.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2e46db85-0764-4360-a174-0f575eba72de

📥 Commits

Reviewing files that changed from the base of the PR and between 5973f22 and f6bcd19.

📒 Files selected for processing (4)
  • examples/function-samples/helmchart-samples/inference-test-sample/README.md
  • examples/function-samples/helmchart-samples/inference-test-sample/inference-test/Chart.yaml
  • examples/function-samples/helmchart-samples/inference-test-sample/inference-test/templates/deployment.yaml
  • examples/function-samples/helmchart-samples/inference-test-sample/inference-test/values.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

The task and inference Helm samples now define configurable CPU and memory requests and limits. Their workloads render these resources. Chart versions and documentation now reflect the updated configuration.

Changes

Helm resource enforcement

Layer / File(s) Summary
Task chart resource configuration
examples/task-samples/task-helmchart-sample/...
The chart sets CPU to 100m and memory to 128Mi for requests and limits. The Job renders .Values.resources. The chart version changes to 0.1.1. The README documents resource overrides.
Inference chart resource configuration
examples/function-samples/helmchart-samples/inference-test-sample/...
The chart sets CPU to 100m and memory to 128Mi for requests and limits. The deployment renders .Values.resources. The chart version changes to 0.1.1. Examples use a chart version placeholder and document resource overrides.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to f6bcd

The sample charts now provide configurable resource requests and limits as intended, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning For #1887, the task Helm chart now renders configurable CPU and memory requests and limits on the task container. The default values set both to 100m and 128Mi. The chart version and README docu… Add the linked tests/bdd Helm task scenarios. Submit a chart without resource limits and assert terminal task failure. Submit a compliant chart and assert COMPLETED. Verify the scenarios through the public NVCT API in the supported loca…
Out of Scope Changes check ⚠️ Warning The changes under examples/function-samples/helmchart-samples/inference-test-sample update a Helm function chart, its deployment resources, its version, and its documentation. Issue #1887 targets th… Remove the function-sample changes from this pull request, or link them to a separate requirement that requires resource configuration for the Helm function sample.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format and accurately describes the primary fix: adding resource limits to Helm workload samples. The required scope is present for the customer-impacting fix ty…
Full details: Linked Issues check

Explanation

For #1887, the task Helm chart now renders configurable CPU and memory requests and limits on the task container. The default values set both to 100m and 128Mi. The chart version and README document the change. The required live BDD coverage is still absent. Helm linting, template rendering, and whitespace checks do not verify NVCT task submission, terminal failure for a chart without limits, successful completion for a compliant chart, or the default feature gates.

Resolution

Add the linked tests/bdd Helm task scenarios. Submit a chart without resource limits and assert terminal task failure. Submit a compliant chart and assert COMPLETED. Verify the scenarios through the public NVCT API in the supported local single- and multi-cluster Helmfile deployments.

Full details: Out of Scope Changes check

Explanation

The changes under examples/function-samples/helmchart-samples/inference-test-sample update a Helm function chart, its deployment resources, its version, and its documentation. Issue #1887 targets the public Helm task sample. The issue states that Helm functions continue to work and does not require changes to the function sample. These changes are therefore outside the linked issue scope.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/helm-task-sample-resources

Comment @coderabbitai help to get the list of available commands.

Closes #1887

Signed-off-by: Stephanie Baum <sbaum@nvidia.com>
@sbaum1994
sbaum1994 force-pushed the fix/helm-task-sample-resources branch from 5973f22 to f6bcd19 Compare September 15, 2026 04:10
@sbaum1994 sbaum1994 changed the title fix(examples): add resource limits to Helm task sample fix(examples): add resource limits to Helm workload samples Sep 15, 2026
@sbaum1994
sbaum1994 merged commit 9ff9c91 into main Sep 15, 2026
20 checks passed
@sbaum1994
sbaum1994 deleted the fix/helm-task-sample-resources branch September 15, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(examples): make Helm workload samples compatible with resource enforcement

1 participant