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.

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

✨ Make leader election resourcelock configurable #1147

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions pkg/leaderelection/leader_election.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(),
Expand Down
26 changes: 21 additions & 5 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
timebertt marked this conversation as resolved.
Show resolved Hide resolved

// 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

Expand Down Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,15 @@ var _ = Describe("manger.Manager", func() {
<-m2done
})

It("should return an error if it can't create a ResourceLock", func() {
timebertt marked this conversation as resolved.
Show resolved Hide resolved
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())
Expand All @@ -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() {
Expand Down