diff --git a/controllers/vmware/virtualmachinegroup_reconciler.go b/controllers/vmware/virtualmachinegroup_reconciler.go index d075debebe..ab303e4bd0 100644 --- a/controllers/vmware/virtualmachinegroup_reconciler.go +++ b/controllers/vmware/virtualmachinegroup_reconciler.go @@ -476,13 +476,23 @@ func shouldCreateVirtualMachineGroup(ctx context.Context, mds []clusterv1.Machin currentVSphereMachineCount++ } - // If the number of workers VSphereMachines matches the number of expected replicas in the MachineDeployments, - // then all the VSphereMachines required for the initial placement decision do exist, then it is possible to create - // the VirtualMachineGroup. - if currentVSphereMachineCount != expectedVSphereMachineCount { + // Gate initial VMG creation on having enough worker VSphereMachines for the sum of MachineDeployment replicas. + // If current < expected, wait. Initial placement is meant to wait for enough VSphereMachines to be created to form a batch. + // When current > expected (for e.g. MachineDeployment surge during a rolling update): + // (1) New clusters with Node Auto Placement enabled: worker VSphereMachines are created in a batch, and all are + // included together in the same initial placement, and membership are maintained afterward. + // (2) Existing clusters to adopt Node Auto Placement: CAPV can set per-MachineDeployment zone annotations on + // the VMG based on the placement result of exsting MachineDeployments, and pins new VSphereMachines to the right failureDomain. Proceed includes + // all current VSphereMachines, and membership are maintained afterward. + if currentVSphereMachineCount < expectedVSphereMachineCount { log.Info(fmt.Sprintf("Waiting for VSphereMachines required for the initial placement (expected %d, current %d)", expectedVSphereMachineCount, currentVSphereMachineCount)) return false } + + if currentVSphereMachineCount > expectedVSphereMachineCount { + log.Info(fmt.Sprintf("More VSphereMachines than expected replicas; proceeding to create VirtualMachineGroup with all current VSphereMachines (expected %d, current %d)", expectedVSphereMachineCount, currentVSphereMachineCount)) + } + return true } diff --git a/controllers/vmware/virtualmachinegroup_reconciler_test.go b/controllers/vmware/virtualmachinegroup_reconciler_test.go index 4a63181a63..5de3e87188 100644 --- a/controllers/vmware/virtualmachinegroup_reconciler_test.go +++ b/controllers/vmware/virtualmachinegroup_reconciler_test.go @@ -108,6 +108,22 @@ func Test_shouldCreateVirtualMachineGroup(t *testing.T) { }, want: false, // tot replicas = 4, 3 VSphereMachine exist }, + { + name: "Should create a VMG when more VSphereMachines than expected replicas exist", + mds: []clusterv1.MachineDeployment{ + *createMD("md1", "test-cluster", "", 2), + *createMD("md2", "test-cluster", "", 1), + *createMD("md3", "test-cluster", "zone1", 1), + }, + vSphereMachines: []vmwarev1.VSphereMachine{ + *createVSphereMachine("m1", "test-cluster", "md1"), + *createVSphereMachine("m2", "test-cluster", "md1"), + *createVSphereMachine("m3", "test-cluster", "md1"), // surge: one extra for md1 + *createVSphereMachine("m4", "test-cluster", "md2"), + *createVSphereMachine("m5", "test-cluster", "md3"), + }, + want: true, // tot replicas = 4, 5 VSphereMachines (current > expected) + }, { name: "Should not create a VMG there are no expected VSphereMachines", mds: []clusterv1.MachineDeployment{}, // No Machine deployments @@ -1110,6 +1126,45 @@ func TestVirtualMachineGroupReconciler_ReconcileSequence(t *testing.T) { }, }, }, + { + name: "VirtualMachineGroup should be created when more vSphereMachines than replicas exist", + cluster: clusterInitialized, + mds: []clusterv1.MachineDeployment{ + *createMD("md1", clusterInitialized.Name, "zone1", 2), + *createMD("md2", clusterInitialized.Name, "", 1), + }, + vSphereMachines: []vmwarev1.VSphereMachine{ + *createVSphereMachine("m1", clusterInitialized.Name, "md1", withCustomNamingStrategy()), + *createVSphereMachine("m2", clusterInitialized.Name, "md1", withCustomNamingStrategy()), + *createVSphereMachine("m3", clusterInitialized.Name, "md1", withCustomNamingStrategy()), // surge: extra worker for md1 + *createVSphereMachine("m4", clusterInitialized.Name, "md2"), + }, + existingVMG: nil, + wantResult: ctrl.Result{}, + wantVMG: &vmoprvhub.VirtualMachineGroup{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: clusterInitialized.Namespace, + Name: clusterInitialized.Name, + UID: types.UID("uid"), + Annotations: map[string]string{ + fmt.Sprintf("%s/%s", vmoperator.ZoneAnnotationPrefix, "md1"): "zone1", // failureDomain for md1 is explicitly set by the user + }, + // Not setting labels and ownerReferences for sake of simplicity + }, + Spec: vmoprvhub.VirtualMachineGroupSpec{ + BootOrder: []vmoprvhub.VirtualMachineGroupBootOrderGroup{ + { + Members: []vmoprvhub.GroupMember{ + {Name: "m1-vm", Kind: "VirtualMachine"}, + {Name: "m2-vm", Kind: "VirtualMachine"}, + {Name: "m3-vm", Kind: "VirtualMachine"}, + {Name: "m4", Kind: "VirtualMachine"}, + }, + }, + }, + }, + }, + }, // During initial placement {