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

chore(stateful-app): Explicitly set the default UpdateStrategy #273

Closed
wants to merge 1 commit into from

Conversation

schahal
Copy link
Contributor

@schahal schahal commented Feb 21, 2024

Can I get thoughts on explicitly setting the default?

RollingUpdate becomes the default if nothing (null) is set, anyways.

I see some advantages to this:

  • Users don't have to guess on the DeploymentStrategy anymore
  • They can explicitly see where to update partition if they want to, for example, do a canary rollout
  • [Highly unlikely] If default were to change in a future k8s release (realistically this will only even be plausible in a majore release, not 1.x), at least apps subcharting this will stay consistent.

(but I may be missing some historical context on why we want null)

@schahal schahal requested a review from diranged February 21, 2024 07:05
@schahal schahal requested a review from a team as a code owner February 21, 2024 07:05
@diranged
Copy link
Contributor

My general feeling is that I don't like copying and pasting defaults ... so I look to this and ask, "What problem are we solving?"

@schahal
Copy link
Contributor Author

schahal commented Feb 21, 2024

Closing out this PR (for context, we chatted offline.. we'll stick to the pattern not copy-pasting defaults in).

In general, if a user wants to fiddle with deployment strategies, they are already aware of what they'd want to change and can do so in their own "values.yaml" overlay (the problem to solve of seeing which default deployment strategy used isn't that great to change the general rule above)

@schahal schahal closed this Feb 21, 2024
@LaikaN57 LaikaN57 deleted the chore/sfs-update-strategy branch April 17, 2024 03:57
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

Successfully merging this pull request may close these issues.

2 participants