fix(nvca-operator): [backport v3.2] resolve image tag through appVersion, not chart version - #1250
Merged
kristinapathak merged 3 commits intoAug 26, 2026
Conversation
…ersion helm upgrade --reuse-values carries a release's fully-resolved values.yaml forward but never re-merges the new chart's Chart.yaml. Falling the image tag back to .Chart.Version left every --reuse-values upgrade pinned to whichever chart last published a literal image.tag, since that literal overrides the new chart's own default. Fall back through .Chart.AppVersion instead in the operator, mirror, cluster-validator, and cleanup-job image references. appVersion is chart metadata Helm always re-reads from the target chart, so it can't be pinned by a previous release's reused values. Strengthen the vendoring test to cover a chart-version/appVersion divergence and a --reuse-values upgrade simulation. Closes #1246
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…ator's The regression test grepped for one image string that happened to match three of five image render sites (operator, nvca-mirror, and the pre-delete cleanup job all share .Values.image.repository), so a regression in any single one of them could still pass. clusterValidator's two render sites (deployment initContainer, its own CronJob) were never enabled or asserted at all. Enable clusterValidator in both renders and assert exact occurrence counts per distinct image, so a stale tag on any of the five sites fails the test.
estroz
approved these changes
Aug 26, 2026
grep -cF matched image lines by substring, so an unexpected tag like 3.2.11-hotfix still counted as a match for 3.2.11 and could mask a regression. Switch to awk and require the full image field to equal the expected value exactly.
kristinapathak
merged commit Aug 26, 2026
6cf2780
into
release-src/compute-plane-services/nvca/v3.2
12 checks passed
kristinapathak
deleted the
fix/nvca-operator-image-tag-reuse-values-v3.2
branch
August 26, 2026 23:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #1248 to release-src/compute-plane-services/nvca/v3.2. Clean
cherry-pick (80c2dd2), no conflicts.
Why
helm upgrade --reuse-valuesagainst the nvca-operator chart carries theprevious release's fully-resolved
values.yamlforward, but never re-mergesthe new chart's
Chart.yaml.templates/deployment.yaml(andcronjob.yaml,pre-delete-cleanup-job.yaml) resolved the image tag asdefault .Chart.Version .Values.image.tag. Once any release ever publisheda chart where
image.tagresolved to a non-empty literal, every later--reuse-valuesupgrade kept that literal forever, regardless of what thenewly published chart's own default said -- so the operator, mirror, and
cluster-validator containers kept running a stale image even though
helm listand the chart NOTES reported the new chart/app version. This is theactive 3.2.x line QA is testing against.
What changed
deployment.yaml,cronjob.yaml,pre-delete-cleanup-job.yaml(nativesource chart and its vendored copy): image tag now falls back through
.Chart.AppVersioninstead of.Chart.Version.Chart.yaml(includingappVersion) is never part of a--reuse-valuesmerge, so this fallbackstays correct on every future upgrade, not just a fresh install.
values.yaml/ README doc comments updated to describe the new fallback.vendor_chart_image_tag_test.shstrengthened with a chart-version vs.appVersion divergence case and a
--reuse-valuesupgrade simulation.Testing
bash deploy/helm/nvca-operator/tests/vendor_chart_image_tag_test.shbash src/compute-plane-services/nvca/scripts/lint_helm.shBoth pass on this branch. No QA needed beyond the above.
References
None
Related Pull Requests
#1248 (main)