re-introduces patchMachineStatus

controllers/machineset_controller.go

@@ -257,25 +257,13 @@ func (r *MachineSetReconciler) reconcile(ctx context.Context, cluster *clusterv1
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to calculate MachineSet's Status")
 	}
-	// add the newly calculated status to the machine set
-	newStatus.DeepCopyInto(&ms.Status)
 
 	// Always updates status as machines come up or die.
-	//updatedMS, err := r.patchMachineSetStatus(ctx, machineSet, newStatus)
-	//if err != nil {
-	//	if syncErr != nil {
-	//		return ctrl.Result{}, errors.Wrapf(err, "failed to sync machines: %v. failed to patch MachineSet's Status", syncErr)
-	//	}
-	//	return ctrl.Result{}, errors.Wrap(err, "failed to patch MachineSet's Status")
-	//}
-
-	//if syncErr != nil {
-	//	return ctrl.Result{}, errors.Wrapf(syncErr, "failed to sync MachineSet replicas")
-	//}
+	updatedMS := r.patchMachineSetStatus(ctx, machineSet, newStatus)
 
 	var replicas int32
-	if ms.Spec.Replicas != nil {
-		replicas = *ms.Spec.Replicas
+	if updatedMS.Spec.Replicas != nil {
+		replicas = *updatedMS.Spec.Replicas
 	}
 
 	// Resync the MachineSet after MinReadySeconds as a last line of defense to guard against clock-skew.
@@ -285,15 +273,15 @@ func (r *MachineSetReconciler) reconcile(ctx context.Context, cluster *clusterv1
 	// exceeds MinReadySeconds could be incorrect.
 	// To avoid an available replica stuck in the ready state, we force a reconcile after MinReadySeconds,
 	// at which point it should confirm any available replica to be available.
-	if ms.Spec.MinReadySeconds > 0 &&
-		ms.Status.ReadyReplicas == replicas &&
-		ms.Status.AvailableReplicas != replicas {
+	if updatedMS.Spec.MinReadySeconds > 0 &&
+		updatedMS.Status.ReadyReplicas == replicas &&
+		updatedMS.Status.AvailableReplicas != replicas {
 
-		return ctrl.Result{RequeueAfter: time.Duration(ms.Spec.MinReadySeconds) * time.Second}, nil
+		return ctrl.Result{RequeueAfter: time.Duration(updatedMS.Spec.MinReadySeconds) * time.Second}, nil
 	}
 
 	// Quickly reconcile until the nodes become Ready.
-	if ms.Status.ReadyReplicas != replicas {
+	if updatedMS.Status.ReadyReplicas != replicas {
 		log.V(4).Info("Some nodes are not ready yet, requeuing until they are ready")
 		return ctrl.Result{RequeueAfter: 15 * time.Second}, nil
 	}
@@ -642,7 +630,7 @@ func (r *MachineSetReconciler) calculateStatus(ctx context.Context, cluster *clu
 }
 
 // patchMachineSetStatus attempts to update the Status.Replicas of the given MachineSet.
-func (r *MachineSetReconciler) patchMachineSetStatus(ctx context.Context, ms *clusterv1.MachineSet, newStatus *clusterv1.MachineSetStatus) (*clusterv1.MachineSet, error) {
+func (r *MachineSetReconciler) patchMachineSetStatus(ctx context.Context, ms *clusterv1.MachineSet, newStatus *clusterv1.MachineSetStatus) *clusterv1.MachineSet {
 	log := ctrl.LoggerFrom(ctx)
 
 	// This is the steady state. It happens when the MachineSet doesn't have any expectations, since
@@ -653,11 +641,9 @@ func (r *MachineSetReconciler) patchMachineSetStatus(ctx context.Context, ms *cl
 		ms.Status.ReadyReplicas == newStatus.ReadyReplicas &&
 		ms.Status.AvailableReplicas == newStatus.AvailableReplicas &&
 		ms.Generation == ms.Status.ObservedGeneration {
-		return ms, nil
+		return ms
 	}
 
-	patch := client.MergeFrom(ms.DeepCopyObject())
-
 	// Save the generation number we acted on, otherwise we might wrongfully indicate
 	// that we've seen a spec update when we retry.
 	newStatus.ObservedGeneration = ms.Generation
@@ -675,10 +661,8 @@ func (r *MachineSetReconciler) patchMachineSetStatus(ctx context.Context, ms *cl
 		fmt.Sprintf("sequence No: %v->%v", ms.Status.ObservedGeneration, newStatus.ObservedGeneration))
 
 	newStatus.DeepCopyInto(&ms.Status)
-	if err := r.Client.Status().Patch(ctx, ms, patch); err != nil {
-		return nil, err
-	}
-	return ms, nil
+
+	return ms
 }
 
 func (r *MachineSetReconciler) getMachineNode(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) (*corev1.Node, error) {

controllers/machineset_controller_test.go

@@ -621,7 +621,7 @@ func TestAdoptOrphan(t *testing.T) {
 	fakeRecorder := record.NewFakeRecorder(32)
 
 	r := &MachineSetReconciler{
-		Client: fake.NewFakeClientWithScheme(scheme.Scheme, &machine),
+		Client:   fake.NewFakeClientWithScheme(scheme.Scheme, &machine),
 		recorder: fakeRecorder,
 	}