Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/config/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
35 changes: 35 additions & 0 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})
})
14 changes: 14 additions & 0 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
}
}

Expand All @@ -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)
}

Expand Down
25 changes: 25 additions & 0 deletions pkg/internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down