Merge pull request #1147 from timebertt/configurable-le-resourcelock

✨ Make leader election resourcelock configurable

Author: k8s-ci-robot

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,26 @@ type Options struct {
 	// starting the manager.
 	LeaderElection bool
 
+	// LeaderElectionResourceLock determines which resource lock to use for leader election,
+	// defaults to "configmapsleases". Change this value only if you know what you are doing.
+	// Otherwise, users of your controller might end up with multiple running instances that
+	// each acquired leadership through different resource locks during upgrades and thus
+	// act on the same resources concurrently.
+	// If you want to migrate to the "leases" resource lock, you might do so by migrating to the
+	// respective multilock first ("configmapsleases" or "endpointsleases"), which will acquire a
+	// leader lock on both resources. After all your users have migrated to the multilock, you can
+	// go ahead and migrate to "leases". Please also keep in mind, that users might skip versions
+	// of your controller.
+	//
+	// Note: before controller-runtime version v0.7, the resource lock was set to "configmaps".
+	// Please keep this in mind, when planning a proper migration path for your controller.
+	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 +344,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

@@ -296,6 +296,15 @@ 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())
@@ -320,6 +329,20 @@ var _ = Describe("manger.Manager", func() {
 				Expect(secondaryIsLeaseLock).To(BeTrue())
 
 			})
+			It("should use the specified ResourceLock", func() {
+				m, err := New(cfg, Options{
+					LeaderElection:             true,
+					LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
+					LeaderElectionID:           "controller-runtime",
+					LeaderElectionNamespace:    "my-ns",
+				})
+				Expect(m).ToNot(BeNil())
+				Expect(err).ToNot(HaveOccurred())
+				cm, ok := m.(*controllerManager)
+				Expect(ok).To(BeTrue())
+				_, isLeaseLock := cm.resourceLock.(*resourcelock.LeaseLock)
+				Expect(isLeaseLock).To(BeTrue())
+			})
 		})
 
 		It("should create a listener for the metrics if a valid address is provided", func() {