feat(chart): add optional PodDisruptionBudget for the operator pod - #550
Conversation
Signed-off-by: somaz <genius5711@gmail.com>
c71167e to
82002a2
Compare
| apiVersion: policy/v1 | ||
| kind: PodDisruptionBudget | ||
| metadata: | ||
| name: {{ include "dragonfly-operator.fullname" . }} |
There was a problem hiding this comment.
can you please name it {{ include "dragonfly-operator.fullname" . }}-controller-manger-pod-disruption-budget instead?
There was a problem hiding this comment.
Done in e874513 — renamed to {{ include "dragonfly-operator.fullname" . }}-controller-manager-pod-disruption-budget.
One note: I used the corrected spelling manager (your suggestion had a typo manger). Happy to switch to the literal manger if you actually intended that.
I also fixed a small related issue the bot flagged: the minAvailable guard used a truthiness test, so a valid minAvailable: 0 fell through to maxUnavailable. Changed it to not (kindIs "invalid" ...) so 0 is preserved. helm lint + template renders (default / 0 / maxUnavailable) all pass.
There was a problem hiding this comment.
no, it was indeed a typo :)
There was a problem hiding this comment.
Pull request overview
Adds optional PodDisruptionBudget (PDB) support to the dragonfly-operator Helm chart so operator pods can be protected from voluntary disruptions in HA deployments.
Changes:
- Introduces
podDisruptionBudgetvalues (enabled flag +minAvailable/maxUnavailable+unhealthyPodEvictionPolicy) invalues.yaml. - Adds a new
PodDisruptionBudgettemplate that targets the operator Deployment’s pod selector labels.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/dragonfly-operator/values.yaml | Adds chart values to configure an optional PDB for the operator. |
| charts/dragonfly-operator/templates/poddisruptionbudget.yaml | Adds the PDB manifest template gated by podDisruptionBudget.enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- if .Values.podDisruptionBudget.minAvailable }} | ||
| minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} | ||
| {{- else if .Values.podDisruptionBudget.maxUnavailable }} | ||
| maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} | ||
| {{- end }} |
| # -- A [PodDisruptionBudget] for the operator pod, useful when running more | ||
| # than one replica (replicaCount > 1) so voluntary disruptions (e.g. node | ||
| # drains) keep at least one operator pod available. | ||
| ## Ref: https://kubernetes.io/docs/tasks/run-application/configure-pdb/ | ||
| podDisruptionBudget: | ||
| # When set true a PodDisruptionBudget is created for the operator pod | ||
| enabled: false | ||
| # minAvailable and maxUnavailable are mutually exclusive; minAvailable takes | ||
| # precedence when both are set. Each may be an integer or a percentage string. | ||
| minAvailable: 1 | ||
| maxUnavailable: "" |
|
Thanks @Abhra303! Quick note on the two Copilot comments so they're not left hanging:
Nothing else outstanding on my side — ready whenever you are 🙂 |
|
This has been approved and mergeable with CI green since 07-14. Is there anything still needed from my side, or is it just waiting for a merge window? Happy to rebase if it's gone stale in the meantime. |
|
Hey, Thanks! |
Adds an optional
PodDisruptionBudgettemplate to the operator Helm chart, disabled by default and toggleable viapodDisruptionBudget.enabled. Useful when running the operator withreplicaCount > 1so voluntary disruptions (e.g. node drains) keep at least one operator pod available. The chart already exposes the sibling HA primitives (replicaCount,priorityClassName,topologySpreadConstraints,affinity) — this fills the remaining gap.Supports
minAvailable(default1) /maxUnavailable(mutually exclusive,minAvailablewins) and the optionalunhealthyPodEvictionPolicy(k8s 1.27+). The PDB selector matches the deployment's pod selector (selectorLabels+control-plane: controller-manager).Validation (helm v3.18.1)
helm lint— 0 charts failedminAvailable: 1): renders with correct selectormaxUnavailable+unhealthyPodEvictionPolicy: renders correctlyminAvailableandmaxUnavailableset:minAvailabletakes precedenceminAvailable: 0is preserved (not treated as unset)related: #280
Follows up on the closed #281 with a single-purpose, scoped change (PDB only — no ServiceMonitor / kube-rbac-proxy changes).