Skip to content

Commit

Permalink
⚠️ Remove defaulting for leader election ID
Browse files Browse the repository at this point in the history
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.

Fixes #445
  • Loading branch information
alvaroaleman committed May 25, 2019
1 parent 13ee2bc commit d25b1ee
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
5 changes: 3 additions & 2 deletions pkg/leaderelection/leader_election.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package leaderelection

import (
"errors"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit d25b1ee

Please sign in to comment.