Skip to content

test(charts): Helm Chart Tests#336

Open
ravisoundar wants to merge 1 commit into
mainfrom
rs-chart-tests
Open

test(charts): Helm Chart Tests#336
ravisoundar wants to merge 1 commit into
mainfrom
rs-chart-tests

Conversation

@ravisoundar
Copy link
Copy Markdown
Collaborator

Description

Tests for validating the helm charts

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • All commits are signed off per DCO (git commit -s).

@ravisoundar ravisoundar requested a review from dmitsh as a code owner May 15, 2026 21:27
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 15, 2026

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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR adds a comprehensive Helm chart test suite (scripts/chart-test.sh) covering the topograph umbrella chart and its node-data-broker/node-observer subcharts, wires it into CI as a dedicated chart-test job, and fixes a pre-existing standalone-lint breakage in node-observer templates that referenced the parent-chart-scoped topograph.url helper.

  • scripts/chart-test.sh: Implements golden-file regression tests (one per values*.yaml fixture), positive rendering checks (Ingress, ServiceMonitor, helm test hooks), and negative validation tests (mutually-exclusive ingress+gateway, incomplete GCP WIF config); temp-file cleanup uses a subshell EXIT trap.
  • node-observer template fix: Adds node-observer.topographBaseURL in the subchart's own helper namespace (matching the pattern of topograph.url in the parent), replacing the cross-namespace reference that broke helm lint when the subchart was used standalone.
  • CI: New chart-test job with timeout-minutes: 5 runs make chart-test on every push/PR; golden files under tests/charts/topograph/ are committed as regression baselines.

Confidence Score: 5/5

Safe to merge — changes are additive (new test script, CI job, golden files) plus a narrow template-namespace fix in node-observer verified by the committed golden files.

All functional changes are confined to test infrastructure and a well-scoped Helm helper fix. The golden files provide concrete proof that the node-observer configmap and deployment render correctly with the new helper. No production runtime paths are affected.

No files require special attention. The golden files under tests/charts/topograph/ are the canonical ground truth for template output and should be reviewed whenever charts/topograph/ templates or values change.

Important Files Changed

Filename Overview
scripts/chart-test.sh New comprehensive Helm chart test script covering golden-file comparisons, validation failure cases, and subchart smoke tests; temp-file cleanup handled via subshell EXIT trap.
.github/workflows/go.yml Adds chart-test CI job (Helm v3.20.0, timeout-minutes: 5) that runs make chart-test on every push/PR trigger.
charts/topograph/charts/node-observer/templates/_helpers.tpl Adds node-observer.topographBaseURL helper so the subchart constructs the service URL without referencing the parent-chart-scoped topograph.url, fixing standalone helm lint.
charts/topograph/charts/node-observer/templates/configmap.yml Switches generateTopologyUrl from parent-scoped topograph.url to node-observer.topographBaseURL so the configmap renders correctly both standalone and as a dependency.
charts/topograph/charts/node-observer/templates/deployment.yaml Same template reference fix as configmap.yml – init-container health-check URL updated to node-observer.topographBaseURL.
tests/charts/node-data-broker-ci.yaml Minimal stub values supplying global.provider.name for standalone node-data-broker subchart lint and template tests.
tests/charts/node-observer-ci.yaml Minimal stub values (provider, engine, service.port) for standalone node-observer subchart lint and template tests.
tests/charts/topograph/values.yaml.golden.yaml Golden snapshot of helm template output for the default values.yaml; used as baseline for regression detection.

Reviews (3): Last reviewed commit: "test(charts): Helm Chart Tests" | Re-trigger Greptile

Comment thread scripts/chart-test.sh Outdated
Comment thread .github/workflows/go.yml
Comment on lines +9 to +11
global:
provider:
name: test
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this duplication?

Copy link
Copy Markdown
Collaborator Author

@ravisoundar ravisoundar May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it for running the tests for node-data-broker chart stand-alone. Removed it now, and isolated the testing conf changes.

Comment on lines +9 to +15
global:
provider:
name: test
engine:
name: k8s
service:
port: 49021
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't have this duplication

Copy link
Copy Markdown
Collaborator Author

@ravisoundar ravisoundar May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it for running the tests for node-observer chart stand-alone. Removed it now, and isolated the testing conf changes.

Signed-off-by: Ravi Shankar <ravish@nvidia.com>
@ravisoundar ravisoundar requested a review from dmitsh May 16, 2026 00:04
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