[Backport release-1.6] fix(kubernetes): retry the tenant CNI install instead of uninstalling it - #4033
Merged
Conversation
The tenant cilium HelmRelease carried no install strategy, so Flux applied its default, RemediateOnFailure, which uninstalls the release between attempts. Cilium is the tenant cluster's only CNI, so every cycle takes it away and gives it back when the next install applies the manifests again, and retries: -1 lets that repeat without end. What sets a cycle off is an install that outruns its budget, which can happen while the tenant still has no registered node, since the operator Deployment cannot become Available until one exists. Remediation neither causes that delay nor is fixed by this change; what it adds is a repeated teardown of the CNI on top of a delay that resolves itself once the node arrives. This removes the amplifier, not the cause. RetryOnFailure keeps the applied manifests in place and retries a failed install as an upgrade, so the rollout completes on its own once a worker registers. Both actions carry the strategy because the retry of a failed install runs as an upgrade, which would otherwise fall back to upgrade remediation. It is the strategy and retry interval cozystack-api and cozystack-operator already set on every HelmRelease they generate, except that they leave remediation nil where this release keeps it. The other tenant addons the chart renders still carry the remediation-only shape; this one is where the teardown removes the cluster's only CNI rather than churning a single component. The cost is that a genuine upgrade failure no longer rolls back either, it retries on the failed manifests instead. That is forced, since the controller cannot tell a retried install from a real upgrade, and what it replaces was an unbounded rollback-and-retry flap rather than a stable safety net. The remediation blocks stay, since they cover the branches no strategy reaches: an absent release, which consults the install block, and an out-of-sync one, which consults upgrade. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]> (cherry picked from commit 80a2090) Signed-off-by: Myasnikov Daniil <[email protected]>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Backport of #3552 to
release-1.6.Tenant cilium HelmRelease on this branch sets
install.remediation.retries: -1with nostrategy:, so helm-controller uses its default install remediation, which is uninstall. Cilium is the tenant cluster's only CNI, so a tenant whose install misses its budget gets the CNI torn down and reinstalled once per cycle, and-1lets that repeat without end.RetryOnFailurekeeps applied manifests in place and retries the failed install as an upgrade instead. Strategy is set on both actions because the retry of a failed install runs as an upgrade.Shipped broken in v1.6.0, v1.6.1 and v1.6.2.
RetryOnFailureappears in none of the 19 helmrelease templates on this branch, and the Flux CRD release-1.6 already ships accepts it - enum atspec.install.strategy.nameandspec.upgrade.strategy.name, with theretryIntervalCEL guard.Cost is carried over from the original PR: a genuine upgrade failure now retries on failed manifests instead of rolling back. Controller can't tell a retried install from a real upgrade, and what it replaces was an unbounded rollback-and-retry flap.
Auto backport did not apply here, and it was not a conflict. Bot picked four commits from the PR branch, three of them empty
ci: re-runcommits, andgit cherry-pickexits non-zero on an empty pick with no unmerged paths, which the action reads as a conflict. Picking80a209044alone is the whole backport and applies with a line offset only. Resulting tree matches the bot's four-SHA-with-three-skips recipe byte for byte.Testing
make testinpackages/apps/kubernetes- 18 suites, 186 tests green.install.strategy.nametoRollbackreds exactly the install test, removing theupgrade.strategyblock reds exactly the upgrade test, other 185 unaffected both times.make unit-testsandmake test-controllersgreen,make generateleaves no drift.targetVersionstill 54.