fix(networking): correct what disabledPackages does to ouroboros - #683
Merged
Aleksei Sviridkin (lexfrei) merged 3 commits intoSep 8, 2026
Merged
Conversation
Aleksei Sviridkin (lexfrei)
requested review from
Andrei Kvapil (kvaps),
Timofei Larkin (lllamnyp),
myasnikovdaniil and
Timur Tukaev (tym83)
as code owners
September 6, 2026 22:50
✅ 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 |
12 tasks
The host disable sequence offered bundles.disabledPackages as an alternative to `kubectl delete package.cozystack.io cozystack.ouroboros` and promised it reached the same place: helm uninstall, the chart's pre-delete hook, a patched kube-system/coredns. Neither half holds. The list is read in exactly one place, the platform chart's package helper, where it gates whether the Package document gets emitted at all. Every Package that is emitted carries an unconditional helm.sh/resource-policy: keep, so Helm leaves behind a Package it has stopped rendering. The live Package keeps the HelmRelease it owns, ouroboros keeps running, and the pre-delete hook never fires. The keep annotation was added in v1.1.5 precisely so that disabling a package stops deleting it, so the sentence described behaviour that had been gone for several releases. Deleting the Package CR remains the only step that reaches the hook, which the page already told operators to run. Same line in next, v1.4, v1.5 and v1.6. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
fix/disabled-packages-ouroboros
branch
from
September 6, 2026 22:52
483bfe6 to
f7df68a
Compare
12 tasks
IvanHunters
previously approved these changes
Sep 7, 2026
IvanHunters
left a comment
IvanHunters
left a comment
Contributor
There was a problem hiding this comment.
Verdict
LGTM with non-blocking notes
The correction is right and it is complete. disabledPackages is read in exactly three places in the platform chart, templates/_helpers.tpl:9 and :37 to decide whether the Package document is emitted at all, and templates/bundles/iaas.yaml:44 for the gpu gate. Nothing in the repository deletes a Package because its name appears in that list, and the helm.sh/resource-policy: keep annotation is there on purpose: the 1.1.5 and 1.2.1 changelogs record it being added so that moving a package into disabledPackages would stop deleting it. The sentence this PR removes was describing behaviour that was taken away deliberately, four minor releases ago.
Coverage is complete too. hairpin-proxy-protocol.md exists in four branches, next, v1.4, v1.5 and v1.6, and all four are patched identically. Nothing else under content/en/docs still promises that the list uninstalls anything.
One note, and it is about the page this one now agrees with rather than about this diff.
operations/configuration/components.md gives a different remedy for the same problem. It says to remove an already-installed component with kubectl delete hr -n <namespace> <component>, and for a platform package that does not stick: PackageReconciler is registered as For(&Package{}).Owns(&HelmRelease{}) in internal/operator/package_reconciler.go:968, so deleting the HelmRelease under a live Package is an ownership event that brings the HelmRelease straight back. That is exactly the trap this PR is closing on the hairpin page, one page over, and it is in all eight versioned copies of components.md. Worth a follow-up; nothing here needs to change for it.
Two smaller things I checked and left alone. The paragraph above the numbered list still says the render guard points the operator at kubectl delete package.cozystack.io cozystack.ouroboros, which stays accurate since it never mentioned the list. And the new sentence's claim that the HelmRelease survives alongside the Package is the same mechanism as the note above, so the two are consistent with each other.
Aleksei Sviridkin (lexfrei)
added a commit
to cozystack/cozystack
that referenced
this pull request
Sep 7, 2026
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does `bundles.disabledPackages` does not uninstall anything. Two comments in the platform chart said it does, offered next to `kubectl delete package.cozystack.io cozystack.ouroboros` as if the two were interchangeable. A name in that list only stops `_helpers.tpl` emitting the Package document, which is the same suppression you already get from leaving `publishing.proxyProtocol` false. Every emitted Package carries `helm.sh/resource-policy: keep`, and that annotation is there precisely so disabling a package stops deleting it, so Helm leaves the Package CR alone. The CR stays, the HelmRelease it owns stays, ouroboros keeps running, and the pre-delete hook never fires because nothing uninstalls the release. Deleting the Package CR is what gets there: GC takes the HelmRelease, Flux uninstalls, the hook runs on the way out. There was a second cost. The guard in the system bundle looks up the live Package CR and never reads `disabledPackages`, so an operator who followed the old advice hit the guard anyway with no idea why. New test case pins that. Comments only in the templates, so rendered output is unchanged. Chart suite green at 156 tests. ### Screenshots Not a UI change. <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> Walked the trigger map against the diff. One repo is reached, and I left its box empty rather than tick it without a link. `cozystack/website` has the same wrong instruction verbatim in three files: `content/en/docs/next/networking/hairpin-proxy-protocol.md:122`, and the identical line under `v1.4/` and `v1.5/`. It tells the reader that removing the Package CR "(or add it to `bundles.disabledPackages`)" triggers helm uninstall and fires the pre-delete hook. It also contradicts `operations/configuration/components.md:72` on the same site, which says `disabledPackages` will not remove components that are already installed. The site fix is cozystack/website#683. It turned out to be four files rather than three: `origin/main` there also carries a `v1.6` tree, and all four copies were byte-identical. Nothing else is reached: no package added or renamed, no `values.schema.json`, no version enum, no default, no key the Ansible role sets, nothing under `hack/`. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#683 - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(platform): the platform chart no longer says `bundles.disabledPackages` uninstalls a package. A name in that list only stops the Package document being rendered. `helm.sh/resource-policy: keep` leaves the existing Package CR on the cluster and the component keeps running, so deleting the Package CR is what uninstalls it. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified proxy protocol safety guidance, including the effects of disabling the Ouroboros package and the limitations of using `bundles.disabledPackages`. - Documented that disabling package rendering does not remove an existing live package or stop its associated resources. - **Tests** - Added regression coverage confirming that disabling the package does not bypass the proxy protocol safety guard when the live package remains active. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
The English home page gained a Live Demo block; the four translations did not, so their recorded source digest no longer matched and the freshness guard failed on every pull request against main. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
dismissed
IvanHunters’s stale review
via
September 7, 2026 22:59
ce6302a
IvanHunters
approved these changes
Sep 8, 2026
Aleksei Sviridkin (lexfrei)
deleted the
fix/disabled-packages-ouroboros
branch
September 8, 2026 08:06
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
2 participants
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.
The disable sequence said an operator reaches the ouroboros cleanup hook either by deleting the Package CR or by putting the name in
bundles.disabledPackages. The second one does not work.bundles.disabledPackagesis read in one place, the platform chart's package helper, and all it does there is decide whether the Package document gets emitted. Every emitted Package carries an unconditionalhelm.sh/resource-policy: keep, so Helm leaves behind a Package it has stopped rendering. The Package stays, so does the HelmRelease it owns, ouroboros keeps running, and the pre-delete hook that strips the rewrite block fromkube-system/corednsnever fires. Follow the old wording and the disable looks clean while the rewrite is still live.That annotation landed in v1.1.5 so that disabling a package stops deleting it, several releases before the oldest version this page exists in.
cozystack/cozystack#4098 fixes the same claim in the platform values and the render guard message.
hugo --gc --minifybuilds clean.hack/check-i18n.shexits 1, but the four stale-translation errors are oncontent/{de,hi,ru,zh-cn}/_index.mdand are already on main, so i18n-lint will be red for something this PR did not touch.