SPLAT-1811: Add vSphere multi disk support

cmd/machineset/main.go

@@ -110,7 +110,7 @@ func main() {
 
 	// Sets up feature gates
 	defaultMutableGate := feature.DefaultMutableFeatureGate
-	gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration)
+	gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereMultiDisk)
 	if err != nil {
 		klog.Fatalf("Error setting up feature gates: %v", err)
 	}

cmd/vsphere/main.go

@@ -92,7 +92,7 @@ func main() {
 
 	// Sets up feature gates
 	defaultMutableGate := feature.DefaultMutableFeatureGate
-	gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration)
+	gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereMultiDisk)
 	if err != nil {
 		klog.Fatalf("Error setting up feature gates: %v", err)
 	}
@@ -153,6 +153,9 @@ func main() {
 	staticIPFeatureGateEnabled := defaultMutableGate.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereStaticIPs))
 	klog.Infof("FeatureGateVSphereStaticIPs initialised: %t", staticIPFeatureGateEnabled)
 
+	multiDiskFeatureGateEnabled := defaultMutableGate.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereMultiDisk))
+	klog.Infof("FeatureGateVSphereMultiDisk initialised: %t", multiDiskFeatureGateEnabled)
+
 	// Setup a Manager
 	mgr, err := manager.New(cfg, opts)
 	if err != nil {

go.mod

@@ -277,3 +277,9 @@ require (
 	sigs.k8s.io/kustomize/kyaml v0.17.1 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )
+
+replace github.com/openshift/api => github.com/vr4manta/api v0.0.0-20241203153954-b8357fac9edf
+
+replace github.com/openshift/library-go => github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1
+
+// replace k8s.io/cloud-provider-vsphere => github.com/vr4manta/cloud-provider-vsphere v0.0.0-20240926163731-40cee92a0401

go.sum

@@ -371,16 +371,12 @@ github.com/onsi/ginkgo/v2 v2.20.2 h1:7NVCeyIWROIAheY21RLS+3j2bb52W0W82tkberYytp4
 github.com/onsi/ginkgo/v2 v2.20.2/go.mod h1:K9gyxPIlb+aIvnZ8bd9Ak+YP18w3APlR+5coaZoE2ag=
 github.com/onsi/gomega v1.34.2 h1:pNCwDkzrsv7MS9kpaQvVb1aVLahQXyJ/Tv5oAZMI3i8=
 github.com/onsi/gomega v1.34.2/go.mod h1:v1xfxRgk0KIsG+QOdm7p8UosrOzPYRo60fd3B/1Dukc=
-github.com/openshift/api v0.0.0-20241118215836-09b63162bb15 h1:5bt8CBbXv1iSmBNCBXugsIT04NgHFCSkM1mOyuE8s2Q=
-github.com/openshift/api v0.0.0-20241118215836-09b63162bb15/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
 github.com/openshift/client-go v0.0.0-20240918182115-6a8ead8397fd h1:Gd0+bYdcfGIsDOJ8BwTJJjQeXoziyIsTwqp/s38rKyM=
 github.com/openshift/client-go v0.0.0-20240918182115-6a8ead8397fd/go.mod h1:EB7GeA/vpf9AHklMgnnT0+uG6l/3f8cChtCFbJFrk4g=
 github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20241007145816-7038c320d36c h1:9A/0QoTZo2xh5j6nmh5CGNVBG8Ql1RmXmCcrikBnG+w=
 github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20241007145816-7038c320d36c/go.mod h1:EN1Sv7kcVtaLUiXpZ8V0iSiJxNPPz1H3ZhCmNRpJWZM=
 github.com/openshift/cluster-control-plane-machine-set-operator v0.0.0-20240909043600-373ac49835bf h1:mfMmaD9+vZIZQq3MGXsS/AGHXekj4wIn3zc1Cs1EY8M=
 github.com/openshift/cluster-control-plane-machine-set-operator v0.0.0-20240909043600-373ac49835bf/go.mod h1:2fZsjZ3QSPkoMUc8QntXfeBb8AnvW+WIYwwQX8vmgvQ=
-github.com/openshift/library-go v0.0.0-20240919205913-c96b82b3762b h1:y2DduJug7UZqTu0QTkRPAu73nskuUbFA66fmgxVf/fI=
-github.com/openshift/library-go v0.0.0-20240919205913-c96b82b3762b/go.mod h1:f8QcnrooSwGa96xI4UaKbKGJZskhTCGeimXKyc4t/ZU=
 github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
 github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU=
 github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w=
@@ -521,8 +517,16 @@ github.com/uudashr/gocognit v1.1.3 h1:l+a111VcDbKfynh+airAy/DJQKaXh2m9vkoysMPSZy
 github.com/uudashr/gocognit v1.1.3/go.mod h1:aKH8/e8xbTRBwjbCkwZ8qt4l2EpKXl31KMHgSS+lZ2U=
 github.com/vmware/govmomi v0.43.0 h1:7Kg3Bkdly+TrE67BYXzRq7ZrDnn7xqpKX95uEh2f9Go=
 github.com/vmware/govmomi v0.43.0/go.mod h1:IOv5nTXCPqH9qVJAlRuAGffogaLsNs8aF+e7vLgsHJU=
+github.com/vr4manta/api v0.0.0-20241203153954-b8357fac9edf h1:hX1fjMqY94xx8NgSJDwbOp8oWwHXT2my22tGxJhnW04=
+github.com/vr4manta/api v0.0.0-20241203153954-b8357fac9edf/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
+github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1 h1:1gyQPwD2jHfHESPJxs4/R7yxCLwiT2hJuavRvIksyWY=
+github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1/go.mod h1:HRCtk80UWFVTW7UShIFimUV0smpP7LGcQJw5mHuAmIw=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
 github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
+github.com/vr4manta/api v0.0.0-20240924150740-ec5392903387 h1:WDXPgFYRTEmlo3wKeo3/9idb42JbckSd75GyfjpgLPU=
+github.com/vr4manta/api v0.0.0-20240924150740-ec5392903387/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
+github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1 h1:1gyQPwD2jHfHESPJxs4/R7yxCLwiT2hJuavRvIksyWY=
+github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1/go.mod h1:HRCtk80UWFVTW7UShIFimUV0smpP7LGcQJw5mHuAmIw=
 github.com/xen0n/gosmopolitan v1.2.2 h1:/p2KTnMzwRexIW8GlKawsTWOxn7UHA+jCMF/V8HHtvU=
 github.com/xen0n/gosmopolitan v1.2.2/go.mod h1:7XX7Mj61uLYrj0qmeN0zi7XDon9JRAEhYQqAPLVNTeg=
 github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ=

pkg/controller/vsphere/reconciler.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net"
 	"net/netip"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -458,9 +459,12 @@ func (r *Reconciler) delete() error {
 	if err != nil {
 		return fmt.Errorf("%v: can not obtain virtual disks attached to the vm: %w", r.machine.GetName(), err)
 	}
-	// Currently, MAPI does not provide any API knobs to configure additional volumes for a VM.
-	// So, we are expecting the VM to have only one disk, which is OS disk.
-	if len(disks) > 1 {
+
+	additionalDisks := len(r.providerSpec.DataDisks)
+	// Currently, MAPI only allows VMs to be configured w/ 1 primary did in the template and a limited number of additional
+	// disks via the data disks configuration.  So, we are expecting the VM to have only one disk, which is OS disk, plus
+	// the additional disks defined in the DataDisks configuration.
+	if len(disks) > 1+additionalDisks {
 		// If node drain was skipped we need to detach disks forcefully to prevent possible data corruption.
 		if drainSkipped {
 			klog.V(1).Infof(
@@ -968,6 +972,13 @@ func clone(s *machineScope) (string, error) {
 		deviceSpecs = append(deviceSpecs, diskSpec)
 	}
 
+	// Add any additional disks
+	additionalDisks, err := getAdditionalDiskSpecs(s, devices, datastore)
+	if err != nil {
+		return "", fmt.Errorf("error getting additional disk specs: %w", err)
+	}
+	deviceSpecs = append(deviceSpecs, additionalDisks...)
+
 	klog.V(3).Infof("Getting network devices")
 	networkDevices, err := getNetworkDevices(s, resourcepool, devices)
 	if err != nil {
@@ -1088,6 +1099,113 @@ func getDiskSpec(s *machineScope, devices object.VirtualDeviceList) (types.BaseV
 	}, nil
 }
 
+func getAdditionalDiskSpecs(s *machineScope, devices object.VirtualDeviceList, datastore *object.Datastore) ([]types.BaseVirtualDeviceConfigSpec, error) {
+	var diskSpecs []types.BaseVirtualDeviceConfigSpec
+	unit := int32(1)
+
+	// Only add additional disks if the feature gate is enabled.
+	if len(s.providerSpec.DataDisks) > 0 && !s.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereMultiDisk)) {
+		return nil, machinecontroller.InvalidMachineConfiguration(
+			"machines cannot contain additional disks due to VSphereMultiDisk feature gate being disabled")
+	}
+
+	// Get primary disk
+	disks := devices.SelectByType((*types.VirtualDisk)(nil))
+	primaryDisk := disks[0].(*types.VirtualDisk)
+
+	// Let's create the data disks now
+	additionalDisks := []types.BaseVirtualDeviceConfigSpec{}
+	for i := range s.providerSpec.DataDisks {
+		diskSpec := s.providerSpec.DataDisks[i]
+		klog.InfoS("Adding disk", "spec", diskSpec)
+
+		//  Get controller.  Only supporting using same controller as primary disk at this time
+		controller, ok := devices.FindByKey(primaryDisk.ControllerKey).(types.BaseVirtualController)
+		if !ok {
+			klog.Infof("Unable to get scsi controller")
+		}
+
+		dev := &types.VirtualDisk{
+			VirtualDevice: types.VirtualDevice{
+				Key: devices.NewKey() - int32(i),
+				Backing: &types.VirtualDiskFlatVer2BackingInfo{
+					DiskMode:        string(types.VirtualDiskModePersistent),
+					ThinProvisioned: types.NewBool(true),
+					VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
+						FileName:  "",
+						Datastore: types.NewReference(datastore.Reference()),
+					},
+				},
+				ControllerKey: controller.GetVirtualController().Key,
+			},
+			CapacityInKB: int64(diskSpec.SizeGiB * 1024 * 1024),
+		}
+
+		assignUnitNumber(dev, devices, additionalDisks, controller, unit)
+		unit = *dev.UnitNumber
+
+		diskConfigSpec := types.VirtualDeviceConfigSpec{
+			Device:        dev,
+			Operation:     types.VirtualDeviceConfigSpecOperationAdd,
+			FileOperation: types.VirtualDeviceConfigSpecFileOperationCreate,
+		}
+
+		klog.V(1).InfoS("Generated device", "disk", dev)
+
+		additionalDisks = append(additionalDisks, &diskConfigSpec)
+	}
+	diskSpecs = append(diskSpecs, additionalDisks...)
+
+	return diskSpecs, nil
+}
+
+// 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
+	}
+
+	switch sc := controller.(type) {
+	case types.BaseVirtualSCSIController:
+		//  The SCSI controller sits on its own bus
+		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
+		}
+	}
+
+	for unit, used := range units {
+		if !used {
+			unit32 := int32(unit)
+			vd.UnitNumber = &unit32
+			klog.Infof("Assigned unit number %d = %d", unit, *vd.UnitNumber)
+			break
+		}
+	}
+}
+
 func getNetworkDevices(s *machineScope, resourcepool *object.ResourcePool, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) {
 	var networkDevices []types.BaseVirtualDeviceConfigSpec
 	// Remove any existing NICs
@@ -1520,15 +1638,16 @@ type attachedDisk struct {
 	diskMode string
 }
 
-// Filters out disks that look like vm OS disk.
+// Filters out disks that look like vm OS disk or any of the additional disks.
 // VM os disks filename contains the machine name in it
 // and has the format like "[DATASTORE] path-within-datastore/machine-name.vmdk".
 // This is based on vSphere behavior, an OS disk file gets a name that equals the target VM name during the clone operation.
 func filterOutVmOsDisk(attachedDisks []attachedDisk, machine *machinev1.Machine) []attachedDisk {
 	var disks []attachedDisk
+	regex, _ := regexp.Compile(fmt.Sprintf(".*\\/%s(_\\d*)?.vmdk", machine.GetName()))
 
 	for _, disk := range attachedDisks {
-		if strings.HasSuffix(disk.fileName, fmt.Sprintf("/%s.vmdk", machine.GetName())) {
+		if regex.MatchString(disk.fileName) {
 			continue
 		}
 		disks = append(disks, disk)

pkg/controller/vsphere/reconciler_test.go

@@ -1162,6 +1162,7 @@ func TestGetDiskSpec(t *testing.T) {
 		expectedError        error
 		devices              func() object.VirtualDeviceList
 		diskSize             int32
+		diskCount            int32
 		expectedCapacityInKB int64
 	}{
 		{
@@ -1174,6 +1175,7 @@ func TestGetDiskSpec(t *testing.T) {
 				return devices
 			},
 			diskSize:             10,
+			diskCount:            1,
 			expectedCapacityInKB: 10485760,
 		},
 		{
@@ -1186,6 +1188,7 @@ func TestGetDiskSpec(t *testing.T) {
 				return devices
 			},
 			diskSize:             30,
+			diskCount:            1,
 			expectedCapacityInKB: 31457280,
 		},
 		{
@@ -1200,6 +1203,7 @@ func TestGetDiskSpec(t *testing.T) {
 			},
 			expectedError:        errors.New("invalid disk count: 2"),
 			diskSize:             1,
+			diskCount:            1,
 			expectedCapacityInKB: 1048576,
 		},
 	}
@@ -3247,6 +3251,21 @@ func TestVmDisksManipulation(t *testing.T) {
 					"some nonsense",
 				},
 			},
+			{
+				name: "multiple vmdk names with machine name should be filtered out",
+				vmdkFilenames: []string{
+					"[DS] foo.vmdk",
+					fmt.Sprintf("[DS] foo/%s.vmdk", machineObj.Name),
+					fmt.Sprintf("[DS] foo/%s_1.vmdk", machineObj.Name),
+					"[DS] bar.vmdk",
+					"some nonsense",
+				},
+				expectedFilenames: []string{
+					"[DS] foo.vmdk",
+					"[DS] bar.vmdk",
+					"some nonsense",
+				},
+			},
 		}
 		for _, tc := range mockDisksTestCases {
 			t.Run(tc.name, func(t *testing.T) {

pkg/operator/operator.go

@@ -478,6 +478,7 @@ func (optr *Operator) maoConfigFromInfrastructure() (*OperatorConfig, error) {
 		string(apifeatures.FeatureGateVSphereStaticIPs):      featureGates.Enabled(apifeatures.FeatureGateVSphereStaticIPs),
 		string(apifeatures.FeatureGateGCPLabelsTags):         featureGates.Enabled(apifeatures.FeatureGateGCPLabelsTags),
 		string(apifeatures.FeatureGateAzureWorkloadIdentity): featureGates.Enabled(apifeatures.FeatureGateAzureWorkloadIdentity),
+		string(apifeatures.FeatureGateVSphereMultiDisk):      featureGates.Enabled(apifeatures.FeatureGateVSphereMultiDisk),
 	}
 	if features[string(apifeatures.FeatureGateMachineAPIMigration)] {
 		klog.V(2).Info("Enabling MachineAPIMigration for provider controller and machinesets")

pkg/operator/operator_test.go

@@ -45,13 +45,15 @@ var (
 		{Name: apifeatures.FeatureGateVSphereStaticIPs},
 		{Name: apifeatures.FeatureGateGCPLabelsTags},
 		{Name: apifeatures.FeatureGateAzureWorkloadIdentity},
+		{Name: apifeatures.FeatureGateVSphereMultiDisk},
 	}
 
 	enabledFeatureMap = map[string]bool{
 		"MachineAPIMigration":   true,
 		"GCPLabelsTags":         true,
 		"AzureWorkloadIdentity": true,
 		"VSphereStaticIPs":      true,
+		"VSphereMultiDisk":      true,
 	}
 )
 

pkg/util/testing/testing.go

@@ -178,7 +178,7 @@ func NewMachineHealthCheck(name string) *machinev1.MachineHealthCheck {
 
 func NewDefaultMutableFeatureGate() (featuregate.MutableFeatureGate, error) {
 	defaultMutableGate := feature.DefaultMutableFeatureGate
-	_, err := features.NewFeatureGateOptions(defaultMutableGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateMachineAPIMigration, openshiftfeatures.FeatureGateVSphereStaticIPs)
+	_, err := features.NewFeatureGateOptions(defaultMutableGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateMachineAPIMigration, openshiftfeatures.FeatureGateVSphereStaticIPs, openshiftfeatures.FeatureGateVSphereMultiDisk)
 	if err != nil {
 		return nil, fmt.Errorf("failed to set up default feature gate: %w", err)
 	}

pkg/webhooks/machine_webhook.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"strings"
 
+	apifeatures "github.com/openshift/api/features"
 	"k8s.io/component-base/featuregate"
 
 	corev1 "k8s.io/api/core/v1"
@@ -175,7 +176,8 @@ const (
 	minVSphereCPU       = 2
 	minVSphereMemoryMiB = 2048
 	// https://docs.openshift.com/container-platform/4.1/installing/installing_vsphere/installing-vsphere.html#minimum-resource-requirements_installing-vsphere
-	minVSphereDiskGiB = 120
+	minVSphereDiskGiB  = 120
+	maxAdditionalDisks = 15
 
 	// Nutanix Defaults
 	// Minimum Nutanix values taken from Nutanix reconciler
@@ -1494,6 +1496,14 @@ func validateVSphere(m *machinev1beta1.Machine, config *admissionConfig) (bool,
 		}
 	}
 
+	if len(providerSpec.DataDisks) > 0 {
+		if !config.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereMultiDisk)) {
+			errs = append(errs, field.Forbidden(field.NewPath("providerSpec", "disks"), "this field is protected by the VSphereMultiDisk feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set"))
+		} else if len(providerSpec.DataDisks) > maxAdditionalDisks {
+			errs = append(errs, field.Invalid(field.NewPath("providerSpec", "disks"), len(providerSpec.DataDisks), fmt.Sprintf("additional disk count must not exceed %d", maxAdditionalDisks)))
+		}
+	}
+
 	if len(errs) > 0 {
 		return false, warnings, errs
 	}

pkg/webhooks/machineset_webhook.go

@@ -3,6 +3,7 @@ package webhooks
 import (
 	"context"
 	"fmt"
+	"k8s.io/component-base/featuregate"
 	"reflect"
 
 	osconfigv1 "github.com/openshift/api/config/v1"

pkg/webhooks/machineset_webhook_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
 	"testing"
 
 	testutils "github.com/openshift/machine-api-operator/pkg/util/testing"