chore(base-cluster): change descheduler syntax #1483
Conversation
|
Warning Rate limit exceeded@cwrau has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 9 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
WalkthroughThe changes update the descheduler Helm chart version selection mechanism from a static, version-specific mapping to a dynamic, pattern-based lookup. The values.yaml file is restructured to use semantic versioning keys, and the template logic is simplified to select the appropriate version based on the Kubernetes minor version. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmRelease Template
participant Values.yaml
User->>HelmRelease Template: Deploy descheduler
HelmRelease Template->>Values.yaml: Retrieve Kubernetes minor version
HelmRelease Template->>Values.yaml: Lookup "descheduler 0.<minor>.x" version key
alt Key exists
Values.yaml-->>HelmRelease Template: Return specific version
else Key missing
Values.yaml-->>HelmRelease Template: Return default descheduler version
end
HelmRelease Template->>User: Deploys with selected version
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.
Pull Request Overview
This PR updates the descheduler configuration syntax in the base-cluster chart. The changes update the chart keys in the values file to use a new naming convention and adjust the template logic for selecting the appropriate descheduler version.
- Updated chart keys in values.yaml from "descheduler_27/28/29" to "descheduler 0.X.x"
- Refactored version selection logic in the descheduler template using a formatted key lookup
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/base-cluster/values.yaml | Updated descheduler chart keys to new naming format |
| charts/base-cluster/templates/descheduler/descheduler.yaml | Modified version lookup logic to match the new key naming convention |
Comments suppressed due to low confidence (1)
charts/base-cluster/templates/descheduler/descheduler.yaml:3
- Ensure that the new lookup for descheduler chart versions correctly handles all expected Kubernetes minor versions; consider adding tests to validate the key resolution and fallback behavior.
{{- with (index . (printf "descheduler 0.%d.x" ($.Capabilities.KubeVersion.Minor | int))) -}}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/base-cluster/templates/descheduler/descheduler.yaml (1)
3-4: Ensure robust conversion of Kubernetes minor version
If.Capabilities.KubeVersion.Minorever contains non-digits (e.g., “29+”), the| intcast can fail. Consider stripping non-numeric characters before conversion:- {{- with (index . (printf "descheduler 0.%d.x" ($.Capabilities.KubeVersion.Minor | int))) -}} + {{- $minor := regexReplaceAll "[^0-9]" $.Capabilities.KubeVersion.Minor | int -}} + {{- with (index . (printf "descheduler 0.%d.x" $minor)) -}}🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
charts/base-cluster/values.yaml (1)
124-126: Quote multi-word keys in thecharts.deschedulermapping
YAML keys containing spaces should be quoted to avoid parsing ambiguities:-charts: - descheduler: 0.32.2 - descheduler 0.29.x: 0.31.2 - descheduler 0.28.x: 0.30.2 - descheduler 0.27.x: 0.29.0 +charts: + descheduler: 0.32.2 + "descheduler 0.29.x": 0.31.2 + "descheduler 0.28.x": 0.30.2 + "descheduler 0.27.x": 0.29.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/templates/descheduler/descheduler.yaml(1 hunks)charts/base-cluster/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/descheduler/descheduler.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 4
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 2
(indentation)
🪛 GitHub Actions: Lint Helm Charts
charts/base-cluster/templates/descheduler/descheduler.yaml
[error] 6-6: Template parse error: function "descheduler" not defined.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generateDiffCommentBody
- GitHub Check: wait-for-checks
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
cwrau
force-pushed
the
chore/base-cluster/change-descheduler-syntax
branch
from
6266ca6 to
e0f6ddc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
charts/base-cluster/templates/descheduler/descheduler.yaml (1)
61-61:⚠️ Potential issueRemove extraneous
{{- end -}}
The trailing{{- end -}}on this line isn’t closing any open block, causing a Helm parse error during linting. Please remove it:- {{- end -}}🧰 Tools
🪛 GitHub Actions: Lint Helm Charts
[error] 61-61: Helm lint error: parse error at line 61 - unexpected {{end}}
♻️ Duplicate comments (1)
charts/base-cluster/templates/descheduler/descheduler.yaml (1)
5-7: Fallback lookup quoting reminder
You've correctly quoted the"descheduler"key in the fallback to avoid Helm parsing it as a function. This matches the previous recommendation to fix the parser error.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 4
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 2
(indentation)
🧹 Nitpick comments (1)
charts/base-cluster/templates/descheduler/descheduler.yaml (1)
1-4: Ensure fallback handling for missing chart mappings and simplify version lookup
Initializing$selectedVersionto an empty string will lead to an invalidversion:if.Values.global.helmRepositories.descheduler.chartsis unset or empty. You can both enforce the presence of the chart map and collapse the nestedwith/elseinto a singledefaultcall:-{{- $selectedVersion := "" -}} -{{- with .Values.global.helmRepositories.descheduler.charts -}} - {{- with (index . (printf "descheduler 0.%d.x" ($.Capabilities.KubeVersion.Minor | int))) -}} - {{- $selectedVersion = . -}} - {{- else -}} - {{- $selectedVersion = index . "descheduler" -}} - {{- end -}} -{{- end -}} +{{- $charts := required "global.helmRepositories.descheduler.charts must be set" .Values.global.helmRepositories.descheduler.charts -}} +{{- $key := printf "descheduler 0.%d.x" ($.Capabilities.KubeVersion.Minor | int) -}} +{{- $selectedVersion := default (index $charts "descheduler") (index $charts $key) -}}🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/base-cluster/templates/descheduler/descheduler.yaml(1 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/base-cluster/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/descheduler/descheduler.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 4
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 2
(indentation)
🪛 GitHub Actions: Lint Helm Charts
charts/base-cluster/templates/descheduler/descheduler.yaml
[error] 61-61: Helm lint error: parse error at line 61 - unexpected {{end}}
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generateDiffCommentBody
- GitHub Check: wait-for-checks
cwrau
force-pushed
the
chore/base-cluster/change-descheduler-syntax
branch
from
e0f6ddc to
0665e01
Compare
🤖 I have created a release *beep* *boop* --- ## [8.1.0](base-cluster-v8.0.0...base-cluster-v8.1.0) (2025-06-06) ### Features * **base-cluster/monitoring:** allow upsizing tempo storage ([#1448](#1448)) ([db1a742](db1a742)) * **base-cluster/monitoring:** also read secrets for datasources ([#1479](#1479)) ([83ba8bd](83ba8bd)) * **base-cluster/monitoring:** configure service graph for grafana ([#1422](#1422)) ([8d4bb4c](8d4bb4c)) * **base-cluster/monitoring:** set code challenge for grafana ([#1500](#1500)) ([aa803da](aa803da)) * **base-cluster/monitoring:** set code_challenge_method for oauth2-proxy ([#1496](#1496)) ([b252cd7](b252cd7)) ### Bug Fixes * **base-cluster:** this prevents the user from installing this under another name ([#1418](#1418)) ([f4807e8](f4807e8)) ### Miscellaneous Chores * **base-cluster/docs:** update flux helmrelease command to update CRDs ([#1421](#1421)) ([a8fd535](a8fd535)) * **base-cluster/monitoring:** remove unnecessary open-telemetry-collector dashboard ([#1449](#1449)) ([520e9e1](520e9e1)) * **base-cluster:** change descheduler syntax ([#1483](#1483)) ([907bdae](907bdae)) --- 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 - **New Features** - Enhanced monitoring with support for upsizing tempo storage. - Enabled reading secrets for datasources and configuring the service graph in Grafana. - Added options to set the code challenge and code_challenge_method for Grafana and oauth2-proxy. - Introduced an optional persistence configuration for tracing ingester storage size. - **Bug Fixes** - Resolved an issue preventing installation of the chart under unintended names. - **Chores** - Updated helmrelease command for CRD updates. - Removed an unnecessary dashboard and adjusted descheduler syntax. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit