Merge pull request #3296 from alvaroaleman/reconciliationtimeout

✨ Add a ReconciliationTimeout option

Author: k8s-ci-robot

pkg/config/controller.go

@@ -81,4 +81,8 @@ type Controller struct {
 
 	// Logger is the logger controllers should use.
 	Logger logr.Logger
+
+	// ReconciliationTimeout is used as the timeout passed to the context of each Reconcile call.
+	// By default, there is no timeout.
+	ReconciliationTimeout time.Duration
 }

pkg/controller/controller.go

@@ -106,6 +106,10 @@ type TypedOptions[request comparable] struct {
 	// leader election do not wait on leader election to start their sources.
 	// Defaults to false.
 	EnableWarmup *bool
+
+	// ReconciliationTimeout is used as the timeout passed to the context of each Reconcile call.
+	// By default, there is no timeout.
+	ReconciliationTimeout time.Duration
 }
 
 // DefaultFromConfig defaults the config from a config.Controller
@@ -141,6 +145,10 @@ func (options *TypedOptions[request]) DefaultFromConfig(config config.Controller
 	if options.EnableWarmup == nil {
 		options.EnableWarmup = config.EnableWarmup
 	}
+
+	if options.ReconciliationTimeout == 0 {
+		options.ReconciliationTimeout = config.ReconciliationTimeout
+	}
 }
 
 // Controller implements an API. A Controller manages a work queue fed reconcile.Requests
@@ -271,6 +279,7 @@ func NewTypedUnmanaged[request comparable](name string, options TypedOptions[req
 		RecoverPanic:            options.RecoverPanic,
 		LeaderElected:           options.NeedLeaderElection,
 		EnableWarmup:            options.EnableWarmup,
+		ReconciliationTimeout:   options.ReconciliationTimeout,
 	}), nil
 }
 

pkg/controller/controller_test.go

@@ -540,5 +540,40 @@ var _ = Describe("controller.Controller", func() {
 			Expect(ok).To(BeTrue())
 			Expect(internalCtrlOverridingWarmup.EnableWarmup).To(HaveValue(BeFalse()))
 		})
+
+		It("should default ReconciliationTimeout from manager if unset", func() {
+			m, err := manager.New(cfg, manager.Options{
+				Controller: config.Controller{ReconciliationTimeout: 30 * time.Second},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			c, err := controller.New("mgr-reconciliation-timeout", m, controller.Options{
+				Reconciler: rec,
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			ctrl, ok := c.(*internalcontroller.Controller[reconcile.Request])
+			Expect(ok).To(BeTrue())
+
+			Expect(ctrl.ReconciliationTimeout).To(Equal(30 * time.Second))
+		})
+
+		It("should not override an existing ReconciliationTimeout", func() {
+			m, err := manager.New(cfg, manager.Options{
+				Controller: config.Controller{ReconciliationTimeout: 30 * time.Second},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			c, err := controller.New("ctrl-reconciliation-timeout", m, controller.Options{
+				Reconciler:            rec,
+				ReconciliationTimeout: time.Minute,
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			ctrl, ok := c.(*internalcontroller.Controller[reconcile.Request])
+			Expect(ok).To(BeTrue())
+
+			Expect(ctrl.ReconciliationTimeout).To(Equal(time.Minute))
+		})
 	})
 })

pkg/internal/controller/controller.go

@@ -82,6 +82,10 @@ type Options[request comparable] struct {
 	// Defaults to false, which means that the controller will wait for leader election to start
 	// before starting sources.
 	EnableWarmup *bool
+
+	// ReconciliationTimeout is used as the timeout passed to the context of each Reconcile call.
+	// By default, there is no timeout.
+	ReconciliationTimeout time.Duration
 }
 
 // Controller implements controller.Controller.
@@ -162,6 +166,8 @@ type Controller[request comparable] struct {
 	// leader election do not wait on leader election to start their sources.
 	// Defaults to false.
 	EnableWarmup *bool
+
+	ReconciliationTimeout time.Duration
 }
 
 // New returns a new Controller configured with the given options.
@@ -177,6 +183,7 @@ func New[request comparable](options Options[request]) *Controller[request] {
 		RecoverPanic:            options.RecoverPanic,
 		LeaderElected:           options.LeaderElected,
 		EnableWarmup:            options.EnableWarmup,
+		ReconciliationTimeout:   options.ReconciliationTimeout,
 	}
 }
 
@@ -199,6 +206,13 @@ func (c *Controller[request]) Reconcile(ctx context.Context, req request) (_ rec
 			panic(r)
 		}
 	}()
+
+	if c.ReconciliationTimeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, c.ReconciliationTimeout)
+		defer cancel()
+	}
+
 	return c.Do.Reconcile(ctx, req)
 }
 

pkg/internal/controller/controller_test.go

@@ -142,6 +142,31 @@ var _ = Describe("controller", func() {
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("[recovered]"))
 		})
+
+		It("should time out if ReconciliationTimeout is set", func(ctx SpecContext) {
+			ctrl.ReconciliationTimeout = time.Duration(1) // One nanosecond
+			ctrl.Do = reconcile.Func(func(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {
+				<-ctx.Done()
+				return reconcile.Result{}, ctx.Err()
+			})
+			_, err := ctrl.Reconcile(ctx,
+				reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"}})
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(context.DeadlineExceeded))
+		})
+
+		It("should not configure a timeout if ReconciliationTimeout is zero", func(ctx SpecContext) {
+			ctrl.Do = reconcile.Func(func(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {
+				defer GinkgoRecover()
+
+				_, ok := ctx.Deadline()
+				Expect(ok).To(BeFalse())
+				return reconcile.Result{}, nil
+			})
+			_, err := ctrl.Reconcile(ctx,
+				reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"}})
+			Expect(err).NotTo(HaveOccurred())
+		})
 	})
 
 	Describe("Start", func() {