From 289d4e54769e6fc03dc56b231e9401b853ba2dc1 Mon Sep 17 00:00:00 2001 From: mintaka Date: Thu, 15 Oct 2026 03:31:32 -0400 Subject: [PATCH 1/2] fix(runtime): don't cancel podman start/run mid netns setup (RIG-5334) A signal to the podman client after pasta writes its pid file but before podman writes `rootless-netns/resolv.conf` leaves pasta alive with no `resolv.conf`. podman reuses that netns, so every later run fails with `mount resolv.conf to "/etc/resolv.conf": no such file or directory`. Caller ctx cancellation (runner shutdown) SIGKILLed in-flight `podman start`. `PodmanCLI.Start`, the pasta canary `podman run`, and the stack adapters' `podman run` now ignore caller cancellation. They stay bounded by their own command timeout and return the ctx error after exit. Co-authored-by: Matt Wilkinson --- go/internal/runtime/clispawn.go | 34 +++++- go/internal/runtime/podman.go | 4 +- go/internal/runtime/podman_test.go | 87 +++++++++++++++ .../stack/adapters/postgres_container.go | 26 ++++- .../stack/adapters/postgres_container_test.go | 104 ++++++++++++++++++ 5 files changed, 241 insertions(+), 14 deletions(-) diff --git a/go/internal/runtime/clispawn.go b/go/internal/runtime/clispawn.go index 892e9fe3c..58975bb8b 100644 --- a/go/internal/runtime/clispawn.go +++ b/go/internal/runtime/clispawn.go @@ -33,7 +33,15 @@ type cliEngine struct { // Only a spawn failure, this call's own timeout, or parent-context cancellation // is a non-nil error. func (e cliEngine) spawnCapture(ctx context.Context, summary string, args []string, stdin *string) (stdout, stderr []byte, exitCode int, err error) { - cctx, cancel := context.WithTimeout(ctx, e.timeout) + return e.spawnCaptureWithCancellation(ctx, summary, args, stdin, false) +} + +func (e cliEngine) spawnCaptureWithCancellation(ctx context.Context, summary string, args []string, stdin *string, detached bool) (stdout, stderr []byte, exitCode int, err error) { + commandCtx := ctx + if detached { + commandCtx = context.WithoutCancel(ctx) + } + cctx, cancel := context.WithTimeout(commandCtx, e.timeout) defer cancel() //nolint:gosec // G204: this is the container-engine seam — spawning the @@ -55,7 +63,11 @@ func (e cliEngine) spawnCapture(ctx context.Context, summary string, args []stri cmd.Stdin = strings.NewReader(*stdin) } - if runErr := cmd.Run(); runErr != nil { + runErr := cmd.Run() + if detached && ctx.Err() != nil { + return nil, nil, 0, ctx.Err() + } + if runErr != nil { switch { case cctx.Err() == context.DeadlineExceeded && ctx.Err() == nil: // This call's own timeout fired (not the parent): the process was @@ -75,10 +87,14 @@ func (e cliEngine) spawnCapture(ctx context.Context, summary string, args []stri return out.Bytes(), errBuf.Bytes(), 0, nil } -// run runs ` `, requiring a zero exit (a non-zero becomes a -// CommandError). For fire-and-check operations like create/start/stop/remove. -func (e cliEngine) run(ctx context.Context, summary string, args []string) ([]byte, error) { - stdout, stderr, exitCode, err := e.spawnCapture(ctx, summary, args, nil) +// runDetached is run without caller cancellation, bounded only by the timeout: +// a signal mid netns setup can leave pasta running without resolv.conf. +func (e cliEngine) runDetached(ctx context.Context, summary string, args []string) ([]byte, error) { + return e.runWithCancellation(ctx, summary, args, true) +} + +func (e cliEngine) runWithCancellation(ctx context.Context, summary string, args []string, detached bool) ([]byte, error) { + stdout, stderr, exitCode, err := e.spawnCaptureWithCancellation(ctx, summary, args, nil, detached) if err != nil { return nil, err } @@ -92,6 +108,12 @@ func (e cliEngine) run(ctx context.Context, summary string, args []string) ([]by return stdout, nil } +// run runs ` `, requiring a zero exit (a non-zero becomes a +// CommandError). For fire-and-check operations like create/start/stop/remove. +func (e cliEngine) run(ctx context.Context, summary string, args []string) ([]byte, error) { + return e.runWithCancellation(ctx, summary, args, false) +} + // stopWithClientScript runs "$@" with stdin relayed by a watcher in the same // process group. Killing the engine client closes that stdin, and the watcher // then SIGKILLs the group; a natural exit kills the watcher and keeps the status. diff --git a/go/internal/runtime/podman.go b/go/internal/runtime/podman.go index 7c9dabc03..87f94d600 100644 --- a/go/internal/runtime/podman.go +++ b/go/internal/runtime/podman.go @@ -569,7 +569,7 @@ func (p *PodmanCLI) VerifyNoPastaForwards(ctx context.Context, image string) err if mode != "pasta" && !strings.HasPrefix(mode, "pasta:") { return nil } - _, err = p.run(ctx, "podman run (pasta canary)", []string{ + _, err = p.runDetached(ctx, "podman run (pasta canary)", []string{ "run", "--rm", "--network", pastaCanaryNetwork, probeEntrypoint, image, }) if cmdErr, ok := errors.AsType[*CommandError](err); ok && strings.Contains(cmdErr.Stderr, "conflicts with previous options") { @@ -605,7 +605,7 @@ func parsePodmanVersion(s string) (major, minor int, err error) { // Start starts a created container. func (p *PodmanCLI) Start(ctx context.Context, id WorkloadID) error { - _, err := p.run(ctx, "podman start", []string{"start", id.String()}) + _, err := p.runDetached(ctx, "podman start", []string{"start", id.String()}) return err } diff --git a/go/internal/runtime/podman_test.go b/go/internal/runtime/podman_test.go index eb0341970..63e56623b 100644 --- a/go/internal/runtime/podman_test.go +++ b/go/internal/runtime/podman_test.go @@ -629,3 +629,90 @@ esac }) } } + +func TestPodmanStartSurvivesCallerCancellation(t *testing.T) { + dir := t.TempDir() + started := filepath.Join(dir, "started") + proceed := filepath.Join(dir, "go") + doneMarker := filepath.Join(dir, "done") + prog := filepath.Join(dir, "podman-stub.sh") + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" != "start" ]; then exit 91; fi +: > %q +while [ ! -e %q ]; do :; done +: > %q +`, started, proceed, doneMarker) + if err := os.WriteFile(prog, []byte(script), 0o755); err != nil { + t.Fatalf("writing stub: %v", err) + } + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + cli := NewPodmanCLI().WithProgram(prog) + result := make(chan error, 1) + go func() { result <- cli.Start(ctx, WorkloadID("container")) }() + waitForMarker(t, started) + cancel() + if err := os.WriteFile(proceed, nil, 0o600); err != nil { + t.Fatalf("creating go marker: %v", err) + } + + var err error + select { + case err = <-result: + case <-time.After(10 * time.Second): + t.Fatal("podman start did not return after the stub was released") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("Start error = %v, want context.Canceled", err) + } + if _, err := os.Stat(doneMarker); err != nil { + t.Errorf("stub did not finish after caller cancellation: %v", err) + } +} + +func TestPodmanStartTimeoutKillsHungProcess(t *testing.T) { + pidFile := filepath.Join(t.TempDir(), "child.pid") + prog := filepath.Join(t.TempDir(), "podman-stub.sh") + script := fmt.Sprintf("#!/bin/sh\necho $$ > %q\nexec sleep 30\n", pidFile) + if err := os.WriteFile(prog, []byte(script), 0o755); err != nil { + t.Fatalf("writing stub: %v", err) + } + t.Cleanup(func() { killRecordedPid(t, pidFile) }) + cli := NewPodmanCLI().WithProgram(prog).WithTimeout(100 * time.Millisecond) + result := make(chan error, 1) + go func() { result <- cli.Start(t.Context(), WorkloadID("container")) }() + pid := waitForRecordedPid(t, pidFile) + + select { + case err := <-result: + if _, ok := errors.AsType[*TimeoutError](err); !ok { + t.Fatalf("Start error = %v (%T), want *TimeoutError", err, err) + } + case <-time.After(5 * time.Second): + t.Fatal("podman start did not return after its command timeout") + } + if err := syscall.Kill(pid, 0); !errors.Is(err, syscall.ESRCH) { + t.Fatalf("after start timeout, kill(%d, 0) = %v, want ESRCH", pid, err) + } +} + +func waitForMarker(t *testing.T, path string) { + t.Helper() + timer := time.NewTimer(10 * time.Second) + defer timer.Stop() + ticker := time.NewTicker(5 * time.Millisecond) + defer ticker.Stop() + for { + if _, err := os.Stat(path); err == nil { + return + } else if !os.IsNotExist(err) { + t.Fatalf("checking marker %q: %v", path, err) + } + select { + case <-timer.C: + t.Fatalf("marker %q was not created before the deadline", path) + case <-ticker.C: + } + } +} diff --git a/go/internal/stack/adapters/postgres_container.go b/go/internal/stack/adapters/postgres_container.go index 87545a515..fce37f038 100644 --- a/go/internal/stack/adapters/postgres_container.go +++ b/go/internal/stack/adapters/postgres_container.go @@ -292,9 +292,11 @@ func newPodmanExec() *podmanExec { return &podmanExec{program: "podman", timeout: containerCommandTimeout} } -// run runs `podman ` to completion, requiring a zero exit. +// run runs `podman ` to completion, requiring a zero exit. Caller +// cancellation is not delivered: a signal mid netns setup can leave pasta +// running without resolv.conf, breaking every later podman run. func (e *podmanExec) run(ctx context.Context, args []string) error { - return e.fireAndCheck(ctx, args) + return e.fireAndCheckWithCancellation(ctx, args, true) } // wait blocks until the named container exits (`podman wait `). A @@ -387,18 +389,30 @@ func (e *podmanExec) exists(ctx context.Context, name string) (bool, error) { // fireAndCheck runs `podman ` under the command timeout, folding a // non-zero exit into an error carrying the captured stderr for diagnosis. func (e *podmanExec) fireAndCheck(ctx context.Context, args []string) error { - cctx, cancel := context.WithTimeout(ctx, e.timeout) + return e.fireAndCheckWithCancellation(ctx, args, false) +} + +func (e *podmanExec) fireAndCheckWithCancellation(ctx context.Context, args []string, detached bool) error { + commandCtx := ctx + if detached { + commandCtx = context.WithoutCancel(ctx) + } + cctx, cancel := context.WithTimeout(commandCtx, e.timeout) defer cancel() cmd := exec.CommandContext(cctx, e.program, args...) //nolint:gosec // G204: the container seam — program is the operator-set engine and args are Stack-built from a state-dir-derived spec, neither attacker-controlled cmd.WaitDelay = podmanWaitDelay var stderr strings.Builder cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { + runErr := cmd.Run() + if detached && ctx.Err() != nil { + return ctx.Err() + } + if runErr != nil { msg := strings.TrimSpace(stderr.String()) if msg == "" { - return fmt.Errorf("podman %s: %w", args[0], err) + return fmt.Errorf("podman %s: %w", args[0], runErr) } - return fmt.Errorf("podman %s: %w: %s", args[0], err, msg) + return fmt.Errorf("podman %s: %w: %s", args[0], runErr, msg) } return nil } diff --git a/go/internal/stack/adapters/postgres_container_test.go b/go/internal/stack/adapters/postgres_container_test.go index 669c151f6..bdbf0cfed 100644 --- a/go/internal/stack/adapters/postgres_container_test.go +++ b/go/internal/stack/adapters/postgres_container_test.go @@ -5,10 +5,13 @@ package adapters import ( "context" "errors" + "fmt" "os" "path/filepath" "reflect" + "strconv" "strings" + "syscall" "testing" "time" @@ -350,3 +353,104 @@ func TestStopSecondsRounding(t *testing.T) { } } } + +// fireAndCheck previously surfaced a killed-process error; detached run returns +// the caller's context error after the command exits. +func TestPodmanRunSurvivesCallerCancellation(t *testing.T) { + dir := t.TempDir() + started := filepath.Join(dir, "started") + proceed := filepath.Join(dir, "go") + doneMarker := filepath.Join(dir, "done") + prog := filepath.Join(dir, "podman-stub.sh") + script := fmt.Sprintf(`#!/bin/sh +if [ "$1" != "run" ]; then exit 91; fi +: > %q +while [ ! -e %q ]; do :; done +: > %q +`, started, proceed, doneMarker) + if err := os.WriteFile(prog, []byte(script), 0o700); err != nil { + t.Fatalf("writing stub: %v", err) + } + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + cli := &podmanExec{program: prog, timeout: 10 * time.Second} + result := make(chan error, 1) + go func() { result <- cli.run(ctx, []string{"run", "--detach", "image"}) }() + waitForPodmanMarker(t, started) + cancel() + if err := os.WriteFile(proceed, nil, 0o600); err != nil { + t.Fatalf("creating go marker: %v", err) + } + + var err error + select { + case err = <-result: + case <-time.After(10 * time.Second): + t.Fatal("podman run did not return after the stub was released") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("run error = %v, want context.Canceled", err) + } + if _, err := os.Stat(doneMarker); err != nil { + t.Errorf("stub did not finish after caller cancellation: %v", err) + } +} + +func TestPodmanRunTimeoutKillsHungProcess(t *testing.T) { + pidFile := filepath.Join(t.TempDir(), "child.pid") + prog := filepath.Join(t.TempDir(), "podman-stub.sh") + script := fmt.Sprintf("#!/bin/sh\necho $$ > %q\nexec sleep 30\n", pidFile) + if err := os.WriteFile(prog, []byte(script), 0o700); err != nil { + t.Fatalf("writing stub: %v", err) + } + cli := &podmanExec{program: prog, timeout: 100 * time.Millisecond} + result := make(chan error, 1) + go func() { result <- cli.run(t.Context(), []string{"run", "--detach", "image"}) }() + waitForPodmanMarker(t, pidFile) + data, err := os.ReadFile(pidFile) + if err != nil { + t.Fatalf("reading child pid: %v", err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + if err != nil { + t.Fatalf("parsing child pid %q: %v", data, err) + } + t.Cleanup(func() { + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + t.Errorf("killing recorded podman process %d: %v", pid, err) + } + }) + + select { + case err := <-result: + if err == nil { + t.Fatal("run returned nil after the command timeout") + } + case <-time.After(5 * time.Second): + t.Fatal("podman run did not return after its command timeout") + } + if err := syscall.Kill(pid, 0); !errors.Is(err, syscall.ESRCH) { + t.Fatalf("after run timeout, kill(%d, 0) = %v, want ESRCH", pid, err) + } +} + +func waitForPodmanMarker(t *testing.T, path string) { + t.Helper() + timer := time.NewTimer(10 * time.Second) + defer timer.Stop() + ticker := time.NewTicker(5 * time.Millisecond) + defer ticker.Stop() + for { + if _, err := os.Stat(path); err == nil { + return + } else if !os.IsNotExist(err) { + t.Fatalf("checking marker %q: %v", path, err) + } + select { + case <-timer.C: + t.Fatalf("marker %q was not created before the deadline", path) + case <-ticker.C: + } + } +} From 9bf061b2c9345ff97558dc9f7ca0bd09c88c97d9 Mon Sep 17 00:00:00 2001 From: mintaka Date: Thu, 15 Oct 2026 03:50:27 -0400 Subject: [PATCH 2/2] fix(runtime): return podman start/run's real outcome under cancellation (RIG-5334) Review fix: a run that succeeded after caller cancellation was reported as cancelled, so the stack dropped a live container from its teardown record. Return the command's own result; the caller's next ctx check still unwinds. Cancellation tests now prove the stub is alive while gated, so a regressed CommandContext kill fails deterministically. Co-authored-by: Matt Wilkinson --- go/internal/runtime/clispawn.go | 3 -- go/internal/runtime/podman_test.go | 46 ++++++++----------- .../stack/adapters/postgres_container.go | 3 -- .../stack/adapters/postgres_container_test.go | 43 +++++++++++++---- 4 files changed, 53 insertions(+), 42 deletions(-) diff --git a/go/internal/runtime/clispawn.go b/go/internal/runtime/clispawn.go index 58975bb8b..b5403cad3 100644 --- a/go/internal/runtime/clispawn.go +++ b/go/internal/runtime/clispawn.go @@ -64,9 +64,6 @@ func (e cliEngine) spawnCaptureWithCancellation(ctx context.Context, summary str } runErr := cmd.Run() - if detached && ctx.Err() != nil { - return nil, nil, 0, ctx.Err() - } if runErr != nil { switch { case cctx.Err() == context.DeadlineExceeded && ctx.Err() == nil: diff --git a/go/internal/runtime/podman_test.go b/go/internal/runtime/podman_test.go index 63e56623b..fcdbaaa6f 100644 --- a/go/internal/runtime/podman_test.go +++ b/go/internal/runtime/podman_test.go @@ -630,6 +630,8 @@ esac } } +// A cancelled caller must not kill podman start: a signal mid netns setup +// leaves pasta running without resolv.conf. The real outcome is returned. func TestPodmanStartSurvivesCallerCancellation(t *testing.T) { dir := t.TempDir() started := filepath.Join(dir, "started") @@ -638,34 +640,44 @@ func TestPodmanStartSurvivesCallerCancellation(t *testing.T) { prog := filepath.Join(dir, "podman-stub.sh") script := fmt.Sprintf(`#!/bin/sh if [ "$1" != "start" ]; then exit 91; fi -: > %q +echo $$ > %q while [ ! -e %q ]; do :; done : > %q `, started, proceed, doneMarker) if err := os.WriteFile(prog, []byte(script), 0o755); err != nil { t.Fatalf("writing stub: %v", err) } + t.Cleanup(func() { killRecordedPid(t, started) }) ctx, cancel := context.WithCancel(t.Context()) defer cancel() cli := NewPodmanCLI().WithProgram(prog) result := make(chan error, 1) go func() { result <- cli.Start(ctx, WorkloadID("container")) }() - waitForMarker(t, started) + pid := waitForRecordedPid(t, started) cancel() + // A cancelled exec.CommandContext kills at once; the stub stays gated, so + // any return here is the kill. The bound can only miss a regression. + select { + case err := <-result: + t.Fatalf("Start returned %v while the stub was still gated: cancellation killed podman", err) + case <-time.After(200 * time.Millisecond): + } + if err := syscall.Kill(pid, 0); err != nil { + t.Fatalf("stub pid %d after cancellation: kill(0) = %v, want alive", pid, err) + } if err := os.WriteFile(proceed, nil, 0o600); err != nil { t.Fatalf("creating go marker: %v", err) } - var err error select { - case err = <-result: + case err := <-result: + if err != nil { + t.Errorf("Start error = %v, want nil (the stub exited 0)", err) + } case <-time.After(10 * time.Second): t.Fatal("podman start did not return after the stub was released") } - if !errors.Is(err, context.Canceled) { - t.Errorf("Start error = %v, want context.Canceled", err) - } if _, err := os.Stat(doneMarker); err != nil { t.Errorf("stub did not finish after caller cancellation: %v", err) } @@ -696,23 +708,3 @@ func TestPodmanStartTimeoutKillsHungProcess(t *testing.T) { t.Fatalf("after start timeout, kill(%d, 0) = %v, want ESRCH", pid, err) } } - -func waitForMarker(t *testing.T, path string) { - t.Helper() - timer := time.NewTimer(10 * time.Second) - defer timer.Stop() - ticker := time.NewTicker(5 * time.Millisecond) - defer ticker.Stop() - for { - if _, err := os.Stat(path); err == nil { - return - } else if !os.IsNotExist(err) { - t.Fatalf("checking marker %q: %v", path, err) - } - select { - case <-timer.C: - t.Fatalf("marker %q was not created before the deadline", path) - case <-ticker.C: - } - } -} diff --git a/go/internal/stack/adapters/postgres_container.go b/go/internal/stack/adapters/postgres_container.go index fce37f038..a1fdd200c 100644 --- a/go/internal/stack/adapters/postgres_container.go +++ b/go/internal/stack/adapters/postgres_container.go @@ -404,9 +404,6 @@ func (e *podmanExec) fireAndCheckWithCancellation(ctx context.Context, args []st var stderr strings.Builder cmd.Stderr = &stderr runErr := cmd.Run() - if detached && ctx.Err() != nil { - return ctx.Err() - } if runErr != nil { msg := strings.TrimSpace(stderr.String()) if msg == "" { diff --git a/go/internal/stack/adapters/postgres_container_test.go b/go/internal/stack/adapters/postgres_container_test.go index bdbf0cfed..c1428f889 100644 --- a/go/internal/stack/adapters/postgres_container_test.go +++ b/go/internal/stack/adapters/postgres_container_test.go @@ -354,8 +354,9 @@ func TestStopSecondsRounding(t *testing.T) { } } -// fireAndCheck previously surfaced a killed-process error; detached run returns -// the caller's context error after the command exits. +// A cancelled caller must not kill podman run: a signal mid netns setup leaves +// pasta running without resolv.conf. The real outcome is returned, so a +// container that did start is still recorded for teardown. func TestPodmanRunSurvivesCallerCancellation(t *testing.T) { dir := t.TempDir() started := filepath.Join(dir, "started") @@ -364,10 +365,10 @@ func TestPodmanRunSurvivesCallerCancellation(t *testing.T) { prog := filepath.Join(dir, "podman-stub.sh") script := fmt.Sprintf(`#!/bin/sh if [ "$1" != "run" ]; then exit 91; fi -: > %q +echo $$ > %q.tmp && mv %q.tmp %q while [ ! -e %q ]; do :; done : > %q -`, started, proceed, doneMarker) +`, started, started, started, proceed, doneMarker) if err := os.WriteFile(prog, []byte(script), 0o700); err != nil { t.Fatalf("writing stub: %v", err) } @@ -378,25 +379,49 @@ while [ ! -e %q ]; do :; done result := make(chan error, 1) go func() { result <- cli.run(ctx, []string{"run", "--detach", "image"}) }() waitForPodmanMarker(t, started) + pid := readPid(t, started) + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) cancel() + // A cancelled exec.CommandContext kills at once; the stub stays gated, so + // any return here is the kill. The bound can only miss a regression. + select { + case err := <-result: + t.Fatalf("run returned %v while the stub was still gated: cancellation killed podman", err) + case <-time.After(200 * time.Millisecond): + } + if err := syscall.Kill(pid, 0); err != nil { + t.Fatalf("stub pid %d after cancellation: kill(0) = %v, want alive", pid, err) + } if err := os.WriteFile(proceed, nil, 0o600); err != nil { t.Fatalf("creating go marker: %v", err) } - var err error select { - case err = <-result: + case err := <-result: + if err != nil { + t.Errorf("run error = %v, want nil (the stub exited 0)", err) + } case <-time.After(10 * time.Second): t.Fatal("podman run did not return after the stub was released") } - if !errors.Is(err, context.Canceled) { - t.Errorf("run error = %v, want context.Canceled", err) - } if _, err := os.Stat(doneMarker); err != nil { t.Errorf("stub did not finish after caller cancellation: %v", err) } } +func readPid(t *testing.T, path string) int { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading pid file: %v", err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + if err != nil { + t.Fatalf("parsing pid %q: %v", data, err) + } + return pid +} + func TestPodmanRunTimeoutKillsHungProcess(t *testing.T) { pidFile := filepath.Join(t.TempDir(), "child.pid") prog := filepath.Join(t.TempDir(), "podman-stub.sh")