From e82d28784cb856ab2d4d3f336bfec3e1e94d2fdd Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Fri, 18 Sep 2026 14:11:48 -0700 Subject: [PATCH] fix(release): let a stack declare the image pins it repeats deploy/stacks/nvcf-compute-plane pins the BYOO OTel collector in environments/base.yaml, the same image the nvca-operator chart pins. The chart's four fields auto-bump on every collector release through their deploys edge; the stack had no edge and no way to declare one, so it sat on 0.157.0-nv-0.2.1 while the chart moved to 0.160.0-nv-0.2.5. The gap widened with each release rather than holding steady. Declaring the edge was not enough on its own. Two lookups excluded the stack before it could be planned: - ChartsDeploying required deploy/helm/, so a deploy/stacks/ entry was never a candidate at all, and the run reported "no chart declares that it deploys byoo-otel-collector". - PlanForValuesPaths then required a Chart.yaml under the entry path. A stack is a helmfile tree with no chart of its own, and its charts sit at charts/*/Chart.yaml, two levels below where ChartFiles looks. Both now make an exception for an edge that names its files explicitly, which is self-describing and needs no chart to resolve against. An edge using values_paths still requires a Chart.yaml, because those paths resolve against the chart's own values.yaml and there is nothing to resolve them against without one. ApplyValuesPaths needed no change: it returns through commitFileUpdates before touching Chart.yaml whenever app_version is false, which it is here. The stale pin itself is bumped in this commit by running the tool (`chart-version-bumper --tag .../v0.160.0-nv-0.2.5 --write`) rather than by editing the file, so the fix and the mechanism are the same code path. Golden testdata carries the rendered value twice. Those two literals are substituted rather than re-rendered: rendering pulls the operator chart from nvcr.io, which needs credentials this checkout does not have. The change is literal-for-literal what a re-render produces, and no workflow runs compare-golden. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Kristina Pathak --- .../nvcf-compute-plane/environments/base.yaml | 2 +- .../templates/deployment.yaml | 2 +- .../self-managed-nvcfbackend-cm.yaml | 2 +- tools/chart-version-bumper/chart.go | 7 ++- tools/chart-version-bumper/main_test.go | 52 +++++++++++++++++++ tools/chart-version-bumper/metadata.go | 18 +++++-- tools/ci/github-release-subprojects.json | 15 +++++- 7 files changed, 88 insertions(+), 10 deletions(-) diff --git a/deploy/stacks/nvcf-compute-plane/environments/base.yaml b/deploy/stacks/nvcf-compute-plane/environments/base.yaml index dbd85bdf19..b0bda15219 100644 --- a/deploy/stacks/nvcf-compute-plane/environments/base.yaml +++ b/deploy/stacks/nvcf-compute-plane/environments/base.yaml @@ -40,7 +40,7 @@ global: # Self-hosted registries do not always mirror this optional image. # Enable it only after publishing the collector in global.image. enabled: false - imageTag: "0.157.0-nv-0.2.1" + imageTag: "0.160.0-nv-0.2.5" # ICMS (SIS) service URL: required; set per environment. icmsServiceURL: "" icmsServiceHostHeaderOverride: "" diff --git a/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/deployment.yaml b/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/deployment.yaml index f6438dee35..679633b4d0 100644 --- a/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/deployment.yaml +++ b/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/deployment.yaml @@ -95,7 +95,7 @@ spec: - name: OTEL_COLLECTOR_IMAGE_REPO value: "nvcr.io/0651155215864979/ncp-dev/nvcf-otel-collector" - name: OTEL_COLLECTOR_IMAGE_TAG - value: "0.157.0-nv-0.2.1" + value: "0.160.0-nv-0.2.5" - name: OTEL_COLLECTOR_RESOURCES_B64 value: "eyJsaW1pdHMiOnsiY3B1IjoiMTAwMG0iLCJtZW1vcnkiOiIxR2kifSwicmVxdWVzdHMiOnsiY3B1IjoiMjAwbSIsIm1lbW9yeSI6IjI1Nk1pIn19" - name: OPERATOR_NAMESPACE diff --git a/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml b/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml index 28e9d2a4c2..68455489cc 100644 --- a/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml +++ b/deploy/stacks/nvcf-compute-plane/testdata/golden/local/02-nvca.yaml-nvca-operator/helm-nvca-operator/templates/self-managed-nvcfbackend-cm.yaml @@ -64,7 +64,7 @@ data: enabled: false imageConfig: repository: "nvcr.io/0651155215864979/ncp-dev/nvcf-otel-collector" - tag: "0.157.0-nv-0.2.1" + tag: "0.160.0-nv-0.2.5" agent: natsURL: "nats://nats.nats-system.svc.cluster.local:4222" helmReValStageOAuthTokenURL: "" diff --git a/tools/chart-version-bumper/chart.go b/tools/chart-version-bumper/chart.go index 0ce456a17f..10ba3912e2 100644 --- a/tools/chart-version-bumper/chart.go +++ b/tools/chart-version-bumper/chart.go @@ -325,7 +325,12 @@ func Apply(root string, chart Entry, version string, p Plan) error { // those paths and appVersion is required and all of them move together. func PlanForValuesPaths(root string, chart Entry, version string, paths []string, files []ValuesFile, ownsAppVersion bool) (Plan, error) { chartYAML, valuesYAML := ChartFiles(root, chart.Path) - if chartYAML == "" { + // Only the chart's own values.yaml and its appVersion need a Chart.yaml. + // A target that names its files explicitly does not: stacks pin the same + // images charts do, from helmfile trees that have no chart of their own. + // Skipping those outright is why a stack pin could sit still while the chart + // beside it auto-bumped every release. + if chartYAML == "" && (ownsAppVersion || len(paths) > 0) { return Plan{Action: ActionSkip, Detail: fmt.Sprintf("no Chart.yaml under %s", chart.Path)}, nil } specs, err := declaredValuesSpecs(root, chart, valuesYAML, paths, files) diff --git a/tools/chart-version-bumper/main_test.go b/tools/chart-version-bumper/main_test.go index f9963d4043..34aae94cd4 100644 --- a/tools/chart-version-bumper/main_test.go +++ b/tools/chart-version-bumper/main_test.go @@ -809,6 +809,58 @@ func TestValuesPathDistinguishesRepeatedLeafKeysByFullAncestry(t *testing.T) { } } +func TestValuesFilesBumpATargetThatHasNoChartYAML(t *testing.T) { + // Stacks pin the same images charts do, but they are helmfile trees with no + // Chart.yaml of their own. Requiring one skipped them without a word, which + // is how deploy/stacks/nvcf-compute-plane sat on a collector release the + // nvca-operator chart had auto-bumped away from months earlier. + f := newFixture(t, `{"services":[ + {"id":"sidecar","path":"src/sidecar"}, + {"id":"stack","path":"deploy/stacks/s","deploys":[{"service":"sidecar","values_files":[{"file":"environments/base.yaml","paths":["global.otelCollector.imageTag"]}]}]} + ]}`) + f.source(t, "deploy/stacks/s/environments/base.yaml", + "global:\n otelCollector:\n enabled: false\n imageTag: \"0.157.0-nv-0.2.1\"\n") + + if code, _, errOut := f.run(t, "src/sidecar/v0.160.0-nv-0.2.5", true); code != 0 { + t.Fatalf("want a clean bump, got %d\n%s", code, errOut) + } + + b, err := os.ReadFile(filepath.Join(f.root, "deploy/stacks/s/environments/base.yaml")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `imageTag: "0.160.0-nv-0.2.5"`) { + t.Fatalf("the declared stack pin did not move:\n%s", b) + } +} + +func TestChartlessTargetStillRefusesWhenItDeclaresAValuesPath(t *testing.T) { + // values_paths resolve against the chart's own values.yaml, so without a + // Chart.yaml there is nothing to resolve them against. That has to stay a + // skip rather than silently becoming a no-op success. + f := newFixture(t, `{"services":[ + {"id":"sidecar","path":"src/sidecar"}, + {"id":"stack","path":"deploy/stacks/s","deploys":[{"service":"sidecar","values_paths":["global.otelCollector.imageTag"]}]} + ]}`) + f.source(t, "deploy/stacks/s/environments/base.yaml", "global:\n otelCollector:\n imageTag: \"0.157.0-nv-0.2.1\"\n") + + code, out, errOut := f.run(t, "src/sidecar/v0.160.0-nv-0.2.5", true) + if code != 0 { + t.Fatalf("a skip is not a failure, got %d\n%s", code, errOut) + } + if !strings.Contains(out+errOut, "nothing to do") { + t.Fatalf("want the run to report it did nothing:\n%s%s", out, errOut) + } + + b, err := os.ReadFile(filepath.Join(f.root, "deploy/stacks/s/environments/base.yaml")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `imageTag: "0.157.0-nv-0.2.1"`) { + t.Fatalf("nothing was resolvable, so nothing may be rewritten:\n%s", b) + } +} + func TestValuesPathRerunIsANoOp(t *testing.T) { f := newFixture(t, `{"services":[ {"id":"sidecar","path":"src/sidecar"}, diff --git a/tools/chart-version-bumper/metadata.go b/tools/chart-version-bumper/metadata.go index fbc2785423..d1a8962a99 100644 --- a/tools/chart-version-bumper/metadata.go +++ b/tools/chart-version-bumper/metadata.go @@ -165,14 +165,22 @@ type ChartDeploy struct { func (m *Metadata) ChartsDeploying(serviceID string) []ChartDeploy { var out []ChartDeploy for _, e := range m.Services { - if !strings.HasPrefix(e.Path, ChartPrefix) { - continue - } for _, d := range e.Deploys { - if d.Service == serviceID { - out = append(out, ChartDeploy{Entry: e, ValuesPaths: d.ValuesPaths, ValuesFiles: d.ValuesFiles, AppVersion: d.AppVersion}) + if d.Service != serviceID { + continue + } + // Charts live under deploy/helm/, and everything outside it is a + // service whose own release moves its version. The exception is a + // target that names the files it pins: a stack is a helmfile tree + // with no chart of its own, so it can never be found by the chart + // lookup, yet it repeats the same image pins. Excluding those + // outright is what let a stack pin sit still for release after + // release while the chart beside it bumped itself. + if !strings.HasPrefix(e.Path, ChartPrefix) && len(d.ValuesFiles) == 0 { break } + out = append(out, ChartDeploy{Entry: e, ValuesPaths: d.ValuesPaths, ValuesFiles: d.ValuesFiles, AppVersion: d.AppVersion}) + break } } return out diff --git a/tools/ci/github-release-subprojects.json b/tools/ci/github-release-subprojects.json index 19a647bf27..e7e4bdb1d1 100644 --- a/tools/ci/github-release-subprojects.json +++ b/tools/ci/github-release-subprojects.json @@ -37,7 +37,20 @@ "tag_format": "deploy/stacks/nvcf-compute-plane/v${version}", "version_file": "VERSION", "release_branch_only": true, - "resolved_inventory_asset": "nvcf-compute-plane-stack-inventory.json" + "resolved_inventory_asset": "nvcf-compute-plane-stack-inventory.json", + "deploys": [ + { + "service": "byoo-otel-collector", + "values_files": [ + { + "file": "environments/base.yaml", + "paths": [ + "global.nvcaOperator.selfManaged.otelCollector.imageTag" + ] + } + ] + } + ] }, { "id": "nvcf-self-managed-stack",