From 343cb9f829fadef6afcc3f6bc5ae8b94db29cf2c Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Fri, 20 Dec 2024 20:47:37 -0800 Subject: [PATCH 1/2] Initial draft Signed-off-by: George Jenkins --- hips/hip-XXXX.md | 157 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 hips/hip-XXXX.md diff --git a/hips/hip-XXXX.md b/hips/hip-XXXX.md new file mode 100644 index 00000000..38abfa6e --- /dev/null +++ b/hips/hip-XXXX.md @@ -0,0 +1,157 @@ +--- +hip: "9999" +title: "Helm CRD updating" +authors: [ "George Jenkins " ] +created: "2024-12-19" +type: "feature" +status: "draft" +helm-version: "4" +--- + +## Abstract + + + +Helm has historically taken a conservative approach to Custom Resource Definitions (CRDs). +The concerns detailed in [HIP-0011](./hip-0011.md). + +This HIP aims to improve Helm's support for CRD management, while remaining in line with the rationales in `HIP-0011`. +Ensuring chart consumers can continue to safely operate charts that provide CRDs. +But extending Helm's CRD management to enable updates to CRDs. + + +## Motivation + + + +This HIP aims to improve the experience for chart authors and operators by extending Helm's ability to manage CRDs. +Helm's conservative approach to date has limited the abilty for Helm's users (both authors and chart operators) to package and use Kubernetes applications that make use of CRDs. +Today, Helm will only install CRDs (from the `crds/` directory) when installing a chart. +CRDs are otherwise ignored for additional operations, including upgrades. + +CRDs have become more commonly utilized to extend Kubernetes functionality beyond its core primatives. +Being conservative ensured Helm erred on the side of safety and didn't create an irreversible position for Helm nor its users. +But has limited Helm's users, both chart operators and authors, to produce charts that expect to be able to manage CRDs. +As such, improvements to Helm's CRD management, that allows Helm and charts to better manage and update CRDs are desired. + +The solution here aims to extend Helm's CRD management, while adhering to the concerns detailed in [HIP-0011](./hip-0011.md). +While providing for better CRD management, but continuing to provide for safe operation of charts by chart operators. + + +## Rationale + + +The two main reasons inferred from [HIP-0011](./hip-0011.md) that Helm is conservative with CRDs are: +1. CRDs are a cluster-wide resource. + Changes to cluster wide resources can (more easily) break applications beyond the scope of the chart +2. Custom Resources (CRs) can be used as a data store. + And removing the CRDs for those will irrevocably remove any CRs and cause parmenat loss of any contained data + +For 1., it is thought that Helm should not treat CRDs specially here. +Helm will readily operate on many other cluster wide resources today: Cluster roles, priority classes, namespaces, etc. +That the modification/removal of could easily cause breakage outside of the chart's release. + +In general, Helm as a package manager should not try to prempt unintended functional changes from a chart. +Validating functional changes is well beyond the scope of a package manager. +Helm should treat CRDs the same as other (cluster-wide) resources, where a Helm chart upgrade that causes unintended functional effects should be reverted (rolled back) by the user (chart operator). +And as long as that rollback path exists, this is a suitable path for users to mitigate breaking functional changes. + +For 2., Data loss should be treated more carefully. +As data loss can be irrevocable or require significant effort to restore. +And especially, an application/chart operator should not expect a chart upgrade to cause data loss. +Helm can prevent Data loss can be prevented by ensuring CRDs are "appended" only (with special exceptions in the specification below). An in particular, appending allows a rollback to (effectively) restore existing cluster state. +(It should also be noted that Helm will today remove other resources which may cause data loss e.g. secrets, config maps, namespace, etc. A special, hard-coded, exception does exist for persistent volumes) + + +## Specification + + + +_Note: This specification applies to CRDs in a chart's `crds/` directory only. CRDs deployed via `templates/` are out of scope. As Helm only considered CRDs released via the `crds/` directory to be managed by Helm_ + +When installing for upgrading, Helm will install new CRDs from the `crds/` directory if they don't exist in the cluster. +If the CRD already exists, Helm will "append" or merge CRD updates into the cluster's existing CRD object with the logic detailed below. + +When rolling back, Helm will simply revert merge in the previously installed version. Effectively, this is reverting to the prior "storage" version only. + +When uninstalling: Helm will leave CRDs as is + +The append/merge logic is as follows. When updsting an existing CRD resource, Helm will: + +- merge/update CRDs metadata (labels, annotations etc) +- merge/update `/spec/names` +- append new versions to the `/versions` list +- update existing versions with the exception of the schema field + - If in the future, functionality exists that shows a CRD version schema changes are backwards compatible, Helm may allow updating CRD version's schema field + - In particular, Helm will expect the update from the chart to correctly set a single version to `storage: true` +- merge/update `/conversion` field +- set `/preserveUnknownFields` if the incoming or existing preserveUnknownFields values is set (logical OR) + +If in the future, Kubernetes introduces new fields to the CRD API, Helm will ignore these fields by default. +Helm will consider introducing new logic to update any new new field that follows the rationale section as appropriate. + +If the `--skip-crds` flag is supplied on install/upgrade/rollback, similar to today, Helm will ignore CRD changes. + +## Backwards compatibility + +The adjustment in CRD management behavior will be visible to end users. +Chart upgrades which previously ignored CRD changes will now action them. +Which may cause user facing behaviour + +These behavioural changes are consider to acceptable for Helm 4. + +## Security implications + +None determined. + +## How to teach this + +Helm's CRD handling docs need to be updated: + +In particular, the merge logic described will need to be documented/described. + +## Reference implementation + + + +## Rejected ideas + + + +### Deleting CRDs / versions + +In order to prevent data loss, this HIP just considered how to update CRDs. +And did not consider how to remove CRD versions nor CRDs themselves. + +## Open issues + + + +## References + + + +- [HIP-0011: CRD Handling in Helm 3](./hip-0011.md) +- Kubernetes CRD docuemntation: + - + - + - From 836107bc4d64365f1cd6a55058e7674dce3d8633 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Sat, 1 Feb 2025 20:42:37 -0800 Subject: [PATCH 2/2] review Signed-off-by: George Jenkins --- hips/hip-XXXX.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hips/hip-XXXX.md b/hips/hip-XXXX.md index 38abfa6e..fd4c88a5 100644 --- a/hips/hip-XXXX.md +++ b/hips/hip-XXXX.md @@ -49,8 +49,9 @@ While providing for better CRD management, but continuing to provide for safe op Describe why particular design decisions were made. --> The two main reasons inferred from [HIP-0011](./hip-0011.md) that Helm is conservative with CRDs are: -1. CRDs are a cluster-wide resource. - Changes to cluster wide resources can (more easily) break applications beyond the scope of the chart +1. CRDs are a cluster-wide resource, and cluster-wide resources can more easily affect all users of a cluster. + Rather than just the application / chart the operator is deploying, as operators and applications are often scoped to a single namespace. + Frequently user That may affect multiple users within the cluster. 2. Custom Resources (CRs) can be used as a data store. And removing the CRDs for those will irrevocably remove any CRs and cause parmenat loss of any contained data @@ -67,7 +68,6 @@ For 2., Data loss should be treated more carefully. As data loss can be irrevocable or require significant effort to restore. And especially, an application/chart operator should not expect a chart upgrade to cause data loss. Helm can prevent Data loss can be prevented by ensuring CRDs are "appended" only (with special exceptions in the specification below). An in particular, appending allows a rollback to (effectively) restore existing cluster state. -(It should also be noted that Helm will today remove other resources which may cause data loss e.g. secrets, config maps, namespace, etc. A special, hard-coded, exception does exist for persistent volumes) ## Specification