Repository: Azure/ARO-RP
Pull request #1967 (2022-02-15 14:03:26)

Refactors operator requeues


### What this PR does / why we need it:

* 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.

### Test plan for issue:

Unit tests updated where applicable.

### Is there any documentation that needs to be updated for this PR?

No, just refactoring



Patch:
diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go
index 965370423..579cad709 100644
--- 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
index 879a164fc..2b53dfc83 100644
--- 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
index cd8fef05e..a99c63614 100644
--- 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


Test patch:
diff --git a/pkg/operator/controllers/workaround/workaround_controller_test.go b/pkg/operator/controllers/workaround/workaround_controller_test.go
index bd5c5181b..9bd6e774f 100644
--- a/pkg/operator/controllers/workaround/workaround_controller_test.go
+++ b/pkg/operator/controllers/workaround/workaround_controller_test.go
@@ -68,7 +68,7 @@ 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",
@@ -76,7 +76,7 @@ func TestWorkaroundReconciler(t *testing.T) {
 				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",