-
Notifications
You must be signed in to change notification settings - Fork 315
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] Add maxUnavailable option for rolling updates #291
Conversation
cc @mariusv @walkafwalka can you help reviewing helm charts changes pls? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation about the maxUnavailable
on the readme on how it will be used.
I prefer using something more flexible for updateStrategy but that would require a breaking change. We should take note of situations like this.
Besides the documentation, LGTM. |
@walkafwalka I 100% agree, I reached out to @mariusv on slack about it to propose the same before I saw this PR but didn't get an answer. |
Regardless of implementation, this one is urgent for us as large clusters need some pretty high timeouts set w/ helm to get through a rolling update with the hard-coded settings. |
Allow user to set maxUnavailable option for rolling updates in order to speed up deployment times on large clusters.
d2ba828
to
56ee37c
Compare
I have updated the default value and added the missing doc. I opted not to implement the braking change at this point. However, if you think we should do it then I can do that no problem |
Allow user to set maxUnavailable option for rolling updates in order
to speed up deployment times on large clusters.