Skip to content

Commit b2522f6

Browse files
Merge pull request #1290 from vr4manta/vsphere_disk
SPLAT-1811: Add vSphere multi disk support
2 parents d9a57b6 + f8b5539 commit b2522f6

File tree

48 files changed

+1623
-60
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1623
-60
lines changed

cmd/machineset/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func main() {
112112

113113
// Sets up feature gates
114114
defaultMutableGate := feature.DefaultMutableFeatureGate
115-
gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereHostVMGroupZonal)
115+
gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereHostVMGroupZonal, apifeatures.FeatureGateVSphereMultiDisk)
116116
if err != nil {
117117
klog.Fatalf("Error setting up feature gates: %v", err)
118118
}

cmd/vsphere/main.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func main() {
9393

9494
// Sets up feature gates
9595
defaultMutableGate := feature.DefaultMutableFeatureGate
96-
gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereHostVMGroupZonal)
96+
gateOpts, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration, apifeatures.FeatureGateVSphereHostVMGroupZonal, apifeatures.FeatureGateVSphereMultiDisk)
9797
if err != nil {
9898
klog.Fatalf("Error setting up feature gates: %v", err)
9999
}
@@ -155,6 +155,8 @@ func main() {
155155
klog.Infof("FeatureGateVSphereStaticIPs initialised: %t", staticIPFeatureGateEnabled)
156156
hostVMGroupZonalFeatureGateEnabled := defaultMutableGate.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereHostVMGroupZonal))
157157
klog.Infof("FeatureGateVSphereHostVMGroupZonal initialised %t", hostVMGroupZonalFeatureGateEnabled)
158+
multiDiskFeatureGateEnabled := defaultMutableGate.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereMultiDisk))
159+
klog.Infof("FeatureGateVSphereMultiDisk initialised: %t", multiDiskFeatureGateEnabled)
158160

