Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Issue: LFXV2-1166 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughIntroduces PodDisruptionBudget configuration to the lfx-v2-query-service Helm chart through a new template and values schema. The template conditionally creates a PodDisruptionBudget resource with validation to prevent simultaneous configuration of both minAvailable and maxUnavailable fields. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds optional PodDisruptionBudget (PDB) support to the lfx-v2-query-service Helm chart so operators can limit voluntary disruptions during node drains/rollouts.
Changes:
- Introduces
podDisruptionBudgetconfiguration invalues.yaml(disabled by default). - Adds a
templates/pdb.yamlmanifest that renders apolicy/v1PodDisruptionBudget withminAvailable/maxUnavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/lfx-v2-query-service/values.yaml | Adds user-facing values for enabling/configuring the PDB. |
| charts/lfx-v2-query-service/templates/pdb.yaml | Adds the PDB Kubernetes manifest template gated by a values flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- with .Values.podDisruptionBudget.minAvailable }} | ||
| minAvailable: {{ . }} | ||
| {{- end }} | ||
| {{- with .Values.podDisruptionBudget.maxUnavailable }} | ||
| maxUnavailable: {{ . }} | ||
| {{- end }} |
There was a problem hiding this comment.
When podDisruptionBudget.enabled is true, this template can render an invalid PodDisruptionBudget if neither minAvailable nor maxUnavailable is set, and it can also render an invalid spec if both are set at the same time. Add template validation (e.g., fail/required logic) to enforce that exactly one of these fields is provided whenever the PDB is enabled.
| # minAvailable: 1 | ||
| # maxUnavailable: 1 |
There was a problem hiding this comment.
The values comments suggest both minAvailable and maxUnavailable are configurable, but Kubernetes requires exactly one of them to be set for a PodDisruptionBudget. Consider adding a brief note here (and/or explicit null defaults) clarifying that when enabled, users must set exactly one of these fields.
| # minAvailable: 1 | |
| # maxUnavailable: 1 | |
| # When enabled, you must set exactly one of minAvailable or maxUnavailable. | |
| minAvailable: null | |
| maxUnavailable: null |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Issue: LFXV2-1166 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Summary
podDisruptionBudgetvalues to Helm chart (disabled by default)minAvailableandmaxUnavailable🤖 Generated with Claude Code