From cb1bd09d18a069e0bb8ba9922fe0322dbb1c8a03 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 17 Aug 2026 21:14:33 +0000 Subject: [PATCH] fix(docker): disambiguate self-inflicted ping timeout from daemon refusal Ping killed a still-running `podman info` at its own hard-coded 10s deadline under CI runner CPU/IO contention (CI run 32065209141, up-provider-podman-rootless-exec), then reported the bare "signal: killed" from that kill as an unqualified "daemon is not reachable" -- indistinguishable from a genuine refusal. This masqueraded as test flakiness; --ginkgo.flake-attempts=2 just re-rolled the same race. Ping, StartPodmanMachine, and the systemctl call in StartRootlessPodmanSocket now route through runCmd (via a new runCmdCombined helper) so a kill caused by our own context deadline is wrapped with ctx.Err(), and pingTimeout is raised from 10s to 30s. runPreflight reports a distinct 'did not respond in time' message when the ping error carries context.DeadlineExceeded, instead of always asserting the daemon is down. --- pkg/docker/helper.go | 43 ++++++++++---------- pkg/docker/helper_test.go | 62 +++++++++++++++++++++++++++++ pkg/driver/docker/docker.go | 10 ++++- pkg/driver/docker/preflight_test.go | 29 ++++++++++++++ 4 files changed, 120 insertions(+), 24 deletions(-) diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go index 4442a055a..55607e66d 100644 --- a/pkg/docker/helper.go +++ b/pkg/docker/helper.go @@ -130,7 +130,22 @@ func (r *DockerHelper) ClientVersion(ctx context.Context) string { } // podmanMachineStartTimeout bounds a Podman machine boot, which spins up a VM. -const podmanMachineStartTimeout = 90 * time.Second +var podmanMachineStartTimeout = 90 * time.Second + +var pingTimeout = 30 * time.Second + +func runCmdCombined(ctx context.Context, cmd *exec.Cmd) error { + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + if err := runCmd(ctx, cmd); err != nil { + if msg := strings.TrimSpace(out.String()); msg != "" { + return fmt.Errorf("%s: %w", msg, err) + } + return err + } + return nil +} // Ping reports whether the runtime daemon is reachable, returning its own // message (e.g. "Cannot connect to Podman") on failure. It runs a bare `info` @@ -138,17 +153,10 @@ const podmanMachineStartTimeout = 90 * time.Second // between docker (.ServerVersion) and podman/nerdctl, so a shared template // would falsely fail non-docker runtimes. func (r *DockerHelper) Ping(ctx context.Context) error { - cctx, cancel := context.WithTimeout(ctx, 10*time.Second) + cctx, cancel := context.WithTimeout(ctx, pingTimeout) defer cancel() - out, err := r.buildCmd(cctx, "info").CombinedOutput() - if err != nil { - if msg := strings.TrimSpace(string(out)); msg != "" { - return fmt.Errorf("%s: %w", msg, err) - } - return err - } - return nil + return runCmdCombined(cctx, r.buildCmd(cctx, "info")) } // StartPodmanMachine starts the default Podman machine, which must already exist. @@ -156,14 +164,7 @@ func (r *DockerHelper) StartPodmanMachine(ctx context.Context) error { cctx, cancel := context.WithTimeout(ctx, podmanMachineStartTimeout) defer cancel() - out, err := r.buildCmd(cctx, "machine", "start").CombinedOutput() - if err != nil { - if msg := strings.TrimSpace(string(out)); msg != "" { - return fmt.Errorf("%s: %w", msg, err) - } - return err - } - return nil + return runCmdCombined(cctx, r.buildCmd(cctx, "machine", "start")) } // PodmanMachineExists reports whether a Podman machine exists. @@ -197,11 +198,7 @@ func (r *DockerHelper) StartRootlessPodmanSocket(ctx context.Context) error { if runtime.GOOS == "linux" && isSystemdRunning(cctx) { cmd := exec.CommandContext(cctx, "systemctl", "--user", "start", "podman.socket") - out, err := cmd.CombinedOutput() - if err != nil { - if msg := strings.TrimSpace(string(out)); msg != "" { - return fmt.Errorf("%s: %w", msg, err) - } + if err := runCmdCombined(cctx, cmd); err != nil { return err } } diff --git a/pkg/docker/helper_test.go b/pkg/docker/helper_test.go index e581b6b22..940622b33 100644 --- a/pkg/docker/helper_test.go +++ b/pkg/docker/helper_test.go @@ -454,3 +454,65 @@ func TestSystemdStateIsUsable(t *testing.T) { } assert.True(t, systemdStateIsUsable(systemdStateDegraded+"\n")) } + +func withPingTimeout(t *testing.T, d time.Duration) { + t.Helper() + original := pingTimeout + pingTimeout = d + t.Cleanup(func() { pingTimeout = original }) +} + +func TestPing_Succeeds(t *testing.T) { + bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh +echo '{"ServerVersion":"1.0"}' +`) + h := &DockerHelper{DockerCommand: bin} + assert.NoError(t, h.Ping(context.Background())) +} + +func TestPing_DaemonRefusalIsNotAttributedToTimeout(t *testing.T) { + bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh +echo "Cannot connect to the Docker daemon at unix:///var/run/docker.sock" >&2 +exit 1 +`) + h := &DockerHelper{DockerCommand: bin} + err := h.Ping(context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "Cannot connect to the Docker daemon") + assert.NotErrorIs(t, err, context.DeadlineExceeded, + "a fast, native refusal must not look like our own timeout") +} + +func TestPing_SelfInflictedTimeoutIsDistinguishableFromDaemonDown(t *testing.T) { + withPingTimeout(t, 50*time.Millisecond) + + bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh +sleep 5 +`) + h := &DockerHelper{DockerCommand: bin} + err := h.Ping(context.Background()) + + require.Error(t, err) + assert.ErrorIs( + t, + err, + context.DeadlineExceeded, + "a kill caused by Ping's own deadline must be attributable to it, not reported as a bare daemon-down error", + ) +} + +func TestStartPodmanMachine_SelfInflictedTimeoutIsDistinguishable(t *testing.T) { + original := podmanMachineStartTimeout + podmanMachineStartTimeout = 50 * time.Millisecond + t.Cleanup(func() { podmanMachineStartTimeout = original }) + + bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh +sleep 5 +`) + h := &DockerHelper{DockerCommand: bin} + err := h.StartPodmanMachine(context.Background()) + + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) +} diff --git a/pkg/driver/docker/docker.go b/pkg/driver/docker/docker.go index ac9e539be..d73957b67 100644 --- a/pkg/driver/docker/docker.go +++ b/pkg/driver/docker/docker.go @@ -2,6 +2,7 @@ package docker import ( "context" + "errors" "fmt" "os" "os/exec" @@ -213,9 +214,16 @@ func runPreflight(ctx context.Context, opts driver.PreflightOptions, p dockerPro } } + reachability := fmt.Sprintf("%s daemon is not reachable", p.runtime) + if errors.Is(err, context.DeadlineExceeded) { + reachability = fmt.Sprintf( + "%s daemon did not respond in time (it may just be slow to start, not necessarily down)", + p.runtime, + ) + } return &driver.PreflightError{ Provider: runtimeName, - Err: fmt.Errorf("%w: %s daemon is not reachable", err, p.runtime), + Err: fmt.Errorf("%w: %s", err, reachability), } } diff --git a/pkg/driver/docker/preflight_test.go b/pkg/driver/docker/preflight_test.go index 257476a01..a3e52861c 100644 --- a/pkg/driver/docker/preflight_test.go +++ b/pkg/driver/docker/preflight_test.go @@ -3,6 +3,7 @@ package docker import ( "context" "errors" + "fmt" "os" "path/filepath" "strings" @@ -352,3 +353,31 @@ func TestIsRootlessDockerHostEmptyFallsBackToUID(t *testing.T) { t.Errorf("isRootlessDockerHost(\"\") = %v, want %v (uid=%d)", got, want, os.Geteuid()) } } + +func TestRunPreflightDistinguishesTimeoutFromRefusal(t *testing.T) { + timedOut := fmt.Errorf("%w: %w", context.DeadlineExceeded, errors.New("signal: killed")) + p := dockerProbe{ + runtime: docker.RuntimeDocker, + lookPath: installed, + ping: func(context.Context) error { return timedOut }, + } + err := runPreflight(context.Background(), driver.PreflightOptions{}, p) + require.Error(t, err) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.NotContains(t, err.Error(), "is not reachable", + "a self-inflicted timeout must not be reported as an authoritative refusal") + require.Contains(t, err.Error(), "did not respond in time") +} + +func TestRunPreflightDaemonRefusalKeepsUnreachableMessage(t *testing.T) { + down := errors.New("Cannot connect to the Docker daemon") + p := dockerProbe{ + runtime: docker.RuntimeDocker, + lookPath: installed, + ping: func(context.Context) error { return down }, + } + err := runPreflight(context.Background(), driver.PreflightOptions{}, p) + require.Error(t, err) + require.NotErrorIs(t, err, context.DeadlineExceeded) + require.Contains(t, err.Error(), "is not reachable") +}