-
Notifications
You must be signed in to change notification settings - Fork 633
feat: add PodDisruptionBudget to kgateway helm chart #12173
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
Conversation
4638eb9 to
d6a4694
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds PodDisruptionBudget (PDB) support to the kgateway Helm chart to provide high availability guarantees during cluster maintenance operations.
- Add configurable PodDisruptionBudget resource with support for minAvailable, maxUnavailable, and unhealthyPodEvictionPolicy settings
- Update image pullPolicy default from empty string to "IfNotPresent" for consistency
- Include comprehensive JSON schema validation for PDB configuration
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| install/helm/kgateway/values.yaml | Add podDisruptionBudget configuration section and fix pullPolicy default |
| install/helm/kgateway/values.schema.json | Add complete JSON schema with PDB validation rules |
| install/helm/kgateway/templates/pdb.yaml | Create PDB template with conditional rendering |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,16 @@ | |||
| {{- if .Values.podDisruptionBudget }} | |||
Copilot
AI
Sep 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks if the podDisruptionBudget key exists, but it should check if it's non-empty. An empty object {} will evaluate to true, creating a PDB with no spec fields. Use {{- if and .Values.podDisruptionBudget (or .Values.podDisruptionBudget.minAvailable .Values.podDisruptionBudget.maxUnavailable) }} instead.
| {{- if .Values.podDisruptionBudget }} | |
| {{- if and .Values.podDisruptionBudget (or .Values.podDisruptionBudget.minAvailable .Values.podDisruptionBudget.maxUnavailable) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's incorrect.
In Helm's if expression, the empty object {} evaluates to false.
|
Hmm, it seems there was a temporary issue with quay.io. |
1fa1f99 to
a3ef116
Compare
Signed-off-by: frauniki <[email protected]>
Signed-off-by: frauniki <[email protected]>
|
@frauniki Thanks for taking a stab at this. I talked with a couple of maintainers offline, and we determined that PDB support needs more thought, discussion, etc as it has implications on the overall 2.x HA story, which is still being ironed out and actively worked on. At a minimum, we need a tracking issue for this and likely a design doc. Can you create an issue? No worries if not, I'll queue one up sometime this week. |
|
Closing as a result of #12173 (comment). I'll create a tracker for the PDB work. |
Description
Change Type
Changelog
Additional Notes