Merge pull request #13007 from fabriziopandini/defer-in-place-for-machines-not-yet-provisioned

🌱 Defer in-place updates for machines not yet provisioned

Author: k8s-ci-robot

internal/controllers/machine/machine_controller_inplace_update.go

@@ -79,6 +79,11 @@ func (r *Reconciler) reconcileInPlaceUpdate(ctx context.Context, s *scope) (ctrl
 		return ctrl.Result{}, nil
 	}
 
+	if !s.machine.Status.NodeRef.IsDefined() {
+		log.V(5).Info("Machine status.nodeRef is not yet set, skipping in-place update")
+		return ctrl.Result{}, nil
+	}
+
 	if s.infraMachine == nil {
 		s.updatingReason = clusterv1.MachineInPlaceUpdateFailedReason
 		s.updatingMessage = "In-place update not possible: InfraMachine not found"

internal/controllers/machine/machine_controller_inplace_update_test.go

@@ -130,6 +130,7 @@ func TestReconcileInPlaceUpdate(t *testing.T) {
 				machine.Annotations[runtimev1.PendingHooksAnnotation] = runtimecatalog.HookName(runtimehooksv1.UpdateMachine)
 				machine.Status.Initialization.InfrastructureProvisioned = ptr.To(true)
 				machine.Status.Initialization.BootstrapDataSecretCreated = ptr.To(true)
+				machine.Status.NodeRef = clusterv1.MachineNodeReference{Name: "foo"}
 				return &Reconciler{}, &scope{machine: machine}
 			},
 			wantResult:      ctrl.Result{},
@@ -181,6 +182,7 @@ func TestReconcileInPlaceUpdate(t *testing.T) {
 				machine.Annotations[runtimev1.PendingHooksAnnotation] = runtimecatalog.HookName(runtimehooksv1.UpdateMachine)
 				machine.Status.Initialization.InfrastructureProvisioned = ptr.To(true)
 				machine.Status.Initialization.BootstrapDataSecretCreated = ptr.To(true)
+				machine.Status.NodeRef = clusterv1.MachineNodeReference{Name: "foo"}
 
 				infra := newTestUnstructured("GenericInfrastructureMachine", "infrastructure.cluster.x-k8s.io/v1beta2", "infra")
 				infra.SetAnnotations(map[string]string{clusterv1.UpdateInProgressAnnotation: ""})
@@ -242,6 +244,7 @@ func TestReconcileInPlaceUpdate(t *testing.T) {
 				machine.Annotations[runtimev1.PendingHooksAnnotation] = runtimecatalog.HookName(runtimehooksv1.UpdateMachine)
 				machine.Status.Initialization.InfrastructureProvisioned = ptr.To(true)
 				machine.Status.Initialization.BootstrapDataSecretCreated = ptr.To(true)
+				machine.Status.NodeRef = clusterv1.MachineNodeReference{Name: "foo"}
 
 				infra := newTestUnstructured("GenericInfrastructureMachine", "infrastructure.cluster.x-k8s.io/v1beta2", "infra")
 				infra.SetAnnotations(map[string]string{clusterv1.UpdateInProgressAnnotation: ""})

internal/controllers/machineset/machineset_controller.go

@@ -1000,6 +1000,11 @@ func (r *Reconciler) startMoveMachines(ctx context.Context, s *scope, targetMSNa
 			continue
 		}
 
+		// Make sure we are not moving machines not yet provisioned.
+		if !machine.Status.NodeRef.IsDefined() {
+			continue
+		}
+
 		// Note. Machines with the DeleteMachineAnnotation are going to be moved and the new MS
 		//       will take care of fulfilling this intent as soon as it scales down.
 		// Note. Also Machines marked as unhealthy by MHC are going to be moved, because otherwise

internal/controllers/machineset/machineset_controller_test.go

@@ -3012,6 +3012,27 @@ func TestMachineSetReconciler_startMoveMachines(t *testing.T) {
 			wantMovedMachines:    []string{"m3", "m4"}, // newest machines moved first with NewestMachineSetDeletionOrder
 			wantErr:              false,
 		},
+		{
+			name: "should not move machines not yet provisioned",
+			ms: newMachineSet("ms1", "cluster1", 2,
+				withDeletionOrder(clusterv1.NewestMachineSetDeletionOrder),
+				withMachineSetLabels(map[string]string{clusterv1.MachineDeploymentUniqueLabel: "123"}),
+				withMachineSetAnnotations(map[string]string{clusterv1.MachineSetMoveMachinesToMachineSetAnnotation: "ms2"}),
+			),
+			targetMS: newMachineSet("ms2", "cluster1", 2,
+				withMachineSetLabels(map[string]string{clusterv1.MachineDeploymentUniqueLabel: "456"}),
+				withMachineSetAnnotations(map[string]string{clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation: "ms1,ms3"}),
+			),
+			machines: []*clusterv1.Machine{
+				fakeMachine("m1", withOwnerMachineSet("ms1"), withMachineLabels(map[string]string{clusterv1.MachineDeploymentUniqueLabel: "123"}), withMachineFinalizer(), withCreationTimestamp(time.Now().Add(-4*time.Minute))),
+				fakeMachine("m2", withOwnerMachineSet("ms1"), withMachineLabels(map[string]string{clusterv1.MachineDeploymentUniqueLabel: "123"}), withMachineFinalizer(), withCreationTimestamp(time.Now().Add(-3*time.Minute))),
+			},
+			machinesToMove:       2,
+			interceptorFuncs:     interceptor.Funcs{},
+			wantMachinesNotMoved: []string{"m1", "m2"},
+			wantMovedMachines:    []string{},
+			wantErr:              false,
+		},
 		{
 			name: "should not move deleting machines, decrease the move count",
 			ms: newMachineSet("ms1", "cluster1", 2,