fix: retry ClusterSummary status update in updateStatusForNonReferencedHelmReleases - #1937
Merged
Merged
Conversation
…edHelmReleases
In SyncModeContinuousWithDriftDetection, updateStatusForNonReferencedHelmReleases did a
single Get plus a single Status().Update with no retry, unlike its two siblings on the
same pass (updateStatusForReferencedHelmReleases, updateValueHashOnHelmChartSummary),
which both already wrap their Get+Update in retry.RetryOnConflict.
A concurrent status write (e.g. updateValueHashOnHelmChartSummary a few ms earlier) can
bump the ClusterSummary's resourceVersion between this function's cached Get and its own
Status().Update, so the write loses with:
Operation cannot be fulfilled on clustersummaries.config.projectsveltos.io "...":
the object has been modified; please apply your changes to the latest version and try again
handleCharts returns that error immediately, even though the Helm deploy already
succeeded. Consequences while this keeps happening:
- the feature hash never advances, so every reconcile re-runs a full no-op deploy pass
- the Helm feature reports Failed with consecutiveFailures climbing, while the release
itself is healthy and deployed
- drift detection registration (postProcessDeployedHelmCharts) is never reached, so
ResourceSummary.spec.chartResources stays empty and out-of-band drift goes undetected
This mostly hits profiles under frequent reconcile pressure (e.g. an HPA-autoscaled
Deployment keeps requesting reconciliation), since each failed pass's own status write
(consecutiveFailures) supplies the next conflicting write, making the loop
self-sustaining once triggered.
Fix: wrap the Get and Status().Update in updateStatusForNonReferencedHelmReleases in
retry.RetryOnConflict, matching its two siblings exactly.
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.
In SyncModeContinuousWithDriftDetection, updateStatusForNonReferencedHelmReleases did a single Get plus a single Status().Update with no retry, unlike its two siblings on the same pass (updateStatusForReferencedHelmReleases, updateValueHashOnHelmChartSummary), which both already wrap their Get+Update in retry.RetryOnConflict.
A concurrent status write (e.g. updateValueHashOnHelmChartSummary a few ms earlier) can bump the ClusterSummary's resourceVersion between this function's cached Get and its own Status().Update, so the write loses with:
handleCharts returns that error immediately, even though the Helm deploy already succeeded. Consequences while this keeps happening:
This mostly hits profiles under frequent reconcile pressure (e.g. an HPA-autoscaled Deployment keeps requesting reconciliation), since each failed pass's own status write (consecutiveFailures) supplies the next conflicting write, making the loop self-sustaining once triggered.
Fix: wrap the Get and Status().Update in updateStatusForNonReferencedHelmReleases in retry.RetryOnConflict, matching its two siblings exactly.
Fixes #1933