Skip to content

Commit

Permalink
⚠️ Rename leader election config fields
Browse files Browse the repository at this point in the history 
Change the names of the leader election config fields to be more
explanatory and reflective of what they actually do:

- `LeaderElectionNamespace` -> `LeaderElectionLockNamespace` since
  it actually is the namespace for the lock.

- `LeaderElectionID` -> `LeaderElectionLockName` since it actually
  is the resource name of the lock.

- Also updated `pkg/leaderelection` to remove stuttering in the field
  names since the type is already `leaderelection.Options` and no
  need to repeat the `LeaderElection` prefix.
  • Loading branch information
@ahmetb
ahmetb committed Jan 5, 2024
1 parent 91f642b commit 4b954bd
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 81 deletions.
34 changes: 17 additions & 17 deletions pkg/leaderelection/leader_election.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,17 @@ type Options struct {
// starting the manager.
LeaderElection bool

// LeaderElectionResourceLock determines which resource lock to use for leader election,
// ResourceLock determines which resource lock to use for leader election,
// defaults to "leases".
LeaderElectionResourceLock string
ResourceLock string

// LeaderElectionNamespace determines the namespace in which the leader
// LockNamespace determines the namespace in which the leader
// election resource will be created.
LeaderElectionNamespace string
LockNamespace string

// LeaderElectionID determines the name of the resource that leader election
// LockName determines the name of the resource that leader election
// will use for holding the leader lock.
LeaderElectionID string
LockName string
}

// NewResourceLock creates a new resource lock for use in a leader election loop.
Expand All @@ -61,19 +61,19 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op
// 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.LeasesResourceLock
if options.ResourceLock == "" {
options.ResourceLock = resourcelock.LeasesResourceLock
}

