Skip to content

fix(base-cluster/ingress): add missing prometheus block 🙄#1767

Merged
cwrau merged 2 commits intomainfrom
fix/base-cluster/traefik-settings
Oct 24, 2025
Merged

fix(base-cluster/ingress): add missing prometheus block 🙄#1767
cwrau merged 2 commits intomainfrom
fix/base-cluster/traefik-settings

Conversation

@cwrau
Copy link
Copy Markdown
Member

@cwrau cwrau commented Oct 24, 2025

Summary by CodeRabbit

Release Notes

  • Chores
    • Updated Traefik ingress controller configuration to specify non-default ingress class handling.
    • Reorganized metrics monitoring configuration structure for improved maintainability.

Copilot AI review requested due to automatic review settings October 24, 2025 13:27
@cwrau cwrau enabled auto-merge October 24, 2025 13:27
Copy link
Copy Markdown

Copilot AI left a 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 fixes a configuration issue in the Traefik ingress configuration by adding the missing prometheus block in the metrics section and explicitly setting the ingress class to not be the default.

Key Changes:

  • Added explicit ingressClass.isDefaultClass: false configuration
  • Restructured metrics configuration to properly nest the prometheus service monitor under a prometheus block

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread charts/base-cluster/templates/ingress/traefik.yaml
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 24, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Modified Traefik HelmRelease configuration in the base-cluster chart to add ingressClass setting with isDefaultClass: false and reorganize metrics configuration structure for Prometheus ServiceMonitor.

Changes

Cohort / File(s) Summary
Traefik Ingress Configuration
charts/base-cluster/templates/ingress/traefik.yaml
Added ingressClass section with isDefaultClass: false to Traefik HelmRelease spec; restructured metrics configuration under prometheus.serviceMonitor with nested enabled and additionalLabels fields

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested labels

base-cluster

Suggested reviewers

  • tasches
  • teutonet-bot
  • marvinWolff

Poem

🐰 A Traefik fine-tune, so neat and so clean,
With ingressClass off, no default to be seen,
Metrics restructured, all tidy and smart,
The helm chart evolves, a small engineering art! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "add missing prometheus block" is partially related to the changeset. The summary confirms that the changes include reorganizing metrics under the prometheus.serviceMonitor structure, which aligns with the title's focus on the prometheus block. However, the title does not capture the full scope of changes, as the summary also documents the addition of an ingressClass section with isDefaultClass: false. Additionally, the phrasing "add missing" may not precisely reflect that the prometheus metrics were reorganized while preserving values, rather than introducing entirely new functionality. Despite these limitations, the title does accurately refer to a real aspect of the change and is clear and specific.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/base-cluster/traefik-settings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6efd87b and 2a1eed4.

📒 Files selected for processing (1)
  • charts/base-cluster/templates/ingress/traefik.yaml (1 hunks)
⏰ 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: check licenses
  • GitHub Check: Update release-please config file for a possibly new chart
  • GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (2)
charts/base-cluster/templates/ingress/traefik.yaml (2)

57-58: ✓ ingressClass configuration is appropriate.

Setting isDefaultClass: false prevents this Traefik ingress from being the default IngressClass, which is a sensible approach for explicitly managed ingress controllers.


64-69: ✓ Metrics configuration now matches official Traefik Helm chart structure.

The nested structure metrics.prometheus.serviceMonitor with enabled and additionalLabels fields aligns with the official Traefik Helm chart template, which references .Values.metrics.prometheus.serviceMonitor.enabled and .Values.metrics.prometheus.serviceMonitor.additionalLabels. This is the correct and current configuration pattern for the Traefik Helm chart.

The past review comment questioning the removal of a top-level enabled field appears to be based on an earlier chart version structure. The current official values.yaml shows serviceMonitor.enabled is nested under the prometheus configuration, not at the metrics level.


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.

@cwrau cwrau added this pull request to the merge queue Oct 24, 2025
Merged via the queue into main with commit a329e1a Oct 24, 2025
32 of 33 checks passed
@cwrau cwrau deleted the fix/base-cluster/traefik-settings branch October 24, 2025 13:39
github-merge-queue Bot pushed a commit that referenced this pull request Oct 27, 2025
🤖 I have created a release *beep* *boop*
---


##
[10.0.1](base-cluster-v10.0.0...base-cluster-v10.0.1)
(2025-10-27)


### Bug Fixes

* **base-cluster/descheduler:** don't remove pods with too many restarts
([#1744](#1744))
([9c1ed51](9c1ed51))
* **base-cluster/ingress:** add missing `prometheus` block 🙄
([#1767](#1767))
([a329e1a](a329e1a))
* **base-cluster/loki:** adjust retention settings for loki logs
([#1745](#1745))
([1985d34](1985d34))
* **base-cluster/monitoring:** use the correct prometheus datasource id
([#1764](#1764))
([511cc84](511cc84))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Fixed descheduler to prevent removal of pods with excessive restart
counts
  * Added missing Prometheus monitoring configuration to ingress
  * Adjusted log retention settings in Loki
  * Corrected Prometheus datasource ID in monitoring

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants