Flux: Flux Helm Chart CRD deployment marked as keep is throwing reinstalls off

[isen-ng]

Describe the bug
When I try to re-install flux using helm charts (on a deployment that I have flux deleted), I get this error:

release flux failed: customresourcedefinitions.apiextensions.k8s.io "helmreleases.flux.weave.works" already exists

This is due to this change: https://github.com/weaveworks/flux/pull/1543

I'm not exactly sure of the technical details behind this, but since this was added, I assume that this is required for flux deletion to work properly. However, marking a helm release as "keep" will bring problems when the release is being installed again after deletion. This can manifest itself when trying to provision/re-provision a cluster using terraform-helm-provider (I would really like to use ansible for this, but terraform has horrible ansible support, and ansible has horrible helm support).

Helm says this:

The annotation "helm.sh/resource-policy": keep instructs Tiller to skip this resource during a helm delete operation. However, this resource becomes orphaned. Helm will no longer manage it in any way. This can lead to problems if using helm install --replace on a release that has already been deleted, but has kept resources.

On your introduction page, it recommends that the crd is not automatically created by the chart and be created separately through kubectl with a url to an outdated crd yaml (at least when compared to the crd yaml in the chart itself).

To Reproduce
Steps to reproduce the behaviour:

1. Add flux repo helm repo add weaveworks https://weaveworks.github.io/flux
2. Install flux with auto crd

$ helm install --name flux \
--set [email protected]:weaveworks/flux-get-started \
--set helmOperator.create=true \
--set helmOperator.createCRD=true \
--namespace flux \
weaveworks/flux

1. Delete flux: helm delete --purge flux
2. Install flux again

$ helm install --name flux \
--set [email protected]:weaveworks/flux-get-started \
--set helmOperator.create=true \
--set helmOperator.createCRD=true \
--namespace flux \
weaveworks/flux

Expected behavior
It should be easy to re-install flux

Proposal
There should be a separate chart to install the updated CRDs, helmOperator.createCRD should now default to false and the readme updated with new instructions to release new CRD chart first (readme should also caution users to delete flux release completely before releasing CRD)

Do let me know if you want a PR for this (since I'm going to create the separate CRD chart on my own anyway)

[stefanprodan]

In our install docs we recommend the CRD to be deploying using kubectl apply https://github.com/weaveworks/flux/blob/master/site/helm-get-started.md#install-weave-flux

Moving the CRD to it's own chart will not solve the problem since Helm can't handle CRD updates, when the CRD struct changes Helm will error out see https://github.com/helm/helm/issues/5853

Since Helm doesn't implement a 3-way merge, there is no way to ensure CRDs will be updatable. Please use kubectl apply -f https://raw.githubusercontent.com/weaveworks/flux/master/deploy-helm/flux-helm-release-crd.yaml and --set helmOperator.createCRD=false until Helm v3 supports managing CRDs lifecycle https://github.com/helm/community/pull/64

[isen-ng]

I realised that https://raw.githubusercontent.com/weaveworks/flux/master/deploy-helm/flux-helm-release-crd.yaml and https://github.com/weaveworks/flux/blob/master/chart/flux/templates/helm-operator-crd.yaml are different.

The one in the chart has an additional resource, fluxhelmreleases.helm.integrations.flux.weave.works.

Which one is the correct one?

---

until Helm v3 supports managing CRDs lifecycle helm/community#64

It doesn't seem like helm v3 is going GA anytime soon (and this feature is still in the discussion stage). Wouldn't it be more prudent to set the default value of helmOperator.createCRD to false to avoid such issues?

Personally, I would remove this CRD in the chart altogether and force users to create the CRD using kubectl to remove confusion around this.

Eg, Remove the confusion around the situation where it still works even if I don't follow the recommended installation steps (making me question the authenticity/updatedness of the recommendation) and then running into problems later due to this (after already diving knee deep)

(it also makes me wonder why the recommendation is to divert away from using default values, which makes it sound like a hack or a workaround until something better comes along)

[stefanprodan]

The defaults don't match the recommended install due to historical reasons. When we first released the chart there was no "helm.sh/resource-policy": keep annotation in place and Helm would delete the CRD when upgrading. To avoid this for our early adopters we decided to keep the createCRD=true.

[stefanprodan]

We should consider setting createCRD=false and removing the old fluxhelmreleases CRD from the chart for the 1.0 release of Helm Operator cc @hiddeco

[starkers]

Maybe just have a pattern similar to the istio-init chart (which is only ever used as helm template)?

[stefanprodan]

@starkers that doesn't make sense for us since we don't have 20+ CRDs like Istio. Also the istio-init is not used only with template, that chart packages the CRDs into 3 jobs. Here is how you can use Helm to deploy the Istio CRDs https://github.com/weaveworks/flagger/blob/master/test/e2e-istio.sh#L16

[starkers]

Agreed.. I frankly just use something like this:

GITHUB_TAG=helm-0.9.2
kubectl apply -f https://raw.githubusercontent.com/weaveworks/flux/${GITHUB_TAG}/deploy-helm/flux-helm-release-crd.yaml