From 3ba83638cf3101bc4b25e07c011e8bd867792e80 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Tue, 15 Feb 2022 14:00:17 +0000 Subject: [PATCH] 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. --- pkg/operator/controllers/checker/checker_controller.go | 6 +++++- pkg/operator/controllers/node/node_controller.go | 4 +--- .../controllers/workaround/workaround_controller.go | 2 +- .../controllers/workaround/workaround_controller_test.go | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/operator/controllers/checker/checker_controller.go b/pkg/operator/controllers/checker/checker_controller.go index 965370423ff..579cad709ea 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 879a164fc39..2b53dfc83af 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 cd8fef05eb1..a99c6361427 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 diff --git a/pkg/operator/controllers/workaround/workaround_controller_test.go b/pkg/operator/controllers/workaround/workaround_controller_test.go index bd5c5181b3c..9bd6e774f1f 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",