From 6c4d94736d88495da30a4241dec034b0fa177d94 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Tue, 11 Jan 2022 15:06:22 -0500 Subject: [PATCH] leaderelection: use 'leases' as default resource lock object Signed-off-by: Joe Lanford --- pkg/leaderelection/leader_election.go | 12 +++++++----- pkg/manager/manager_test.go | 21 +++++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/pkg/leaderelection/leader_election.go b/pkg/leaderelection/leader_election.go index 3dedd462f3..6e74d10d49 100644 --- a/pkg/leaderelection/leader_election.go +++ b/pkg/leaderelection/leader_election.go @@ -27,6 +27,7 @@ import ( corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/leaderelection/resourcelock" + "sigs.k8s.io/controller-runtime/pkg/recorder" ) @@ -39,7 +40,7 @@ type Options struct { LeaderElection bool // LeaderElectionResourceLock determines which resource lock to use for leader election, - // defaults to "configmapsleases". + // defaults to "leases". LeaderElectionResourceLock string // LeaderElectionNamespace determines the namespace in which the leader @@ -57,11 +58,12 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op 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. + // Default resource lock to "leases". The previous default (from v0.7.0 to v0.11.x) was configmapsleases, which was + // used to migrate from configmaps to leases. Since the default was "configmapsleases" for over a year, spanning + // five minor releases, any actively maintained operators are very likely to have a released version that uses + // "configmapsleases". Therefore defaulting to "leases" should be safe. if options.LeaderElectionResourceLock == "" { - options.LeaderElectionResourceLock = resourcelock.ConfigMapsLeasesResourceLock + options.LeaderElectionResourceLock = resourcelock.LeasesResourceLock } // LeaderElectionID must be provided to prevent clashes diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 0dd1cc1f5b..b795bbf655 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -40,6 +40,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/leaderelection/resourcelock" configv1alpha1 "k8s.io/component-base/config/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/cache/informertest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -446,24 +447,20 @@ var _ = Describe("manger.Manager", func() { // 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() { + It("should default to LeasesResourceLock", func() { m, err := New(cfg, Options{LeaderElection: true, 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()) + _, isLeaseLock := cm.resourceLock.(*resourcelock.LeaseLock) + Expect(isLeaseLock).To(BeTrue()) }) It("should use the specified ResourceLock", func() { m, err := New(cfg, Options{ LeaderElection: true, - LeaderElectionResourceLock: resourcelock.LeasesResourceLock, + LeaderElectionResourceLock: resourcelock.ConfigMapsLeasesResourceLock, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns", }) @@ -471,8 +468,12 @@ var _ = Describe("manger.Manager", func() { Expect(err).ToNot(HaveOccurred()) cm, ok := m.(*controllerManager) Expect(ok).To(BeTrue()) - _, isLeaseLock := cm.resourceLock.(*resourcelock.LeaseLock) - Expect(isLeaseLock).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()) }) It("should release lease if ElectionReleaseOnCancel is true", func() { var rl resourcelock.Interface