resource/helm_release: don't skip the uninstall when the release lookup fails - #1867
Open
bitte-ein-bit wants to merge 1 commit into
Open
resource/helm_release: don't skip the uninstall when the release lookup fails#1867bitte-ein-bit wants to merge 1 commit into
bitte-ein-bit wants to merge 1 commit into
Conversation
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
…up fails resourceReleaseExists reports false both when a release is genuinely absent and when the lookup against the cluster failed, telling the two apart only through the returned diagnostics. Delete acts on the boolean before appending those diagnostics, so a failed lookup is indistinguishable from "already uninstalled": the function returns early, the resource is removed from state, and the uninstall never runs. The release stays behind in the cluster with nothing tracking it. We ran into the underlying error during a short API server outage on AKS, where the same hiccup surfaced in the kubernetes provider as "net/http: TLS handshake timeout". Check the diagnostics first, which is what Create already does. The identical ordering issue in Read is being fixed in hashicorp#1734, so it is left alone here.
bitte-ein-bit
force-pushed
the
b-helm-release-read-transient-error
branch
from
July 31, 2026 13:41
c3e49ff to
ba81f82
Compare
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.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Changes to Security Controls
No changes to security controls.
Description
resourceReleaseExistsreturnsfalsein two very different situations: the release really is gone, and the lookup against the cluster failed. The two are only distinguishable through the diagnostics it returns alongside the boolean.Deleteacts on the boolean before appending those diagnostics, so the error is discarded:A failed lookup is therefore treated as "already uninstalled".
Deletereturns without an error, the framework removes the resource from state, and the uninstall never runs — the release is left behind in the cluster with nothing tracking it any more. Recovering means importing it again or cleaning up withhelmby hand.We ran into the underlying failure on AKS during a brief API server outage: the same hiccup surfaced in the
kubernetesprovider aswhich was retried and succeeded, while the
helmprovider swallowed it. It hit us on theReadpath rather thanDelete(a release was dropped from state and the next apply failed withcannot re-use a name that is still in use), butDeletehas the same ordering and a worse outcome, since the release is not just untracked but silently left running.This moves the diagnostics check ahead of the
existscheck, so a failed lookup surfaces as an error instead of being interpreted as absence.Createalready handles it in this order.Scope: the identical ordering issue in
Readis already addressed in #1734, so this PR deliberately leavesReadalone to avoid a conflicting change. #1734 does not touchDelete; the two are complementary and can merge in either order.Acceptance tests
No acceptance test: reproducing this requires the API server lookup to fail at a specific point during destroy, which the existing test harness has no hook for. Happy to add one if a maintainer can point at a suitable way to inject the failure.
Release Note
References
ReadCommunity Note