Misc fixes

vmware-tanzu · Sep 25, 2023 · 1a24c7d · 1a24c7d
1 parent 862586c
commit 1a24c7d
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 176 deletions.
diff --git a/pkg/vmprovider/providers/vsphere2/virtualmachine/configspec.go b/pkg/vmprovider/providers/vsphere2/virtualmachine/configspec.go
@@ -23,8 +23,8 @@ func CreateConfigSpec(
 	vmCtx context.VirtualMachineContextA2,
 	vmClassConfigSpec *types.VirtualMachineConfigSpec,
 	vmClassSpec *vmopv1.VirtualMachineClassSpec,
-	minFreq uint64,
-	imageFirmware string) *types.VirtualMachineConfigSpec {
+	vmImageStatus *vmopv1.VirtualMachineImageStatus,
+	minFreq uint64) *types.VirtualMachineConfigSpec {
 
 	configSpec := types.VirtualMachineConfigSpec{}
 
@@ -49,9 +49,9 @@ func CreateConfigSpec(
 
 	if val, ok := vmCtx.VM.Annotations[constants.FirmwareOverrideAnnotation]; ok {
 		configSpec.Firmware = val
-	} else if configSpec.Firmware == "" {
-		// Use firmware type from the image if config spec doesn't have it.
-		configSpec.Firmware = imageFirmware
+	} else if configSpec.Firmware == "" && vmImageStatus != nil {
+		// Use firmware type from the image if ConfigSpec doesn't have it.
+		configSpec.Firmware = vmImageStatus.Firmware
 	}
 
 	// TODO: Otherwise leave as-is? Our ChangeBlockTracking could be better as a *bool.
@@ -103,8 +103,9 @@ func CreateConfigSpec(
 	return &configSpec
 }
 
-// CreateConfigSpecForPlacement2 creates a ConfigSpec that is suitable for Placement.
-func CreateConfigSpecForPlacement2(
+// CreateConfigSpecForPlacement creates a ConfigSpec that is suitable for Placement.
+// baseConfigSpec will likely be - or at least derived from - the ConfigSpec returned by CreateConfigSpec above.
+func CreateConfigSpecForPlacement(
 	vmCtx context.VirtualMachineContextA2,
 	baseConfigSpec *types.VirtualMachineConfigSpec,
 	storageClassesToIDs map[string]string) *types.VirtualMachineConfigSpec {
@@ -157,73 +158,6 @@ func CreateConfigSpecForPlacement2(
 		}
 	}
 
-	return &configSpec
-}
-
-// CreateConfigSpecForPlacement creates a ConfigSpec to use for placement. Once CL deploy can accept
-// a ConfigSpec, this should largely - or ideally entirely - be folded into CreateConfigSpec() above.
-func CreateConfigSpecForPlacement(
-	vmCtx context.VirtualMachineContextA2,
-	vmClassSpec *vmopv1.VirtualMachineClassSpec,
-	minFreq uint64,
-	storageClassesToIDs map[string]string,
-	imageFirmware string,
-	vmClassConfigSpec *types.VirtualMachineConfigSpec) *types.VirtualMachineConfigSpec {
-
-	configSpec := CreateConfigSpec(vmCtx, vmClassConfigSpec, vmClassSpec, minFreq, imageFirmware)
-
-	// Add a dummy disk for placement: PlaceVmsXCluster expects there to always be at least one disk.
-	// Until we're in a position to have the OVF envelope here, add a dummy disk satisfy it.
-	configSpec.DeviceChange = append(configSpec.DeviceChange, &types.VirtualDeviceConfigSpec{
-		Operation:     types.VirtualDeviceConfigSpecOperationAdd,
-		FileOperation: types.VirtualDeviceConfigSpecFileOperationCreate,
-		Device: &types.VirtualDisk{
-			CapacityInBytes: 1024 * 1024,
-			VirtualDevice: types.VirtualDevice{
-				Key: -42,
-				Backing: &types.VirtualDiskFlatVer2BackingInfo{
-					ThinProvisioned: pointer.Bool(true),
-				},
-			},
-		},
-		Profile: []types.BaseVirtualMachineProfileSpec{
-			&types.VirtualMachineDefinedProfileSpec{
-				ProfileId: storageClassesToIDs[vmCtx.VM.Spec.StorageClass],
-			},
-		},
-	})
-
-	// With DaynDate FSS, PCI devices are specified via ConfigSpec.  Ignore such devices from the
-	// VM Class Hardware to avoid duplicate devices being added to the placement spec.
-	if !lib.IsVMClassAsConfigFSSDaynDateEnabled() {
-		for _, dev := range CreatePCIDevicesFromVMClass(vmClassSpec.Hardware.Devices) {
-			configSpec.DeviceChange = append(configSpec.DeviceChange, &types.VirtualDeviceConfigSpec{
-				Operation: types.VirtualDeviceConfigSpecOperationAdd,
-				Device:    dev,
-			})
-		}
-	}
-
-	if lib.IsInstanceStorageFSSEnabled() {
-		isVolumes := instancestorage.FilterVolumes(vmCtx.VM)
-
-		for idx, dev := range CreateInstanceStorageDiskDevices(isVolumes) {
-			configSpec.DeviceChange = append(configSpec.DeviceChange, &types.VirtualDeviceConfigSpec{
-				Operation:     types.VirtualDeviceConfigSpecOperationAdd,
-				FileOperation: types.VirtualDeviceConfigSpecFileOperationCreate,
-				Device:        dev,
-				Profile: []types.BaseVirtualMachineProfileSpec{
-					&types.VirtualMachineDefinedProfileSpec{
-						ProfileId: storageClassesToIDs[isVolumes[idx].PersistentVolumeClaim.InstanceVolumeClaim.StorageClass],
-						ProfileData: &types.VirtualMachineProfileRawData{
-							ExtensionKey: "com.vmware.vim.sps",
-						},
-					},
-				},
-			})
-		}
-	}
-
 	// TODO: Add more devices and fields
 	//  - boot disks from OVA
 	//  - storage profile/class
@@ -233,21 +167,20 @@ func CreateConfigSpecForPlacement(
 	//  - any way to do the cluster modules for anti-affinity?
 	//  - whatever else I'm forgetting
 
-	return configSpec
+	return &configSpec
 }
 
 // ConfigSpecFromVMClassDevices creates a ConfigSpec that adds the standalone hardware devices from
 // the VMClass if any. This ConfigSpec will be used as the class ConfigSpec to CreateConfigSpec, with
 // the rest of the class fields - like CPU count - applied on top.
 func ConfigSpecFromVMClassDevices(vmClassSpec *vmopv1.VirtualMachineClassSpec) *types.VirtualMachineConfigSpec {
-	devices := vmClassSpec.Hardware.Devices
-
-	if len(devices.VGPUDevices) == 0 && len(devices.DynamicDirectPathIODevices) == 0 {
+	devsFromClass := CreatePCIDevicesFromVMClass(vmClassSpec.Hardware.Devices)
+	if len(devsFromClass) == 0 {
 		return nil
 	}
 
 	configSpec := &types.VirtualMachineConfigSpec{}
-	for _, dev := range CreatePCIDevicesFromVMClass(devices) {
+	for _, dev := range devsFromClass {
 		configSpec.DeviceChange = append(configSpec.DeviceChange, &types.VirtualDeviceConfigSpec{
 			Operation: types.VirtualDeviceConfigSpecOperationAdd,
 			Device:    dev,