helm-chart: fix missing PDB config maxUnavailable, and default to 1 #1418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6cffef5 to
c167f29
Compare
23 tasks
c167f29 to
d61fb4b
Compare
consideRatio
pushed a commit
to jupyterhub/helm-chart
that referenced
this pull request
Oct 11, 2021
jupyterhub/binderhub#1418 Merge pull request #1418 from consideRatio/pr/fix-pdb-failure
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PDBs are tricky resources resource that can make things lock stuck in some situations unless we are careful. If we for example have one single replica and have
minAvailable: 1then automated k8s node upgrades will fail until someone manually deletes the pod on the old node bing removed.PDBs can block automated maintenance in a k8s cluster and shouldn't do that unless explicitly configured to do so I'd say. But it should help ensure that if you have two replicas running, both shouldn't be removed at the same time. To do that, one can either use
minAvailable: 1ormaxUnavailable: 1. UsingmaxUnavailable: 1can work well with both one or two replicas of a pod, whileminAvailable: 1can only work well with two replicas.This PR makes us use
maxUnavailable: 1instead ofminAvailable: 1. It also fixes a bug that made the helm chart configpdb.maxUnavailablebe ignored.I suspect this PR can cause upgrade issues, and it would be great to have an upgrade test of the Helm chart as well. This is represented by #1195. EDIT: rebased and we have now tested this to be upgradable without issues.