Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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(
Expand All @@ -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{}{}
}
}

Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down