diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 0beb363735..c08075ec98 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -227,12 +227,23 @@ func (s *composeService) setupNavigationMenu(ctx context.Context, options *api.U return formatter.NewKeyboardManager(isDockerDesktopActive, isLogsViewEnabled, signalChan), kEvents, nil } +// appendErr records err for the final report, unless it is nothing more than +// fallout from our own shutdown: once u.globalCtx is canceled (monitor +// detecting termination, SIGINT/SIGTERM, or an earlier setup failure), the +// in-flight goroutines it carries (log/attach streaming in particular) get a +// context.Canceled error that reports no real failure and must not turn a +// clean exit into a non-zero one (#13985). func (u *upSession) appendErr(err error) { - if err != nil { - u.mu.Lock() - u.errs = append(u.errs, err) - u.mu.Unlock() + if err == nil { + return + } + if errors.Is(err, context.Canceled) && u.globalCtx.Err() != nil { + logrus.Debugf("ignoring canceled error after shutdown: %v", err) + return } + u.mu.Lock() + u.errs = append(u.errs, err) + u.mu.Unlock() } // runEventLoop reacts to cancellation, SIGINT/SIGTERM and keyboard input until diff --git a/pkg/compose/up_test.go b/pkg/compose/up_test.go index f38a9e1aee..9cdfa49c82 100644 --- a/pkg/compose/up_test.go +++ b/pkg/compose/up_test.go @@ -17,6 +17,9 @@ package compose import ( + "context" + "errors" + "fmt" "testing" "gotest.tools/v3/assert" @@ -102,3 +105,35 @@ func TestShouldFollowStartEvent(t *testing.T) { }) } } + +// TestAppendErrDropsCancellationAfterShutdown is the #13985 follow-up: once +// our own shutdown has canceled globalCtx (monitor detecting termination, +// SIGINT/SIGTERM, ...), a lingering goroutine (log/attach streaming) racing +// that cancellation reports a context.Canceled error carrying no real +// failure. appendErr must drop it instead of turning a clean exit into a +// non-zero one, while still reporting any other, genuine error. +func TestAppendErrDropsCancellationAfterShutdown(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + u := &upSession{globalCtx: ctx} + + u.appendErr(errors.New("boom")) + assert.Equal(t, len(u.errs), 1) + + cancel() + + u.appendErr(fmt.Errorf("streaming logs: %w", context.Canceled)) + assert.Equal(t, len(u.errs), 1, "a context-canceled error after our own shutdown must be dropped") + + u.appendErr(errors.New("a real, unrelated failure")) + assert.Equal(t, len(u.errs), 2, "a genuine error occurring after shutdown must still be reported") +} + +// TestAppendErrKeepsCancellationBeforeShutdown pins the guard on +// globalCtx.Err(): a context.Canceled error must still be reported if it +// didn't come from our own globalCtx being canceled. +func TestAppendErrKeepsCancellationBeforeShutdown(t *testing.T) { + u := &upSession{globalCtx: t.Context()} + + u.appendErr(context.Canceled) + assert.Equal(t, len(u.errs), 1) +}