Skip to content

Fix Helm effective namespace validation - #2044

Open
ajavanma wants to merge 1 commit into
NVIDIA:mainfrom
ajavanma:fix/helm-effective-namespace
Open

ajavanma wants to merge 1 commit into
NVIDIA:mainfrom
ajavanma:fix/helm-effective-namespace

Conversation

@ajavanma

Copy link
Copy Markdown

Description

Validate the Helm chart's effective resource namespace so --namespace default --set namespaceOverride=gpu-system succeeds and the reverse override requires allowDefaultNamespace=true.

Use the existing namespace helper in one validation guard and remove the duplicate release-namespace guard. Preserve the error text and explicit opt-in. Add Go/Helm regression coverage for release namespaces with and without overrides, both with opt-in disabled and enabled, asserting errors and rendered DaemonSet namespaces.

Checklist

  • No secrets, sensitive information, or unrelated changes
  • Lint checks passing (make lint)
  • Unit tests passing (make test)
  • Go mod artifacts in-sync (make check-modules)
  • Third-party notices in-sync (make check-third-party-notices)
  • Test cases are added for new code paths

Testing

  • Added regression test on the unchanged template: fails in the two override-direction cases, with 10 control cases passing.
  • make test-helm: passes with the fix, including all 12 new regression cases.
  • make fmt plus gofmt -w tests/helm/helm_template_test.go: passes, no unrelated changes.
  • make check: passes, 0 lint issues.
  • make test: passes, including package compilation and all four command builds. Vendored NVML code emits deprecation warnings.
  • git diff --check: passes.
  • Additional separate-module lint (cd tests && golangci-lint run --allow-serial-runners --timeout=5m ./helm/...) reports three modernize/newexpr suggestions in unchanged ptr code. Running the same lint against the base test file reproduces all three; no unrelated modernization is included.

Validated on Linux ARM64 with Go 1.27.1, Helm 4.3.0, and golangci-lint 2.13.2. GPU/cluster e2e tests were not run; this change is exercised through real Helm rendering without a cluster. Module and third-party-notice regeneration checks were not run because no dependencies or license inputs changed; their checklist boxes remain unchecked.

Signed-off-by: Arash Javanmardi <arash@javanmardi.info>
@copy-pr-bot

copy-pr-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@ajavanma
ajavanma force-pushed the fix/helm-effective-namespace branch from 9d6aa0a to 1b91fb4 Compare September 18, 2026 09:06
@myeolenv

Copy link
Copy Markdown

Thanks for the contribution. We will review this PR soon.

This branch has not been deployed

No deployments
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.

2 participants