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.

Sign up for GitHub

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

Jump to bottom

fix(tiller): Supersede multiple deployments #3539

Merged
merged 4 commits into from
Feb 27, 2018
Merged

fix(tiller): Supersede multiple deployments #3539

merged 4 commits into from
Feb 27, 2018

Conversation

balboah
Copy link
Contributor

@balboah balboah commented Feb 20, 2018

There are cases when multiple revisions of a release has been marked with DEPLOYED status. This makes sure any previous deployment will be set to SUPERSEDED when doing rollbacks.

The original unit test was updated by @bacongobbler and cherry picked from d05bc61.

(hopefully) Closes #2941 #3513 #3275

Matthew Fisher and others added 4 commits February 20, 2018 10:44
@balboah
add test for rolling back from a FAILED deployment
ac5987c
@balboah
Update naming of release variables
35b76ed 
Use same naming as the rest of the file.
@balboah
Update rollback test
48f19e8 
- Add logging
- Verify other release names not changed
@balboah
fix(tiller): Supersede multiple deployments
32590cd 
There are cases when multiple revisions of a release has been
marked with DEPLOYED status. This makes sure any previous deployment
will be set to SUPERSEDED when doing rollbacks.

Closes helm#2941 helm#3513 helm#3275
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 20, 2018
bacongobbler
bacongobbler approved these changes Feb 27, 2018
View reviewed changes
Copy link
Member

@bacongobbler bacongobbler left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really liked how you improved the variable names to make it more clear on what behaviour changed. LGTM!

@adamreese adamreese merged commit 5f1a21b into helm:master Feb 27, 2018
This was referenced Feb 27, 2018
Closed
Closed
@bacongobbler bacongobbler added this to the 2.8.2 - Bugfix milestone Feb 27, 2018
@bacongobbler bacongobbler added the needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. label Feb 27, 2018
@bacongobbler
Copy link
Member

slating for v2.8.2

@stealthybox
Copy link
Contributor

I really liked how you improved the variable names to make it more clear on what behaviour changed. LGTM!

Agreed -- so much more clear
thanks for this change @balboah !

@balboah balboah deleted the issue-3513 branch February 28, 2018 09:07
@balboah
Copy link
Contributor Author

balboah commented Feb 28, 2018

@bacongobbler @stealthybox sure thing, it was mainly for myself to be able to be follow what was happening :)

@zaa
Copy link

zaa commented Mar 7, 2018 •
edited
Loading

This makes sure any previous deployment will be set to SUPERSEDED when doing rollbacks.

I believe the issue applies to helm upgrade as well. For example, I managed to create two DEPLOYED releases by running two helm upgrade commands concurrently. It looks like we need to cleanup such duplicate releases in case of upgrades too.

