Fixed logic to handle vSphere image template w/ multiple disks

Author: vr4manta

config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachines.yaml

@@ -995,12 +995,6 @@ spec:
                   description: VSphereDisk describes additional disks for vSphere
                     to be added to VM that are not part of the VM OVA template.
                   properties:
-                    deviceName:
-                      description: |-
-                        DeviceName is a name to be used to identify the disk definition. If deviceName is not specified,
-                        the disk will still be created.  The deviceName should be unique so that it can be used to clearly
-                        identify purpose of the disk, but is not required to be unique.
-                      type: string
                     sizeGiB:
                       description: SizeGiB is the size of the disk (in GiB).
                       format: int64

config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml

@@ -865,12 +865,6 @@ spec:
                             vSphere to be added to VM that are not part of the VM
                             OVA template.
                           properties:
-                            deviceName:
-                              description: |-
-                                DeviceName is a name to be used to identify the disk definition. If deviceName is not specified,
-                                the disk will still be created.  The deviceName should be unique so that it can be used to clearly
-                                identify purpose of the disk, but is not required to be unique.
-                              type: string
                             sizeGiB:
                               description: SizeGiB is the size of the disk (in GiB).
                               format: int64

config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherevms.yaml

@@ -1088,12 +1088,6 @@ spec:
                   description: VSphereDisk describes additional disks for vSphere
                     to be added to VM that are not part of the VM OVA template.
                   properties:
-                    deviceName:
-                      description: |-
-                        DeviceName is a name to be used to identify the disk definition. If deviceName is not specified,
-                        the disk will still be created.  The deviceName should be unique so that it can be used to clearly
-                        identify purpose of the disk, but is not required to be unique.
-                      type: string
                     sizeGiB:
                       description: SizeGiB is the size of the disk (in GiB).
                       format: int64

pkg/services/govmomi/vcenter/clone.go

@@ -374,18 +374,50 @@ func getDiskSpec(vmCtx *capvcontext.VMContext, devices object.VirtualDeviceList)
 
 	// Now if we have increased the disk size of any additional disks that were in the template, we can now add new disks
 	// that are present in the additionalDisks list.
-	for i, dataDisk := range vmCtx.VSphereVM.Spec.DataDisks {
+	if len(vmCtx.VSphereVM.Spec.DataDisks) > 0 {
+		additionalDisks, diskErr := createAdditionalDisks(vmCtx.VSphereVM.Spec.DataDisks, primaryDisk, devices)
+		if diskErr != nil {
+			klog.Errorf("Unable to add additional disks: %v", diskErr)
+			return nil, diskErr
+		}
+		diskSpecs = append(diskSpecs, additionalDisks...)
+	}
+
+	return diskSpecs, nil
+}
+
+func getDiskConfigSpec(disk *types.VirtualDisk, diskCloneCapacityKB int64) (types.BaseVirtualDeviceConfigSpec, error) {
+	switch {
+	case diskCloneCapacityKB == 0:
+		// No disk size specified for the clone. Default to the template disk capacity.
+	case diskCloneCapacityKB > 0 && diskCloneCapacityKB >= disk.CapacityInKB:
+		disk.CapacityInKB = diskCloneCapacityKB
+	case diskCloneCapacityKB > 0 && diskCloneCapacityKB < disk.CapacityInKB:
+		return nil, errors.Errorf(
+			"can't resize template disk down, initial capacity is larger: %dKiB > %dKiB",
+			disk.CapacityInKB, diskCloneCapacityKB)
+	}
+
+	return &types.VirtualDeviceConfigSpec{
+		Operation: types.VirtualDeviceConfigSpecOperationEdit,
+		Device:    disk,
+	}, nil
+}
+
+func createAdditionalDisks(disks []infrav1.VSphereDisk, primaryDisk *types.VirtualDisk, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) {
+	additionalDisks := []types.BaseVirtualDeviceConfigSpec{}
+	unit := int32(1)
+
+	for i, dataDisk := range disks {
 		klog.InfoS("Adding disk", "spec", dataDisk)
 
-		// Need storage policy
-		// Need scsi controller
-		controller, err := devices.FindDiskController("scsi")
-		if err != nil {
+		//  Get controller.  Only supporting using same controller as primary disk at this time
+		controller, ok := devices.FindByKey(primaryDisk.ControllerKey).(types.BaseVirtualController) // TODO: Do this
+		if !ok {
 			klog.Infof("Unable to get scsi controller")
+			return nil, errors.Errorf("unable to find controller with key=%v", primaryDisk.ControllerKey)
 		}
 
-		unit := int32(i + 1)
-
 		dev := &types.VirtualDisk{
 			VirtualDevice: types.VirtualDevice{
 				Key: devices.NewKey() - int32(i),
@@ -394,15 +426,17 @@ func getDiskSpec(vmCtx *capvcontext.VMContext, devices object.VirtualDeviceList)
 					ThinProvisioned: types.NewBool(true),
 					VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
 						FileName: "",
-						//Datastore: types.NewReference(datastore.Reference()),
 					},
 				},
-				UnitNumber:    &unit,
 				ControllerKey: controller.GetVirtualController().Key,
 			},
 			CapacityInKB: dataDisk.SizeGiB * 1024 * 1024,
 		}
 
+		// Assign unit number to next slot on controller
+		assignUnitNumber(dev, devices, additionalDisks, controller, unit)
+		unit = *dev.UnitNumber
+
 		diskConfigSpec := types.VirtualDeviceConfigSpec{
 			Device:        dev,
 			Operation:     types.VirtualDeviceConfigSpecOperationAdd,
@@ -411,28 +445,59 @@ func getDiskSpec(vmCtx *capvcontext.VMContext, devices object.VirtualDeviceList)
 
 		klog.InfoS("Generated device", "dev", dev)
 
-		diskSpecs = append(diskSpecs, &diskConfigSpec)
+		additionalDisks = append(additionalDisks, &diskConfigSpec)
 	}
 
-	return diskSpecs, nil
+	return additionalDisks, nil
 }
 
