fix(operations): replace the component removal recipe that undoes itself - #684
Open
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Open
fix(operations): replace the component removal recipe that undoes itself#684Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Conversation
Aleksei Sviridkin (lexfrei)
requested review from
Andrei Kvapil (kvaps),
Timofei Larkin (lllamnyp),
myasnikovdaniil and
Timur Tukaev (tym83)
as code owners
September 6, 2026 23:16
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 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 |
Aleksei Sviridkin (lexfrei)
force-pushed
the
fix/component-removal-recipe
branch
5 times, most recently
from
September 7, 2026 00:10
41cf622 to
003e787
Compare
Removing an installed component was documented as `kubectl delete hr -n <namespace> <component>`. The Package that emitted the release owns it through a controller ownerReference, and the operator watches HelmReleases with Owns(), so the delete fires the reconcile that recreates it. The command reports success, nothing logs an error, and the component is still running. Deleting the cluster-scoped Package is what cascades into the uninstall. It needs the disabledPackages entry to land first, or the next platform upgrade renders the Package again and the removal undoes itself one level up. That ordering is stated rather than checked, because no command discriminates it. A condition wait on the platform HelmRelease returns instantly against the Ready left standing by the previous reconcile, the trap hack/e2e-install-cozystack.bats already refuses for the same reason, and re-reading the Package after deleting it returns NotFound unconditionally, since only the platform chart renders Packages and no driftDetection is configured. A check that cannot fail is worse than prose, so the page says what the order buys and admits the failure is silent. Making the removal stick arms a data loss the broken recipe masked, so the page states it. CRDs that a chart renders as ordinary templates and does not annotate resource-policy: keep go when the release is uninstalled, and the apiserver cascades every custom resource of those kinds. Removing cozystack.metallb takes the metallb.io CRDs and with them every IPAddressPool, L2Advertisement and BGPPeer on the cluster. The annotation is the whole condition: cert-manager-crds sets crds.keep and survives the same uninstall. Applied to next and v1.1 through v1.6. Left out v1.0, whose line never received the resource-policy: keep annotation, so disabledPackages alone still prunes the Package there and that page needs its own correction. v0 predates Package objects. The v1.1 disabledPackages example listed bare names, which match nothing against the fully-qualified names the platform chart feeds the helper. The recipe now routes readers through that list, so the example has to work. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
fix/component-removal-recipe
branch
from
September 7, 2026 00:19
003e787 to
f2f6925
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.
kubectl delete hr -n <namespace> <component>is documented as the way to remove an installed component. It removes nothing. The Package that produced the release owns the HelmRelease (package_reconciler.go:290) and the operator watches them withOwns(&helmv2.HelmRelease{}), so the delete fires the reconcile that puts it back. The command reports success, nothing logs an error, and the component keeps running.Deleting the Package is what cascades into the uninstall. The
disabledPackagesentry has to land first, or the next platform upgrade renders the Package again and the removal undoes itself one level up. That ordering is stated rather than checked, because no command discriminates it. A condition wait on the platform HelmRelease returns instantly against the Ready left standing by the previous reconcile, whichhack/e2e-install-cozystack.batsrefuses for that same reason; re-reading the Package after deleting it returns NotFound unconditionally, since only the platform chart renders Packages and nothing setsdriftDetection. I tried both and dropped both. A check that cannot fail is worse than prose, so the page says what the order buys and admits the failure is silent.Making removal stick arms a data loss the broken recipe was hiding, so the page now states it.
packages/system/metallb/charts/metallb/charts/crds/templates/crds.yamlrenders its CRDs as ordinary subchart templates with noresource-policyannotation and that chart has no top-levelcrds/directory, so helm uninstall deletes them and the apiserver cascades every IPAddressPool, L2Advertisement and BGPPeer along with them. The annotation is the whole condition rather than template placement:cert-manager-crdssetscrds.keepand its CRDs survive the same uninstall, which a helm-unittest suite in that package already pins. The page names no CRD count, since metallb ships eight through v1.4 and nine from v1.5.v1.0 is left out on purpose. That line never got the
helm.sh/resource-policy: keepannotation, sodisabledPackagesalone still prunes the Package there and the page needs the opposite correction. v0 predates Package objects.v1.1 also had bare
metallb/hetzner-robotlbin itsdisabledPackagesexample, which match nothing against the fully-qualified names the chart feeds the helper. The recipe sends readers to that list now, so the example has to work.One known gap:
keepreached v1.1 and v1.2 mid-line, in v1.1.5 and v1.2.1, so on earlier patches of those two minorsdisabledPackagesprunes the Package on its own. Same end state, and the docs are scoped per minor, so those pages describe their line at its final patch.hugo --gc --minifybuilds clean.hack/check-i18n.shexits 1 on four stale translations undercontent/{de,hi,ru,zh-cn}/_index.md, already on main and untouched here, so i18n-lint goes red for something this PR did not cause.Not taken:
cozypkg delresolves dependents and deletes in reverse topological order, which rawkubectl deletedoes not, but whether that subcommand exists varies across the seven version trees and I did not want a second version-dependent fact in one paragraph.