159161
// Setup a Manager
160162
mgr, err := manager.New(cfg, opts)

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ require (
1111
github.com/google/uuid v1.6.0
1212
github.com/onsi/ginkgo/v2 v2.20.2
1313
github.com/onsi/gomega v1.34.2
14-
github.com/openshift/api v0.0.0-20241210144725-fa836ae33dad
14+
github.com/openshift/api v0.0.0-20250108172834-78bd56dba39b
1515
github.com/openshift/client-go v0.0.0-20241203091221-452dfb8fa071
1616
github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20241007145816-7038c320d36c
1717
github.com/openshift/cluster-control-plane-machine-set-operator v0.0.0-20240909043600-373ac49835bf
18-
github.com/openshift/library-go v0.0.0-20241210151741-8d0b2ce3fb30
18+
github.com/openshift/library-go v0.0.0-20250114132252-af5b21ebad2f
1919
github.com/prometheus/client_golang v1.20.5
2020
github.com/spf13/cobra v1.8.1
2121
github.com/spf13/pflag v1.0.5

go.sum

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,16 +371,16 @@ github.com/onsi/ginkgo/v2 v2.20.2 h1:7NVCeyIWROIAheY21RLS+3j2bb52W0W82tkberYytp4
371371
github.com/onsi/ginkgo/v2 v2.20.2/go.mod h1:K9gyxPIlb+aIvnZ8bd9Ak+YP18w3APlR+5coaZoE2ag=
372372
github.com/onsi/gomega v1.34.2 h1:pNCwDkzrsv7MS9kpaQvVb1aVLahQXyJ/Tv5oAZMI3i8=
373373
github.com/onsi/gomega v1.34.2/go.mod h1:v1xfxRgk0KIsG+QOdm7p8UosrOzPYRo60fd3B/1Dukc=
374-
github.com/openshift/api v0.0.0-20241210144725-fa836ae33dad h1:AehKFLGZosHv1+stPXervUQQkd6/D/TdTprv1CgMpmU=
375-
github.com/openshift/api v0.0.0-20241210144725-fa836ae33dad/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
374+
github.com/openshift/api v0.0.0-20250108172834-78bd56dba39b h1:Nt4V9k5pyw2CiUL2L5IFlstvURf+12Z7uSzi/v30UpE=
375+
github.com/openshift/api v0.0.0-20250108172834-78bd56dba39b/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
376376
github.com/openshift/client-go v0.0.0-20241203091221-452dfb8fa071 h1:l0++HnGVKBcs8kXFL/1yeozxioxPGNpp0PYe3Y+0sq4=
377377
github.com/openshift/client-go v0.0.0-20241203091221-452dfb8fa071/go.mod h1:gL0laCCiIaNTNw1ZsMQZXBVu2NeQFpNWm9bLtYO9+ZU=
378378
github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20241007145816-7038c320d36c h1:9A/0QoTZo2xh5j6nmh5CGNVBG8Ql1RmXmCcrikBnG+w=
379379
github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20241007145816-7038c320d36c/go.mod h1:EN1Sv7kcVtaLUiXpZ8V0iSiJxNPPz1H3ZhCmNRpJWZM=
380380
github.com/openshift/cluster-control-plane-machine-set-operator v0.0.0-20240909043600-373ac49835bf h1:mfMmaD9+vZIZQq3MGXsS/AGHXekj4wIn3zc1Cs1EY8M=
381381
github.com/openshift/cluster-control-plane-machine-set-operator v0.0.0-20240909043600-373ac49835bf/go.mod h1:2fZsjZ3QSPkoMUc8QntXfeBb8AnvW+WIYwwQX8vmgvQ=
382-
github.com/openshift/library-go v0.0.0-20241210151741-8d0b2ce3fb30 h1:GgQjncgAS++/7mTB6p5vCQGof46gdeY74WXn21Rz7CE=
383-
github.com/openshift/library-go v0.0.0-20241210151741-8d0b2ce3fb30/go.mod h1:eGSI6tp7yUVr4V2d0WrVt2l5s3iCwAh8Hi0RC9Fo16U=
382+
github.com/openshift/library-go v0.0.0-20250114132252-af5b21ebad2f h1:inJ2wNKevyuR+7VfBecMJ+HjEsVwbTtw0x3SoZZ24uI=
383+
github.com/openshift/library-go v0.0.0-20250114132252-af5b21ebad2f/go.mod h1:Dsex3pPrZ+krgVFZIv21DGzeFvcC0muMvaQe9E/q0uI=
384384
github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
385385
github.com/otiai10/copy v1.14.0 h1:dCI/t1iTdYGtkvCuBG2BgR6KZa83PTclw4U5n2wAllU=
386386
github.com/otiai10/copy v1.14.0/go.mod h1:ECfuL02W+/FkTWZWgQqXPWZgW9oeKCSQ5qVfSc4qc4w=
@@ -523,6 +523,10 @@ github.com/vmware/govmomi v0.43.0 h1:7Kg3Bkdly+TrE67BYXzRq7ZrDnn7xqpKX95uEh2f9Go
523523
github.com/vmware/govmomi v0.43.0/go.mod h1:IOv5nTXCPqH9qVJAlRuAGffogaLsNs8aF+e7vLgsHJU=
524524
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
525525
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
526+
github.com/vr4manta/api v0.0.0-20240924150740-ec5392903387 h1:WDXPgFYRTEmlo3wKeo3/9idb42JbckSd75GyfjpgLPU=
527+
github.com/vr4manta/api v0.0.0-20240924150740-ec5392903387/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
528+
github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1 h1:1gyQPwD2jHfHESPJxs4/R7yxCLwiT2hJuavRvIksyWY=
529+
github.com/vr4manta/library-go v0.0.0-20240910183943-6bfccc981bf1/go.mod h1:HRCtk80UWFVTW7UShIFimUV0smpP7LGcQJw5mHuAmIw=
526530
github.com/xen0n/gosmopolitan v1.2.2 h1:/p2KTnMzwRexIW8GlKawsTWOxn7UHA+jCMF/V8HHtvU=
527531
github.com/xen0n/gosmopolitan v1.2.2/go.mod h1:7XX7Mj61uLYrj0qmeN0zi7XDon9JRAEhYQqAPLVNTeg=
528532
github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ=

pkg/controller/vsphere/reconciler.go

Lines changed: 137 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"net"
1010
"net/netip"
11+
"regexp"
1112
"slices"
1213
"strconv"
1314
"strings"
@@ -50,6 +51,10 @@ const (
5051
regionKey = "region"
5152
zoneKey = "zone"
5253
minimumHWVersion = 15
54+
// maxUnitNumber constant is used to define the maximum number of devices that can be assigned to a virtual machine's controller.
55+
// Not all controllers support up to 30, but the maximum is 30.
56+
// xref: https://docs.vmware.com/en/VMware-vSphere/8.0/vsphere-vm-administration/GUID-5872D173-A076-42FE-8D0B-9DB0EB0E7362.html#:~:text=If%20you%20add%20a%20hard,values%20from%200%20to%2014.
57+
maxUnitNumber = 30
5358
)
5459

5560
// These are the guestinfo variables used by Ignition.
@@ -479,9 +484,12 @@ func (r *Reconciler) delete() error {
479484
if err != nil {
480485
return fmt.Errorf("%v: can not obtain virtual disks attached to the vm: %w", r.machine.GetName(), err)
481486
}
482-
// Currently, MAPI does not provide any API knobs to configure additional volumes for a VM.
483-
// So, we are expecting the VM to have only one disk, which is OS disk.
484-
if len(disks) > 1 {
487+
488+
additionalDisks := len(r.providerSpec.DataDisks)
489+
// Currently, MAPI only allows VMs to be configured w/ 1 primary disk in the template and a limited number of additional
490+
// disks via the data disks configuration. So, we are expecting the VM to have only one disk, which is OS disk, plus
491+
// the additional disks defined in the DataDisks configuration.
492+
if len(disks) > 1+additionalDisks {
485493
// If node drain was skipped we need to detach disks forcefully to prevent possible data corruption.
486494
if drainSkipped {
487495
klog.V(1).Infof(
@@ -996,6 +1004,13 @@ func clone(s *machineScope) (string, error) {
9961004
deviceSpecs = append(deviceSpecs, diskSpec)
9971005
}
9981006

1007+
// Process all DataDisks definitions to dynamically create and add disks to the VM
1008+
additionalDisks, err := createDataDisks(s, devices)
1009+
if err != nil {
1010+
return "", fmt.Errorf("error getting additional disk specs: %w", err)
1011+
}
1012+
deviceSpecs = append(deviceSpecs, additionalDisks...)
1013+
9991014
klog.V(3).Infof("Getting network devices")
10001015
networkDevices, err := getNetworkDevices(s, resourcepool, devices)
10011016
if err != nil {
@@ -1196,6 +1211,122 @@ func getDiskSpec(s *machineScope, devices object.VirtualDeviceList) (types.BaseV
11961211
}, nil
11971212
}
11981213

1214+
func createDataDisks(s *machineScope, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) {
1215+
var diskSpecs []types.BaseVirtualDeviceConfigSpec
1216+
1217+
// Only add additional disks if the feature gate is enabled.
1218+
if len(s.providerSpec.DataDisks) > 0 && !s.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereMultiDisk)) {
1219+
return nil, machinecontroller.InvalidMachineConfiguration(
1220+
"machines cannot contain additional disks due to VSphereMultiDisk feature gate being disabled")
1221+
}
1222+
1223+
// Get primary disk
1224+
disks := devices.SelectByType((*types.VirtualDisk)(nil))
1225+
if len(disks) == 0 {
1226+
return nil, fmt.Errorf("invalid disk count: %d", len(disks))
1227+
}
1228+
1229+
// There is at least one disk
1230+
primaryDisk := disks[0].(*types.VirtualDisk)
1231+
1232+
// Get the controller of the primary disk.
1233+
controller, ok := devices.FindByKey(primaryDisk.ControllerKey).(types.BaseVirtualController)
1234+
if !ok {
1235+
return nil, fmt.Errorf("unable to find controller with key=%v", primaryDisk.ControllerKey)
1236+
}
1237+
1238+
controllerKey := controller.GetVirtualController().Key
1239+
unitNumberAssigner, err := newUnitNumberAssigner(controller, devices)
1240+
if err != nil {
1241+
return nil, fmt.Errorf("unable to create unit number assigner: %v", err)
1242+
}
1243+
1244+
// Let's create the data disks now
1245+
for i, dataDisk := range s.providerSpec.DataDisks {
1246+
klog.V(2).InfoS("Adding disk", "name", dataDisk.Name, "spec", dataDisk)
1247+
1248+
dev := &types.VirtualDisk{
1249+
VirtualDevice: types.VirtualDevice{
1250+
// Key needs to be unique and cannot match another new disk being added. So we'll use the index as an
1251+
// input to NewKey. NewKey() will always return same value since our new devices are not part of devices yet.
1252+
Key: devices.NewKey() - int32(i),
1253+
Backing: &types.VirtualDiskFlatVer2BackingInfo{
1254+
DiskMode: string(types.VirtualDiskModePersistent),
1255+
ThinProvisioned: types.NewBool(true),
1256+
VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
1257+
FileName: "",
1258+
},
1259+
},
1260+
ControllerKey: controller.GetVirtualController().Key,
1261+
},
1262+
CapacityInKB: int64(dataDisk.SizeGiB) * 1024 * 1024,
1263+
}
1264+
1265+
vd := dev.GetVirtualDevice()
1266+
vd.ControllerKey = controllerKey
1267+
1268+
// Assign unit number to the new disk. Should be next available slot on the controller.
1269+
unitNumber, err := unitNumberAssigner.assign()
1270+
if err != nil {
1271+
return nil, err
1272+
}
1273+
vd.UnitNumber = &unitNumber
1274+
1275+
klog.V(2).InfoS("Created device for data disk device", "name", dataDisk.Name, "spec", dataDisk, "device", dev)
1276+
diskSpecs = append(diskSpecs, &types.VirtualDeviceConfigSpec{
1277+
Device: dev,
1278+
Operation: types.VirtualDeviceConfigSpecOperationAdd,
1279+
FileOperation: types.VirtualDeviceConfigSpecFileOperationCreate,
1280+
})
1281+
}
1282+
1283+
return diskSpecs, nil
1284+
}
1285+
1286+
type unitNumberAssigner struct {
1287+
used []bool
1288+
offset int32
1289+
}
1290+
1291+
func newUnitNumberAssigner(controller types.BaseVirtualController, existingDevices object.VirtualDeviceList) (*unitNumberAssigner, error) {
1292+
if controller == nil {
1293+
return nil, errors.New("controller parameter cannot be nil")
1294+
}
1295+
used := make([]bool, maxUnitNumber)
1296+
1297+
// SCSIControllers also use a unit.
1298+
if scsiController, ok := controller.(types.BaseVirtualSCSIController); ok {
1299+
used[scsiController.GetVirtualSCSIController().ScsiCtlrUnitNumber] = true
1300+
}
1301+
controllerKey := controller.GetVirtualController().Key
1302+
1303+
// Mark all unit numbers of existing devices as used
1304+
for _, device := range existingDevices {
1305+
d := device.GetVirtualDevice()
1306+
if d.ControllerKey == controllerKey && d.UnitNumber != nil {
1307+
used[*d.UnitNumber] = true
1308+
}
1309+
}
1310+
1311+
// Set offset to 0, it will auto-increment on the first assignment.
1312+
return &unitNumberAssigner{used: used, offset: 0}, nil
1313+
}
1314+
1315+
func (a *unitNumberAssigner) assign() (int32, error) {
1316+
if int(a.offset) > len(a.used) {
1317+
return -1, fmt.Errorf("all unit numbers are already in-use")
1318+
}
1319+
for i, isInUse := range a.used[a.offset:] {
1320+
unit := int32(i) + a.offset
1321+
if !isInUse {
1322+
a.used[unit] = true
1323+
a.offset++
1324+
return unit, nil
1325+
}
1326+
}
1327+
return -1, fmt.Errorf("all unit numbers are already in-use")
1328+
}
1329+
11991330
func getNetworkDevices(s *machineScope, resourcepool *object.ResourcePool, devices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) {
12001331
var networkDevices []types.BaseVirtualDeviceConfigSpec
12011332
// Remove any existing NICs
@@ -1628,15 +1759,16 @@ type attachedDisk struct {
16281759
diskMode string
16291760
}
16301761

1631-
// Filters out disks that look like vm OS disk.
1762+
// Filters out disks that look like vm OS disk or any of the additional disks.
16321763
// VM os disks filename contains the machine name in it
16331764
// and has the format like "[DATASTORE] path-within-datastore/machine-name.vmdk".
16341765
// This is based on vSphere behavior, an OS disk file gets a name that equals the target VM name during the clone operation.
16351766
func filterOutVmOsDisk(attachedDisks []attachedDisk, machine *machinev1.Machine) []attachedDisk {
16361767
var disks []attachedDisk
1768+
regex, _ := regexp.Compile(fmt.Sprintf(".*\\/%s(_\\d*)?.vmdk", machine.GetName()))
16371769

16381770
for _, disk := range attachedDisks {
1639-
if strings.HasSuffix(disk.fileName, fmt.Sprintf("/%s.vmdk", machine.GetName())) {
1771+
if regex.MatchString(disk.fileName) {
16401772
continue
16411773
}
16421774
disks = append(disks, disk)

0 commit comments

Comments
 (0)