Skip to content

Commit

Permalink
Quiet context.Canceled errors during shutdown
Browse files Browse the repository at this point in the history
Runnable implementations that return ctx.Err() cause a spurious
"error received after stop" log message.
  • Loading branch information
cbandy committed Apr 2, 2024
1 parent 2136860 commit 732bd60
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,8 @@ func (cm *controllerManager) engageStopProcedure(stopComplete <-chan struct{}) e
cm.internalCancel()
})
select {
case err, ok := <-cm.errChan:
if ok {
case err := <-cm.errChan:
if !errors.Is(err, context.Canceled) {
cm.logger.Error(err, "error received after stop sequence was engaged")
}
case <-stopComplete:
Expand Down
49 changes: 49 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/go-logr/logr/funcr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -1044,6 +1045,54 @@ var _ = Describe("manger.Manager", func() {
}))).NotTo(Succeed())
})

It("should not return runnables context.Canceled errors", func() {
Expect(options.Logger).To(BeZero(), "this test overrides Logger")

var log struct {
sync.Mutex
messages []string
}
options.Logger = funcr.NewJSON(func(object string) {
log.Lock()
log.messages = append(log.messages, object)
log.Unlock()
}, funcr.Options{})

m, err := New(cfg, options)
Expect(err).NotTo(HaveOccurred())
for _, cb := range callbacks {
cb(m)
}

// Runnables may return ctx.Err() as shown in some [context.Context] examples.
started := make(chan struct{})
Expect(m.Add(RunnableFunc(func(ctx context.Context) error {
close(started)
<-ctx.Done()
return ctx.Err()
}))).To(Succeed())

stopped := make(chan error)
ctx, cancel := context.WithCancel(context.Background())
go func() {
stopped <- m.Start(ctx)
}()

// Wait for runnables to start, signal the manager, and wait for it.
<-started
cancel()
Expect(<-stopped).To(Succeed())

// TODO: Diagnose why LeaderElection races after Start() returns.
if options.LeaderElection {
time.Sleep(time.Second)
}

Expect(log.messages).To(Not(ContainElement(
ContainSubstring(context.Canceled.Error()),
)))
})

It("should return both runnables and stop errors when both error", func() {
m, err := New(cfg, options)
Expect(err).NotTo(HaveOccurred())
Expand Down

0 comments on commit 732bd60

Please sign in to comment.