Skip to content

Commit

Permalink
⚠️ Use leader elector with client timeout
Browse files Browse the repository at this point in the history
This change makes the leader elector use a client that internally has
a smaller timeout than the renew deadline, which avoids a situation
where a single request timing out makes us lose the leader lease.
  • Loading branch information
alvaroaleman committed Nov 30, 2024
1 parent b88f351 commit 5bf5874
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
24 changes: 9 additions & 15 deletions pkg/leaderelection/leader_election.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ import (
"errors"
"fmt"
"os"
"time"

"k8s.io/apimachinery/pkg/util/uuid"
coordinationv1client "k8s.io/client-go/kubernetes/typed/coordination/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/leaderelection/resourcelock"

Expand All @@ -49,6 +48,9 @@ type Options struct {
// LeaderElectionID determines the name of the resource that leader election
// will use for holding the leader lock.
LeaderElectionID string

// RewnewDeadline is the renew deadline for this leader election client
RewnewDeadline time.Duration
}

// NewResourceLock creates a new resource lock for use in a leader election loop.
Expand Down Expand Up @@ -88,25 +90,17 @@ func NewResourceLock(config *rest.Config, recorderProvider recorder.Provider, op

// Construct clients for leader election
rest.AddUserAgent(config, "leader-election")
corev1Client, err := corev1client.NewForConfig(config)
if err != nil {
return nil, err
}

coordinationClient, err := coordinationv1client.NewForConfig(config)
if err != nil {
return nil, err
}

return resourcelock.New(options.LeaderElectionResourceLock,
return resourcelock.NewFromKubeconfig(options.LeaderElectionResourceLock,
options.LeaderElectionNamespace,
options.LeaderElectionID,
corev1Client,
coordinationClient,
resourcelock.ResourceLockConfig{
Identity: id,
EventRecorder: recorderProvider.GetEventRecorderFor(id),
})
},
config,
options.RewnewDeadline,
)
}

func getInClusterNamespace() (string, error) {
Expand Down
1 change: 1 addition & 0 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
LeaderElectionResourceLock: options.LeaderElectionResourceLock,
LeaderElectionID: options.LeaderElectionID,
LeaderElectionNamespace: options.LeaderElectionNamespace,
RewnewDeadline: *options.RenewDeadline,
})
if err != nil {
return nil, err
Expand Down
22 changes: 22 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,28 @@ var _ = Describe("manger.Manager", func() {
<-m2done
})

It("should default RenewDeadline for leader election config", func() {
var rl resourcelock.Interface
m1, 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) {
if options.RewnewDeadline != 10*time.Second {
return nil, fmt.Errorf("expected RenewDeadline to be 10s, got %v", options.RewnewDeadline)
}
var err error
rl, err = leaderelection.NewResourceLock(config, recorderProvider, options)
return rl, err
},
HealthProbeBindAddress: "0",
Metrics: metricsserver.Options{BindAddress: "0"},
PprofBindAddress: "0",
})
Expect(err).ToNot(HaveOccurred())
Expect(m1).ToNot(BeNil())
})

It("should default ID to controller-runtime if ID is not set", func() {
var rl resourcelock.Interface
m1, err := New(cfg, Options{
Expand Down

0 comments on commit 5bf5874

Please sign in to comment.