diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index ceb1450d1b..591a252722 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -51,15 +51,20 @@ import ( const ( // Values taken from: https://github.com/kubernetes/component-base/blob/master/config/v1alpha1/defaults.go - defaultLeaseDuration = 15 * time.Second - defaultRenewDeadline = 10 * time.Second - defaultRetryPeriod = 2 * time.Second + defaultLeaseDuration = 15 * time.Second + defaultRenewDeadline = 10 * time.Second + defaultRetryPeriod = 2 * time.Second + defaultGracefulShutdownPeriod = 30 * time.Second defaultReadinessEndpoint = "/readyz" defaultLivenessEndpoint = "/healthz" ) +// errLeaderElectionLost is returned by OnStoppedLeading when leader election is lost. +// It is used to suppress this expected error during graceful shutdown. +var errLeaderElectionLost = errors.New("leader election lost") + var _ Runnable = &controllerManager{} type controllerManager struct { @@ -528,8 +533,15 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e cm.internalCancel() }) select { + // errLeaderElectionLost is safe to suppress here. This drain goroutine + // is only started from engageStopProcedure, which is called via defer + // in Start() — meaning Start() has already exited its select loop and + // returned any mid-run errLeaderElectionLost to the caller before this + // goroutine ever reads from errChan. Any errLeaderElectionLost seen + // here is therefore always the result of our own cm.internalCancel() + // call above, not a genuine unexpected loss. case err := <-cm.errChan: - if !errors.Is(err, context.Canceled) { + if !errors.Is(err, context.Canceled) && !errors.Is(err, errLeaderElectionLost) { cm.logger.Error(err, "error received after stop sequence was engaged") } case <-stopComplete: @@ -634,7 +646,7 @@ func (cm *controllerManager) initLeaderElector() (*leaderelection.LeaderElector, // Most implementations of leader election log.Fatal() here. // Since Start is wrapped in log.Fatal when called, we can just return // an error here which will cause the program to exit. - cm.errChan <- errors.New("leader election lost") + cm.errChan <- errLeaderElectionLost }, }, ReleaseOnCancel: cm.leaderElectionReleaseOnCancel, diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 10a73072bb..d815b84fa6 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -45,6 +45,7 @@ import ( utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/client-go/rest" "k8s.io/client-go/tools/leaderelection/resourcelock" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/cache/informertest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -2073,6 +2074,32 @@ var _ = Describe("manger.Manager", func() { <-m.Elected() Expect(<-runnableExecutionOrderChan).To(Equal(leaderElectionRunnableName)) }) + + It("should not return leader election lost during graceful shutdown", func(ctx SpecContext) { + runCtx, cancel := context.WithCancel(ctx) + defer cancel() + + m, err := New(cfg, Options{ + LeaderElection: true, + LeaderElectionID: "leader-election-lost-on-stop-test", + LeaderElectionNamespace: "default", + GracefulShutdownTimeout: ptr.To(5 * time.Second), + }) + Expect(err).NotTo(HaveOccurred()) + + doneCh := make(chan error, 1) + go func() { + doneCh <- m.Start(runCtx) + }() + + Eventually(m.Elected()).Should(BeClosed()) + + cancel() + + Eventually(doneCh).Should( + Receive(Or(BeNil(), MatchError(context.Canceled))), + ) + }) }) type runnableError struct{}