chore(main): [bot] release base-cluster:8.0.0#1398
Conversation
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
d95ab9b to
17167ee
Compare
d48fbc7 to
52c9d61
Compare
adccb5f to
d4b34ad
Compare
|
""" WalkthroughThe pull request updates version numbers for the base-cluster component across multiple files. The JSON manifest and Helm chart file now reference version "8.0.0" instead of "7.2.1". The CHANGELOG.md records a new version entry dated May 27, 2025, detailing breaking changes including adding a traefik ingress option and migrating promtail to alloy, new features like ingress-nginx risk-level configuration and oauth-proxies renaming, bug fixes for cert-manager metrics collection and RBAC RoleBinding collisions, and miscellaneous formatting chores. Changes
Sequence Diagram(s)sequenceDiagram
participant Ingress as base-cluster/ingress
participant Monitoring as base-cluster/monitoring
participant Alloy as Alloy (new component)
Ingress->>Ingress: Add "traefik" option and set default
Monitoring->>Alloy: Migrate promtail metrics collection
Alloy-->>Monitoring: Provide metrics data
Assessment against linked issues
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/Chart.yaml (1)
22-22: Update Chart.yaml version field.
The chart’s version is updated to8.0.0, which is correct for the new release. However, note that thesourcesfield (line 19) still refers to the previous version (base-cluster-v7.2.1). For consistency and clarity in documentation, consider updating the source URL to reflect the new version if appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/release-please/manifest.json(1 hunks)charts/base-cluster/CHANGELOG.md(1 hunks)charts/base-cluster/Chart.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Update release-please config file for a possibly new chart
- GitHub Check: update metadata file for release
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (2)
.github/release-please/manifest.json (1)
1-2: Update JSON manifest version numbers.
The manifest now updates the entry for"charts/base-cluster"to"8.0.0", along with updates for other charts. This aligns with the new release version.charts/base-cluster/CHANGELOG.md (1)
3-14: New changelog entry for version 8.0.0 added.
The changelog clearly documents the breaking change—migrating promtail to alloy—as well as new features (e.g., using risk-level Critical for ingress-nginx) and a bug fix for certificate-manager metrics collection. This entry is well-structured and aligns with the release objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/base-cluster/README.md (1)
241-241: Source code link correctly updated but has a minor formatting issue.The source code link has been properly updated to point to the new version tag. However, there's a minor markdown formatting inconsistency - using an asterisk instead of a dash for the list item.
-* <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster> +- <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
241-241: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
charts/base-cluster/Chart.yaml (1)
52-52: Avoid Using the 'latest' Tag for the RendererThe
grafana-image-rendereris tagged aslatest. Using thelatesttag can lead to unpredictable updates in production. It’s recommended to pin the image to a specific version. For example:- - image: docker.io/grafana/grafana-image-renderer:latest + - image: docker.io/grafana/grafana-image-renderer:vX.Y.Z # pin to a specific, tested version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/Chart.yaml(3 hunks)charts/base-cluster/README.md(8 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
charts/base-cluster/README.md
241-241: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: generateDiffCommentBody
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (13)
charts/base-cluster/README.md (5)
3-3: Version update looks good.The version badge has been correctly updated to 8.0.0, matching the new chart version.
26-26: Installation command correctly updated for 8.x.x.The flux HelmRelease creation command has been properly updated to reference the new chart version range.
53-53: Manual installation command correctly updated for 8.x.x.The helm install command has been properly updated to use the new chart version range.
336-347: Migration notes correctly document the breaking changes.The migration notes clearly document the breaking changes in version 8.0.0, specifically:
- The migration from
loki-stackto the standardlokihelm chart- The namespace change from
lokitomonitoring- The replacement of
promtailandotel-collectorwithalloyThis provides users with the necessary information to understand the impact of upgrading.
2624-2667: Loki configuration changes align with migration notes.The configuration changes in the
monitoring.lokisection align with the migration notes. The section now includesresourcesPresetandresourcesproperties, and thepromtailsection is maintained as an optional configuration, which is consistent with the migration from promtail to alloy mentioned in the migration notes.charts/base-cluster/Chart.yaml (8)
19-20: Update Source URL for the New VersionThe source URL is now updated to point to the base-cluster-v8.0.0 directory. This correctly reflects the new release version. Please double-check that any tooling or scripts relying on the URL are updated as well.
22-22: Bump Chart Version to 8.0.0The chart version has been updated from 7.2.1 to 8.0.0 to signal a major release with breaking changes. Ensure that consumers of this chart are aware of the update and review their upgrade procedures.
27-27: Update External-DNS Image VersionThe external-dns image has been updated to
0.15.1-debian-12-r8. This minor version bump should bring in the latest fixes or improvements. Verify that this version is compatible with your current ingress setup.
28-32: Update Grafana-Tempo ImagesAll grafana-tempo image references have been updated to
2.7.1-debian-12-r4(for compactor, distributor, metrics-generator, querier, and ingester). This unifies the image version across multiple deployments. Ensure that these changes have been tested in your staging environment.
40-40: Update Metrics-Server Image VersionThe metrics-server image is now set to
0.7.2-debian-12-r18. Confirm that this version aligns with the Kubernetes version requirements (>=1.27.0) and that no breaking changes affect metrics collection.
54-54: Update Loki Image VersionThe loki image has been updated to
3.4.2, which appears to be a significant change from the previous version. Ensure that this upgrade is fully compatible with your logging setup and that any configuration changes required by Loki 3.x are properly handled.
68-68: Verify Cert-Manager Cainjector VersionThe cert-manager-cainjector image has been updated to
v1.17.1. Please confirm that this version is in sync with the other cert-manager components in your deployment to avoid any compatibility issues regarding metrics or admission review.
79-79: Update Ingress-Nginx Kube-Webhook-Certgen ImageThe image for the ingress-nginx kube-webhook-certgen component is updated to
registry.k8s.io/ingress-nginx/kube-webhook-certgen:v20221220-controller-v1.5.1-58-g787ea74b6.
Please verify that this update is compatible with the ingress-nginx controller configuration and does not introduce regressions in webhook functionality.
d079264 to
594e290
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/CHANGELOG.md (1)
3-19: Review of New Release 8.0.0 Entry
The new changelog entry for version 8.0.0 is well structured and accurately reflects the PR objectives:
- The header is properly formatted with a link comparing version v7.2.1 to v8.0.0 and the release date "2025-03-27" matches the expected release date.
- Under ⚠ BREAKING CHANGES, the migration of Promtail to Alloy is clearly noted.
- In the Features section, the new functionality—using risk-level Critical for ingress-nginx when annotations are enabled and renaming Alloy to a generic name—is documented with clear references to the associated issues.
- The Bug Fixes section correctly highlights the cert-manager metrics collection improvement.
One minor point to consider: the changelog includes two mentions of migrating Promtail to Alloy (once under breaking changes and again as a feature). Verify if this duplication is intentional (to emphasize the breaking nature as well as its functional update) or if one instance could potentially be removed for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/release-please/manifest.json(1 hunks)charts/base-cluster/CHANGELOG.md(1 hunks)charts/base-cluster/Chart.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/release-please/manifest.json
- charts/base-cluster/Chart.yaml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: update metadata file for release
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (1)
charts/base-cluster/CHANGELOG.md (1)
20-1110: Review of Historical Changelog Entries
The remaining sections (versions 7.2.1 and earlier) provide a complete historical record and are consistently formatted. All previous entries:
- Maintain consistent markdown headers and bullet formatting.
- Include clear commit references and issue links.
- Are in proper descending order, ensuring traceability of past changes.
No major issues were identified in these sections.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/README.md (1)
241-241: Source code link updated, but has a Markdown style inconsistency.The link has been correctly updated to point to the new version, but the list style uses an asterisk instead of a dash, which is inconsistent with the rest of the document.
-* <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster> -* <https://github.com/teutonet/teutonet-helm-charts/tree/main/charts/base-cluster> +- <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster> +- <https://github.com/teutonet/teutonet-helm-charts/tree/main/charts/base-cluster>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
241-241: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/Chart.yaml(3 hunks)charts/base-cluster/README.md(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/base-cluster/Chart.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
charts/base-cluster/README.md
241-241: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generateDiffCommentBody
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (5)
charts/base-cluster/README.md (5)
3-3: Version badge correctly updated to 8.0.0.The version badge has been properly updated to reflect the new version.
26-26: Command properly updated to use version 8.x.x.The flux create command has been correctly updated to reference the new chart version.
53-53: Helm install command properly updated to version 8.x.x.The helm install command has been correctly updated to reference the new chart version.
336-347: Comprehensive migration notes for the breaking changes.The migration notes clearly document the breaking changes in version 8.0.0:
- Migration from
loki-stackto standardlokihelm chart- Namespace change from
lokitomonitoring- Introduction of a new storage engine
- Replacement of
promtailandotel-collectorwithalloyThese notes are essential for users to understand how to migrate to the new version successfully.
2641-2667: Code structure changes related to the Promtail replacement.This section shows the structural changes necessary for the migration from Promtail to Alloy, as mentioned in the migration notes. The code maintains the same configuration pattern while referencing the new component.
4e2db00 to
9b217b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/CHANGELOG.md (1)
3-21: Review of the new 8.0.0 release entry:
- The new version entry for 8.0.0 is clearly delineated, showing the correct version number and release date (2025-03-27) as expected from the PR objectives.
- The BREAKING CHANGES section (line 6–8) appropriately highlights the migration of Promtail to Alloy, which is a critical breaking change.
- The Features section (lines 10–16) details several enhancements:
- It mentions that ingress-nginx now uses a risk-level of Critical when annotations are enabled.
- It documents the renaming of oauth-proxies to a cleaner name.
- It includes an additional mention of "migrate promtail to alloy" and the subsequent renaming of Alloy to be a generic name.
- Observation: The migration of Promtail to Alloy is noted in both the BREAKING CHANGES section and again in the Features section. This duplication might be intentional for emphasis, but it could also lead to redundancy. Consider clarifying or consolidating this point to improve clarity.
- The Bug Fixes section (lines 18–21) correctly notes the fix for cert-manager’s metrics collection issue.
Overall, the changelog entry is comprehensive and well organized. A minor nitpick is to compare the duplicate mention of the Promtail migration for consistency and clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/release-please/manifest.json(1 hunks)charts/base-cluster/CHANGELOG.md(1 hunks)charts/base-cluster/Chart.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/release-please/manifest.json
- charts/base-cluster/Chart.yaml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
3e154dc to
b15adbf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/base-cluster/CHANGELOG.md (2)
6-9: Breaking Changes Section
The "### ⚠ BREAKING CHANGES" header followed by the bullet indicating "* base-cluster/monitoring: migrate promtail to alloy" is clear and prominent. Consider adding a short note or a link to migration instructions (if available) to further assist users in adapting to this breaking change.
8-8: Duplicate Entry for Promtail Migration
The migration of promtail to alloy appears both under the breaking changes (line 8) and again as a feature (line 14). If this duplication is intentional to emphasize its significance both as a breaking change and as an enhancement, consider adding a clarifying note (for example, "Note: Although this change breaks backward compatibility, it also introduces new functionality...") to avoid potential confusion.Also applies to: 14-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/release-please/manifest.json(1 hunks)charts/base-cluster/CHANGELOG.md(1 hunks)charts/base-cluster/Chart.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/release-please/manifest.json
- charts/base-cluster/Chart.yaml
🔇 Additional comments (3)
charts/base-cluster/CHANGELOG.md (3)
3-4: New Version Entry Added
The new version entry for 8.0.0 is correctly formatted with a link comparing v7.2.1 to v8.0.0 and the release date (2025-04-14). This clearly marks the start of the release notes for this new version.
12-13: New Features Documentation
The features under the "### Features" section—specifically the ingress-nginx risk-level update (line 12) and the renaming of oauth-proxies (line 13)—are well documented with clear references to the corresponding GitHub issues and commits. This enhances traceability for users wishing to review the rationale behind these updates.
18-21: Bug Fix for Cert-Manager
The bug fix entry for cert-manager metrics collection (line 20) is clearly noted and includes references to the relevant issue and commit. This ensures users are aware of the improvements made to metrics collection in cert-manager.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/README.md (1)
241-241: Source URL updated, but inconsistent list style.The source URL has been correctly updated to point to the v8.0.0 tag. However, there's a minor markdown formatting issue with the list style.
-* <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster> +- <https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
241-241: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/Chart.yaml(3 hunks)charts/base-cluster/README.md(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/base-cluster/Chart.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
charts/base-cluster/README.md
241-241: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generateDiffCommentBody
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (4)
charts/base-cluster/README.md (4)
3-3: Version update correctly reflects the new release.The version badge has been updated to show 8.0.0, which accurately represents the new release version.
26-26: Flux commands appropriately updated with new version range.The Flux commands have been updated to use the 8.x.x version range, ensuring users will get the latest 8.x version when deploying.
Also applies to: 53-53
336-348: Migration documentation clearly explains breaking changes.The migration section for 7.x.x -> 8.0.0 effectively communicates the breaking changes:
- Migration from
loki-stacktolokihelm chart- Change in storage engine
- Deployment moved from
lokitomonitoringnamespace- Replacement of
promtailandotel-collectorwithalloyThis documentation is essential for users to understand the impact of upgrading.
2640-2667: Monitoring section updated to reflect changes in Loki component.The monitoring section has been updated to include the new resource settings for Loki that are compatible with the migration from promtail to alloy. This ensures that users have the proper configuration options available.
7d8330b to
52a0f99
Compare
d6ddcf8 to
ad00b67
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
charts/base-cluster/CHANGELOG.md (1)
13-17: Avoid duplicating breaking changes in Features
Theadd option traefikandmigrate promtail to alloyentries appear under both ⚠ BREAKING CHANGES and Features. This redundancy can confuse readers—consider removing or consolidating these in the Features section.charts/base-cluster/README.md (3)
249-251: Consistent unordered list style
The source code URLs use*for list items, but project markdownlint expects-. Consider switching to dashes:- * https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster - * https://github.com/teutonet/teutonet-helm-charts/tree/main/charts/base-cluster + - https://github.com/teutonet/teutonet-helm-charts/tree/base-cluster-v8.0.0/charts/base-cluster + - https://github.com/teutonet/teutonet-helm-charts/tree/main/charts/base-cluster🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
249-249: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
250-250: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
333-334: Typo in wording: "you cluster" → "your cluster"
Correct the grammar in the sentence to improve readability.- This also makes kyverno HA, so be aware that kyverno will need more resources in you cluster. + This also makes kyverno HA, so be aware that kyverno will need more resources in your cluster.
3857-3859: Switch list bullets to dashes per markdownlint
Lines 3857–3859 use*for list items; change to-to comply with MD004.- * "nginx" - * "traefik" - * "none" + - "nginx" + - "traefik" + - "none"🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3857-3857: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
3858-3858: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
3859-3859: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/release-please/manifest.json(1 hunks)charts/base-cluster/CHANGELOG.md(1 hunks)charts/base-cluster/Chart.yaml(4 hunks)charts/base-cluster/README.md(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/release-please/manifest.json
- charts/base-cluster/Chart.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
charts/base-cluster/README.md
249-249: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
3857-3857: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
3858-3858: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
3859-3859: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generateDiffCommentBody
- GitHub Check: lint helm chart (base-cluster)
🔇 Additional comments (14)
charts/base-cluster/CHANGELOG.md (5)
3-3: Approve new version entry
The## [8.0.0]…(2025-05-27)header is correctly formatted and references the right compare link.
6-6: Approve BREAKING CHANGES header
The### ⚠ BREAKING CHANGESsection follows established style and clearly signals breaking updates.
8-9: Approve breaking‐change bullets
Bothingress: add option traefik…andmonitoring: migrate promtail to alloyare correctly captured as breaking changes.
22-22: Approve Bug Fix entry for cert-manager
The fix for metrics collection is clear and correctly linked.
28-30: Approve Miscellaneous Chores entries
The formatting, plugin removal, and version-pinning chores are well documented.charts/base-cluster/README.md (9)
3-3: Version badge updated to reflect v8.0.0
The Helm chart version badge now correctly shows 8.0.0.
111-120: Ingress documentation updated to include Traefik support
The chart now supports bothnginxandtraefikas ingress controllers, with details on metrics, tracing, and Gateway API support. Looks good.
134-134: Clarified IP retrieval for both ingress controllers
The command now covers bothnginxandtraefikservice names, improving clarity for end users.
336-336: Add migration section 6.x.x → 7.0.0
Good addition summarizing the breaking changes and role collisions for the 6.x → 7.0.0 migration.
756-758: Schema updated formetricsLabels,dnsLabels, andingressLabels
The properties now allow a union type (object|string) via a oneOf. This aligns with the migration notes and adds flexibility.
2734-2741:monitoring.loki.resourcesPresetdocumentation added
The new enum property for resource presets is well documented and references the global definition.
2749-2754:monitoring.loki.promtailsection introduced
Optional Promtail configuration is now documented undermonitoring.loki.promtail. Looks correct.
2761-2769: NestedresourcesPresetandresourcesfor Promtail
The nested properties correctly reference existing definitions and clarify resource requirements.
3816-3816: Newproviderenum underingressconfiguration
Addingproviderto enum values (nginx,traefik,none) is necessary to support the new default. Well done.
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
|
🤖 Created releases: |
🤖 I have created a release beep boop
8.0.0 (2025-05-27)
⚠ BREAKING CHANGES
Features
Bug Fixes
Miscellaneous Chores
This PR was generated with Release Please. See documentation.
Summary by CodeRabbit
New Features
Breaking Changes
Bug Fixes
Chores