HEP: support CDI way for non-root volume [skip ci]#10326
HEP: support CDI way for non-root volume [skip ci]#10326Vicente-Cheng wants to merge 1 commit intoharvester:masterfrom
Conversation
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Vicente-Cheng
requested review from
a team,
WebberHuang1118,
ibrokethecloud and
ihcsim
There was a problem hiding this comment.
Pull request overview
Adds a new Harvester Enhancement Proposal (HEP) documenting how to extend CDI/StorageProfile behavior to non-root volumes and introduce an offline volume data migration action.
Changes:
- Proposes an opt-in non-root volume creation flow using CDI DataVolumes alongside the existing PVC flow
- Describes new “Advanced Options” in the UI (access mode / volume mode) and a new “Data Migration” volume action/API
- Documents removal of the legacy
volumeForVirtualMachine+ filesystem blank-source workaround
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| This enhancement addresses this limitation by: | ||
| 1. Providing an optional CDI DataVolume creation path alongside the existing PVC creation path, allowing users to opt in to CDI StorageProfile support when creating non-root volumes. | ||
| 2. Exposing advanced volume creation options (access mode, volume mode) in the UI. | ||
| 3. Adding a **Data Migration** action on the Volume page to allow offline storage migration between StorageClasses via CDI. | ||
| 4. Removing the legacy `volumeForVirtualMachine` annotation mechanism and its associated filesystem blank source workaround, which are no longer needed. |
There was a problem hiding this comment.
The proposed approach in this HEP relies on an optional DataVolume creation path, but the linked issue (#9412) describes solving non-root PVCs via CDI PVC rendering (enabling the WebhookPvcRendering feature gate and labeling opt-in PVCs with cdi.kubevirt.io/applyStorageProfile: "true"). Please either align the proposal/implementation overview with that PVC-rendering approach or explicitly document why the DataVolume path is chosen and how it satisfies the issue requirements (including any needed CDI config/labels).
|
|
||
| Before this enhancement, migrating data from a volume on one StorageClass to another required manual steps: creating a new volume, attaching both to a VM, and copying data. | ||
|
|
||
| After this enhancement, the user can select "Data Migration" from the volume actions menu. They specify a target volume name and target StorageClass, and the system creates a CDI DataVolume that clones the data automatically. The action is only available when the volume is not attached to any running VM. |
There was a problem hiding this comment.
The conditions for showing/allowing the Data Migration action are inconsistent: Story 3 says "not attached to any running VM", while later the UX section says "not attached to any VM" and the validation list mentions "not in use by any Pod". Please define a single, precise eligibility rule (e.g., “PVC not mounted by any Pod”) and use the same wording throughout.
| After this enhancement, the user can select "Data Migration" from the volume actions menu. They specify a target volume name and target StorageClass, and the system creates a CDI DataVolume that clones the data automatically. The action is only available when the volume is not attached to any running VM. | |
| After this enhancement, the user can select "Data Migration" from the volume actions menu. They specify a target volume name and target StorageClass, and the system creates a CDI DataVolume that clones the data automatically. The action is only available when the volume's PVC is not mounted by any Pod. |
| **Input type:** | ||
| ```go | ||
| type DataMigrationInput struct { | ||
| TargetVolumeName string `json:"targetVolumeName"` | ||
| TargetStorageClassName string `json:"targetStorageClassName"` | ||
| } |
There was a problem hiding this comment.
The Go example for DataMigrationInput includes literal \t and escaped struct tag quotes (e.g., json:\"targetVolumeName\"), which is not valid Go and will copy/paste incorrectly. In a fenced markdown code block you can use normal indentation and json:"..." tags without escaping.
|
|
||
| ## Motivation | ||
|
|
||
| The StorageProfile of CDI provide a centralized way to configure storage attributes (access modes, volume modes, clone strategies, snapshot classes) per StorageClass. However, these profiles only apply to CDI DataVolumes, not to standard PVCs. When a user creates a non-root volume from the Harvester UI, it is created as a plain PVC. This means: |
There was a problem hiding this comment.
Grammar: "The StorageProfile of CDI provide" should be singular (e.g., "The StorageProfile of CDI provides" or "CDI StorageProfiles provide").
| The StorageProfile of CDI provide a centralized way to configure storage attributes (access modes, volume modes, clone strategies, snapshot classes) per StorageClass. However, these profiles only apply to CDI DataVolumes, not to standard PVCs. When a user creates a non-root volume from the Harvester UI, it is created as a plain PVC. This means: | |
| The StorageProfile of CDI provides a centralized way to configure storage attributes (access modes, volume modes, clone strategies, snapshot classes) per StorageClass. However, these profiles only apply to CDI DataVolumes, not to standard PVCs. When a user creates a non-root volume from the Harvester UI, it is created as a plain PVC. This means: |
|
|
||
| ### Upgrade strategy | ||
|
|
||
| Upgrade would not affected with this change. The new DataVolume creation path and data migration action are additive features that do not interfere with existing PVC creation or VM workflows. The removal of the legacy `volumeForVirtualMachine` annotation and filesystem blank source mechanism is safe. |
There was a problem hiding this comment.
Grammar: "Upgrade would not affected" is ungrammatical; consider "Upgrade will not be affected" (or "Upgrades are not affected") for clarity.
| Upgrade would not affected with this change. The new DataVolume creation path and data migration action are additive features that do not interfere with existing PVC creation or VM workflows. The removal of the legacy `volumeForVirtualMachine` annotation and filesystem blank source mechanism is safe. | |
| The upgrade process will not be affected by this change. The new DataVolume creation path and data migration action are additive features that do not interfere with existing PVC creation or VM workflows. The removal of the legacy `volumeForVirtualMachine` annotation and filesystem blank source mechanism is safe. |
Problem:
HEP: support CDI way for non-root volume
Solution:
HEP: support CDI way for non-root volume
Related Issue(s):
#9412
Test plan:
Additional documentation or context
We discussed the feature offline. This HEP is the record for the discussion summary.