fix(storage): when pruning release versions, never delete the last deployed revision #4978
Conversation
helm-bot
added
the
size/M
There was a problem hiding this comment.
@ultimateboy This is looking good. I know there wasn't an existing test for this, but could you add a test for this new functionality? This seems like something that would be easy to miss later and cause a regression
| @@ -210,6 +226,16 @@ func (s *Storage) removeLeastRecent(name string, max int) error { | |||
| } | |||
| } | |||
|
|
|||
| func (s *Storage) deleteReleaseVersion(name string, version int32) error { | |||
There was a problem hiding this comment.
Thanks for the nice refactor into its own function!
|
hey @ultimateboy! In the last month or so, we switched from a CLA to a DCO. This means that the commit will need to be signed off in order to agree to the DCO. Would you mind amending your commit to add that in? More details on how to generate the signoff line here: https://github.com/helm/helm/pull/4978/checks?check_run_id=35785210 Thanks! |
…ployed revision Signed-off-by: Matt Tucker <[email protected]>
ultimateboy
force-pushed
the
dont-delete-deployed
branch
from
c6616ab to
5bf38a2
Compare
helm-bot
added
size/M
There was a problem hiding this comment.
LGTM! We can merge once the tests pass. This should definitely help out users who end up in a state with no deployable releases with TILLER_HISTORY_MAX set. Thanks @ultimateboy! :)
|
It wont help users who are already in that state, but it should prevent users from getting into that state in the future :) |
fix(storage): when pruning release versions, never delete the last deployed revision Signed-off-by: Kevin Labesse <[email protected]>
When tiller-max-history has been set, let's say to 3, if you attempt 3 releases that fail, you end up with a release with no deployed releases.
This doesn't make sense. By changing the prune function to never delete the most recent successfully deployed revision, we never enter a case where a release has no deployed revisions.
See the test case for a good example of this.