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>
WalkthroughIntroduces PodDisruptionBudget support to the lfx-v2-indexer-service Helm chart through a new template file with validation logic and corresponding configuration values, conditionally rendering the resource when enabled. 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-indexer-service Helm chart so operators can configure disruption limits via values.
Changes:
- Introduces
podDisruptionBudgetvalues (disabled by default) in the chart values. - Adds a new
PodDisruptionBudgettemplate that can renderminAvailableormaxUnavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| charts/lfx-v2-indexer-service/values.yaml | Adds configurable podDisruptionBudget values (enabled flag + knobs). |
| charts/lfx-v2-indexer-service/templates/pdb.yaml | Adds a PDB manifest gated behind podDisruptionBudget.enabled. |
💡 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.
minAvailable and maxUnavailable are mutually exclusive for PodDisruptionBudgets; if a user sets both values, this template will render both fields and Kubernetes will reject the manifest. Add a guard/validation to prevent both from being set at the same time (and provide a clear error message).
🤖 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>
🤖 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>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/lfx-v2-indexer-service/templates/pdb.yaml`:
- Around line 10-16: The PDB template hard-codes namespace and the app label;
update templates/pdb.yaml to reference values instead of literal strings by
replacing the namespace and selector.matchLabels.app with {{ .Values.namespace
}} and {{ .Values.appLabel }}, add corresponding keys (namespace and appLabel)
to values.yaml with appropriate defaults (e.g., "lfx" and
"lfx-v2-indexer-service"), and ensure deployment.yaml uses the same
.Values.appLabel for its pod labels/selector so the PDB selector exactly matches
the Deployment selector.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0de12a5b-3442-48e5-ba7a-7804bd6844a0
📒 Files selected for processing (2)
charts/lfx-v2-indexer-service/templates/pdb.yamlcharts/lfx-v2-indexer-service/values.yaml
Summary
podDisruptionBudgetvalues to Helm chart (disabled by default)minAvailableandmaxUnavailable🤖 Generated with Claude Code