From fb21f97c41104f885c41fb5c6aae3d2d46cc8c19 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 16 Aug 2026 16:30:23 +0200 Subject: [PATCH] containerd: make shutdown waiting explicit Always return a containerd shutdown wait function after successful initialization, including when using an externally managed system containerd, where the function is a no-op. Use the returned function during the normal shutdown sequence, after the API server has stopped, so managed and embedded containerd instances have finished shutting down before tracing is torn down. Keep a deferred context cancellation as a safeguard for early returns. Bound OpenTelemetry shutdown to avoid blocking daemon shutdown indefinitely, and defer returning ServeAPI errors until containerd and tracing shutdown have completed. Also report failures while waiting for containerd to stop instead of silently discarding them. Signed-off-by: Sebastiaan van Stijn --- daemon/command/daemon.go | 43 +++++++++++++++++++++----------- daemon/command/daemon_unix.go | 2 +- daemon/command/daemon_windows.go | 5 ++-- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/daemon/command/daemon.go b/daemon/command/daemon.go index 7c80778785..4511884c9a 100644 --- a/daemon/command/daemon.go +++ b/daemon/command/daemon.go @@ -202,15 +202,22 @@ func (cli *daemonCLI) start(ctx context.Context) (retErr error) { } ctx, cancel := context.WithCancel(ctx) - waitForContainerDShutdown, err := cli.initContainerd(ctx) - if waitForContainerDShutdown != nil { - defer waitForContainerDShutdown(10 * time.Second) - } + waitShutdown, err := cli.initContainerd(ctx) if err != nil { cancel() return err } - defer cancel() + + // The normal shutdown path calls this before tracing is torn down. Defer it + // as a fallback for startup failures after containerd has started; OnceFunc + // prevents the deferred call from repeating the wait. + shutdownContainerd := sync.OnceFunc(func() { + cancel() + if err := waitShutdown(10 * time.Second); err != nil { + log.G(ctx).WithError(err).Warn("failed to wait for containerd to stop") + } + }) + defer shutdownContainerd() httpServer := &http.Server{ ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. @@ -405,17 +412,20 @@ func (cli *daemonCLI) start(ctx context.Context) (retErr error) { // shutdown / close BuildKit backend shutdownBuildKit() - // Stop notification processing and any background processes - cancel() + // Stop notification processing and background processes, then wait for + // containerd to stop. + shutdownContainerd() + + shutdownCtx, shutdownCancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + defer shutdownCancel() + + if err := otelShutdown(shutdownCtx); err != nil { + log.G(ctx).WithError(err).Error("failed to shutdown OTEL tracing") + } if err, ok := <-errAPI; ok { return errors.Wrap(err, "shutting down due to ServeAPI error") } - - if err := otelShutdown(context.WithoutCancel(ctx)); err != nil { - log.G(ctx).WithError(err).Error("Failed to shutdown OTEL tracing") - } - return nil } @@ -1141,15 +1151,20 @@ func overrideProxyEnv(ctx context.Context, name, val string) { _ = os.Setenv(name, val) } +// nopWaitFunc is used when containerd is managed externally. +func nopWaitFunc(time.Duration) error { + return nil +} + func (cli *daemonCLI) initializeContainerd(ctx context.Context) (func(time.Duration) error, error) { systemContainerdAddr, ok, err := systemContainerdRunning(honorXDG) if err != nil { return nil, errors.Wrap(err, "could not determine whether the system containerd is running") } if ok { - // detected a system containerd at the given address. + // Use the externally managed system containerd detected at this address. cli.Config.ContainerdAddr = systemContainerdAddr - return nil, nil + return nopWaitFunc, nil } log.G(ctx).Info("containerd not running, starting managed containerd") diff --git a/daemon/command/daemon_unix.go b/daemon/command/daemon_unix.go index c85cfc8627..1389e04405 100644 --- a/daemon/command/daemon_unix.go +++ b/daemon/command/daemon_unix.go @@ -124,7 +124,7 @@ func (cli *daemonCLI) initContainerd(ctx context.Context) (func(time.Duration) e } if cli.Config.ContainerdAddr != "" { // use system containerd at the given address. - return nil, nil + return nopWaitFunc, nil } return cli.initializeContainerd(ctx) diff --git a/daemon/command/daemon_windows.go b/daemon/command/daemon_windows.go index 0c9f915dc0..e22d2cc8ea 100644 --- a/daemon/command/daemon_windows.go +++ b/daemon/command/daemon_windows.go @@ -116,12 +116,13 @@ func (cli *daemonCLI) initContainerd(ctx context.Context) (func(time.Duration) e return cli.initEmbeddedContainerd(ctx) } if cli.Config.ContainerdAddr != "" { - return nil, nil + // use system containerd at the given address. + return nopWaitFunc, nil } if cli.Config.DefaultRuntime == "" || cli.Config.DefaultRuntime == config.WindowsV1RuntimeName { // Legacy non-containerd runtime is used - return nil, nil + return nopWaitFunc, nil } return cli.initializeContainerd(ctx)