Repository navigation
fix: skip --reuse-values for a release that is not installed (#481) - #1086
Merged
yxxhero merged 2 commits intoOct 8, 2026
Merged
Conversation
…23#481) With --install/--allow-unreleased and an explicit --reuse-values or --reset-then-reuse-values, helm-diff still ran `helm get values` for a release that does not exist and failed with "Failed to render chart: exit status 1". Only the implicit reuse of values was limited to upgrades. `helm upgrade --install` ignores both flags when it installs, so do the same and only fetch the existing values when the release exists.
The new condition in template() moved isUpgrade out of shouldDefaultReusingValues, but no case exercised the two boundaries of that rewrite: - an existing release without any --set/--values flags still fetches the release values (helm enables --reuse-values by default), and - a new install without any value flags must not fetch them. Add both cases to TestUpgradeCommand_Execution_ReuseValues. Signed-off-by: yxxhero <aiopsclub@163.com>
yxxhero
approved these changes
Oct 8, 2026
yxxhero
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the change in depth against all call paths of template() — the fix is correct. Summary of the review, plus one small commit added on top.
Why it's correct
template(isUpgrade)is called only fromrunHelm3()asd.template(!newInstall), andnewInstallbecomestrueonly afterhelm get manifestfails withrelease: not foundwhile--allow-unreleased/--installis set. So gating the whole reuse block onisUpgrademeans exactly "the release exists", which mirrors whathelm upgrade --installdoes (ignore reuse flags for a new install).- For an existing release (
isUpgrade == true) the new condition is equivalent to the old one, so behavior there is unchanged. --dry-run=client/--dry-run=truepaths remain safe:clusterAccessAllowed()is false there, so the block was already skipped.--revision > 0on a missing release still errors out beforetemplate(), unchanged.- The
unreleasedfake-helm mode failshelm getwithError: release: not found, which is whatoutputWithRichErrorsurfaces andrunHelm3matches on — consistent with real helm. The only othergetconsumer (getChartinrelease.go) is not part of the upgrade flow. - The dropped
--reset-then-reuse-valuesversion error for a missing release (helm < 3.14) is fine: the flag is ignored for a new install, so erroring about it would be inconsistent.
Commit added on top (08f8526): the rewrite moved isUpgrade out of shouldDefaultReusingValues, but no case exercised the two boundaries of that exact condition:
- existing release without any value flags → implicit default reuse must still fetch
helm get values --all(guardsshouldDefaultReusingValuesagainst being dropped from the condition); - new install without any value flags (bare
--install, the most common new-release invocation) → must not fetch values.
Both cases are in TestUpgradeCommand_Execution_ReuseValues now. PR description case counts updated accordingly.
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
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.
helm diff upgrade --install --reuse-valuesfails withError: Failed to render chart: exit status 1when the release does not exist yet. The same happens with--allow-unreleasedand with--reset-then-reuse-values.helm upgrade --install --reuse-valuesworks in that situation.Cause:
template()incmd/helm.gorunshelm get valueswhenever one of the two flags is set. Only the implicit reuse of values (no--set/--valuesgiven) has been limited to upgrades since a25763a. For a new install there is no release, sohelm get valuesexits withrelease: not foundand the render is aborted.Fix: only fetch the existing values when the release exists. For a new install the two flags are ignored, which is what
helm upgrade --installdoes. Nothing changes for a release that exists.Tests:
TestUpgradeCommand_Execution_ReuseValuesruns the upgrade command against the fake helm. A newunreleasedfake helm mode makes everyhelm getfail withrelease: not found. Six cases for a missing release fail on master with the error from the issue and pass with this change. They also check thathelm get valuesis not called. Four cases for an existing release check that it still is.--installor--allow-unreleasedwith--reuse-valuesor--reset-then-reuse-valuesfails, also together with--dry-run=server,--three-way-mergeandHELM_DIFF_USE_UPGRADE_DRY_RUN=true. With this change each of them prints the manifest as added. For an installed release,--reuse-values --set other=changedgives the same diff before and after, with the previously set value kept.make format,make lint,make testandmake verify-readmepass, and golangci-lint v2.14.0 reports 0 issues.Not tested: a kind cluster, so the CI integration job has not been run locally. I did not add a
scripts/issues/481.sh; I can add one if you want it.Two things this does not change or changes on purpose:
--reset-then-reuse-valueson a release that does not exist no longer returns the "requires at least helm version 3.14.0" error, because the flag is now ignored for a new install.--reuse-valuesthrough tohelm upgradewhenHELM_DIFF_USE_UPGRADE_DRY_RUNis set. That is not part of this PR. In that mode the values of an existing release are still read withhelm get values, as before.Fixes #481