Hey there. Thanks for the PR.

I'm curious to know what's the difference between this and wrapping the whole object in a {{ if .Release.IsInstall }}? I'm pretty sure this capability is already possible today without needing to make any changes to Helm.

I tried that, but helm will delete the resource on upgrade.

As a curious bystander I have a question:
Do this PR have any effect when running "helm template"?

@msvticket, helm template --help says:

This does not require Tiller. However, any values that would normally be
looked up or retrieved in-cluster will be faked locally.

The no-upgrade-existing needs to find resources by looking them up in-cluster, and I haven't implemented any "local faking" for it, so no, there is no effect when running helm template.

Ah yes, forgot about that. Sorry!

What about a pre-install hook instead? If I recall, this should work without an issue (and solve the same use case as described):

apiVersion: v1
kind: Secret
metadata:
  name: {{ template "helm-random-secret.fullname" . }}
  annotations:
    "helm.sh/hook": "pre-install"
    "helm.sh/hook-delete-policy": "before-hook-creation"
  labels:
    app: {{ template "helm-random-secret.name" . }}
    chart: {{ template "helm-random-secret.chart" . }}
    release: {{ .Release.Name }}
    heritage: {{ .Release.Service }}
data:
  some-password: {{ default (randAlphaNum 10) .Values.somePassword | b64enc | quote }}

That hook will render the secret before any resources are created in Kubernetes, and will not be updated during a helm upgrade. As far as I understand, that should be the same behaviour as what's described here.

Have you tried that before, and if so, what difference does the no-upgrade-existing annotation provide vs. the pre-install hook?

@bacongobbler The difference between no-upgrade-existing and the pre-install hook is that the user can explicitly set the password at any time in my example, and that will upgrade the secret (and a subsequent upgrade will not touch the secret unless it is manually deleted or again explicitly set), as well as automatically deleting the secret on a helm delete.

The use cases provided in https://github.com/helm/charts/issues/5167 has been

1. When I install a chart, I want a password generated for me, which is then persisted and should not change.
2. As a chart user, I want to be able to set that password during helm install, which is then persisted and should not change.

The pre-install + before-hook-creation hook deletion policy example solves those use cases, which is why I'm resistant to merge a PR that seems to already be solved with existing features.

the user can explicitly set the password at any time in my example

Why would you want to install a chart with a password that gets changed every time you call helm upgrade, then once it's override and lock it in place? Why wouldn't you want to just lock it in place the first time the chart is installed?

Why would you want to install a chart with a password that gets changed every time you call helm upgrade, then once it's override and lock it in place? Why wouldn't you want to just lock it in place the first time the chart is installed?

It wouldn't be changed every time you call helm upgrade, just if you wanted to, say change the random password to an explicit one with helm upgrade ... --set somePassword='mychosenpassword'.

Also, the main advantage (that I've perhaps not stated very well) is that the annotation can be added to an existing chart, and if somebody already installed the old chart with a random password, upgrading to the new chart will not overwrite it. With an init hook, the default Helm behaviours will mean that user of the old chart is out of luck: it will still be deleted (or in the best case, overwritten) when they upgrade to the new chart.

Okay, I'm starting to see the use case now. So essentially it's kind of "pinning" an object to its state. Thank you. I'll have another look.

Would no-replace-existing be a better name? I would also like to test that it has the same meaning on helm install, not just upgrade.

Also, what if the chart wants both this policy and also keep? Should resource policies be comma-separated?

pin might be a better name. It's a common term used in package management to pin an object at a certain point in time.

I think pin is counterintuitive, as it doesn't pin the resource you add the annotation to, rather just prevents the annotated resource from overriding an existing one, just for this particular helm run.

related discussion: #3053 (comment)

@FrenchBen, @bacongobbler will this be merged?

Thanks,
Michael.

Right now I'm focusing on getting 2.13.0 out the door as well as Helm 3 core development work. When I have more time I will get back to this, but for now I don't have the time to review. Sorry!

@FrenchBen's input is appreciated, but he is not a core maintainer. It needs an LGTM from a core maintainer before it can be merged. :)

Would this work if we wanted to update one field in the resource, but not another (say a randomly generated ID or password)?

@cgetzen, sorry but no, subsets of a resource are not kept.

The current patch is all-or-nothing.

Is there any ETA for merging this?

No ETA. I've been focusing more recently on security fixes for Helm 2 as well as ongoing development for Helm 3 so I have not been able to give this PR the time to think through the design.

If others could actually test this PR out and give it some feedback, that would be very helpful. I'm concerned that it might not solve the use cases in helm/charts#5167, so feedback from users running this in staging/production would be more helpful moving this forward.

What are your thoughts on why this doesn't resolve #5167 @bacongobbler? It really seems like it's exactly what's needed. Would love to see this get in.

It only checks the "target" (the chart you are upgrading to) for the "pin" annotation. As soon as the annotation is removed from the template on a helm upgrade, the "pin" is lifted. A very simple use case where this won't work is with rollbacks. If you roll back to an older release that did not have that pin annotation, it will roll back rather than ignoring the update. That probably isn't the desired behaviour.

IMO it should be a "set and forget" setting that occurs outside of the update logic. In other words the user should mark the existing resource as "pinned" with the annotation, which then Helm will recognize the annotation and ignore the update. The user must remove the annotation from the resource to allow updates. IOW we should be checking the source for the annotation, not the target.

Closing as this is not the right approach for the problem. If someone wants to implement a PR as well as the supporting documentation that covers the approach described here for Helm 3, that'd be great. Helm 2.15.0 is the last feature release for Helm 2 so we are no longer accepting new features in Helm 2.