[LFXV2-1211] Add pod annotations and labels support#45
Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Issue: LFXV2-1211 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes add optional pod-level metadata configuration to a Kubernetes Deployment template. New Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 configurable pod-level metadata to the lfx-v2-indexer-service Helm chart so operators can attach custom annotations/labels to the Deployment’s Pod template.
Changes:
- Introduces
podAnnotationsandpodLabelsvalues (default empty maps). - Updates the Deployment template to render the configured Pod annotations and labels.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/lfx-v2-indexer-service/values.yaml | Adds new configurable values for pod annotations/labels. |
| charts/lfx-v2-indexer-service/templates/deployment.yaml | Renders podAnnotations and podLabels into the Pod template metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| labels: | ||
| app: lfx-v2-indexer-service | ||
| {{- with .Values.podLabels }} | ||
| {{- toYaml . | nindent 8 }} |
There was a problem hiding this comment.
podLabels are rendered after the required app: lfx-v2-indexer-service label. If a user sets podLabels.app, the rendered YAML will contain duplicate app keys and can override/break the pod label so it no longer matches spec.selector.matchLabels, causing the Deployment to stop managing its Pods. Consider preventing overrides of selector-critical labels (e.g., omit/ignore reserved keys like app, or explicitly merge labels giving precedence to the chart’s required labels and/or fail template rendering when a reserved key is provided).
| {{- toYaml . | nindent 8 }} | |
| {{- $podLabels := omit . "app" }} | |
| {{- toYaml $podLabels | nindent 8 }} |
|
|
||
| podAnnotations: {} | ||
|
|
There was a problem hiding this comment.
New top-level values podAnnotations and podLabels are added without any description, while the rest of values.yaml uses inline comments to document each section/field. Add brief comments (and ideally an example) explaining what these maps apply to (Pod template metadata) and any restrictions (e.g., avoiding selector labels like app).
| podAnnotations: {} | |
| # podAnnotations are applied to the Pod template metadata | |
| # Example: | |
| # podAnnotations: | |
| # prometheus.io/scrape: "true" | |
| podAnnotations: {} | |
| # podLabels are extra labels applied to the Pod template metadata | |
| # Avoid labels that are used in selectors (e.g. "app") to prevent rollout issues | |
| # Example: | |
| # podLabels: | |
| # role: "indexer-worker" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
charts/lfx-v2-indexer-service/values.yaml (1)
107-110: Consider adding documentation comments for consistency.Other values in this file include helpful comments (e.g.,
# nats is the configuration for the NATS server). Adding similar comments for the new values would maintain consistency and improve discoverability.📝 Suggested documentation
+# podAnnotations are annotations added to the pod template spec podAnnotations: {} +# podLabels are additional labels added to the pod template spec podLabels: {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/lfx-v2-indexer-service/values.yaml` around lines 107 - 110, The new top-level values podAnnotations and podLabels lack explanatory comments; add short documentation comments above each (matching the style used elsewhere in the file) explaining their purpose and expected format (e.g., "podAnnotations: annotations applied to the pod metadata (map[string]string)" and "podLabels: labels applied to the pod metadata (map[string]string)"), so users can discover and understand how to use these keys when overriding chart values.charts/lfx-v2-indexer-service/templates/deployment.yaml (1)
21-25: Potential conflict ifpodLabelscontainsappkey.If a user specifies
appinpodLabels, the resulting YAML would have duplicate keys. While most parsers take the last value, this could inadvertently break the Deployment's selector matching (Line 14).This is a common Helm pattern and low risk since users typically wouldn't override
app, but you could add a note in the values.yaml documentation warning against overriding selector labels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/lfx-v2-indexer-service/templates/deployment.yaml` around lines 21 - 25, The labels block can be shadowed if users include an "app" key in .Values.podLabels causing duplicate keys and selector mismatches; update the template where .Values.podLabels is rendered (labels: / app: lfx-v2-indexer-service / {{- with .Values.podLabels }}) to exclude the "app" key (e.g., pass the map through a filter such as omit "app" before toYaml) so the explicit app label is authoritative, and also add a short warning in values.yaml documentation telling users not to override the selector/app label to avoid breaking the Deployment selector.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@charts/lfx-v2-indexer-service/templates/deployment.yaml`:
- Around line 21-25: The labels block can be shadowed if users include an "app"
key in .Values.podLabels causing duplicate keys and selector mismatches; update
the template where .Values.podLabels is rendered (labels: / app:
lfx-v2-indexer-service / {{- with .Values.podLabels }}) to exclude the "app" key
(e.g., pass the map through a filter such as omit "app" before toYaml) so the
explicit app label is authoritative, and also add a short warning in values.yaml
documentation telling users not to override the selector/app label to avoid
breaking the Deployment selector.
In `@charts/lfx-v2-indexer-service/values.yaml`:
- Around line 107-110: The new top-level values podAnnotations and podLabels
lack explanatory comments; add short documentation comments above each (matching
the style used elsewhere in the file) explaining their purpose and expected
format (e.g., "podAnnotations: annotations applied to the pod metadata
(map[string]string)" and "podLabels: labels applied to the pod metadata
(map[string]string)"), so users can discover and understand how to use these
keys when overriding chart values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b722a87b-b3ff-4701-9d84-39ac58f9fe31
📒 Files selected for processing (2)
charts/lfx-v2-indexer-service/templates/deployment.yamlcharts/lfx-v2-indexer-service/values.yaml
🤖 Generated with [Claude Code](https://claude.com/claude-code) Issue: LFXV2-1211 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
Summary
podAnnotationsandpodLabelsvalues to Helm chart (default empty)🤖 Generated with Claude Code