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

Helm (tiller) might try to upgrade two revisions at the same time #3513

Closed
balboah opened this issue Feb 15, 2018 · 5 comments
Closed

Helm (tiller) might try to upgrade two revisions at the same time #3513

balboah opened this issue Feb 15, 2018 · 5 comments

Comments

@balboah
Copy link
Contributor

balboah commented Feb 15, 2018

I'm creating a new issue out of comments from #3275 as this looks like a bug and not a support/question. Feel free to close this one if its invalid!

After trying to apply a small configmap value change and upgrading with:
helm upgrade --install -f values-control.yaml control .
I'm getting UPGRADE FAILED: no Secret with the name "foobar" found.

I deleted this secret and re-ran the upgrade which then caused errors on some configmap instead, deleting the configmap and doing a 3rd run once again complained on the previous secret.

Looking at tiller logs I'm seeing

[storage] 2018/02/15 10:20:50 updating release "control.v136"
[storage] 2018/02/15 10:20:50 updating release "control.v226"

It seems that tiller tries to apply the change to two revisions of one release at the same time?
helm ls confirms that there is only one release:

NAME   	REVISION	UPDATED                 	STATUS	CHART                     	NAMESPACE
control	226     	Thu Feb 15 11:20:50 2018	FAILED	foo-0.1.0	default

Trying to list history of this release causes grpc errors (related to #3512 and #3322):

helm history control
Error: grpc: trying to send message larger than max (49338925 vs. 20971520)

This might have been triggered after upgrading from helm 2.7.x to 2.8.1 but I'm not sure as this particular cluster have been idle for some time since after the upgrade.


Client: &version.Version{SemVer:"v2.8.1", GitCommit:"6af75a8fd72e2aa18a2b278cfe5c7a1c5feca7f2", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.8.1", GitCommit:"6af75a8fd72e2aa18a2b278cfe5c7a1c5feca7f2", GitTreeState:"clean"}
@balboah
Copy link
Contributor Author

balboah commented Feb 15, 2018

Maybe the double release log is normal as,

s.recordRelease(originalRelease, true)
s.recordRelease(updatedRelease, true)

is called on failures. But it seems like a long way between the numbers?
I'm not sure what is causing the missing metadata errors

@balboah
Copy link
Contributor Author

balboah commented Feb 20, 2018

the newest revision that has status DEPLOYED is 222. When checking helm status --revision 136 that one also has status DEPLOYED. 134 and 136 has SUPERSEDED. Could the bug be about tiller storage failing to change the status of one of the revisions causing it to conflict with itself?

Also after applying #3514 and finding out about --max argument, I can list up to about 60 revisions in helm history. Anything larger than 60 revisions will get the error grpc: trying to send message larger than max (21451805 vs. 20971520).
Which also makes me wonder if the amount of bytes (20MB) to display the history list is another bug as well?

@balboah
Copy link
Contributor Author

balboah commented Feb 20, 2018

manually editing the configmap to change DEPLOYED to SUPERSEDED in
kubectl -n kube-system edit configmap control.v136 makes the upgrade work again.

@balboah balboah changed the title Helm (tiller) might try to upgrade two release versions at the same time Helm (tiller) might try to upgrade two revisions at the same time Feb 20, 2018
@balboah
Copy link
Contributor Author

balboah commented Feb 20, 2018

Maybe same issue as #2941 except in this case, helm list only has one entry

balboah added a commit to joonix/helm that referenced this issue 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.

Closes helm#2941 helm#3513 helm#3275
adamreese pushed a commit that referenced this issue Feb 27, 2018
* 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
@bacongobbler
Copy link
Member

closed via #3539

bacongobbler pushed a commit that referenced this issue Mar 9, 2018
* 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)
splisson pushed a commit to splisson/helm that referenced this issue Dec 6, 2018
* 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 issue Dec 6, 2018
* 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
jianghang8421 pushed a commit to jianghang8421/helm that referenced this issue Feb 17, 2019
* 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
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

No branches or pull requests

2 participants