🐛 Promote VM disks to PVCs

api/v1alpha5/virtualmachine_storage_types.go

@@ -222,8 +222,8 @@ type PersistentVolumeClaimVolumeSource struct {
 type UnmanagedVolumeClaimVolumeType string
 
 const (
-	UnmanagedVolumeClaimVolumeTypeFromImage = "FromImage"
-	UnmanagedVolumeClaimVolumeTypeFromVM    = "FromVM"
+	UnmanagedVolumeClaimVolumeTypeFromImage UnmanagedVolumeClaimVolumeType = "FromImage"
+	UnmanagedVolumeClaimVolumeTypeFromVM    UnmanagedVolumeClaimVolumeType = "FromVM"
 )
 
 type UnmanagedVolumeClaimVolumeSource struct {
@@ -237,12 +237,12 @@ type UnmanagedVolumeClaimVolumeSource struct {
 
 	// +required
 
-	// Name describes the name of the unmanaged volume.
+	// Name describes the information used to identify the unmanaged volume.
 	//
-	// For volumes from an image, the name is from the image's
+	// For volumes from an image, the value is from the image's
 	// status.disks[].name field.
 	//
-	// For volumes from the VM, the name is from the VM's
+	// For volumes from the VM, the value is from the VM's
 	// status.volumes[].name field.
 	//
 	// Please note, specifying the name of an existing, managed volume is not

config/crd/bases/vmoperator.vmware.com_virtualmachinereplicasets.yaml

@@ -7556,12 +7556,12 @@ spec:
                                   properties:
                                     name:
                                       description: |-
-                                        Name describes the name of the unmanaged volume.
+                                        Name describes the information used to identify the unmanaged volume.
 
-                                        For volumes from an image, the name is from the image's
+                                        For volumes from an image, the value is from the image's
                                         status.disks[].name field.
 
-                                        For volumes from the VM, the name is from the VM's
+                                        For volumes from the VM, the value is from the VM's
                                         status.volumes[].name field.
 
                                         Please note, specifying the name of an existing, managed volume is not

config/crd/bases/vmoperator.vmware.com_virtualmachines.yaml

@@ -12184,12 +12184,12 @@ spec:
                           properties:
                             name:
                               description: |-
-                                Name describes the name of the unmanaged volume.
+                                Name describes the information used to identify the unmanaged volume.
 
-                                For volumes from an image, the name is from the image's
+                                For volumes from an image, the value is from the image's
                                 status.disks[].name field.
 
-                                For volumes from the VM, the name is from the VM's
+                                For volumes from the VM, the value is from the VM's
                                 status.volumes[].name field.
 
                                 Please note, specifying the name of an existing, managed volume is not

config/rbac/role.yaml

@@ -104,6 +104,14 @@ rules:
   verbs:
   - get
   - list
+- apiGroups:
+  - cns.vmware.com
+  resources:
+  - cnsregistervolumes
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - cns.vmware.com
   resources:

pkg/constants/constants.go

@@ -240,4 +240,8 @@ const (
 	// ReconcilePriorityAnnotationKey is the annotation key that specifies the
 	// reconcile priority for an object.
 	ReconcilePriorityAnnotationKey = "vmoperator.vmware.com.protected/reconcile-priority"
+
+	// CreatedByLabel is the label key that indicates an object was
+	// created on behalf of a VM Operator VM. The value is the name of a VM.
+	CreatedByLabel = "vmoperator.vmware.com/created-by"
 )

pkg/providers/vsphere/contentlibrary/content_library_utils.go

@@ -14,14 +14,14 @@ import (
 	"github.com/vmware/govmomi/vim25/mo"
 	vimtypes "github.com/vmware/govmomi/vim25/types"
 	"github.com/vmware/govmomi/vmdk"
-	"k8s.io/apimachinery/pkg/api/resource"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha5"
 	"github.com/vmware-tanzu/vm-operator/api/v1alpha5/common"
 	"github.com/vmware-tanzu/vm-operator/pkg/conditions"
 	pkgconst "github.com/vmware-tanzu/vm-operator/pkg/constants"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
+	pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util"
 	kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/ptr"
 )
@@ -158,46 +158,21 @@ func UpdateVmiWithVirtualMachine(
 	for _, bd := range vm.Config.Hardware.Device {
 		if disk, ok := bd.(*vimtypes.VirtualDisk); ok {
 
-			var (
-				capacity *resource.Quantity
-				size     *resource.Quantity
-			)
-
-			var uuid string
-			switch tb := disk.Backing.(type) {
-			case *vimtypes.VirtualDiskSeSparseBackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskSparseVer2BackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskFlatVer2BackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskLocalPMemBackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskRawDiskMappingVer1BackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskRawDiskVer2BackingInfo:
-				uuid = tb.Uuid
-			case *vimtypes.VirtualDiskPartitionedRawDiskVer2BackingInfo:
-				uuid = tb.Uuid
-			}
-
-			if uuid != "" {
+			if info := pkgutil.GetVirtualDiskInfo(disk); info.UUID != "" {
 				di, _ := vmdk.GetVirtualDiskInfoByUUID(
 					ctx,
 					nil,   /* client is nil since props are not re-fetched */
 					vm,    /* use props from this object */
 					false, /* do not refetch props */
 					false, /* include disks related to snapshots */
-					uuid)
-				capacity = kubeutil.BytesToResource(di.CapacityInBytes)
-				size = kubeutil.BytesToResource(di.Size)
+					info.UUID)
 
 				status.Disks = append(
 					status.Disks,
 					vmopv1.VirtualMachineImageDiskInfo{
-						Name:      uuid,
-						Limit:     capacity,
-						Requested: size,
+						Name:      info.UUID,
+						Limit:     kubeutil.BytesToResource(di.CapacityInBytes),
+						Requested: kubeutil.BytesToResource(di.Size),
 					})
 			}
 		}

pkg/providers/vsphere/vmprovider_vm.go

@@ -29,6 +29,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apierrorsutil "k8s.io/apimachinery/pkg/util/errors"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -67,6 +68,7 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource"
 	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 	vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm"
+	vmconfalldisksarepvcs "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/alldisksarepvcs"
 	vmconfcrypto "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto"
 	vmconfdiskpromo "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/diskpromo"
 	vmconfpolicy "github.com/vmware-tanzu/vm-operator/pkg/vmconfig/policy"
@@ -88,6 +90,7 @@ var (
 	ErrUpdate                 = pkgerr.NoRequeueNoErr("updated vm")
 	ErrSnapshotRevert         = pkgerr.RequeueError{Message: "reverted snapshot"}
 	ErrPolicyNotReady         = vmconfpolicy.ErrPolicyNotReady
+	ErrUnmanagedVols          = vmconfalldisksarepvcs.ErrUnmanagedVols
 )
 
 // VMCreateArgs contains the arguments needed to create a VM on VC.
@@ -1087,10 +1090,63 @@ func (vs *vSphereVMProvider) getTags(
 func (vs *vSphereVMProvider) reconcileUnmanagedToManagedDisks(
 	vmCtx pkgctx.VirtualMachineContext) error {
 
-	_ = vmCtx // Remove once vmCtx is used.
+	return vmconfalldisksarepvcs.Reconcile(
+		vmCtx,
+		vs.k8sClient,
+		nil,
+		vmCtx.VM,
+		vmCtx.MoVM,
+		nil)
+}
+
+// isUnmanagedVolumeFromImage checks if a volume is an unmanaged volume of type
+// "FromImage".
+func isUnmanagedVolumeFromImage(
+	vol *vmopv1.VirtualMachineVolume) bool {
+
+	return vol != nil &&
+		vol.PersistentVolumeClaim != nil &&
+		vol.PersistentVolumeClaim.UnmanagedVolumeClaim != nil &&
+		vol.PersistentVolumeClaim.UnmanagedVolumeClaim.Type == vmopv1.UnmanagedVolumeClaimVolumeTypeFromImage
+}
+
+// updateDiskDeviceFromPVC updates a disk device configuration from PVC
+// information.
+func (vs *vSphereVMProvider) updateDiskDeviceFromPVC(
+	disk *vimtypes.VirtualDisk,
+	pvc *corev1.PersistentVolumeClaim,
+	deviceChange *vimtypes.BaseVirtualDeviceConfigSpec) error {
+
+	_, ok := (*deviceChange).(*vimtypes.VirtualDeviceConfigSpec)
+	if !ok {
+		return fmt.Errorf("device change is not a VirtualDeviceConfigSpec")
+	}
+
+	// Get PVC storage class
+	storageClass := pvc.Spec.StorageClassName
+
+	// Get PVC requested capacity
+	var pvcCapacity *resource.Quantity
+	if requestedStorage, ok := pvc.Spec.Resources.Requests[corev1.ResourceStorage]; ok {
+		pvcCapacity = &requestedStorage
+	}
+
+	// Update disk capacity if PVC capacity is larger than current disk capacity
+	if pvcCapacity != nil {
+		pvcCapacityBytes, ok := pvcCapacity.AsInt64()
+		if ok {
+			currentCapacityBytes := disk.CapacityInBytes
+			if pvcCapacityBytes > currentCapacityBytes {
+				disk.CapacityInBytes = pvcCapacityBytes
+				// Also update CapacityInKB for consistency
+				disk.CapacityInKB = pvcCapacityBytes / 1024
+			}
+		}
+	}
 
 	// TODO(AllDisksArePVCs)
 	//
+	//
 	// Implement this function. For example, for each of the non-FCD disks in
 	// vmCtx.MoVM.Config.Hardware.Devices, do the following:
 	//
@@ -1171,6 +1227,12 @@ func (vs *vSphereVMProvider) reconcileUnmanagedToManagedDisks(
 	// loop, uses the K8s client to list all CnsRegisterVolume objects
 	// with a label vmoperator.vmware.com/created-by whose value is the
 	// name of the current VM. If there are any, then delete them.
+	//
+	// Update storage class information on the disk device
+	// This would typically involve setting storage policy or other
+	// storage-related configurations based on the PVC's storage class. For now,
+	// we'll just note that we have the storage class.
+	_ = storageClass // Acknowledge we have the storage class for future use
 
 	return nil
 }
@@ -2473,47 +2535,80 @@ func (vs *vSphereVMProvider) vmCreateGenConfigSpecImagePVCDataSourceRefs(
 	vmCtx pkgctx.VirtualMachineContext,
 	createArgs *VMCreateArgs) error {
 
-	_, _ = vmCtx, createArgs // Remove once args are used.
+	// Check if the feature is enabled
+	if !pkgcfg.FromContext(vmCtx).Features.AllDisksArePVCs {
+		return nil
+	}
 
-	// TODO(AllDisksArePVCs)
-	//
-	// Implement this function. For example, for each of the disks in
-	// createArgs.ConfigSpec, do the following:
-	//
-	// 1. Iterate over spec.volumes and look for an entry with an
-	//    unmanagedVolumeSource with a type of "FromImage" and name set to
-	//    the value that identifies the current disk (the logic for
-	//    deriving this name is located in the UpdateVmiWithOVF and
-	//    UpdateVmiWithVirtualMachine functions in the file
-	//    pkg/providers/vsphere/contentlibrary/content_library_utils.go).
-	//
-	// 2. If no such entry exists, then continue to the next iteration of the
-	//    loop.
-	//
-	// 3. Use the k8s client's Get function (do not use CreateOrPatch)
-	//    to see if a PersistentVolumeClaim object already exists for
-	//    this disk. The name of the PersistentVolumeClaim object should
-	//    be what was specified in the field
-	//    spec.volumes[].persistentVolumeClaim.claimName from the prior
-	//    step.
-	//
-	//    If no such entry exists, then return an error indicating the PVC for
-	//    the specified claim is missing and VM creation may not proceed.
-	//
-	// 4. Get the PVC's specified storage class and requested capacity.
-	//
-	//    The PVC *may* specify a different encryption class from the VM, but it
-	//    is probably too much work to handle multiple encryption classes in our
-	//    BYOK reconciler at this point. We will allow the PVC to be recrypted
-	//    by CSI *after* the VM is created and the disk is turned into a PVC.
-	//
-	//    Update the disk device's specified storage class using the value from
-	//    the PVC. Update the disk device's specified capacity using the value
-	//    from the PVC *if* it is larger than the disk's current requested size.
-	//
-	// That should be it! We are not actually registering these disks as PVCs
-	// at this point. That happens in the VM's update workflow later. For now
-	// we just need the information about these disks *from* the PVCs.
+	if createArgs.ConfigSpec.DeviceChange == nil {
+		return nil
+	}
+
+	// Build a map of volumes by name for quick lookup
+	volumeByName := make(map[string]*vmopv1.VirtualMachineVolume)
+	for i := range vmCtx.VM.Spec.Volumes {
+		vol := &vmCtx.VM.Spec.Volumes[i]
+		volumeByName[vol.Name] = vol
+	}
+
+	var errs []error
+
+	// Iterate through device changes to find disks
+	for i, deviceChange := range createArgs.ConfigSpec.DeviceChange {
+		deviceSpec, ok := deviceChange.(*vimtypes.VirtualDeviceConfigSpec)
+		if !ok || deviceSpec.Device == nil {
+			continue
+		}
+
+		disk, ok := deviceSpec.Device.(*vimtypes.VirtualDisk)
+		if !ok {
+			continue
+		}
+
+		// The device label, prior to the actual create, will match the value
+		// from UVC.Name.
+		//
+		// disk.DeviceInfo.GetDescription().Label
+
+		// Generate disk name using the same logic as content library utils
+		diskInfo := pkgutil.GetVirtualDiskInfo(disk)
+		diskName := diskInfo.Name()
+		if diskName == "" {
+			continue
+		}
+
+		// Step 1: Look for volume entry with unmanagedVolumeSource type "FromImage"
+		volume, exists := volumeByName[diskName]
+		if !exists || !isUnmanagedVolumeFromImage(volume) {
+			continue // Step 2: Skip if no such entry exists
+		}
+
+		// Step 3: Get the PVC
+		pvcName := volume.PersistentVolumeClaim.ClaimName
+		pvc := &corev1.PersistentVolumeClaim{}
+		pvcKey := ctrlclient.ObjectKey{Namespace: vmCtx.VM.Namespace, Name: pvcName}
+
+		if err := vs.k8sClient.Get(vmCtx, pvcKey, pvc); err != nil {
+			if apierrors.IsNotFound(err) {
+				errs = append(errs, fmt.Errorf("PVC %s for disk %s is missing and VM creation cannot proceed", pvcName, diskName))
+			} else {
+				errs = append(errs, fmt.Errorf("failed to get PVC %s for disk %s: %w", pvcName, diskName, err))
+			}
+			continue
+		}
+
+		// TODO Clean this up.
+		disk.Backing.(*vimtypes.VirtualDiskFlatVer2BackingInfo).Uuid = volume.PersistentVolumeClaim.UnmanagedVolumeClaim.UUID
+
+		// Step 4: Update disk device with PVC information
+		if err := vs.updateDiskDeviceFromPVC(disk, pvc, &createArgs.ConfigSpec.DeviceChange[i]); err != nil {
+			errs = append(errs, fmt.Errorf("failed to update disk device %s from PVC %s: %w", diskName, pvcName, err))
+		}
+	}
+
+	if len(errs) > 0 {
+		return apierrorsutil.NewAggregate(errs)
+	}
 
 	return nil
 }

pkg/util/devices.go

@@ -5,9 +5,7 @@
 package util
 
 import (
-	"path"
 	"reflect"
-	"strings"
 
 	vimtypes "github.com/vmware/govmomi/vim25/types"
 )
@@ -254,12 +252,9 @@ type VirtualDiskInfo struct {
 	Device          *vimtypes.VirtualDisk
 }
 
-// Name returns the name of the disk to use in the volume status.
-// TODO(akutz) This cannot be used in the status as it may not be unique.
+// Name returns the name of the disk to use in the volume status as well as
+// the UnmanagedVolumeSource.ID field.
 func (vdi VirtualDiskInfo) Name() string {
-	if vdi.FileName != "" {
-		return strings.TrimSuffix(path.Base(vdi.FileName), path.Ext(vdi.FileName))
-	}
 	return vdi.Label
 }