Fix emptiness reconciler when ttl not expired (#1262)

* Fix emptiness reconciler when ttl not expired * Changing expectations * Fix variable naming
aws · Feb 2, 2022 · 6d861af · 6d861af
1 parent 7713370
commit 6d861af
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 3 deletions.
diff --git a/pkg/controllers/node/emptiness.go b/pkg/controllers/node/emptiness.go
@@ -78,7 +78,7 @@ func (r *Emptiness) Reconcile(ctx context.Context, provisioner *v1alpha5.Provisi
 			return reconcile.Result{}, fmt.Errorf("deleting node, %w", err)
 		}
 	}
-	return reconcile.Result{}, nil
+	return reconcile.Result{RequeueAfter: emptinessTime.Add(ttl).Sub(injectabletime.Now())}, nil
 }
 
 func (r *Emptiness) isEmpty(ctx context.Context, n *v1.Node) (bool, error) {

diff --git a/pkg/controllers/node/suite_test.go b/pkg/controllers/node/suite_test.go
@@ -34,6 +34,7 @@ import (
 	. "knative.dev/pkg/logging/testing"
 	"knative.dev/pkg/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 var ctx context.Context
@@ -285,6 +286,28 @@ var _ = Describe("Controller", func() {
 			node = ExpectNodeExists(ctx, env.Client, node.Name)
 			Expect(node.DeletionTimestamp.IsZero()).To(BeFalse())
 		})
+		It("should requeue reconcile if node is empty, but not past emptiness TTL", func() {
+			provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30)
+			now := time.Now()
+			injectabletime.Now = func() time.Time { return now } // injectabletime.Now() is called multiple times in function being tested.
+			emptinessTime := injectabletime.Now().Add(-10 * time.Second)
+			// Emptiness timestamps are first formatted to a string friendly (time.RFC3339) (to put it in the node object)
+			// and then eventually parsed back into time.Time when comparing ttls. Repeating that logic in the test.
+			emptinessTimestamp, _ := time.Parse(time.RFC3339, emptinessTime.Format(time.RFC3339))
+			expectedRequeueTime := emptinessTimestamp.Add(time.Duration(30 * time.Second)).Sub(injectabletime.Now()) // we should requeue in ~20 seconds.
+			node := test.Node(test.NodeOptions{ObjectMeta: metav1.ObjectMeta{
+				Finalizers: []string{v1alpha5.TerminationFinalizer},
+				Labels:     map[string]string{v1alpha5.ProvisionerNameLabelKey: provisioner.Name},
+				Annotations: map[string]string{
+					v1alpha5.EmptinessTimestampAnnotationKey: emptinessTime.Format(time.RFC3339),
+				}},
+			})
+			ExpectCreated(ctx, env.Client, provisioner, node)
+			result := ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
+			Expect(result).To(Equal(reconcile.Result{Requeue: true, RequeueAfter: expectedRequeueTime}))
+			node = ExpectNodeExists(ctx, env.Client, node.Name)
+			Expect(node.DeletionTimestamp.IsZero()).To(BeTrue())
+		})
 	})
 	Context("Finalizer", func() {
 		It("should add the termination finalizer if missing", func() {

diff --git a/pkg/test/expectations/expectations.go b/pkg/test/expectations/expectations.go
@@ -186,7 +186,8 @@ func ExpectProvisioned(ctx context.Context, c client.Client, selectionController
 	return result
 }
 
-func ExpectReconcileSucceeded(ctx context.Context, reconciler reconcile.Reconciler, key client.ObjectKey) {
-	_, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: key})
+func ExpectReconcileSucceeded(ctx context.Context, reconciler reconcile.Reconciler, key client.ObjectKey) reconcile.Result {
+	result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: key})
 	Expect(err).ToNot(HaveOccurred())
+	return result
 }