Tweak `helm [un]install` to make upgrades easier

[mboersma]

To upgrade Deis Workflow (as one example of a complex Chart), it's required that cluster ingress is preserved. Essentialy any DNS pointing toward a LoadBalancer should not need to be recreated. That requires that the Namespace in which it lives is also not destroyed.

This is possible by running a passel o' kubectl commands (here is a quick-and-dirty version). I propose we modify helmc behavior slightly to accomodate this.

Edit: this proposal changed quite a bit from here--see the end of this discussion and #474.

helmc uninstall --upgrading would change the current uninstall behavior to leave intact:

1. k8s Services containing either a LoadBalancer or NodePort
2. Namespaces

helmc install --upgrading would change the current install behavior to use kubectl apply instead of kubectl create. This should create any missing resources while updating the definitions of those that already exist.

kubectl apply may not effect runtime changes on an existing Service, but my assumption is that will be rare and can be worked around. The onus is on on the Chart developer to maintain compatibility with --upgrade: essentially "don't make important changes to an externally routed Service definition without providing additional upgrade instructions."

While this is designed to solve the immediate needs of upgrading Deis Workflow charts, it's a general mechanism. Here's a typical upgrade process based on this change:

$ helmc uninstall --upgrading -n deis -y workflow-beta3
$ helmc fetch deis/workflow-beta4
$ helmc generate -x manifests workflow-beta4
$ helmc install --upgrading workflow-beta4

ping @smothiki @krancour

See also #324.

[smothiki]

How about the idea of structuring folders. Right now we have

• manifests
• tpl

what ever that is there inside manifest gets installed and uninstalled. If we introduce one more folder standard which doesn't get touched during uninstall, basically we keep namesapce and router service in standards. I'm okay with above implementation as well.
During install we ignore already exists error from standards directory manifests.

[mboersma]

So files in this "standard" folder would basically have the behavior I described automatically applied to them without a command line flag I suppose? So helmc uninstall would skip over those manifests (unless given a --force flag?), and helmc install would kubectl apply them (while continuing to kubectl create the others).

That seems like a reasonable implementation too. We should prefer whatever is less disruptive to existing helm classic usage, but I'm not sure which that is.

[smothiki]

Yeah , much improvisation SGTM.

[krancour]

I like @smothiki's idea, primarily because the --upgrading flag would be too easily forgotten or overlooked by people uninstalling to facilitate an upgrade-- especially if they haven't read all the docs.

Requiring --force to complete a full uninstall seems ok. I think, just to cover our bases, if someone uninstalls without using --force, there should at least be a message output to inform them of what resources have not been deleted. This way, once again for the sake of people who haven't read the docs, they will at least know what's going on.

While I think this is important, I think the 0.7.0 release needs to happen before the Workflow beta4 release. As such, I believe this particular feature should be deferred to 0.8.0. (Technically, the project is in maintenance mode and shouldn't be getting new features, but I do believe Workflow needs this feature badly enough to justify deviating from that.)

[rimusz]

totally agreed with @krancour suggestion

[mboersma]

if someone uninstalls without using --force, there should at least be a message output

Agreed.

I believe this particular feature should be deferred to 0.8.0.

I don't want it to get in the way of current 0.7.0 stuff, but I'm going to start implementation soon and would like to finish before workflow-beta4. No reason we can't do two releases in a row if that's how it works out.

[mboersma]

Is "standard" informative enough as a name for this special directory? Maybe "keep"? "manifests-keep"?

mychart
- Chart.yaml
  manifests-keep/
  manifests/
  tpl/

Also I think this will require helmc to process both the "standard" and "manifests" directories and then interleave their manifests for install and uninstall. So I think this implementation will end up being somewhat more complicated than the command-line flag I proposed.

[smothiki]

I'm okay but how about keep-manifests or std-manifests

[mboersma]

I think adding an annotation to each manifest like helmc: keep would be more backward-compatible and simpler to implement. So there's no keep-manifests/ dir needed, and everything works as before unless you're using the newer helmc, in which case manifests with this annotation would be skipped on uninstall without the --force flag, and on install would get kubectl apply instead of kubectl create.

I would love more feedback on this idea.

[krancour]

I like the idea. I would ask that for cosistency, the header start with helm: instead of helmc:. This would make it consistent with the helm:generate header, which was deliberately not altered or renamed in the transition to Helm Classic, because we wanted to ensure charts authored by users of the original Helm remained compatible with Helm Classic, while also ensuring charts authored by users of Helm Classic remained compatible with recent versions of the original Helm. (So an easy way to look at it is that although the tool has been updated, the "language" it parses has stayed the same.)

Granted, I do know the new header would have no effect in older versions of Helm that do not understand it, so compatibility isn't a huge factor, but with that being said, the simple act of prepending it with helmc: would introduce and inordinate degree of confusion that we should be trying to avoid.

So go with helm:keep and I am 👍 .

[mboersma]

@krancour I was actually making this an annotation (in the YAML/JSON), not a header. I could make the annotation helm: keep, or I can explore putting it in the manifest header instead.

[krancour]

I was actually making this an annotation (in the YAML/JSON), not a header

Sorry for misunderstanding that.

Now that I'm you've got me on the same page... the annotation probably makes more sense.

I might suggest helm-keep: true as the annotation though. I feel the key half of the annotation should adequately describe the annotation's purpose. The helm: in helm: keep would be too ambiguous to a human reader.

[smothiki]

helm:keep is a good idea but we should also keep in mind the helm:generate header as well and how it works

[mboersma]

I might suggest helm-keep: true as the annotation

That's a good suggestion, I'll use that.

we should also keep in mind the helm:generate header

I'm proposing implementing this as a k8s annotation, not in a header line of the file. Or maybe I missed your point.

[smothiki]

Ohh I misread and didn't consider annotations. Cool if its annotation