fix(kubernetes): default worker disk storageClass to the application storageClass - #3129

Code Review

This pull request updates the Kubernetes application package to fall back to the application-level storageClass (defaulting to replicated) when the individual nodeGroup's storageClass is left empty. This ensures that worker VM persistent disks are provisioned on a DRBD-backed StorageClass supporting ReadWriteMany (RWX) for live migration, preventing issues with linstor-csi. The changes include updates to the Helm templates, schema definitions, documentation, and comprehensive unit tests to verify the fallback logic and guard against unexpected worker-pool rolling upgrades. As there are no review comments, I have no additional feedback to provide.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/kubernetes/values.yaml`:
- Line 85: Update the StorageClass field documentation in values.yaml to remove
the implication that the fallback is always DRBD-backed. The comment for the
storageClass setting should say that worker node disks fall back to the
application-level storageClass, and that this application-level value defaults
to replicated; keep the rest of the RWX/immutability guidance intact. Use the
storageClass field description itself as the target location for the wording
change.

Summary

⚠️ MERGE CONFLICT — mergeable: CONFLICTING against main. Needs a rebase before this can land.

The fix is correct and well-tested. When a node group leaves storageClass empty, the worker DataVolume previously omitted storageClassName and fell through to the management cluster's default StorageClass, which is not guaranteed to be DRBD-backed. Since worker VMs use evictionStrategy: External, their disks must be RWX-capable, and linstor-csi only grants RWX on DRBD-backed classes. The one-liner {{- with .group.storageClass | default $.Values.storageClass }} fixes the root cause, and the both-empty opt-out path (omit storageClassName) is correctly preserved.

All documentation surfaces (types.go, values.yaml, values.schema.json, README.md, cozyrds/kubernetes.yaml) are updated in lockstep. Test coverage is thorough: six new/updated cases cover the fallback, custom app SC, group override wins, and both-empty omit path. The two hash-pin tests that document the upgrade blast radius are a nice touch.

Upgrade blast-radius note

On upgrade, CAPI will rename the KubevirtMachineTemplate for every existing tenant cluster whose md0 leaves storageClass empty — even on clusters where the management cluster default was already replicated, because the roll is driven by the body hash, not the effective StorageClass. This is safe (same class, one roll) and is clearly called out in the release note, which is the right place for it. The hash-pin tests document it in the test suite.

Blocking

• Rebase onto current main to resolve the merge conflict.

Non-blocking suggestions

• See inline comment on the hash-pin tests.

Verdict

LGTM

Correct, well-scoped fix: the empty-storageClass fallback for worker VM DataVolumes now targets the application-level storageClass (default replicated, a DRBD/LINSTOR class) instead of the management-cluster default, which fixes live migration on clusters whose infra default is not DRBD-backed; all four render cases are tested and pass, codegen surfaces are in lockstep, and the one-time worker-pool roll is correctly documented and guarded.

Verification performed

• Template render: helm template with tests/values/common.yaml produces storageClassName: "replicated" on the shipped default (md0 empty), template name test-k8s-md0-63ff81 matching the pin; --set storageClass="" produces no storageClassName (name test-k8s-md0-dcdf58, matching the omit pin). MachineDeployment infrastructureRef points at the hashed name, so the body-hash roll-on-upgrade claim is mechanically accurate.
• helm unittest: 140/140 pass, including the four new cases (fallback to app class, custom app class, per-node-group override wins, both-empty omit) and the two hash pins.
• go vet ./kubernetes/... (module api/apps/v1alpha1): clean. The types.go change is a doc-comment only.
• Codegen lockstep: the new nodeGroups[].storageClass description is byte-identical across types.go, values.yaml, values.schema.json, README.md (padding recomputed), and packages/system/kubernetes-rd/cozyrds/kubernetes.yaml. Schema is valid JSON, cozyrds valid YAML, working tree clean.
• Semantics: .Values.storageClass already denotes an infra-cluster StorageClass name in existing code (templates/helmreleases/csi.yaml:34 uses it as infraStorageClass), so reusing it for the worker DataVolume (also provisioned on the infra cluster) is consistent, not a tenant-vs-infra class mix-up.

Upgrade & fresh-install impact

• Upgrade (existing tenants): one-time worker-pool roll for every tenant cluster whose md0 leaves storageClass empty (the shipped default), driven by the template-body hash — accurate as stated, and true even for clusters already effectively on replicated. Not data-lossy: the disk-system DataVolume is the ephemeral OS/kubelet/containerd disk re-imaged from the Talos factory artifact; workload data lives in separate LINSTOR-replicated tenant PVCs with independent lifecycle. The roll is disruptive (rolling worker replacement) but standard for any worker-template change. No migration script required.
• Fresh install: defaulting to replicated cannot reference a non-existent StorageClass. The iaas bundle that provides KubeVirt (and thus tenant worker VMs) is hard-gated to bundles.system.variant in {isp-full, isp-full-generic} (packages/core/platform/templates/bundles/iaas.yaml:1-2, fail otherwise), and both variants unconditionally install LINSTOR (packages/core/platform/templates/bundles/system.yaml:27 and :96), which provisions replicated. isp-hosted lacks LINSTOR but also cannot enable iaas, so it cannot run these VMs at all. Operators who deliberately want the infra default retain the both-empty opt-out (set app-level storageClass: "").