fix(runtime): don't cancel podman start/run mid netns setup (RIG-5334)

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,8 @@ 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 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 +84,14 @@ func (e cliEngine) spawnCapture(ctx context.Context, summary string, args []stri
 	return out.Bytes(), errBuf.Bytes(), 0, nil
 }
 
-// run runs `<program> <args>`, 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 +105,12 @@ func (e cliEngine) run(ctx context.Context, summary string, args []string) ([]by
 	return stdout, nil
 }
 
+// run runs `<program> <args>`, 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.

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
 }
 

go/internal/runtime/podman_test.go

@@ -629,3 +629,82 @@ 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")
+	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
+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")) }()
+	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)
+	}
+
+	select {
+	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 _, 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)
+	}
+}

go/internal/stack/adapters/postgres_container.go

@@ -292,9 +292,11 @@ func newPodmanExec() *podmanExec {
 	return &podmanExec{program: "podman", timeout: containerCommandTimeout}
 }
 
-// run runs `podman <args>` to completion, requiring a zero exit.
+// run runs `podman <args>` 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 <name>`). A
@@ -387,18 +389,27 @@ func (e *podmanExec) exists(ctx context.Context, name string) (bool, error) {
 // fireAndCheck runs `podman <args>` 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 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
 }

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,129 @@ func TestStopSecondsRounding(t *testing.T) {
 		}
 	}
 }
+
+// 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")
+	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
+echo $$ > %q.tmp && mv %q.tmp %q
+while [ ! -e %q ]; do :; done
+: > %q
+`, started, started, 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)
+	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)
+	}
+
+	select {
+	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 _, 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")
+	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:
+		}
+	}
+}