diff --git a/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register.go b/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register.go index e719a4ad5..dff7a3fe5 100644 --- a/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register.go +++ b/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register.go @@ -327,6 +327,9 @@ func ensureUnmanagedDisksHaveStoragePolicies( var ( pbmRefToDeviceKey = map[string]int32{} pbmRefs = make([]pbmtypes.PbmServerObjectRef, len(info.Disks)) + // Track disk indices that need their storage class determined from + // their actual storage profile (i.e., disks without an existing PVC). + disksNeedingStorageClassFromProfile = map[int]struct{}{} ) // Get the storage profile IDs used by the VM and disks (if any). @@ -344,7 +347,9 @@ func ensureUnmanagedDisksHaveStoragePolicies( } } if key.Name == "" { - info.Disks[i].StorageClass = vm.Spec.StorageClass + // No PVC exists for this disk. We'll determine the storage class + // from the disk's actual storage profile after querying it. + disksNeedingStorageClassFromProfile[i] = struct{}{} } else { if err := k8sClient.Get(ctx, key, obj); err != nil { return nil, nil, fmt.Errorf( @@ -357,9 +362,9 @@ func ensureUnmanagedDisksHaveStoragePolicies( info.Disks[i].StorageClass = *scn } else { // Could not find a PVC with an existing storage class for the - // unmanaged disk, so default the VM to using the VM's storage - // class. - info.Disks[i].StorageClass = vm.Spec.StorageClass + // unmanaged disk, so we'll determine it from the disk's actual + // storage profile. + disksNeedingStorageClassFromProfile[i] = struct{}{} } } @@ -420,6 +425,25 @@ func ensureUnmanagedDisksHaveStoragePolicies( } } + // For disks that need their storage class determined from their actual + // storage profile (i.e., disks without an existing PVC or with a PVC that + // has no storage class), set the storage class based on the disk's profile. + for i := range disksNeedingStorageClassFromProfile { + storageClassSet := false + for _, pid := range info.Disks[i].ProfileIDs { + if className := policyIDToStorageClass[pid]; className != "" { + info.Disks[i].StorageClass = className + storageClassSet = true + break + } + } + // Only fall back to VM's storage class if the disk has no profile or + // no matching storage class was found. + if !storageClassSet { + info.Disks[i].StorageClass = vm.Spec.StorageClass + } + } + return storageClassToPolicyID, policyIDToStorageClass, nil } diff --git a/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register_test.go b/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register_test.go index 147446204..21bbf52f2 100644 --- a/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register_test.go +++ b/pkg/vmconfig/volumes/unmanaged/register/unmanagedvolumes_register_test.go @@ -550,6 +550,125 @@ var _ = Describe("Reconcile", func() { }) }) + When("VM has unmanaged disk with storage profile different from VM storage class", func() { + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMSharedDisks = true + }) + + Expect(unmanagedvolsfill.Reconcile( + ctx, + nil, + nil, + vm, + moVM, + nil)).To(MatchError(unmanagedvolsfill.ErrPendingBackfill)) + Expect(unmanagedvolsfill.Reconcile( + ctx, + nil, + nil, + vm, + moVM, + nil)).To(Succeed()) + }) + + BeforeEach(func() { + // Add a second storage class with a different profile + withObjs = append(withObjs, + &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-storage-class-2", + }, + Parameters: map[string]string{ + "storagePolicyID": "profile-456", + }, + }) + + // Override mock profile results to return profile-456 for the disk + // which is different from the VM's storage class (my-storage-class-1 -> profile-123) + mockProfileResults = []pbmtypes.PbmQueryProfileResult{ + { + Object: pbmtypes.PbmServerObjectRef{ + Key: "vm-1:300", + }, + ProfileId: []pbmtypes.PbmProfileId{ + { + UniqueId: "profile-456", // Different from VM's profile-123 + }, + }, + }, + } + + // Start with empty volumes - let Reconcile add them + vm.Spec.Volumes = []vmopv1.VirtualMachineVolume{} + + disk := &vimtypes.VirtualDisk{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 300, + ControllerKey: 200, + UnitNumber: ptr.To(int32(0)), + Backing: &vimtypes.VirtualDiskFlatVer2BackingInfo{ + VirtualDeviceFileBackingInfo: vimtypes.VirtualDeviceFileBackingInfo{ + FileName: "[LocalDS_0] vm1/different-policy-disk.vmdk", + }, + Uuid: "disk-uuid-different-policy", + }, + }, + CapacityInBytes: 2 * 1024 * 1024 * 1024, + } + + scsiController := &vimtypes.VirtualSCSIController{ + VirtualController: vimtypes.VirtualController{ + VirtualDevice: vimtypes.VirtualDevice{ + Key: 200, + }, + BusNumber: 1, + }, + } + + moVM.Config = &vimtypes.VirtualMachineConfigInfo{ + Hardware: vimtypes.VirtualHardware{ + Device: []vimtypes.BaseVirtualDevice{ + scsiController, + disk, + }, + }, + } + }) + + It("should create PVC with storage class matching disk's actual profile, not VM's storage class", func() { + Expect(unmanagedvolsreg.Reconcile( + ctx, + k8sClient, + vimClient, + vm, + moVM, + configSpec)).To(MatchError(unmanagedvolsreg.ErrPendingRegister)) + + claimName := vmopv1util.FindByTargetID( + vmopv1.VirtualControllerTypeSCSI, + 1, 0, vm.Spec.Volumes...).PersistentVolumeClaim.ClaimName + + var pvc corev1.PersistentVolumeClaim + Expect(k8sClient.Get(ctx, ctrlclient.ObjectKey{ + Namespace: vm.Namespace, + Name: claimName, + }, &pvc)).To(Succeed()) + + // Verify PVC has storage class matching the disk's actual profile (profile-456 -> my-storage-class-2) + // NOT the VM's storage class (my-storage-class-1) + Expect(pvc.Spec.StorageClassName).ToNot(BeNil()) + Expect(*pvc.Spec.StorageClassName).To(Equal("my-storage-class-2")) + + // Verify other PVC properties + Expect(pvc.Spec.VolumeMode).ToNot(BeNil()) + Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeBlock)) + Expect(pvc.Spec.DataSourceRef).ToNot(BeNil()) + Expect(pvc.Spec.DataSourceRef.Kind).To(Equal("VirtualMachine")) + Expect(pvc.Spec.DataSourceRef.Name).To(Equal(vm.Name)) + }) + }) + When("PVC exists and is bound", func() { BeforeEach(func() { // Set up VM with volumes already in spec