mirror of
https://github.com/docker/compose.git
synced 2026-10-12 02:13:08 +00:00
fix(up): drop benign context.Canceled noise from the final report
The new TestUpAttachedTerminatesOnExternalStop (#13990) occasionally observed `up` exit 1 instead of 0 after an external stop/down cancels a service's restart backoff. The cause: once the monitor detects termination and cancels globalCtx, a log-streaming goroutine still blocked on logs?follow=1 for the backoff container gets a context.Canceled error, which appendErr collected unfiltered into the final errors.Join. That race could only surface once #13990 made the monitor loop actually return in this scenario -- before that fix, up hung forever and never reached this code path. appendErr now drops an error that is nothing more than fallout from our own shutdown (errors.Is(err, context.Canceled) once u.globalCtx.Err() is set), the same way a SIGINT-triggered teardown's residual goroutine errors are already expected to be harmless. A genuine error is still reported regardless of shutdown state. Signed-off-by: Nicolas De Loof <[email protected]>
This commit is contained in:
committed by
Guillaume Lours
parent
6970069cc6
commit
01a60a5e99
+15
-4
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user