Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

⚠ leaderelection: use 'leases' as default resource lock object #1773

Merged
merged 1 commit into from Jan 19, 2022
+18 −15
Merged

⚠ leaderelection: use 'leases' as default resource lock object #1773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions pkg/leaderelection/leader_election.go
View file
Open in desktop
Expand Up @@ -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"
)

Expand All @@ -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
Expand All @@ -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
Expand Down
21 changes: 11 additions & 10 deletions pkg/manager/manager_test.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -446,33 +447,33 @@ 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",
})
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())
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
Expand Down