Skip to content

feat: pod label configuration in cosmo router helm chart#2200

Merged
miklosbarabas merged 1 commit intomainfrom
miklos/eng-8118-pod-label-configuration-in-cosmo-router-helm-chart
Sep 10, 2025
Merged

feat: pod label configuration in cosmo router helm chart#2200
miklosbarabas merged 1 commit intomainfrom
miklos/eng-8118-pod-label-configuration-in-cosmo-router-helm-chart

Conversation

@miklosbarabas
Copy link
Copy Markdown
Contributor

@miklosbarabas miklosbarabas commented Sep 9, 2025

Summary by CodeRabbit

  • New Features
    • Added support for setting pod-level labels on the router via a new configuration option.
  • Documentation
    • Updated charts documentation to include the new pod label option and clarified label scopes.
  • Chores
    • Adjusted build cleanup command to use Unix-compatible removal for artifacts.

Checklist

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 9, 2025

Warning

Rate limit exceeded

@miklosbarabas has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 735372d and 491069e.

📒 Files selected for processing (7)
  • Makefile (1 hunks)
  • helm/cosmo/README.md (1 hunks)
  • helm/cosmo/charts/router/README.md (1 hunks)
  • helm/cosmo/charts/router/templates/_helpers.tpl (1 hunks)
  • helm/cosmo/charts/router/templates/deployment.yaml (1 hunks)
  • helm/cosmo/charts/router/values.yaml (1 hunks)
  • helm/cosmo/values.yaml (1 hunks)

Walkthrough

Switches a Makefile cleanup command from del to rm. Updates Helm values and docs to enable the router by default and introduces support for additional Pod labels in the router chart by adding a new values key, helper template, and wiring it into the Deployment Pod template labels.

Changes

Cohort / File(s) Summary of Changes
Build script cleanup
Makefile
In docker-build-minikube, replaced Windows-style deletion command with Unix-style: del mk-*.tarrm mk-*.tar.
Helm router enablement and labeling
helm/cosmo/values.yaml, helm/cosmo/README.md, helm/cosmo/charts/router/README.md, helm/cosmo/charts/router/values.yaml, helm/cosmo/charts/router/templates/_helpers.tpl, helm/cosmo/charts/router/templates/deployment.yaml
Enabled global.router.enabled by default. Added router.additionalPodLabels in top-level and chart values. Documented the new option and clarified label scopes. Introduced template router.additionalPodLabels and injected it into Deployment spec.template.metadata.labels.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The current pull request description consists solely of a generic contribution template and checklist without any details about the actual changes introduced by this PR, so it does not describe or relate to the changeset in any meaningful way. Please update the pull request description to include a clear summary of the implemented changes—such as enabling the Cosmo router by default, adding the additionalPodLabels Helm chart configuration, and updating the Makefile cleanup command—so reviewers can understand the intent and scope of this feature.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “feat: pod label configuration in cosmo router helm chart” clearly and concisely highlights the primary feature introduced by this changeset, namely the addition of pod label configuration to the Cosmo router Helm chart, without extraneous detail or noise. It follows the Conventional Commits format with an appropriate scope and accurately reflects the main intent of the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
helm/cosmo/README.md (1)

35-255: Regenerate chart docs to include router.additionalPodLabels and sync wording
Run make setup-dev-tools && helm-docs -c ./helm to update helm/cosmo/README.md and add the missing router.additionalPodLabels entry (CI is failing due to this omission and wording drift).

🧹 Nitpick comments (3)
helm/cosmo/values.yaml (1)

23-27: Update stale comment for router.enabled (now true).

Comment says “Disabled by default…” but default is true. Align to avoid confusion and incorrect generated docs.

Apply:

   router:
-    # -- Disabled by default because we don't have a token yet
+    # -- Enabled by default; set to false if you don't have a router token yet
     enabled: true
helm/cosmo/README.md (1)

152-152: Description mismatch: “Disabled by default…” while value is true.

This comes from values.yaml comments. Update the comment there (see suggested diff in helm/cosmo/values.yaml) and re-run helm-docs.

helm/cosmo/charts/router/templates/_helpers.tpl (1)

55-63: Optional: render labels in stable key order to reduce noisy diffs.

Go map iteration order is non-deterministic. Sorting keys makes manifests stable in diffs.

Apply:

-{{- define "router.additionalPodLabels" -}}
-{{- range $key, $value := .Values.additionalPodLabels }}
-{{ $key }}: {{ quote $value }}
-{{- end }}
-{{- end }}
+{{- define "router.additionalPodLabels" -}}
+{{- $keys := keys .Values.additionalPodLabels | sortAlpha -}}
+{{- range $key := $keys }}
+{{ $key }}: {{ quote (index $.Values.additionalPodLabels $key) }}
+{{- end }}
+{{- end }}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 735372d and 72909c5.

📒 Files selected for processing (7)
  • Makefile (1 hunks)
  • helm/cosmo/README.md (1 hunks)
  • helm/cosmo/charts/router/README.md (1 hunks)
  • helm/cosmo/charts/router/templates/_helpers.tpl (1 hunks)
  • helm/cosmo/charts/router/templates/deployment.yaml (1 hunks)
  • helm/cosmo/charts/router/values.yaml (1 hunks)
  • helm/cosmo/values.yaml (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Helm CI
helm/cosmo/charts/router/README.md

[error] 9-9: Diff detected by 'git diff --no-ext-diff --exit-code ./helm' affecting helm/cosmo/charts/router/README.md: updated description for 'additionalPodLabels' (from 'deployment pods' to 'deployment pod template').

helm/cosmo/README.md

[error] 241-241: Diff detected by 'git diff --no-ext-diff --exit-code ./helm' affecting helm/cosmo/README.md: added configuration line for 'router.additionalPodLabels'.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: build_test
🔇 Additional comments (4)
helm/cosmo/charts/router/templates/deployment.yaml (1)

31-34: Pod-label injection LGTM.

Correctly wires additional pod labels into template.metadata.labels with proper nindent.

helm/cosmo/charts/router/README.md (1)

11-12: Regenerate Helm docs: install helm-docs, run helm-docs -c ./helm, and commit the updated README to resolve the CI diff.

helm/cosmo/values.yaml (1)

255-257: New router.additionalPodLabels default looks good.

Public value added and documented; matches template usage.

helm/cosmo/charts/router/values.yaml (1)

17-22: Values keys and descriptions are clear and consistent.

Distinguishing deployment metadata vs. pod template labels is accurate.

Comment thread Makefile Outdated
@miklosbarabas miklosbarabas force-pushed the miklos/eng-8118-pod-label-configuration-in-cosmo-router-helm-chart branch from 72909c5 to 71f7d1a Compare September 9, 2025 22:35
@miklosbarabas miklosbarabas force-pushed the miklos/eng-8118-pod-label-configuration-in-cosmo-router-helm-chart branch from 71f7d1a to 491069e Compare September 9, 2025 22:37
@miklosbarabas
Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 9, 2025

✅ Actions performed

Summary regeneration triggered.

@miklosbarabas
Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 9, 2025

✅ Actions performed

Summary regeneration triggered.

@miklosbarabas miklosbarabas merged commit b5b13a1 into main Sep 10, 2025
10 checks passed
@miklosbarabas miklosbarabas deleted the miklos/eng-8118-pod-label-configuration-in-cosmo-router-helm-chart branch September 10, 2025 06:05
@Noroth Noroth mentioned this pull request Sep 30, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants