From e28a03931446725d4993c34c2655bb3f90565422 Mon Sep 17 00:00:00 2001
From: Christopher Broglie <cbroglie@cloudflare.com>
Date: Tue, 21 Jun 2022 17:05:28 -0700
Subject: [PATCH] Use RequeueAfter to poll instead of SyncPeriod

When polling for changes to resources which can't be watched (i.e.
current alerts in alertmanager), controller-runtime recommends setting
RequeueAfter individually on each result over reducing the global
SyncPeriod. This reduces load on the API server, and avoids
re-reconciling any resources which have already been reconciled within
the polling interval for another reason, such as a watch event.
---
 cmd/sciuro/main.go               | 2 +-
 internal/node/reconciler.go      | 7 +++++--
 internal/node/reconciler_test.go | 7 ++++---
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/cmd/sciuro/main.go b/cmd/sciuro/main.go
index 876fb5d..3d247d5 100644
--- a/cmd/sciuro/main.go
+++ b/cmd/sciuro/main.go
@@ -74,7 +74,6 @@ func main() {
 	entryLog := log.WithName("entrypoint")
 
 	mgr, err := manager.New(clientconfig.GetConfigOrDie(), manager.Options{
-		SyncPeriod:              &cfg.NodeResync,
 		LeaderElection:          true,
 		LeaderElectionID:        cfg.LeaderElectionID,
 		LeaderElectionNamespace: cfg.LeaderElectionNamespace,
@@ -118,6 +117,7 @@ func main() {
 			mgr.GetClient(),
 			log.WithName("reconciler"),
 			metrics.Registry,
+			cfg.NodeResync,
 			cfg.ReconcileTimeout,
 			cfg.LingerResolvedDuration,
 			as,
diff --git a/internal/node/reconciler.go b/internal/node/reconciler.go
index e0f38dd..33fb4f0 100644
--- a/internal/node/reconciler.go
+++ b/internal/node/reconciler.go
@@ -37,6 +37,7 @@ const (
 type nodeStatusReconciler struct {
 	c                   client.Client
 	log                 logr.Logger
+	resyncInterval      time.Duration
 	reconcileTimeout    time.Duration
 	linger              time.Duration
 	alertCache          alert.Cache
@@ -71,6 +72,7 @@ func NewNodeStatusReconciler(
 	c client.Client,
 	log logr.Logger,
 	prom prometheus.Registerer,
+	resyncInterval,
 	reconcileTimeout,
 	linger time.Duration,
 	ac alert.Cache,
@@ -87,6 +89,7 @@ func NewNodeStatusReconciler(
 	return &nodeStatusReconciler{
 		c:                   c,
 		log:                 log,
+		resyncInterval:      resyncInterval,
 		reconcileTimeout:    reconcileTimeout,
 		linger:              linger,
 		alertCache:          ac,
@@ -115,14 +118,14 @@ func (n *nodeStatusReconciler) Reconcile(ctx context.Context, request reconcile.
 		return reconcile.Result{}, err
 	}
 	if equality.Semantic.DeepEqual(desiredNode, currentNode) {
-		return reconcile.Result{}, nil
+		return reconcile.Result{RequeueAfter: n.resyncInterval}, nil
 	}
 	patch := client.MergeFrom(currentNode)
 	if err := n.c.Status().Patch(ctx, desiredNode, patch); err != nil {
 		log.Error(err, "could not patch node")
 		return reconcile.Result{}, err
 	}
-	return reconcile.Result{}, nil
+	return reconcile.Result{RequeueAfter: n.resyncInterval}, nil
 }
 
 func (n *nodeStatusReconciler) updateNodeStatuses(log logr.Logger, node *corev1.Node) error {
diff --git a/internal/node/reconciler_test.go b/internal/node/reconciler_test.go
index 393e643..7893d29 100644
--- a/internal/node/reconciler_test.go
+++ b/internal/node/reconciler_test.go
@@ -30,6 +30,7 @@ var (
 )
 
 func Test_Reconcile(t *testing.T) {
+	const resyncInterval = 2 * time.Minute
 	tests := []struct {
 		name        string
 		updateMocks func(c *mockK8SClient, cache *mockAlertCache)
@@ -85,7 +86,7 @@ func Test_Reconcile(t *testing.T) {
 				}
 				c.On("Patch", mock.Anything, mock.MatchedBy(match), mock.Anything, mock.Anything).Return(nil)
 			},
-			want:    reconcile.Result{},
+			want:    reconcile.Result{RequeueAfter: resyncInterval},
 			wantErr: false,
 		},
 		{
@@ -129,7 +130,7 @@ func Test_Reconcile(t *testing.T) {
 					},
 				).Return(nil)
 			},
-			want:    reconcile.Result{},
+			want:    reconcile.Result{RequeueAfter: resyncInterval},
 			wantErr: false,
 		},
 	}
@@ -141,7 +142,7 @@ func Test_Reconcile(t *testing.T) {
 			c := &mockK8SClient{}
 			ac := &mockAlertCache{}
 			tt.updateMocks(c, ac)
-			n := NewNodeStatusReconciler(c, logr.Discard(), prometheus.NewRegistry(), time.Minute, time.Minute, ac)
+			n := NewNodeStatusReconciler(c, logr.Discard(), prometheus.NewRegistry(), resyncInterval, time.Minute, time.Minute, ac)
 			got, err := n.Reconcile(context.Background(), request)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Reconcile() error = %v, wantErr %v", err, tt.wantErr)