From b7561613a41a7759668636e9a69fa377d7989f84 Mon Sep 17 00:00:00 2001 From: Jonathan Stacks Date: Thu, 5 Jan 2023 17:37:58 -0600 Subject: [PATCH] :sparkles: Add controller option for setting leader election (#2117) * Add controller option for overrding the need for leader election * Update Options to be clearer Addresses PR review comment --- pkg/controller/controller.go | 5 +++ pkg/controller/controller_test.go | 44 +++++++++++++++++++++++++++ pkg/internal/controller/controller.go | 11 +++++++ 3 files changed, 60 insertions(+) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index fe7f94fdc1..8e0a9a91de 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -58,6 +58,10 @@ type Options struct { // RecoverPanic indicates whether the panic caused by reconcile should be recovered. // Defaults to the Controller.RecoverPanic setting from the Manager if unset. RecoverPanic *bool + + // NeedLeaderElection indicates whether the controller needs to use leader election. + // Defaults to true, which means the controller will use leader election. + NeedLeaderElection *bool } // Controller implements a Kubernetes API. A Controller manages a work queue fed reconcile.Requests @@ -156,6 +160,7 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller Name: name, LogConstructor: options.LogConstructor, RecoverPanic: options.RecoverPanic, + LeaderElected: options.NeedLeaderElection, }, nil } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index b5d816bc28..66e11b7d50 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -178,6 +178,50 @@ var _ = Describe("controller.Controller", func() { Expect(ctrl.RecoverPanic).NotTo(BeNil()) Expect(*ctrl.RecoverPanic).To(BeFalse()) }) + + It("should default NeedLeaderElection on the controller to true", func() { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + c, err := controller.New("new-controller", m, controller.Options{ + Reconciler: rec, + }) + Expect(err).NotTo(HaveOccurred()) + + ctrl, ok := c.(*internalcontroller.Controller) + Expect(ok).To(BeTrue()) + + Expect(ctrl.NeedLeaderElection()).To(BeTrue()) + }) + + It("should allow for setting leaderElected to false", func() { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + c, err := controller.New("new-controller", m, controller.Options{ + Reconciler: rec, + NeedLeaderElection: pointer.Bool(false), + }) + Expect(err).NotTo(HaveOccurred()) + + ctrl, ok := c.(*internalcontroller.Controller) + Expect(ok).To(BeTrue()) + + Expect(ctrl.NeedLeaderElection()).To(BeFalse()) + }) + + It("should implement manager.LeaderElectionRunnable", func() { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + c, err := controller.New("new-controller", m, controller.Options{ + Reconciler: rec, + }) + Expect(err).NotTo(HaveOccurred()) + + _, ok := c.(manager.LeaderElectionRunnable) + Expect(ok).To(BeTrue()) + }) }) }) diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go index f7734695ce..1f8f8b7398 100644 --- a/pkg/internal/controller/controller.go +++ b/pkg/internal/controller/controller.go @@ -93,6 +93,9 @@ type Controller struct { // RecoverPanic indicates whether the panic caused by reconcile should be recovered. RecoverPanic *bool + + // LeaderElected indicates whether the controller is leader elected or always running. + LeaderElected *bool } // watchDescription contains all the information necessary to start a watch. @@ -152,6 +155,14 @@ func (c *Controller) Watch(src source.Source, evthdler handler.EventHandler, prc return src.Start(c.ctx, evthdler, c.Queue, prct...) } +// NeedLeaderElection implements the manager.LeaderElectionRunnable interface. +func (c *Controller) NeedLeaderElection() bool { + if c.LeaderElected == nil { + return true + } + return *c.LeaderElected +} + // Start implements controller.Controller. func (c *Controller) Start(ctx context.Context) error { // use an IIFE to get proper lock handling