K8SPSMDB-1633 - protect CRDs from deletion via preserveCrds#901
K8SPSMDB-1633 - protect CRDs from deletion via preserveCrds#901DrFaust92 wants to merge 5 commits into
Conversation
Adds an opt-in `preserveCrds` value that stamps the `helm.sh/resource-policy: keep` annotation onto the psmdb CRDs so they (and every cluster stored under them) survive a `helm uninstall` of this chart -- a safety net against a GitOps tool accidentally pruning it. The CRD content-sync check strips the templated annotation before diffing, so the CRD content is still validated against the operator's crd.yaml. Closes percona#825 (psmdb variant) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
psmdb
operators
ct lint requires a chart version bump for the CRD-chart change, but the crd-sync-check pinned the crds chart version to the operator appVersion. Relax that rule so the crds chart version only needs to share the operator appVersion's major.minor and may carry its own patch bump (mirrors how the operator chart version floats above appVersion). The crds appVersion stays pinned to the operator, and CRD content is still validated independently by the content-sync diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| # `helm uninstall` of this chart. Set to true to guard against a GitOps tool | ||
| # (Flux, Argo CD, etc.) accidentally deleting the CRDs. See: | ||
| # https://helm.sh/docs/howto/charts_tips_and_tricks/#tell-helm-not-to-uninstall-a-resource | ||
| preserveCrds: false |
There was a problem hiding this comment.
i wonder if this should be true by default
There was a problem hiding this comment.
Agreed -- I've flipped the default to true (89c5871).
The failure modes are asymmetric: with false, an accidental helm uninstall / GitOps prune cascades through the CRDs to every PerconaServerMongoDB and its data (irreversible). With true, the worst case is orphaned, cluster-scoped CRDs left after a teardown (recoverable with kubectl delete crd). Safe-by-default is also the ecosystem norm here (cert-manager, prometheus-operator both keep CRDs). And since helm.sh/resource-policy: keep only affects delete/uninstall -- not upgrade -- flipping the default doesn't touch the templated-CRD upgrade path these live in templates/ for.
One caveat worth flagging: it's a behavior change on uninstall, so ephemeral/CI flows that uninstall+reinstall expecting a clean cluster will now leave CRDs behind. I documented the opt-out (preserveCrds: false) in the README/values for that case.
I'll make the PXC/PS/PG siblings match once the pattern here is settled so all four charts stay consistent.
Flip the preserveCrds default from false to true per maintainer review (egegunes). The failure mode of leaving CRDs deletable is an accidental `helm uninstall` / GitOps prune cascading through every cluster's data (irreversible); the failure mode of keeping them is orphaned, cluster-scoped CRDs after a teardown (recoverable with `kubectl delete crd`). Given that asymmetry -- and that this matches the wider ecosystem norm (cert-manager, prometheus-operator) -- safe-by-default is the better default. `helm.sh/resource-policy: keep` only affects delete/uninstall, not upgrade, so this does not touch the templated-CRD upgrade path. README and values docs updated to describe the new default and the opt-out (`preserveCrds: false`) for ephemeral/CI clusters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 participants
Summary
Resolves the psmdb part of #825 (K8SPSMDB-1633).
Adds an opt-in
preserveCrdsvalue to thepsmdb-operator-crdschart. When enabled, thehelm.sh/resource-policy: keepannotation is stamped onto all three CRDs, so ahelm uninstall(or a GitOps tool like Flux/Argo CD accidentally pruning the release) leaves the CRDs — and every cluster stored under them — in place instead of deleting them.Since these CRDs live in
templates/(so they can be upgraded), they are otherwise subject to deletion on uninstall — which, as the issue notes, can be devastating.Changes
values.yaml: newpreserveCrds: false(documented; disabled by default → no behavior change for existing users).helm.sh/resource-policy: keepintometadata.annotations.README.md: new "Protecting CRDs from Deletion" section + values table.psmdb-operator-crd-sync-check.yaml: the content-sync check strips the templated annotation before diffing, so the CRD content is still validated againstpsmdb-operator/crds/crd.yaml.Testing
helm template(default) → CRDs render identically to before (0 annotations).helm template --set preserveCrds=true→ all 3 CRDs get the annotation.helm lint→ 0 failures.Notes
Kept intentionally opt-in (
false) to avoid changing uninstall behavior for existing users. Happy to default it totrueif maintainers prefer safe-by-default. The sibling tickets (K8SPXC-1849 / K8SPS-677 / K8SPG-996) can follow the same pattern in separate PRs.This is the first standalone change to the
psmdb-operator-crdschart, which surfaced a conflict between two CI gates:ct lintrequires a chart version bump on any chart change.psmdb-operator-crd-sync-checkpinned the crds chartversionto equal the operatorappVersion.Together these made a standalone crds-chart change impossible to land. I resolved it by:
psmdb-operator-crdschartversion1.22.0 → 1.22.1(appVersion stays1.22.0, pinned to the operator).versiononly needs to share the operatorappVersion's major.minor and may carry its own patch bump — mirroring how the operator chart version already floats above its appVersion.appVersionremains strictly pinned, and CRD content is still validated byte-for-byte by the existing content-sync diff.Happy to adjust if you'd prefer a different versioning convention here.