diff --git a/pkg/config/controller.go b/pkg/config/controller.go index 8920480319..23c4d8a634 100644 --- a/pkg/config/controller.go +++ b/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 } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 03e51abcc5..a3eba1f5e3 100644 --- a/pkg/controller/controller.go +++ b/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 } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 1e125a0f58..335e6d830e 100644 --- a/pkg/controller/controller_test.go +++ b/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)) + }) }) }) diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go index d0f5be44e6..ea79681862 100644 --- a/pkg/internal/controller/controller.go +++ b/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) } diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 74e5f3f2a2..306e0b0126 100644 --- a/pkg/internal/controller/controller_test.go +++ b/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() {