From e59992041459d5c6a09c3ad34bbe82ceee713a7d Mon Sep 17 00:00:00 2001 From: liketosweep Date: Thu, 25 Jun 2026 15:47:37 +0000 Subject: [PATCH 1/2] fix: suppress expected leader-election-lost error on graceful shutdown --- pkg/manager/internal.go | 15 ++++++++++----- pkg/manager/manager_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index ceb1450d1b..57d090cd8f 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 { @@ -529,7 +534,7 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e }) select { 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 +639,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..d03f690fb6 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -2073,6 +2073,41 @@ var _ = Describe("manger.Manager", func() { <-m.Elected() Expect(<-runnableExecutionOrderChan).To(Equal(leaderElectionRunnableName)) }) + + It("should not log an error when leader election is lost during graceful shutdown", func() { + // This is the scenario from https://github.com/kubernetes-sigs/controller-runtime/issues/3535: + // Stopping the manager releases the leader lock, which triggers OnStoppedLeading. + // That callback sends errLeaderElectionLost onto errChan. The drain goroutine in + // engageStopProcedure must not log this as an unexpected error. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + m, err := New(cfg, Options{ + LeaderElection: true, + LeaderElectionID: "leader-election-lost-on-stop-test", + LeaderElectionNamespace: "default", + GracefulShutdownTimeout: func() *time.Duration { d := 5 * time.Second; return &d }(), + }) + Expect(err).NotTo(HaveOccurred()) + + doneCh := make(chan error, 1) + go func() { + doneCh <- m.Start(ctx) + }() + + // Wait until elected. + select { + case <-m.Elected(): + case <-time.After(10 * time.Second): + Fail("timed out waiting for manager to be elected") + } + + // Cancel the context — this is the normal shutdown path. + cancel() + + // Start() must return cleanly (nil or context.Canceled), not "leader election lost". + Eventually(doneCh, "10s").Should(Receive(Or(BeNil(), MatchError(context.Canceled)))) + }) }) type runnableError struct{} From 6912a08a56e76f3bb82f45dd67c76fa242be3ad2 Mon Sep 17 00:00:00 2001 From: liketosweep Date: Thu, 25 Jun 2026 16:40:10 +0000 Subject: [PATCH 2/2] issue-3535 fix Signed-off-by: liketosweep --- pkg/manager/internal.go | 7 +++++++ pkg/manager/manager_test.go | 26 +++++++++----------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 57d090cd8f..591a252722 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -533,6 +533,13 @@ 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) && !errors.Is(err, errLeaderElectionLost) { cm.logger.Error(err, "error received after stop sequence was engaged") diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index d03f690fb6..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" @@ -2074,39 +2075,30 @@ var _ = Describe("manger.Manager", func() { Expect(<-runnableExecutionOrderChan).To(Equal(leaderElectionRunnableName)) }) - It("should not log an error when leader election is lost during graceful shutdown", func() { - // This is the scenario from https://github.com/kubernetes-sigs/controller-runtime/issues/3535: - // Stopping the manager releases the leader lock, which triggers OnStoppedLeading. - // That callback sends errLeaderElectionLost onto errChan. The drain goroutine in - // engageStopProcedure must not log this as an unexpected error. - ctx, cancel := context.WithCancel(context.Background()) + 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: func() *time.Duration { d := 5 * time.Second; return &d }(), + GracefulShutdownTimeout: ptr.To(5 * time.Second), }) Expect(err).NotTo(HaveOccurred()) doneCh := make(chan error, 1) go func() { - doneCh <- m.Start(ctx) + doneCh <- m.Start(runCtx) }() - // Wait until elected. - select { - case <-m.Elected(): - case <-time.After(10 * time.Second): - Fail("timed out waiting for manager to be elected") - } + Eventually(m.Elected()).Should(BeClosed()) - // Cancel the context — this is the normal shutdown path. cancel() - // Start() must return cleanly (nil or context.Canceled), not "leader election lost". - Eventually(doneCh, "10s").Should(Receive(Or(BeNil(), MatchError(context.Canceled)))) + Eventually(doneCh).Should( + Receive(Or(BeNil(), MatchError(context.Canceled))), + ) }) })