fix: dedupe expiration reconciliations

kubernetes-sigs · Oct 31, 2024 · 65bd97d · 65bd97d
1 parent f7abd62
commit 65bd97d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/pkg/controllers/nodeclaim/expiration/controller.go b/pkg/controllers/nodeclaim/expiration/controller.go
@@ -22,9 +22,12 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"k8s.io/utils/clock"
 	controllerruntime "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	v1 "sigs.k8s.io/karpenter/pkg/apis/v1"
@@ -46,6 +49,9 @@ func NewController(clk clock.Clock, kubeClient client.Client) *Controller {
 }
 
 func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1.NodeClaim) (reconcile.Result, error) {
+	if !nodeClaim.DeletionTimestamp.IsZero() {
+		return reconcile.Result{}, nil
+	}
 	// From here there are three scenarios to handle:
 	// 1. If ExpireAfter is not configured, exit expiration loop
 	if nodeClaim.Spec.ExpireAfter.Duration == nil {
@@ -72,9 +78,14 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *v1.NodeClaim) (re
 }
 
 func (c *Controller) Register(_ context.Context, m manager.Manager) error {
-	builder := controllerruntime.NewControllerManagedBy(m)
-	return builder.
+	return controllerruntime.NewControllerManagedBy(m).
 		Named("nodeclaim.expiration").
-		For(&v1.NodeClaim{}).
+		For(&v1.NodeClaim{}, builder.WithPredicates(predicate.TypedFuncs[client.Object]{
+			CreateFunc: func(_ event.TypedCreateEvent[client.Object]) bool { return true },
+			DeleteFunc: func(_ event.TypedDeleteEvent[client.Object]) bool { return false },
+			UpdateFunc: func(_ event.TypedUpdateEvent[client.Object]) bool { return false },
+			GenericFunc: func(_ event.TypedGenericEvent[client.Object]) bool { return false },
+
+		})).
 		Complete(reconcile.AsReconciler(m.GetClient(), c))
 }
diff --git a/pkg/controllers/nodeclaim/expiration/suite_test.go b/pkg/controllers/nodeclaim/expiration/suite_test.go
@@ -164,4 +164,22 @@ var _ = Describe("Expiration", func() {
 		result := ExpectObjectReconciled(ctx, env.Client, expirationController, nodeClaim)
 		Expect(result.RequeueAfter).To(BeNumerically("~", time.Second*100, time.Second))
 	})
+	It("shouldn't expire the same NodeClaim multiple times", func() {
+		nodeClaim.ObjectMeta.Finalizers = append(nodeClaim.ObjectMeta.Finalizers, "test-finalizer")
+		ExpectApplied(ctx, env.Client, nodePool, nodeClaim)
+
+		// step forward to make the node expired
+		fakeClock.Step(60 * time.Second)
+		ExpectObjectReconciled(ctx, env.Client, expirationController, nodeClaim)
+		ExpectExists(ctx, env.Client, nodeClaim)
+		ExpectMetricCounterValue(metrics.NodeClaimsDisruptedTotal, 1, map[string]string{
+			metrics.ReasonLabel: metrics.ExpiredReason,
+			"nodepool":          nodePool.Name,
+		})
+		ExpectObjectReconciled(ctx, env.Client, expirationController, nodeClaim)
+		ExpectMetricCounterValue(metrics.NodeClaimsDisruptedTotal, 1, map[string]string{
+			metrics.ReasonLabel: metrics.ExpiredReason,
+			"nodepool":          nodePool.Name,
+		})
+	})
 })