Refactors operator requeues

* Adds the clarifying comment on requeues into the checker controller * Removes `Requeue: true` in places where we use `RequeueAfter` as it is has no effect.
Azure · Feb 15, 2022 · 3ba8363 · 3ba8363
1 parent 7ffb8d3
commit 3ba8363
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 7 deletions.
diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go
@@ -83,7 +83,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 		}
 	}
 
-	return reconcile.Result{RequeueAfter: time.Hour, Requeue: true}, err
+	// We always requeue here:
+	// * Either immediately (with rate limiting) based on the error
+	//   when err != nil.
+	// * Or based on RequeueAfter when err == nil.
+	return reconcile.Result{RequeueAfter: time.Hour}, err
 }
 
 // SetupWithManager setup our manager

diff --git a/pkg/operator/controllers/node/node_controller.go b/pkg/operator/controllers/node/node_controller.go
@@ -101,9 +101,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	deadline := t.Add(gracePeriod)
 	now := time.Now()
 	if deadline.After(now) {
-		return reconcile.Result{
-			RequeueAfter: deadline.Sub(now),
-		}, err
+		return reconcile.Result{RequeueAfter: deadline.Sub(now)}, nil
 	}
 
 	// drain the node disabling eviction

diff --git a/pkg/operator/controllers/workaround/workaround_controller.go b/pkg/operator/controllers/workaround/workaround_controller.go
@@ -87,7 +87,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 			return reconcile.Result{}, err
 		}
 	}
-	return reconcile.Result{RequeueAfter: time.Hour, Requeue: true}, nil
+	return reconcile.Result{RequeueAfter: time.Hour}, nil
 }
 
 // SetupWithManager setup our manager

diff --git a/pkg/operator/controllers/workaround/workaround_controller_test.go b/pkg/operator/controllers/workaround/workaround_controller_test.go
@@ -68,15 +68,15 @@ func TestWorkaroundReconciler(t *testing.T) {
 				c := mw.EXPECT().IsRequired(gomock.Any()).Return(true)
 				mw.EXPECT().Ensure(gomock.Any()).After(c).Return(nil)
 			},
-			want: ctrl.Result{Requeue: true, RequeueAfter: time.Hour},
+			want: ctrl.Result{RequeueAfter: time.Hour},
 		},
 		{
 			name: "is not required",
 			mocker: func(mw *mock_workaround.MockWorkaround) {
 				c := mw.EXPECT().IsRequired(gomock.Any()).Return(false)
 				mw.EXPECT().Remove(gomock.Any()).After(c).Return(nil)
 			},
-			want: ctrl.Result{Requeue: true, RequeueAfter: time.Hour},
+			want: ctrl.Result{RequeueAfter: time.Hour},
 		},
 		{
 			name: "has error",