-
Notifications
You must be signed in to change notification settings - Fork 989
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
feat: added "strategy.rolling_update.max_surge" attr to daemonset #2416
feat: added "strategy.rolling_update.max_surge" attr to daemonset #2416
Conversation
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.
Hi @manjinder-mckc, thank you for these changes! Please take a look at the comment I left in the code.
Description: "The maximum number of nodes with an existing available DaemonSet pod that can have an updated DaemonSet pod during during an update. Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%). This can not be 0 if MaxUnavailable is 0. Absolute number is calculated from percentage by rounding up to a minimum of 1. Default value is 0. Example: when this is set to 30%, at most 30% of the total number of nodes that should be running the daemon pod (i.e. status.desiredNumberScheduled) can have their a new pod created before the old pod is marked as deleted.", | ||
Optional: true, | ||
Default: "0", | ||
ValidateFunc: validation.StringMatch(regexp.MustCompile(`^([0-9]+|[0-9]+%|)$`), ""), |
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.
This expression allows percentages higher than 100, here is an example that captures a more precise range of values.
Quick check-in on this PR. @manjinder-mckc please take a look at our comment and let us know if you have any questions with the suggested improvement. We would love to move forward with merging. Thanks! |
@iBrandyJackson @sheneska Apologies for late reply. Thank you for reviewing. I will take a look and pick this up now. |
Hi @manjinder-mckc ! any help needed for this? |
closing in favor of #2609 |
Description
Enhancement: #2106
Acceptance tests
Output from acceptance testing:
Release Note
Release note for CHANGELOG:
References
Community Note