Use maxUnavailable for PodDisruptionBudgets - #36
Conversation
Every PDB in the chart set minAvailable, while analytics and tables default to a single replica. minAvailable: 1 against one replica evaluates to disruptionsAllowed: 0, so those pods cannot be evicted and every voluntary disruption fails: cluster upgrades, node image upgrades and autoscaler scale-down all stall while it is set. This is not theoretical. It left an Azure AKS estate on three-month-old node images across four clusters, with the pools reporting Failed and nothing alerting on it, because the drain could never complete. maxUnavailable: 1 is equivalent at two replicas and drainable at one, so all five components now default to it rather than only fixing the two that deadlock today. Overriding replicas down to 1 is a supported thing to do and should not reintroduce the deadlock. minAvailable still works when maxUnavailable is unset, so existing overrides are unaffected. A PDB may not set both; maxUnavailable takes precedence. One behaviour change to note: at three or more replicas maxUnavailable: 1 permits one pod down at a time where minAvailable: 1 permitted all but one. That is safer but makes drains slower. tables uses ReadWriteOnce storage and cannot be scaled past one replica, so it still incurs brief downtime while its node drains. This makes the drain possible, not seamless. Verified by rendering the chart: all five PDBs emit maxUnavailable: 1 with default values, and an override of maxUnavailable: null with minAvailable: 2 still emits minAvailable: 2. helm lint and both CI template steps pass. Note that CI never exercises this path, since values.ci.yaml disables PDBs. Part of OPS-4725
There was a problem hiding this comment.
Pull request overview
Updates the Helm chart’s PodDisruptionBudget (PDB) configuration to default to maxUnavailable: 1 (replica-count safe for replicas: 1) while still supporting minAvailable overrides, and aligns documentation/examples with the chart’s real values schema.
Changes:
- Switch component PDB defaults from
minAvailabletomaxUnavailable: 1inchart/values.yaml. - Update all five PDB templates to render
maxUnavailablewhen set, otherwise fall back tominAvailable. - Fix docs/examples (README + EKS guide + AGENTS) to use
*.podDisruptionBudgetschema and document themaxUnavailablepreference.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates PDB configuration example and adds guidance on maxUnavailable vs minAvailable. |
| docs/DEPLOY_TO_AWS_EKS.md | Updates the EKS deployment values example to the correct PDB schema/defaults. |
| chart/values.yaml | Changes default PDB value keys to maxUnavailable: 1 for all components. |
| chart/templates/pdb-app.yaml | Renders maxUnavailable when set, else falls back to minAvailable. |
| chart/templates/pdb-engine.yaml | Renders maxUnavailable when set, else falls back to minAvailable. |
| chart/templates/pdb-nginx.yaml | Renders maxUnavailable when set, else falls back to minAvailable. |
| chart/templates/pdb-analytics.yaml | Renders maxUnavailable when set, else falls back to minAvailable. |
| chart/templates/pdb-tables.yaml | Renders maxUnavailable when set, else falls back to minAvailable. |
| AGENTS.md | Updates internal guidance to reflect the maxUnavailable: 1 default and drain-safety rationale. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The template guarded on truthiness, and Go templates treat 0 as false, so maxUnavailable: 0 fell through to rendering minAvailable: 1 — silently producing a PDB that differs from the values that asked for it. 0 is a valid PodDisruptionBudget value. kindIs "invalid" tests for nil instead, which keeps all four cases correct: an explicit 0 renders as 0, an explicit null falls back to minAvailable, an absent key falls back, and a set value renders. hasKey would not work here, since it is true for maxUnavailable: null and would render an empty field. Also correct two documentation errors. The README claimed a PDB "may not set both" fields and then that maxUnavailable "wins if you set both", conflating the rendered resource with the values schema. AGENTS.md described PDBs as covering "all stateless components" while tables, which has one, is stateful. Part of OPS-4725
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (6)
chart/values.yaml:196
- Setting
maxUnavailable: 1in the chart defaults means any existing user overrides that only setpodDisruptionBudget.minAvailablewill now be ignored, because the PDB templates rendermaxUnavailablewhenever it’s non-nil (e.g.chart/templates/pdb-app.yaml:12-16). If backward-compatibility for existingminAvailableoverrides is required, consider leavingmaxUnavailableunset/null invalues.yamland defaulting tomaxUnavailable: 1in the templates only when bothmaxUnavailableandminAvailableare unset.
# Pod Disruption Budget - ENABLED BY DEFAULT
podDisruptionBudget:
enabled: true
maxUnavailable: 1
chart/templates/pdb-app.yaml:15
default 1treats an explicitminAvailable: 0as “empty” and will render1instead of0. If someone intentionally wantsminAvailable: 0(valid for PDBs), this template can’t express it. Default only when the value is actually nil/unset.
minAvailable: {{ .Values.app.podDisruptionBudget.minAvailable | default 1 }}
chart/templates/pdb-engine.yaml:15
default 1treats an explicitminAvailable: 0as “empty” and will render1instead of0. If someone intentionally wantsminAvailable: 0(valid for PDBs), this template can’t express it. Default only when the value is actually nil/unset.
minAvailable: {{ .Values.engine.podDisruptionBudget.minAvailable | default 1 }}
chart/templates/pdb-nginx.yaml:15
default 1treats an explicitminAvailable: 0as “empty” and will render1instead of0. If someone intentionally wantsminAvailable: 0(valid for PDBs), this template can’t express it. Default only when the value is actually nil/unset.
minAvailable: {{ .Values.nginx.podDisruptionBudget.minAvailable | default 1 }}
chart/templates/pdb-analytics.yaml:15
default 1treats an explicitminAvailable: 0as “empty” and will render1instead of0. If someone intentionally wantsminAvailable: 0(valid for PDBs), this template can’t express it. Default only when the value is actually nil/unset.
minAvailable: {{ .Values.analytics.podDisruptionBudget.minAvailable | default 1 }}
chart/templates/pdb-tables.yaml:15
default 1treats an explicitminAvailable: 0as “empty” and will render1instead of0. If someone intentionally wantsminAvailable: 0(valid for PDBs), this template can’t express it. Default only when the value is actually nil/unset.
minAvailable: {{ .Values.tables.podDisruptionBudget.minAvailable | default 1 }}
Part of OPS-4725