// LeaderElectionID must be provided to prevent clashes
if options.LeaderElectionID == "" {
return nil, errors.New("LeaderElectionID must be configured")
// LockName must be provided to prevent clashes
if options.LockName == "" {
return nil, errors.New("LockName must be configured")
}

// Default the namespace (if running in cluster)
if options.LeaderElectionNamespace == "" {
if options.LockNamespace == "" {
var err error
options.LeaderElectionNamespace, err = getInClusterNamespace()
options.LockNamespace, err = getInClusterNamespace()
if err != nil {
return nil, fmt.Errorf("unable to find leader election namespace: %w", err)
}
Expand All @@ -98,9 +98,9 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op
return nil, err
}

return resourcelock.New(options.LeaderElectionResourceLock,
options.LeaderElectionNamespace,
options.LeaderElectionID,
return resourcelock.New(options.ResourceLock,
options.LockNamespace,
options.LockName,
corev1Client,
coordinationClient,
resourcelock.ResourceLockConfig{
Expand All @@ -113,7 +113,7 @@ func getInClusterNamespace() (string, error) {
// Check whether the namespace file exists.
// If not, we are not running in cluster so can't guess the namespace.
if _, err := os.Stat(inClusterNamespacePath); os.IsNotExist(err) {
return "", fmt.Errorf("not running in-cluster, please specify LeaderElectionNamespace")
return "", fmt.Errorf("not running in-cluster, please specify LeaderElectionLockNamespace")
} else if err != nil {
return "", fmt.Errorf("error checking namespace file: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/manager/internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ type controllerManager struct {
// webhookServer if unset, and Add() it to controllerManager.
webhookServerOnce sync.Once

// leaderElectionID is the name of the resource that leader election
// leaderElectionLockName is the name of the resource that leader election
// will use for holding the leader lock.
leaderElectionID string
leaderElectionLockName string
// leaseDuration is the duration that non-leader candidates will
// wait to force acquire leadership.
leaseDuration time.Duration
Expand Down Expand Up @@ -585,7 +585,7 @@ func (cm *controllerManager) startLeaderElection(ctx context.Context) (err error
},
},
ReleaseOnCancel: cm.leaderElectionReleaseOnCancel,
Name: cm.leaderElectionID,
Name: cm.leaderElectionLockName,
})
if err != nil {
return err
Expand Down
30 changes: 15 additions & 15 deletions pkg/manager/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,13 @@ type Options struct {
// act on the same resources concurrently.
LeaderElectionResourceLock string

// LeaderElectionNamespace determines the namespace in which the leader
// election resource will be created.
LeaderElectionNamespace string
// LeaderElectionLockNamespace determines the namespace of the leader election lock
// resource will be created in.
LeaderElectionLockNamespace string

// LeaderElectionID determines the name of the resource that leader election
// LeaderElectionLockName determines the name of the resource that leader election
// will use for holding the leader lock.
LeaderElectionID string
LeaderElectionLockName string

// LeaderElectionConfig can be specified to override the default configuration
// that is used to build the leader election client.
Expand All @@ -196,7 +196,7 @@ type Options struct {
LeaderElectionReleaseOnCancel bool

// LeaderElectionResourceLockInterface allows to provide a custom resourcelock.Interface that was created outside
// of the controller-runtime. If this value is set the options LeaderElectionID, LeaderElectionNamespace,
// of the controller-runtime. If this value is set the options LeaderElectionLockName, LeaderElectionLockNamespace,
// LeaderElectionResourceLock, LeaseDuration, RenewDeadline and RetryPeriod will be ignored. This can be useful if you
// want to use a locking mechanism that is currently not supported, like a MultiLock across two Kubernetes clusters.
LeaderElectionResourceLockInterface resourcelock.Interface
Expand Down Expand Up @@ -379,10 +379,10 @@ func New(config *rest.Config, options Options) (Manager, error) {
resourceLock = options.LeaderElectionResourceLockInterface
} else {
resourceLock, err = options.newResourceLock(leaderConfig, leaderRecorderProvider, leaderelection.Options{
LeaderElection: options.LeaderElection,
LeaderElectionResourceLock: options.LeaderElectionResourceLock,
LeaderElectionID: options.LeaderElectionID,
LeaderElectionNamespace: options.LeaderElectionNamespace,
LeaderElection: options.LeaderElection,
ResourceLock: options.LeaderElectionResourceLock,
LockName: options.LeaderElectionLockName,
LockNamespace: options.LeaderElectionLockNamespace,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -423,7 +423,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
logger: options.Logger,
elected: make(chan struct{}),
webhookServer: options.WebhookServer,
leaderElectionID: options.LeaderElectionID,
leaderElectionLockName: options.LeaderElectionLockName,
leaseDuration: *options.LeaseDuration,
renewDeadline: *options.RenewDeadline,
retryPeriod: *options.RetryPeriod,
Expand Down Expand Up @@ -535,12 +535,12 @@ func (o Options) setLeaderElectionConfig(obj v1alpha1.ControllerManagerConfigura
o.LeaderElectionResourceLock = obj.LeaderElection.ResourceLock
}

if o.LeaderElectionNamespace == "" && obj.LeaderElection.ResourceNamespace != "" {
o.LeaderElectionNamespace = obj.LeaderElection.ResourceNamespace
if o.LeaderElectionLockNamespace == "" && obj.LeaderElection.ResourceNamespace != "" {
o.LeaderElectionLockNamespace = obj.LeaderElection.ResourceNamespace
}

if o.LeaderElectionID == "" && obj.LeaderElection.ResourceName != "" {
o.LeaderElectionID = obj.LeaderElection.ResourceName
if o.LeaderElectionLockName == "" && obj.LeaderElection.ResourceName != "" {
o.LeaderElectionLockName = obj.LeaderElection.ResourceName
}

if o.LeaseDuration == nil && !reflect.DeepEqual(obj.LeaderElection.LeaseDuration, metav1.Duration{}) {
Expand Down
92 changes: 46 additions & 46 deletions pkg/manager/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ var _ = Describe("manger.Manager", func() {
Expect(*m.Cache.SyncPeriod).To(Equal(duration.Duration))
Expect(m.LeaderElection).To(Equal(leaderElect))
Expect(m.LeaderElectionResourceLock).To(Equal("leases"))
Expect(m.LeaderElectionNamespace).To(Equal("default"))
Expect(m.LeaderElectionID).To(Equal("ctrl-lease"))
Expect(m.LeaderElectionLockNamespace).To(Equal("default"))
Expect(m.LeaderElectionLockName).To(Equal("ctrl-lease"))
Expect(m.LeaseDuration.String()).To(Equal(duration.Duration.String()))
Expect(m.RenewDeadline.String()).To(Equal(duration.Duration.String()))
Expect(m.RetryPeriod.String()).To(Equal(duration.Duration.String()))
Expand Down Expand Up @@ -218,17 +218,17 @@ var _ = Describe("manger.Manager", func() {
SyncPeriod: &optDuration,
DefaultNamespaces: map[string]cache.Config{"ctrl": {}},
},
LeaderElection: true,
LeaderElectionResourceLock: "configmaps",
LeaderElectionNamespace: "ctrl",
LeaderElectionID: "ctrl-configmap",
LeaseDuration: &optDuration,
RenewDeadline: &optDuration,
RetryPeriod: &optDuration,
Metrics: metricsserver.Options{BindAddress: ":7000"},
HealthProbeBindAddress: "5000",
ReadinessEndpointName: "/readiness",
LivenessEndpointName: "/liveness",
LeaderElection: true,
LeaderElectionResourceLock: "configmaps",
LeaderElectionLockNamespace: "ctrl",
LeaderElectionLockName: "ctrl-configmap",
LeaseDuration: &optDuration,
RenewDeadline: &optDuration,
RetryPeriod: &optDuration,
Metrics: metricsserver.Options{BindAddress: ":7000"},
HealthProbeBindAddress: "5000",
ReadinessEndpointName: "/readiness",
LivenessEndpointName: "/liveness",
WebhookServer: webhook.NewServer(webhook.Options{
Port: 8080,
Host: "example.com",
Expand All @@ -241,8 +241,8 @@ var _ = Describe("manger.Manager", func() {
Expect(m.Cache.SyncPeriod.String()).To(Equal(optDuration.String()))
Expect(m.LeaderElection).To(BeTrue())
Expect(m.LeaderElectionResourceLock).To(Equal("configmaps"))
Expect(m.LeaderElectionNamespace).To(Equal("ctrl"))
Expect(m.LeaderElectionID).To(Equal("ctrl-configmap"))
Expect(m.LeaderElectionLockNamespace).To(Equal("ctrl"))
Expect(m.LeaderElectionLockName).To(Equal("ctrl-configmap"))
Expect(m.LeaseDuration.String()).To(Equal(optDuration.String()))
Expect(m.RenewDeadline.String()).To(Equal(optDuration.String()))
Expect(m.RetryPeriod.String()).To(Equal(optDuration.String()))
Expand Down Expand Up @@ -302,12 +302,12 @@ var _ = Describe("manger.Manager", func() {
Context("with leader election enabled", func() {
It("should only cancel the leader election after all runnables are done", func() {
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id-2",
HealthProbeBindAddress: "0",
Metrics: metricsserver.Options{BindAddress: "0"},
PprofBindAddress: "0",
LeaderElection: true,
LeaderElectionLockNamespace: "default",
LeaderElectionLockName: "test-leader-election-id-2",
HealthProbeBindAddress: "0",
Metrics: metricsserver.Options{BindAddress: "0"},
PprofBindAddress: "0",
})
Expect(err).ToNot(HaveOccurred())
gvkcorev1 := schema.GroupVersionKind{Group: corev1.SchemeGroupVersion.Group, Version: corev1.SchemeGroupVersion.Version, Kind: "ConfigMap"}
Expand Down Expand Up @@ -351,12 +351,12 @@ var _ = Describe("manger.Manager", func() {
})
It("should disable gracefulShutdown when stopping to lead", func() {
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id-3",
HealthProbeBindAddress: "0",
Metrics: metricsserver.Options{BindAddress: "0"},
PprofBindAddress: "0",
LeaderElection: true,
LeaderElectionLockNamespace: "default",
LeaderElectionLockName: "test-leader-election-id-3",
HealthProbeBindAddress: "0",
Metrics: metricsserver.Options{BindAddress: "0"},
PprofBindAddress: "0",
})
Expect(err).ToNot(HaveOccurred())

Expand All @@ -381,9 +381,9 @@ var _ = Describe("manger.Manager", func() {
It("should default ID to controller-runtime if ID is not set", func() {
var rl resourcelock.Interface
m1, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id",
LeaderElection: true,
LeaderElectionLockNamespace: "default",
LeaderElectionLockName: "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)
Expand All @@ -402,9 +402,9 @@ var _ = Describe("manger.Manager", func() {
m1cm.onStoppedLeading = func() {}

m2, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id",
LeaderElection: true,
LeaderElectionLockNamespace: "default",
LeaderElectionLockName: "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)
Expand Down Expand Up @@ -471,17 +471,17 @@ var _ = Describe("manger.Manager", func() {
})

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"})
m, err := New(cfg, Options{LeaderElection: true, LeaderElectionLockName: "controller-runtime"})
Expect(m).To(BeNil())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace: not running in-cluster, please specify LeaderElectionNamespace"))
Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace: not running in-cluster, please specify LeaderElectionLockNamespace"))
})

// 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 LeasesResourceLock", func() {
m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns"})
m, err := New(cfg, Options{LeaderElection: true, LeaderElectionLockName: "controller-runtime", LeaderElectionLockNamespace: "my-ns"})
Expect(m).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
cm, ok := m.(*controllerManager)
Expand All @@ -492,10 +492,10 @@ var _ = Describe("manger.Manager", func() {
})
It("should use the specified ResourceLock", func() {
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
LeaderElectionID: "controller-runtime",
LeaderElectionNamespace: "my-ns",
LeaderElection: true,
LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
LeaderElectionLockName: "controller-runtime",
LeaderElectionLockNamespace: "my-ns",
})
Expect(m).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
Expand All @@ -509,8 +509,8 @@ var _ = Describe("manger.Manager", func() {
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
LeaderElectionID: "controller-runtime",
LeaderElectionNamespace: "my-ns",
LeaderElectionLockName: "controller-runtime",
LeaderElectionLockNamespace: "my-ns",
LeaderElectionReleaseOnCancel: true,
newResourceLock: func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) {
var err error
Expand Down Expand Up @@ -1216,10 +1216,10 @@ var _ = Describe("manger.Manager", func() {
Context("with leaderelection enabled", func() {
startSuite(
Options{
LeaderElection: true,
LeaderElectionID: "controller-runtime",
LeaderElectionNamespace: "default",
newResourceLock: fakeleaderelection.NewResourceLock,
LeaderElection: true,
LeaderElectionLockName: "controller-runtime",
LeaderElectionLockNamespace: "default",
newResourceLock: fakeleaderelection.NewResourceLock,
},
func(m Manager) {
cm, ok := m.(*controllerManager)
Expand Down

0 comments on commit 4b954bd

Please sign in to comment.