feat(kubernetes)!: replace ephemeral emptyDisk with persistent storage for worker nodes - #2454
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces ephemeral worker disks with CDI-backed persistent DataVolumes and renames NodeGroup Changes
Sequence Diagram(s)sequenceDiagram
participant User as Chart values
participant Helm as Helm renderer
participant CDI as CDI (DataVolume controller)
participant VM as KubeVirt VM / kubelet
User->>Helm: provide nodeGroups.diskSize & storageClass
Helm->>CDI: render dataVolumeTemplate "disk-kubelet" (size/class, annotations)
Helm->>VM: render VM spec referencing DataVolume "disk-kubelet" and blockSize.matchVolume
CDI->>VM: provision DataVolume backing disk (DataVolume created/populated)
VM->>VM: kubeadm scripts mount /persistent and configure kubelet/containerd
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the stability of tenant Kubernetes worker nodes by transitioning from ephemeral storage to persistent PVC-backed disks. This change ensures that critical node state, such as kubelet certificates and containerd data, is preserved across VM reboots, preventing nodes from unexpectedly leaving the cluster. The update includes necessary configuration schema changes and adds robust unit testing to verify the new storage implementation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request renames ephemeralStorage to diskSize and introduces a storageClass field for worker node persistent disks in the Kubernetes package. The implementation transitions from using emptyDisk to dataVolumeTemplates to ensure data persistence for kubelet and containerd. The changes include updates to API types, Helm templates, documentation, and the addition of a new test suite using helm unittest. Feedback was provided to ensure Helm template correctness by using the quote function for storage values and storage class names, as per the project style guide.
| storage: | ||
| resources: | ||
| requests: | ||
| storage: {{ .group.diskSize | default "20Gi" }} |
There was a problem hiding this comment.
It is recommended to use the quote function for storage values in Helm templates to ensure they are always treated as strings, especially if the value could be interpreted as a number.
storage: {{ .group.diskSize | default "20Gi" | quote }}References
- Helm template correctness: missing
quote(link)
There was a problem hiding this comment.
Already addressed — storage uses | quote since commit 21a8c95e:
storage: {{ .group.diskSize | default "20Gi" | quote }}
| requests: | ||
| storage: {{ .group.diskSize | default "20Gi" }} | ||
| {{- with .group.storageClass }} | ||
| storageClassName: {{ . }} |
There was a problem hiding this comment.
The storageClassName value should be quoted to ensure it is correctly parsed as a string in the generated manifest.
storageClassName: {{ . | quote }}References
- Helm template correctness: missing
quote(link)
There was a problem hiding this comment.
Already addressed — storageClassName uses | quote since commit 21a8c95e:
storageClassName: {{ . | quote }}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/apps/kubernetes/tests/cluster_test.yaml (1)
20-418: Optional: factor out duplicated test fixtures.Each of the seven test cases repeats the same ~30 lines of
release,_namespace,controlPlane, and nodeGroup boilerplate, with only 1–2 fields actually varying per case. Consider extracting the common values into a shared YAML file and referencing viavalues:(or a YAML anchor with<<: *base) so each test only expresses its delta. This makes the intent of each case obvious and keeps future schema changes from requiring seven identical edits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/kubernetes/tests/cluster_test.yaml` around lines 20 - 418, The test file repeats the same release/_namespace/controlPlane/nodeGroups (md0) fixture across multiple "it" cases; extract the common block into a shared values object and reference it from each test (e.g., create a base YAML mapping called baseValues containing release, _namespace, version, controlPlane, and nodeGroups.md0) or use a YAML anchor (<<: *base) and then override only the differing keys (diskSize, storageClass, etc.) in each "it" case so tests like "renders dataVolumeTemplate with default 20Gi storage", "renders dataVolumeTemplate with custom 50Gi storage", and the other cases only contain the deltas.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/system/kubernetes-rd/cozyrds/kubernetes.yaml`:
- Line 29: The keysOrder array is missing the per-nodeGroup storage key
spec.nodeGroups.md0.storageClass, so add
["spec","nodeGroups","md0","storageClass"] into the keysOrder list immediately
after ["spec","nodeGroups","md0","diskSize"] so the new per-nodeGroup storage
option is ordered with the other worker storage settings; update the keysOrder
entry in kubernetes.yaml (look for the keysOrder array and the nearby
["spec","nodeGroups","md0","diskSize"] entry) to include this new key.
---
Nitpick comments:
In `@packages/apps/kubernetes/tests/cluster_test.yaml`:
- Around line 20-418: The test file repeats the same
release/_namespace/controlPlane/nodeGroups (md0) fixture across multiple "it"
cases; extract the common block into a shared values object and reference it
from each test (e.g., create a base YAML mapping called baseValues containing
release, _namespace, version, controlPlane, and nodeGroups.md0) or use a YAML
anchor (<<: *base) and then override only the differing keys (diskSize,
storageClass, etc.) in each "it" case so tests like "renders dataVolumeTemplate
with default 20Gi storage", "renders dataVolumeTemplate with custom 50Gi
storage", and the other cases only contain the deltas.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23ca0f22-241b-4852-a17b-ea7d6a7a4df0
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
api/apps/v1alpha1/kubernetes/types.goapi/apps/v1alpha1/kubernetes/zz_generated.deepcopy.gopackages/apps/kubernetes/Makefilepackages/apps/kubernetes/README.mdpackages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/tests/cluster_test.yamlpackages/apps/kubernetes/values.schema.jsonpackages/apps/kubernetes/values.yamlpackages/system/kubernetes-rd/cozyrds/kubernetes.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/apps/kubernetes/templates/cluster.yaml`:
- Around line 247-249: The cloud-init mounts list includes bind mounts for
/var/lib/kubelet and /var/lib/containerd but the target directories may not
exist, causing mount failures before preKubeadmCommands run; update the
cloud-init config to create both source (e.g., /persistent/kubelet,
/persistent/containerd) and target directories (e.g., /var/lib/kubelet,
/var/lib/containerd) prior to performing mounts (use mkdir -p for targets), and
replace direct mount entries with idempotent mount logic that checks mountpoint
(or use a small shell snippet invoked before mounts) to avoid duplicate mounts
and handle timing variations; reference the mounts section entries and the
preKubeadmCommands block when adding these mkdir -p and mountpoint checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba24933a-bd8b-442e-8822-4e2b3153eb0d
📒 Files selected for processing (2)
packages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/tests/cluster_test.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/apps/kubernetes/tests/cluster_test.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/system/kubernetes-rd/cozyrds/kubernetes.yaml (1)
29-29:⚠️ Potential issue | 🟡 MinorAdd the per-nodeGroup
storageClasstokeysOrder.Line 29 orders
diskSizebut skips the newspec.nodeGroups.md0.storageClass, so the dashboard will not place this worker disk option with the rest of the node group storage settings.Suggested insertion
- ["spec", "nodeGroups", "md0", "diskSize"], ["spec", "nodeGroups", "md0", "roles"] + ["spec", "nodeGroups", "md0", "diskSize"], ["spec", "nodeGroups", "md0", "storageClass"], ["spec", "nodeGroups", "md0", "roles"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/kubernetes-rd/cozyrds/kubernetes.yaml` at line 29, The keysOrder array is missing the per-nodeGroup storageClass entry so the dashboard won't group storage settings with the node group; update keysOrder to include ["spec","nodeGroups","md0","storageClass"] adjacent to the existing ["spec","nodeGroups","md0","diskSize"] entry (i.e., insert the ["spec","nodeGroups","md0","storageClass"] key immediately before or after the ["spec","nodeGroups","md0","diskSize"] element in the keysOrder array) so spec.nodeGroups.md0.storageClass is ordered with the node group's disk settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/system/kubernetes-rd/cozyrds/kubernetes.yaml`:
- Line 29: The keysOrder array is missing the per-nodeGroup storageClass entry
so the dashboard won't group storage settings with the node group; update
keysOrder to include ["spec","nodeGroups","md0","storageClass"] adjacent to the
existing ["spec","nodeGroups","md0","diskSize"] entry (i.e., insert the
["spec","nodeGroups","md0","storageClass"] key immediately before or after the
["spec","nodeGroups","md0","diskSize"] element in the keysOrder array) so
spec.nodeGroups.md0.storageClass is ordered with the node group's disk settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6cb3e079-636a-44da-a897-c5b0d24ca181
📒 Files selected for processing (1)
packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
…config Rename the ephemeralStorage field to diskSize in the NodeGroup typedef and default values to reflect the upcoming switch from ephemeral emptyDisk volumes to persistent dataVolumeTemplates for worker nodes. BREAKING CHANGE: The nodeGroups.*.ephemeralStorage field has been renamed to nodeGroups.*.diskSize. Users must update their values overrides. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
…tes for worker nodes Replace the ephemeral emptyDisk volume with a CDI dataVolumeTemplate backed by the tenant's storageClass. This ensures kubelet certificates, kubeconfig, and containerd state survive VM reboots, preventing worker nodes from falling out of the cluster. Changes in the KubevirtMachineTemplate: - Add dataVolumeTemplates with a blank source disk sized by diskSize - Switch the volume reference from emptyDisk to dataVolume - Rename disk and volume from "ephemeral" to "disk-kubelet" Changes in the KubeadmConfigTemplate: - Rename filesystem label from "ephemeral" to "persistent" - Update all mount paths from /ephemeral to /persistent BREAKING CHANGE: Worker node VMs now use persistent PVC-backed disks instead of ephemeral emptyDisk volumes. Existing clusters will trigger a rolling replacement of worker nodes on upgrade. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
With PVC-backed persistent disks, the virt-launcher pod no longer needs ephemeral-storage sized to the full disk. Set a fixed 2Gi allocation for QEMU runtime needs and proper scheduler behavior. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Add cdi.kubevirt.io/storage.usePopulator: "false" annotation to the dataVolumeTemplate to prevent CDI volume population flow. The populator mechanism creates an intermediate "prime" PVC and rebinds the PV, which causes ClaimMisbound errors with LINSTOR CSI. The legacy import path writes directly to the target PVC. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Allow each node group to specify its own storageClass for persistent worker node disks. When empty, the cluster default StorageClass is used. This enables using local storage for worker disks while keeping replicated storage for tenant workloads. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
…lues Signed-off-by: Arsolitt <arsolitt@gmail.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
The helm unit tests in tests/cluster_test.yaml were silently skipped by CI because the Makefile had no test target. CI runs hack/helm-unit-tests.sh which only invokes make test when the target exists. Signed-off-by: Arsolitt <arsolitt@gmail.com>
…r output Signed-off-by: Arsolitt <arsolitt@gmail.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
…Kn compatibility Signed-off-by: Arsolitt <arsolitt@gmail.com>
…ode disks Signed-off-by: Arsolitt <arsolitt@gmail.com>
DataVolumes are tied to VM lifecycle — PVC does not survive VM recreation. The real reason for overwrite: false is the same-VM reboot path: XFS on /dev/vdb already exists and cloud-init must skip reformatting. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Add explicit scope: persistence covers same-VM reboots only. VM replacement by CAPI still provisions a fresh PVC. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Specify that empty storageClass uses the management cluster default StorageClass (annotated with is-default-class: true), not the top-level chart storageClass value. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Arsolitt <arsolitt@gmail.com>
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Thanks for the thorough follow-ups — all three items addressed precisely:
overwrite: falsecomment now reflects the actual same-VM reboot reasoning (e2b10d42).- Breaking Changes note clarifies persistence scope: same-VM reboots vs CAPI VM replacement (
3afcaa5d). storageClassdescription now specifies the management cluster default StorageClass with theis-default-class: trueannotation (cd4590b4).
LGTM.
Incorporate images.waitForKubeconfig support from main while keeping the persistent-storage diskSize and storageClass fields introduced by this branch. Signed-off-by: Arsolitt <arsolitt@gmail.com>
Integrate the HAMi GPU virtualization addon from main while keeping the persistent-storage diskSize and storageClass fields introduced by this branch. Signed-off-by: Arsolitt <arsolitt@gmail.com>
998cc77
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Re-verified the PR substance after the merge from main.
Problem and scope: Real and well-scoped — kubelet client/serving certs in /var/lib/kubelet/pki and containerd state are wiped on emptyDisk-backed VM reboot, dropping the worker out of the tenant cluster. Reproducible, well-described.
Approach vs alternatives: DataVolumeTemplate with blank source is the right pick for the CAPI+KubeVirt model. Stateless re-bootstrap on every reboot (Talos-style) would require long-lived bootstrap tokens, CSR auto-approval, and a redesigned KubeadmConfigTemplate — out of scope. PVC referencing instead of dataVolumeTemplate breaks per-VM lifecycle management. kubelet cert rotation only rotates existing certs and cannot recover from full loss. The storage: (vs pvc:) spec, blockSize.matchVolume for DRBD 4Kn, usePopulator: false workaround for the LINSTOR CSI + CDI populator interaction, fixed 2Gi virt-launcher ephemeral-storage, and the mountpoint -q / mkdir -p idempotency guards in cloud-init all line up correctly with the model.
Acknowledged tradeoffs: Rolling replacement of all workers on upgrade (KubevirtMachineTemplate hash change), state lost on CAPI VM replacement, manual PVC deletion is break-glass — all named in the README.
Two non-blocking observations worth raising for follow-up (do not need to address before merge):
-
Default StorageClass for worker disks may be a regression in resource efficiency. The README itself notes "The
localStorageClass is recommended for worker node disks" because kubelet/containerd state is local-only by nature, replication has no failover use case here, and CAPI provisions a fresh PVC on VM replacement anyway. WithstorageClass: ""the chart falls back to the cluster-default SC, which on typical cozystack clusters isreplicated— so the default deployment pays DRBD replication cost (2-3x storage + network) for no benefit. Worth either documentingstorageClass: localas the recommended override invalues.yamlnext to the field, or splitting "tenant data SC" from "worker disk SC" at the chart level. Idiomatic-Kubernetes-wise, deferring to the operator's default-class annotation is defensible — flagging for visibility, not as a fix request. -
Silent CRD field drop on direct kubectl path. The Helm migration guard fails loudly on
ephemeralStorage, butkubectl apply -f kubernetes-cr.yamlwith the legacy field name silently drops it and reverts to the 20Gi default — README acknowledges this. A CEL validation rule on the CRD (e.g., rejecting unknown legacy field names with a helpful message) would close the gap. Not blocking the breaking-change bump since the rename is signposted in the release-note and README.
LGTM.
Merge origin/main into feat/kubernetes-kubelet-reserved-resources. PR #2454 (persistent storage for worker nodes) landed in main and touched the same nodeGroups schema. Conflicts resolved by keeping both feature sets (kubelet reservations + persistent storage): - api/apps/v1alpha1/kubernetes/types.go: merged kubebuilder default marker with all keys from both features - packages/apps/kubernetes/README.md: merged Parameters table with diskSize/storageClass + kubelet* rows - packages/system/kubernetes-rd/cozyrds/kubernetes.yaml: merged openAPISchema and keysOrder, dropped stale ephemeralStorage path Signed-off-by: Arsolitt <arsolitt@gmail.com>
PR #2454 renamed the nodeGroups field ephemeralStorage to diskSize and added a migration guard that fails template rendering when the old name is used. Update kubelet reservation tests to use the new field name. Signed-off-by: Arsolitt <arsolitt@gmail.com>
Add pre-upgrade migration 40 that walks all kuberneteses.apps.cozystack.io Application CRs and renames nodeGroups[*].ephemeralStorage to diskSize, preserving the user's value. Without this migration, clusters upgraded after PR #2454 either fail to reconcile (hard fail blocks all Flux operations) or silently lose the user's disk size setting (default 20Gi replaces whatever was configured). The migration is idempotent and best-effort: a failed patch is logged and leaves the version stamp at 40 for retry on next upgrade. Update the chart-side guard error message to direct operators to the migration job logs when the field appears post-upgrade (regression detector). Bump migrations.targetVersion 40 -> 41. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add pre-upgrade migration 41 that walks all kuberneteses.apps.cozystack.io Application CRs and renames nodeGroups[*].ephemeralStorage to diskSize, preserving the user's value. Without this migration, clusters upgraded after PR #2454 either fail to reconcile (hard fail blocks all Flux operations) or silently lose the user's disk size setting (default 20Gi replaces whatever was configured). The migration is idempotent and best-effort: a failed patch is logged and leaves the version stamp at 41 for retry on next upgrade. Update the chart-side guard error message to direct operators to the migration job logs when the field appears post-upgrade (regression detector). Bump migrations.targetVersion 41 -> 42. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…ook (#2688) ## Problem PR #2454 renamed `nodeGroups[*].ephemeralStorage` → `nodeGroups[*].diskSize` with a hard `{{ fail }}` guard. Any cluster whose HelmRelease still carries the legacy field cannot be reconciled by Flux at all — unrelated control-plane changes and MachineHealthCheck remediations are also blocked. ## Solution Add platform migration 41 that runs as a pre-upgrade hook before any chart resources are applied. The migration walks every `kuberneteses.apps.cozystack.io` Application CR cluster-wide and renames `nodeGroups[*].ephemeralStorage` to `nodeGroups[*].diskSize`, preserving the user's value. - Idempotent: a second run is a no-op (field is already absent) - Best-effort: a failed patch is logged; the version stamp stays at 41 so the migration retries on the next platform upgrade - Bumps `migrations.targetVersion` 41 → 42 The chart-side guard remains in place with an updated error message that directs operators to the migration job logs if the field somehow reappears post-upgrade. ## Testing Verified on dev cluster: migration correctly renames `ephemeralStorage` values in existing Application CRs. All 123 helm unit tests pass. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Documentation** * Platform migration 41 now automatically handles the `ephemeralStorage` to `diskSize` field rename for Kubernetes node groups * Migration is transparent during upgrade with no manual configuration changes required * Error messages updated with clearer troubleshooting guidance * **Tests** * Updated test assertions for migration error messages <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2688?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…kSize via pre-upgrade hook (#2712) Add pre-upgrade migration 41 that walks all kuberneteses.apps.cozystack.io Application CRs and renames nodeGroups[*].ephemeralStorage to diskSize, preserving the user's value. Without this migration, clusters upgraded after PR #2454 either fail to reconcile (hard fail blocks all Flux operations) or silently lose the user's disk size setting (default 20Gi replaces whatever was configured). The migration is idempotent and best-effort: a failed patch is logged and leaves the version stamp at 41 for retry on next upgrade. Update the chart-side guard error message to direct operators to the migration job logs when the field appears post-upgrade (regression detector). Bump migrations.targetVersion 41 -> 42. Assisted-By: Claude <noreply@anthropic.com> (cherry picked from commit ed1bb53) <!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note ```
What this PR does
Replace the ephemeral
emptyDiskvolume on tenant Kubernetes worker VMs with persistent PVC-backed storage via KubeVirtdataVolumeTemplates. Previously, kubelet certificates, kubeconfig, and containerd state were lost on VM reboot, causing the node to fall out of the tenant cluster.Changes:
ephemeralStoragefield todiskSizein NodeGroup configurationemptyDiskvolume withdataVolumeTemplatesusing CDI blank sourcecdi.kubevirt.io/storage.usePopulator: "false"annotation to work around CDI volume population issues with LINSTOR CSIstorageClassfield (defaults to cluster default)/ephemeralto/persistentcluster_test.yaml) covering dataVolumeTemplates defaults, custom disk size, storageClass presence/absence, fixed ephemeral-storage, disk name consistency, and CDI populator annotationdiskSize,storageClassName) for Helm correctnessmountpoint -qchecks and create target directories to handle fresh images and cloud-init retriesblockSize.matchVolumeon worker node disks for automatic block size detection, fixing DRBD 4Kn compatibility with QEMUKnown gotchas:
ClaimMisbounderrors with LINSTOR CSI driver, regardless of StorageClass (replicatedorlocal). TheusePopulator: falseannotation switches to the legacy import path which works correctly. This is a known CDI issue documented in kubevirt/containerized-data-importer#3146.replicatedStorageClass with DRBDsuspend-ioquorum policy can cause transient VM IO errors during LINSTOR satellite restarts (quorum loss). ThelocalStorageClass is recommended for worker node disks. WhenstorageClassis not specified in a nodeGroup, the cluster default StorageClass is used.block-size 4096). Without explicit block size configuration, QEMU defaults to 512-byte sectors, causing immediate IO errors and VM pause. TheblockSize.matchVolumesetting in the disk spec ensures KubeVirt auto-detects the correct sector size from the volume.Tested on a live cluster:
localStorageClassScreenshots
N/A — no UI changes.
Release note
Summary by CodeRabbit
New Features
Configuration Changes
Documentation