Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --cleanup-on-fail option to default helm upgrade command? #55

Closed
scottyhq opened this issue Mar 12, 2020 · 3 comments · Fixed by #56
Closed

Add --cleanup-on-fail option to default helm upgrade command? #55

scottyhq opened this issue Mar 12, 2020 · 3 comments · Fixed by #56

Comments

@scottyhq
Copy link
Contributor

scottyhq commented Mar 12, 2020

In light of occasional failed deployments with hubploy for whatever reason (for example pangeo-data/pangeo-cloud-federation#560) There might be some utility in adding the --cleanup-on-fail or --atomic flags as defaults or options?

from docs https://helm.sh/docs/helm/helm_upgrade/

--cleanup-on-fail          allow deletion of new resources created in this upgrade when upgrade fails
--atomic                   if set, upgrade process rolls back changes made in case of failed upgrade. The --wait flag will be set automatically if --atomic is used

https://github.com/yuvipanda/hubploy/blob/edf788812b2441b10b8f935ae951b5f6bd5a84d5/hubploy/helm.py#L71-L84

@consideRatio
Copy link
Collaborator

consideRatio commented Mar 12, 2020

Wow these options are VERY important I think, and I wish they would have defaulted to use these instead.

I'm not confident what to opt for, --cleanup-on-fail or --atomic. I think I understand --atomic, but what does really the --cleanup-on-fail option mean?

allow deletion of new resources created in this upgrade when upgrade fails

Allow deletion? It won't do anything, but it will allow something to be done? I assume it will actively cleanup, so it will actively delete new resources, but it will not roll back modified but not new resources?

Perhaps --atomic is the most reliable option then? I currently understand --atomic to imply --cleanup-on-fail, that atomic is either equal to or more than cleanup-on-fail.

@consideRatio
Copy link
Collaborator

consideRatio commented Mar 12, 2020

The adding of the --atomic flag, which was news to me: https://github.com/helm/helm/pull/5143/files
The adding of the --cleanup-on-fail flag, which was also news to me: helm/helm#4871

Apparently the --cleanup-on-fail flag was made to a optional flag to avoid adding a breaking change, but it is explicitly so that after having made an install that failed without this flag option, the option is of little use, so its main use is before this becomes an issue, not after: helm/helm#4871 (comment)

I note that the --atomic option was added after --cleanup-on-fail. Hmm... Yeah I think --atomic is the way to go, it is the more robust all round solution as I understand it, and I will start using this at all time I think.

@scottyhq thank you so much for spotting this, this is very relevant to me and for the z2jh documentation!

@consideRatio
Copy link
Collaborator

For Hubploy, i suggest replacing --wait with --atomic. --atomic implies wait, and is robust and sensible for helm install as well as helm upgrade and helm ugprade --install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants