From 687d7fc8ef4fa76256188c9046afe8bbf30acbf8 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 25 May 2019 19:41:58 +0200 Subject: [PATCH] :warning: Remove defaulting for leader election ID With the current defaulting for the leader election ID, there is a clash as soon as two controller that do not have it explicitly configured run in the same namespace or have the same namespace configured for leader election. This is especially bad since there is no logging about the lock being held by a different controller, so from a users perspective this looks like the controller just froze. --- pkg/leaderelection/leader_election.go | 5 +++-- pkg/manager/manager_test.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/leaderelection/leader_election.go b/pkg/leaderelection/leader_election.go index 9e13fd7968..d6f56d55b8 100644 --- a/pkg/leaderelection/leader_election.go +++ b/pkg/leaderelection/leader_election.go @@ -17,6 +17,7 @@ limitations under the License. package leaderelection import ( + "errors" "fmt" "io/ioutil" "os" @@ -52,9 +53,9 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op return nil, nil } - // Default the LeaderElectionID + // LeaderElectionID must be provided to prevent clashes if options.LeaderElectionID == "" { - options.LeaderElectionID = "controller-leader-election-helper" + return nil, errors.New("LeaderElectionID must be configured") } // Default the namespace (if running in cluster) diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 4141e56593..0dc3d5692e 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -143,15 +143,16 @@ var _ = Describe("manger.Manager", func() { m, err := New(cfg, Options{ LeaderElection: true, LeaderElectionNamespace: "default", + LeaderElectionID: "test-leader-election-id", newResourceLock: func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) { var err error rl, err = leaderelection.NewResourceLock(config, recorderProvider, options) return rl, err }, }) - Expect(m).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) - Expect(rl.Describe()).To(Equal("default/controller-leader-election-helper")) + Expect(m).ToNot(BeNil()) + Expect(rl.Describe()).To(Equal("default/test-leader-election-id")) }) It("should return an error if namespace not set and not running in cluster", func() {