helm / helm

The Kubernetes Package Manager
https://helm.sh
Apache License 2.0
27.09k stars 7.12k forks source link

Annotation `"helm.sh/resource-policy": keep` not honored when uninstalling #10890

Open ml0renz0 opened 2 years ago

ml0renz0 commented 2 years ago

Output of helm version:

version.BuildInfo{Version:"v3.7.2", GitCommit:"663a896f4a815053445eec4153677ddc24a0a361", GitTreeState:"clean", GoVersion:"go1.16.10"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.11", GitCommit:"38d3c1f3d5306401bcf39a71bad3b5a5106033d7", GitTreeState:"clean", BuildDate:"2022-03-16T14:08:11Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.11", GitCommit:"38d3c1f3d5306401bcf39a71bad3b5a5106033d7", GitTreeState:"clean", BuildDate:"2022-03-16T14:02:06Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}

Helm documentation states that:

The annotation "helm.sh/resource-policy": keep instructs Helm to skip deleting this resource when a helm operation (such as helm uninstall, helm upgrade or helm rollback) would result in its deletion.

As I commented on issue #3673, I noticed that the annotation "helm.sh/resource-policy": keep on a resource is not honored when the release is uninstalled.

I see it was fixed for Update method, could it be that this change needs also to be included in the Delete method?

github-actions[bot] commented 2 years ago

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

smaanika-px commented 2 years ago

when will we have this feature supported in helm uninstall? This sounds like a valid usecase and would be great, if we can have this PR merged into master. Thanks.

github-actions[bot] commented 2 years ago

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

r10r commented 1 year ago

This hit me today.

I did set the annotation with kubectl annotate pvc -n hedgedoc helm.sh/resource-policy=keep but the resources were deleted by helm delete

version.BuildInfo{Version:"v3.9.0", GitCommit:"7ceeda6c585217a19a1131663d8cd1f7d641b2a7", GitTreeState:"clean", GoVersion:"go1.17.5"}

The docs should be updated then, because they state that uninstall honors the annotation:

From https://helm.sh/docs/howto/charts_tips_and_tricks/#tell-helm-not-to-uninstall-a-resource

The annotation "helm.sh/resource-policy": keep instructs Helm to skip deleting this resource when a helm operation (such as helm -->uninstall<--, helm upgrade or helm rollback) would result in its deletion.

bczoma commented 1 year ago

Helm uninstall seems to respect the annotation now, tried latest version version.BuildInfo{Version:"v3.12.3", GitCommit:"3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e", GitTreeState:"clean", GoVersion:"go1.20.7"}

iAnomaly commented 1 year ago

Helm uninstall seems to respect the annotation now, tried latest version version.BuildInfo{Version:"v3.12.3", GitCommit:"3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e", GitTreeState:"clean", GoVersion:"go1.20.7"}

This is NOT the case for me even on version.BuildInfo{Version:"v3.12.3", GitCommit:"3a31588ad33fe3b89af5a2a54ee1d25bfe6eaa5e", GitTreeState:"clean", GoVersion:"go1.20.7"}

helm.sh/resource-policy=keep was still ignored during helm uninstall and the resources were deleted when they should not be.

PR #10893 needs to be merged to fix this.

Pela2silveira commented 1 year ago

Hi there, I found that annotating by kubectl does not make effect. But if you make the annotation through helm it is respected later in the uninstall process. In my case, I modified the chart I was using for testing purpose. This may work as a workarround.

Regards

fischerman commented 1 year ago

I can also confirm that if the annotation is deployed with Helm, then the resources will stay intact.

helm delete crds
These resources were kept due to the resource policy:
[CustomResourceDefinition] crdA
[CustomResourceDefinition] crdB
[CustomResourceDefinition] crdC

release "..." uninstalled

I did not test to add the annotation by hand.

Helm version v3.12.0

odise commented 1 year ago

@fischerman I tested adding the annotation manually to a Helm managed CRD and it will be ignored (CRD will be removed with helm uninstall).

Helm version v3.12.3

Subetov commented 10 months ago

Faced the same issue =( Patching the resource with kubectl doesn't prevent deletion (helm uninstall/upgrade).

AndreaScarselliAx commented 9 months ago

Any news on this?

Pela2silveira commented 9 months ago

I think is not bug. Because, helm manages it's state through a secret. It don't compare with the final manifest of the resource. So if you want to respect this behavior, the configuration should be added by helm. It is only an opinion.

Shrooblord commented 8 months ago

@Pela2silveira is correct. Helm appears to look only at the Secret called sh.helm.release.v{#}.{chartname-randomstring}.v{#} that it creates whenever you helm install or helm upgrade a chart in order to determine the current state of its managed resources. This secret contains the entire chart rendered out fully as if you did helm template (including Secrets and generated passwords), then gzip'd and converted to base64 and then stored in this one 'release' Secret.

I think there is something being worked on here (although I'm not 100% following why the work has gone to the GitHub Actions Runner repo instead of the main Helm repo but never mind my confusion 😇) to enable us to go helm upgrade --custom-annotations=(...) or something along those lines. Not sure what the status is of that work; asked for a follow-up... we'll see.


Chart owners appear to be adding this functionality manually:

zip-chanko commented 3 weeks ago

The use case of the annotation helm.sh/resource-policy=keep should support ondemand basic by using kubectl though without upgrading again the helm chart. It will be hard to deal with helm chart which is shared with multiple apps. 😔