-func getDiskConfigSpec(disk *types.VirtualDisk, diskCloneCapacityKB int64) (types.BaseVirtualDeviceConfigSpec, error) {
-	switch {
-	case diskCloneCapacityKB == 0:
-		// No disk size specified for the clone. Default to the template disk capacity.
-	case diskCloneCapacityKB > 0 && diskCloneCapacityKB >= disk.CapacityInKB:
-		disk.CapacityInKB = diskCloneCapacityKB
-	case diskCloneCapacityKB > 0 && diskCloneCapacityKB < disk.CapacityInKB:
-		return nil, errors.Errorf(
-			"can't resize template disk down, initial capacity is larger: %dKiB > %dKiB",
-			disk.CapacityInKB, diskCloneCapacityKB)
+// assignController assigns a device to a controller.
+func assignUnitNumber(device types.BaseVirtualDevice, existingDevices object.VirtualDeviceList, newDevices []types.BaseVirtualDeviceConfigSpec, controller types.BaseVirtualController, offset int32) {
+	vd := device.GetVirtualDevice()
+	vd.ControllerKey = controller.GetVirtualController().Key
+	vd.UnitNumber = &offset
+
+	units := make([]bool, 30)
+
+	for i := 0; i < int(offset); i++ {
+		units[i] = true
 	}
 
-	return &types.VirtualDeviceConfigSpec{
-		Operation: types.VirtualDeviceConfigSpecOperationEdit,
-		Device:    disk,
-	}, nil
+	sc, ok := controller.(types.BaseVirtualSCSIController)
+	if ok {
+		//  The SCSI controller sits on its own bus
+		klog.Infof("Marking SCSI Controller's unit number: %d", sc.GetVirtualSCSIController().ScsiCtlrUnitNumber)
+		units[sc.GetVirtualSCSIController().ScsiCtlrUnitNumber] = true
+	}
+
+	key := controller.GetVirtualController().Key
+
+	// Check all existing devices
+	for _, device := range existingDevices {
+		d := device.GetVirtualDevice()
+
+		if d.ControllerKey == key && d.UnitNumber != nil {
+			units[int(*d.UnitNumber)] = true
+		}
+	}
+
+	// Check new devices
+	for _, device := range newDevices {
+		d := device.GetVirtualDeviceConfigSpec().Device.GetVirtualDevice()
+
+		if d.ControllerKey == key && d.UnitNumber != nil {
+			units[int(*d.UnitNumber)] = true
+		}
+	}
+
+	// Assign first unused unit number
+	for unit, used := range units {
+		if !used {
+			unit32 := int32(unit)
+			vd.UnitNumber = &unit32
+			break
+		}
+	}
 }
 
 const ethCardType = "vmxnet3"