✨ Make leader election resourcelock configurable

Author: timebertt

pkg/leaderelection/leader_election.go

@@ -37,22 +37,32 @@ type Options struct {
 	// starting the manager.
 	LeaderElection bool
 
+	// LeaderElectionResourceLock determines which resource lock to use for leader election,
+	// defaults to "configmapsleases".
+	LeaderElectionResourceLock string
+
 	// LeaderElectionNamespace determines the namespace in which the leader
-	// election configmap will be created.
+	// election resource will be created.
 	LeaderElectionNamespace string
 
-	// LeaderElectionID determines the name of the configmap that leader election
+	// LeaderElectionID determines the name of the resource that leader election
 	// will use for holding the leader lock.
 	LeaderElectionID string
 }
 
-// NewResourceLock creates a new config map resource lock for use in a leader
-// election loop
+// NewResourceLock creates a new resource lock for use in a leader election loop.
 func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, options Options) (resourcelock.Interface, error) {
 	if !options.LeaderElection {
 		return nil, nil
 	}
 
+	// Default resource lock to "configmapsleases". We must keep this default until we are sure all controller-runtime
+	// users have upgraded from the original default ConfigMap lock to a controller-runtime version that has this new
+	// default. Many users of controller-runtime skip versions, so we should be extremely conservative here.
+	if options.LeaderElectionResourceLock == "" {
+		options.LeaderElectionResourceLock = resourcelock.ConfigMapsLeasesResourceLock
+	}
+
 	// LeaderElectionID must be provided to prevent clashes
 	if options.LeaderElectionID == "" {
 		return nil, errors.New("LeaderElectionID must be configured")
@@ -80,8 +90,7 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op
 		return nil, err
 	}
 
-	// TODO(JoelSpeed): switch to leaderelection object in 1.12
-	return resourcelock.New(resourcelock.ConfigMapsLeasesResourceLock,
+	return resourcelock.New(options.LeaderElectionResourceLock,
 		options.LeaderElectionNamespace,
 		options.LeaderElectionID,
 		client.CoreV1(),

pkg/manager/manager.go

@@ -142,11 +142,15 @@ type Options struct {
 	// starting the manager.
 	LeaderElection bool
 
+	// LeaderElectionResourceLock determines which resource lock to use for leader election,
+	// defaults to "configmapsleases".
+	LeaderElectionResourceLock string
+
 	// LeaderElectionNamespace determines the namespace in which the leader
-	// election configmap will be created.
+	// election resource will be created.
 	LeaderElectionNamespace string
 
-	// LeaderElectionID determines the name of the configmap that leader election
+	// LeaderElectionID determines the name of the resource that leader election
 	// will use for holding the leader lock.
 	LeaderElectionID string
 
@@ -329,9 +333,10 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		leaderConfig = options.LeaderElectionConfig
 	}
 	resourceLock, err := options.newResourceLock(leaderConfig, recorderProvider, leaderelection.Options{
-		LeaderElection:          options.LeaderElection,
-		LeaderElectionID:        options.LeaderElectionID,
-		LeaderElectionNamespace: options.LeaderElectionNamespace,
+		LeaderElection:             options.LeaderElection,
+		LeaderElectionResourceLock: options.LeaderElectionResourceLock,
+		LeaderElectionID:           options.LeaderElectionID,
+		LeaderElectionNamespace:    options.LeaderElectionNamespace,
 	})
 	if err != nil {
 		return nil, err

pkg/manager/manager_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
 	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/goleak"
@@ -296,30 +297,48 @@ var _ = Describe("manger.Manager", func() {
 				<-m2done
 			})
 
+			It("should return an error if it can't create a ResourceLock", func() {
+				m, err := New(cfg, Options{
+					newResourceLock: func(_ *rest.Config, _ recorder.Provider, _ leaderelection.Options) (resourcelock.Interface, error) {
+						return nil, fmt.Errorf("expected error")
+					},
+				})
+				Expect(m).To(BeNil())
+				Expect(err).To(MatchError(ContainSubstring("expected error")))
+			})
 			It("should return an error if namespace not set and not running in cluster", func() {
 				m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime"})
 				Expect(m).To(BeNil())
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace: not running in-cluster, please specify LeaderElectionNamespace"))
 			})
 
-			// We must keep this default until we are sure all controller-runtime users have upgraded from the original default
-			// ConfigMap lock to a controller-runtime version that has this new default. Many users of controller-runtime skip
-			// versions, so we should be extremely conservative here.
-			It("should default to ConfigMapsLeasesResourceLock", func() {
-				m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns"})
+			DescribeTable("should create the correct ResourceLock", func(resourceLock string, expectMultiLock bool, expectedLocks ...resourcelock.Interface) {
+				m, err := New(cfg, Options{LeaderElection: true, LeaderElectionResourceLock: resourceLock, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns"})
 				Expect(m).ToNot(BeNil())
 				Expect(err).ToNot(HaveOccurred())
 				cm, ok := m.(*controllerManager)
 				Expect(ok).To(BeTrue())
-				multilock, isMultiLock := cm.resourceLock.(*resourcelock.MultiLock)
-				Expect(isMultiLock).To(BeTrue())
-				_, primaryIsConfigMapLock := multilock.Primary.(*resourcelock.ConfigMapLock)
-				Expect(primaryIsConfigMapLock).To(BeTrue())
-				_, secondaryIsLeaseLock := multilock.Secondary.(*resourcelock.LeaseLock)
-				Expect(secondaryIsLeaseLock).To(BeTrue())
 
-			})
+				if expectMultiLock {
+					Expect(cm.resourceLock).To(BeAssignableToTypeOf(&resourcelock.MultiLock{}))
+					multiLock := cm.resourceLock.(*resourcelock.MultiLock)
+					Expect(multiLock.Primary).To(BeAssignableToTypeOf(expectedLocks[0]))
+					Expect(multiLock.Secondary).To(BeAssignableToTypeOf(expectedLocks[1]))
+				} else {
+					Expect(cm.resourceLock).To(BeAssignableToTypeOf(expectedLocks[0]))
+				}
+			},
+				// We must keep this default until we are sure all controller-runtime users have upgraded from the original default
+				// ConfigMap lock to a controller-runtime version that has this new default. Many users of controller-runtime skip
+				// versions, so we should be extremely conservative here.
+				Entry("not specified (should default to ConfigMapsLeasesResourceLock)", "", true, &resourcelock.ConfigMapLock{}, &resourcelock.LeaseLock{}),
+				Entry("EndpointsResourceLock", resourcelock.EndpointsResourceLock, false, &resourcelock.EndpointsLock{}),
+				Entry("ConfigMapsResourceLock", resourcelock.ConfigMapsResourceLock, false, &resourcelock.ConfigMapLock{}),
+				Entry("LeasesResourceLock", resourcelock.LeasesResourceLock, false, &resourcelock.LeaseLock{}),
+				Entry("EndpointsLeasesResourceLock", resourcelock.EndpointsLeasesResourceLock, true, &resourcelock.EndpointsLock{}, &resourcelock.LeaseLock{}),
+				Entry("ConfigMapsLeasesResourceLock", resourcelock.ConfigMapsLeasesResourceLock, true, &resourcelock.ConfigMapLock{}, &resourcelock.LeaseLock{}),
+			)
 		})
 
 		It("should create a listener for the metrics if a valid address is provided", func() {