diff --git a/CHANGELOG.md b/CHANGELOG.md index 67c498a0..7af943a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Adds HTTP request-body inspection to Keep policies. File- and pack-based `networ - **Copilot CLI settings passthrough** — `moat copilot` now carries over user preferences from the host's Copilot settings file (`$COPILOT_HOME/settings.json` when set, otherwise `~/.copilot/settings.json`; contextTier, effortLevel, footer, includeCoAuthoredBy, model, mouse, subagents, tabs, theme). Legacy `colorMode` values are written as the current `theme` setting. An optional `~/.moat/copilot/settings.json` provides moat-specific overrides that win over host settings. Settings that execute commands (`statusLine`) are only allowed from the moat override file. CLI flags and `moat.yaml` fields take precedence over settings.json values. ([#438](https://github.com/majorcontext/moat/pull/438)) - **GitHub Copilot CLI agent** — run GitHub Copilot CLI with `moat copilot`. Copilot uses the existing `github` grant: Moat injects that GitHub token for GitHub/Copilot API hosts plus HTTPS git, while the container receives only placeholders. `moat copilot` installs `@github/copilot`, stages Copilot config/context, passes `--allow-all` by default, and supports `copilot.model`, `copilot.context`, `copilot.reasoning_effort`, `copilot.experimental`, and `copilot.autopilot` in `moat.yaml`. See [Running GitHub Copilot CLI](https://majorcontext.com/moat/guides/copilot). ([#436](https://github.com/majorcontext/moat/pull/436)) +- **Podman support** — moat's Docker runtime now works against Podman's Docker-API-compatible socket. Podman machine sockets (macOS) and rootless/rootful sockets (Linux) are auto-detected like Rancher Desktop's, when the default Docker socket is unreachable and `DOCKER_HOST` is unset; `--runtime podman` / `MOAT_RUNTIME=podman` / `runtime: podman` force it. Each run's actual engine is now recorded (never guessed) and shown in `moat list` and `moat status` as `docker (podman)`; `moat doctor` instead separately reports the engine behind whatever endpoint it can currently reach (a live probe, not a per-run lookup), and no longer trusts Podman's gVisor claim outright there, since its compat API lists configured OCI runtimes even when they aren't installed. A run whose engine can't be reached can be torn down with the new `moat destroy --force-running`. See [Installation](https://majorcontext.com/moat/getting-started/installation) for version and platform requirements. ([#444](https://github.com/majorcontext/moat/pull/444)) - **Pi packages & safe defaults** — declare Pi extensions/skills/themes in `pi.packages` (remote `npm:`/`git:`/`https:`/`ssh:` sources) and Moat installs them into the image at build time via `pi install`, baked into a reproducible cached layer. Every `moat pi` image also bakes a safe `~/.pi/agent/settings.json` — `defaultProjectTrust: never` (a checked-out repo's own `.pi/` extensions, which are arbitrary code, do not auto-load), telemetry off, quiet startup — that a workspace cannot override. Because Pi config can redirect model traffic to any host, `moat pi` now warns under a permissive network policy (only `network.policy: strict` truly constrains egress). See [Running Pi](https://majorcontext.com/moat/guides/pi). ([#434](https://github.com/majorcontext/moat/pull/434)) - **Pi coding agent** — run the [Pi coding agent](https://github.com/earendil-works/pi) with `moat pi`. Pi has no credential of its own; it runs against your existing `anthropic` or `openai` grant. When exactly one is configured it is used automatically; when both are, choose one with `--provider` or `pi.provider` in `moat.yaml`. Only the `anthropic` and `openai` backends are supported today — any other backend, or a missing/ambiguous grant, fails before a container is created. Configure with the `pi:` block (`provider`, `model`). See [Running Pi](https://majorcontext.com/moat/guides/pi) and `examples/agent-pi`. ([#433](https://github.com/majorcontext/moat/pull/433)) - **`opentofu` and `terragrunt` dependencies** — two new managed cloud tools. `opentofu` installs the OpenTofu CLI as the `tofu` command; `terragrunt` installs the Terragrunt orchestration wrapper. Both install as prebuilt release binaries with no image rebuild cost beyond their own layer. Terragrunt delegates to a Terraform or OpenTofu binary on `PATH`, so pair it with an engine — `dependencies: [terraform, terragrunt]`, or `dependencies: [opentofu, terragrunt]` with `env.TERRAGRUNT_TFPATH: tofu`. See [Dependencies](https://majorcontext.com/moat/reference/dependencies). ([#430](https://github.com/majorcontext/moat/pull/430)) @@ -25,6 +26,7 @@ Adds HTTP request-body inspection to Keep policies. File- and pack-based `networ ### Fixed - Fix all Claude sessions inside moat freezing at once on macOS during a package install — previously, the shared credential-injecting proxy daemon inherited the host's default `RLIMIT_NOFILE` soft limit (typically 256 on macOS), so a burst of concurrent connections through the single proxy (`bun install` opens up to 64 parallel connections) could exhaust its file descriptors and stall every run's `claude.ai` traffic until the burst cleared, then recover. The daemon now raises its file-descriptor soft limit toward 65536 (capped at the hard limit) at startup, before the proxy accepts connections. ([#439](https://github.com/majorcontext/moat/pull/439)) +- Fix `moat stop` silently orphaning a container when the wrong engine is selected — previously, a not-found from the resolved engine only logged a warning and still marked the run stopped, so a run stopped under a different `MOAT_RUNTIME`/`DOCKER_HOST` could be recorded stopped while its container kept running. `moat stop` now fails loudly only when the run has no recorded engine endpoint *and* a different, live Podman endpoint is reachable, where the container could genuinely still be running. A genuinely gone container (pruned, rebooted, `--rm`'d), or a host where Podman is the sole engine, still stops cleanly with no error. The run stays in its prior state; retry with the original runtime, or `moat destroy --force-running`. ([#444](https://github.com/majorcontext/moat/pull/444)) - Fix the injected agent context misrepresenting network access — previously, the "Moat Environment" instructions file (CLAUDE.md/AGENTS.md) listed grant/rule hosts under "Allowed hosts" regardless of policy, so under the default `permissive` policy (where all outbound traffic is allowed) it read as an egress allowlist restricting the agent to those few hosts. The Network Policy section is now policy-aware: `permissive` states that all outbound access is allowed (surfacing only explicit per-path rules, which still apply), and `strict` is described as the allowlist it actually is. The context also now reflects **Docker/DIND availability**, the resolved **workspace mode** (bind vs. ephemeral `volume`), and **installed tool dependencies** — all previously omitted, which could lead an agent to assume capabilities were absent. ([#431](https://github.com/majorcontext/moat/pull/431)) - Fix `moat logs -f` silently doing nothing — previously, follow mode printed a debug-log line ("not yet implemented", only visible with `--verbose`) and exited 0 as if it had streamed, so `-f` looked like it worked. moat now prints a visible notice that follow mode isn't supported yet and shows the current logs. ([#413](https://github.com/majorcontext/moat/pull/413)) - Fix non-deterministic Dockerfile generation causing spurious image rebuilds — previously, dependency `ENV` lines were emitted in random map order, so the generated Dockerfile changed between runs and missed Docker's layer cache. `ENV` keys are now sorted. ([#413](https://github.com/majorcontext/moat/pull/413)) diff --git a/README.md b/README.md index a8332422..8d1e4dc8 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ Or with Go: go install github.com/majorcontext/moat/cmd/moat@latest ``` -**Requirements:** Docker or Apple containers (macOS 15+ with Apple Silicon—auto-detected). +**Requirements:** Docker, Podman, or Apple containers (macOS 15+ with Apple Silicon—auto-detected). ## Quick start @@ -174,7 +174,7 @@ See the [CLI reference](docs/content/reference/01-cli.md) for all commands and f ## How it works -**Container runtimes**: Auto-detects Apple containers (macOS 15+, Apple Silicon) or Docker. +**Container runtimes**: Auto-detects Apple containers (macOS 15+, Apple Silicon), Docker, or a Docker-API-compatible engine like Podman. **Credential injection**: A TLS-intercepting proxy sits between the container and the internet. It inspects requests and injects `Authorization` headers for granted services. The proxy binds to localhost (Docker) or uses per-run token auth (Apple containers). diff --git a/cmd/moat/cli/clean.go b/cmd/moat/cli/clean.go index 8723f1e2..7b71c4f4 100644 --- a/cmd/moat/cli/clean.go +++ b/cmd/moat/cli/clean.go @@ -329,7 +329,7 @@ func cleanResources(cmd *cobra.Command, args []string) error { skippedCount++ continue } - if err := manager.Destroy(ctx, r.ID); err != nil { + if err := manager.Destroy(ctx, r.ID, false); err != nil { fmt.Printf("%s\n", ui.Red(fmt.Sprintf("error: %v", err))) failedCount++ continue diff --git a/cmd/moat/cli/destroy.go b/cmd/moat/cli/destroy.go index d25a35d2..864bb5d3 100644 --- a/cmd/moat/cli/destroy.go +++ b/cmd/moat/cli/destroy.go @@ -11,7 +11,10 @@ import ( "github.com/spf13/cobra" ) -var destroyForce bool +var ( + destroyForce bool + destroyForceRunning bool +) var destroyCmd = &cobra.Command{ Use: "destroy [run]", @@ -24,7 +27,11 @@ If a name matches multiple runs, you'll be prompted to confirm. For volume-mode runs, the workspace lives only in a Docker volume. Destroying such a run deletes that volume and loses all agent changes unless an extraction snapshot was captured first. The command refuses to destroy a volume-mode run -with no extraction snapshot; pass --force to override.`, +with no extraction snapshot; pass --force to override. + +A run that cannot be stopped cleanly — its container lives on an engine this +process cannot reach — can be torn down with --force-running, which skips the +running-state guard. The two flags are independent.`, Args: cobra.MaximumNArgs(1), RunE: destroyRun, } @@ -32,6 +39,7 @@ with no extraction snapshot; pass --force to override.`, func init() { rootCmd.AddCommand(destroyCmd) destroyCmd.Flags().BoolVarP(&destroyForce, "force", "f", false, "force destroy even if a volume-mode run has no extraction snapshot") + destroyCmd.Flags().BoolVar(&destroyForceRunning, "force-running", false, "destroy a run that is still running, without stopping it first") } // hasExtractionSnapshot reports whether the run has at least one snapshot that @@ -113,7 +121,7 @@ func destroyRun(cmd *cobra.Command, args []string) error { continue } - if err := manager.Destroy(ctx, runID); err != nil { + if err := manager.Destroy(ctx, runID, destroyForceRunning); err != nil { return fmt.Errorf("destroying run %s: %w", runID, err) } diff --git a/cmd/moat/cli/destroy_test.go b/cmd/moat/cli/destroy_test.go index fa344650..4e883503 100644 --- a/cmd/moat/cli/destroy_test.go +++ b/cmd/moat/cli/destroy_test.go @@ -82,3 +82,52 @@ func writeRunSnapshots(t *testing.T, runID string, metas []snapshot.Metadata) { t.Fatalf("write snapshots.json: %v", err) } } + +// TestDestroyForceFlagsAreIndependent pins the split between --force (skip the +// volume-mode extraction-snapshot guard) and --force-running (tear down a run +// that is still running). They were briefly the same flag, which meant anyone +// passing --force for the data-loss guard silently also lost the running-run +// guard. +func TestDestroyForceFlagsAreIndependent(t *testing.T) { + flags := destroyCmd.Flags() + + force := flags.Lookup("force") + if force == nil { + t.Fatal("--force should still exist") + } + forceRunning := flags.Lookup("force-running") + if forceRunning == nil { + t.Fatal("--force-running should exist as its own flag") + } + if forceRunning.Shorthand != "" { + t.Errorf("--force-running should not take a shorthand, got -%s", forceRunning.Shorthand) + } + + // Each flag must move only its own variable. + t.Cleanup(func() { + destroyForce, destroyForceRunning = false, false + _ = flags.Set("force", "false") + _ = flags.Set("force-running", "false") + }) + + if err := flags.Set("force", "true"); err != nil { + t.Fatalf("set --force: %v", err) + } + if !destroyForce { + t.Error("--force should set destroyForce") + } + if destroyForceRunning { + t.Error("--force must NOT grant running-run teardown") + } + + destroyForce = false + if err := flags.Set("force-running", "true"); err != nil { + t.Fatalf("set --force-running: %v", err) + } + if !destroyForceRunning { + t.Error("--force-running should set destroyForceRunning") + } + if destroyForce { + t.Error("--force-running must NOT skip the extraction-snapshot guard") + } +} diff --git a/cmd/moat/cli/doctor.go b/cmd/moat/cli/doctor.go index d0333c9d..5806dd7b 100644 --- a/cmd/moat/cli/doctor.go +++ b/cmd/moat/cli/doctor.go @@ -104,6 +104,7 @@ func (s *containerSection) Print(w io.Writer) error { // Check which runtimes are available var runtimes []string var dockerRT *container.DockerRuntime + identity := engineUnknown // Check Docker if rt, err := container.NewDockerRuntime(false); err == nil { @@ -112,7 +113,22 @@ func (s *containerSection) Print(w io.Writer) error { if defaultRT.Type() == container.RuntimeDocker { marker = " (default)" } - runtimes = append(runtimes, "docker"+marker) + + // Client creation doesn't dial, so ping before trusting IsPodmanEngine. + // On timeout or error identity stays engineUnknown — never fail open. + pingCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + if rt.Ping(pingCtx) == nil { + if isPodman, err := rt.IsPodmanEngine(pingCtx); err == nil { + if isPodman { + identity = enginePodman + } else { + identity = engineDocker + } + } + } + cancel() + + runtimes = append(runtimes, dockerRuntimeEntry(marker, identity)) } // Check Apple Containers @@ -131,14 +147,16 @@ func (s *containerSection) Print(w io.Writer) error { fmt.Fprintln(tw, "Available:\tnone") } + // Surface an idle podman socket when the engine isn't already confirmed + // podman — a machine may be running alongside a real Docker daemon. + if line := podmanSocketLine(identity, container.PodmanSocketPaths()); line != "" { + fmt.Fprintf(tw, "Podman:\t%s\n", line) + } + // Check for Docker-specific features if dockerRT != nil { // Check gVisor - if hasGVisor() { - fmt.Fprintf(tw, "gVisor:\t%s available\n", ui.OKTag()) - } else { - fmt.Fprintf(tw, "gVisor:\t%s not available\n", ui.Dim("—")) - } + fmt.Fprintf(tw, "gVisor:\t%s\n", gvisorLine(identity, hasGVisor())) // Check BuildKit buildkit := os.Getenv("DOCKER_BUILDKIT") @@ -467,6 +485,56 @@ func (s *storageSection) Print(w io.Writer) error { return tw.Flush() } +// engineIdentity is what doctor could confirm about the engine behind the +// Docker-API client. A failed ping or IsPodmanEngine error leaves it +// engineUnknown, which must be treated as untrusted rather than real Docker. +type engineIdentity int + +const ( + engineUnknown engineIdentity = iota + engineDocker + enginePodman +) + +// dockerRuntimeEntry formats the "Available:" entry for the Docker runtime, +// labeling it when the engine is confirmed podman. marker is appended as-is +// (e.g. " (default)"). engineUnknown renders as plain docker — the label must +// not speculate about an identity doctor never confirmed. +func dockerRuntimeEntry(marker string, identity engineIdentity) string { + label := "docker" + if identity == enginePodman { + label = "docker (podman)" + } + return label + marker +} + +// gvisorLine formats the doctor "gVisor:" status line. Podman's compat /info +// lists every OCI runtime in containers.conf — runsc included — whether or not +// it's installed, so a reported runsc can't be trusted from podman, nor from +// an unidentified engine that may well be podman. reported is raw hasGVisor(). +func gvisorLine(identity engineIdentity, reported bool) string { + switch { + case !reported: + return ui.Dim("—") + " not available" + case identity == enginePodman: + return ui.WarnTag() + " reported by engine — unverified (podman lists configured OCI runtimes even when not installed)" + case identity == engineUnknown: + return ui.WarnTag() + " reported — engine identity unverified (daemon did not respond to ping)" + default: + return ui.OKTag() + " available" + } +} + +// podmanSocketLine formats the doctor "Podman:" status line, or "" when it +// should be suppressed — for enginePodman the "Available:" line already says +// so. sockets is the stat-only result of container.PodmanSocketPaths(). +func podmanSocketLine(identity engineIdentity, sockets []string) string { + if identity == enginePodman || len(sockets) == 0 { + return "" + } + return fmt.Sprintf("socket found at %s — use --runtime podman", strings.Join(sockets, ", ")) +} + // hasBuildx checks if docker buildx is available func hasBuildx() bool { cmd := exec.Command("docker", "buildx", "version") diff --git a/cmd/moat/cli/doctor_test.go b/cmd/moat/cli/doctor_test.go index cf75d5a3..a852a4f9 100644 --- a/cmd/moat/cli/doctor_test.go +++ b/cmd/moat/cli/doctor_test.go @@ -113,6 +113,82 @@ func TestGetTokenPrefix(t *testing.T) { } } +func TestDockerRuntimeEntry(t *testing.T) { + tests := []struct { + name string + marker string + identity engineIdentity + expected string + }{ + {"confirmed docker, not default", "", engineDocker, "docker"}, + {"confirmed docker, default", " (default)", engineDocker, "docker (default)"}, + {"confirmed podman, not default", "", enginePodman, "docker (podman)"}, + {"confirmed podman, default", " (default)", enginePodman, "docker (podman) (default)"}, + {"unknown identity, not default", "", engineUnknown, "docker"}, + {"unknown identity, default", " (default)", engineUnknown, "docker (default)"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := dockerRuntimeEntry(tt.marker, tt.identity) + if result != tt.expected { + t.Errorf("dockerRuntimeEntry(%q, %v) = %q, want %q", tt.marker, tt.identity, result, tt.expected) + } + }) + } +} + +func TestGvisorLine(t *testing.T) { + tests := []struct { + name string + identity engineIdentity + reported bool + want string + }{ + {"confirmed docker, gVisor reported", engineDocker, true, "✓ available"}, + {"confirmed docker, gVisor not reported", engineDocker, false, "— not available"}, + {"confirmed podman, gVisor reported (untrustworthy)", enginePodman, true, "⚠ reported by engine — unverified (podman lists configured OCI runtimes even when not installed)"}, + {"confirmed podman, gVisor not even listed", enginePodman, false, "— not available"}, + {"unknown identity, gVisor reported", engineUnknown, true, "⚠ reported — engine identity unverified (daemon did not respond to ping)"}, + {"unknown identity, gVisor not reported", engineUnknown, false, "— not available"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := gvisorLine(tt.identity, tt.reported) + if result != tt.want { + t.Errorf("gvisorLine(%v, %v) = %q, want %q", tt.identity, tt.reported, result, tt.want) + } + }) + } +} + +func TestPodmanSocketLine(t *testing.T) { + tests := []struct { + name string + identity engineIdentity + sockets []string + want string + }{ + {"confirmed docker, socket present", engineDocker, []string{"/tmp/podman.sock"}, "socket found at /tmp/podman.sock — use --runtime podman"}, + {"confirmed docker, no socket", engineDocker, nil, ""}, + {"unknown identity, socket present", engineUnknown, []string{"/tmp/podman.sock"}, "socket found at /tmp/podman.sock — use --runtime podman"}, + {"unknown identity, no socket", engineUnknown, nil, ""}, + {"confirmed podman, socket present (suppressed — Available already says podman)", enginePodman, []string{"/tmp/podman.sock"}, ""}, + {"confirmed podman, no socket", enginePodman, nil, ""}, + {"confirmed docker, multiple sockets", engineDocker, []string{"/tmp/a.sock", "/tmp/b.sock"}, "socket found at /tmp/a.sock, /tmp/b.sock — use --runtime podman"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := podmanSocketLine(tt.identity, tt.sockets) + if result != tt.want { + t.Errorf("podmanSocketLine(%v, %v) = %q, want %q", tt.identity, tt.sockets, result, tt.want) + } + }) + } +} + func TestPrintClaims(t *testing.T) { claims := map[string]interface{}{ "exp": float64(1735689600), // Fixed timestamp diff --git a/cmd/moat/cli/helpers.go b/cmd/moat/cli/helpers.go index 654471dc..c443cca0 100644 --- a/cmd/moat/cli/helpers.go +++ b/cmd/moat/cli/helpers.go @@ -22,6 +22,23 @@ func parseEnvFlags(envFlags []string, cfg *config.Config) error { return intcli.ParseEnvFlags(envFlags, cfg) } +// runtimeDisplayLabel formats the runtime column in `moat list` and `moat +// status`. Podman runs record their type as "docker", so a recorded engine +// of "podman" is labeled "docker (podman)" to match `moat doctor`. engine is +// the value recorded at run creation (storage.Metadata.Engine / run.Run.Engine) +// — never guessed here. A legacy run persisted before that field existed has +// engine == "" and renders as plain "docker": empty means "unknown", not +// "not podman", so it must not fall back to sniffing DockerHost. +func runtimeDisplayLabel(runtime, engine string) string { + if runtime == "" { + return "-" + } + if runtime == "docker" && engine == "podman" { + return "docker (podman)" + } + return runtime +} + // shortenPath shortens a path for display, using ~ for home directory. func shortenPath(path string) string { home, err := os.UserHomeDir() diff --git a/cmd/moat/cli/helpers_test.go b/cmd/moat/cli/helpers_test.go index 7b54a84c..a4279758 100644 --- a/cmd/moat/cli/helpers_test.go +++ b/cmd/moat/cli/helpers_test.go @@ -327,3 +327,35 @@ func TestFormatTimeAgo(t *testing.T) { }) } } + +func TestRuntimeDisplayLabel(t *testing.T) { + tests := []struct { + name string + runtime string + engine string + want string + }{ + // Podman is surfaced only from the recorded engine identity. + {"recorded podman engine", "docker", "podman", "docker (podman)"}, + // Companion: a docker run keeps reading "docker". + {"recorded docker engine", "docker", "docker", "docker"}, + // Legacy run persisted before Engine existed: Engine == "" MUST render + // as plain "docker", never guessed from anything else (there is no + // dockerHost parameter anymore — this is the whole point of D2). + {"legacy run, no recorded engine", "docker", "", "docker"}, + // Other runtimes and the empty legacy case are untouched. + {"apple", "apple", "", "apple"}, + {"empty runtime", "", "", "-"}, + // Guard: a "podman" engine value is only honored for the docker runtime. + {"apple with stray engine value", "apple", "podman", "apple"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := runtimeDisplayLabel(tt.runtime, tt.engine) + if got != tt.want { + t.Errorf("runtimeDisplayLabel(%q, %q) = %q, want %q", tt.runtime, tt.engine, got, tt.want) + } + }) + } +} diff --git a/cmd/moat/cli/list.go b/cmd/moat/cli/list.go index 47e3997e..0ed4ca57 100644 --- a/cmd/moat/cli/list.go +++ b/cmd/moat/cli/list.go @@ -82,10 +82,7 @@ func listRuns(cmd *cobra.Command, args []string) error { sort.Strings(names) endpoints = strings.Join(names, ", ") } - rtLabel := r.Runtime - if rtLabel == "" { - rtLabel = "-" - } + rtLabel := runtimeDisplayLabel(r.Runtime, r.Engine) if hasWorktree { wt := "" if r.WorktreeBranch != "" { diff --git a/cmd/moat/cli/status.go b/cmd/moat/cli/status.go index 77174a18..b5199caf 100644 --- a/cmd/moat/cli/status.go +++ b/cmd/moat/cli/status.go @@ -89,11 +89,18 @@ func showStatus(cmd *cobra.Command, args []string) error { return runs[i].CreatedAt.After(runs[j].CreatedAt) }) - // Get images and runtime names from all available runtimes + // Get images and runtime names from all available runtimes. A podman + // engine reached for a host-pinned run reports Type() "docker" just like a + // real Docker daemon, so dedupe by type to avoid listing "docker" twice in + // the Runtimes header (the per-run RUNTIME column distinguishes the engine). var images []imageInfo var runtimeNames []string + seenRuntime := make(map[string]bool) if err := pool.ForEachAvailable(func(rt container.Runtime) error { - runtimeNames = append(runtimeNames, string(rt.Type())) + if name := string(rt.Type()); !seenRuntime[name] { + seenRuntime[name] = true + runtimeNames = append(runtimeNames, name) + } rtImages, err := rt.ListImages(ctx) if err != nil { log.Debug("listing images failed", "runtime", rt.Type(), "error", err) @@ -163,7 +170,7 @@ func showStatus(cmd *cobra.Command, args []string) error { output.ActiveRuns = append(output.ActiveRuns, runInfo{ Name: r.Name, ID: r.ID, - Runtime: r.Runtime, + Runtime: runtimeDisplayLabel(r.Runtime, r.Engine), State: string(r.GetState()), Age: age, DiskMB: diskMB, diff --git a/docs/content/concepts/07-runtimes.md b/docs/content/concepts/07-runtimes.md index 7261c939..c63ba2e8 100644 --- a/docs/content/concepts/07-runtimes.md +++ b/docs/content/concepts/07-runtimes.md @@ -1,13 +1,13 @@ --- title: "Container runtimes" navTitle: "Runtimes" -description: "Docker, Apple containers, and gVisor sandbox configuration." -keywords: ["moat", "runtime", "docker", "apple containers", "gvisor", "sandbox"] +description: "Docker, Podman, Apple containers, and gVisor sandbox configuration." +keywords: ["moat", "runtime", "docker", "podman", "apple containers", "gvisor", "sandbox"] --- # Container runtimes -Moat runs agents in isolated containers using either Docker or Apple containers. This page explains how runtime detection works, the security model for each runtime, and how to configure sandboxing. +Moat runs agents in isolated containers using Docker (or a Docker-API-compatible engine such as Podman) or Apple containers. This page explains how runtime detection works, the security model for each runtime, and how to configure sandboxing. ## Runtime detection @@ -17,9 +17,9 @@ Moat detects the available runtime automatically: 2. If Apple containers are unavailable, it uses Docker 3. On Linux and Windows, it uses Docker -If the default Docker socket is unreachable and `DOCKER_HOST` is not set, Moat checks known alternative socket locations before returning an error. +If the default Docker socket is unreachable and `DOCKER_HOST` is not set, Moat checks known alternative socket locations before returning an error: Podman machine sockets on macOS (`$TMPDIR/podman/*-api.sock`) and rootless/rootful Podman sockets on Linux (`$XDG_RUNTIME_DIR/podman/podman.sock`, `/run/podman/podman.sock`). **This fallback probe has no Windows candidates**, so a Podman endpoint on Windows is never auto-detected. Setting `DOCKER_HOST` to a Podman `npipe://` endpoint yourself (with `--runtime podman` / `MOAT_RUNTIME=podman` to also assert engine identity) isn't blocked by anything Windows-specific in Moat's own code, but this configuration hasn't been verified. -The `MOAT_RUNTIME` environment variable overrides automatic detection, forcing either `docker` or `apple`. If the requested runtime is unavailable, Moat returns an error. +The `MOAT_RUNTIME` environment variable overrides automatic detection, forcing `docker`, `podman`, or `apple`. `podman` selects the same Docker-API runtime as `docker`, pointed at a Podman socket — there is no separate Podman runtime implementation. If the requested runtime is unavailable, Moat returns an error. ## Docker runtime @@ -66,6 +66,16 @@ Since the proxy listens only on localhost, only processes on the host machine ca On macOS and Windows, Moat automatically uses standard mode. Apple containers (macOS 26+ with Apple Silicon) provide an alternative with native macOS isolation. +### Podman + +Podman exposes a Docker-API-compatible socket (podman machine on macOS, the native daemonless socket on Linux), and Moat's Docker runtime talks to it unmodified — set `DOCKER_HOST` or let auto-detection find it, or force it with `--runtime podman` / `MOAT_RUNTIME=podman`. See [Installation](../getting-started/02-installation.md#podman-macos-linux) for setup and [Troubleshooting](../reference/08-troubleshooting.md) for the gVisor false-positive caveat on Linux. + +**Why `podman` is a runtime value when it isn't a separate runtime.** Auto-detection only probes alternative sockets when the default Docker socket is *unreachable*, so on a machine running Docker and Podman side by side it will always pick Docker. Selecting Podman there otherwise means locating the machine socket yourself and exporting `DOCKER_HOST`. The `podman` value does two things that neither auto-detection nor `DOCKER_HOST` can: it asserts engine identity — Moat verifies the endpoint really answers as Podman and fails if it doesn't, where a mis-set `DOCKER_HOST` would silently run on Docker — and it produces actionable errors (how to start a machine) when no Podman socket answers. + +On macOS with several machines running, Moat probes the one Podman itself targets, honoring `CONTAINER_CONNECTION` and otherwise the default connection from `podman-connections.json`. + +**Engine identity is recorded, not guessed.** At run creation, Moat asks the connected daemon which engine it actually is and stores the answer (`docker` or `podman`) alongside the endpoint the run's containers live on. `moat list` and `moat status` display each run's recorded identity as `docker (podman)` for a Podman-backed run; a run created before this existed has no recorded identity and shows as plain `docker`. `moat doctor` is different: it has no per-run display at all, and instead separately reports the identity of the engine behind whatever endpoint it can currently reach — a live probe, not a lookup of any run's recorded value. The recorded endpoint is also what lets lifecycle commands (`moat stop`, `moat logs`, ...) reconnect to the right engine when Docker and Podman are both present. Moat does not warn when `MOAT_RUNTIME=docker` happens to land on a Podman endpoint — checking eagerly would cost every startup with `DOCKER_HOST` set a blocking round trip for a warning most would never see. Forcing `MOAT_RUNTIME=podman` is the only case that hard-errors on engine identity, and its hard-error isn't limited to a confirmed non-Podman engine: it also fires when a preset `DOCKER_HOST` is unreachable, when the engine behind it can't be identified, when a Podman socket is found but unusable, or when no Podman socket is found at all — in short, whenever a usable Podman engine cannot be reached. + ## Apple containers Apple containers require macOS 26+ (Tahoe) on Apple Silicon, with the `container` CLI installed from the [Apple container releases](https://github.com/apple/container/releases) page. They use macOS virtualization frameworks rather than Docker. diff --git a/docs/content/getting-started/02-installation.md b/docs/content/getting-started/02-installation.md index 54186c8a..2484b444 100644 --- a/docs/content/getting-started/02-installation.md +++ b/docs/content/getting-started/02-installation.md @@ -1,14 +1,14 @@ --- title: "Installation" -description: "Install Moat on macOS or Linux with Docker or Apple containers." -keywords: ["moat", "installation", "docker", "apple containers", "setup", "homebrew"] +description: "Install Moat on macOS or Linux with Docker, Podman, or Apple containers." +keywords: ["moat", "installation", "docker", "podman", "apple containers", "setup", "homebrew"] --- # Installation ## Requirements -- **Container runtime** -- Docker or Apple containers (macOS 26+ with Apple Silicon) +- **Container runtime** -- Docker, Podman, or Apple containers (macOS 26+ with Apple Silicon) ## Install Moat @@ -140,6 +140,91 @@ Runtime: docker ... ``` +### Podman (macOS, Linux) + +Moat's Docker runtime works unmodified against Podman's Docker-API-compatible socket -- there is no separate Podman runtime, just a different socket. + +**macOS (Homebrew):** + +```bash +brew install podman +podman machine init +podman machine start +``` + +Some Podman 6.x builds default to the `libkrun` machine provider on macOS (observed with podman 6.0.0; Homebrew's 6.0.1 came up with `applehv` directly). If `podman machine start` fails with `exec: "krunkit" not found`, the machine was created with the `libkrun` provider, which needs a separate `krunkit` binary. Recreate it with the `applehv` provider, which uses the `vfkit` binary bundled with the Homebrew formula: + +```bash +podman machine rm -f +CONTAINERS_MACHINE_PROVIDER=applehv podman machine init +podman machine start +``` + +**Linux (Debian/Ubuntu):** + +```bash +sudo apt-get update +sudo apt-get install podman +systemctl --user enable --now podman.socket +``` + +This starts the rootless Podman socket at `$XDG_RUNTIME_DIR/podman/podman.sock`. For a rootful socket, use `sudo systemctl enable --now podman.socket` instead (`/run/podman/podman.sock`). + +**Using it with Moat:** + +Runtime selection follows this precedence: + +1. **macOS 26+ on Apple Silicon:** Moat prefers Apple's `container` tool if it's available, before probing Docker or Podman at all. If Apple containers are running, auto-detection picks them -- it doesn't reach Podman. Use `--runtime podman` (or `MOAT_RUNTIME=podman`) to select Podman explicitly on these machines. +2. **Everywhere else (or when Apple containers aren't available):** Moat tries the default Docker socket first, then falls back to known alternative sockets -- including Podman's, the same way it detects Rancher Desktop's -- when the default socket is unreachable and `DOCKER_HOST` is unset. On a host without Apple containers, this fallback is how auto-detection reaches Podman. + + On macOS, the fallback probe matches the machine socket layout used by Podman 5.x and later (`$TMPDIR/podman/-api.sock`). Podman 4.x machines used a different socket location and aren't found by auto-detection -- set `DOCKER_HOST` directly (see below) to use one. + +To select Podman explicitly on any platform: + +```bash +moat run --runtime podman ... +# or +export MOAT_RUNTIME=podman +``` + +You can also point `DOCKER_HOST` directly at the socket (useful for scripting, non-default machine names, or forcing Podman ahead of Apple containers): + +```bash +export DOCKER_HOST=unix://$(podman machine inspect --format '{{.ConnectionInfo.PodmanSocket.Path}}') +``` + +Verify: + +```bash +$ moat run --runtime podman -- sh -c 'echo "$container"' +podman +``` + +Podman sets the `container=podman` environment variable inside every container it runs, so this confirms the workload actually ran under Podman. + +**Caveats:** + +- **gVisor false positive (Linux):** Podman's compatibility API reports `runsc` (and other OCI runtimes) as available whenever they're listed in `containers.conf`, even if not installed. Moat's Linux default requires gVisor; if the check passes spuriously, container creation fails. Either install `runsc` as a Podman OCI runtime, or run with `--no-sandbox` (or `MOAT_NO_SANDBOX=1`), which accepts reduced isolation. macOS has sandboxing off by default, so this doesn't apply there. +- **Custom base images** must default to the root user -- Moat's generated Dockerfile installs packages without a `USER root` escape. Rootless Podman's UID mapping (container root -> host user) doesn't change this requirement. +- **Versions and platforms.** + + | Platform | Podman version | Support | + |----------|-----------------|---------| + | Linux | below 4.7 | Not supported — on Linux Moat passes the `host-gateway` sentinel to `--add-host`, which Podman only accepts from 4.7.0 | + | Linux | 4.7+ | Full support, socket auto-detected (rootless and rootful) | + | macOS | 4.x | Expected to work, but the machine socket isn't auto-detected — set `DOCKER_HOST` yourself (see above). Not exercised | + | macOS | 5.x+ | Full support, machine socket auto-detected | + | macOS, arm64 | 6.0.0, 6.0.1 | The specific builds this support was developed and verified against; other versions meeting the floors above are expected to work but weren't exercised | + + The `host-gateway` floor is Linux-only: Moat emits that sentinel just for a + Docker-API runtime on a Linux host. On macOS it reaches the host through + `MOAT_EXTRA_HOSTS` instead, so `host-gateway` support is irrelevant there and + the macOS floor is set by the machine socket layout alone. + + On macOS with more than one machine running, auto-detection probes the machine Podman itself targets by reading `podman-connections.json` (a Podman 5+ file, at `$XDG_CONFIG_HOME/containers/` or `~/.config/containers/`). If that file is missing or unreadable, probing silently falls back to alphabetical machine-name order instead of matching the active connection -- harmless with a single machine, but worth knowing if you run several. + + Windows is not covered by this table: Podman auto-detection has no Windows candidates at all today. See [Container runtimes](../concepts/07-runtimes.md#runtime-detection). + ## GitHub authentication setup (optional) `moat grant github` automatically uses credentials from these sources (in order): diff --git a/docs/content/getting-started/03-quick-start.md b/docs/content/getting-started/03-quick-start.md index 6be5fedb..1548025f 100644 --- a/docs/content/getting-started/03-quick-start.md +++ b/docs/content/getting-started/03-quick-start.md @@ -10,7 +10,7 @@ This tutorial walks through running an agent with credential injection. By the e **Prerequisites:** - Moat installed ([Installation](./02-installation.md)) -- Docker running, or [Apple containers](./02-installation.md#apple-containers-macos-26-with-apple-silicon) installed (macOS 26+) +- Docker or [Podman](./02-installation.md#podman-macos-linux) running, or [Apple containers](./02-installation.md#apple-containers-macos-26-with-apple-silicon) installed (macOS 26+) - GitHub authentication (one of: `gh` CLI, `GITHUB_TOKEN` env var, or Personal Access Token) ## Step 1: Grant GitHub credentials diff --git a/docs/content/getting-started/04-comparison.md b/docs/content/getting-started/04-comparison.md index 88b8c957..dbd91048 100644 --- a/docs/content/getting-started/04-comparison.md +++ b/docs/content/getting-started/04-comparison.md @@ -22,7 +22,7 @@ These tools solve related problems. The right choice depends on what you're opti | **Network monitoring** | Yes | No | Yes | No | | **Audit logging** | Tamper-proof with hash chain | No | Event streaming | No | | **Policy language** | YAML (allow list) | None | Cedar | None | -| **Container runtime** | Docker, Apple containers | Docker | Docker, Podman, OrbStack | Docker | +| **Container runtime** | Docker, Podman, Apple containers | Docker | Docker, Podman, OrbStack | Docker | | **Setup complexity** | Low | Low | Medium | Low-Medium | | **macOS native** | Yes (Apple containers) | No | Experimental | No | @@ -137,12 +137,12 @@ Leash offers the most granular control. Moat provides network-level policies. pa | Tool | macOS (Intel) | macOS (Apple Silicon) | Linux | |------|--------------|----------------------|-------| -| **Moat** | Docker | Docker or Apple containers | Docker | +| **Moat** | Docker or Podman | Docker, Podman, or Apple containers | Docker or Podman | | **packnplay** | Docker | Docker | Docker | | **Leash** | Docker | Docker or native (experimental) | Docker + eBPF | | **Dev Containers** | Docker | Docker | Docker | -Moat's Apple container support on macOS 26+ provides native virtualization without Docker Desktop. Leash's macOS native mode is experimental and does not support credential injection. +Moat's Apple container support on macOS 26+ provides native virtualization without Docker Desktop. Podman support runs through Moat's existing Docker runtime against Podman's Docker-API-compatible socket -- see [Runtimes](../concepts/07-runtimes.md#podman). Leash's macOS native mode is experimental and does not support credential injection. ## Migration paths diff --git a/docs/content/reference/01-cli.md b/docs/content/reference/01-cli.md index bea40e56..3b8d6952 100644 --- a/docs/content/reference/01-cli.md +++ b/docs/content/reference/01-cli.md @@ -47,7 +47,7 @@ The agent commands (`moat claude`, `moat copilot`, `moat codex`, `moat gemini`, | `-n`, `--name NAME` | Run name (default: from `moat.yaml` or random) | | `--rebuild` | Force rebuild of container image | | `--allow-host HOST` | Additional hosts to allow network access to (repeatable) | -| `--runtime RUNTIME` | Container runtime to use (`apple`, `docker`) | +| `--runtime RUNTIME` | Container runtime to use (`apple`, `docker`, `podman`) | | `--keep` | Keep container after run completes | | `--workspace-mode bind\|volume` | Workspace mode: `bind` (default) or `volume` (isolated Docker named volume). Overrides `workspace.mode` in `moat.yaml`. Docker-only for `volume`. | | `--no-clipboard` | Disable host clipboard bridging for this run | @@ -127,7 +127,7 @@ moat run [flags] [path] [-- command] | `-m`, `--mount SOURCE:TARGET[:MODE]` | Additional mount (repeatable). See [Mounts reference](./05-mounts.md). | | `-i`, `--interactive` | Enable interactive mode (stdin + TTY) | | `--rebuild` | Force rebuild of container image | -| `--runtime RUNTIME` | Container runtime to use (apple, docker) | +| `--runtime RUNTIME` | Container runtime to use (apple, docker, podman) | | `--keep` | Keep container after run completes | | `--no-clipboard` | Disable host clipboard bridging for this run | | `--workspace-mode bind\|volume` | Workspace mode: `bind` (default) mounts the host directory at `/workspace`; `volume` copies it into an isolated Docker named volume. Overrides `workspace.mode` in `moat.yaml`. Docker-only for `volume`. | @@ -537,7 +537,7 @@ Configuration is read from `moat.yaml` in the repository root. If a run is alrea | `-e KEY=VALUE` | Set environment variable (repeatable) | | `--rebuild` | Force image rebuild | | `--keep` | Keep container after completion | -| `--runtime` | Container runtime to use (`apple`, `docker`) | +| `--runtime` | Container runtime to use (`apple`, `docker`, `podman`) | | `--no-clipboard` | Disable host clipboard bridging for this run | | `--no-sandbox` | Disable gVisor sandbox (Docker only) | | `--no-prompt` | Never prompt to grant missing credentials; fail instead. Also set via `MOAT_NO_PROMPT=1`. | @@ -1070,7 +1070,7 @@ moat list |--------|-------------| | NAME | Run name | | RUN ID | Unique identifier | -| RUNTIME | Container runtime (docker, apple) | +| RUNTIME | Container runtime (`docker`, `apple`, or `docker (podman)` for a run recorded against a Podman engine) | | STATE | running, stopped, failed | | AGE | Time since run was created | | WORKTREE | Branch name (appears when any run has a worktree) | @@ -1146,7 +1146,7 @@ moat status |--------|-------------| | NAME | Run name | | RUN ID | Unique run identifier | -| RUNTIME | Container runtime (docker or apple) | +| RUNTIME | Container runtime (`docker`, `apple`, or `docker (podman)` for a run recorded against a Podman engine) | | AGE | Time since run was created | | DISK | Disk usage in MB | | ENDPOINTS | Exposed services (from ports) | @@ -1315,11 +1315,14 @@ moat destroy [run] [flags] | Flag | Description | |------|-------------| | `-f`, `--force` | Destroy even if a volume-mode run has no extraction snapshot | +| `--force-running` | Destroy a run that is still running, without stopping it first | If a name matches multiple runs, you'll be prompted to confirm destroying all of them. For volume-mode runs, the workspace lives only in the Docker named volume. Destroying such a run without first capturing a snapshot permanently deletes the agent's work. The command refuses unless an extraction snapshot exists; pass `-f`/`--force` to override. +A run whose container lives on an engine this process can't reach can't be stopped cleanly. `--force-running` skips the running-state guard so such a run can still be torn down. The two flags are independent: `--force` never waives the running-run guard, and `--force-running` never waives the snapshot guard. + ### Examples ```bash @@ -1331,6 +1334,9 @@ moat destroy run_a1b2c3d4e5f6 # Destroy a volume-mode run even without an extraction snapshot moat destroy --force run_a1b2c3d4e5f6 + +# Tear down a run whose engine is unreachable, without stopping it first +moat destroy --force-running run_a1b2c3d4e5f6 ``` --- diff --git a/docs/content/reference/02-moat-yaml.md b/docs/content/reference/02-moat-yaml.md index 281851fd..7931f072 100644 --- a/docs/content/reference/02-moat-yaml.md +++ b/docs/content/reference/02-moat-yaml.md @@ -227,14 +227,14 @@ version: 1.0.0 ### runtime -Force a specific container runtime (Docker or Apple containers). +Force a specific container runtime (Docker, Apple containers, or Podman). ```yaml runtime: docker # Force Docker runtime ``` - Type: `string` -- Values: `docker` | `apple` +- Values: `docker` | `apple` | `podman` (`podman` runs the Docker runtime against a Podman socket) - Default: Auto-detected (Apple containers on macOS 26+ with Apple Silicon, Docker otherwise) - CLI override: `--runtime` diff --git a/docs/content/reference/03-environment.md b/docs/content/reference/03-environment.md index b158c5a2..c0af3d36 100644 --- a/docs/content/reference/03-environment.md +++ b/docs/content/reference/03-environment.md @@ -46,10 +46,12 @@ Force a specific container runtime instead of auto-detection. ```bash export MOAT_RUNTIME=docker # Force Docker runtime +export MOAT_RUNTIME=podman # Force Docker runtime over a Podman socket export MOAT_RUNTIME=apple # Force Apple containers runtime ``` -- Default: Auto-detect (Apple containers on macOS 26+ with Apple Silicon, Docker otherwise) +- Default: Auto-detect (Apple containers on macOS 26+ with Apple Silicon, Docker otherwise -- including Docker-API-compatible sockets like Podman machine or Rancher Desktop) +- `podman` is not a separate runtime implementation; it points the Docker runtime at a Podman socket - When the requested runtime is unavailable, Moat returns an error See [Runtimes](../concepts/07-runtimes.md) for details on runtime selection. diff --git a/docs/content/reference/08-troubleshooting.md b/docs/content/reference/08-troubleshooting.md index e1a05bc5..f4a7f7b5 100644 --- a/docs/content/reference/08-troubleshooting.md +++ b/docs/content/reference/08-troubleshooting.md @@ -285,13 +285,16 @@ To start Apple containers manually: To force a specific runtime: moat run --runtime apple moat run --runtime docker + moat run --runtime podman ``` -**Cause:** Neither Docker nor Apple containers are available. +**Cause:** Neither Docker, Podman, nor Apple containers are available. Moat probes the default Docker socket, then known alternative sockets (Rancher Desktop, Podman machine on macOS, rootless/rootful Podman on Linux) before failing. **Fix:** - **Docker:** Start Docker Desktop or the Docker daemon. +- **Podman (macOS):** `podman machine start` +- **Podman (Linux):** `systemctl --user enable --now podman.socket` (rootless) or `sudo systemctl enable --now podman.socket` (rootful) - **Apple containers (macOS 26+):** Start the container system: container system start @@ -353,6 +356,47 @@ gVisor (runsc) is required but not available > **Warning:** Running without gVisor reduces container isolation. +### gVisor check passes under Podman but the container still fails to start + +**Cause:** Podman's compatibility API (`/info`) lists OCI runtimes (`runsc`, `kata`, `krun`, `youki`, ...) that are configured in `containers.conf`, even if the binary isn't actually installed. Moat's gVisor availability check can pass against this list, then container creation fails because `runsc` doesn't actually exist. Moat cannot tell the two cases apart from the engine's report alone, so it warns once per run (`gVisor availability is engine-reported and unverified under podman...`) rather than trusting podman's claim outright. + +If container creation then fails, the error is annotated with the same diagnosis: + +``` +podman reported runsc as available but creating the container with it failed; +runsc is very likely not actually installed. Check with: podman machine ssh -- +which runsc (macOS) or which runsc (Linux). Use --no-sandbox or +MOAT_NO_SANDBOX=1 to bypass: ... +``` + +**Check whether runsc is actually installed:** + +- **macOS** (podman machine runs the OCI runtime inside its VM): + + podman machine ssh -- which runsc + +- **Linux** (podman runs directly on the host): + + which runsc + +**Fix:** Either install `runsc` as a real Podman OCI runtime, or bypass the sandbox requirement and accept reduced isolation: + + moat run --no-sandbox ./my-project + # or + export MOAT_NO_SANDBOX=1 + +> **Warning:** Running without gVisor reduces container isolation. This doesn't apply on macOS, where sandboxing is off by default regardless of runtime. + +### `podman machine start` fails with `exec: "krunkit" not found` + +**Cause:** The machine was created with the `libkrun` machine provider (the default in some Podman 6.x builds on macOS), which requires a separate `krunkit` binary that isn't installed. + +**Fix:** Recreate the machine with the `applehv` provider, which uses the `vfkit` binary bundled with the Homebrew `podman` formula: + + podman machine rm -f + CONTAINERS_MACHINE_PROVIDER=applehv podman machine init + podman machine start + ### `agent is already running` ``` diff --git a/internal/cli/types.go b/internal/cli/types.go index ccd06333..f9efe3c4 100644 --- a/internal/cli/types.go +++ b/internal/cli/types.go @@ -31,7 +31,7 @@ func AddExecFlags(cmd *cobra.Command, flags *ExecFlags) { cmd.Flags().StringVarP(&flags.Name, "name", "n", "", "name for this run (default: from moat.yaml or random)") cmd.Flags().BoolVar(&flags.Rebuild, "rebuild", false, "force rebuild of container image") cmd.Flags().BoolVar(&flags.KeepContainer, "keep", false, "keep container after run completes (for debugging)") - cmd.Flags().StringVar(&flags.Runtime, "runtime", "", "container runtime to use (apple, docker)") + cmd.Flags().StringVar(&flags.Runtime, "runtime", "", "container runtime to use (apple, docker, podman)") cmd.Flags().StringVar(&flags.WorkspaceMode, "workspace-mode", "", "workspace mode: 'bind' (default) or 'volume' (isolated copy in a named volume)") cmd.Flags().BoolVar(&flags.NoSandbox, "no-sandbox", false, "disable gVisor sandbox (reduced isolation, Docker only)") cmd.Flags().BoolVar(&flags.NoClipboard, "no-clipboard", false, "disable host clipboard bridging") diff --git a/internal/config/config.go b/internal/config/config.go index 2072e162..3f231ffd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -60,9 +60,10 @@ type Config struct { // Empty string or omitted uses default (gVisor enabled). Sandbox string `yaml:"sandbox,omitempty"` - // Runtime forces a specific container runtime ("docker" or "apple"). + // Runtime forces a specific container runtime ("docker", "apple", or "podman"). // If not set, moat auto-detects the best available runtime. // Useful when agent needs docker:dind on macOS (Apple containers can't run dind). + // "podman" runs the Docker runtime against a podman socket. Runtime string `yaml:"runtime,omitempty"` Volumes []VolumeConfig `yaml:"volumes,omitempty"` @@ -596,9 +597,9 @@ func Load(dir string) (*Config, error) { return nil, err } - // Validate runtime field (only "docker" or "apple" allowed) - if cfg.Runtime != "" && cfg.Runtime != "docker" && cfg.Runtime != "apple" { - return nil, fmt.Errorf("invalid runtime %q: must be 'docker' or 'apple'", cfg.Runtime) + // Validate runtime field (only "docker", "apple", or "podman" allowed) + if cfg.Runtime != "" && cfg.Runtime != "docker" && cfg.Runtime != "apple" && cfg.Runtime != "podman" { + return nil, fmt.Errorf("invalid runtime %q: must be 'docker', 'apple', or 'podman'", cfg.Runtime) } // Validate workspace mode diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b2331602..b33a1ea1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -359,6 +359,26 @@ runtime: docker } } +func TestLoadConfigAcceptsPodmanRuntime(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "moat.yaml") + + content := ` +name: myapp +agent: test +runtime: podman +` + os.WriteFile(configPath, []byte(content), 0o644) + + cfg, err := Load(dir) + if err != nil { + t.Fatalf("Load should accept runtime: podman, got error: %v", err) + } + if cfg.Runtime != "podman" { + t.Errorf("Runtime = %q, want %q", cfg.Runtime, "podman") + } +} + func TestLoadConfigRejectsInvalidRuntime(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "moat.yaml") diff --git a/internal/container/detect.go b/internal/container/detect.go index f3750038..7a4d3f2d 100644 --- a/internal/container/detect.go +++ b/internal/container/detect.go @@ -43,11 +43,19 @@ func NewRuntimeWithOptions(opts RuntimeOptions) (Runtime, error) { switch strings.ToLower(override) { case "docker": log.Debug("using Docker runtime (MOAT_RUNTIME=docker)") - rt, err := newDockerRuntimeWithPing(opts.Sandbox) + rt, err := newDockerRuntimeWithPingCandidates(opts.Sandbox, genuineDockerSockets()) if err != nil { hint := "Set MOAT_RUNTIME=apple, use --runtime apple, or remove 'runtime: docker' from moat.yaml to use auto-detection." return nil, fmt.Errorf("Docker runtime requested (via MOAT_RUNTIME or moat.yaml) but not available: %w\n\n%s", err, hint) } + // A mismatch here (DOCKER_HOST pointing at a podman engine while + // "docker" was explicitly requested) is not probed for eagerly: + // that would cost every startup with DOCKER_HOST set — including + // the common remote-Docker and Rancher Desktop paths — a blocking + // ServerVersion call purely on the chance of a warning most of + // them would never see. Identity is instead reported lazily via + // (*DockerRuntime).EngineName wherever it's first determined + // (e.g. at run-creation time), reusing IsPodmanEngine's cache. return rt, nil case "apple": log.Debug("using Apple container runtime (MOAT_RUNTIME=apple)") @@ -56,8 +64,15 @@ func NewRuntimeWithOptions(opts RuntimeOptions) (Runtime, error) { return rt, nil } return nil, fmt.Errorf("Apple container runtime not available: %s\n\nTo start the container system manually:\n container system start", reason) + case "podman": + log.Debug("using Docker runtime over podman socket (MOAT_RUNTIME=podman)") + rt, err := newPodmanRuntimeWithPing(opts.Sandbox) + if err != nil { + return nil, err + } + return rt, nil default: - return nil, fmt.Errorf("unknown MOAT_RUNTIME value %q (use 'docker' or 'apple')", override) + return nil, fmt.Errorf("unknown MOAT_RUNTIME value %q (use 'docker', 'apple', or 'podman')", override) } } @@ -78,7 +93,7 @@ func NewRuntimeWithOptions(opts RuntimeOptions) (Runtime, error) { rt, err := newDockerRuntimeWithPing(opts.Sandbox) if err != nil { if appleReason != "" { - return nil, fmt.Errorf("no container runtime available:\n Apple containers: %s\n Docker: %w\n\nTo start Apple containers manually:\n container system start\n\nTo force a specific runtime:\n moat run --runtime apple\n moat run --runtime docker", appleReason, err) + return nil, fmt.Errorf("no container runtime available:\n Apple containers: %s\n Docker: %w\n\nTo start Apple containers manually:\n container system start\n\nTo force a specific runtime:\n moat run --runtime apple\n moat run --runtime docker\n moat run --runtime podman", appleReason, err) } return nil, fmt.Errorf("no container runtime available: %w", err) } @@ -92,18 +107,31 @@ func NewRuntimeWithOptions(opts RuntimeOptions) (Runtime, error) { // The MOAT_RUNTIME environment variable can override auto-detection: // - MOAT_RUNTIME=docker: force Docker runtime // - MOAT_RUNTIME=apple: force Apple container runtime +// - MOAT_RUNTIME=podman: force the Docker runtime against a podman socket func NewRuntime() (Runtime, error) { return NewRuntimeWithOptions(DefaultRuntimeOptions()) } // newDockerRuntimeWithPing creates a Docker runtime and verifies it's accessible. // If the default Docker socket is unreachable and DOCKER_HOST is not set, it -// probes known alternative socket locations (see tryAlternativeDockerSockets). -// As a side effect, if an alternative socket is found, DOCKER_HOST is set -// permanently in the process environment to point to it. +// probes known alternative socket locations, including podman's (see +// alternativeDockerSockets). As a side effect, if an alternative socket is +// found, DOCKER_HOST is set permanently in the process environment to point +// to it. +// +// The probe includes podman candidates, so use it only where landing on a +// podman socket is acceptable. An explicit MOAT_RUNTIME=docker request must +// instead pass genuineDockerSockets to newDockerRuntimeWithPingCandidates. func newDockerRuntimeWithPing(sandbox bool) (Runtime, error) { + return newDockerRuntimeWithPingCandidates(sandbox, alternativeDockerSockets()) +} + +// newDockerRuntimeWithPingCandidates creates a Docker runtime and verifies +// it's accessible, falling back (when DOCKER_HOST is not explicitly set) to +// probing the given socket candidates. +func newDockerRuntimeWithPingCandidates(sandbox bool, fallbackCandidates []dockerSocketCandidate) (Runtime, error) { var rt Runtime - dockerRT, err := NewDockerRuntime(sandbox) + dockerRT, err := detectEnv.newDockerRuntime(sandbox) if err != nil { return nil, fmt.Errorf("Docker runtime error: %w", err) } @@ -119,7 +147,7 @@ func newDockerRuntimeWithPing(sandbox bool) (Runtime, error) { if os.Getenv("DOCKER_HOST") != "" { return nil, err } - altRT := tryAlternativeDockerSockets(sandbox) + altRT, _ := tryDockerSocketCandidates(fallbackCandidates, sandbox) if altRT == nil { return nil, err } @@ -138,6 +166,51 @@ func newDockerRuntimeWithPing(sandbox bool) (Runtime, error) { return rt, nil } +// newPodmanRuntimeWithPing creates a Docker runtime targeting a podman socket; +// podman's compat API works with it unmodified, so there is no separate podman +// Runtime implementation. A preset DOCKER_HOST is used as-is but verified to +// actually be podman, so MOAT_RUNTIME=podman can't silently succeed against a +// real Docker daemon. Otherwise podmanSocketCandidates are probed and +// DOCKER_HOST is set to the first that answers. +func newPodmanRuntimeWithPing(sandbox bool) (Runtime, error) { + hint := "To start podman:\n macOS: podman machine start\n Linux: systemctl --user enable --now podman.socket" + + if os.Getenv("DOCKER_HOST") != "" { + dockerRT, err := NewDockerRuntime(sandbox) + if err != nil { + return nil, fmt.Errorf("podman runtime error: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if pingErr := dockerRT.Ping(ctx); pingErr != nil { + return nil, fmt.Errorf("podman runtime requested (via MOAT_RUNTIME or moat.yaml) but DOCKER_HOST is unreachable: %w\n\n%s", pingErr, hint) + } + isPodman, err := dockerRT.IsPodmanEngine(ctx) + if err != nil { + return nil, fmt.Errorf("podman runtime requested (via MOAT_RUNTIME or moat.yaml) but could not identify the engine behind DOCKER_HOST=%s: %w", os.Getenv("DOCKER_HOST"), err) + } + if !isPodman { + return nil, fmt.Errorf("podman runtime requested (via MOAT_RUNTIME or moat.yaml) but DOCKER_HOST=%s points at a non-podman engine", os.Getenv("DOCKER_HOST")) + } + return dockerRT, nil + } + + // Verify each candidate is actually podman's compat API, not some other + // engine answering on a podman-looking path. + verifyPodman := func(dockerRT *DockerRuntime, ctx context.Context) (bool, error) { + return dockerRT.IsPodmanEngine(ctx) + } + rt, probeErr := tryDockerSocketCandidatesVerified(podmanSocketCandidates(), sandbox, verifyPodman) + if rt == nil { + if probeErr != nil { + return nil, fmt.Errorf("podman runtime requested (via MOAT_RUNTIME or moat.yaml): a podman socket was found but is unusable: %w\n\n%s", probeErr, hint) + } + return nil, fmt.Errorf("podman runtime requested (via MOAT_RUNTIME or moat.yaml) but no podman socket was found\n\n%s", hint) + } + return rt, nil +} + // dockerSocketCandidate represents a known Docker-compatible socket from a // third-party container tool. type dockerSocketCandidate struct { @@ -152,15 +225,131 @@ type dockerSocketCandidate struct { // Entries are platform-specific: macOS-only paths are guarded by // runtime.GOOS so they are not probed unnecessarily on Linux. func alternativeDockerSockets() []dockerSocketCandidate { - if runtime.GOOS != "darwin" { - return nil + return append(genuineDockerSockets(), podmanSocketCandidates()...) +} + +// genuineDockerSockets returns sockets backed by a real Docker engine, as +// opposed to podman's compat API. An explicit MOAT_RUNTIME=docker falls back +// only to these, so it can't silently land on a podman socket. +func genuineDockerSockets() []dockerSocketCandidate { + var candidates []dockerSocketCandidate + if runtime.GOOS == "darwin" { + if home, err := os.UserHomeDir(); err == nil { + candidates = append(candidates, dockerSocketCandidate{filepath.Join(home, ".rd", "docker.sock"), "Rancher Desktop"}) + } } - home, err := os.UserHomeDir() - if err != nil { - return nil + return candidates +} + +// detectEnviron holds the filesystem/constructor seams that tests need to +// redirect in order to exercise fallback and auto-detection paths +// hermetically (without touching the real HOME, XDG_RUNTIME_DIR, or an +// actual dockerd). Consolidated into a single struct — rather than one +// package variable per seam — so tests have exactly one thing to save and +// restore (see SwapDetectEnv in export_test.go) instead of several +// independent globals that could be left half-restored between tests. This +// also keeps the shipped binary from carrying more test-only mutable state +// than necessary: production code reads through detectEnv, but there's one +// declaration site for what the seams are, not five scattered across the +// file. +// +// Tests that call SwapDetectEnv must not use t.Parallel(): detectEnv is +// package-level mutable state, and a parallel test could observe another +// test's swapped values. +type detectEnviron struct { + // rootfulSocket is the well-known path to podman's rootful Docker-API + // socket on Linux. Redirected in tests: unlike the other candidates it + // can't be neutralized via HOME/XDG_RUNTIME_DIR/TMPDIR, so a test host + // running rootful podman would otherwise dial the real socket. + rootfulSocket string + + // xdgRuntimeDir returns the runtime-dir base for podman's rootless socket + // when XDG_RUNTIME_DIR is unset, as in sudo/cron/CI — podman still uses + // systemd's /run/user/ regardless of whether the variable is + // exported. Redirected in tests to avoid depending on the invoking uid. + xdgRuntimeDir func() string + + // connectionsPath locates podman's connections file + // ($XDG_CONFIG_HOME/containers/podman-connections.json by default, see + // podmanDefaultConnection in podman_machine.go). Redirected in tests so + // they don't touch the caller's real HOME. + connectionsPath func() string + + // newDockerRuntime constructs a Docker runtime for the default endpoint. + // Redirected in tests to pin the "default socket unreachable" + // precondition the podman-fallback tests need, even on a host with a + // live dockerd. + newDockerRuntime func(sandbox bool) (*DockerRuntime, error) +} + +// defaultDetectEnviron returns detectEnviron's production values — the real +// filesystem paths and constructors, as opposed to whatever a test has +// substituted via SwapDetectEnv. +func defaultDetectEnviron() detectEnviron { + return detectEnviron{ + rootfulSocket: "/run/podman/podman.sock", + xdgRuntimeDir: func() string { + return fmt.Sprintf("/run/user/%d", os.Getuid()) + }, + connectionsPath: func() string { + base := os.Getenv("XDG_CONFIG_HOME") + if base == "" { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + base = filepath.Join(home, ".config") + } + return filepath.Join(base, "containers", "podman-connections.json") + }, + newDockerRuntime: NewDockerRuntime, } - return []dockerSocketCandidate{ - {filepath.Join(home, ".rd", "docker.sock"), "Rancher Desktop"}, +} + +// detectEnv is the package's single instance of the seams above. Production +// code reads fields off this variable rather than holding independent +// package-level vars; tests redirect it via SwapDetectEnv (export_test.go). +var detectEnv = defaultDetectEnviron() + +// podmanSocketCandidates returns paths to podman's Docker-API-compatible +// socket: +// +// - macOS (podman machine): $TMPDIR/podman/-api.sock, with +// podman's own default connection probed first +// - Linux rootless: $XDG_RUNTIME_DIR/podman/podman.sock, falling back to +// /run/user//podman/podman.sock when XDG_RUNTIME_DIR is unset +// - Linux rootful: /run/podman/podman.sock +func podmanSocketCandidates() []dockerSocketCandidate { + switch runtime.GOOS { + case "darwin": + matches, err := filepath.Glob(filepath.Join(os.TempDir(), "podman", "*-api.sock")) + if err != nil { + return nil + } + var candidates []dockerSocketCandidate + for _, m := range matches { + desc := "Podman machine" + if name := podmanMachineName(m); name != "" { + desc = "Podman machine " + name + } + candidates = append(candidates, dockerSocketCandidate{m, desc}) + } + // Glob is sorted, so with several machines running the alphabetically + // first would win regardless of which one podman itself targets. + return preferDefaultPodmanMachine(candidates) + case "linux": + var candidates []dockerSocketCandidate + xdg := os.Getenv("XDG_RUNTIME_DIR") + if xdg == "" { + xdg = detectEnv.xdgRuntimeDir() + } + if xdg != "" { + candidates = append(candidates, dockerSocketCandidate{filepath.Join(xdg, "podman", "podman.sock"), "Podman (rootless)"}) + } + candidates = append(candidates, dockerSocketCandidate{detectEnv.rootfulSocket, "Podman (rootful)"}) + return candidates + default: + return nil } } @@ -168,9 +357,33 @@ func alternativeDockerSockets() []dockerSocketCandidate { // third-party container tools when the default Docker socket is unreachable. // // If a working socket is found, DOCKER_HOST is set so all subsequent Docker -// client creation uses the discovered socket. +// client creation uses the discovered socket. Any candidate-probe error is +// discarded — callers that need it should use tryDockerSocketCandidates +// directly (see newPodmanRuntimeWithPing). func tryAlternativeDockerSockets(sandbox bool) Runtime { - for _, c := range alternativeDockerSockets() { + rt, _ := tryDockerSocketCandidates(alternativeDockerSockets(), sandbox) + return rt +} + +// tryDockerSocketCandidates checks a list of known Docker-compatible socket +// paths, returning the Runtime for the first one that stats as a socket and +// answers a ping. If a working socket is found, DOCKER_HOST is set so all +// subsequent Docker client creation uses the discovered socket. +// +// If none succeeds, the returned error is the last failure among candidates +// that did stat as a socket, and nil if no candidate path existed at all — so +// callers can tell "nothing there" from "something there but broken". +func tryDockerSocketCandidates(candidates []dockerSocketCandidate, sandbox bool) (Runtime, error) { + return tryDockerSocketCandidatesVerified(candidates, sandbox, nil) +} + +// tryDockerSocketCandidatesVerified is tryDockerSocketCandidates with an +// optional identity check, called after a successful ping. A false result +// skips the candidate; an error is treated as a failed ping. The podman probe +// uses it because a matching socket path alone is no proof of engine identity. +func tryDockerSocketCandidatesVerified(candidates []dockerSocketCandidate, sandbox bool, verify func(*DockerRuntime, context.Context) (bool, error)) (Runtime, error) { + var lastErr error + for _, c := range candidates { // Use os.Stat (not Lstat) to follow symlinks — on macOS, // ~/.rd/docker.sock is a symlink to the actual socket. info, err := os.Stat(c.path) @@ -178,33 +391,85 @@ func tryAlternativeDockerSockets(sandbox bool) Runtime { continue } - host := "unix://" + c.path - log.Debug("trying alternative Docker socket", "path", c.path, "tool", c.name) - - // Set DOCKER_HOST so NewDockerRuntime picks up the socket, then - // ping to verify it's reachable before committing to it. - os.Setenv("DOCKER_HOST", host) - - rt, err := NewDockerRuntime(sandbox) + rt, err := tryDockerSocketCandidate(c, sandbox, verify) if err != nil { - os.Unsetenv("DOCKER_HOST") + lastErr = err continue } - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - pingErr := rt.Ping(ctx) - cancel() - if pingErr != nil { - os.Unsetenv("DOCKER_HOST") + if rt == nil { + // Candidate answered but failed the identity check; already + // logged by tryDockerSocketCandidate. continue } - // Socket is reachable — DOCKER_HOST is already set. + // Socket is reachable (and, if verify was given, confirmed to be the + // expected engine) — DOCKER_HOST is already set. log.Debug("auto-detected Docker via "+c.name, "socket", c.path) - return rt + return rt, nil } - return nil + return nil, lastErr +} + +// tryDockerSocketCandidate dials a single socket candidate: sets DOCKER_HOST, +// constructs a client, pings, and (if verify is given) checks engine +// identity. Split out of tryDockerSocketCandidatesVerified's loop body so +// each candidate's ping context can be released via a single deferred +// cancel() instead of three separate manual cancel() calls scattered across +// the failure exits — one call away from a leak on the old shape. +// +// Returns (rt, nil) on success; (nil, nil) if the candidate answered the ping +// but verify rejected it (not an error worth reporting — already logged); or +// (nil, err) if stat/construction/ping/verify failed outright. +func tryDockerSocketCandidate(c dockerSocketCandidate, sandbox bool, verify func(*DockerRuntime, context.Context) (bool, error)) (*DockerRuntime, error) { + host := "unix://" + c.path + log.Debug("trying alternative Docker socket", "path", c.path, "tool", c.name) + + // Set DOCKER_HOST so NewDockerRuntime picks up the socket, then ping to + // verify it's reachable before committing to it. Left set only if this + // candidate is ultimately accepted. + os.Setenv("DOCKER_HOST", host) + accepted := false + defer func() { + if !accepted { + os.Unsetenv("DOCKER_HOST") + } + }() + + rt, err := NewDockerRuntime(sandbox) + if err != nil { + return nil, fmt.Errorf("%s (%s): %w", c.path, c.name, err) + } + + pingCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if pingErr := rt.Ping(pingCtx); pingErr != nil { + rt.Close() + return nil, fmt.Errorf("%s (%s): ping failed: %w", c.path, c.name, pingErr) + } + + if verify != nil { + // A fresh timeout rather than reusing pingCtx: pingCtx may already be + // mostly spent by a slow-but-successful ping, and IsPodmanEngine (the + // verify implementation in practice) layers its own 5s on top of + // whatever it's handed — so a slow ping must not be allowed to starve + // identification of a genuinely-alive engine. + verifyCtx, verifyCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer verifyCancel() + ok, verr := verify(rt, verifyCtx) + if verr != nil { + rt.Close() + return nil, fmt.Errorf("%s (%s): identifying engine: %w", c.path, c.name, verr) + } + if !ok { + log.Debug("candidate socket is not the expected engine, skipping", "path", c.path, "tool", c.name) + rt.Close() + return nil, nil + } + } + + accepted = true + return rt, nil } // tryAppleRuntime attempts to create and verify an Apple runtime. diff --git a/internal/container/detect_test.go b/internal/container/detect_test.go index 1a733fc4..f813ee67 100644 --- a/internal/container/detect_test.go +++ b/internal/container/detect_test.go @@ -1,14 +1,23 @@ package container import ( + "bytes" "context" + "encoding/json" + "fmt" "net" + "net/http" + "net/http/httptest" + "net/url" "os" "path/filepath" "runtime" "strings" "testing" "time" + + "github.com/docker/docker/api/types" + "github.com/majorcontext/moat/internal/ui" ) func TestGVisorAvailable(t *testing.T) { @@ -117,10 +126,235 @@ func TestAlternativeDockerSocketPaths(t *testing.T) { } } +func TestPodmanSocketCandidatesDarwin(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("darwin-only podman socket layout") + } + + // Point TMPDIR at a scratch dir containing a fake podman machine socket, + // so the glob in podmanSocketCandidates has something deterministic to find. + dir := t.TempDir() + t.Setenv("TMPDIR", dir+"/") + + podmanDir := filepath.Join(dir, "podman") + if err := os.MkdirAll(podmanDir, 0o755); err != nil { + t.Fatal(err) + } + sockPath := filepath.Join(podmanDir, "podman-machine-default-api.sock") + if err := os.WriteFile(sockPath, nil, 0o644); err != nil { + t.Fatal(err) + } + + candidates := podmanSocketCandidates() + if len(candidates) != 1 { + t.Fatalf("expected 1 candidate, got %d: %+v", len(candidates), candidates) + } + if candidates[0].path != sockPath { + t.Errorf("path = %q, want %q", candidates[0].path, sockPath) + } + if want := "Podman machine podman-machine-default"; candidates[0].name != want { + t.Errorf("name = %q, want %q", candidates[0].name, want) + } +} + +// TestPodmanSocketCandidatesDarwinPrefersDefaultMachine covers the multi-machine +// case the single-machine test above cannot: with two machines running, the +// candidate order must follow podman's own default connection rather than the +// glob's alphabetical order. +func TestPodmanSocketCandidatesDarwinPrefersDefaultMachine(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("darwin-only podman socket layout") + } + + dir := t.TempDir() + t.Setenv("TMPDIR", dir+"/") + t.Setenv("CONTAINER_CONNECTION", "") + + podmanDir := filepath.Join(dir, "podman") + if err := os.MkdirAll(podmanDir, 0o755); err != nil { + t.Fatal(err) + } + // "dev" sorts first and would win on glob order alone. + for _, name := range []string{"dev", "podman-machine-default"} { + if err := os.WriteFile(filepath.Join(podmanDir, name+"-api.sock"), nil, 0o644); err != nil { + t.Fatal(err) + } + } + writePodmanConnections(t, "podman-machine-default") + + candidates := podmanSocketCandidates() + if len(candidates) != 2 { + t.Fatalf("expected 2 candidates, got %d: %+v", len(candidates), candidates) + } + if got := podmanMachineName(candidates[0].path); got != "podman-machine-default" { + t.Errorf("first candidate = %q, want the default machine", got) + } + if got := podmanMachineName(candidates[1].path); got != "dev" { + t.Errorf("second candidate = %q, want the non-default machine preserved", got) + } +} + +func TestPodmanSocketCandidatesDarwinNoMachine(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("darwin-only podman socket layout") + } + + t.Setenv("TMPDIR", t.TempDir()+"/") + + if candidates := podmanSocketCandidates(); len(candidates) != 0 { + t.Errorf("expected no candidates when no podman machine socket exists, got %+v", candidates) + } +} + +func TestPodmanSocketCandidatesLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("linux-only podman socket layout") + } + + t.Setenv("XDG_RUNTIME_DIR", "/run/user/1000") + candidates := podmanSocketCandidates() + + wantRootless := "/run/user/1000/podman/podman.sock" + wantRootful := "/run/podman/podman.sock" + if len(candidates) != 2 { + t.Fatalf("expected 2 candidates, got %d: %+v", len(candidates), candidates) + } + if candidates[0].path != wantRootless { + t.Errorf("rootless path = %q, want %q", candidates[0].path, wantRootless) + } + if candidates[1].path != wantRootful { + t.Errorf("rootful path = %q, want %q", candidates[1].path, wantRootful) + } +} + +func TestPodmanSocketCandidatesLinuxNoXDGRuntimeDir(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("linux-only podman socket layout") + } + + // Redirect the uid-fallback seam so this test doesn't depend on the + // actual invoking uid (sudo/cron/CI contexts lack XDG_RUNTIME_DIR but + // still have podman's socket under /run/user/). + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.xdgRuntimeDir = func() string { return "/run/user/9999" } + return e + }) + t.Cleanup(restore) + + t.Setenv("XDG_RUNTIME_DIR", "") + candidates := podmanSocketCandidates() + + wantRootless := "/run/user/9999/podman/podman.sock" + wantRootful := "/run/podman/podman.sock" + if len(candidates) != 2 { + t.Fatalf("expected the uid-fallback rootless candidate plus rootful when XDG_RUNTIME_DIR is unset, got %+v", candidates) + } + if candidates[0].path != wantRootless { + t.Errorf("rootless path = %q, want %q", candidates[0].path, wantRootless) + } + if candidates[1].path != wantRootful { + t.Errorf("rootful path = %q, want %q", candidates[1].path, wantRootful) + } +} + +func TestXDGRuntimeDirFallbackUsesUID(t *testing.T) { + // The real (non-overridden) fallback must derive the path from the + // current process's uid — the systemd/podman convention — not a fixed or + // empty value. Checked against defaultDetectEnviron() directly (rather + // than the possibly-swapped package var detectEnv) so this test's result + // doesn't depend on whether it happens to run while some other test has + // detectEnv redirected. + want := fmt.Sprintf("/run/user/%d", os.Getuid()) + if got := defaultDetectEnviron().xdgRuntimeDir(); got != want { + t.Errorf("defaultDetectEnviron().xdgRuntimeDir() = %q, want %q", got, want) + } +} + +func TestAlternativeDockerSocketsIncludesPodman(t *testing.T) { + // alternativeDockerSockets should append podman candidates after any + // platform-specific third-party sockets (Rancher Desktop on macOS keeps + // precedence). This just checks podman candidates aren't dropped. + got := alternativeDockerSockets() + want := podmanSocketCandidates() + if len(got) < len(want) { + t.Fatalf("alternativeDockerSockets() returned fewer entries (%d) than podmanSocketCandidates() (%d)", len(got), len(want)) + } + gotTail := got[len(got)-len(want):] + for i := range want { + if gotTail[i] != want[i] { + t.Errorf("alternativeDockerSockets() tail[%d] = %+v, want %+v", i, gotTail[i], want[i]) + } + } +} + +func TestVersionIsPodman(t *testing.T) { + tests := []struct { + name string + components []string + want bool + }{ + {"podman engine", []string{"Podman Engine"}, true}, + {"docker engine", []string{"Engine"}, false}, + {"docker desktop style", []string{"Engine", "containerd", "runc", "docker-init"}, false}, + {"empty", nil, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := types.Version{} + for _, name := range tt.components { + v.Components = append(v.Components, types.ComponentVersion{Name: name}) + } + if got := versionIsPodman(v); got != tt.want { + t.Errorf("versionIsPodman(%v) = %v, want %v", tt.components, got, tt.want) + } + }) + } +} + +func TestNewRuntimeWithOptionsPodmanOverrideNoPodman(t *testing.T) { + // Without a live podman socket (and no DOCKER_HOST), MOAT_RUNTIME=podman + // should fail with an actionable hint rather than silently falling back + // to another runtime. + t.Setenv("MOAT_RUNTIME", "podman") + t.Setenv("DOCKER_HOST", "") + t.Setenv("HOME", t.TempDir()) + if runtime.GOOS == "darwin" { + t.Setenv("TMPDIR", t.TempDir()+"/") + } + if runtime.GOOS == "linux" { + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + } + + _, err := NewRuntimeWithOptions(RuntimeOptions{}) + if err == nil { + t.Skip("a podman socket appears to be reachable on this machine; skipping negative-path assertion") + } + if !strings.Contains(err.Error(), "podman") { + t.Errorf("error should mention podman, got: %v", err) + } + if !strings.Contains(err.Error(), "podman machine start") && !strings.Contains(err.Error(), "podman.socket") { + t.Errorf("error should include a start hint, got: %v", err) + } +} + func TestTryAlternativeDockerSocketsNoSockets(t *testing.T) { - // On non-darwin, alternativeDockerSockets returns nil immediately. - // On darwin, point HOME at an empty dir so no candidate paths exist. + // Point HOME (Rancher Desktop), TMPDIR (podman machine on macOS), and + // XDG_RUNTIME_DIR (podman rootless on Linux) at empty scratch dirs so no + // candidate paths exist, isolating this from any real Docker/podman + // tooling running on the test host. t.Setenv("HOME", t.TempDir()) + t.Setenv("TMPDIR", t.TempDir()+"/") + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + + // The Linux rootful podman candidate (/run/podman/podman.sock) is a fixed + // path that can't be neutralized via env vars — redirect it to a scratch + // path so this test stays hermetic even on a Linux host with rootful + // podman running. + restoreRootful := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = filepath.Join(t.TempDir(), "podman.sock") + return e + }) + t.Cleanup(restoreRootful) rt := tryAlternativeDockerSockets(false) if rt != nil { @@ -128,6 +362,205 @@ func TestTryAlternativeDockerSocketsNoSockets(t *testing.T) { } } +// newFakeDockerAPIServer starts an httptest server that serves just enough of +// the Docker Engine API for client.Client's version negotiation, Ping, and +// ServerVersion calls: HEAD/GET /_ping and GET .../version. If podman is +// true, the version response includes podman's compat-API marker component +// ("Podman Engine"), as podman's real compat API does. +func newFakeDockerAPIServer(t *testing.T, podman bool) *httptest.Server { + t.Helper() + + version := types.Version{APIVersion: "1.44", Version: "24.0.0"} + if podman { + version.Components = []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}} + } + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/_ping", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/version") { + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func TestNewRuntimeWithOptionsPodmanOverrideDockerHostNonPodman(t *testing.T) { + srv := newFakeDockerAPIServer(t, false) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + + t.Setenv("MOAT_RUNTIME", "podman") + t.Setenv("DOCKER_HOST", "tcp://"+u.Host) + + _, err = NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err == nil { + t.Fatal("expected error when DOCKER_HOST points at a non-podman engine") + } + if !strings.Contains(err.Error(), "non-podman engine") { + t.Errorf("error should identify a non-podman engine, got: %v", err) + } +} + +func TestNewRuntimeWithOptionsPodmanOverrideDockerHostPodman(t *testing.T) { + srv := newFakeDockerAPIServer(t, true) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + + t.Setenv("MOAT_RUNTIME", "podman") + t.Setenv("DOCKER_HOST", "tcp://"+u.Host) + + rt, err := NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err != nil { + t.Fatalf("expected success when DOCKER_HOST points at podman's compat API, got: %v", err) + } + if rt.Type() != RuntimeDocker { + t.Errorf("Type() = %v, want %v (podman is served via the Docker runtime)", rt.Type(), RuntimeDocker) + } +} + +// TestNewRuntimeWithOptionsPodmanOverrideCandidateRejectsNonPodman is the +// auto-probe companion to the DockerHost case: with DOCKER_HOST unset, the +// candidate probe must reject a Docker engine on a podman candidate socket +// rather than trusting the path alone. +func TestNewRuntimeWithOptionsPodmanOverrideCandidateRejectsNonPodman(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skip("unix-socket-based candidate probing is unix/darwin-only") + } + + t.Setenv("MOAT_RUNTIME", "podman") + t.Setenv("DOCKER_HOST", "") + t.Setenv("HOME", t.TempDir()) + + var podmanSockPath string + switch runtime.GOOS { + case "darwin": + dir := shortTempDir(t) + t.Setenv("TMPDIR", dir+"/") + podmanDir := filepath.Join(dir, "podman") + if err := os.MkdirAll(podmanDir, 0o755); err != nil { + t.Fatal(err) + } + podmanSockPath = filepath.Join(podmanDir, "podman-machine-default-api.sock") + case "linux": + t.Setenv("XDG_RUNTIME_DIR", "") + dir := shortTempDir(t) + rootful := filepath.Join(dir, "podman.sock") + restoreRootful := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = rootful + return e + }) + t.Cleanup(restoreRootful) + podmanSockPath = rootful + } + + // podman=false: the fake engine answers /version without podman's + // "Podman Engine" component marker, as a plain Docker-compatible engine + // (that happened to be listening on a podman-shaped path) would. + serveFakeDockerAPIUnixSocket(t, podmanSockPath, false) + + _, err := NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err == nil { + t.Fatal("MOAT_RUNTIME=podman should not accept a non-podman engine found via candidate probing") + } + if !strings.Contains(err.Error(), "podman") { + t.Errorf("error should mention podman, got: %v", err) + } +} + +func TestIsPodmanEngineDoesNotCacheError(t *testing.T) { + // Server that fails ServerVersion (/version) until toldRecovered flips, + // but always answers /_ping so NewDockerRuntime/Ping succeed regardless. + // This lets us call IsPodmanEngine twice against the *same* runtime: once + // while the version endpoint is broken (must return an error and must not + // cache "false"), and once after it recovers (must then report true). + var recovered bool + version := types.Version{ + APIVersion: "1.44", + Version: "24.0.0", + Components: []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}}, + } + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/_ping", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + if strings.HasSuffix(r.URL.Path, "/version") { + if !recovered { + http.Error(w, "temporarily unavailable", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + http.NotFound(w, r) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + t.Setenv("DOCKER_HOST", "tcp://"+u.Host) + + rt, err := NewDockerRuntime(false) + if err != nil { + t.Fatalf("NewDockerRuntime: %v", err) + } + defer rt.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if _, err := rt.IsPodmanEngine(ctx); err == nil { + t.Fatal("expected an error while the version endpoint is broken") + } + + recovered = true + isPodman, err := rt.IsPodmanEngine(ctx) + if err != nil { + t.Fatalf("expected the earlier error not to be cached, got: %v", err) + } + if !isPodman { + t.Error("expected IsPodmanEngine to report true once the version endpoint recovers") + } +} + // TestSocketStatFollowsSymlink verifies that os.Stat (not Lstat) is the correct // call for socket detection. On macOS, ~/.rd/docker.sock is a symlink to the // actual socket. os.Lstat returns ModeSymlink (not ModeSocket), causing @@ -170,3 +603,316 @@ func TestSocketStatFollowsSymlink(t *testing.T) { t.Error("os.Stat should report ModeSocket when following a symlink to a socket") } } + +// serveFakeDockerAPIUnixSocket starts an HTTP server on a unix socket at +// path, serving the same minimal Docker Engine API surface as +// newFakeDockerAPIServer (HEAD/GET /_ping, GET .../version), so it can stand +// in for a live podman/docker socket at a specific filesystem path (rather +// than httptest's TCP listener). path's parent directory must already +// exist. The server is stopped via t.Cleanup. +func serveFakeDockerAPIUnixSocket(t *testing.T, path string, podman bool) { + t.Helper() + + version := types.Version{APIVersion: "1.44", Version: "24.0.0"} + if podman { + version.Components = []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}} + } + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/version") { + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + }) + + ln, err := net.Listen("unix", path) + if err != nil { + t.Fatalf("listening on unix socket %s: %v", path, err) + } + srv := &http.Server{Handler: mux, ReadHeaderTimeout: 10 * time.Second} + go func() { _ = srv.Serve(ln) }() + t.Cleanup(func() { _ = srv.Close() }) +} + +// forceDefaultDockerUnreachable redirects the newDockerRuntime seam to a +// scratch path with nothing listening, so the initial ping fails even on a +// host with a live dockerd (as ubuntu-latest has). Restored via t.Cleanup. +func forceDefaultDockerUnreachable(t *testing.T) { + t.Helper() + dead := filepath.Join(shortTempDir(t), "dead-default.sock") + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.newDockerRuntime = func(sandbox bool) (*DockerRuntime, error) { + return NewDockerRuntimeWithHost("unix://"+dead, sandbox) + } + return e + }) + t.Cleanup(restore) +} + +// shortTempDir creates a scratch directory directly under /tmp (bypassing +// t.TempDir(), which nests under a per-test path derived from the test name +// — e.g. /var/folders/.../TestMOATRuntimeAutoDetectFallsBackToPodman.../003) +// and, combined with a unix-socket filename, can exceed macOS's ~104-byte +// sun_path limit. Registers cleanup via t.Cleanup. +func shortTempDir(t *testing.T) string { + t.Helper() + dir, err := os.MkdirTemp("/tmp", "moat") + if err != nil { + t.Fatalf("creating short scratch dir: %v", err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + return dir +} + +// TestMOATRuntimeDockerDoesNotFallBackToPodman pins the genuineDockerSockets +// /alternativeDockerSockets split: with the default socket unreachable and a +// live podman-shaped socket in the candidate seam, MOAT_RUNTIME=docker must +// not land on it. Reverting the split passes the rest of the suite, not this. +func TestMOATRuntimeDockerDoesNotFallBackToPodman(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skip("unix-socket-based fallback probing is unix/darwin-only") + } + + t.Setenv("DOCKER_HOST", "") + forceDefaultDockerUnreachable(t) + + // Isolate genuineDockerSockets() (HOME, for the Rancher Desktop + // candidate) from any real third-party tooling on this machine. + t.Setenv("HOME", t.TempDir()) + + // Plant a live fake podman-shaped socket in the platform-specific podman + // candidate seam, redirecting the seams so only our fake socket is found. + var podmanSockPath string + switch runtime.GOOS { + case "darwin": + dir := shortTempDir(t) + t.Setenv("TMPDIR", dir+"/") + podmanDir := filepath.Join(dir, "podman") + if err := os.MkdirAll(podmanDir, 0o755); err != nil { + t.Fatal(err) + } + podmanSockPath = filepath.Join(podmanDir, "podman-machine-default-api.sock") + case "linux": + t.Setenv("XDG_RUNTIME_DIR", "") + dir := shortTempDir(t) + rootful := filepath.Join(dir, "podman.sock") + restoreRootful := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = rootful + return e + }) + t.Cleanup(restoreRootful) + podmanSockPath = rootful + } + serveFakeDockerAPIUnixSocket(t, podmanSockPath, true) + + t.Setenv("MOAT_RUNTIME", "docker") + _, err := NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err == nil { + t.Fatal("MOAT_RUNTIME=docker should not silently fall back to a podman socket") + } + if !strings.Contains(err.Error(), "Docker runtime requested") { + t.Errorf("error should mention Docker was requested, got: %v", err) + } +} + +// TestMOATRuntimeDockerWithPodmanDockerHostSucceedsSilently pins the F5 +// behavior: MOAT_RUNTIME=docker against a podman-backed DOCKER_HOST succeeds +// (unlike the podman override, which fails hard on a genuine mismatch) +// without eagerly probing for or warning about the mismatch at construction +// time. An eager probe here would tax every startup with DOCKER_HOST set — +// including the common remote-Docker and Rancher Desktop paths — with a +// blocking ServerVersion call for a warning most of them would never see. +// Engine identity is available lazily instead, via EngineName, exercised +// below for both the podman and genuine-docker cases. +func TestMOATRuntimeDockerWithPodmanDockerHostSucceedsSilently(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skip("unix-socket-based fake engines are unix/darwin-only") + } + + tests := []struct { + name string + podman bool + wantEngine string + }{ + {"podman engine behind DOCKER_HOST", true, "podman"}, + {"genuine docker engine behind DOCKER_HOST", false, "docker"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sockPath := filepath.Join(shortTempDir(t), "engine.sock") + serveFakeDockerAPIUnixSocket(t, sockPath, tt.podman) + + t.Setenv("MOAT_RUNTIME", "docker") + t.Setenv("DOCKER_HOST", "unix://"+sockPath) + + var buf bytes.Buffer + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + rt, err := NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err != nil { + t.Fatalf("MOAT_RUNTIME=docker with a reachable DOCKER_HOST engine (podman=%v) must succeed, got: %v", tt.podman, err) + } + if rt == nil { + t.Fatal("expected a non-nil runtime") + } + + // No eager probe means no engine-identity output at construction + // time, regardless of whether the engine turns out to be podman + // or genuine docker. (The unrelated "without gVisor sandbox" + // warning is expected here since Sandbox is false — only the + // podman-mismatch warning is what F5 removed.) + if got := buf.String(); strings.Contains(got, "podman") { + t.Errorf("MOAT_RUNTIME=docker should not eagerly warn about engine identity, got: %q", got) + } + + dockerRT, ok := rt.(*DockerRuntime) + if !ok { + t.Fatalf("expected *DockerRuntime, got %T", rt) + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + name, err := dockerRT.EngineName(ctx) + if err != nil { + t.Fatalf("EngineName: %v", err) + } + if name != tt.wantEngine { + t.Errorf("EngineName() = %q, want %q", name, tt.wantEngine) + } + }) + } +} + +// TestMOATRuntimeAutoDetectFallsBackToPodman is the companion to +// TestMOATRuntimeDockerDoesNotFallBackToPodman: with the same dead-default, +// live-podman-socket setup, auto-detection (MOAT_RUNTIME unset) DOES land on +// the podman socket, since podman-or-docker auto-detect is allowed to use +// alternativeDockerSockets (which includes podman candidates). +func TestMOATRuntimeAutoDetectFallsBackToPodman(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skip("unix-socket-based fallback probing is unix/darwin-only") + } + + t.Setenv("DOCKER_HOST", "") + forceDefaultDockerUnreachable(t) + + // Auto-detect tries Apple containers first on darwin/arm64 — take that + // branch out of the running so this test exercises the Docker fallback + // path regardless of whether Apple's container CLI is installed here. + t.Setenv("PATH", "/nonexistent") + + t.Setenv("HOME", t.TempDir()) + + var podmanSockPath string + switch runtime.GOOS { + case "darwin": + dir := shortTempDir(t) + t.Setenv("TMPDIR", dir+"/") + podmanDir := filepath.Join(dir, "podman") + if err := os.MkdirAll(podmanDir, 0o755); err != nil { + t.Fatal(err) + } + podmanSockPath = filepath.Join(podmanDir, "podman-machine-default-api.sock") + case "linux": + t.Setenv("XDG_RUNTIME_DIR", "") + dir := shortTempDir(t) + rootful := filepath.Join(dir, "podman.sock") + restoreRootful := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = rootful + return e + }) + t.Cleanup(restoreRootful) + podmanSockPath = rootful + } + serveFakeDockerAPIUnixSocket(t, podmanSockPath, true) + + rt, err := NewRuntimeWithOptions(RuntimeOptions{Sandbox: false}) + if err != nil { + t.Fatalf("auto-detect should land on the fake podman socket: %v", err) + } + if rt.Type() != RuntimeDocker { + t.Errorf("Type() = %v, want %v (podman is served via the Docker runtime)", rt.Type(), RuntimeDocker) + } +} + +// TestTryDockerSocketCandidatesVerifiedGivesVerifyFreshTimeout pins F7: verify +// must get its own timeout rather than inheriting whatever's left of the +// ping's context. It proves this by inspecting the deadline verify actually +// receives after a slow-but-successful ping, rather than depending on a +// ~5s-long timeout race (which the old shared-context code would only fail +// once the ping had eaten nearly all 5s). +func TestTryDockerSocketCandidatesVerifiedGivesVerifyFreshTimeout(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skip("unix-socket-based fake engines are unix/darwin-only") + } + + // A successful candidate leaves DOCKER_HOST set (see + // tryDockerSocketCandidate) — t.Setenv here, rather than leaving it to + // production code's os.Setenv, ensures it's restored to its prior value + // once this test ends instead of leaking into later tests. + t.Setenv("DOCKER_HOST", "") + + const pingDelay = 2 * time.Second + sockPath := filepath.Join(shortTempDir(t), "slow-ping.sock") + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/_ping") { + time.Sleep(pingDelay) + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + }) + ln, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("listen: %v", err) + } + srv := &http.Server{Handler: mux, ReadHeaderTimeout: 10 * time.Second} + go func() { _ = srv.Serve(ln) }() + t.Cleanup(func() { _ = srv.Close() }) + + var verifyDeadline time.Time + verify := func(rt *DockerRuntime, ctx context.Context) (bool, error) { + dl, ok := ctx.Deadline() + if !ok { + t.Fatal("expected verify's context to carry a deadline") + } + verifyDeadline = dl + return true, nil + } + + candidates := []dockerSocketCandidate{{path: sockPath, name: "test"}} + beforeVerify := time.Now() + rt, err := tryDockerSocketCandidatesVerified(candidates, false, verify) + if err != nil { + t.Fatalf("tryDockerSocketCandidatesVerified: %v", err) + } + if rt == nil { + t.Fatal("expected a non-nil runtime") + } + + // A fresh 5s timeout starting when verify runs (after the pingDelay-long + // ping already completed) deadlines close to 5s from beforeVerify. A + // timeout inherited from the ping's own context (created before the + // ping, also with a 5s budget) would instead deadline close to + // 5s-pingDelay from beforeVerify — pingDelay earlier. + remaining := verifyDeadline.Sub(beforeVerify) + if remaining < 4*time.Second { + t.Errorf("verify's context deadline was %s after the call started; expected close to a fresh 5s timeout, not one inherited from the %s ping (which would leave ~%s)", + remaining, pingDelay, 5*time.Second-pingDelay) + } +} diff --git a/internal/container/docker.go b/internal/container/docker.go index 956d71f6..3090808b 100644 --- a/internal/container/docker.go +++ b/internal/container/docker.go @@ -18,6 +18,7 @@ import ( "time" "github.com/containerd/errdefs" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/build" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" @@ -54,6 +55,19 @@ For Docker Desktop (macOS/Windows): To bypass (reduced isolation): moat run --no-sandbox`) +// warnPodmanGvisorUnverified emits the unverified-gVisor warning, at most once +// per process since several DockerRuntimes may be constructed in a single run. +func warnPodmanGvisorUnverified() { + podmanGvisorWarnOnce.Do(func() { + ui.Warn("gVisor availability is engine-reported and unverified under podman; container creation may fail if runsc isn't actually installed. Check with: podman machine ssh -- which runsc (macOS) or which runsc (Linux). Use --no-sandbox or MOAT_NO_SANDBOX=1 to bypass.") + }) +} + +// podmanGvisorWarnOnce guards warnPodmanGvisorUnverified. Because it is +// process-global, a test asserting the warning must reset it (see +// resetPodmanGvisorWarnOnce) rather than depend on running first. +var podmanGvisorWarnOnce sync.Once + // DockerRuntime implements Runtime using Docker. type DockerRuntime struct { cli *client.Client @@ -63,6 +77,12 @@ type DockerRuntime struct { gvisorOnce sync.Once gvisorAvail bool + // podman engine identification cache; nil means "not yet determined". + // Guarded by podmanMu rather than sync.Once so a transient error doesn't + // consume the one-shot initialization. + podmanMu sync.Mutex + podmanIsRT *bool + networkMgr *dockerNetworkManager sidecarMgr *dockerSidecarManager buildMgr *dockerBuildManager @@ -98,7 +118,38 @@ func NewDockerRuntime(sandbox bool) (*DockerRuntime, error) { if err != nil { return nil, fmt.Errorf("creating docker client: %w", err) } + return newDockerRuntimeFromClient(cli, sandbox) +} + +// NewDockerRuntimeWithHost creates a new Docker runtime pinned to the given +// Docker-API endpoint (e.g. a podman or Rancher Desktop socket), without +// reading or mutating the process-wide DOCKER_HOST environment variable. +// Used when reconnecting to a run whose containers live on a non-default +// endpoint recorded in its metadata (storage.Metadata.DockerHost). +// +// FromEnv must be applied before WithHost: opts apply in order, and WithHost +// overrides only the host field, so FromEnv still supplies TLS config +// (DOCKER_TLS_VERIFY/DOCKER_CERT_PATH) needed to reconnect to a secured tcp:// +// endpoint. The later WithHost always wins, so the environment's DOCKER_HOST +// can't override the caller's. +func NewDockerRuntimeWithHost(host string, sandbox bool) (*DockerRuntime, error) { + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host), client.WithAPIVersionNegotiation()) + if err != nil { + return nil, fmt.Errorf("creating docker client for host %s: %w", host, err) + } + return newDockerRuntimeFromClient(cli, sandbox) +} +// DaemonHost returns the Docker-API endpoint this runtime is connected to, +// e.g. "unix:///var/run/docker.sock". ForEachAvailable uses it to spot a +// host-pinned runtime that points at the pool's default engine. +func (r *DockerRuntime) DaemonHost() string { + return r.cli.DaemonHost() +} + +// newDockerRuntimeFromClient builds a DockerRuntime around an already-constructed +// Docker API client, shared by NewDockerRuntime and NewDockerRuntimeWithHost. +func newDockerRuntimeFromClient(cli *client.Client, sandbox bool) (*DockerRuntime, error) { r := &DockerRuntime{ cli: cli, } @@ -119,6 +170,14 @@ func NewDockerRuntime(sandbox bool) (*DockerRuntime, error) { return nil, fmt.Errorf("%w", ErrGVisorNotAvailable) } ociRuntime = "runsc" + + // Podman reports every OCI runtime configured in containers.conf as + // available whether or not the binary is installed, and the compat API + // can't tell the difference — warn so a later creation failure isn't a + // surprise. + if isPodman, err := r.IsPodmanEngine(context.Background()); err == nil && isPodman { + warnPodmanGvisorUnverified() + } } r.ociRuntime = ociRuntime @@ -426,12 +485,40 @@ func (r *DockerRuntime) CreateContainer(ctx context.Context, cfg Config) (string cfg.Name, ) if err != nil { - return "", fmt.Errorf("creating container: %w", err) + return "", r.diagnoseRunscPodmanCreateError(ctx, fmt.Errorf("creating container: %w", err)) } return resp.ID, nil } +// diagnoseRunscPodmanCreateError wraps a container-creation failure with a +// diagnostic when the runtime requested runsc and the engine is podman: +// podman reports every OCI runtime configured in containers.conf as +// available whether or not the binary is installed (see +// warnPodmanGvisorUnverified), so a creation failure in this exact +// configuration is very likely explained by runsc simply not being +// installed, rather than by whatever the raw engine error says on its own. +// +// Uses IsPodmanEngine's cache rather than probing: by the time +// CreateContainer can fail with r.ociRuntime == "runsc", the sandbox path in +// newDockerRuntimeFromClient has already populated it, so this does not add +// a new network round trip to the error path. If the cache was never +// populated and the lookup itself errors, the original error is returned +// unwrapped rather than risk masking it with a diagnostic we can't confirm. +// +// err is wrapped with %w so errdefs-based classification (e.g. IsNotFound) +// keeps working through the added diagnostic. +func (r *DockerRuntime) diagnoseRunscPodmanCreateError(ctx context.Context, err error) error { + if r.ociRuntime != "runsc" { + return err + } + isPodman, ierr := r.IsPodmanEngine(ctx) + if ierr != nil || !isPodman { + return err + } + return fmt.Errorf("podman reported runsc as available but creating the container with it failed; runsc is very likely not actually installed. Check with: podman machine ssh -- which runsc (macOS) or which runsc (Linux). Use --no-sandbox or MOAT_NO_SANDBOX=1 to bypass: %w", err) +} + // StartContainer starts an existing container. func (r *DockerRuntime) StartContainer(ctx context.Context, containerID string) error { if err := r.cli.ContainerStart(ctx, containerID, container.StartOptions{}); err != nil { @@ -611,6 +698,13 @@ func (r *DockerRuntime) StopContainer(ctx context.Context, containerID string) e return nil } +// IsNotFound reports whether err indicates the engine has no such container +// (or other object). It unwraps, so it still matches errors wrapped with %w by +// the runtime methods above. +func IsNotFound(err error) bool { + return errdefs.IsNotFound(err) +} + // WaitContainer blocks until the container exits. func (r *DockerRuntime) WaitContainer(ctx context.Context, containerID string) (int64, error) { statusCh, errCh := r.cli.ContainerWait(ctx, containerID, container.WaitConditionNotRunning) @@ -774,6 +868,64 @@ func (r *DockerRuntime) gvisorAvailable() bool { return r.gvisorAvail } +// IsPodmanEngine reports whether the connected daemon is podman rather than +// real Docker. A successful determination is cached for this runtime's +// lifetime; a transient error is returned and never cached, so a later call +// can retry instead of being permanently treated as "not podman". +func (r *DockerRuntime) IsPodmanEngine(ctx context.Context) (bool, error) { + r.podmanMu.Lock() + cached := r.podmanIsRT + r.podmanMu.Unlock() + if cached != nil { + return *cached, nil + } + + checkCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + version, err := r.cli.ServerVersion(checkCtx) + if err != nil { + log.Debug("podman engine check failed - not caching, will retry", "error", err) + return false, err + } + + isPodman := versionIsPodman(version) + r.podmanMu.Lock() + r.podmanIsRT = &isPodman + r.podmanMu.Unlock() + return isPodman, nil +} + +// EngineName returns the identity of the engine behind this runtime's +// Docker-API client — "podman" or "docker". It's a thin, human-readable +// wrapper over IsPodmanEngine and shares that method's cache, so calling both +// (or calling EngineName repeatedly) never costs more than one probe per +// runtime. Callers that want to report a "you asked for X but got Y" mismatch +// (see the forced-docker path in NewRuntimeWithOptions) should call this +// lazily, only once identity is otherwise needed — not as a dedicated probe. +func (r *DockerRuntime) EngineName(ctx context.Context) (string, error) { + isPodman, err := r.IsPodmanEngine(ctx) + if err != nil { + return "", err + } + if isPodman { + return "podman", nil + } + return "docker", nil +} + +// versionIsPodman reports whether a Docker Engine API /version response +// describes podman's compat API rather than real Docker. Podman's compat API +// includes a Components entry named "Podman Engine"; real Docker never does. +func versionIsPodman(version types.Version) bool { + for _, c := range version.Components { + if strings.Contains(c.Name, "Podman") { + return true + } + } + return false +} + // SetupFirewall configures iptables and ip6tables to block all outbound traffic // except to the proxy, covering both IPv4 and IPv6. // The proxyHost parameter is accepted for interface consistency but not used in the diff --git a/internal/container/docker_test.go b/internal/container/docker_test.go index 492dc3d4..df9fa2c3 100644 --- a/internal/container/docker_test.go +++ b/internal/container/docker_test.go @@ -2,6 +2,13 @@ package container import ( "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" "os" "reflect" "strconv" @@ -10,9 +17,13 @@ import ( "testing" "time" + "github.com/containerd/errdefs" + + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" "github.com/docker/docker/client" + "github.com/majorcontext/moat/internal/ui" ) func TestBuildContainerMounts_TmpfsWritableAndExec(t *testing.T) { @@ -737,3 +748,206 @@ func TestDockerRuntime_BuildImage_PathSelection(t *testing.T) { }) } } + +func TestIsNotFound(t *testing.T) { + if !IsNotFound(errdefs.ErrNotFound) { + t.Error("IsNotFound should match errdefs.ErrNotFound") + } + // Must unwrap through the %w wrapping the runtime methods apply. + wrapped := fmt.Errorf("stopping container: %w", errdefs.ErrNotFound) + if !IsNotFound(wrapped) { + t.Error("IsNotFound should unwrap a wrapped not-found error") + } + // Companion: an unrelated error is not a false positive. + if IsNotFound(errors.New("daemon unreachable")) { + t.Error("IsNotFound should not match an unrelated error") + } +} + +// TestDockerRuntimeEngineName pins F5's cached accessor: EngineName reports +// "podman" or "docker" per the underlying engine, implemented purely in terms +// of IsPodmanEngine so there's exactly one identity probe and one cache. +func TestDockerRuntimeEngineName(t *testing.T) { + tests := []struct { + name string + podman bool + want string + }{ + {"podman engine", true, "podman"}, + {"docker engine", false, "docker"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := newFakeDockerAPIServer(t, tt.podman) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + + rt, err := NewDockerRuntimeWithHost("tcp://"+u.Host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + got, err := rt.EngineName(ctx) + if err != nil { + t.Fatalf("EngineName: %v", err) + } + if got != tt.want { + t.Errorf("EngineName() = %q, want %q", got, tt.want) + } + + // EngineName must reuse IsPodmanEngine's cache: once EngineName + // has determined identity, IsPodmanEngine must return the same + // result from cache without a second probe (proven the same way + // TestIsPodmanEngineDoesNotCacheError proves caching: this would + // be indistinguishable here since the fake server always answers + // the same way, but the shared podmanIsRT field is what + // TestIsPodmanEngineDoesNotCacheError exercises directly). + isPodman, err := rt.IsPodmanEngine(ctx) + if err != nil { + t.Fatalf("IsPodmanEngine: %v", err) + } + if isPodman != tt.podman { + t.Errorf("IsPodmanEngine() = %v, want %v", isPodman, tt.podman) + } + }) + } +} + +// newFakeDockerAPIServerForCreateFailure starts a fake Docker-API server +// covering the full path CreateContainer exercises before it ever reaches +// ContainerCreate itself (version negotiation, /_ping, /images/.../json for +// ensureImage's exists-check, and /info for gvisorAvailable), so that only +// the /containers/create call fails. podman controls whether /version +// reports podman's compat-API marker; createStatus/createBody control the +// synthetic ContainerCreate failure. +func newFakeDockerAPIServerForCreateFailure(t *testing.T, podman bool, createStatus int, createBody string) *httptest.Server { + t.Helper() + + version := types.Version{APIVersion: "1.44", Version: "24.0.0"} + if podman { + version.Components = []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}} + } + versionBody, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/_ping"): + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + case strings.HasSuffix(r.URL.Path, "/version"): + w.Header().Set("Content-Type", "application/json") + w.Write(versionBody) + case strings.HasSuffix(r.URL.Path, "/info"): + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"Runtimes":{"runsc":{"path":"runsc"}}}`)) + case strings.Contains(r.URL.Path, "/images/") && strings.HasSuffix(r.URL.Path, "/json"): + // ensureImage's exists-check: report the image present so + // CreateContainer proceeds straight to ContainerCreate. + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"Id":"sha256:fake"}`)) + case strings.HasSuffix(r.URL.Path, "/containers/create"): + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(createStatus) + w.Write([]byte(createBody)) + default: + http.NotFound(w, r) + } + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// TestCreateContainerPodmanRunscFailureGetsDiagnosticWrap pins B2: when +// r.ociRuntime == "runsc" and the engine is podman, a ContainerCreate +// failure is wrapped with a diagnostic naming runsc's likely absence, the +// verification command, and the --no-sandbox bypass — while the original +// error stays classifiable via errdefs (a 404 status must still satisfy +// errdefs.IsNotFound after wrapping). +func TestCreateContainerPodmanRunscFailureGetsDiagnosticWrap(t *testing.T) { + resetPodmanGvisorWarnOnce(t) + ui.SetWriter(io.Discard) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + srv := newFakeDockerAPIServerForCreateFailure(t, true, http.StatusNotFound, + `{"message":"OCI runtime create failed: runsc: executable file not found in $PATH: unknown"}`) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + + rt, err := NewDockerRuntimeWithHost("tcp://"+u.Host, true) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err = rt.CreateContainer(ctx, Config{Image: "alpine:latest", Cmd: []string{"true"}}) + if err == nil { + t.Fatal("expected CreateContainer to fail") + } + + if !strings.Contains(err.Error(), "runsc is very likely not actually installed") { + t.Errorf("expected the podman/runsc diagnostic, got: %v", err) + } + if !strings.Contains(err.Error(), "--no-sandbox") { + t.Errorf("expected the --no-sandbox bypass hint, got: %v", err) + } + if !strings.Contains(err.Error(), "which runsc") { + t.Errorf("expected the verification command, got: %v", err) + } + if !errdefs.IsNotFound(err) { + t.Errorf("expected errdefs.IsNotFound to still classify the wrapped error, got: %v", err) + } +} + +// TestCreateContainerNonPodmanRunscFailureNoDiagnosticWrap is the companion: +// against a real (non-podman) Docker engine, the same runsc creation failure +// must NOT get the podman-specific diagnostic — it doesn't apply — while +// errdefs classification must still work identically either way. +func TestCreateContainerNonPodmanRunscFailureNoDiagnosticWrap(t *testing.T) { + resetPodmanGvisorWarnOnce(t) + ui.SetWriter(io.Discard) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + srv := newFakeDockerAPIServerForCreateFailure(t, false, http.StatusNotFound, + `{"message":"OCI runtime create failed: runsc: executable file not found in $PATH: unknown"}`) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + + rt, err := NewDockerRuntimeWithHost("tcp://"+u.Host, true) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err = rt.CreateContainer(ctx, Config{Image: "alpine:latest", Cmd: []string{"true"}}) + if err == nil { + t.Fatal("expected CreateContainer to fail") + } + + if strings.Contains(err.Error(), "runsc is very likely not actually installed") { + t.Errorf("the podman diagnostic should not apply against a real docker engine, got: %v", err) + } + if !errdefs.IsNotFound(err) { + t.Errorf("expected errdefs.IsNotFound to classify the unwrapped error, got: %v", err) + } +} diff --git a/internal/container/export_test.go b/internal/container/export_test.go new file mode 100644 index 00000000..0127a0ae --- /dev/null +++ b/internal/container/export_test.go @@ -0,0 +1,42 @@ +package container + +import ( + "sync" + "testing" +) + +// SwapDetectEnv replaces detectEnv (the package's filesystem/constructor test +// seams, see the detectEnviron type in detect.go) for the duration of a test. +// mutate receives the current environment — start from it and override just +// the field(s) a given test cares about — and returns the replacement. +// Restore puts detectEnv back to what it was before the swap; call it via +// t.Cleanup so a failing or early-returning test still restores it. +// +// detectEnv is shared, mutable package state, so a test that calls +// SwapDetectEnv must not also call t.Parallel() — a parallel sibling could +// otherwise observe (or stomp) the swapped values. +// +// Typical use: +// +// restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { +// e.rootfulSocket = filepath.Join(t.TempDir(), "podman.sock") +// return e +// }) +// t.Cleanup(restore) +func SwapDetectEnv(mutate func(detectEnviron) detectEnviron) (restore func()) { + prev := detectEnv + detectEnv = mutate(prev) + return func() { detectEnv = prev } +} + +// resetPodmanGvisorWarnOnce clears the process-global podmanGvisorWarnOnce +// guard so a test can observe warnPodmanGvisorUnverified's output regardless +// of whether an earlier test already consumed it. Without this the warning is +// only ever visible to whichever test happens to run first. Kept as a +// sync.Once reset (rather than folded into detectEnviron) because resetting a +// one-shot guard genuinely needs direct package access, not a swappable seam. +func resetPodmanGvisorWarnOnce(t *testing.T) { + t.Helper() + podmanGvisorWarnOnce = sync.Once{} + t.Cleanup(func() { podmanGvisorWarnOnce = sync.Once{} }) +} diff --git a/internal/container/podman_doctor.go b/internal/container/podman_doctor.go new file mode 100644 index 00000000..b7382236 --- /dev/null +++ b/internal/container/podman_doctor.go @@ -0,0 +1,16 @@ +package container + +import "os" + +// PodmanSocketPaths returns the podman sockets that currently exist on disk, +// for callers such as `moat doctor` that want to surface an idle socket. Only +// stats the filesystem — never dials the socket or sets DOCKER_HOST. +func PodmanSocketPaths() []string { + var paths []string + for _, c := range podmanSocketCandidates() { + if _, err := os.Stat(c.path); err == nil { + paths = append(paths, c.path) + } + } + return paths +} diff --git a/internal/container/podman_gvisor_warn_test.go b/internal/container/podman_gvisor_warn_test.go new file mode 100644 index 00000000..3c8afbf4 --- /dev/null +++ b/internal/container/podman_gvisor_warn_test.go @@ -0,0 +1,64 @@ +package container + +import ( + "bytes" + "os" + "strings" + "testing" + + "github.com/majorcontext/moat/internal/ui" +) + +// resetPodmanGvisorWarnOnce lives in export_test.go, alongside SwapDetectEnv, +// as the package's other test-only global-state reset helper. + +// TestWarnPodmanGvisorUnverified pins both halves of the contract: the warning +// says something actionable, and it fires at most once per process no matter +// how many runtimes are constructed. +func TestWarnPodmanGvisorUnverified(t *testing.T) { + resetPodmanGvisorWarnOnce(t) + + var buf bytes.Buffer + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + warnPodmanGvisorUnverified() + first := buf.String() + + if !strings.Contains(first, "unverified under podman") { + t.Errorf("warning should explain the engine report is unverified, got: %q", first) + } + if !strings.Contains(first, "--no-sandbox") { + t.Errorf("warning should name the escape hatch, got: %q", first) + } + + // Companion: subsequent constructions must stay silent. + warnPodmanGvisorUnverified() + warnPodmanGvisorUnverified() + if got := buf.String(); got != first { + t.Errorf("warning should fire once per process, got repeat output: %q", got) + } +} + +// TestWarnPodmanGvisorUnverifiedResetIsObservable guards the seam itself: if +// resetting stopped working, the test above would silently pass on a reused +// once and stop asserting anything. +func TestWarnPodmanGvisorUnverifiedResetIsObservable(t *testing.T) { + resetPodmanGvisorWarnOnce(t) + + var buf bytes.Buffer + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + warnPodmanGvisorUnverified() + if buf.Len() == 0 { + t.Fatal("warning should be observable after a reset") + } + + resetPodmanGvisorWarnOnce(t) + buf.Reset() + warnPodmanGvisorUnverified() + if buf.Len() == 0 { + t.Error("reset should make the warning observable again") + } +} diff --git a/internal/container/podman_machine.go b/internal/container/podman_machine.go new file mode 100644 index 00000000..7c05c329 --- /dev/null +++ b/internal/container/podman_machine.go @@ -0,0 +1,77 @@ +package container + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" +) + +// podmanConnectionsFile is the subset of podman's connections file we read. +// Podman 5+ stores it as JSON at $XDG_CONFIG_HOME/containers/podman-connections.json, +// with Connection.Default naming the active connection. +type podmanConnectionsFile struct { + Connection struct { + Default string `json:"Default"` + } `json:"Connection"` +} + +// podmanDefaultConnection returns the name of podman's active connection, or "" +// when it can't be determined. CONTAINER_CONNECTION is podman's own per-command +// override and wins over the stored default, matching what `podman` itself +// would talk to. +func podmanDefaultConnection() string { + if name := os.Getenv("CONTAINER_CONNECTION"); name != "" { + return name + } + path := detectEnv.connectionsPath() + if path == "" { + return "" + } + data, err := os.ReadFile(path) + if err != nil { + return "" + } + var cf podmanConnectionsFile + if err := json.Unmarshal(data, &cf); err != nil { + return "" + } + return cf.Connection.Default +} + +// preferDefaultPodmanMachine reorders machine sockets so the one belonging to +// podman's active connection is probed first. A podman machine's API socket is +// named -api.sock, and with several machines running the candidate +// list is otherwise sorted by filename — so `dev` would beat +// `podman-machine-default` purely on alphabetical order, and IsPodmanEngine +// can't tell them apart because both are genuinely podman. +// +// Order is otherwise preserved, and an unmatched default changes nothing. +func preferDefaultPodmanMachine(candidates []dockerSocketCandidate) []dockerSocketCandidate { + name := podmanDefaultConnection() + if name == "" || len(candidates) < 2 { + return candidates + } + want := name + "-api.sock" + for i, c := range candidates { + if filepath.Base(c.path) != want { + continue + } + reordered := make([]dockerSocketCandidate, 0, len(candidates)) + reordered = append(reordered, c) + reordered = append(reordered, candidates[:i]...) + reordered = append(reordered, candidates[i+1:]...) + return reordered + } + return candidates +} + +// podmanMachineName recovers the machine name from an API socket path, for +// display. Returns "" when path isn't a machine socket. +func podmanMachineName(path string) string { + base := filepath.Base(path) + if !strings.HasSuffix(base, "-api.sock") { + return "" + } + return strings.TrimSuffix(base, "-api.sock") +} diff --git a/internal/container/podman_machine_test.go b/internal/container/podman_machine_test.go new file mode 100644 index 00000000..252c7e2b --- /dev/null +++ b/internal/container/podman_machine_test.go @@ -0,0 +1,148 @@ +package container + +import ( + "os" + "path/filepath" + "testing" +) + +// writePodmanConnections points the connections seam at a scratch file holding +// def as the active connection. An empty def writes a file with no default. +func writePodmanConnections(t *testing.T, def string) { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "podman-connections.json") + body := `{"Connection":{"Default":"` + def + `","Connections":{}},"Farm":{}}` + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write connections file: %v", err) + } + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.connectionsPath = func() string { return path } + return e + }) + t.Cleanup(restore) +} + +func machineCandidates(names ...string) []dockerSocketCandidate { + cands := make([]dockerSocketCandidate, 0, len(names)) + for _, n := range names { + cands = append(cands, dockerSocketCandidate{"/tmp/podman/" + n + "-api.sock", "Podman machine " + n}) + } + return cands +} + +func firstMachine(t *testing.T, cands []dockerSocketCandidate) string { + t.Helper() + if len(cands) == 0 { + t.Fatal("no candidates") + } + return podmanMachineName(cands[0].path) +} + +// TestPreferDefaultPodmanMachine pins the fix for two running machines: the +// alphabetically-first socket must not win over podman's actual default. +func TestPreferDefaultPodmanMachine(t *testing.T) { + t.Setenv("CONTAINER_CONNECTION", "") + writePodmanConnections(t, "podman-machine-default") + + // "dev" sorts before "podman-machine-default" and would win on Glob order. + got := preferDefaultPodmanMachine(machineCandidates("dev", "podman-machine-default")) + if name := firstMachine(t, got); name != "podman-machine-default" { + t.Errorf("default connection should be probed first, got %q", name) + } + if len(got) != 2 { + t.Errorf("no candidate should be dropped, got %d", len(got)) + } + // The non-default machine must still be reachable as a fallback. + if name := podmanMachineName(got[1].path); name != "dev" { + t.Errorf("remaining candidate should be preserved, got %q", name) + } +} + +// TestPreferDefaultPodmanMachineContainerConnectionWins is the companion: +// podman's own CONTAINER_CONNECTION override beats the stored default, so moat +// follows the same machine podman would. +func TestPreferDefaultPodmanMachineContainerConnectionWins(t *testing.T) { + writePodmanConnections(t, "podman-machine-default") + t.Setenv("CONTAINER_CONNECTION", "dev") + + got := preferDefaultPodmanMachine(machineCandidates("dev", "podman-machine-default")) + if name := firstMachine(t, got); name != "dev" { + t.Errorf("CONTAINER_CONNECTION should win, got %q", name) + } +} + +// TestPreferDefaultPodmanMachineNoDefault covers the cases where there is +// nothing to prefer — order must be left exactly as found rather than +// reshuffled arbitrarily. +func TestPreferDefaultPodmanMachineNoDefault(t *testing.T) { + t.Setenv("CONTAINER_CONNECTION", "") + + t.Run("no connections file", func(t *testing.T) { + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.connectionsPath = func() string { return filepath.Join(t.TempDir(), "absent.json") } + return e + }) + t.Cleanup(restore) + + got := preferDefaultPodmanMachine(machineCandidates("dev", "prod")) + if name := firstMachine(t, got); name != "dev" { + t.Errorf("order should be unchanged, got %q", name) + } + }) + + t.Run("default names an absent machine", func(t *testing.T) { + writePodmanConnections(t, "not-running") + got := preferDefaultPodmanMachine(machineCandidates("dev", "prod")) + if name := firstMachine(t, got); name != "dev" { + t.Errorf("order should be unchanged when the default isn't present, got %q", name) + } + if len(got) != 2 { + t.Errorf("no candidate should be dropped, got %d", len(got)) + } + }) + + t.Run("malformed connections file", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "podman-connections.json") + if err := os.WriteFile(path, []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.connectionsPath = func() string { return path } + return e + }) + t.Cleanup(restore) + + got := preferDefaultPodmanMachine(machineCandidates("dev", "prod")) + if name := firstMachine(t, got); name != "dev" { + t.Errorf("malformed file should be ignored, got %q", name) + } + }) + + t.Run("single candidate", func(t *testing.T) { + writePodmanConnections(t, "podman-machine-default") + got := preferDefaultPodmanMachine(machineCandidates("dev")) + if len(got) != 1 || firstMachine(t, got) != "dev" { + t.Errorf("single candidate should pass through untouched, got %v", got) + } + }) +} + +func TestPodmanMachineName(t *testing.T) { + tests := []struct { + path string + want string + }{ + {"/tmp/podman/podman-machine-default-api.sock", "podman-machine-default"}, + {"/tmp/podman/dev-api.sock", "dev"}, + {"/tmp/podman/podman.sock", ""}, + {"/var/run/docker.sock", ""}, + {"", ""}, + } + for _, tt := range tests { + if got := podmanMachineName(tt.path); got != tt.want { + t.Errorf("podmanMachineName(%q) = %q, want %q", tt.path, got, tt.want) + } + } +} diff --git a/internal/container/podman_probe.go b/internal/container/podman_probe.go new file mode 100644 index 00000000..7eaac0f6 --- /dev/null +++ b/internal/container/podman_probe.go @@ -0,0 +1,141 @@ +package container + +import ( + "context" + "net/url" + "os" + "time" + + "github.com/docker/docker/client" +) + +// probeOverallDeadline bounds the total cost of +// ReachablePodmanEndpointOtherThan. It runs on Manager.Stop's not-found +// path, so it must not turn an already-slow "container is gone" case into a +// long hang while it dials sockets nobody answers on. +const probeOverallDeadline = 3 * time.Second + +// probeCandidateDeadline bounds a single candidate's ping-and-identify, so +// one wedged socket (accepting the connection but never responding) can't +// eat the whole overall deadline and starve the remaining candidates. +const probeCandidateDeadline = 1 * time.Second + +// ReachablePodmanEndpointOtherThan reports whether a live podman endpoint, +// distinct from endpoint, is reachable on this host. It exists to tell a +// genuinely ambiguous not-found — a second, different engine that really +// could be holding the container — from the common case where podman is +// either absent or is the only engine present (i.e. its socket resolves to +// endpoint, the very one the caller already queried). +// +// Walks podmanSocketCandidates(), skipping anything that doesn't stat as a +// socket and anything that names the same socket as endpoint by filesystem +// identity (see sameUnixEndpoint) — not just by matching string, since e.g. +// a podman-docker host's /run/docker.sock is a symlink to +// /run/podman/podman.sock, and the two would otherwise look like distinct, +// genuinely ambiguous engines. For the rest, it constructs a bare Docker API +// client pinned to the candidate host — never through anything that reads or +// mutates the process-wide DOCKER_HOST, since this runs mid-Stop on an +// unrelated run and must not have side effects on it — pings it, and +// confirms it identifies as podman via versionIsPodman. Returns the first +// endpoint that passes both checks. Every constructed client is closed +// before returning, on every path. +// +// ctx supplies cancellation, and also bounds the deadline: WithTimeout below +// takes the earlier of ctx's own deadline and now+probeOverallDeadline, so a +// caller already close to its own deadline shortens the probe rather than +// extending it — context.WithTimeout can only tighten a parent deadline, +// never loosen one. In the unlikely case that leaves too little time to +// finish probing, the probe returns false (no other engine found), and the +// ambiguous not-found is downgraded to a warning rather than a hard error — +// the same fate as the "podman genuinely absent" case, so it's a graceful +// degradation, not a correctness break, but it does mean a very tight caller +// deadline can cause a genuinely-orphaned container to be missed rather than +// reported. It may only slow (never fail on its own) an already-in-flight +// Stop. +func ReachablePodmanEndpointOtherThan(ctx context.Context, endpoint string) (string, bool) { + ctx, cancel := context.WithTimeout(ctx, probeOverallDeadline) + defer cancel() + + for _, c := range podmanSocketCandidates() { + // Use os.Stat (not Lstat) to follow symlinks, matching the socket + // checks elsewhere in this package. + info, err := os.Stat(c.path) + if err != nil || info.Mode()&os.ModeSocket == 0 { + continue + } + + host := "unix://" + c.path + if sameUnixEndpoint(host, endpoint, info) { + continue + } + + if isReachablePodman(ctx, host) { + return host, true + } + } + return "", false +} + +// sameUnixEndpoint reports whether host and endpoint name the same unix +// socket. The string comparison is checked first as a cheap, always-correct +// short-circuit; when it doesn't match, endpoint is parsed as a URL and, if +// it has the unix:// scheme, compared against candidateInfo (host's +// already-stat'd os.FileInfo, from the ModeSocket check the caller already +// did) by filesystem identity rather than by path string — so +// /run/docker.sock and /run/podman/podman.sock are recognized as the same +// engine when the former is a symlink to the latter (the podman-docker +// package's layout), and likewise when /var/run is itself a symlink to +// /run. If endpoint isn't a unix:// URL (tcp://, npipe://, empty, or +// unparseable) or os.Stat on its path fails, the two are never treated as +// the same: a stat error here must not cause a candidate to be silently +// skipped, or a genuine orphan could go unreported. +func sameUnixEndpoint(host, endpoint string, candidateInfo os.FileInfo) bool { + if host == endpoint { + return true + } + + u, err := url.Parse(endpoint) + if err != nil || u.Scheme != "unix" || u.Path == "" { + return false + } + + endpointInfo, statErr := os.Stat(u.Path) + if statErr != nil { + return false + } + + return os.SameFile(candidateInfo, endpointInfo) +} + +// isReachablePodman pings host and confirms it identifies as podman, under +// its own sub-timeout derived from ctx so a single wedged candidate can't +// consume the rest of ctx's overall deadline. +// +// This is purely an identity check — Ping plus ServerVersion — so it builds +// a bare Docker API client directly rather than a full DockerRuntime: no OCI +// runtime selection, no network/sidecar/build managers, and critically, none +// of newDockerRuntimeFromClient's sandbox=false handling, which on Linux +// unconditionally prints "Running without gVisor sandbox" — a false alarm +// here, since this probe creates and runs no container at all. Always +// closes the client it constructs, including on every failure path. +func isReachablePodman(ctx context.Context, host string) bool { + // FromEnv must precede WithHost: opts apply in order, and FromEnv + // supplies TLS config (DOCKER_TLS_VERIFY/DOCKER_CERT_PATH) for a secured + // tcp:// endpoint, while the later WithHost always wins on the host + // field — see NewDockerRuntimeWithHost's comment in docker.go, which + // this mirrors. + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(host), client.WithAPIVersionNegotiation()) + if err != nil { + return false + } + defer cli.Close() + + candCtx, cancel := context.WithTimeout(ctx, probeCandidateDeadline) + defer cancel() + + if _, pingErr := cli.Ping(candCtx); pingErr != nil { + return false + } + version, err := cli.ServerVersion(candCtx) + return err == nil && versionIsPodman(version) +} diff --git a/internal/container/podman_probe_test.go b/internal/container/podman_probe_test.go new file mode 100644 index 00000000..b589bfda --- /dev/null +++ b/internal/container/podman_probe_test.go @@ -0,0 +1,278 @@ +package container + +import ( + "bytes" + "context" + "net" + "os" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/majorcontext/moat/internal/ui" +) + +// These tests pin podmanSocketCandidates' Linux layout ([rootless, rootful]) +// directly, via XDG_RUNTIME_DIR (which the rootless branch reads first) and +// SwapDetectEnv's rootfulSocket field (which the rootful branch always +// uses). On darwin the candidate list is built from a TMPDIR glob instead +// and ignores both, so these tests are Linux-only — matching the +// environment gotest.sh actually runs in (golang:1.25 in Docker). + +// TestReachablePodmanEndpointOtherThanExcludesSelf pins the substantive half +// of D2: when the only reachable podman endpoint IS the one the caller +// already queried (e.g. moat auto-detected podman and it's the only engine +// on the host), the probe must report no OTHER engine — there is no +// ambiguity to warn about. +func TestReachablePodmanEndpointOtherThanExcludesSelf(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + sockPath := filepath.Join(dir, "podman.sock") + serveFakeDockerAPIUnixSocket(t, sockPath, true) + + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = sockPath + return e + }) + t.Cleanup(restore) + // No rootless candidate, so the rootful one (== endpoint) is the only + // candidate in play. + t.Setenv("XDG_RUNTIME_DIR", filepath.Join(dir, "no-such-runtime-dir")) + + endpoint := "unix://" + sockPath + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), endpoint) + if ok { + t.Errorf("expected no other reachable endpoint (the only candidate equals endpoint), got %q", got) + } +} + +// TestReachablePodmanEndpointOtherThanFindsDistinctEngine is the companion: +// a second, distinct, live podman endpoint alongside the one already queried +// is genuinely ambiguous and must be reported. +func TestReachablePodmanEndpointOtherThanFindsDistinctEngine(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + + // Rootful candidate: the one already queried. + queriedPath := filepath.Join(dir, "queried-podman.sock") + serveFakeDockerAPIUnixSocket(t, queriedPath, true) + + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = queriedPath + return e + }) + t.Cleanup(restore) + + // Rootless candidate: a second, distinct, live podman engine. + xdgDir := filepath.Join(dir, "xdg") + rootlessDir := filepath.Join(xdgDir, "podman") + if err := os.MkdirAll(rootlessDir, 0o755); err != nil { + t.Fatal(err) + } + otherPath := filepath.Join(rootlessDir, "podman.sock") + serveFakeDockerAPIUnixSocket(t, otherPath, true) + t.Setenv("XDG_RUNTIME_DIR", xdgDir) + + endpoint := "unix://" + queriedPath + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), endpoint) + if !ok { + t.Fatal("expected a distinct reachable podman endpoint to be found") + } + if got != "unix://"+otherPath { + t.Errorf("got %q, want the distinct candidate unix://%s", got, otherPath) + } +} + +// TestReachablePodmanEndpointOtherThanExcludesSymlinkedSelf pins the +// podman-docker fix (F1): on Fedora/RHEL with the podman-docker package +// installed, /run/docker.sock is a symlink to /run/podman/podman.sock, so +// Manager.Stop's queried endpoint (DaemonHost(), resolved through the +// symlink) is a different *string* from the rootful candidate +// podmanSocketCandidates() finds directly — even though both name the same +// socket, the same engine, and the same container namespace. A string-only +// comparison would fail to exclude the candidate, ping it successfully, and +// report it as a second, distinct, live podman engine — turning a clean +// "container already gone" case into a hard-error false positive. This test +// creates a real socket and a symlink to it at a second path, and queries +// through the symlink, mirroring that layout. +func TestReachablePodmanEndpointOtherThanExcludesSymlinkedSelf(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + + // The real socket: this is what podmanSocketCandidates() will find + // directly as the rootful candidate. + realPath := filepath.Join(dir, "podman.sock") + serveFakeDockerAPIUnixSocket(t, realPath, true) + + // A symlink to the real socket, standing in for /run/docker.sock -> + // /run/podman/podman.sock under podman-docker. + symlinkPath := filepath.Join(dir, "docker.sock") + if err := os.Symlink(realPath, symlinkPath); err != nil { + t.Fatalf("creating symlink: %v", err) + } + + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = realPath + return e + }) + t.Cleanup(restore) + t.Setenv("XDG_RUNTIME_DIR", filepath.Join(dir, "no-such-runtime-dir")) + + // The caller queried through the symlink path, not the real path. + endpoint := "unix://" + symlinkPath + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), endpoint) + if ok { + t.Errorf("expected the candidate to be excluded as the same socket reached via a symlink, got %q", got) + } +} + +// TestReachablePodmanEndpointOtherThanSkipsNonSocketPaths verifies a +// candidate whose path exists but isn't a socket (e.g. a stale regular +// file) is skipped rather than dialed, mirroring the stat check used +// elsewhere in this package (tryDockerSocketCandidatesVerified). +func TestReachablePodmanEndpointOtherThanSkipsNonSocketPaths(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + rootful := filepath.Join(dir, "not-a-socket") + if err := os.WriteFile(rootful, []byte("not a socket"), 0o644); err != nil { + t.Fatalf("writing plain file: %v", err) + } + + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = rootful + return e + }) + t.Cleanup(restore) + t.Setenv("XDG_RUNTIME_DIR", filepath.Join(dir, "no-such-runtime-dir")) + + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), "unix://never-queried") + if ok { + t.Errorf("a non-socket path must be skipped, got %q", got) + } +} + +// TestReachablePodmanEndpointOtherThanNoneReachable verifies the "podman +// absent entirely" case: no candidate stats as a socket, so the probe +// reports nothing reachable (this is the common Docker-only host, and must +// stay cheap and side-effect-free). +func TestReachablePodmanEndpointOtherThanNoneReachable(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = filepath.Join(dir, "no-rootful-socket-here") + return e + }) + t.Cleanup(restore) + t.Setenv("XDG_RUNTIME_DIR", filepath.Join(dir, "no-such-runtime-dir")) + + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), "unix://never-queried") + if ok { + t.Errorf("expected no reachable endpoint on a host with no podman socket, got %q", got) + } +} + +// TestIsReachablePodmanEmitsNoUIOutput pins Part A's regression: this probe +// runs on Manager.Stop's not-found path, where no container is being created +// or run at all. isReachablePodman must build a bare Docker API client +// rather than a full DockerRuntime — going through +// NewDockerRuntimeWithHost(host, false) would route through +// newDockerRuntimeFromClient's sandbox=false handling, which on Linux +// unconditionally prints "Running without gVisor sandbox. Container +// isolation is reduced." before this function ever pings anything. That +// would be a false security alarm from a command that creates and runs no +// container whatsoever — moat must never cry wolf here. +func TestIsReachablePodmanEmitsNoUIOutput(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("the gVisor warning this guards against only fires on linux (see newDockerRuntimeFromClient)") + } + + var buf bytes.Buffer + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(os.Stderr) }) + + dir := shortTempDir(t) + sockPath := filepath.Join(dir, "podman.sock") + serveFakeDockerAPIUnixSocket(t, sockPath, true) + + if !isReachablePodman(context.Background(), "unix://"+sockPath) { + t.Fatal("expected the fake podman socket to be recognized as reachable") + } + if got := buf.String(); got != "" { + t.Errorf("isReachablePodman must not print anything to the UI, got %q", got) + } +} + +// TestReachablePodmanEndpointOtherThanWedgedCandidateDoesNotStarveOthers +// pins the per-candidate sub-timeout: a first candidate (rootless, tried +// first) that accepts the connection but never answers must not be allowed +// to consume the whole ~3s overall deadline and starve a second, good +// candidate (rootful). The elapsed-time bound also stands in for "every +// constructed runtime is closed promptly" — a client left open against the +// wedged candidate would tend to push this well past the ~1s sub-timeout. +func TestReachablePodmanEndpointOtherThanWedgedCandidateDoesNotStarveOthers(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("candidate layout below is linux-specific") + } + dir := shortTempDir(t) + + xdgDir := filepath.Join(dir, "xdg") + rootlessDir := filepath.Join(xdgDir, "podman") + if err := os.MkdirAll(rootlessDir, 0o755); err != nil { + t.Fatal(err) + } + wedgedPath := filepath.Join(rootlessDir, "podman.sock") + wedgedLn, err := net.Listen("unix", wedgedPath) + if err != nil { + t.Fatalf("listen (wedged): %v", err) + } + defer wedgedLn.Close() + // Accept connections but never write a response, simulating a socket + // that answers at the TCP level but hangs at the HTTP/API level. + go func() { + for { + conn, err := wedgedLn.Accept() + if err != nil { + return + } + _ = conn // held open, deliberately never responded to + } + }() + t.Setenv("XDG_RUNTIME_DIR", xdgDir) + + goodPath := filepath.Join(dir, "good-podman.sock") + serveFakeDockerAPIUnixSocket(t, goodPath, true) + + restore := SwapDetectEnv(func(e detectEnviron) detectEnviron { + e.rootfulSocket = goodPath + return e + }) + t.Cleanup(restore) + + start := time.Now() + got, ok := ReachablePodmanEndpointOtherThan(context.Background(), "unix://never-queried") + elapsed := time.Since(start) + + if !ok { + t.Fatal("expected the good candidate to be found despite the wedged one preceding it") + } + if got != "unix://"+goodPath { + t.Errorf("got %q, want the good candidate unix://%s", got, goodPath) + } + // Generous margin above the ~1s per-candidate deadline (not the ~3s + // overall one): if the wedged candidate were consuming the whole + // budget, this would land much closer to probeOverallDeadline. + if elapsed > 2*time.Second { + t.Errorf("probe took %s; the wedged first candidate should not have consumed most of the overall deadline", elapsed) + } +} diff --git a/internal/container/pool.go b/internal/container/pool.go index 85437b09..b7788223 100644 --- a/internal/container/pool.go +++ b/internal/container/pool.go @@ -1,8 +1,13 @@ package container import ( + "context" "fmt" + "os" + goruntime "runtime" + "strings" "sync" + "time" ) // RuntimePool manages multiple container runtime instances, keyed by RuntimeType. @@ -15,6 +20,34 @@ type RuntimePool struct { defaultRT Runtime opts RuntimeOptions closed bool + + // dockerHosts caches Docker runtimes pinned to a specific non-default + // DOCKER_HOST endpoint (podman or Rancher Desktop sockets), keyed by host. + // Populated by GetDockerAt. + dockerHosts map[string]Runtime + + // dockerHostsUnavailable negatively caches hosts that failed construction + // or ping in GetDockerAt, so a dead endpoint isn't re-pinged on every + // reconnect. Unlike the unavailable map above (which caches runtime + // *types* — these don't appear mid-process), a podman machine restarting + // mid-session is routine, and moat runs as a long-lived daemon, so + // entries expire after dockerHostNegativeCacheTTL rather than living for + // the life of the process. + dockerHostsUnavailable map[string]dockerHostFailure + + // now returns the current time; overridable so tests can exercise TTL + // expiry in dockerHostsUnavailable without sleeping. An unexported field + // rather than a package-global clock, since tests here are in-package and + // can set it directly per-pool. Defaults to time.Now in every + // constructor. + now func() time.Time +} + +// dockerHostFailure is a cached GetDockerAt failure, timestamped so it can +// expire (see dockerHostNegativeCacheTTL). +type dockerHostFailure struct { + err error + at time.Time } // NewRuntimePool creates a pool with the auto-detected default runtime. @@ -30,6 +63,7 @@ func NewRuntimePool(opts RuntimeOptions) (*RuntimePool, error) { runtimes: map[RuntimeType]Runtime{rt.Type(): rt}, defaultRT: rt, opts: opts, + now: time.Now, } return pool, nil } @@ -40,6 +74,31 @@ func NewRuntimePoolWithDefault(rt Runtime) *RuntimePool { return &RuntimePool{ runtimes: map[RuntimeType]Runtime{rt.Type(): rt}, defaultRT: rt, + now: time.Now, + } +} + +// NewRuntimePoolWithDockerHost is NewRuntimePoolWithDefault with rt also +// resolvable as the host-pinned runtime for host — GetDockerAt(ctx, host) +// returns the same rt rather than dialing a real engine. Used by +// internal/run's tests so a run carrying a recorded DockerHost resolves to +// the stub. +// +// rt is deliberately the identical object in both p.runtimes and +// p.dockerHosts — a test needs Get(RuntimeDocker) and GetDockerAt(ctx, host) +// to both return the same stub. That double-insertion used to cost two +// things it shouldn't have: Close would close rt twice (fixed below by +// deduping by pointer identity before closing), and ForEachAvailable would +// invoke fn on it twice, because its dedupe only recognized *DockerRuntime +// and this constructor's rt is typically an arbitrary test stub (fixed in +// ForEachAvailable by also comparing against the visited default runtime by +// identity, not just by *DockerRuntime.DaemonHost()). +func NewRuntimePoolWithDockerHost(rt Runtime, host string) *RuntimePool { + return &RuntimePool{ + runtimes: map[RuntimeType]Runtime{rt.Type(): rt}, + defaultRT: rt, + dockerHosts: map[string]Runtime{host: rt}, + now: time.Now, } } @@ -90,19 +149,195 @@ func (p *RuntimePool) Get(typ RuntimeType) (Runtime, error) { return rt, nil } -// ForEachAvailable calls fn for each runtime type that can be successfully -// initialized, skipping unavailable runtimes. Iteration is sequential — -// fn is never called concurrently, so closures may safely append to -// external slices without synchronization. +// dockerAtPingTimeout bounds how long GetDockerAt waits for a pinned endpoint +// to answer, capped further by the caller's ctx. +const dockerAtPingTimeout = 5 * time.Second + +// dockerHostNegativeCacheTTL bounds how long a GetDockerAt failure is cached +// in dockerHostsUnavailable before being retried. 30s is short enough that a +// podman machine or Rancher Desktop VM restarting mid-session — routine, and +// something moat's long-lived daemon will observe — recovers on its own +// within a session, while still being long enough to fail fast against a +// genuinely dead endpoint that's polled repeatedly in a tight loop. +const dockerHostNegativeCacheTTL = 30 * time.Second + +// GetDockerAt returns a Docker runtime pinned to the given endpoint, lazily +// creating and caching it, without mutating the process-wide DOCKER_HOST. Used +// to reconnect to runs recorded against a podman or Rancher Desktop socket. An +// empty host is equivalent to Get(RuntimeDocker). +// +// Construction and the ping happen outside the pool mutex, so a wedged endpoint +// doesn't block unrelated callers. Failures are negatively cached per host so +// repeat attempts fail fast rather than re-paying the timeout, but the cache +// entry expires after dockerHostNegativeCacheTTL — unlike the unavailable map +// above, whose entries are valid for the process lifetime because a runtime +// *type* doesn't come and go, a specific endpoint can recover mid-process. +func (p *RuntimePool) GetDockerAt(ctx context.Context, host string) (Runtime, error) { + if host == "" { + return p.Get(RuntimeDocker) + } + + p.mu.Lock() + if p.closed { + p.mu.Unlock() + return nil, fmt.Errorf("runtime pool is closed") + } + + // If the process-wide DOCKER_HOST already matches and a default docker + // runtime is cached, reuse it rather than creating a second client. + if os.Getenv("DOCKER_HOST") == host { + if rt, ok := p.runtimes[RuntimeDocker]; ok { + p.mu.Unlock() + return rt, nil + } + } + + if rt, ok := p.dockerHosts[host]; ok { + p.mu.Unlock() + return rt, nil + } + + if failure, failed := p.dockerHostsUnavailable[host]; failed { + if p.now().Sub(failure.at) < dockerHostNegativeCacheTTL { + p.mu.Unlock() + return nil, failure.err + } + // Entry has expired: fall through and retry construction/ping below, + // same as if nothing had been cached for this host. + } + p.mu.Unlock() + + // Construct and ping outside the lock — this is the part that can take + // up to dockerAtPingTimeout against a wedged endpoint, and must not + // starve other pool callers. + dockerRT, err := NewDockerRuntimeWithHost(host, p.opts.Sandbox) + if err != nil { + wrapped := fmt.Errorf("docker runtime for host %s: %w%s", host, err, podmanUnreachableHint(host)) + p.cacheDockerHostFailure(host, wrapped) + return nil, wrapped + } + + pingCtx, cancel := context.WithTimeout(ctx, dockerAtPingTimeout) + defer cancel() + if err := dockerRT.Ping(pingCtx); err != nil { + dockerRT.Close() + wrapped := fmt.Errorf("docker host %s not accessible: %w%s", host, err, podmanUnreachableHint(host)) + p.cacheDockerHostFailure(host, wrapped) + return nil, wrapped + } + + p.mu.Lock() + defer p.mu.Unlock() + + if p.closed { + dockerRT.Close() + return nil, fmt.Errorf("runtime pool is closed") + } + + // Another goroutine may have raced us and already inserted a runtime for + // this host while we were pinging outside the lock. Prefer theirs and + // close our duplicate rather than leaking a second client. + if existing, ok := p.dockerHosts[host]; ok { + dockerRT.Close() + return existing, nil + } + + if p.dockerHosts == nil { + p.dockerHosts = make(map[string]Runtime) + } + p.dockerHosts[host] = dockerRT + // A later successful connection supersedes any earlier cached failure. + delete(p.dockerHostsUnavailable, host) + return dockerRT, nil +} + +// cacheDockerHostFailure records a GetDockerAt failure for host, timestamped +// so it expires after dockerHostNegativeCacheTTL, so subsequent calls within +// the TTL fail fast instead of re-attempting construction/ping. +func (p *RuntimePool) cacheDockerHostFailure(host string, err error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.dockerHostsUnavailable == nil { + p.dockerHostsUnavailable = make(map[string]dockerHostFailure) + } + p.dockerHostsUnavailable[host] = dockerHostFailure{err: err, at: p.now()} +} + +// podmanUnreachableHint returns a recovery hint for GetDockerAt errors when +// host looks like a podman endpoint, and "" otherwise. +func podmanUnreachableHint(host string) string { + if !strings.Contains(host, "podman") { + return "" + } + hint := "\n\nThis endpoint was recorded in the run's metadata (~/.moat/runs//metadata.json). To restart podman:\n" + if goruntime.GOOS == "linux" { + hint += " systemctl --user enable --now podman.socket" + } else { + hint += " podman machine start" + } + return hint +} + +// ForEachAvailable calls fn for each runtime type that initializes, then for +// each host-pinned Docker runtime already sitting in dockerHosts. It does not +// populate dockerHosts itself — that only happens via GetDockerAt — so a +// podman or Rancher Desktop engine is visited here only if something already +// called GetDockerAt for its host before this ran. An engine that's up but +// has zero persisted runs pinned to it (so nothing has ever called +// GetDockerAt for its host) is NOT discovered and will NOT be visited: this +// method has no way to enumerate engines it hasn't been told about. +// +// In practice this works for `moat clean` and `moat status` today because +// NewManagerWithOptions calls loadPersistedRuns — which calls GetDockerAt for +// every run's recorded DockerHost — before either command calls +// ForEachAvailable. That's an ordering contract between internal/run and this +// method that ForEachAvailable itself neither documents in code nor can +// verify; a caller that invoked it before loadPersistedRuns would silently +// see only the default runtime's engine. +// +// A pinned runtime matching the already-visited default runtime — by +// identity, or by DaemonHost() for *DockerRuntime — is skipped. Iteration is +// sequential, so fn may append to external slices unsynchronized. // // Note: this lazily initializes runtimes as a side effect. Runtimes // initialized here will be closed when the pool is closed. func (p *RuntimePool) ForEachAvailable(fn func(Runtime) error) error { + var visitedDockerRuntime Runtime + var visitedDockerEndpoint string for _, typ := range AllRuntimeTypes() { rt, err := p.Get(typ) if err != nil { continue // Runtime not available (or pool closed) } + if typ == RuntimeDocker { + visitedDockerRuntime = rt + if dr, ok := rt.(*DockerRuntime); ok { + visitedDockerEndpoint = dr.DaemonHost() + } + } + if err := fn(rt); err != nil { + return err + } + } + + p.mu.Lock() + hostRuntimes := make([]Runtime, 0, len(p.dockerHosts)) + for _, rt := range p.dockerHosts { + hostRuntimes = append(hostRuntimes, rt) + } + p.mu.Unlock() + + for _, rt := range hostRuntimes { + // Pointer-identity check first: robust regardless of the runtime's + // concrete type, and catches e.g. NewRuntimePoolWithDockerHost, which + // seeds the very same object as both the default runtime and a + // host-pinned one (typically a test stub, not a *DockerRuntime). + if visitedDockerRuntime != nil && rt == visitedDockerRuntime { + continue // identical object already visited as the default runtime + } + if dr, ok := rt.(*DockerRuntime); ok && visitedDockerEndpoint != "" && dr.DaemonHost() == visitedDockerEndpoint { + continue // same engine as the already-visited default Docker runtime + } if err := fn(rt); err != nil { return err } @@ -112,6 +347,13 @@ func (p *RuntimePool) ForEachAvailable(fn func(Runtime) error) error { // Close closes all runtimes in the pool. After Close, Get and Default // return errors. +// +// A runtime is closed at most once even if it appears in both p.runtimes and +// p.dockerHosts — which NewRuntimePoolWithDockerHost deliberately does, to +// make a single stub resolve from both Get(RuntimeDocker) and GetDockerAt. +// Deduping by pointer identity (rather than, say, only checking +// *DockerRuntime) keeps that safe for any Runtime implementation, including +// test stubs. func (p *RuntimePool) Close() error { p.mu.Lock() defer p.mu.Unlock() @@ -121,11 +363,27 @@ func (p *RuntimePool) Close() error { } p.closed = true + // Runtime is an interface, so keying a map by it is only safe while + // every implementation is a pointer type — true today (*DockerRuntime, + // *AppleRuntime, and the test stubs). A future value-receiver Runtime + // would still satisfy the interface but panic with "hash of unhashable + // type" the moment it's inserted here. + closed := make(map[Runtime]struct{}, len(p.runtimes)+len(p.dockerHosts)) var firstErr error - for _, rt := range p.runtimes { + closeOnce := func(rt Runtime) { + if _, ok := closed[rt]; ok { + return + } + closed[rt] = struct{}{} if err := rt.Close(); err != nil && firstErr == nil { firstErr = err } } + for _, rt := range p.runtimes { + closeOnce(rt) + } + for _, rt := range p.dockerHosts { + closeOnce(rt) + } return firstErr } diff --git a/internal/container/pool_test.go b/internal/container/pool_test.go index 78cd628d..4b7cc19a 100644 --- a/internal/container/pool_test.go +++ b/internal/container/pool_test.go @@ -4,7 +4,13 @@ import ( "context" "fmt" "io" + "net" + "net/url" + "path/filepath" + goruntime "runtime" + "strings" "testing" + "time" ) // newTestPool creates a RuntimePool for testing, skipping if no runtime is available. @@ -75,11 +81,12 @@ func TestRuntimePoolCloseIdempotent(t *testing.T) { // poolStubRuntime is a minimal Runtime implementation for pool-level tests. // It only implements Type() and Close(); other methods panic if called. type poolStubRuntime struct { - closed bool + closed bool + closeCount int } func (s *poolStubRuntime) Type() RuntimeType { return RuntimeDocker } -func (s *poolStubRuntime) Close() error { s.closed = true; return nil } +func (s *poolStubRuntime) Close() error { s.closed = true; s.closeCount++; return nil } func (s *poolStubRuntime) Ping(context.Context) error { panic("not implemented") } func (s *poolStubRuntime) CreateContainer(context.Context, Config) (string, error) { panic("not implemented") @@ -258,6 +265,223 @@ func TestRuntimePoolUnavailableCached(t *testing.T) { } } +// --- GetDockerAt tests --- + +func TestRuntimePoolGetDockerAtEmptyHost(t *testing.T) { + pool := newStubPool() + defer pool.Close() + + dflt, _ := pool.Default() + rt, err := pool.GetDockerAt(context.Background(), "") + if err != nil { + t.Fatalf("GetDockerAt(\"\"): %v", err) + } + if rt != dflt { + t.Fatal("GetDockerAt(\"\") should return the default runtime, same as Get(RuntimeDocker)") + } +} + +func TestRuntimePoolGetDockerAtCachesPerHost(t *testing.T) { + srv := newFakeDockerAPIServer(t, false) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + pool := newStubPool() + defer pool.Close() + + rt1, err := pool.GetDockerAt(context.Background(), host) + if err != nil { + t.Fatalf("GetDockerAt(%q): %v", host, err) + } + if rt1.Type() != RuntimeDocker { + t.Fatalf("Type() = %v, want %v", rt1.Type(), RuntimeDocker) + } + + rt2, err := pool.GetDockerAt(context.Background(), host) + if err != nil { + t.Fatalf("second GetDockerAt(%q): %v", host, err) + } + if rt1 != rt2 { + t.Fatal("GetDockerAt should return the same cached instance for the same host") + } +} + +func TestRuntimePoolGetDockerAtUnreachable(t *testing.T) { + pool := newStubPool() + defer pool.Close() + + // Port 1 is reserved and nothing should be listening there. + _, err := pool.GetDockerAt(context.Background(), "tcp://127.0.0.1:1") + if err == nil { + t.Fatal("expected error for an unreachable docker host") + } +} + +func TestRuntimePoolGetDockerAtNegativeCache(t *testing.T) { + // A black-hole listener that accepts but never responds, so the first + // (short-deadline) call is forced to fail via ping timeout rather than an + // instant connection-refused. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + _ = conn + } + }() + host := "tcp://" + ln.Addr().String() + + pool := newStubPool() + defer pool.Close() + + shortCtx, shortCancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer shortCancel() + _, err1 := pool.GetDockerAt(shortCtx, host) + if err1 == nil { + t.Fatal("expected error for an unreachable docker host") + } + + // Second call, with a ctx that would happily wait out the full ping + // timeout, should instead hit the negative cache and fail immediately — + // proving the failure was cached rather than a new ping attempted. + longCtx, longCancel := context.WithTimeout(context.Background(), dockerAtPingTimeout) + defer longCancel() + start := time.Now() + _, err2 := pool.GetDockerAt(longCtx, host) + elapsed := time.Since(start) + if err2 == nil { + t.Fatal("expected cached error for an unreachable docker host") + } + if elapsed > 500*time.Millisecond { + t.Errorf("second GetDockerAt took %s; expected a fast-fail from the negative cache instead of re-pinging", elapsed) + } +} + +func TestRuntimePoolGetDockerAtCtxCancellationAbortsPing(t *testing.T) { + // A listener that accepts connections but never responds, so the ping + // would otherwise block for the full dockerAtPingTimeout. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + // Accept and hold the connection open without responding. + _ = conn + } + }() + + pool := newStubPool() + defer pool.Close() + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(50 * time.Millisecond) + cancel() + }() + + start := time.Now() + _, err = pool.GetDockerAt(ctx, "tcp://"+ln.Addr().String()) + elapsed := time.Since(start) + if err == nil { + t.Fatal("expected error from GetDockerAt against a black-hole listener") + } + if elapsed >= dockerAtPingTimeout { + t.Errorf("GetDockerAt took %s, expected ctx cancellation to abort the ping well before the %s ping timeout", elapsed, dockerAtPingTimeout) + } +} + +func TestRuntimePoolGetDockerAtDoesNotBlockOtherCallsDuringPing(t *testing.T) { + // A listener that accepts connections but never responds, simulating a + // wedged endpoint whose ping hangs for the full ping timeout. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + _ = conn // accept and never respond + } + }() + + pool := newStubPool() + defer pool.Close() + + host := "tcp://" + ln.Addr().String() + + // Start a GetDockerAt that will block (against its own ctx deadline) for + // up to dockerAtPingTimeout while pinging the black-hole listener. + go func() { + ctx, cancel := context.WithTimeout(context.Background(), dockerAtPingTimeout) + defer cancel() + _, _ = pool.GetDockerAt(ctx, host) + }() + + // Give the goroutine above time to enter the ping (outside the pool + // mutex). A concurrent Default() call must return promptly rather than + // blocking on the wedged ping. + time.Sleep(50 * time.Millisecond) + + start := time.Now() + if _, err := pool.Default(); err != nil { + t.Fatalf("Default(): %v", err) + } + if elapsed := time.Since(start); elapsed > 500*time.Millisecond { + t.Errorf("Default() took %s while a concurrent GetDockerAt was pinging a black-hole listener; the pool mutex should not be held across the ping", elapsed) + } +} + +func TestRuntimePoolGetDockerAtAfterClose(t *testing.T) { + srv := newFakeDockerAPIServer(t, false) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + pool := newStubPool() + if err := pool.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + if _, err := pool.GetDockerAt(context.Background(), host); err == nil { + t.Fatal("expected error from GetDockerAt after Close()") + } +} + +func TestRuntimePoolCloseClosesDockerHosts(t *testing.T) { + pool := newStubPool() + + stub := &poolStubRuntime{} + pool.dockerHosts = map[string]Runtime{"tcp://127.0.0.1:1234": stub} + + if err := pool.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + if !stub.closed { + t.Error("Close() should close host-pinned docker runtimes") + } +} + func TestRuntimePoolForEachAvailablePropagatesError(t *testing.T) { pool := newStubPool() defer pool.Close() @@ -270,3 +494,222 @@ func TestRuntimePoolForEachAvailablePropagatesError(t *testing.T) { t.Fatalf("expected ForEachAvailable to propagate callback error, got: %v", err) } } + +// --- podman-aware unreachable-endpoint error tests --- + +func TestGetDockerAtPodmanHintOnPodmanLikeHost(t *testing.T) { + pool := newStubPool() + defer pool.Close() + + // Path contains "podman" but nothing is listening there. + host := "unix:///tmp/does-not-exist/podman/podman.sock" + _, err := pool.GetDockerAt(context.Background(), host) + if err == nil { + t.Fatal("expected error for an unreachable podman-shaped host") + } + if !strings.Contains(err.Error(), "podman") { + t.Errorf("error should mention podman, got: %v", err) + } + if !strings.Contains(err.Error(), "metadata.json") { + t.Errorf("error should point at the run's recorded metadata, got: %v", err) + } + wantHint := "podman machine start" + if goruntime.GOOS == "linux" { + wantHint = "systemctl --user enable --now podman.socket" + } + if !strings.Contains(err.Error(), wantHint) { + t.Errorf("error should include a platform-specific restart hint (%q), got: %v", wantHint, err) + } +} + +func TestGetDockerAtNoPodmanHintOnNonPodmanHost(t *testing.T) { + pool := newStubPool() + defer pool.Close() + + host := "unix:///tmp/does-not-exist/docker.sock" + _, err := pool.GetDockerAt(context.Background(), host) + if err == nil { + t.Fatal("expected error for an unreachable host") + } + if strings.Contains(err.Error(), "podman") { + t.Errorf("error for a non-podman-shaped host should not mention podman, got: %v", err) + } +} + +// --- ForEachAvailable host-pinned runtime tests --- + +func TestForEachAvailableVisitsHostPinnedRuntime(t *testing.T) { + srv := newFakeDockerAPIServer(t, false) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + pool := newStubPool() // default runtime is a poolStubRuntime, Type() == RuntimeDocker + defer pool.Close() + + dockerRT, err := NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + pool.dockerHosts = map[string]Runtime{host: dockerRT} + + var visited []Runtime + if err := pool.ForEachAvailable(func(rt Runtime) error { + visited = append(visited, rt) + return nil + }); err != nil { + t.Fatalf("ForEachAvailable: %v", err) + } + + var sawPinned bool + for _, rt := range visited { + if rt == Runtime(dockerRT) { + sawPinned = true + } + } + if !sawPinned { + t.Error("ForEachAvailable should visit host-pinned Docker runtimes from GetDockerAt") + } +} + +func TestForEachAvailableSkipsSameEndpointDuplicate(t *testing.T) { + srv := newFakeDockerAPIServer(t, false) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + // Default runtime and a host-pinned runtime both point at the SAME + // endpoint (constructed independently, as would happen if a run's + // recorded DockerHost happens to equal the process's default docker + // endpoint via two different code paths). + defaultRT, err := NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost (default): %v", err) + } + pinnedRT, err := NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost (pinned): %v", err) + } + + pool := NewRuntimePoolWithDefault(defaultRT) + defer pool.Close() + pool.dockerHosts = map[string]Runtime{host: pinnedRT} + + var visited []Runtime + if err := pool.ForEachAvailable(func(rt Runtime) error { + visited = append(visited, rt) + return nil + }); err != nil { + t.Fatalf("ForEachAvailable: %v", err) + } + + // pinnedRT itself must never be visited (it's a same-endpoint duplicate of + // the default runtime). Other runtime types (e.g. Apple, if available on + // this machine) may legitimately also be visited, so this doesn't assert + // a fixed total count. + var sawPinned, dockerVisits int + for _, rt := range visited { + if rt == Runtime(pinnedRT) { + sawPinned++ + } + if rt.Type() == RuntimeDocker { + dockerVisits++ + } + } + if sawPinned != 0 { + t.Error("ForEachAvailable should not double-visit a host-pinned runtime whose endpoint matches the already-visited default runtime") + } + if dockerVisits != 1 { + t.Errorf("expected exactly 1 Docker-typed visit (the default runtime only), got %d: %+v", dockerVisits, visited) + } +} + +// --- F2: NewRuntimePoolWithDockerHost double-insert tests --- + +// TestNewRuntimePoolWithDockerHostClosesRuntimeOnce pins the Close() half of +// F2: the runtime seeded as both the default and a host-pinned entry must be +// closed exactly once, not once per map it appears in. +func TestNewRuntimePoolWithDockerHostClosesRuntimeOnce(t *testing.T) { + stub := &poolStubRuntime{} + pool := NewRuntimePoolWithDockerHost(stub, "tcp://127.0.0.1:1234") + + if err := pool.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + if stub.closeCount != 1 { + t.Errorf("Close() should close the seeded runtime exactly once, got %d closes", stub.closeCount) + } +} + +// TestNewRuntimePoolWithDockerHostForEachAvailableVisitsOnce pins the +// ForEachAvailable half of F2: the old dedupe only recognized *DockerRuntime +// via DaemonHost(), so a non-*DockerRuntime stub seeded into both maps (as +// NewRuntimePoolWithDockerHost does) was visited twice. +func TestNewRuntimePoolWithDockerHostForEachAvailableVisitsOnce(t *testing.T) { + stub := &poolStubRuntime{} + pool := NewRuntimePoolWithDockerHost(stub, "tcp://127.0.0.1:1234") + defer pool.Close() + + var visits int + if err := pool.ForEachAvailable(func(rt Runtime) error { + if rt == Runtime(stub) { + visits++ + } + return nil + }); err != nil { + t.Fatalf("ForEachAvailable: %v", err) + } + if visits != 1 { + t.Errorf("ForEachAvailable should visit the seeded runtime exactly once (once as default, not again as host-pinned), got %d", visits) + } +} + +// --- F4: dockerHostsUnavailable TTL --- + +// TestRuntimePoolGetDockerAtNegativeCacheExpires proves a negative-cache +// entry is retried once dockerHostNegativeCacheTTL has elapsed, using an +// injected clock rather than sleeping: the endpoint starts unreachable (no +// socket), and only starts answering after the fake clock has been advanced, +// so a stale success would be impossible — only a genuine retry after +// expiry can produce it. +func TestRuntimePoolGetDockerAtNegativeCacheExpires(t *testing.T) { + dir := t.TempDir() + sockPath := filepath.Join(dir, "engine.sock") + host := "unix://" + sockPath + + pool := newStubPool() + defer pool.Close() + + fakeNow := time.Now() + pool.now = func() time.Time { return fakeNow } + + // Nothing listening yet: first call fails and populates the negative + // cache, timestamped at fakeNow. + if _, err := pool.GetDockerAt(context.Background(), host); err == nil { + t.Fatal("expected an error before the endpoint exists") + } + + // Bring the endpoint up, but stay within the TTL: the cached failure + // must still be returned, proving GetDockerAt didn't just get lucky by + // retrying regardless of TTL. + serveFakeDockerAPIUnixSocket(t, sockPath, false) + fakeNow = fakeNow.Add(dockerHostNegativeCacheTTL - time.Second) + if _, err := pool.GetDockerAt(context.Background(), host); err == nil { + t.Fatal("expected the still-cached failure to be returned before TTL expiry") + } + + // Past the TTL: the entry must be treated as a miss and retried, + // succeeding now that the endpoint answers. + fakeNow = fakeNow.Add(2 * time.Second) + rt, err := pool.GetDockerAt(context.Background(), host) + if err != nil { + t.Fatalf("expected the negative-cache entry to expire and retry successfully, got: %v", err) + } + if rt == nil { + t.Fatal("expected a non-nil runtime after cache expiry") + } +} diff --git a/internal/e2e/daemon_test.go b/internal/e2e/daemon_test.go index 52ebb6fc..79197c76 100644 --- a/internal/e2e/daemon_test.go +++ b/internal/e2e/daemon_test.go @@ -57,7 +57,7 @@ func TestDaemonStartsWithRun(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Verify the daemon is running via lock file daemonDir := filepath.Join(config.GlobalConfigDir(), "proxy") @@ -121,7 +121,7 @@ func TestDaemonReusedAcrossRuns(t *testing.T) { if err != nil { t.Fatalf("Create r1: %v", err) } - defer mgr.Destroy(context.Background(), r1.ID) + defer mgr.Destroy(context.Background(), r1.ID, true) // Read daemon info after first run daemonDir := filepath.Join(config.GlobalConfigDir(), "proxy") @@ -141,7 +141,7 @@ func TestDaemonReusedAcrossRuns(t *testing.T) { if err != nil { t.Fatalf("Create r2: %v", err) } - defer mgr.Destroy(context.Background(), r2.ID) + defer mgr.Destroy(context.Background(), r2.ID, true) // Read daemon info after second run lock2, err := daemon.ReadLockFile(daemonDir) @@ -207,7 +207,7 @@ func TestDaemonNetworkLogging(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -290,7 +290,7 @@ func TestDaemonCredentialInjection(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -360,7 +360,7 @@ func TestDaemonProxyEnvInContainer(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -437,7 +437,7 @@ func TestDaemonNetworkLoggingIsolation(t *testing.T) { if err != nil { t.Fatalf("Create r1: %v", err) } - defer mgr.Destroy(context.Background(), r1.ID) + defer mgr.Destroy(context.Background(), r1.ID, true) ws2 := createTestWorkspace(t) r2, err := mgr.Create(ctx, run.Options{ @@ -452,7 +452,7 @@ func TestDaemonNetworkLoggingIsolation(t *testing.T) { if err != nil { t.Fatalf("Create r2: %v", err) } - defer mgr.Destroy(context.Background(), r2.ID) + defer mgr.Destroy(context.Background(), r2.ID, true) // Start both runs if err := mgr.Start(ctx, r1.ID); err != nil { diff --git a/internal/e2e/docker_test.go b/internal/e2e/docker_test.go index 4404f8c0..1a339039 100644 --- a/internal/e2e/docker_test.go +++ b/internal/e2e/docker_test.go @@ -71,7 +71,7 @@ func TestDockerDependency(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -256,7 +256,7 @@ func TestDockerDindDependency(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -386,7 +386,7 @@ func TestDockerDindIsolation(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -499,7 +499,7 @@ CMD ["echo", "Hello from BuildKit"] if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) diff --git a/internal/e2e/e2e_test.go b/internal/e2e/e2e_test.go index 75ff4447..be2ffc83 100644 --- a/internal/e2e/e2e_test.go +++ b/internal/e2e/e2e_test.go @@ -260,7 +260,7 @@ func TestProxyBindsToLocalhostOnly(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Start the run if err := mgr.Start(ctx, r.ID); err != nil { @@ -328,7 +328,7 @@ func TestProxyNotAccessibleFromNetwork(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -417,7 +417,7 @@ func TestNetworkRequestsAreCaptured(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -511,7 +511,7 @@ func TestContainerCanReachProxyViaHostDockerInternal(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -554,7 +554,7 @@ func TestRunWithoutGrantsNoProxy(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // With no grants, the proxy port should still be set (daemon manages the proxy), // but the run should have no proxy auth token since no credentials are needed. @@ -590,7 +590,7 @@ func TestLogsAreCaptured(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -660,7 +660,7 @@ func TestWorkspaceIsMounted(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -728,7 +728,7 @@ func TestConfigEnvironmentVariables(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -902,7 +902,7 @@ func TestAppleContainerBasicRun(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -982,7 +982,7 @@ func TestAppleContainerWithProxy(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Verify daemon proxy is configured if r.ProxyPort == 0 { @@ -1210,7 +1210,7 @@ func TestSSHAuthSockEnvSetInContainer(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Verify SSH server was started if r.SSHAgentServer == nil { @@ -1345,7 +1345,7 @@ func TestDependencyNodeRuntime(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1407,7 +1407,7 @@ func TestDependencyPythonRuntime(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1469,7 +1469,7 @@ func TestDependencyGoRuntime(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1533,7 +1533,7 @@ func TestDependencyMultipleRuntimes(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1602,7 +1602,7 @@ func TestDependencyNpmPackage(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1664,7 +1664,7 @@ func TestDependencyGitHubBinary(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1728,7 +1728,7 @@ func TestDependencyMetaBundle(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -1805,7 +1805,7 @@ func TestInteractiveContainer(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Verify Interactive flag was set if !r.Interactive { @@ -1984,7 +1984,7 @@ func TestClaudeLogSyncMountTarget(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) diff --git a/internal/e2e/endpoints_test.go b/internal/e2e/endpoints_test.go index bab543a5..8ddc44c6 100644 --- a/internal/e2e/endpoints_test.go +++ b/internal/e2e/endpoints_test.go @@ -293,7 +293,7 @@ func TestProxyTokenValidInContainer(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -397,7 +397,7 @@ func startEndpointRun(t *testing.T, name string, ports map[string]int, cmd []str } if err := mgr.Start(ctx, r.ID); err != nil { - mgr.Destroy(context.Background(), r.ID) + mgr.Destroy(context.Background(), r.ID, true) mgr.Close() cancel() t.Fatalf("Start: %v", err) @@ -411,7 +411,7 @@ func startEndpointRun(t *testing.T, name string, ports map[string]int, cmd []str cleanup := func() { mgr.Stop(context.Background(), r.ID) - mgr.Destroy(context.Background(), r.ID) + mgr.Destroy(context.Background(), r.ID, true) mgr.Close() cancel() } diff --git a/internal/e2e/host_traffic_test.go b/internal/e2e/host_traffic_test.go index e9b5e544..9b87b8a7 100644 --- a/internal/e2e/host_traffic_test.go +++ b/internal/e2e/host_traffic_test.go @@ -120,7 +120,7 @@ func TestHostTrafficBlockedByDefault(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -206,7 +206,7 @@ func TestHostTrafficAllowedWithNetworkHost(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -268,7 +268,7 @@ func TestHostTrafficWrongPortBlocked(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -346,7 +346,7 @@ func TestHostTrafficStrictPolicyWithRules(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -413,7 +413,7 @@ func TestHostTrafficMultiplePorts(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -480,7 +480,7 @@ func TestHostTrafficProxyBypass(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) diff --git a/internal/e2e/join_test.go b/internal/e2e/join_test.go index 73bf002b..5fac6057 100644 --- a/internal/e2e/join_test.go +++ b/internal/e2e/join_test.go @@ -70,7 +70,7 @@ func TestJoinHeadless(t *testing.T) { if err != nil { t.Fatalf("Create primary run: %v", err) } - defer mgr.Destroy(context.Background(), primaryRun.ID) + defer mgr.Destroy(context.Background(), primaryRun.ID, true) defer mgr.Stop(context.Background(), primaryRun.ID) if err := mgr.Start(ctx, primaryRun.ID); err != nil { diff --git a/internal/e2e/logs_capture_test.go b/internal/e2e/logs_capture_test.go index 531145c2..5d985abc 100644 --- a/internal/e2e/logs_capture_test.go +++ b/internal/e2e/logs_capture_test.go @@ -39,7 +39,12 @@ func TestLogsCapturedInAttachedMode(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + // forceRunning=true is deliberate here and at every other e2e cleanup + // site: this is teardown, not the behavior under test, and a failing + // cleanup must not mask the real assertion. The forceRunning==false + // guard itself is covered by unit tests + // (TestDestroyForceBypassesRunningGuard in internal/run). + defer mgr.Destroy(context.Background(), r.ID, true) // Start and wait for completion (simulating attached mode) if err := mgr.Start(ctx, r.ID); err != nil { @@ -112,7 +117,7 @@ func TestLogsCapturedInDetachedMode(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Start without waiting (detached mode) if err := mgr.Start(ctx, r.ID); err != nil { @@ -181,7 +186,7 @@ func TestLogsCapturedInInteractiveMode(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // StartAttached simulates interactive mode // We can't truly test interactive mode in automated tests, but we can @@ -247,7 +252,7 @@ func TestLogsCapturedAfterStop(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Start the run if err := mgr.Start(ctx, r.ID); err != nil { @@ -323,7 +328,7 @@ func TestLogsAlwaysExistForAudit(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) diff --git a/internal/e2e/mcp_test.go b/internal/e2e/mcp_test.go index 8790bf6e..3bc9d991 100644 --- a/internal/e2e/mcp_test.go +++ b/internal/e2e/mcp_test.go @@ -107,7 +107,7 @@ func TestMCPCredentialInjection_E2E(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Verify daemon proxy is configured (required for credential injection) if r.ProxyPort == 0 { @@ -296,7 +296,7 @@ mcp: if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Build relay URLs using daemon proxy if r.ProxyPort == 0 { diff --git a/internal/e2e/services_test.go b/internal/e2e/services_test.go index a95ae97d..af25a861 100644 --- a/internal/e2e/services_test.go +++ b/internal/e2e/services_test.go @@ -41,7 +41,7 @@ func cleanupRun(t *testing.T, mgr *run.Manager, runID string) { if err := mgr.Stop(ctx, runID); err != nil { t.Logf("cleanup: stop run %s: %v", runID, err) } - if err := mgr.Destroy(ctx, runID); err != nil { + if err := mgr.Destroy(ctx, runID, true); err != nil { t.Logf("cleanup: destroy run %s: %v", runID, err) } }) @@ -397,7 +397,7 @@ func TestServiceCleanup(t *testing.T) { } // Destroy the run (should clean up service containers) - if err := mgr.Destroy(ctx, r.ID); err != nil { + if err := mgr.Destroy(ctx, r.ID, true); err != nil { t.Fatalf("Destroy: %v", err) } diff --git a/internal/e2e/tui_test.go b/internal/e2e/tui_test.go index b2d607e5..96c08ac2 100644 --- a/internal/e2e/tui_test.go +++ b/internal/e2e/tui_test.go @@ -57,7 +57,7 @@ func TestAppleTUIWriterPassthrough(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) // Create a tui.Writer with runtime="apple" to exercise the init phase. // This simulates what setupStatusBar does, but without requiring a real terminal. @@ -119,7 +119,7 @@ func TestAppleTUIWriterAltScreenDuringInit(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) var outputBuf bytes.Buffer bar := tui.NewStatusBar(r.ID, r.Name, "apple") @@ -181,7 +181,7 @@ func TestAppleTUIWriterMultipleWrites(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) var outputBuf bytes.Buffer bar := tui.NewStatusBar(r.ID, r.Name, "apple") diff --git a/internal/e2e/volumes_test.go b/internal/e2e/volumes_test.go index 031f142a..19d679f0 100644 --- a/internal/e2e/volumes_test.go +++ b/internal/e2e/volumes_test.go @@ -113,7 +113,7 @@ func TestVolumePersistenceAcrossRuns(t *testing.T) { } // Destroy run 1 (volume should persist) - if err := mgr.Destroy(ctx, r1.ID); err != nil { + if err := mgr.Destroy(ctx, r1.ID, true); err != nil { t.Fatalf("Destroy run 1: %v", err) } @@ -133,7 +133,7 @@ func TestVolumePersistenceAcrossRuns(t *testing.T) { if err != nil { t.Fatalf("Create run 2: %v", err) } - defer mgr.Destroy(context.Background(), r2.ID) + defer mgr.Destroy(context.Background(), r2.ID, true) if err := mgr.Start(ctx, r2.ID); err != nil { t.Fatalf("Start run 2: %v", err) @@ -206,7 +206,7 @@ func TestVolumeReadOnly(t *testing.T) { if err != nil { t.Fatalf("Create: %v", err) } - defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Destroy(context.Background(), r.ID, true) if err := mgr.Start(ctx, r.ID); err != nil { t.Fatalf("Start: %v", err) @@ -327,7 +327,7 @@ func TestVolumeIsolation(t *testing.T) { t.Fatalf("Agent1 failed to write to volume\nLogs:%s", formatLogEntries(logs1)) } - if err := mgr.Destroy(ctx, r1.ID); err != nil { + if err := mgr.Destroy(ctx, r1.ID, true); err != nil { t.Fatalf("Destroy agent1: %v", err) } @@ -347,7 +347,7 @@ func TestVolumeIsolation(t *testing.T) { if err != nil { t.Fatalf("Create agent2: %v", err) } - defer mgr.Destroy(context.Background(), r2.ID) + defer mgr.Destroy(context.Background(), r2.ID, true) if err := mgr.Start(ctx, r2.ID); err != nil { t.Fatalf("Start agent2: %v", err) diff --git a/internal/run/edge_cases_test.go b/internal/run/edge_cases_test.go index f2c44cc3..8a4186b6 100644 --- a/internal/run/edge_cases_test.go +++ b/internal/run/edge_cases_test.go @@ -2,16 +2,23 @@ package run import ( "context" + "encoding/json" "errors" "fmt" "io" + "net" + "net/http" "os" "path/filepath" + "runtime" "strings" "sync" + "sync/atomic" "testing" "time" + "github.com/containerd/errdefs" + "github.com/docker/docker/api/types" "github.com/majorcontext/moat/internal/container" "github.com/majorcontext/moat/internal/deps" "github.com/majorcontext/moat/internal/routing" @@ -159,6 +166,19 @@ func (f *flexibleRuntime) ExecInteractive(context.Context, string, []string, con // newEdgeCaseManager creates a Manager with the given runtime and a temporary // routes directory. The returned cleanup function should be deferred. func newEdgeCaseManager(t *testing.T, rt container.Runtime) *Manager { + t.Helper() + return newEdgeCaseManagerPool(t, container.NewRuntimePoolWithDefault(rt)) +} + +// newEdgeCaseManagerAtHost is newEdgeCaseManager with rt also pinned as the +// runtime for dockerHost, so runs carrying that recorded endpoint resolve to +// the stub rather than dialing a real engine. +func newEdgeCaseManagerAtHost(t *testing.T, rt container.Runtime, dockerHost string) *Manager { + t.Helper() + return newEdgeCaseManagerPool(t, container.NewRuntimePoolWithDockerHost(rt, dockerHost)) +} + +func newEdgeCaseManagerPool(t *testing.T, pool *container.RuntimePool) *Manager { t.Helper() tmpDir := t.TempDir() routeDir := filepath.Join(tmpDir, "routes") @@ -166,14 +186,19 @@ func newEdgeCaseManager(t *testing.T, rt container.Runtime) *Manager { if err != nil { t.Fatal(err) } + lifecycle, err := routing.NewLifecycle(filepath.Join(tmpDir, "proxy"), 0) + if err != nil { + t.Fatal(err) + } monitorCtx, monitorCancel := context.WithCancel(context.Background()) t.Cleanup(func() { monitorCancel() }) return &Manager{ - runtimePool: container.NewRuntimePoolWithDefault(rt), - runs: make(map[string]*Run), - routes: routes, - monitorCtx: monitorCtx, - monitorCancel: monitorCancel, + runtimePool: pool, + runs: make(map[string]*Run), + routes: routes, + proxyLifecycle: lifecycle, + monitorCtx: monitorCtx, + monitorCancel: monitorCancel, } } @@ -420,6 +445,411 @@ func TestStopHandlesContainerStopError(t *testing.T) { } } +// TestStopFailsLoudOnAmbiguousNotFound verifies that stopping a docker-type +// run with NO recorded endpoint, whose container the engine reports as +// not-found, fails loudly and leaves the run running — rather than silently +// recording "stopped" and potentially orphaning a container that is really +// alive on a different engine (e.g. started on Docker, stopped under +// MOAT_RUNTIME=podman). This is a wiring test: it mocks the probe seam +// directly to pin Stop's branching regardless of the seam's own semantics +// (self-exclusion etc.), which the end-to-end tests further down cover +// against the real probe. +// withReachablePodmanEndpoint pins the probe seam so the ambiguity +// precondition is set by the test rather than by whatever engines the host +// happens to have installed. +func withReachablePodmanEndpoint(t *testing.T, endpoint string, ok bool) { + t.Helper() + prev := reachablePodmanEndpointOtherThan + reachablePodmanEndpointOtherThan = func(context.Context, string) (string, bool) { return endpoint, ok } + t.Cleanup(func() { reachablePodmanEndpointOtherThan = prev }) +} + +func newLegacyNotFoundRun(id string) *Run { + return &Run{ + ID: id, + Name: id, + ContainerID: "ctr-elsewhere", + Runtime: "docker", + DockerHost: "", // legacy run: no recorded endpoint + State: StateRunning, + exitCh: make(chan struct{}), + } +} + +func TestStopFailsLoudOnAmbiguousNotFound(t *testing.T) { + withReachablePodmanEndpoint(t, "unix:///tmp/podman/podman.sock", true) + + rt := &flexibleRuntime{ + done: make(chan struct{}), + stopFn: func(_ context.Context, _ string) error { + return fmt.Errorf("stopping container: %w", errdefs.ErrNotFound) + }, + } + m := newEdgeCaseManager(t, rt) + + r := newLegacyNotFoundRun("run_ambiguous") + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + err := m.Stop(context.Background(), r.ID) + if err == nil { + t.Fatal("Stop should fail loudly on not-found with no recorded endpoint and a reachable, distinct podman engine") + } + if !strings.Contains(err.Error(), "destroy --force-running") { + t.Errorf("error should point at the recovery path, got: %v", err) + } + if !strings.Contains(err.Error(), "podman") { + t.Errorf("error should say a podman engine was detected, got: %v", err) + } + if r.GetState() != StateRunning { + t.Errorf("state should revert to running so the user can retry, got %s", r.GetState()) + } +} + +// TestStopBenignNotFoundWhenNoPodmanPresent is the companion to +// TestStopFailsLoudOnAmbiguousNotFound: with no other live podman engine +// reachable, there is no second engine to be ambiguous about, so a legacy +// run whose container was removed out of band (docker rm, a prune) must +// still stop cleanly. This is the pre-existing Docker behavior and must not +// regress. +func TestStopBenignNotFoundWhenNoPodmanPresent(t *testing.T) { + withReachablePodmanEndpoint(t, "", false) + + rt := &flexibleRuntime{ + done: make(chan struct{}), + stopFn: func(_ context.Context, _ string) error { + return fmt.Errorf("stopping container: %w", errdefs.ErrNotFound) + }, + } + m := newEdgeCaseManager(t, rt) + + r := newLegacyNotFoundRun("run_docker_only") + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + if err := m.Stop(context.Background(), r.ID); err != nil { + t.Fatalf("Stop should proceed when no other podman engine is reachable: %v", err) + } + if r.GetState() != StateStopped { + t.Errorf("state should be stopped, got %s", r.GetState()) + } +} + +// TestStopBenignNotFoundWhenEndpointRecorded is the companion: when the run's +// engine endpoint IS recorded, we are pinned to the right engine, so a +// not-found container genuinely means it is gone — Stop proceeds normally. +func TestStopBenignNotFoundWhenEndpointRecorded(t *testing.T) { + // A path that no engine listens on: the pool is seeded with the stub for + // this host, so resolution must come from the seam rather than a dial. A + // real socket path here would route through GetDockerAt to the host's own + // daemon and silently bypass stopFn. + const pinnedHost = "unix:///nonexistent/moat-test-pinned.sock" + + var stopCalls int32 + rt := &flexibleRuntime{ + done: make(chan struct{}), + runtimeType: container.RuntimeDocker, + stopFn: func(_ context.Context, _ string) error { + atomic.AddInt32(&stopCalls, 1) + return fmt.Errorf("stopping container: %w", errdefs.ErrNotFound) + }, + } + m := newEdgeCaseManagerAtHost(t, rt, pinnedHost) + + r := &Run{ + ID: "run_pinned", + Name: "pinned", + ContainerID: "ctr-gone", + Runtime: "docker", + DockerHost: pinnedHost, // pinned endpoint + State: StateRunning, + exitCh: make(chan struct{}), + } + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + if err := m.Stop(context.Background(), r.ID); err != nil { + t.Fatalf("Stop should proceed when the endpoint is pinned: %v", err) + } + if r.GetState() != StateStopped { + t.Errorf("state should be stopped, got %s", r.GetState()) + } + // Without this the test passes even if Stop never reached the runtime. + if n := atomic.LoadInt32(&stopCalls); n != 1 { + t.Errorf("StopContainer should have been called once on the pinned runtime, got %d", n) + } +} + +// --- End-to-end coverage of the reachable-podman probe itself --- +// +// The two tests below drive Stop against the real +// container.ReachablePodmanEndpointOtherThan (the default value of the +// reachablePodmanEndpointOtherThan seam), rather than mocking it, so they +// pin the actual self-exclusion semantics — not just Stop's branching on +// whatever a mock hands back. They need a real *container.DockerRuntime (so +// Stop's rt.(*container.DockerRuntime) type assertion succeeds and produces +// a real, comparable DaemonHost()) and a real socket file at a path +// podmanSocketCandidates() will actually scan. +// +// rootfulPodmanSocketPath is podmanSocketCandidates' hardcoded Linux rootful +// default (see internal/container/detect.go's defaultDetectEnviron). It is +// not reachable via a seam from outside that package, so these tests claim +// the literal path; the rootless candidate, by contrast, is fully +// controllable via XDG_RUNTIME_DIR. +const rootfulPodmanSocketPath = "/run/podman/podman.sock" + +// serveFakePodmanUnixSocket starts a minimal Docker-Engine-API server on ln +// whose /version response carries podman's "Podman Engine" component marker +// (see container.IsPodmanEngine), and whose every other route 404s — which +// the docker client surfaces as errdefs.ErrNotFound, standing in for +// StopContainer on a container that's genuinely gone. Closed via t.Cleanup. +func serveFakePodmanUnixSocket(t *testing.T, ln net.Listener) { + t.Helper() + + version := types.Version{ + APIVersion: "1.44", + Version: "24.0.0", + Components: []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}}, + } + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/version") { + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) // e.g. .../containers//stop -> ErrNotFound + }) + + srv := &http.Server{Handler: mux, ReadHeaderTimeout: 10 * time.Second} + go func() { _ = srv.Serve(ln) }() + t.Cleanup(func() { _ = srv.Close() }) +} + +// claimRootfulPodmanSocketPath serves a fake podman engine at the literal +// rootfulPodmanSocketPath, skipping (not failing) the test if the path can't +// be claimed — e.g. a sandboxed CI runner without permission to create +// /run/podman, or a real podman.sock already listening there. Removes the +// socket file via t.Cleanup. +func claimRootfulPodmanSocketPath(t *testing.T) { + t.Helper() + dir := filepath.Dir(rootfulPodmanSocketPath) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Skipf("cannot create %s (need root in this environment): %v", dir, err) + } + _ = os.Remove(rootfulPodmanSocketPath) // stale socket from a prior run + ln, err := net.Listen("unix", rootfulPodmanSocketPath) + if err != nil { + t.Skipf("cannot claim %s: %v", rootfulPodmanSocketPath, err) + } + t.Cleanup(func() { os.Remove(rootfulPodmanSocketPath) }) + serveFakePodmanUnixSocket(t, ln) +} + +// TestStopWarnsWhenPodmanIsOnlyEngine is the regression this ticket fixes: +// on a host where podman is present but IS the same engine Stop already +// queried (moat auto-detected it, or MOAT_RUNTIME=podman was used), a +// not-found container is not ambiguous — podman is the only engine in play, +// so Stop must warn and record stopped, not hard-fail. The old condition (a +// podman socket merely exists on disk) could not tell this case apart from a +// genuinely different second engine and would wrongly hard-fail it; this +// test fails against that condition (see the ticket's load-bearing check). +func TestStopWarnsWhenPodmanIsOnlyEngine(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("relies on the linux rootful podman socket path") + } + claimRootfulPodmanSocketPath(t) + // No rootless candidate present, so the rootful one (which equals the + // queried endpoint below) is the only candidate in play. + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + + dockerRT, err := container.NewDockerRuntimeWithHost("unix://"+rootfulPodmanSocketPath, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + t.Cleanup(func() { dockerRT.Close() }) + + m := newEdgeCaseManager(t, dockerRT) + + r := newLegacyNotFoundRun("run_podman_only") + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + if err := m.Stop(context.Background(), r.ID); err != nil { + t.Fatalf("Stop should warn, not fail, when the only reachable podman engine is the one it already queried: %v", err) + } + if r.GetState() != StateStopped { + t.Errorf("state should be stopped, got %s", r.GetState()) + } +} + +// TestStopFailsLoudOnDistinctReachablePodmanEngine is the companion: a +// second, distinct, live podman engine (the rootless candidate) is reachable +// alongside the one Stop already queried (the rootful candidate) — a +// genuinely ambiguous case that must still fail loudly. +func TestStopFailsLoudOnDistinctReachablePodmanEngine(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("relies on the linux rootful podman socket path") + } + claimRootfulPodmanSocketPath(t) // this becomes the queried endpoint + + xdgDir := t.TempDir() + t.Setenv("XDG_RUNTIME_DIR", xdgDir) + rootlessDir := filepath.Join(xdgDir, "podman") + if err := os.MkdirAll(rootlessDir, 0o755); err != nil { + t.Fatalf("mkdir rootless podman dir: %v", err) + } + rootlessLn, err := net.Listen("unix", filepath.Join(rootlessDir, "podman.sock")) + if err != nil { + t.Fatalf("listen on rootless podman socket: %v", err) + } + serveFakePodmanUnixSocket(t, rootlessLn) // a second, distinct engine + + dockerRT, err := container.NewDockerRuntimeWithHost("unix://"+rootfulPodmanSocketPath, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + t.Cleanup(func() { dockerRT.Close() }) + + m := newEdgeCaseManager(t, dockerRT) + + r := newLegacyNotFoundRun("run_ambiguous_real") + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + err = m.Stop(context.Background(), r.ID) + if err == nil { + t.Fatal("Stop should fail loudly: a second, distinct, live podman engine is reachable") + } + if !strings.Contains(err.Error(), "destroy --force-running") { + t.Errorf("error should point at the recovery path, got: %v", err) + } + if r.GetState() != StateRunning { + t.Errorf("state should revert to running so the user can retry, got %s", r.GetState()) + } +} + +// TestStopRestoresStateWhenRuntimeResolutionFails verifies that when Stop +// cannot resolve the run's runtime (e.g. the pinned DOCKER_HOST endpoint is +// unreachable — a stopped podman machine), the run's state is restored rather +// than left in StateStopping. Without the restore, the next Stop would hit the +// "already stopped" early return and silently no-op while the container may +// still exist. +func TestStopRestoresStateWhenRuntimeResolutionFails(t *testing.T) { + rt := &flexibleRuntime{done: make(chan struct{})} + m := newEdgeCaseManager(t, rt) + + // A docker-type run pinned to an endpoint that cannot exist: GetDockerAt + // fails to connect/ping and negative-caches the endpoint. + r := &Run{ + ID: "run_rt_unreachable", + Name: "rt-unreachable", + ContainerID: "ctr-somewhere", + Runtime: "docker", + DockerHost: "unix://" + filepath.Join(t.TempDir(), "no-such-machine.sock"), + State: StateRunning, + exitCh: make(chan struct{}), + } + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + err := m.Stop(context.Background(), r.ID) + if err == nil { + t.Fatal("Stop should fail when the run's runtime cannot be resolved") + } + if got := r.GetState(); got != StateRunning { + t.Fatalf("state should be restored to running after resolution failure, got %s", got) + } + + // Second Stop must not be a silent no-op: with the state restored it + // re-attempts resolution and errors again (negative-cached endpoint). + err = m.Stop(context.Background(), r.ID) + if err == nil { + t.Fatal("second Stop should also fail, not silently succeed") + } + if got := r.GetState(); got != StateRunning { + t.Fatalf("state should still be running after second failed Stop, got %s", got) + } +} + +// TestStopBenignNotFoundOnAppleRuntime is the second companion: the loud-fail +// is docker-only. Apple's StopContainer swallows not-found, and cross-engine +// ambiguity does not apply, so an apple run proceeds even without an endpoint. +func TestStopBenignNotFoundOnAppleRuntime(t *testing.T) { + rt := &flexibleRuntime{ + done: make(chan struct{}), + runtimeType: container.RuntimeApple, + stopFn: func(_ context.Context, _ string) error { + return fmt.Errorf("stopping container: %w", errdefs.ErrNotFound) + }, + } + m := newEdgeCaseManager(t, rt) + + r := &Run{ + ID: "run_apple", + Name: "apple", + ContainerID: "ctr-gone", + Runtime: "apple", + State: StateRunning, + exitCh: make(chan struct{}), + } + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + + if err := m.Stop(context.Background(), r.ID); err != nil { + t.Fatalf("Stop should proceed for apple runs: %v", err) + } + if r.GetState() != StateStopped { + t.Errorf("state should be stopped, got %s", r.GetState()) + } +} + +// TestDestroyForceBypassesRunningGuard verifies the escape hatch that keeps the +// loud-fail from wedging a run: destroy refuses a running run by default but +// --force (force=true) tears it down anyway. +func TestDestroyForceBypassesRunningGuard(t *testing.T) { + rt := &flexibleRuntime{done: make(chan struct{})} + m := newEdgeCaseManager(t, rt) + + newRunning := func(id string) *Run { + r := &Run{ID: id, Name: id, ContainerID: "ctr", State: StateRunning, exitCh: make(chan struct{})} + m.mu.Lock() + m.runs[r.ID] = r + m.mu.Unlock() + return r + } + + // Default: refused, with a hint at the escape hatch. + newRunning("run_guard") + err := m.Destroy(context.Background(), "run_guard", false) + if err == nil || !strings.Contains(err.Error(), "--force-running") { + t.Fatalf("destroy without force should refuse a running run and mention --force-running, got: %v", err) + } + + // Companion: force tears it down. + newRunning("run_forced") + if err := m.Destroy(context.Background(), "run_forced", true); err != nil { + t.Fatalf("destroy --force-running should tear down a running run: %v", err) + } +} + // TestStopHandlesRemoveContainerError verifies that Stop completes // even when RemoveContainer fails. func TestStopHandlesRemoveContainerError(t *testing.T) { diff --git a/internal/run/manager.go b/internal/run/manager.go index 52dbc009..122de46e 100644 --- a/internal/run/manager.go +++ b/internal/run/manager.go @@ -64,11 +64,60 @@ type Manager struct { monitorWg sync.WaitGroup } +// runtimeForEndpoint resolves the runtime for an existing run, shared by +// runtimeForRun and loadPersistedRuns so the two can't drift on how DockerHost +// is interpreted. A docker-type run with a recorded endpoint pins to it; +// everything else (including legacy runs with none) uses the pool's default. +func (m *Manager) runtimeForEndpoint(ctx context.Context, runtimeType, dockerHost string) (container.Runtime, error) { + if runtimeType == string(container.RuntimeDocker) && dockerHost != "" { + return m.runtimePool.GetDockerAt(ctx, dockerHost) + } + return m.runtimePool.Get(container.RuntimeType(runtimeType)) +} + // runtimeForRun returns the correct container runtime for an existing run. // It uses the run's Runtime field to look up the matching runtime from the pool. // For legacy runs without a Runtime field, falls back to the default runtime. func (m *Manager) runtimeForRun(r *Run) (container.Runtime, error) { - return m.runtimePool.Get(container.RuntimeType(r.Runtime)) + // TODO: no ctx parameter here, so GetDockerAt can't derive its ping + // timeout from a caller deadline. + return m.runtimeForEndpoint(context.Background(), r.Runtime, r.DockerHost) +} + +// reachablePodmanEndpointOtherThan reports a live podman endpoint distinct +// from the one Stop just queried, or "", false if none is reachable. A +// package variable so tests can drive both branches of Stop's +// ambiguous-not-found guard without a live podman engine. +var reachablePodmanEndpointOtherThan = container.ReachablePodmanEndpointOtherThan + +// recordedDockerHost returns the endpoint to persist for a new docker-type +// run: the runtime's resolved DaemonHost rather than the DOCKER_HOST env var, +// since the pool may have selected a podman or Rancher Desktop socket by some +// other route. Non-docker runtimes have no such endpoint and record "". +func recordedDockerHost(rt container.Runtime) string { + dr, ok := rt.(*container.DockerRuntime) + if !ok { + return "" + } + return dr.DaemonHost() +} + +// recordedEngine returns the engine identity to persist for a new docker-type +// run: an authoritative "docker" or "podman" asked of the connected daemon at +// creation time, rather than guessed later from the endpoint string. Non-docker +// runtimes have no such identity and record "". Callers must not fail run +// creation when this returns "" — see the call site in Create(). +func recordedEngine(ctx context.Context, rt container.Runtime) string { + dr, ok := rt.(*container.DockerRuntime) + if !ok { + return "" + } + engine, err := dr.EngineName(ctx) + if err != nil { + log.Debug("engine identification failed, recording no engine", "error", err) + return "" + } + return engine } // defaultRuntime returns the default runtime for new run creation. diff --git a/internal/run/manager_create.go b/internal/run/manager_create.go index b85c6c20..e8576a96 100644 --- a/internal/run/manager_create.go +++ b/internal/run/manager_create.go @@ -1222,6 +1222,17 @@ region = %s } r.Image = containerImage r.Runtime = string(m.defaultRuntime().Type()) + if r.Runtime == string(container.RuntimeDocker) { + // Pin the resolved endpoint so reconnects reach the same engine. + r.DockerHost = recordedDockerHost(m.defaultRuntime()) + // Ask the connected daemon which engine it actually is, for display + // only (see storage.Metadata.Engine). Bounded so a slow/unreachable + // daemon can't stall run creation; identification failure must never + // fail Create — record "" and move on. + engineCtx, engineCancel := context.WithTimeout(ctx, 5*time.Second) + r.Engine = recordedEngine(engineCtx, m.defaultRuntime()) + engineCancel() + } needsCustomImage := imageSpec.NeedsCustomImage(hasDeps) diff --git a/internal/run/manager_docker_host_test.go b/internal/run/manager_docker_host_test.go new file mode 100644 index 00000000..5410c0e5 --- /dev/null +++ b/internal/run/manager_docker_host_test.go @@ -0,0 +1,344 @@ +package run + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + + "github.com/docker/docker/api/types" + "github.com/majorcontext/moat/internal/container" +) + +// newFakeDockerAPIServer starts an httptest server serving just enough of the +// Docker Engine API for client.Client's version negotiation, Ping, and +// ServerVersion calls, so container.RuntimePool.GetDockerAt can construct and +// ping a real docker client against it without a live daemon. Mirrors the +// helper of the same name in internal/container/detect_test.go. +func newFakeDockerAPIServer(t *testing.T) *httptest.Server { + t.Helper() + + version := types.Version{APIVersion: "1.44", Version: "24.0.0"} + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/version") { + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// TestRuntimeForRunDockerWithoutDockerHost verifies the unchanged path: +// a docker run with no recorded DockerHost (the default-socket case, +// including all runs persisted before DockerHost existed) resolves through +// the pool's ordinary Get(), returning the default runtime. +func TestRuntimeForRunDockerWithoutDockerHost(t *testing.T) { + stub := &stubRuntime{} + m := mgrWithRuntime(stub) + + r := &Run{Runtime: string(container.RuntimeDocker)} + rt, err := m.runtimeForRun(r) + if err != nil { + t.Fatalf("runtimeForRun: %v", err) + } + if rt != container.Runtime(stub) { + t.Fatal("expected runtimeForRun to return the pool's default runtime when DockerHost is empty") + } +} + +// TestRuntimeForRunDockerWithDockerHost is the companion case: a docker run +// recorded against a non-default DOCKER_HOST (podman or Rancher Desktop) +// must resolve to a runtime pinned to that host via GetDockerAt, not the +// pool's default runtime. +func TestRuntimeForRunDockerWithDockerHost(t *testing.T) { + srv := newFakeDockerAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + stub := &stubRuntime{} + m := mgrWithRuntime(stub) + + r := &Run{Runtime: string(container.RuntimeDocker), DockerHost: host} + rt, err := m.runtimeForRun(r) + if err != nil { + t.Fatalf("runtimeForRun: %v", err) + } + if rt == container.Runtime(stub) { + t.Fatal("expected runtimeForRun to route to a host-pinned runtime, not the pool default") + } + if rt.Type() != container.RuntimeDocker { + t.Fatalf("Type() = %v, want %v", rt.Type(), container.RuntimeDocker) + } +} + +// TestRecordedDockerHost_DockerRuntime pins the creation-side decision: a +// DockerRuntime records its resolved DaemonHost, non-empty even with +// DOCKER_HOST unset, guarding against regressing to the raw env var. +func TestRecordedDockerHost_DockerRuntime(t *testing.T) { + t.Setenv("DOCKER_HOST", "tcp://this-is-not-the-runtimes-endpoint:9999") + + srv := newFakeDockerAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + rt, err := container.NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + got := recordedDockerHost(rt) + if got == "" { + t.Fatal("recordedDockerHost returned empty for a docker runtime; want the runtime's resolved endpoint") + } + if got != rt.DaemonHost() { + t.Fatalf("recordedDockerHost = %q, want rt.DaemonHost() = %q", got, rt.DaemonHost()) + } + if got == os.Getenv("DOCKER_HOST") { + t.Fatalf("recordedDockerHost returned the raw DOCKER_HOST env var (%q) instead of the runtime's actual endpoint", got) + } +} + +// TestRecordedDockerHost_NonDockerRuntime is the companion case: a +// non-docker runtime (Apple containers, or any Runtime that isn't a +// *container.DockerRuntime) has no Docker-API endpoint to record. +func TestRecordedDockerHost_NonDockerRuntime(t *testing.T) { + stub := &stubRuntime{} + if got := recordedDockerHost(stub); got != "" { + t.Fatalf("recordedDockerHost(non-docker runtime) = %q, want empty", got) + } +} + +// newFakePodmanAPIServer is like newFakeDockerAPIServer, but its /version +// response carries the "Podman Engine" component podman's real compat API +// reports — the marker DockerRuntime.EngineName uses to identify podman. +func newFakePodmanAPIServer(t *testing.T) *httptest.Server { + t.Helper() + + version := types.Version{ + APIVersion: "1.44", + Version: "4.9.0", + Components: []types.ComponentVersion{{Name: "Podman Engine", Version: "4.9.0"}}, + } + body, err := json.Marshal(version) + if err != nil { + t.Fatalf("marshal version: %v", err) + } + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/version") { + w.Header().Set("Content-Type", "application/json") + w.Write(body) + return + } + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.44") + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// TestRecordedEngine_Docker pins the creation-side identity call: a +// DockerRuntime backed by a real-Docker-shaped compat API asks the engine via +// EngineName and records "docker" — not a guess derived from the endpoint. +func TestRecordedEngine_Docker(t *testing.T) { + srv := newFakeDockerAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + rt, err := container.NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + if got := recordedEngine(context.Background(), rt); got != "docker" { + t.Fatalf("recordedEngine = %q, want %q", got, "docker") + } +} + +// TestRecordedEngine_Podman is the companion case: a DockerRuntime backed by +// a podman-shaped compat API (the "Podman Engine" component marker) records +// "podman" — the exact scenario the path-sniffing heuristic could miss (a +// podman machine whose name lacks "podman", or podman over tcp://). +func TestRecordedEngine_Podman(t *testing.T) { + srv := newFakePodmanAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + // Deliberately a tcp:// endpoint with no "podman" in the path at all, so a + // path-sniffing heuristic would have mislabeled this "docker". + host := "tcp://" + u.Host + + rt, err := container.NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + + if got := recordedEngine(context.Background(), rt); got != "podman" { + t.Fatalf("recordedEngine = %q, want %q", got, "podman") + } +} + +// TestRecordedEngine_NonDockerRuntime is the companion case: a non-docker +// runtime has no engine identity to probe. +func TestRecordedEngine_NonDockerRuntime(t *testing.T) { + stub := &stubRuntime{} + if got := recordedEngine(context.Background(), stub); got != "" { + t.Fatalf("recordedEngine(non-docker runtime) = %q, want empty", got) + } +} + +// TestRecordedEngine_ProbeFailureNeverPanics guards the "must never fail +// creation" contract at the recordedEngine call site: a runtime whose +// EngineName call errors (server closed) must yield "" rather than a panic +// or propagated error. +func TestRecordedEngine_ProbeFailureNeverPanics(t *testing.T) { + srv := newFakeDockerAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + rt, err := container.NewDockerRuntimeWithHost(host, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost: %v", err) + } + defer rt.Close() + srv.Close() // Engine probe will now fail to connect. + + if got := recordedEngine(context.Background(), rt); got != "" { + t.Fatalf("recordedEngine after server close = %q, want empty", got) + } +} + +// TestRuntimeForEndpoint_RoutingDrift is the drift-guard for the routing +// helper shared by runtimeForRun and loadPersistedRuns: a recorded endpoint +// pins to it, an empty one falls back to the pool default. +func TestRuntimeForEndpoint_RoutingDrift(t *testing.T) { + srv := newFakeDockerAPIServer(t) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("parsing server URL: %v", err) + } + host := "tcp://" + u.Host + + stub := &stubRuntime{} + m := mgrWithRuntime(stub) + + t.Run("non-empty dockerHost routes to the pinned endpoint", func(t *testing.T) { + rt, err := m.runtimeForEndpoint(context.Background(), string(container.RuntimeDocker), host) + if err != nil { + t.Fatalf("runtimeForEndpoint: %v", err) + } + if rt == container.Runtime(stub) { + t.Fatal("expected routing to the host-pinned runtime, not the pool default") + } + dr, ok := rt.(*container.DockerRuntime) + if !ok { + t.Fatalf("expected a *container.DockerRuntime, got %T", rt) + } + if dr.DaemonHost() != host { + t.Fatalf("DaemonHost() = %q, want %q", dr.DaemonHost(), host) + } + }) + + t.Run("empty dockerHost routes to the pool default", func(t *testing.T) { + rt, err := m.runtimeForEndpoint(context.Background(), string(container.RuntimeDocker), "") + if err != nil { + t.Fatalf("runtimeForEndpoint: %v", err) + } + if rt != container.Runtime(stub) { + t.Fatal("expected routing to the pool default when dockerHost is empty") + } + }) +} + +// TestRuntimeForRun_ReproducedScenario recreates the reported bug: a run +// created on one docker-type engine, reconnected in a process whose pool +// default is a different one (`MOAT_RUNTIME=podman moat stop `). +// runtimeForRun must resolve by recorded endpoint, not the pool default. +func TestRuntimeForRun_ReproducedScenario(t *testing.T) { + // "recorded" simulates the real Docker daemon the run was created against. + recordedSrv := newFakeDockerAPIServer(t) + recordedURL, err := url.Parse(recordedSrv.URL) + if err != nil { + t.Fatalf("parsing recorded server URL: %v", err) + } + recordedHost := "tcp://" + recordedURL.Host + + // "podmanDefault" simulates a podman-shaped engine that a later process + // (MOAT_RUNTIME=podman) resolves as its pool default. It is a genuine + // *container.DockerRuntime (Type() == "docker"), just pointed at a + // different endpoint than the run was actually created on. + podmanSrv := newFakeDockerAPIServer(t) + podmanURL, err := url.Parse(podmanSrv.URL) + if err != nil { + t.Fatalf("parsing podman server URL: %v", err) + } + podmanHost := "tcp://" + podmanURL.Host + + podmanDefault, err := container.NewDockerRuntimeWithHost(podmanHost, false) + if err != nil { + t.Fatalf("NewDockerRuntimeWithHost(podman default): %v", err) + } + defer podmanDefault.Close() + + m := mgrWithRuntime(podmanDefault) + + r := &Run{Runtime: string(container.RuntimeDocker), DockerHost: recordedHost} + rt, err := m.runtimeForRun(r) + if err != nil { + t.Fatalf("runtimeForRun: %v", err) + } + + dr, ok := rt.(*container.DockerRuntime) + if !ok { + t.Fatalf("expected a *container.DockerRuntime, got %T", rt) + } + if dr.DaemonHost() == podmanHost { + t.Fatal("runtimeForRun returned the mismatched pool default (podman-shaped engine) instead of the recorded endpoint — this is the reproduced bug") + } + if dr.DaemonHost() != recordedHost { + t.Fatalf("DaemonHost() = %q, want the recorded endpoint %q", dr.DaemonHost(), recordedHost) + } +} diff --git a/internal/run/manager_lifecycle.go b/internal/run/manager_lifecycle.go index e896d8cb..3294588a 100644 --- a/internal/run/manager_lifecycle.go +++ b/internal/run/manager_lifecycle.go @@ -285,11 +285,34 @@ func (m *Manager) Stop(ctx context.Context, runID string) error { rt, rtErr := m.runtimeForRun(r) if rtErr != nil { + // Leaving the run in StateStopping would make every later Stop hit the + // "already stopped" early return above and silently no-op. + r.SetState(currentState) return fmt.Errorf("resolving runtime for run %s: %w", runID, rtErr) } // Stop the main container if err := rt.StopContainer(ctx, r.ContainerID); err != nil { + // Not-found on a run with no recorded endpoint is ambiguous only where + // a second, different, live docker-API engine could actually be + // holding the container. Gating on "a podman socket file exists" was + // too broad: every pre-existing run whose container is legitimately + // gone (prune, reboot, --rm, a manual docker rm) hit the hard-fail on + // any host that merely has an idle podman socket lying around. We + // probe for a reachable podman endpoint instead, and explicitly + // exclude the one we just queried — if that endpoint IS podman (e.g. + // moat auto-detected it), it's the only engine in play and there's no + // ambiguity at all, so that case must warn like every other host. + var queriedEndpoint string + if dr, ok := rt.(*container.DockerRuntime); ok { + queriedEndpoint = dr.DaemonHost() + } + if container.IsNotFound(err) && r.DockerHost == "" && rt.Type() == container.RuntimeDocker { + if otherEndpoint, ok := reachablePodmanEndpointOtherThan(ctx, queriedEndpoint); ok { + r.SetState(currentState) + return fmt.Errorf("run %s: no such container on the docker engine, but a different, live podman engine is also reachable at %s; this run has no recorded engine endpoint, so moat cannot rule out the container still running there. Retry 'moat stop' with the runtime the run was created on, or if it's genuinely gone: 'moat destroy --force-running %s'", runID, otherEndpoint, runID) + } + } ui.Warnf("%v", err) log.Debug("failed to stop container", "container_id", r.ContainerID, "error", err) } @@ -343,8 +366,9 @@ func (m *Manager) Wait(ctx context.Context, runID string) error { } } -// Destroy removes a run and its resources. -func (m *Manager) Destroy(ctx context.Context, runID string) error { +// Destroy removes a run and its resources. forceRunning skips the guard that +// requires a run to be stopped first. +func (m *Manager) Destroy(ctx context.Context, runID string, forceRunning bool) error { m.mu.Lock() r, ok := m.runs[runID] if !ok { @@ -353,8 +377,12 @@ func (m *Manager) Destroy(ctx context.Context, runID string) error { } m.mu.Unlock() - if r.GetState() == StateRunning { - return fmt.Errorf("cannot destroy running run %s; stop it first", runID) + // forceRunning tears down a run that can't be stopped cleanly — its + // container is on an engine this process can't reach. Deliberately separate + // from --force (the extraction-snapshot guard), so neither silently grants + // the other. Cleanup below is idempotent. + if r.GetState() == StateRunning && !forceRunning { + return fmt.Errorf("cannot destroy running run %s; stop it first (or use 'moat destroy --force-running %s')", runID, runID) } // Clean up all run resources (idempotent - may already be done by Stop/monitorContainerExit) diff --git a/internal/run/manager_persistence.go b/internal/run/manager_persistence.go index 2ea2fcd0..2ff478da 100644 --- a/internal/run/manager_persistence.go +++ b/internal/run/manager_persistence.go @@ -8,7 +8,6 @@ import ( "sync" "time" - "github.com/majorcontext/moat/internal/container" "github.com/majorcontext/moat/internal/log" "github.com/majorcontext/moat/internal/storage" ) @@ -93,8 +92,9 @@ func (m *Manager) loadPersistedRuns(ctx context.Context) error { } defer func() { <-sem }() - // Look up the runtime for this run (lazy-init if needed). - rt, rtErr := m.runtimePool.Get(container.RuntimeType(info.meta.Runtime)) + // Look up the runtime for this run (lazy-init if needed), + // pinned to the endpoint it was recorded against. + rt, rtErr := m.runtimeForEndpoint(ctx, info.meta.Runtime, info.meta.DockerHost) if rtErr != nil { log.Debug("runtime not available, preserving persisted state", "id", info.runID, "runtime", info.meta.Runtime, "error", rtErr) @@ -195,6 +195,8 @@ func (m *Manager) registerPersistedRun(runState State, stateConfirmed bool, skip Agent: meta.Agent, Image: meta.Image, Runtime: meta.Runtime, + DockerHost: meta.DockerHost, + Engine: meta.Engine, Ports: meta.Ports, State: runState, ContainerID: meta.ContainerID, diff --git a/internal/run/run.go b/internal/run/run.go index a6a46ccb..f9b59268 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -54,6 +54,8 @@ type Run struct { Agent string // Agent type from config (e.g., "claude-code", "codex") Image string // Container image used for this run Runtime string // Container runtime type ("docker" or "apple") + DockerHost string // DOCKER_HOST endpoint the run's containers live on, when non-default (docker runtime only) + Engine string // Engine actually behind the docker runtime at creation time ("docker" or "podman"), for display only — see storage.Metadata.Engine ProviderMeta map[string]string // Provider-specific metadata (e.g., claude_session_id) Ports map[string]int // endpoint name -> container port HostPorts map[string]int // endpoint name -> host port (after binding) @@ -210,6 +212,8 @@ func (r *Run) SaveMetadata() error { WorktreePath: r.WorktreePath, WorktreeRepoID: r.WorktreeRepoID, Runtime: r.Runtime, + DockerHost: r.DockerHost, + Engine: r.Engine, BuildkitContainerID: r.BuildkitContainerID, NetworkID: r.NetworkID, ServiceContainers: r.ServiceContainers, diff --git a/internal/storage/storage.go b/internal/storage/storage.go index eeb46d6b..a2c03653 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -47,6 +47,24 @@ type Metadata struct { // Used during reconciliation to skip cross-runtime container state checks. Runtime string `json:"runtime,omitempty"` + // DockerHost records the Docker-API endpoint the run's containers live on + // (the runtime's resolved endpoint, not the raw DOCKER_HOST env var), so + // lifecycle commands reconnect to the same engine — a podman or Rancher + // Desktop socket rather than the reconnecting process's default. Additive: + // an older CLI that rewrites metadata drops it, reverting that run to + // legacy default-runtime routing. + DockerHost string `json:"docker_host,omitempty"` + + // Engine records which engine was actually behind the Docker-API endpoint + // when the run was created ("docker" or "podman"), for display only (e.g. + // `moat list`/`moat status` labeling a podman run "docker (podman)"). + // Distinct from DockerHost: DockerHost exists so lifecycle commands can + // *reconnect* to the right endpoint; Engine exists so they can *report* + // the right identity. Neither is derived from the other. Additive: a run + // persisted before this field existed has Engine == "" and must render as + // plain "docker" rather than being guessed from DockerHost. + Engine string `json:"engine,omitempty"` + // BuildKit sidecar fields (docker:dind only) BuildkitContainerID string `json:"buildkit_container_id,omitempty"` NetworkID string `json:"network_id,omitempty"` diff --git a/internal/storage/storage_test.go b/internal/storage/storage_test.go index 70e93d94..74d858f6 100644 --- a/internal/storage/storage_test.go +++ b/internal/storage/storage_test.go @@ -49,6 +49,37 @@ func TestRunStoreMetadata(t *testing.T) { } } +// TestRunStoreMetadata_EngineRoundTrip pins that Engine — the authoritative +// engine identity recorded at run creation (see storage.Metadata.Engine) — +// survives a save/load cycle distinctly from DockerHost, which has a +// different job (reconnection, not reporting). +func TestRunStoreMetadata_EngineRoundTrip(t *testing.T) { + dir := t.TempDir() + s, _ := NewRunStore(dir, "run_test8901") + + meta := Metadata{ + Name: "claude-code", + Workspace: "/home/user/project", + Runtime: "docker", + DockerHost: "unix:///run/podman/podman.sock", + Engine: "podman", + } + if err := s.SaveMetadata(meta); err != nil { + t.Fatalf("SaveMetadata: %v", err) + } + + loaded, err := s.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if loaded.Engine != "podman" { + t.Errorf("Engine = %q, want %q", loaded.Engine, "podman") + } + if loaded.DockerHost != meta.DockerHost { + t.Errorf("DockerHost = %q, want %q (Engine round-trip must not affect it)", loaded.DockerHost, meta.DockerHost) + } +} + func TestRunStoreDir(t *testing.T) { dir := t.TempDir() s, err := NewRunStore(dir, "run_dirtest1") @@ -120,6 +151,81 @@ func TestLoadMetadataPreservesAllFields(t *testing.T) { } } +func TestLoadMetadataPreservesDockerHost(t *testing.T) { + dir := t.TempDir() + s, _ := NewRunStore(dir, "run_dockerhost1") + + meta := Metadata{ + Name: "test-agent", + Runtime: "docker", + DockerHost: "tcp://127.0.0.1:12345", + } + if err := s.SaveMetadata(meta); err != nil { + t.Fatalf("SaveMetadata: %v", err) + } + + loaded, err := s.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if loaded.DockerHost != meta.DockerHost { + t.Errorf("DockerHost = %q, want %q", loaded.DockerHost, meta.DockerHost) + } +} + +// TestLoadMetadataMissingDockerHostDefaultsEmpty is the companion to +// TestLoadMetadataPreservesDockerHost: metadata written before DockerHost +// existed (or simply the default-socket case, where it's never set) must +// load with an empty DockerHost rather than erroring or defaulting to some +// other value. +func TestLoadMetadataMissingDockerHostDefaultsEmpty(t *testing.T) { + dir := t.TempDir() + s, _ := NewRunStore(dir, "run_dockerhost2") + + meta := Metadata{ + Name: "test-agent", + Runtime: "docker", + } + if err := s.SaveMetadata(meta); err != nil { + t.Fatalf("SaveMetadata: %v", err) + } + + loaded, err := s.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if loaded.DockerHost != "" { + t.Errorf("DockerHost = %q, want empty", loaded.DockerHost) + } +} + +// TestLoadMetadataOldFileWithoutDockerHostField simulates loading metadata +// written by a pre-DockerHost version of moat (a JSON file that simply lacks +// the field), verifying old metadata files load unchanged. +func TestLoadMetadataOldFileWithoutDockerHostField(t *testing.T) { + dir := t.TempDir() + s, err := NewRunStore(dir, "run_dockerhost3") + if err != nil { + t.Fatalf("NewRunStore: %v", err) + } + + oldJSON := `{"name":"test-agent","workspace":"/workspace","runtime":"docker"}` + if err := os.WriteFile(filepath.Join(s.Dir(), "metadata.json"), []byte(oldJSON), 0o644); err != nil { + t.Fatalf("writing legacy metadata: %v", err) + } + + loaded, err := s.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if loaded.DockerHost != "" { + t.Errorf("DockerHost = %q, want empty for legacy metadata file", loaded.DockerHost) + } + if loaded.Runtime != "docker" { + t.Errorf("Runtime = %q, want %q", loaded.Runtime, "docker") + } +} + func TestLogWriter(t *testing.T) { dir := t.TempDir() s, _ := NewRunStore(dir, "run_logs1234")