Conversation
cwrau
requested review from
marvinWolff,
tasches and
teutonet-bot
as code owners
|
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 35 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 (6)
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughUpgrade of Cluster API template manifests and Helm helpers: apiVersions bumped from v1beta1 to v1beta2, many resource Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl (1)
2-2: Optional: centralize all CAPI apiVersions via helpersConsider adding similar helpers for bootstrap/controlplane/addons to avoid hard-coding versions in templates.
charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml (1)
3-3: Verify provider support for v1beta2 (especially K0sWorkerConfigTemplate)Confirm the bootstrap providers used (kubeadm and k0smotron) support bootstrap.cluster.x-k8s.io/v1beta2 for the selected kinds; if not, gate the apiVersion via a helper per provider.
charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml (1)
2-2: Remove deadif falsebranch to reduce driftSince the else-branch already uses the centralized helper, drop the hard-coded, unreachable branch to prevent future divergence.
Apply this diff:
-{{- if false }} -apiVersion: infrastructure.cluster.x-k8s.io/v1beta2 -{{- else }} -apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} -{{- end }} +apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }}charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml (1)
4-4: No-op change; consider removing dead branch.This apiVersion bump sits under
{{- if false }}and will never render. To avoid confusion, drop the dead branch or add a brief comment stating why it’s kept.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml(4 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml(1 hunks)charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.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). (1)
- GitHub Check: lint helm chart (t8s-cluster)
🔇 Additional comments (6)
charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml (1)
5-5: Sanity check: runtime tools present in kubectl imageThe script relies on jq and base64. Please confirm the configured kubectl image includes them.
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl (1)
2-2: LGTM: infra API bumped to v1beta2The helper centralizes the infra API version; change looks correct.
charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml (1)
2-2: LGTM: KCPTemplate apiVersion v1beta2Looks consistent with the migration.
Please ensure the controlplane.cluster.x-k8s.io v1beta2 CRDs are installed in the management cluster before deploying.
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (3)
5-5: apiVersion bump to v1beta2 looks good.
32-39: ControlPlane ref apiVersion → v1beta2 is correct.
167-167: Bootstrap ref apiVersion → v1beta2 is correct.
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl
Outdated
Show resolved
Hide resolved
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml
Outdated
Show resolved
Hide resolved
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml
Outdated
Show resolved
Hide resolved
charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml
Outdated
Show resolved
Hide resolved
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
b2cc722 to
f5bdbd3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the t8s-cluster chart from Cluster API v1beta1 to v1beta2, updating API versions and timeout configurations to align with the newer API specification.
- Updates all Cluster API references from v1beta1 to v1beta2 across templates and helpers
- Migrates timeout configurations from time.Duration format to seconds-based fields with new default values
- Restructures machine health check configurations to use the new v1beta2 schema format
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml | Updates ClusterResourceSet lookup to use v1beta2 API version |
| charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml | Updates infrastructure API version to v1beta2 |
| charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml | Updates infrastructure API version to v1beta2 |
| charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml | Updates control plane API version to v1beta2 |
| charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml | Major restructuring of health check configurations and timeout fields for v1beta2 compatibility |
| charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml | Updates bootstrap API version to v1beta2 |
| charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl | Updates infrastructure API version helper to v1beta2 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml
Show resolved
Hide resolved
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
f5bdbd3 to
60d0ba4
Compare
There was a problem hiding this comment.
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 (3)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (3)
34-42: Rename controlPlane.ref → controlPlane.templateRef (v1beta2).Required by v1beta2 ClusterClass schema. (main.cluster-api.sigs.k8s.io)
- ref: + templateRef: apiVersion: controlplane.cluster.x-k8s.io/v1beta2 {{- if .Values.controlPlane.hosted }} kind: K0smotronControlPlaneTemplate name: {{/* the full context is needed for .Files.Get */}}{{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.k0smotronControlPlaneTemplate.specHash" .) }} {{- else }} kind: KubeadmControlPlaneTemplate name: {{/* the full context is needed for .Files.Get */}}{{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.kubeadmControlPlaneTemplate.specHash" .) }} {{- end }}
44-47: Rename infrastructure.ref → infrastructure.templateRef (v1beta2).v1beta2 uses templateRef for template references. (main.cluster-api.sigs.k8s.io)
infrastructure: - ref: + templateRef: apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} kind: OpenStackClusterTemplate name: {{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackClusterTemplate.specHash" (dict "context" $)) }}
170-181: *Workers: migrate template.bootstrap.ref / template.infrastructure.ref to top‑level .templateRef (v1beta2).In v1beta2, bootstrap and infrastructure template references move out of template.* and use templateRef. (main.cluster-api.sigs.k8s.io)
- template: - bootstrap: - ref: - apiVersion: bootstrap.cluster.x-k8s.io/v1beta2 - kind: {{ $.Values.controlPlane.hosted | ternary "K0sWorkerConfigTemplate" "KubeadmConfigTemplate" }} - name: {{ printf "%s-%s-compute-plane" $.Release.Name ($isGpuDeploymentClass | ternary "gpu" "standard") }} - infrastructure: - ref: - apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} - kind: OpenStackMachineTemplate - name: {{ printf "%s-compute-plane-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackMachineTemplate.specHashOfNodePools" (dict "context" $)) }} + bootstrap: + templateRef: + apiVersion: bootstrap.cluster.x-k8s.io/v1beta2 + kind: {{ $.Values.controlPlane.hosted | ternary "K0sWorkerConfigTemplate" "KubeadmConfigTemplate" }} + name: {{ printf "%s-%s-compute-plane" $.Release.Name ($isGpuDeploymentClass | ternary "gpu" "standard") }} + infrastructure: + templateRef: + apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} + kind: OpenStackMachineTemplate + name: {{ printf "%s-compute-plane-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackMachineTemplate.specHashOfNodePools" (dict "context" $)) }}
♻️ Duplicate comments (2)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (2)
144-169: Workers: fix v1beta2 healthCheck name and move timeouts under deletion.
- Rename checks.unhealthyConditions → checks.unhealthyNodeConditions.
- Move nodeDrainTimeoutSeconds and nodeDeletionTimeoutSeconds into deletion. (main.cluster-api.sigs.k8s.io)
healthCheck: checks: nodeStartupTimeoutSeconds: 480 - unhealthyConditions: - - status: Unknown - timeoutSeconds: 300 - type: Ready - - status: 'False' - timeoutSeconds: 300 - type: Ready - nodeDrainTimeoutSeconds: 480 - nodeDeletionTimeoutSeconds: 900 + unhealthyNodeConditions: + - type: Ready + status: Unknown + timeoutSeconds: 300 + - type: Ready + status: "False" + timeoutSeconds: 300 deletion: order: Oldest + nodeDrainTimeoutSeconds: 480 + nodeDeletionTimeoutSeconds: 900 rollout: strategy: type: RollingUpdate
14-27: Fix v1beta2 schema: use unhealthyNodeConditions and move drain timeout under deletion.In v1beta2, checks are under healthCheck.checks.unhealthyNodeConditions (not unhealthyConditions), and timeouts like nodeDrainTimeoutSeconds belong under deletion. This will otherwise fail validation. (main.cluster-api.sigs.k8s.io)
Apply:
healthCheck: remediation: triggerIf: unhealthyLessThanOrEqualTo: 1 checks: nodeStartupTimeoutSeconds: 600 - unhealthyConditions: - - status: Unknown - timeoutSeconds: 600 - type: Ready - - status: 'False' - timeoutSeconds: 600 - type: Ready - nodeDrainTimeoutSeconds: 480 + unhealthyNodeConditions: + - type: Ready + status: Unknown + timeoutSeconds: 600 + - type: Ready + status: "False" + timeoutSeconds: 600 + deletion: + nodeDrainTimeoutSeconds: 480
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml(3 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml(1 hunks)charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl
⏰ 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). (1)
- GitHub Check: lint helm chart (t8s-cluster)
🔇 Additional comments (2)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (2)
5-5: ClusterClass apiVersion bump to v1beta2 looks correct.
35-35: controlplane.cluster.x-k8s.io/v1beta2 looks right for KCP templates.Please confirm your referenced control plane provider versions (e.g., KCP/K0smotron) support v1beta2 in the target management cluster release.
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml
Show resolved
Hide resolved
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
60d0ba4 to
30752bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (4)
34-42: Rename controlPlane.ref → controlPlane.templateRef (v1beta2).Top‑level controlPlane uses templateRef in v1beta2.
- ref: + templateRef: apiVersion: controlplane.cluster.x-k8s.io/v1beta2 {{- if .Values.controlPlane.hosted }} kind: K0smotronControlPlaneTemplate name: {{/* the full context is needed for .Files.Get */}}{{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.k0smotronControlPlaneTemplate.specHash" .) }} {{- else }} kind: KubeadmControlPlaneTemplate name: {{/* the full context is needed for .Files.Get */}}{{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.kubeadmControlPlaneTemplate.specHash" .) }} {{- end }}Source: v1beta2 ClusterClass docs. (cluster-api.sigs.k8s.io)
43-47: Rename infrastructure.ref → infrastructure.templateRef (v1beta2).Same contract change applies to the top‑level infrastructure reference.
- infrastructure: - ref: + infrastructure: + templateRef: apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} kind: OpenStackClusterTemplate name: {{ printf "%s-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackClusterTemplate.specHash" (dict "context" $)) }}Docs example shows templateRef. (cluster-api.sigs.k8s.io)
170-180: Workers templates: use templateRef for bootstrap/infrastructure (v1beta2).template: bootstrap: - ref: + templateRef: apiVersion: bootstrap.cluster.x-k8s.io/v1beta2 kind: {{ $.Values.controlPlane.hosted | ternary "K0sWorkerConfigTemplate" "KubeadmConfigTemplate" }} name: {{ printf "%s-%s-compute-plane" $.Release.Name ($isGpuDeploymentClass | ternary "gpu" "standard") }} infrastructure: - ref: + templateRef: apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} kind: OpenStackMachineTemplate name: {{ printf "%s-compute-plane-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackMachineTemplate.specHashOfNodePools" (dict "context" $)) }}Docs show templateRef under workers.template.* in v1beta2. (cluster-api.sigs.k8s.io)
5-181: Fix ClusterClass v1beta2 schema: replace ref→templateRef, rename unhealthyConditions→unhealthyNodeConditions, and move deletion timeouts.ClusterClass in this chart is using v1beta2 CRD shape but contains legacy v1beta1 keys — update the following in charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml:
- Replace all template references under spec that use "ref:" with "templateRef:" (controlPlane.machineInfrastructure.ref, controlPlane.ref, infrastructure.ref, workers..bootstrap.ref, workers..infrastructure.ref). (cluster-api.sigs.k8s.io)
- Rename health-check key "unhealthyConditions" → "unhealthyNodeConditions" in controlPlane.healthCheck and workers.machineDeployments[].healthCheck (inner timeouts should be timeoutSeconds — your file already uses timeoutSeconds). (main.cluster-api.sigs.k8s.io)
- Move nodeDrainTimeoutSeconds / nodeDeletionTimeoutSeconds into the deletion: block (controlPlane.deletion and workers.machineDeployments[].deletion) instead of at the same level as healthCheck. (main.cluster-api.sigs.k8s.io)
Occurrences were found around the controlPlane / infrastructure / workers sections by the provided script run; these are schema-breaking for v1beta2 and must be fixed before applying the ClusterClass.
♻️ Duplicate comments (3)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (3)
14-27: v1beta2 healthCheck keys + deletion block are off: rename unhealthyConditions and move drain timeout under deletion.
- Use checks.unhealthyNodeConditions (not unhealthyConditions).
- Move nodeDrainTimeoutSeconds under a sibling deletion block.
Apply:
healthCheck: remediation: triggerIf: unhealthyLessThanOrEqualTo: 1 checks: nodeStartupTimeoutSeconds: 600 - unhealthyConditions: + unhealthyNodeConditions: - status: Unknown timeoutSeconds: 600 type: Ready - status: 'False' timeoutSeconds: 600 type: Ready - nodeDrainTimeoutSeconds: 480 + deletion: + nodeDrainTimeoutSeconds: 480Refs: ClusterClass v1beta2 examples and migration notes. (cluster-api.sigs.k8s.io)
28-33: Rename machineInfrastructure.ref → machineInfrastructure.templateRef (v1beta2).Current key won’t validate under v1beta2.
machineInfrastructure: - ref: + templateRef: apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} kind: OpenStackMachineTemplate name: {{ printf "%s-control-plane-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackMachineTemplate.specHashOfControlPlane" (dict "context" $)) }}Spec reference: v1beta2 ClusterClass “Basic” example. (cluster-api.sigs.k8s.io)
144-156: Workers: rename unhealthyConditions → unhealthyNodeConditions; move timeouts under deletion.
- healthCheck.checks must use unhealthyNodeConditions.
- nodeDrainTimeoutSeconds and nodeDeletionTimeoutSeconds belong under deletion.
healthCheck: checks: nodeStartupTimeoutSeconds: 480 - unhealthyConditions: + unhealthyNodeConditions: - status: Unknown timeoutSeconds: 300 type: Ready - status: 'False' timeoutSeconds: 300 type: Ready - nodeDrainTimeoutSeconds: 480 - nodeDeletionTimeoutSeconds: 900 - deletion: - order: Oldest + deletion: + nodeDrainTimeoutSeconds: 480 + nodeDeletionTimeoutSeconds: 900 + order: OldestRefs: v1beta2 healthCheck schema and “fields for Machine deletion are under a new deletion struct”. (cluster-api.sigs.k8s.io)
🧹 Nitpick comments (1)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (1)
14-26: Optional: confirm remediation trigger threshold intent.You set unhealthyLessThanOrEqualTo: 1 for controlPlane. If the intent is percentage-based (e.g., 33% of CP Machines), switch to a string like "33%".
Migration notes/examples show both numeric and percentage forms. (cluster-api.sigs.k8s.io)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml(3 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml(1 hunks)charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml
- charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl
- charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml
⏰ 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). (1)
- GitHub Check: lint helm chart (t8s-cluster)
🔇 Additional comments (1)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (1)
159-169: Rollout block looks fine.Strategy fields and values are valid; no action needed.
If you intend different surge/unavailable for GPU classes, confirm the gating logic via a quick render (helm template) to ensure the ternary behaves as expected.
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
30752bd to
6052d53
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (1)
1-181: Replace v1beta1-era keys and relocate mis-nested timeouts
- Replace unhealthyConditions → unhealthyNodeConditions in charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (lines 20, 147).
- Move nodeDrainTimeoutSeconds and nodeDeletionTimeoutSeconds into the existing deletion: block for the machineDeployment (currently lines 154–155), preserving correct YAML indentation and schema placement.
♻️ Duplicate comments (3)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (3)
14-27: v1beta2 schema fix: use unhealthyNodeConditions and move drain timeout under deletion.
- Rename
checks.unhealthyConditions→checks.unhealthyNodeConditions.- Move
nodeDrainTimeoutSecondsunderdeletion:.healthCheck: remediation: triggerIf: unhealthyLessThanOrEqualTo: 1 checks: nodeStartupTimeoutSeconds: 600 - unhealthyConditions: - - status: Unknown - timeoutSeconds: 600 - type: Ready - - status: 'False' - timeoutSeconds: 600 - type: Ready - nodeDrainTimeoutSeconds: 480 + unhealthyNodeConditions: + - type: Ready + status: Unknown + timeoutSeconds: 600 + - type: Ready + status: "False" + timeoutSeconds: 600 + deletion: + nodeDrainTimeoutSeconds: 480
28-33: v1beta2 rename: machineInfrastructure.ref → machineInfrastructure.templateRef.Required by v1beta2. Current key won’t validate.
- machineInfrastructure: - ref: + machineInfrastructure: + templateRef: apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} kind: OpenStackMachineTemplate name: {{ printf "%s-control-plane-%s" $.Release.Name (include "t8s-cluster.clusterClass.openStackMachineTemplate.specHashOfControlPlane" (dict "context" $)) }}
144-169: Workers v1beta2 alignment: rename to unhealthyNodeConditions and consolidate timeouts under deletion.
healthCheck.checks.unhealthyConditions→unhealthyNodeConditions.- Move
nodeDrainTimeoutSeconds+nodeDeletionTimeoutSecondsinto the existingdeletion:block (keeporder: Oldestthere).- healthCheck: - checks: - nodeStartupTimeoutSeconds: 480 - unhealthyConditions: - - status: Unknown - timeoutSeconds: 300 - type: Ready - - status: 'False' - timeoutSeconds: 300 - type: Ready - nodeDrainTimeoutSeconds: 480 - nodeDeletionTimeoutSeconds: 900 - deletion: - order: Oldest + healthCheck: + checks: + nodeStartupTimeoutSeconds: 480 + unhealthyNodeConditions: + - type: Ready + status: Unknown + timeoutSeconds: 300 + - type: Ready + status: "False" + timeoutSeconds: 300 + deletion: + nodeDrainTimeoutSeconds: 480 + nodeDeletionTimeoutSeconds: 900 + order: Oldest
🧹 Nitpick comments (1)
charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml (1)
1-5: Remove dead if-branch; keep single source of truth for apiVersion.
{{- if false }}never renders. Drop the branch and rely solely on the helper to avoid stale version drift.-{{- if false }} -apiVersion: infrastructure.cluster.x-k8s.io/v1beta2 -{{- else }} -apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }} -{{- end }} +apiVersion: {{ include "t8s-cluster.clusterClass.infrastructureApiVersion" (dict) }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml(3 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackClusterTemplate/openStackClusterTemplate.yaml(1 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml(1 hunks)charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/t8s-cluster/templates/management-cluster/clusterClass/bootstrapConfigTemplate/_bootstrapConfigTemplate.yaml
- charts/t8s-cluster/templates/workload-cluster/pre-install/_uninstall-job.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/openStackMachineTemplates/openStackMachineTemplates.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml
- charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl
🔇 Additional comments (3)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (3)
5-5: LGTM: ClusterClass moved to v1beta2.
35-35: LGTM: control plane template apiVersion → v1beta2.
173-173: LGTM: bootstrap template apiVersion → v1beta2.
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
6052d53 to
10cd42d
Compare
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
10cd42d to
65a78ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml(4 hunks)charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/t8s-cluster/templates/management-cluster/clusterClass/kubeadmControlPlaneTemplate/kubeadmControlPlaneTemplate.yaml
⏰ 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). (1)
- GitHub Check: lint helm chart (t8s-cluster)
🔇 Additional comments (6)
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml (6)
5-11: API version successfully updated to v1beta2.The ClusterClass apiVersion has been properly upgraded from v1beta1 to v1beta2.
29-43: templateRef migration for controlPlane and machineInfrastructure is correct.The migration from
reftotemplateRefis properly implemented with appropriate apiVersion handling (v1alpha1 for hosted, v1beta2 for regular control planes). The internal structure (apiVersion, kind, name) and Helm templating are preserved.
44-48: Infrastructure templateRef migration is correct.The transition from
reftotemplateReffor OpenStackClusterTemplate is properly implemented with apiVersion and Helm templating preserved.
171-180: Bootstrap and infrastructure templateRef migration for machineDeployments is correct.The migration from
reftotemplateRefis properly implemented for both bootstrap and infrastructure references within machineDeployments:
- Bootstrap apiVersion updated to
bootstrap.cluster.x-k8s.io/v1beta2✓- Infrastructure templateRef correctly references OpenStackMachineTemplate with v1beta1 apiVersion ✓
- Helm templating for name generation and GPU-conditional references preserved ✓
159-170: Rollout strategy is valid for v1beta2 MachineDeploymentClass.maxSurge and maxUnavailable support absolute numbers or percentages of desired machines. The code correctly uses:
- GPU path:
maxSurge: 0, maxUnavailable: 1(absolute values)- Non-GPU path:
maxSurge: 75%, maxUnavailable: 0(percentage and absolute)Both cannot be zero simultaneously, and your configuration respects this constraint in both branches. The schema validation is complete; the TODO comment about beta-phase logic is a separate operational concern outside schema scope.
145-158: ---machineDeploymentClass healthCheck structure verified as correct for v1beta2.
The healthCheck block in machineDeploymentClass does not require a remediation sub-block. The remediation (remediationTemplate) is optional for machineDeploymentClass, which explains the structural difference from controlPlane. The code at lines 145-158 is compliant with v1beta2 ClusterClass specification.
charts/t8s-cluster/templates/management-cluster/clusterClass/clusterClass.yaml
Show resolved
Hide resolved
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
65a78ef to
d944df9
Compare
cwrau
force-pushed
the
feat/t8s-cluster/migrate-to-capi-v1beta2
branch
from
d944df9 to
1c2a752
Compare
🤖 I have created a release *beep* *boop* --- ## [9.5.0](t8s-cluster-v9.4.1...t8s-cluster-v9.5.0) (2026-01-15) ### Features * **t8s-cluster/artifacthub:** use centralised helmRepositories template ([#1846](#1846)) ([73a41f9](73a41f9)) * **t8s-cluster/cilium:** enable kubeProxy replacement ([#1815](#1815)) ([b3c412d](b3c412d)) * **t8s-cluster/management-cluster:** add cluster-autoscaler deployment ([#1756](#1756)) ([5b6ead9](5b6ead9)) * **t8s-cluster/management-cluster:** enable ImageVolume feature flag ([#1786](#1786)) ([9676ee0](9676ee0)) * **t8s-cluster/management-cluster:** set apiServerLoadBalancer.provider via TeutonetesCloud ([#1898](#1898)) ([6bf8889](6bf8889)) * **t8s-cluster/management-cluster:** switch to hcp ([#1759](#1759)) ([303b0b6](303b0b6)) * **t8s-cluster/management-cluster:** use new KubeletEnsureSecretPulledImages feature gate ([#1858](#1858)) ([40d7bef](40d7bef)) * **t8s-cluster:** migrate to CAPI v1beta2 ([#1685](#1685)) ([dc5f071](dc5f071)) ### Bug Fixes * **t8s-cluster/autoscaler:** these names are inside the workload cluster ([#1877](#1877)) ([f345cea](f345cea)) * **t8s-cluster/management-cluster:** leave out protocol if `nil` ([#1837](#1837)) ([f370dac](f370dac)) * **t8s-cluster:** only allow nodePools with valid k8s names ([#1851](#1851)) ([b9431c5](b9431c5)) ### Miscellaneous Chores * **t8s-cluster/dependencies:** update common docker tag to v1.6.0 ([#1811](#1811)) ([b3b4c94](b3b4c94)) * **t8s-cluster/dependencies:** update common docker tag to v1.7.0 ([#1873](#1873)) ([71e062f](71e062f)) * **t8s-cluster/dependencies:** update helm release cilium to v1.18.6 ([#1894](#1894)) ([e1adc88](e1adc88)) * **t8s-cluster/dependencies:** update helm release cluster-autoscaler to v9.53.0 ([#1856](#1856)) ([dc67fcd](dc67fcd)) * **t8s-cluster/dependencies:** update helm release openstack-cloud-controller-manager to v2.34.1 ([#1553](#1553)) ([e984d19](e984d19)) * **t8s-cluster/dependencies:** update registry.k8s.io/etcd docker tag to v3.5.24 ([#1793](#1793)) ([a5098e3](a5098e3)) * **t8s-cluster/dependencies:** update registry.k8s.io/etcd docker tag to v3.6.6 ([#1813](#1813)) ([e07ffa7](e07ffa7)) * **t8s-cluster/dependencies:** update registry.k8s.io/etcd docker tag to v3.6.7 ([#1895](#1895)) ([cf1d3b4](cf1d3b4)) * **t8s-cluster/flux:** use centralised HelmRepositories instead of per-instance ([#1758](#1758)) ([3deff65](3deff65)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
Upgrades
Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.