Merge pull request #1362 from damdo/use-patch-for-machine-machineset-status

OCPBUGS-55720: fix: use patch for machine/machineset status

Author: openshift-merge-bot[bot]

pkg/controller/machine/controller_test.go

@@ -756,14 +756,15 @@ func TestUpdateStatus(t *testing.T) {
 					t.Fatalf("error deleting machine: %v", err)
 				}
 			}()
+			machineCopy := machine.DeepCopy()
 
 			if tc.existingProviderStatus != "" {
 				machine.Status.ProviderStatus = &runtime.RawExtension{
 					Raw: []byte(tc.existingProviderStatus),
 				}
 			}
 
-			gs.Expect(k8sClient.Status().Update(ctx, machine)).To(Succeed())
+			gs.Expect(k8sClient.Status().Patch(ctx, machine, client.MergeFrom(machineCopy))).To(Succeed())
 
 			namespacedName := types.NamespacedName{
 				Namespace: machine.Namespace,

pkg/controller/machine/drain_controller.go

@@ -75,6 +75,7 @@ func (d *machineDrainController) Reconcile(ctx context.Context, request reconcil
 		// Error reading the object - requeue the request.
 		return reconcile.Result{}, err
 	}
+	machineCopy := m.DeepCopy()
 
 	existingDrainedCondition := conditions.Get(m, machinev1.MachineDrained)
 	alreadyDrained := existingDrainedCondition != nil && existingDrainedCondition.Status == corev1.ConditionTrue
@@ -110,9 +111,9 @@ func (d *machineDrainController) Reconcile(ctx context.Context, request reconcil
 		}
 
 		conditions.Set(m, drainFinishedCondition)
-		// requeue request in case of failed update
-		if err := d.Client.Status().Update(ctx, m); err != nil {
-			return reconcile.Result{}, fmt.Errorf("could not update machine status: %w", err)
+		// requeue request in case of failed patch
+		if err := d.Client.Status().Patch(ctx, m, client.MergeFrom(machineCopy)); err != nil {
+			return reconcile.Result{}, fmt.Errorf("could not patch machine status: %w", err)
 		}
 		return reconcile.Result{}, nil
 	}

pkg/controller/machineset/status.go

@@ -72,6 +72,7 @@ func (c *ReconcileMachineSet) calculateStatus(ms *machinev1.MachineSet, filtered
 
 // updateMachineSetStatus attempts to update the Status.Replicas of the given MachineSet, with a single GET/PUT retry.
 func updateMachineSetStatus(c client.Client, ms *machinev1.MachineSet, newStatus machinev1.MachineSetStatus) (*machinev1.MachineSet, error) {
+	machineSetCopy := ms.DeepCopy()
 	// This is the steady state. It happens when the MachineSet doesn't have any expectations, since
 	// we do a periodic relist every 30s. If the generations differ but the replicas are
 	// the same, a caller might've resized to the same replica count.
@@ -90,7 +91,7 @@ func updateMachineSetStatus(c client.Client, ms *machinev1.MachineSet, newStatus
 	// same status.
 	newStatus.ObservedGeneration = ms.Generation
 
-	var getErr, updateErr error
+	var getErr, patchErr error
 	for i := 0; ; i++ {
 		var replicas int32
 		if ms.Spec.Replicas != nil {
@@ -105,8 +106,8 @@ func updateMachineSetStatus(c client.Client, ms *machinev1.MachineSet, newStatus
 			fmt.Sprintf("conditions: %v->%v", ms.Status.Conditions, newStatus.Conditions))
 
 		ms.Status = newStatus
-		updateErr = c.Status().Update(context.Background(), ms)
-		if updateErr == nil {
+		patchErr = c.Status().Patch(context.Background(), ms, client.MergeFrom(machineSetCopy))
+		if patchErr == nil {
 			return ms, nil
 		}
 		// Stop retrying if we exceed statusUpdateRetries - the machineSet will be requeued with a rate limit.
@@ -121,7 +122,7 @@ func updateMachineSetStatus(c client.Client, ms *machinev1.MachineSet, newStatus
 		}
 	}
 
-	return nil, updateErr
+	return nil, patchErr
 }
 
 func (c *ReconcileMachineSet) getMachineNode(machine *machinev1.Machine) (*corev1.Node, error) {