Separate nm status initalization from update

controllers/nodemaintenance_controller.go

@@ -148,11 +148,17 @@ func (r *NodeMaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return emptyResult, nil
 	}
 
-	err = initMaintenanceStatus(ctx, nm, drainer, r.Client)
+	needUpdate, err := initMaintenanceStatus(ctx, nm, drainer)
 	if err != nil {
-		r.logger.Error(err, "Failed to update NodeMaintenance with \"Running\" status")
+		r.logger.Error(err, "Failed to initalize NodeMaintenance status")
 		return r.onReconcileError(ctx, nm, drainer, err)
 	}
+	if needUpdate {
+		if err = r.Client.Status().Update(ctx, nm); err != nil {
+			r.logger.Error(err, "Failed to update NodeMaintenance with \"Running\" status")
+			return r.onReconcileError(ctx, nm, drainer, err)
+		}
+	}
 
 	nodeName := nm.Spec.NodeName
 
@@ -390,13 +396,15 @@ func (r *NodeMaintenanceReconciler) fetchNode(ctx context.Context, drainer *drai
 	return node, nil
 }
 
-func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, drainer *drain.Helper, r client.Client) error {
+// initMaintenanceStatus initializes the nm CR status only at the first time, when phase is empty,
+// It returns a bool whether we should update the CR status and an error if it can't be initialized
+func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, drainer *drain.Helper) (bool, error) {
 	if nm.Status.Phase == "" {
 		nm.Status.Phase = v1beta1.MaintenanceRunning
 		setLastUpdate(nm)
 		pendingList, errlist := drainer.GetPodsForDeletion(nm.Spec.NodeName)
 		if errlist != nil {
-			return fmt.Errorf("failed to get pods for eviction while initializing status")
+			return false, fmt.Errorf("failed to get pods for eviction while initializing status")
 		}
 		if pendingList != nil {
 			nm.Status.PendingPods = GetPodNameList(pendingList.Pods())
@@ -409,13 +417,12 @@ func initMaintenanceStatus(ctx context.Context, nm *v1beta1.NodeMaintenance, dra
 				FieldSelector: fields.SelectorFromSet(fields.Set{"spec.nodeName": nm.Spec.NodeName}).String(),
 			})
 		if err != nil {
-			return err
+			return false, err
 		}
 		nm.Status.TotalPods = len(podlist.Items)
-		err = r.Status().Update(ctx, nm)
-		return err
+		return true, nil
 	}
-	return nil
+	return false, nil
 }
 
 func (r *NodeMaintenanceReconciler) onReconcileErrorWithRequeue(ctx context.Context, nm *v1beta1.NodeMaintenance, drainer *drain.Helper, err error, duration *time.Duration) (ctrl.Result, error) {

controllers/nodemaintenance_controller_test.go

@@ -66,8 +66,9 @@ var _ = Describe("Node Maintenance", func() {
 			})
 			When("Status was initalized", func() {
 				It("should be set for running with 2 pods to drain", func() {
-					Expect(initMaintenanceStatus(ctx, nm, drainer, r.Client)).To(HaveOccurred())
-					// status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here
+					toUpdate, err := initMaintenanceStatus(ctx, nm, drainer)
+					Expect(toUpdate).To(BeTrue())
+					Expect(err).NotTo(HaveOccurred())
 					Expect(nm.Status.Phase).To(Equal(v1beta1.MaintenanceRunning))
 					Expect(len(nm.Status.PendingPods)).To(Equal(2))
 					Expect(nm.Status.EvictionPods).To(Equal(2))
@@ -78,8 +79,9 @@ var _ = Describe("Node Maintenance", func() {
 			})
 			When("Owner ref was set", func() {
 				It("should be set properly", func() {
-					Expect(initMaintenanceStatus(ctx, nm, drainer, r.Client)).To(HaveOccurred())
-					// status was initialized but the function will fail on updating the CR status, since we don't create a nm CR here
+					toUpdate, err := initMaintenanceStatus(ctx, nm, drainer)
+					Expect(toUpdate).To(BeTrue())
+					Expect(err).NotTo(HaveOccurred())
 					By("Setting owner ref for a modified nm CR")
 					node := &corev1.Node{}
 					Expect(k8sClient.Get(ctx, client.ObjectKey{Name: taintedNodeName}, node)).To(Succeed())
@@ -101,8 +103,9 @@ var _ = Describe("Node Maintenance", func() {
 				It("Should not modify the CR after initalization", func() {
 					nmCopy := nm.DeepCopy()
 					nmCopy.Status.Phase = v1beta1.MaintenanceFailed
-					Expect(initMaintenanceStatus(ctx, nmCopy, drainer, r.Client)).To(Succeed())
-					// status was not initialized thus the function succeeds
+					toUpdate, err := initMaintenanceStatus(ctx, nmCopy, drainer)
+					Expect(toUpdate).To(BeFalse())
+					Expect(err).NotTo(HaveOccurred())
 					Expect(nmCopy.Status.Phase).To(Equal(v1beta1.MaintenanceFailed))
 					Expect(len(nmCopy.Status.PendingPods)).To(Equal(0))
 					Expect(nmCopy.Status.EvictionPods).To(Equal(0))