@bacongobbler bacongobbler added picked Indicates that a PR has been cherry-picked into the next release candidate. and removed needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. labels Mar 9, 2018
bacongobbler pushed a commit that referenced this pull request Mar 9, 2018
@balboah
fix(tiller): Supersede multiple deployments (#3539)
5847d92 
* add test for rolling back from a FAILED deployment

* Update naming of release variables

Use same naming as the rest of the file.

* Update rollback test

- Add logging
- Verify other release names not changed

* fix(tiller): Supersede multiple deployments

There are cases when multiple revisions of a release has been
marked with DEPLOYED status. This makes sure any previous deployment
will be set to SUPERSEDED when doing rollbacks.

Closes #2941 #3513 #3275

(cherry picked from commit 5f1a21b)
@consideRatio consideRatio mentioned this pull request Apr 1, 2018
Merged
@consideRatio consideRatio mentioned this pull request Apr 17, 2018
Closed
bmperrea added a commit to paxosglobal/helm that referenced this pull request Oct 22, 2018
@bmperrea
fix(tiller): rollback deleted release (helm#3722)
7a7ff5f 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

Signed-off-by: Brent Perreault <bperreault@paxos.com>
@bmperrea bmperrea mentioned this pull request Oct 22, 2018
Merged
bmperrea added a commit to paxosglobal/helm that referenced this pull request Oct 23, 2018
@bmperrea
fix(tiller): rollback deleted release (helm#3722)
f63dac0 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent bmperrea@gmail.com
bmperrea added a commit to paxosglobal/helm that referenced this pull request Oct 23, 2018
@bmperrea
fix(tiller): rollback deleted release (helm#3722)
9d27f8c 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent Perreault <bmperrea@gmail.com>
bmperrea added a commit to paxosglobal/helm that referenced this pull request Oct 23, 2018
@bmperrea
fix(tiller): rollback deleted release (helm#3722)
4d84397 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
bacongobbler pushed a commit that referenced this pull request Nov 12, 2018
@bmperrea
fix(tiller): rollback deleted release (#3722) (#4820)
9ae00be 
Solves #3722 by making the changes in #3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@bmperrea @splisson
fix(tiller): rollback deleted release (helm#3722) (helm#4820)
d5290e2 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@balboah @splisson
fix(tiller): Supersede multiple deployments (helm#3539)
0603143 
* add test for rolling back from a FAILED deployment

* Update naming of release variables

Use same naming as the rest of the file.

* Update rollback test

- Add logging
- Verify other release names not changed

* fix(tiller): Supersede multiple deployments

There are cases when multiple revisions of a release has been
marked with DEPLOYED status. This makes sure any previous deployment
will be set to SUPERSEDED when doing rollbacks.

Closes helm#2941 helm#3513 helm#3275
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@bmperrea @splisson
fix(tiller): rollback deleted release (helm#3722) (helm#4820)
bd1daee 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@balboah @adamreese
fix(tiller): Supersede multiple deployments (helm#3539)
3f5e777 
* add test for rolling back from a FAILED deployment

* Update naming of release variables

Use same naming as the rest of the file.

* Update rollback test

- Add logging
- Verify other release names not changed

* fix(tiller): Supersede multiple deployments

There are cases when multiple revisions of a release has been
marked with DEPLOYED status. This makes sure any previous deployment
will be set to SUPERSEDED when doing rollbacks.

Closes helm#2941 helm#3513 helm#3275
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@bmperrea @splisson
fix(tiller): rollback deleted release (helm#3722) (helm#4820)
248f58b 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
@bmperrea @splisson
fix(tiller): rollback deleted release (helm#3722) (helm#4820)
43e7a55 
Solves helm#3722 by making the changes in helm#3539 more compatible with the previous behavior.

This gives a recovery option for "oops I deleted my helm release" by allowing rollback, which is intended to be a working feature of helm. Note that purging releases removes the history required to rollback, so this doesn't work in that case. However, purging takes significantly more time, so it's harder to accidentally purge everything.

Signed-off-by: Brent <bmperrea@gmail.com>
jianghang8421 pushed a commit to jianghang8421/helm that referenced this pull request Feb 17, 2019
@balboah @jianghang8421
fix(tiller): Supersede multiple deployments (helm#3539)
b6622cf 
* add test for rolling back from a FAILED deployment

* Update naming of release variables

Use same naming as the rest of the file.

* Update rollback test

- Add logging
- Verify other release names not changed

* fix(tiller): Supersede multiple deployments

There are cases when multiple revisions of a release has been
marked with DEPLOYED status. This makes sure any previous deployment
will be set to SUPERSEDED when doing rollbacks.

Closes helm#2941 helm#3513 helm#3275
@novas0x2a novas0x2a mentioned this pull request Oct 5, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bacongobbler bacongobbler bacongobbler approved these changes

@adamreese adamreese adamreese approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. picked Indicates that a PR has been cherry-picked into the next release candidate. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
2.8.2 - Bugfix
Development

Successfully merging this pull request may close these issues.
6 participants
@balboah @bacongobbler @stealthybox @zaa @adamreese @k8s-ci-robot