helm-controller icon indicating copy to clipboard operation
helm-controller copied to clipboard

Prevent execution of helm uninstall during deletion of helmchart object

Open Martin-Weiss opened this issue 4 years ago • 4 comments

We are moving helmchart / helm-controller based installations into fleet and due to that we have to delete the existing helmchart objects. Unfortunately the deletion of an helmchart object causes the deployment to be deleted, too which is not what we want. We want to keep the installed deployments as the "uninstall" of CRDs or longhorn will lead to data-loss.

-> how can we prevent the finalizer on a helmchart object to be executed causing CRDs and longhorn to be uninstalled?

Martin-Weiss avatar Nov 30 '21 10:11 Martin-Weiss

There is not currently a good way to release a Helm chart from management by the Helm controller. Simply removing all the chart manifests from the servers so that they do not ever get updated again is probably the easiest thing to do at the moment; if the HelmChart and HelmChartConfig CRs don't get updated, the chart will never be updated either.

Since just removing the finalizer doesn't appear to prevent the controller from processing the deletion, we could probably add support for an annotation on the HelmChart resource that would tell the controller to skip processing it when it is changed. The process would be to set that annotation, then delete the HelmChart CR.

brandond avatar Jan 14 '22 10:01 brandond

Thanks for the response - so leaving the orphaned helmchart CRD objects in kube-system will never cause the helm operator to do any action on them, anymore even in case there are upgrades?

FYI - we had the issue that during RKE2 upgrade 1.20.8 to 1.20.12 the applications installed via helmchart CRD were uninstalled and re-installed even though we did NOT change anything in manifests/* nor on the CRDs..

Martin-Weiss avatar Jan 14 '22 10:01 Martin-Weiss

RKE2 rewrites HelmCharts in the manifests directory at startup to inject a number of cluster configuration variables for use as chart variables. Even if you don't personally change anything in those manifests, if they are in that directory they will get touched to sync the variables: https://github.com/rancher/rke2/blob/3edb2f77b9e46c9e04427a467ee3403be576c5ec/pkg/bootstrap/bootstrap.go#L248-L264

If you simply delete the manifests from all the servers, absent direct user action there will not be any changes to the deployed HelmChart CRs that would cause an update to the chart.

brandond avatar Jan 14 '22 10:01 brandond

RKE2 rewrites HelmCharts in the manifests directory at startup to inject a number of cluster configuration variables for use as chart variables. Even if you don't personally change anything in those manifests, if they are in that directory they will get touched to sync the variables: https://github.com/rancher/rke2/blob/3edb2f77b9e46c9e04427a467ee3403be576c5ec/pkg/bootstrap/bootstrap.go#L248-L264

I am not a developer so I do not understand what the code does, here but I thought that the files in that directory will only get re-applied in case the timestamp changed. Is that not true anymore?

In case the helm charts are re-applied during every re-start that would even mean that every restart would cause a helm-upgrade..

It still does not explain why the helm-operator did a uninstall/re-install without having any modification... - but that might be an other issue.. not sure why we have seen uninstall/install instead of upgrade (btw. we also have seen this for ingress-nginx delivered by RKE2 1.20.12..)

If you simply delete the manifests from all the servers, absent direct user action there will not be any changes to the deployed HelmChart CRs that would cause an update to the chart.

Probably in RKE2 the manifests should be deleted locally automatically after it got applied... deleting the right ones is also not easy as it seems some are delivered via RKE2 so we can not delete all of them.. but at least for the move of Longhorn from manifest/* to fleet this might help..

Martin-Weiss avatar Jan 14 '22 10:01 Martin-Weiss

@Martin-Weiss this is a workaround we applied:

  1. get the NAME of the HelmChart resource:
kubectl -n kube-system get helmchart
  1. Patch the HelmChart resource you need:
kubectl -n kube-system patch helmchart <NAME> -p '{"metadata":{"annotations":{"helmcharts.cattle.io/managed-by": "helm-controllerDUMMY"}}}' --type=merge
kubectl -n kube-system patch helmchart <NAME> -p '{"metadata":{"finalizers":null}}' --type=merge
  1. Ensure no more helm controller jobs are running/pending:
kubectl -n kube-system delete job helm-install-<NAME>
  1. Ensure no more finalizers and "helmcharts.cattle.io/managed-by" annotation is renamed:
$ kubectl -n kube-system get helmchart <NAME> -o yaml
  1. Remove HelmChart resource:
kubectl -n kube-system delete helmchart <NAME>

In our scenario this prevent helm-controller to invoke the helm-delete job (uninstall)

kladiv avatar Jan 31 '23 21:01 kladiv

FYI, a while ago https://github.com/k3s-io/helm-controller/pull/137 added the helmcharts.helm.cattle.io/unmanaged annotation to the helm controller:

  • Adds support for helmcharts.helm.cattle.io/unmanaged annotation. HelmChart resources with this annotation will no longer be managed by the Helm controller, and can safely be updated or deleted without the Helm controller triggering any Jobs to install/update/delete the chart. This is intended for use when an administrator wants to begin managing a Helm releases via another tool, such as Fleet, ArgoCD, or manual invocation of the Helm CLI tools, and wants to ensure that the embedded Helm controller no longer mucks about with the current release.

Also, you don't need to use kubectl patch to edit annotations, kubectl annotate is specifically meant for adding or removing annotations from resources.

brandond avatar Jan 31 '23 21:01 brandond

I'm going to close this out as addressed by the aforementioned PR.

brandond avatar Jan 31 '23 21:01 brandond