getporter / helm2-mixin

Helm mixin for Porter
https://porter.sh/mixins/helm
Apache License 2.0
13 stars 7 forks source link

Upgrade should always include the --install flag #45

Closed carolynvs closed 5 years ago

carolynvs commented 5 years ago

After watching @squillace try to get a bundle into the right state, we realized that what would work was to run upgrade. But then the helm release could upgrade because we were running a straight helm upgrade instead of helm upgrade --install, so it was failing because the release wasn't there.

I'm wondering if it's more important for us to work or for us to be correct and match the spec?

The user is obviously trying to get into a good state and helm upgrade --install is most likely to ensure successful bundle runs. However I'm pretty sure we didn't do this in the first place because it fit better with the spec.

My vote is to go with the principle of least surprise. Whatever the user expects and wants us to do, we should do. I think that would be bundles that don't get stuck in error states that require uninstalling and lots of fiddling with, and work towards desired state instead.

Thoughts?

carolynvs commented 5 years ago

The upgrade command for helm should include the --install flag at all times.

See https://github.com/deislabs/porter-helm/blob/8325d71c9209daa6a58e01419e305e6aaa2892f3/pkg/helm/upgrade.go#L63 for where the flag should be inserted.

This should also include a test for the new flag.