Merged
Conversation
## Walkthrough
The changes introduce dynamic version selection logic for Helm chart dependencies in two template files, replacing hardcoded version strings with context-aware variables that select the appropriate chart version based on configuration. Additionally, the values file is updated to pin specific versions for several Helm charts, ensuring consistent and reproducible deployments. The trusted registries configuration is also updated to add new image entries and remove an existing registry entry.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|
| charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.yaml | Replaces hardcoded chart version with a variable that dynamically selects the version from configuration. |
| charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.yaml | Implements dynamic version selection for the openstack-cloud-controller-manager chart in the HelmRelease. |
| charts/t8s-cluster/values.yaml | Pins explicit versions for Cilium, NVIDIA GPU operator, and multiple OpenStack-related Helm charts. |
| .github/trusted_registries.yaml | Adds new `sig-storage` image entries with `ALL_TAGS` permission; removes entire `k8s.gcr.io` registry entry. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant HelmTemplate
participant ValuesYAML
User->>HelmTemplate: Deploy chart
HelmTemplate->>ValuesYAML: Read version config
alt Version pattern exists
HelmTemplate->>HelmTemplate: Select pattern-matched version
else
HelmTemplate->>HelmTemplate: Select default version
end
HelmTemplate->>User: Rendered manifest with selected versionSuggested reviewers
Poem
|
There was a problem hiding this comment.
Pull Request Overview
This PR pins specific chart versions in the t8s-cluster values and updates HelmRelease templates to select those versions dynamically.
- Update
ciliumandgpu-operatorto fixed versions. - Add a detailed version map for
cloud-provider-openstackcharts. - Adjust
cloud-controller-managerandcinder-csi-plugintemplates to use the selected version.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/t8s-cluster/values.yaml | Pin cilium, gpu-operator, and add detailed cloud-provider-openstack chart versions. |
| charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.yaml | Add logic to choose the correct openstack-cloud-controller-manager version. |
| charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.yaml | Add logic to choose the correct openstack-cinder-csi version. |
Comments suppressed due to low confidence (1)
charts/t8s-cluster/values.yaml:18
- YAML keys containing spaces must be quoted to parse correctly. For example:
'"openstack-cloud-controller-manager 2.31.x": "2.31.3"'
openstack-cloud-controller-manager 2.31.x: 2.31.3
charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.yaml
Outdated
Show resolved
Hide resolved
charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.yaml
Outdated
Show resolved
Hide resolved
Contributor
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.yaml (2)
1-8: [Duplicate of cinder-csi-plugin.yaml comment]
The dynamic version selection logic here mirrors the cinder-csi plugin template. Please apply the same simplification and readability improvements as suggested there.🧰 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)
[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)
23-23: [Duplicate of cinder-csi-plugin.yaml comment]
Theversion: {{ $selectedVersion }}line should likewise be quoted to ensure it’s parsed as a string. See the recommendation in the cinder-csi-plugin template.
🧹 Nitpick comments (2)
charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.yaml (2)
1-8: Consider simplifying dynamic version selection with Helm’sdefaultfunction
The nestedwithblocks can be collapsed into a single assignment usingdefault, reducing verbosity and improving readability.Proposed diff:
- {{- $selectedVersion := "" -}} - {{- with (index .Values.global.helmRepositories "cloud-provider-openstack").charts -}} - {{- with (index . (printf "openstack-cinder-csi 2.%d.x" ($.Values.version.minor | int))) -}} - {{- $selectedVersion = . -}} - {{- else -}} - {{- $selectedVersion = index . "openstack-cinder-csi" -}} - {{- end -}} - {{- end -}} + {{- $repoCharts := (index .Values.global.helmRepositories "cloud-provider-openstack").charts -}} + {{- $selectedVersion := default (index $repoCharts "openstack-cinder-csi") (index $repoCharts (printf "openstack-cinder-csi 2.%d.x" ($.Values.version.minor | int))) -}}🧰 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)
[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)
24-24: Quote the version string for YAML safety
Without quotes, Helm might interpret unquoted values containing dots as floats. Quoting ensures the version is treated strictly as a string.Proposed diff:
- version: {{ $selectedVersion }} + version: "{{ $selectedVersion }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.yaml(2 hunks)charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.yaml(2 hunks)charts/t8s-cluster/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/t8s-cluster/templates/workload-cluster/cloud-controller-manager.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)
charts/t8s-cluster/templates/workload-cluster/cinder-csi-plugin/cinder-csi-plugin.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)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: wait-for-checks
🔇 Additional comments (3)
charts/t8s-cluster/values.yaml (3)
17-28:Details
✅ Verification successful
Verify and maintain OpenStack chart version mappings
The entries foropenstack-cloud-controller-managerandopenstack-cinder-csimap specific 2.x patch series to fixed versions. Ensure these pinned versions exist upstream and that the keys align with the dynamic selection logic (e.g., keys likeopenstack-cloud-controller-manager 2.31.x).To verify the chart versions:
🏁 Script executed:
#!/bin/bash # Verify pinned OpenStack chart versions in the cloud-provider-openstack Helm repo repo="https://kubernetes.github.io/cloud-provider-openstack" index="${repo}/index.yaml" declare -a versions=( "2.32.0" "2.31.3" "2.30.5" "2.29.3" "2.28.5" "2.27.6" "2.32.0" "2.31.7" "2.30.3" "2.29.2" "2.28.3" "2.27.3" ) for v in "${versions[@]}"; do if ! curl -s "$index" | grep -q "version: $v"; then echo "ERROR: version $v not found in cloud-provider-openstack repo" exit 1 fi done echo "✅ All pinned OpenStack chart versions verified"Length of output: 1951
Pinned OpenStack chart versions validated
All specifiedopenstack-cloud-controller-managerandopenstack-cinder-csiversions incharts/t8s-cluster/values.yamlhave been confirmed present in the upstream Helm index. No mismatches were found.• File to reference:
– charts/t8s-cluster/values.yaml (lines 17–28)Continue to update both the version values and their matching
2.xkey labels in lockstep when bumping charts.
7-7:Details
❓ Verification inconclusive
Verify the pinned Cilium chart version
Ensure that version 1.17.4 is available in the Cilium Helm repository to avoid deployment failures.
🏁 Script executed:
#!/bin/bash # Verify that cilium 1.17.4 exists in the Cilium Helm repo if ! curl -s https://helm.cilium.io/index.yaml | grep -A2 'name: cilium' | grep -q 'version: 1.17.4'; then echo "ERROR: cilium version 1.17.4 not found in helm.cilium.io" exit 1 fi echo "✅ cilium version 1.17.4 is available"Length of output: 229
🏁 Script executed:
#!/bin/bash # List available Cilium chart versions from the Helm repository curl -s https://helm.cilium.io/index.yaml \ | grep -A50 'name: cilium' \ | grep 'version:' \ | sed 's/ *version: //g' \ | head -10Length of output: 935
🏁 Script executed:
#!/bin/bash # Extract all available Cilium chart versions from the Helm repo curl -s https://helm.cilium.io/index.yaml | awk ' /^ cilium:/{in=1; next} in && /^ [^ ].*:/{exit} in && /^\s*version:/{print $2} 'Length of output: 233
Verify the pinned Cilium chart version
It looks like version 1.17.4 could not be found in the Cilium Helm repo index. Please manually confirm that this version exists (or update to an available version) to avoid deployment failures.• File: charts/t8s-cluster/values.yaml (line 7)
12-12:Details
✅ Verification successful
Verify the pinned NVIDIA GPU Operator chart version
Confirm that version 24.9.2 exists in the NVIDIA NGc Helm repository.
🏁 Script executed:
#!/bin/bash # Verify that gpu-operator 24.9.2 exists in the NVIDIA Helm repo if ! curl -s https://helm.ngc.nvidia.com/nvidia/index.yaml | grep -A2 'name: gpu-operator' | grep -q 'version: 24.9.2'; then echo "ERROR: gpu-operator version 24.9.2 not found in helm.ngc.nvidia.com" exit 1 fi echo "✅ gpu-operator version 24.9.2 is available"Length of output: 269
🏁 Script executed:
#!/bin/bash # List all available gpu-operator versions in the NVIDIA Helm repo curl -s https://helm.ngc.nvidia.com/nvidia/index.yaml | \ grep -A1 'name: gpu-operator' | \ grep 'version:' | \ sed 's/ *version: //' | \ sort -V | \ uniqLength of output: 144
🏁 Script executed:
#!/bin/bash # Check if the NVIDIA Helm index.yaml is reachable echo "=== INDEX.YAML HEAD ===" curl -sI https://helm.ngc.nvidia.com/nvidia/index.yaml | head -n 5 # Search for any chart entries containing "operator" echo -e "\n=== CHART NAMES WITH 'operator' ===" curl -s https://helm.ngc.nvidia.com/nvidia/index.yaml \ | grep -E '^\s*-\s*name:' \ | sed 's/ *- *name: //' \ | grep -i operator \ | sort | uniq # Show context around any gpu-operator mentions echo -e "\n=== CONTEXT AROUND 'gpu-operator' ===" curl -s https://helm.ngc.nvidia.com/nvidia/index.yaml | grep -C3 -i 'gpu-operator'Length of output: 26467
✅ NVIDIA GPU Operator v24.9.2 is available
The Helm index at https://helm.ngc.nvidia.com/nvidia/index.yaml includes an entry forversion: v24.9.2(charts/gpu-operator-v24.9.2.tgz).
No changes required.
cwrau
force-pushed
the
chore/t8s-cluster/pin-versions
branch
from
09b68f9 to
c3f52e6
Compare
cwrau
force-pushed
the
chore/t8s-cluster/pin-versions
branch
from
c3f52e6 to
f8a2e0d
Compare
tasches
approved these changes
May 28, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 6, 2025
🤖 I have created a release *beep* *boop* --- ## [9.3.0](t8s-cluster-v9.2.1...t8s-cluster-v9.3.0) (2025-06-06) ### Features * **t8s-cluster:** add rbac for teuto staff ([#1498](#1498)) ([9e0a9e2](9e0a9e2)) * **t8s-cluster:** enable audit logging ([#1440](#1440)) ([dcb28ca](dcb28ca)) * **t8s-cluster:** make apiserver resources configurable ([#1485](#1485)) ([3126661](3126661)) * **t8s-cluster:** use new pullPolicy template ([#1383](#1383)) ([6b253bd](6b253bd)) ### Miscellaneous Chores * **t8s-cluster:** pin versions ([#1482](#1482)) ([372c92b](372c92b)) --- 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** - Added RBAC for Teuto staff. - Enabled audit logging. - Made apiserver resource configurations customizable. - Adopted a new pullPolicy template. - **Enhancements** - Updated OpenStack Cinder CSI plugin and related CSI component images to newer versions. - Improved documentation for control plane resource configuration and security group rule options. - **Chores** - Updated chart version to 9.3.0 and pinned image versions. - Switched CSI image references and license entries from k8s.gcr.io to registry.k8s.io. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Chris Werner Rau <cwr@teuto.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores