-
Notifications
You must be signed in to change notification settings - Fork 91
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
Expose the deployment strategy values for the policy controller #749
base: main
Are you sure you want to change the base?
Expose the deployment strategy values for the policy controller #749
Conversation
30f3e6e
to
711f9e6
Compare
can you rebase and sign the dco? |
@shearn89 thanks for adding this feature. Could you do the necessary to get this merged please? |
Sorry, it's been busy at work and home. I'll try to get this ready for merging soon. |
Prior to this change, the policy controller webhook was not able to have its deployment strategy modified. If you only deployed a single replica, it could not perform a rolling update due to the default `maxSurge: 25%` being rounded down to 0. This change exposes those values, so that the `maxSurge` can be updated and a single instance can be rolled. Fixes sigstore#748. Signed-off-by: Alex Shearn <[email protected]>
Signed-off-by: Alex Shearn <[email protected]>
Signed-off-by: Alex Shearn <[email protected]>
711f9e6
to
8e8d223
Compare
Signed-off-by: Alex Shearn <[email protected]>
Okay - rebased, updated, linted, schema generated, and commits signed off! If you'd like me to squash the commits or squash-merge, let me know. 😁 |
{{- if .Values.deployment.strategy }} | ||
strategy: | ||
{{ toYaml .Values.deployment.strategy | trim | indent 4 }} | ||
{{ if eq .Values.deployment.strategy.type "Recreate" }}rollingUpdate: null{{ end }} |
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.
let's default for the default value when you not set this field, and drop the {{- if .Values.deployment.strategy }}
i dont think we need to add any extra complexity for this
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.
Okay - think I've done that although my templating knowledge is not amazing!
Signed-off-by: Alex Shearn <[email protected]>
…-controlle-webhook Signed-off-by: Alex Shearn <[email protected]>
Description of the change
Prior to this change, the policy controller webhook was not able to have its deployment strategy modified. If you only deployed a single replica, it could not perform a rolling update due to the default
maxSurge: 25%
being rounded down to 0.This change exposes those values, so that the
maxSurge
can be updated and a single instance can be rolled.Existing or Associated Issue(s)
Fixes #748.
Additional Information
Checklist
Lint output: