Extend HelmRelease with CRD update support

[rustrial]

Helm still does not provide any built-in solution to the CRD upgrade problem, which is well-known and
documented:

There is no support at this time for upgrading or deleting CRDs using Helm. This was an explicit decision after much community discussion due to the danger for unintentional data loss. Furthermore, there is currently no community consensus around how to handle CRDs and their lifecycle. As this evolves, Helm will add support for those use cases.

This limitation of Helm makes GitOps style "day 2 operations" of Kubernetes resources managed by
HelmRelease objects very difficult if not to say impossible. Currently, we have to manually
upgrade CRDs from HelmCharts referenced by HelmReleases, which is cumbersome and needs very
tight (timing) coordination between the commit to the GitOps repository and the manual CRD
upgrade on all systems that observe/apply that commit. This apporach is not only very error
prone it also might cause unnecessary long downtime of services.

Note, most of the Helm Charts that we install and operate are created and maintained by
3rd parties and are not under our controll. Extracting all CRDs from every Helm Chart
upon each new release and manually applying those CRD resources is, as mentioned above,
very time intensive, error prone and cumbersome.

Our observation is, that most CRD upgrades are non-critical and only evolve an existing CRD
in a backward compatible fashion. Therefore, I am wondering whether it would make sense to
extends the HelmRelease resources with an opt-in flag to automatically upgrade CRD objects
(if needed).

For this we could extend the helm-controller to do something like this if the opt-in flag on a
HelmRelease is set to true:

1. Read all CRD resources into a buffer, this can be achieved with the help
   of the CRDs()
   func on Chart.
2. Apply the buffer into the cluster (e.g. using strategicMergePatch approach) or
   kubectl.

What do you think?

[rustrial]

Some ideas/questions on how we could map this to the Helm actions. Note, to goal is not to find a perfect solution that covers all use-cases (as this is practically impossible due to the way that CRDs are defined as all versions of the same CRD must be defined in a single CRD object). My intention is to find a solution which covers >80% of real-world use cases, in which CRDs are evolved carefully with overlapping versions and clear deprecation policies for older versions.

• install: already covered by Helm (users can opt-out by setting SkipCRDs to true)
• upgrade: could be implemented based on https://github.com/helm/helm/blob/v3.5.4/pkg/action/install.go#L127 (see proof of concept below) and users can opt-in by setting UpgradeCRDs to true. Note, this will not delete any CRDs, it will just add new and replace existing CRDs.
• rollback: this one is tricky and I can imagine different options:
  • Assuming that CRDs can only evolve in backward compatible manner (semantic versioning like) we might get away (for most use-cases) by just not doing anything. So we never "downgrade" CRDs.
  • We find a way to obtain the *chart.Chart object of the former helm release (the one to which helm will rollback to) and we just call the same upgrade function as the upgrade action would use with the CRDs from that *chart.Chart object. Basically, we would implement downgrade in terms of the upgrade function. User could opt-in to this behaviour by setting DowngradeCRDs to true.
• delete: delete all CRDs with matching meta.helm.sh/release-name and meta.helm.sh/release-namespace annotations if user opts-in by setting DeleteCRDs to true.

Prof-of-concept CRDs upgrade function

// This has been adapte from https://github.com/helm/helm/blob/v3.5.4/pkg/action/install.go#L127
func (r *Runner) upgradeCRDs(crds []chart.CRD) error {
	cfg := r.config
	// We do these one file at a time in the order they were read.
	totalItems := []*resource.Info{}
	for _, obj := range crds {
		// Read in the resources
		res, err := cfg.KubeClient.Build(bytes.NewBuffer(obj.File.Data), false)
		if err != nil {
			return errors.New(fmt.Sprintf("failed to upgrade CRD %s: %s", obj.Name, err))
		}
		// Obtain the originals. Note, we build the originals from the current set of CRDs
		// and therefore this upgrade will never delete CRDs that existed in the former release
		// but no longer exist in the current release.
		original := make(kube.ResourceList, 0)
		for _, r := range res {
			err := r.Get()
			if err == nil {
				original = append(original, r)
			} else if !apierrors.IsNotFound(err) {
				return err
			}
		}
		// Send them to Kube
		if _, err := cfg.KubeClient.Update(original, res, true); err != nil {
			return errors.New(fmt.Sprintf("failed to upgrade CRD %s: %s", obj.Name, err))
		}
		totalItems = append(totalItems, res...)
	}
	if len(totalItems) > 0 {
		// Invalidate the local cache, since it will not have the new CRDs
		// present.
		discoveryClient, err := cfg.RESTClientGetter.ToDiscoveryClient()
		if err != nil {
			return err
		}
		cfg.Log("Clearing discovery cache")
		discoveryClient.Invalidate()
		// Give time for the CRD to be recognized.
		if err := cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil {
			return err
		}
		// Make sure to force a rebuild of the cache.
		discoveryClient.ServerGroups()
	}
	return nil
}

[hiddeco]

Can see how this can benefit users, and am open to a contribution that implements the basis of what is described here (without support for rollbacks) as a test pilot.

[stefanprodan]

I'm for implementing this in stages. I think first we could introduce UpgradeCRDs as proposed here and keep ignoring the CRDs on rollback. Then based on user feedback we could decide what to do in regards to rollback.

[rustrial]

Thanks for the feedback. I will open a (WIP) PR for UpgradeCRDs.

[stefanprodan]

Thanks @rustrial

[alex-berger]

PR fluxcd/helm-controller#250 has landed, review highly appreciated.