Skip to content

[LFXV2-1166] Add pod disruption budget support #59

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.

Sign up for GitHub

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

Open
bramwelt wants to merge 3 commits into
base: main
Choose a base branch
from
+30 −0
Open

[LFXV2-1166] Add pod disruption budget support #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6ebd7f
Add pod disruption budget support
bramwelt Mar 6, 2026
d70a012
Add PDB validation and zero-value support
bramwelt Mar 6, 2026
de43823
Merge main into PDB branch
bramwelt Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions charts/lfx-v2-committee-service/templates/pdb.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright The Linux Foundation and each contributor to LFX.
# SPDX-License-Identifier: MIT
{{- if .Values.podDisruptionBudget.enabled }}
{{- if and (hasKey .Values.podDisruptionBudget "minAvailable") (hasKey .Values.podDisruptionBudget "maxUnavailable") }}
{{- fail "podDisruptionBudget: cannot set both minAvailable and maxUnavailable" }}
{{- end }}
---
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: {{ .Chart.Name }}
namespace: {{ .Release.Namespace }}
spec:
selector:
matchLabels:
app: {{ .Chart.Name }}
{{- if hasKey .Values.podDisruptionBudget "minAvailable" }}
minAvailable: {{ .Values.podDisruptionBudget.minAvailable }}
{{- end }}
{{- if hasKey .Values.podDisruptionBudget "maxUnavailable" }}
maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }}
{{- end }}
Comment on lines +3 to +22
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When podDisruptionBudget.enabled is true, this template can render an invalid PDB if neither minAvailable nor maxUnavailable is set, and it can also render an invalid PDB if both are set. Add Helm-time validation (e.g., fail/required) to enforce that exactly one of these values is provided when enabled.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +22
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing validation: PDB requires at least one of minAvailable or maxUnavailable.

When podDisruptionBudget.enabled is true but neither minAvailable nor maxUnavailable is set, this template renders a PDB without a disruption budget spec, which is invalid. Kubernetes requires at least one of these fields.

🐛 Proposed fix to add validation

Add this validation after line 6:

 {{- if and (hasKey .Values.podDisruptionBudget "minAvailable") (hasKey .Values.podDisruptionBudget "maxUnavailable") }}
   {{- fail "podDisruptionBudget: cannot set both minAvailable and maxUnavailable" }}
 {{- end }}
+{{- if not (or (hasKey .Values.podDisruptionBudget "minAvailable") (hasKey .Values.podDisruptionBudget "maxUnavailable")) }}
+  {{- fail "podDisruptionBudget: must set either minAvailable or maxUnavailable when enabled" }}
+{{- end }}
 ---
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@charts/lfx-v2-committee-service/templates/pdb.yaml` around lines 17 - 22, The
PDB template can render an invalid empty spec when
.Values.podDisruptionBudget.enabled is true but neither
.Values.podDisruptionBudget.minAvailable nor
.Values.podDisruptionBudget.maxUnavailable is set; fix it by adding a
guard/validation that checks hasKey .Values.podDisruptionBudget "minAvailable"
or hasKey .Values.podDisruptionBudget "maxUnavailable" (or both) before
rendering the spec, and if both are missing call fail with a clear message
(e.g., using Helm's fail function) so the chart errors out instead of producing
an invalid PDB; update the conditional logic around the spec generation that
currently uses hasKey checks for minAvailable/maxUnavailable to enforce this
combined requirement.

{{- end }}
7 changes: 7 additions & 0 deletions charts/lfx-v2-committee-service/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ podAnnotations: {}
# environment: production
podLabels: {}

# podDisruptionBudget configures a PodDisruptionBudget for the deployment.
# Only one of minAvailable or maxUnavailable may be set (not both).
podDisruptionBudget:
enabled: false
# minAvailable: 1
# maxUnavailable: 1
Comment on lines +23 to +24
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values docs currently suggest both minAvailable and maxUnavailable can be configured, but PodDisruptionBudget requires exactly one of them. Consider updating these comments to indicate they are mutually exclusive and that one must be set when podDisruptionBudget.enabled=true.

Suggested change
# minAvailable: 1
# maxUnavailable: 1
# minAvailable: 1 # Specify exactly one of minAvailable or maxUnavailable when enabled=true
# maxUnavailable: 1 # Mutually exclusive with minAvailable; uncomment only one of these fields

Copilot uses AI. Check for mistakes.

# image is the configuration for the container images
image:
# repository is the container image repository
Expand Down