From d3b03b4cf70521bdd425acec479e4f6304627225 Mon Sep 17 00:00:00 2001 From: Cameron Daniel Date: Thu, 23 Jul 2026 12:36:02 +1000 Subject: [PATCH 1/5] Add Apple Container as an alternative container backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workbench shelled out to `docker` for every container service. On Apple silicon, Apple's `container` tool runs Linux containers in lightweight VMs without the Docker Desktop dependency, and its CLI is largely argument-compatible with Docker. Introduce a ContainerBackend abstraction driven by a single ContainerRunner. The Docker path is unchanged; the Apple path adapts for the differences that break Docker's assumptions: - No `wait` subcommand: poll `container inspect` for run state, then report a best-effort exit code (inspect does not reliably expose the process exit code, so a clean stop with no code defaults to 0). - No host-gateway alias: containers reach host services (e.g. the OTLP trace collector) via the vmnet gateway IP instead of host.docker.internal. - Daemon/OS floor: require Apple silicon + macOS 26 and a running `container` system service, checked at startup. Selection is via a new global `container_backend` setting (docker | apple | auto, default auto — prefer Apple when installed on Apple silicon) plus an optional `apple.gateway_ip`. The active backend is surfaced per container service in the TUI detail pane and in `bench status` (TYPE column and --json). Adds docs/apple-container.md and updates configuration/troubleshooting docs. --- docs/apple-container.md | 74 ++++++++++ docs/configuration.md | 18 ++- docs/troubleshooting.md | 13 +- internal/api/handlers.go | 2 + internal/cli/cli.go | 38 +++-- internal/config/config.go | 34 ++++- internal/config/config_test.go | 62 ++++++++ internal/config/validate.go | 12 ++ internal/runner/apple.go | 193 +++++++++++++++++++++++++ internal/runner/backend.go | 116 +++++++++++++++ internal/runner/backend_test.go | 208 +++++++++++++++++++++++++++ internal/runner/container.go | 126 ++++++++-------- internal/runner/docker.go | 55 ++++++- internal/service/state.go | 3 + internal/supervisor/buildenv_test.go | 19 ++- internal/supervisor/reload.go | 2 +- internal/supervisor/supervisor.go | 16 ++- internal/tui/app.go | 3 + internal/tui/session_remote.go | 1 + 19 files changed, 896 insertions(+), 99 deletions(-) create mode 100644 docs/apple-container.md create mode 100644 internal/runner/apple.go create mode 100644 internal/runner/backend.go create mode 100644 internal/runner/backend_test.go diff --git a/docs/apple-container.md b/docs/apple-container.md new file mode 100644 index 0000000..7e319a4 --- /dev/null +++ b/docs/apple-container.md @@ -0,0 +1,74 @@ +# Apple Container backend + +On Apple silicon, workbench can run container services on +[Apple's `container`](https://github.com/apple/container) tool instead of +Docker. Apple `container` runs each Linux container in its own lightweight +virtual machine and integrates directly with Virtualization.framework, avoiding +the Docker Desktop dependency. + +The `bench.yaml` service schema is unchanged — the same `container:` block runs +on either backend. Only the global `container_backend` setting differs. + +## Requirements + +- Apple silicon Mac (arm64). +- macOS 26 (Tahoe) or later. +- The [`container`](https://github.com/apple/container) CLI installed and its + system service running: + ```bash + container system start + ``` + +## Selecting the backend + +```yaml +global: + container_backend: auto # docker | apple | auto (default) + apple: + gateway_ip: 192.168.64.1 +``` + +- **`auto`** (default) — use Apple `container` when running on Apple silicon with + the `container` binary installed; otherwise use Docker. +- **`docker`** — always use Docker. +- **`apple`** — always use Apple `container`. Startup fails with a clear message + if the host doesn't meet the requirements above. + +The active backend is shown per container service in the TUI detail pane and in +`bench status` (the `TYPE` column reads `container/apple` or `container/docker`, +and `--json` includes a `backend` field). + +## Host connectivity (tracing) + +Docker exposes `host.docker.internal` so a container can reach services on the +host (workbench uses this for the OTLP trace collector). Apple `container` has +no such alias. Instead, containers reach the host at the vmnet **gateway IP**, +which defaults to `192.168.64.1`. + +Workbench injects that gateway IP as the OTLP endpoint host for Apple-backend +container services automatically. If you've changed the `container` default +subnet (in `~/.config/container/config.toml`), set `apple.gateway_ip` to the +matching gateway address. + +## Differences from Docker + +- **Isolation** — one lightweight VM per container, rather than shared-kernel + namespaces. Startup is slightly slower but isolation is stronger. +- **Exit codes** — `container` has no `wait` subcommand and does not reliably + expose a container's process exit code via `inspect`. Workbench detects that a + container has stopped by polling its status; the reported exit code is + best-effort. This mainly affects `restart.policy: on-failure`, which may not + distinguish a clean exit from a crash as precisely as it does on Docker. +- **Anonymous volumes** — Docker's `-v` removal flag drops anonymous volumes on + cleanup. Apple `container` has no equivalent; anonymous volumes are not + auto-removed. +- **Host networking / `--add-host`** — not used by workbench on this backend; + host access goes through the gateway IP instead. + +## Out of scope + +- Building images (`container build`) — workbench only runs pre-built images. +- `container system dns` domains — workbench uses the gateway IP so it never + needs `sudo` or to disable iCloud Private Relay. +- Starting the `container` system service — workbench reports if it's not + running but does not run `container system start` for you. diff --git a/docs/configuration.md b/docs/configuration.md index a609b64..7b482a5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -90,9 +90,25 @@ Run `bench validate` to surface these errors without starting any services. | `watch_debounce` | duration | `300ms` | Default debounce for file watchers | | `env` | map | | Global environment variables applied to all services | | `env_file` | path | | Global .env file loaded for all services | -| `container_prefix` | string | dirname | Prefix for Docker container names (e.g. `{prefix}-{service}`) | +| `container_prefix` | string | dirname | Prefix for container names (e.g. `{prefix}-{service}`) | +| `container_backend`| string | `auto` | Container runtime: `docker`, `apple`, or `auto` | +| `apple` | object | | Apple `container` backend settings | | `tracing` | object | | Tracing configuration | +#### Container backend + +Container services run on Docker by default. On Apple silicon you can run them +on [Apple's `container`](apple-container.md) tool instead. + +| Field | Type | Default | Description | +| ------------------- | ------ | ------- | -------------------------------------------------------- | +| `container_backend` | string | `auto` | `docker`, `apple`, or `auto` (prefer Apple when present) | +| `apple.gateway_ip` | string | `192.168.64.1` | Host IP an Apple container uses to reach the host | + +`auto` selects the Apple backend when running on Apple silicon with the +`container` binary installed, otherwise Docker. See +[apple-container.md](apple-container.md) for requirements and caveats. + #### Tracing | Field | Type | Default | Description | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index ce06e02..084c194 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -119,17 +119,24 @@ a **container** service produces nothing, check the endpoint the service is actually exporting to. A container's `localhost` is its own loopback, not the host — so `http://localhost:` silently fails with connection-refused. -Workbench injects `http://host.docker.internal:` for container services -and adds a `host.docker.internal:host-gateway` alias to each container run. If -spans still don't arrive: +Workbench injects the OTLP endpoint using a backend-specific host: on Docker it +uses `http://host.docker.internal:` and adds a +`host.docker.internal:host-gateway` alias to each container run; on the +[Apple backend](apple-container.md) it uses the vmnet gateway IP +(`http://192.168.64.1:` by default). If spans still don't arrive: - Confirm the service didn't override `OTEL_EXPORTER_OTLP_ENDPOINT` itself (any env layer outranks the injected default — see `docs/configuration.md`). A hardcoded `localhost` in the service's own config is the usual culprit. - Verify the container can reach the host collector: ```bash + # Docker docker exec getent hosts host.docker.internal + # Apple container + container exec sh -c 'nc -z -v 192.168.64.1 ' ``` + On the Apple backend, if you've changed the `container` default subnet, set + `global.apple.gateway_ip` to the matching gateway address. - Confirm the collector is listening on the host: `lsof -nP -iTCP: -sTCP:LISTEN`. ## Getting debug output diff --git a/internal/api/handlers.go b/internal/api/handlers.go index 986dd1b..e0423cf 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -47,6 +47,7 @@ type ServiceStatus struct { DisplayName string `json:"display_name"` Status string `json:"status"` Type string `json:"type"` + Backend string `json:"backend,omitempty"` PID int `json:"pid,omitempty"` ContainerID string `json:"container_id,omitempty"` Image string `json:"image,omitempty"` @@ -92,6 +93,7 @@ func (s *Server) buildServiceStatus(key string) ServiceStatus { DisplayName: snap.Name(), Status: snap.Status.String(), Type: snap.ServiceType, + Backend: snap.Backend, PID: snap.PID, ContainerID: snap.ContainerID, Image: snap.Image, diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 2a747b4..9b53f83 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -309,10 +309,11 @@ func runUp(args []string) int { applyProfileFilter(cfg, profiles) } - // Check Docker availability if any container services exist + // Check container backend availability if any container services exist. for _, svc := range cfg.Services { if svc.IsContainer() { - if err := runner.CheckDocker(); err != nil { + backend := runner.ResolveBackend(cfg.Global) + if err := backend.Available(); err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) return 1 } @@ -839,10 +840,11 @@ func runStatus(args []string) int { // Table output order, _ := cfg.StartOrder() - fmt.Printf("%-20s %-10s %-12s %-10s %s\n", "SERVICE", "TYPE", "STATUS", "RESTARTS", "COMMAND/IMAGE") - fmt.Printf("%-20s %-10s %-12s %-10s %s\n", + backendName := runner.ResolveBackend(cfg.Global).Name() + fmt.Printf("%-20s %-17s %-12s %-10s %s\n", "SERVICE", "TYPE", "STATUS", "RESTARTS", "COMMAND/IMAGE") + fmt.Printf("%-20s %-17s %-12s %-10s %s\n", strings.Repeat("-", 20), - strings.Repeat("-", 10), + strings.Repeat("-", 17), strings.Repeat("-", 12), strings.Repeat("-", 10), strings.Repeat("-", 30)) @@ -856,12 +858,12 @@ func runStatus(args []string) int { svcType := "process" cmdStr := "" if svc.IsContainer() { - svcType = "container" + svcType = typeLabel("container", backendName) cmdStr = svc.Container.Image } else if svc.Command != nil { cmdStr = svc.Command.String() } - fmt.Printf("%-20s %-10s %-12s %-10s %s\n", + fmt.Printf("%-20s %-17s %-12s %-10s %s\n", key, svcType, status, @@ -955,10 +957,10 @@ func statusFromRunning(client *api.Client, jsonOut, showWhy bool, serviceFilter } } - fmt.Printf("%-20s %-10s %-12s %-8s %-10s %-12s %s\n", "SERVICE", "TYPE", "STATUS", "PID", "RESTARTS", "UPTIME", "REASON") - fmt.Printf("%-20s %-10s %-12s %-8s %-10s %-12s %s\n", + fmt.Printf("%-20s %-17s %-12s %-8s %-10s %-12s %s\n", "SERVICE", "TYPE", "STATUS", "PID", "RESTARTS", "UPTIME", "REASON") + fmt.Printf("%-20s %-17s %-12s %-8s %-10s %-12s %s\n", strings.Repeat("-", 20), - strings.Repeat("-", 10), + strings.Repeat("-", 17), strings.Repeat("-", 12), strings.Repeat("-", 8), strings.Repeat("-", 10), @@ -975,9 +977,9 @@ func statusFromRunning(client *api.Client, jsonOut, showWhy bool, serviceFilter uptime = svc.Uptime } reason := statusReason(svc, showWhy) - fmt.Printf("%-20s %-10s %-12s %-8s %-10d %-12s %s\n", + fmt.Printf("%-20s %-17s %-12s %-8s %-10d %-12s %s\n", svc.Key, - svc.Type, + typeLabel(svc.Type, svc.Backend), svc.Status, pid, svc.RestartCount, @@ -987,6 +989,15 @@ func statusFromRunning(client *api.Client, jsonOut, showWhy bool, serviceFilter return 0 } +// typeLabel renders a service's type for status tables, appending the container +// backend (e.g. "container/apple") so the active runtime is visible. +func typeLabel(svcType, backend string) string { + if svcType == "container" && backend != "" { + return svcType + "/" + backend + } + return svcType +} + // statusReason picks a short, single-line reason to show in the REASON column. // With showWhy=true, last_error is always shown if non-empty. Otherwise only // statuses that already imply a problem (failed/backoff/restarting) surface it, @@ -1018,6 +1029,7 @@ func statusJSON(cfg *config.Config) int { type svcStatus struct { Key string `json:"key"` Type string `json:"type"` + Backend string `json:"backend,omitempty"` Command string `json:"command,omitempty"` Image string `json:"image,omitempty"` Dir string `json:"dir,omitempty"` @@ -1025,6 +1037,7 @@ func statusJSON(cfg *config.Config) int { } var services []svcStatus order, _ := cfg.StartOrder() + backendName := runner.ResolveBackend(cfg.Global).Name() for _, key := range order { svc := cfg.Services[key] s := svcStatus{ @@ -1034,6 +1047,7 @@ func statusJSON(cfg *config.Config) int { } if svc.IsContainer() { s.Type = "container" + s.Backend = backendName s.Image = svc.Container.Image } else { s.Type = "process" diff --git a/internal/config/config.go b/internal/config/config.go index 8c5c131..33bbebf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,6 +12,13 @@ import ( "gopkg.in/yaml.v3" ) +// Container backend identifiers for GlobalConfig.ContainerBackend. +const ( + BackendDocker = "docker" + BackendApple = "apple" + BackendAuto = "auto" +) + type Config struct { Version int `yaml:"version"` Extends string `yaml:"extends"` @@ -26,7 +33,20 @@ type GlobalConfig struct { Env map[string]string `yaml:"env"` EnvFile string `yaml:"env_file"` ContainerPrefix string `yaml:"container_prefix"` - Tracing TracingConfig `yaml:"tracing"` + // ContainerBackend selects the runtime for container services: + // "docker", "apple", or "auto" (default). "auto" prefers Apple's + // `container` on Apple silicon when installed, otherwise Docker. + ContainerBackend string `yaml:"container_backend"` + Apple AppleConfig `yaml:"apple"` + Tracing TracingConfig `yaml:"tracing"` +} + +// AppleConfig holds settings specific to the Apple `container` backend. +type AppleConfig struct { + // GatewayIP is the vmnet gateway address a container uses to reach services + // on the macOS host (e.g. the OTLP trace collector). Defaults to + // 192.168.64.1, the `container` default subnet gateway. + GatewayIP string `yaml:"gateway_ip"` } // Duration wraps time.Duration for YAML unmarshaling from strings like "10s". @@ -631,6 +651,12 @@ func mergeGlobal(p, c GlobalConfig) GlobalConfig { if c.ContainerPrefix != "" { out.ContainerPrefix = c.ContainerPrefix } + if c.ContainerBackend != "" { + out.ContainerBackend = c.ContainerBackend + } + if c.Apple.GatewayIP != "" { + out.Apple.GatewayIP = c.Apple.GatewayIP + } out.Tracing = mergeTracing(p.Tracing, c.Tracing) if len(c.Env) > 0 { merged := make(map[string]string, len(p.Env)+len(c.Env)) @@ -677,6 +703,12 @@ func (c *Config) applyDefaults() { if c.Global.Tracing.BufferSize == 0 { c.Global.Tracing.BufferSize = ByteSize(500 * 1024 * 1024) } + if c.Global.ContainerBackend == "" { + c.Global.ContainerBackend = BackendAuto + } + if c.Global.Apple.GatewayIP == "" { + c.Global.Apple.GatewayIP = "192.168.64.1" + } for key, svc := range c.Services { if svc.Restart.Policy == "" { svc.Restart.Policy = "never" diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b12a402..5a5ea05 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -68,6 +68,14 @@ services: t.Errorf("watch_debounce = %v, want 300ms", cfg.Global.WatchDebounce.Duration) } + // Container backend defaults + if cfg.Global.ContainerBackend != BackendAuto { + t.Errorf("container_backend = %q, want %q", cfg.Global.ContainerBackend, BackendAuto) + } + if cfg.Global.Apple.GatewayIP != "192.168.64.1" { + t.Errorf("apple.gateway_ip = %q, want 192.168.64.1", cfg.Global.Apple.GatewayIP) + } + // Service defaults svc := cfg.Services["web"] if svc.Restart.Policy != "never" { @@ -78,6 +86,60 @@ services: } } +func TestParse_ContainerBackendOverride(t *testing.T) { + yaml := []byte(` +version: 1 +global: + container_backend: apple + apple: + gateway_ip: 10.0.0.1 +services: + db: + container: + image: postgres:16 +`) + cfg, err := Parse(yaml, "/tmp") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Global.ContainerBackend != BackendApple { + t.Errorf("container_backend = %q, want %q", cfg.Global.ContainerBackend, BackendApple) + } + if cfg.Global.Apple.GatewayIP != "10.0.0.1" { + t.Errorf("apple.gateway_ip = %q, want 10.0.0.1", cfg.Global.Apple.GatewayIP) + } +} + +func TestValidate_InvalidContainerBackend(t *testing.T) { + cfg := &Config{ + Version: 1, + Global: GlobalConfig{ContainerBackend: "podman"}, + Services: map[string]ServiceConfig{ + "db": {Container: &ContainerConfig{Image: "postgres:16"}}, + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("expected validation error for invalid container_backend") + } + assertContains(t, err.Error(), "invalid container_backend") +} + +func TestValidate_InvalidAppleGatewayIP(t *testing.T) { + cfg := &Config{ + Version: 1, + Global: GlobalConfig{Apple: AppleConfig{GatewayIP: "not-an-ip"}}, + Services: map[string]ServiceConfig{ + "db": {Container: &ContainerConfig{Image: "postgres:16"}}, + }, + } + err := cfg.Validate() + if err == nil { + t.Fatal("expected validation error for invalid apple.gateway_ip") + } + assertContains(t, err.Error(), "apple.gateway_ip") +} + func TestParse_CommandAsString(t *testing.T) { yaml := []byte(` version: 1 diff --git a/internal/config/validate.go b/internal/config/validate.go index c9e4e51..2704598 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "net" "os" "regexp" "strings" @@ -141,6 +142,17 @@ func (c *Config) Validate() error { errs = append(errs, fmt.Sprintf("container_prefix %q contains invalid character %q (only alphanumeric, hyphens, and underscores are allowed)", c.Global.ContainerPrefix, bad)) } + switch c.Global.ContainerBackend { + case "", BackendDocker, BackendApple, BackendAuto: + // valid (empty is defaulted to auto) + default: + errs = append(errs, fmt.Sprintf("invalid container_backend %q (must be %q, %q, or %q)", c.Global.ContainerBackend, BackendDocker, BackendApple, BackendAuto)) + } + + if c.Global.Apple.GatewayIP != "" && net.ParseIP(c.Global.Apple.GatewayIP) == nil { + errs = append(errs, fmt.Sprintf("apple.gateway_ip %q is not a valid IP address", c.Global.Apple.GatewayIP)) + } + if c.Global.EnvFile != "" { if _, err := os.Stat(c.Global.EnvFile); err != nil { errs = append(errs, fmt.Sprintf("global env_file %q: %v", c.Global.EnvFile, err)) diff --git a/internal/runner/apple.go b/internal/runner/apple.go new file mode 100644 index 0000000..2ff3354 --- /dev/null +++ b/internal/runner/apple.go @@ -0,0 +1,193 @@ +package runner + +import ( + "encoding/json" + "fmt" + "os/exec" + "runtime" + "strconv" + "strings" + "time" + + "github.com/ccakes/workbench/internal/config" +) + +const ( + defaultAppleGatewayIP = "192.168.64.1" + // appleInspectFailureLimit is how many consecutive `inspect` failures the + // exit poll tolerates (transient daemon hiccups) before assuming the + // container is gone and reporting an unknown exit. + appleInspectFailureLimit = 3 + // appleMinMacOSMajor is the minimum macOS major version the Apple backend + // supports (custom networks and DNS domains require macOS 26). + appleMinMacOSMajor = 26 +) + +// appleBackend runs containers via Apple's `container` CLI on Apple silicon. +type appleBackend struct { + binary string + gatewayIP string +} + +func newAppleBackend(g config.GlobalConfig) appleBackend { + gw := g.Apple.GatewayIP + if gw == "" { + gw = defaultAppleGatewayIP + } + return appleBackend{binary: "container", gatewayIP: gw} +} + +func (appleBackend) Name() string { return "apple" } +func (b appleBackend) Binary() string { return b.binary } + +// Available checks that the host can run Apple containers: Apple silicon, +// macOS 26+, and a running `container` system service. +func (b appleBackend) Available() error { + if runtime.GOOS != "darwin" || runtime.GOARCH != "arm64" { + return fmt.Errorf("apple container backend requires an Apple silicon Mac") + } + if v, err := macOSMajorVersion(); err == nil && v < appleMinMacOSMajor { + return fmt.Errorf("apple container backend requires macOS %d or later (found macOS %d)", appleMinMacOSMajor, v) + } + out, err := exec.Command(b.binary, "system", "status").CombinedOutput() + if err != nil { + return fmt.Errorf("apple container is not available: %s (try `container system start`)", strings.TrimSpace(string(out))) + } + return nil +} + +func (b appleBackend) RunArgs(spec RunSpec) []string { + // `container` has no `--add-host`; containers reach the host via the vmnet + // gateway IP (see OTELHost), so no host alias is injected here. + return buildRunArgs(spec, nil) +} + +func (appleBackend) LogsArgs(id string) []string { return []string{"logs", "-f", id} } + +func (appleBackend) StopArgs(id string, timeout time.Duration) []string { + return []string{"stop", "-t", strconv.Itoa(int(timeout.Seconds())), id} +} + +func (appleBackend) KillArgs(id string) []string { return []string{"kill", id} } + +func (appleBackend) RemoveArgs(target string, force bool) []string { + // `container` uses `delete` (aliased `rm`) and has no `-v`; anonymous + // volumes are not auto-removed — a documented difference from Docker. + args := []string{"delete"} + if force { + args = append(args, "-f") + } + return append(args, target) +} + +// WaitExit polls `container inspect` until the container leaves the running +// state, then reports its exit code. `container` has no `wait` subcommand. +// +// Exit-code fidelity is best-effort: `container inspect` does not reliably +// expose the Linux process exit code, so a cleanly stopped container with no +// code reported is treated as exit 0. Repeated inspect failures (the container +// is gone) yield -1. +func (b appleBackend) WaitExit(id string) int { + failures := 0 + for { + time.Sleep(containerPollInterval) + status, code, err := b.inspect(id) + if err != nil { + failures++ + if failures >= appleInspectFailureLimit { + return -1 + } + continue + } + failures = 0 + if status != "" && !strings.EqualFold(status, "running") && !strings.EqualFold(status, "starting") { + return code + } + } +} + +func (b appleBackend) OTELHost() string { return b.gatewayIP } + +// inspect runs `container inspect ` and extracts the status and best-effort +// exit code. `container inspect` emits JSON by default and, unlike Docker, has +// no `--format` flag (passing one errors), so the raw output is parsed here. +func (b appleBackend) inspect(id string) (status string, code int, err error) { + out, err := exec.Command(b.binary, "inspect", id).Output() + if err != nil { + return "", -1, err + } + return parseAppleInspect(out) +} + +// parseAppleInspect extracts the container status and a best-effort exit code +// from `container inspect` output. The payload is an array of one object whose +// "status" field is itself an object holding the run state as {"state": +// "running"|"stopped"|...}; older/alternate shapes expose a bare "status" +// string, which is also handled. The exit code, when present, is found by a +// case-insensitive search for an "exitCode"/"exit_code" field at any nesting +// level. Returns code 0 when no exit code is exposed. +func parseAppleInspect(data []byte) (status string, code int, err error) { + var arr []map[string]any + if err := json.Unmarshal(data, &arr); err != nil || len(arr) == 0 { + // Some versions may emit a bare object rather than a single-element array. + var obj map[string]any + if err2 := json.Unmarshal(data, &obj); err2 != nil { + return "", -1, fmt.Errorf("parsing container inspect output: %w", err) + } + arr = []map[string]any{obj} + } + obj := arr[0] + switch s := obj["status"].(type) { + case string: + status = s + case map[string]any: + if st, ok := s["state"].(string); ok { + status = st + } + } + if c, ok := searchExitCode(obj); ok { + code = c + } + return status, code, nil +} + +// searchExitCode walks a decoded JSON value looking for an exit-code field, +// tolerating naming/nesting differences across `container` versions. +func searchExitCode(v any) (int, bool) { + switch t := v.(type) { + case map[string]any: + for k, val := range t { + key := strings.ToLower(strings.ReplaceAll(k, "_", "")) + if key == "exitcode" || key == "exitstatus" { + if f, ok := val.(float64); ok { + return int(f), true + } + } + } + for _, val := range t { + if c, ok := searchExitCode(val); ok { + return c, true + } + } + case []any: + for _, val := range t { + if c, ok := searchExitCode(val); ok { + return c, true + } + } + } + return 0, false +} + +// macOSMajorVersion returns the major component of `sw_vers -productVersion`. +func macOSMajorVersion() (int, error) { + out, err := exec.Command("sw_vers", "-productVersion").Output() + if err != nil { + return 0, err + } + v := strings.TrimSpace(string(out)) + if i := strings.IndexByte(v, '.'); i >= 0 { + v = v[:i] + } + return strconv.Atoi(v) +} diff --git a/internal/runner/backend.go b/internal/runner/backend.go new file mode 100644 index 0000000..21f46ae --- /dev/null +++ b/internal/runner/backend.go @@ -0,0 +1,116 @@ +package runner + +import ( + "os/exec" + "runtime" + "time" + + "github.com/ccakes/workbench/internal/config" +) + +// ContainerBackend abstracts the container runtime that workbench shells out +// to. The Docker backend preserves workbench's original behavior exactly; the +// Apple backend adapts to the `container` CLI's differences (no `wait` +// subcommand, no host-gateway alias, macOS-26-only). +// +// Both `container` and `docker` share almost the same run/logs/stop/kill +// argument syntax, so the backend mostly builds argument slices that the +// ContainerRunner executes via Binary(). The exceptions — availability checks +// and waiting for exit — differ enough that the backend owns them directly. +type ContainerBackend interface { + // Name is the short identifier shown in the TUI and `bench status`. + Name() string + // Binary is the executable workbench invokes (e.g. "docker", "container"). + Binary() string + // Available reports whether the backend can run containers now, with a + // user-facing error explaining what to fix when it cannot. + Available() error + // RunArgs builds the args for a detached `run`. + RunArgs(spec RunSpec) []string + // LogsArgs builds the args to follow a container's logs. + LogsArgs(id string) []string + // StopArgs builds the args to gracefully stop a container. + StopArgs(id string, timeout time.Duration) []string + // KillArgs builds the args to force-kill a container. + KillArgs(id string) []string + // RemoveArgs builds the args to remove a container by name or id. + RemoveArgs(target string, force bool) []string + // WaitExit blocks until the container terminates and returns its exit code. + // The strategy differs per backend (Docker `wait` vs polling `inspect`). + WaitExit(id string) int + // OTELHost is the hostname a container uses to reach a service on the macOS + // host (e.g. the OTLP trace collector). + OTELHost() string +} + +// RunSpec captures everything needed to build a container `run` invocation. +// It is backend-agnostic; each backend renders it into its own argument list. +type RunSpec struct { + Name string + Labels []string + Env []string + Ports []string + Volumes []string + Network string + Image string + Command []string +} + +// containerPollInterval is how often the Apple backend polls `inspect` to +// detect that a container has stopped. +const containerPollInterval = 250 * time.Millisecond + +// ResolveBackend selects the container backend from global config. +// config.BackendDocker and config.BackendApple are explicit; config.BackendAuto +// (the default) prefers Apple's `container` on Apple silicon when the binary is +// installed, otherwise Docker. Selection is pure and side-effect-free — the +// environment/daemon health check happens later in Available(). +func ResolveBackend(g config.GlobalConfig) ContainerBackend { + switch g.ContainerBackend { + case config.BackendDocker: + return dockerBackend{} + case config.BackendApple: + return newAppleBackend(g) + default: // BackendAuto or unset + if isAppleSilicon() && appleContainerInstalled() { + return newAppleBackend(g) + } + return dockerBackend{} + } +} + +// buildRunArgs assembles a detached-run argument list shared by both backends. +// hostAlias is inserted after the labels: Docker uses it for the host-gateway +// alias; Apple passes nil. Argument order matches workbench's original Docker +// runner so the Docker path is unchanged. +func buildRunArgs(spec RunSpec, hostAlias []string) []string { + args := []string{"run", "-d", "--name", spec.Name} + for _, l := range spec.Labels { + args = append(args, "--label", l) + } + args = append(args, hostAlias...) + for _, e := range spec.Env { + args = append(args, "-e", e) + } + for _, p := range spec.Ports { + args = append(args, "-p", p) + } + for _, v := range spec.Volumes { + args = append(args, "-v", v) + } + if spec.Network != "" { + args = append(args, "--network", spec.Network) + } + args = append(args, spec.Image) + args = append(args, spec.Command...) + return args +} + +func isAppleSilicon() bool { + return runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" +} + +func appleContainerInstalled() bool { + _, err := exec.LookPath("container") + return err == nil +} diff --git a/internal/runner/backend_test.go b/internal/runner/backend_test.go new file mode 100644 index 0000000..6763ea2 --- /dev/null +++ b/internal/runner/backend_test.go @@ -0,0 +1,208 @@ +package runner + +import ( + "reflect" + "testing" + "time" + + "github.com/ccakes/workbench/internal/config" +) + +func sampleSpec() RunSpec { + return RunSpec{ + Name: "bench-db", + Labels: []string{"managed-by=bench"}, + Env: []string{"FOO=bar"}, + Ports: []string{"5432:5432"}, + Volumes: []string{"/data:/var/lib/pg"}, + Network: "backend", + Image: "postgres:16", + Command: []string{"postgres", "-c", "log_statement=all"}, + } +} + +func TestDockerBackend_RunArgs(t *testing.T) { + got := dockerBackend{}.RunArgs(sampleSpec()) + want := []string{ + "run", "-d", "--name", "bench-db", + "--label", "managed-by=bench", + "--add-host", "host.docker.internal:host-gateway", + "-e", "FOO=bar", + "-p", "5432:5432", + "-v", "/data:/var/lib/pg", + "--network", "backend", + "postgres:16", + "postgres", "-c", "log_statement=all", + } + if !reflect.DeepEqual(got, want) { + t.Errorf("RunArgs mismatch\n got: %v\nwant: %v", got, want) + } +} + +func TestAppleBackend_RunArgs_NoHostAlias(t *testing.T) { + b := newAppleBackend(config.GlobalConfig{}) + got := b.RunArgs(sampleSpec()) + // Apple omits --add-host; everything else matches Docker's ordering. + want := []string{ + "run", "-d", "--name", "bench-db", + "--label", "managed-by=bench", + "-e", "FOO=bar", + "-p", "5432:5432", + "-v", "/data:/var/lib/pg", + "--network", "backend", + "postgres:16", + "postgres", "-c", "log_statement=all", + } + if !reflect.DeepEqual(got, want) { + t.Errorf("RunArgs mismatch\n got: %v\nwant: %v", got, want) + } +} + +func TestBackends_LogsStopKillRemoveArgs(t *testing.T) { + tests := []struct { + name string + backend ContainerBackend + logs []string + stop []string + kill []string + rmForce []string + rmSoft []string + }{ + { + name: "docker", + backend: dockerBackend{}, + logs: []string{"logs", "--follow", "cid"}, + stop: []string{"stop", "-t", "10", "cid"}, + kill: []string{"kill", "cid"}, + rmForce: []string{"rm", "-f", "-v", "bench-db"}, + rmSoft: []string{"rm", "-v", "cid"}, + }, + { + name: "apple", + backend: newAppleBackend(config.GlobalConfig{}), + logs: []string{"logs", "-f", "cid"}, + stop: []string{"stop", "-t", "10", "cid"}, + kill: []string{"kill", "cid"}, + rmForce: []string{"delete", "-f", "bench-db"}, + rmSoft: []string{"delete", "cid"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.backend.LogsArgs("cid"); !reflect.DeepEqual(got, tt.logs) { + t.Errorf("LogsArgs = %v, want %v", got, tt.logs) + } + if got := tt.backend.StopArgs("cid", 10*time.Second); !reflect.DeepEqual(got, tt.stop) { + t.Errorf("StopArgs = %v, want %v", got, tt.stop) + } + if got := tt.backend.KillArgs("cid"); !reflect.DeepEqual(got, tt.kill) { + t.Errorf("KillArgs = %v, want %v", got, tt.kill) + } + if got := tt.backend.RemoveArgs("bench-db", true); !reflect.DeepEqual(got, tt.rmForce) { + t.Errorf("RemoveArgs(force) = %v, want %v", got, tt.rmForce) + } + if got := tt.backend.RemoveArgs("cid", false); !reflect.DeepEqual(got, tt.rmSoft) { + t.Errorf("RemoveArgs(soft) = %v, want %v", got, tt.rmSoft) + } + }) + } +} + +func TestBackends_OTELHost(t *testing.T) { + if got := (dockerBackend{}).OTELHost(); got != "host.docker.internal" { + t.Errorf("docker OTELHost = %q", got) + } + if got := newAppleBackend(config.GlobalConfig{}).OTELHost(); got != defaultAppleGatewayIP { + t.Errorf("apple OTELHost = %q, want default %q", got, defaultAppleGatewayIP) + } + custom := newAppleBackend(config.GlobalConfig{Apple: config.AppleConfig{GatewayIP: "10.9.8.7"}}) + if got := custom.OTELHost(); got != "10.9.8.7" { + t.Errorf("apple OTELHost = %q, want 10.9.8.7", got) + } +} + +func TestResolveBackend_Explicit(t *testing.T) { + if got := ResolveBackend(config.GlobalConfig{ContainerBackend: config.BackendDocker}).Name(); got != "docker" { + t.Errorf("docker => %q", got) + } + if got := ResolveBackend(config.GlobalConfig{ContainerBackend: config.BackendApple}).Name(); got != "apple" { + t.Errorf("apple => %q", got) + } +} + +func TestParseAppleInspect(t *testing.T) { + tests := []struct { + name string + data string + wantStatus string + wantCode int + wantErr bool + }{ + { + name: "running array", + data: `[{"status":"running","configuration":{"id":"db"}}]`, + wantStatus: "running", + wantCode: 0, + }, + { + name: "stopped with exitCode", + data: `[{"status":"stopped","exitCode":137}]`, + wantStatus: "stopped", + wantCode: 137, + }, + { + name: "stopped nested exit_code", + data: `[{"status":"stopped","process":{"exit_code":2}}]`, + wantStatus: "stopped", + wantCode: 2, + }, + { + name: "stopped no code defaults zero", + data: `[{"status":"stopped"}]`, + wantStatus: "stopped", + wantCode: 0, + }, + { + name: "bare object", + data: `{"status":"stopped","exitCode":1}`, + wantStatus: "stopped", + wantCode: 1, + }, + { + // Real `container inspect` (v1.1.0) shape: status is an object + // whose "state" holds the run state, not a bare string. + name: "nested status object running", + data: `[{"id":"n-redis","configuration":{"id":"n-redis"},"status":{"state":"running","startedDate":"2026-07-23T00:47:05Z","networks":[{"ipv4Address":"192.168.64.36/24"}]}}]`, + wantStatus: "running", + wantCode: 0, + }, + { + name: "nested status object stopped", + data: `[{"id":"n-redis","status":{"state":"stopped","startedDate":"2026-07-23T00:47:05Z"}}]`, + wantStatus: "stopped", + wantCode: 0, + }, + { + name: "malformed", + data: `not json`, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + status, code, err := parseAppleInspect([]byte(tt.data)) + if (err != nil) != tt.wantErr { + t.Fatalf("err = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return + } + if status != tt.wantStatus { + t.Errorf("status = %q, want %q", status, tt.wantStatus) + } + if code != tt.wantCode { + t.Errorf("code = %d, want %d", code, tt.wantCode) + } + }) + } +} diff --git a/internal/runner/container.go b/internal/runner/container.go index 7c6bf0d..40e7465 100644 --- a/internal/runner/container.go +++ b/internal/runner/container.go @@ -12,83 +12,70 @@ import ( "github.com/ccakes/workbench/internal/logbuf" ) -// ContainerRunner manages a Docker container lifecycle. +// logDrainGrace is how long the exit goroutine waits for the log follower to +// drain naturally after the container stops before force-killing it. Docker's +// `logs --follow` exits on stop; some `container logs -f` cases may not. +const logDrainGrace = 2 * time.Second + +// ContainerRunner manages a container lifecycle via a ContainerBackend +// (Docker or Apple's `container`). type ContainerRunner struct { cfg config.ServiceConfig + backend ContainerBackend containerID string name string logCmd *exec.Cmd } -func NewContainerRunner(cfg config.ServiceConfig, serviceKey string, prefix string) *ContainerRunner { +func NewContainerRunner(cfg config.ServiceConfig, serviceKey, prefix string, backend ContainerBackend) *ContainerRunner { return &ContainerRunner{ - cfg: cfg, - name: prefix + "-" + serviceKey, + cfg: cfg, + backend: backend, + name: prefix + "-" + serviceKey, } } func (r *ContainerRunner) Start(env []string, logs *logbuf.Buffer, bus *events.Bus, key string) (<-chan int, error) { cc := r.cfg.Container - - // Clean up any stale container with same name. - // -v also removes the container's anonymous volumes (e.g. images with a - // VOLUME directive like Postgres/Cassandra) to avoid leaking disk space. - cleanup := exec.Command("docker", "rm", "-f", "-v", r.name) - _ = cleanup.Run() // ignore errors — container may not exist - - // Build docker run args. host-gateway maps host.docker.internal to the - // host so containers can reach host-side services (e.g. the OTLP trace - // collector). Some runtimes provide this alias automatically; adding it - // explicitly makes it portable to plain Docker on Linux. - args := []string{"run", "-d", "--name", r.name, "--label", "managed-by=bench", - "--add-host", "host.docker.internal:host-gateway"} - - // Environment variables from env slice (already merged by supervisor) - for _, e := range env { - // Only pass non-system env vars — filter to config-specified keys - args = append(args, "-e", e) - } - - for _, p := range cc.Ports { - args = append(args, "-p", p) - } - for _, v := range cc.Volumes { - args = append(args, "-v", v) - } - if cc.Network != "" { - args = append(args, "--network", cc.Network) - } - - args = append(args, cc.Image) - - if len(cc.Command.Parts) > 0 { - args = append(args, cc.Command.Parts...) + bin := r.backend.Binary() + + // Clean up any stale container with the same name (force removal). + _ = exec.Command(bin, r.backend.RemoveArgs(r.name, true)...).Run() // ignore errors — container may not exist + + spec := RunSpec{ + Name: r.name, + Labels: []string{"managed-by=bench"}, + Env: env, // already merged by supervisor + Ports: cc.Ports, + Volumes: cc.Volumes, + Network: cc.Network, + Image: cc.Image, + Command: cc.Command.Parts, } // Run container - cmd := exec.Command("docker", args...) - out, err := cmd.Output() + out, err := exec.Command(bin, r.backend.RunArgs(spec)...).Output() if err != nil { if exitErr, ok := err.(*exec.ExitError); ok { - return nil, fmt.Errorf("docker run failed: %s", strings.TrimSpace(string(exitErr.Stderr))) + return nil, fmt.Errorf("%s run failed: %s", bin, strings.TrimSpace(string(exitErr.Stderr))) } - return nil, fmt.Errorf("docker run: %w", err) + return nil, fmt.Errorf("%s run: %w", bin, err) } r.containerID = strings.TrimSpace(string(out)) // containerID stores the full ID; Info() returns the short form // Stream logs - r.logCmd = exec.Command("docker", "logs", "--follow", r.containerID) + r.logCmd = exec.Command(bin, r.backend.LogsArgs(r.containerID)...) stdout, err := r.logCmd.StdoutPipe() if err != nil { - return nil, fmt.Errorf("docker logs stdout pipe: %w", err) + return nil, fmt.Errorf("%s logs stdout pipe: %w", bin, err) } stderr, err := r.logCmd.StderrPipe() if err != nil { - return nil, fmt.Errorf("docker logs stderr pipe: %w", err) + return nil, fmt.Errorf("%s logs stderr pipe: %w", bin, err) } if err := r.logCmd.Start(); err != nil { - return nil, fmt.Errorf("docker logs: %w", err) + return nil, fmt.Errorf("%s logs: %w", bin, err) } var pipeWg sync.WaitGroup @@ -96,28 +83,29 @@ func (r *ContainerRunner) Start(env []string, logs *logbuf.Buffer, bus *events.B go readPipe(logs, bus, key, stdout, "stdout", events.StreamStdout, &pipeWg) go readPipe(logs, bus, key, stderr, "stderr", events.StreamStderr, &pipeWg) - // Wait for container exit - exitCh := make(chan int, 1) + logsDone := make(chan struct{}) go func() { - // docker wait returns the exit code - waitCmd := exec.Command("docker", "wait", r.containerID) - out, err := waitCmd.Output() - // Once container exits, log streaming will end naturally pipeWg.Wait() + close(logsDone) + }() - code := 0 - if err != nil { - code = -1 - } else { - trimmed := strings.TrimSpace(string(out)) - _, _ = fmt.Sscanf(trimmed, "%d", &code) + // Wait for container exit + exitCh := make(chan int, 1) + go func() { + code := r.backend.WaitExit(r.containerID) + + // The container has exited. Give the log follower a moment to drain + // naturally (Docker's `logs --follow` exits on stop); if it doesn't, + // force it down so the pipes close and readPipe returns. + select { + case <-logsDone: + case <-time.After(logDrainGrace): } - - // Clean up log follower if r.logCmd.Process != nil { _ = r.logCmd.Process.Kill() _ = r.logCmd.Wait() } + <-logsDone exitCh <- code }() @@ -129,24 +117,22 @@ func (r *ContainerRunner) Stop(exitCh <-chan int, timeout time.Duration) { if r.containerID == "" { return } + bin := r.backend.Binary() - timeoutSecs := fmt.Sprintf("%d", int(timeout.Seconds())) - stopCmd := exec.Command("docker", "stop", "-t", timeoutSecs, r.containerID) - _ = stopCmd.Run() + _ = exec.Command(bin, r.backend.StopArgs(r.containerID, timeout)...).Run() - // Wait for exit with a grace period beyond the docker stop timeout + // Wait for exit with a grace period beyond the stop timeout select { case <-exitCh: case <-time.After(timeout + 5*time.Second): - killCmd := exec.Command("docker", "kill", r.containerID) - _ = killCmd.Run() + _ = exec.Command(bin, r.backend.KillArgs(r.containerID)...).Run() <-exitCh } - // Remove container along with its anonymous volumes (-v) so repeated - // starts/restarts don't leak dangling volumes for VOLUME-declaring images. - rmCmd := exec.Command("docker", "rm", "-v", r.containerID) - _ = rmCmd.Run() + // Remove the container so repeated starts/restarts don't leave it behind. + // On Docker this also drops anonymous volumes (-v); the Apple backend has + // no equivalent flag. + _ = exec.Command(bin, r.backend.RemoveArgs(r.containerID, false)...).Run() } func (r *ContainerRunner) Info() RunnerInfo { diff --git a/internal/runner/docker.go b/internal/runner/docker.go index 5d4347f..00d32e6 100644 --- a/internal/runner/docker.go +++ b/internal/runner/docker.go @@ -3,15 +3,62 @@ package runner import ( "fmt" "os/exec" + "strconv" "strings" + "time" ) -// CheckDocker verifies that Docker is available and running. -func CheckDocker() error { - cmd := exec.Command("docker", "info") - out, err := cmd.CombinedOutput() +// dockerBackend runs containers via the `docker` CLI. It reproduces +// workbench's original Docker behavior exactly. +type dockerBackend struct{} + +func (dockerBackend) Name() string { return "docker" } +func (dockerBackend) Binary() string { return "docker" } + +// Available verifies that Docker is available and running. +func (dockerBackend) Available() error { + out, err := exec.Command("docker", "info").CombinedOutput() if err != nil { return fmt.Errorf("docker is not available: %s", strings.TrimSpace(string(out))) } return nil } + +func (dockerBackend) RunArgs(spec RunSpec) []string { + // host-gateway maps host.docker.internal to the host so containers can + // reach host-side services (e.g. the OTLP trace collector). Some runtimes + // provide this alias automatically; adding it explicitly makes it portable + // to plain Docker on Linux. + return buildRunArgs(spec, []string{"--add-host", "host.docker.internal:host-gateway"}) +} + +func (dockerBackend) LogsArgs(id string) []string { return []string{"logs", "--follow", id} } + +func (dockerBackend) StopArgs(id string, timeout time.Duration) []string { + return []string{"stop", "-t", strconv.Itoa(int(timeout.Seconds())), id} +} + +func (dockerBackend) KillArgs(id string) []string { return []string{"kill", id} } + +func (dockerBackend) RemoveArgs(target string, force bool) []string { + // -v also removes the container's anonymous volumes (e.g. images with a + // VOLUME directive like Postgres/Cassandra) to avoid leaking disk space. + args := []string{"rm"} + if force { + args = append(args, "-f") + } + return append(args, "-v", target) +} + +func (dockerBackend) WaitExit(id string) int { + // `docker wait` blocks until the container exits and prints the exit code. + out, err := exec.Command("docker", "wait", id).Output() + if err != nil { + return -1 + } + code := 0 + _, _ = fmt.Sscanf(strings.TrimSpace(string(out)), "%d", &code) + return code +} + +func (dockerBackend) OTELHost() string { return "host.docker.internal" } diff --git a/internal/service/state.go b/internal/service/state.go index c6239c8..7267e07 100644 --- a/internal/service/state.go +++ b/internal/service/state.go @@ -75,6 +75,7 @@ type Info struct { LastError string WatchEnabled bool ServiceType string // "process" or "container" + Backend string // container backend name ("docker"/"apple"); empty for processes ContainerID string Image string Ports []string @@ -131,6 +132,7 @@ type Snapshot struct { LastError string WatchEnabled bool ServiceType string + Backend string ContainerID string Image string Ports []string @@ -152,6 +154,7 @@ func (i *Info) Snapshot() Snapshot { LastError: i.LastError, WatchEnabled: i.WatchEnabled, ServiceType: i.ServiceType, + Backend: i.Backend, ContainerID: i.ContainerID, Image: i.Image, Ports: i.Ports, diff --git a/internal/supervisor/buildenv_test.go b/internal/supervisor/buildenv_test.go index f5f1751..f983894 100644 --- a/internal/supervisor/buildenv_test.go +++ b/internal/supervisor/buildenv_test.go @@ -61,7 +61,11 @@ func tracingCfg(svc config.ServiceConfig) *config.Config { return &config.Config{ Version: 1, Global: config.GlobalConfig{ - Tracing: config.TracingConfig{Enabled: true, Port: 4318}, + // Pin the Docker backend so the injected OTEL host is deterministic + // regardless of the test machine (auto-detect could otherwise pick + // the Apple backend on Apple silicon). + ContainerBackend: config.BackendDocker, + Tracing: config.TracingConfig{Enabled: true, Port: 4318}, }, Services: map[string]config.ServiceConfig{"svc": svc}, } @@ -81,6 +85,19 @@ func TestOTEL_InjectsDefaultsWhenUnset(t *testing.T) { } } +// TestOTEL_InjectsAppleGatewayIP verifies that on the Apple backend the +// collector endpoint uses the configured vmnet gateway IP rather than +// host.docker.internal (which Apple containers can't resolve). +func TestOTEL_InjectsAppleGatewayIP(t *testing.T) { + cfg := tracingCfg(containerService()) + cfg.Global.ContainerBackend = config.BackendApple + cfg.Global.Apple.GatewayIP = "10.1.2.3" + env := buildEnvForService(t, cfg, "svc") + if got, want := env["OTEL_EXPORTER_OTLP_ENDPOINT"], "http://10.1.2.3:4318"; got != want { + t.Errorf("endpoint = %q, want %q", got, want) + } +} + // TestOTEL_NotInjectedWhenTracingDisabled verifies no OTEL vars appear when // tracing is off. func TestOTEL_NotInjectedWhenTracingDisabled(t *testing.T) { diff --git a/internal/supervisor/reload.go b/internal/supervisor/reload.go index 639e1d8..def5595 100644 --- a/internal/supervisor/reload.go +++ b/internal/supervisor/reload.go @@ -46,7 +46,7 @@ func (s *Supervisor) Reload(newCfg *config.Config) ReloadReport { ms.mu.Lock() ms.cfg = newSvc ms.mu.Unlock() - applyServiceMetadata(ms.info, key, newSvc, true) + applyServiceMetadata(ms.info, key, newSvc, true, s.backend.Name()) if err := s.RestartService(key, "config reload"); err != nil { report.Errors[key] = err.Error() continue diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index 33e9d8c..9633589 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -25,6 +25,7 @@ type Supervisor struct { bus *events.Bus ctx context.Context cancel context.CancelFunc + backend runner.ContainerBackend } type managedService struct { @@ -59,11 +60,12 @@ func New(cfg *config.Config, bus *events.Bus) *Supervisor { bus: bus, ctx: ctx, cancel: cancel, + backend: runner.ResolveBackend(cfg.Global), } for key, svcCfg := range cfg.Services { info := service.NewInfo(key, displayName(key, svcCfg)) - applyServiceMetadata(info, key, svcCfg, false) + applyServiceMetadata(info, key, svcCfg, false, s.backend.Name()) s.services[key] = &managedService{ info: info, @@ -85,7 +87,7 @@ func displayName(key string, svcCfg config.ServiceConfig) string { return key } -func applyServiceMetadata(info *service.Info, key string, svcCfg config.ServiceConfig, preserveStatus bool) { +func applyServiceMetadata(info *service.Info, key string, svcCfg config.ServiceConfig, preserveStatus bool, backendName string) { info.Lock() defer info.Unlock() @@ -93,10 +95,12 @@ func applyServiceMetadata(info *service.Info, key string, svcCfg config.ServiceC info.WatchEnabled = svcCfg.Watch.IsEnabled() if svcCfg.IsContainer() { info.ServiceType = "container" + info.Backend = backendName info.Image = svcCfg.Container.Image info.Ports = append([]string(nil), svcCfg.Container.Ports...) } else { info.ServiceType = "process" + info.Backend = "" info.Image = "" info.Ports = nil } @@ -325,7 +329,7 @@ func (s *Supervisor) runLoop(ms *managedService) { // Create a fresh runner for each attempt if ms.cfg.IsContainer() { - ms.r = runner.NewContainerRunner(ms.cfg, ms.key, s.cfg.Global.ContainerPrefix) + ms.r = runner.NewContainerRunner(ms.cfg, ms.key, s.cfg.Global.ContainerPrefix, s.backend) } else { ms.r = runner.NewProcessRunner(ms.cfg) } @@ -681,11 +685,11 @@ func (s *Supervisor) buildEnv(ms *managedService) ([]string, error) { } // The collector listens on the host. Host-process services reach it // via localhost, but container services have their own loopback, so - // they must reach the host collector via host.docker.internal (added - // to container runs as a host-gateway alias by the container runner). + // they reach the host collector via the backend's host address + // (host.docker.internal for Docker, the vmnet gateway IP for Apple). otelHost := "localhost" if ms.cfg.IsContainer() { - otelHost = "host.docker.internal" + otelHost = s.backend.OTELHost() } if !alreadySet("OTEL_EXPORTER_OTLP_ENDPOINT") { env = append(env, fmt.Sprintf("OTEL_EXPORTER_OTLP_ENDPOINT=http://%s:%d", otelHost, port)) diff --git a/internal/tui/app.go b/internal/tui/app.go index 91b76c2..6a4c025 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -597,6 +597,9 @@ func (m Model) viewDetail(width, height int) string { } if svcCfg != nil && snap.ServiceType == "container" { + if snap.Backend != "" { + row("Backend", snap.Backend) + } if snap.ContainerID != "" { row("Container", snap.ContainerID) } diff --git a/internal/tui/session_remote.go b/internal/tui/session_remote.go index 7c8821a..53a4997 100644 --- a/internal/tui/session_remote.go +++ b/internal/tui/session_remote.go @@ -385,6 +385,7 @@ func snapFromStatus(s api.ServiceStatus) service.Snapshot { LastError: s.LastError, WatchEnabled: s.WatchEnabled, ServiceType: s.Type, + Backend: s.Backend, ContainerID: s.ContainerID, Image: s.Image, Ports: s.Ports, From 4725eca16ac4a1e9e9ca9d56a2692149992b8ddb Mon Sep 17 00:00:00 2001 From: Cameron Daniel Date: Thu, 23 Jul 2026 12:47:37 +1000 Subject: [PATCH 2/5] Fail terminally on images with no usable architecture variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously a container image with no runnable variant for the host architecture (e.g. an amd64-only image on Apple silicon that Rosetta can't run) failed to start and then looped through the restart policy — retrying something that can never succeed and burying the real cause. Detect the runtime's platform-mismatch output ("does not support required platforms" / "exec format error" on Apple `container`; "no matching manifest" / platform-mismatch on Docker) in the run error, return a sentinel runner.ErrUnsupportedPlatform, and have the supervisor treat it as terminal: the service goes straight to Failed with a clear message and is not restarted, regardless of restart policy. Verified end-to-end on the Apple backend with an amd64-only image and restart.policy: always — the service reaches Failed with zero restarts and the error is logged once. --- docs/apple-container.md | 13 ++++++++ internal/runner/backend.go | 35 ++++++++++++++++++++++ internal/runner/backend_test.go | 49 +++++++++++++++++++++++++++++++ internal/runner/container.go | 9 +++++- internal/supervisor/supervisor.go | 8 +++++ 5 files changed, 113 insertions(+), 1 deletion(-) diff --git a/docs/apple-container.md b/docs/apple-container.md index 7e319a4..e5083db 100644 --- a/docs/apple-container.md +++ b/docs/apple-container.md @@ -65,6 +65,19 @@ matching gateway address. - **Host networking / `--add-host`** — not used by workbench on this backend; host access goes through the gateway IP instead. +## Images without an arm64 variant + +Apple `container` runs images for the host architecture (arm64), using Rosetta +to run amd64 images where possible. An image with **no usable variant** for the +host — e.g. an amd64-only image whose binaries Rosetta can't run — fails at +start with `does not support required platforms` or `exec format error`. + +Workbench treats this as a **terminal failure**: the service goes straight to +`failed` with a clear message (`image does not support this platform: …`) and is +*not* retried, even under `restart.policy: always`. Retrying can never succeed, +so looping would only bury the real reason in noise. Rebuild or source a +multi-arch (arm64) image to fix it. + ## Out of scope - Building images (`container build`) — workbench only runs pre-built images. diff --git a/internal/runner/backend.go b/internal/runner/backend.go index 21f46ae..1773c6e 100644 --- a/internal/runner/backend.go +++ b/internal/runner/backend.go @@ -1,13 +1,48 @@ package runner import ( + "errors" "os/exec" "runtime" + "strings" "time" "github.com/ccakes/workbench/internal/config" ) +// ErrUnsupportedPlatform indicates a container image has no usable variant for +// the host architecture (e.g. an amd64-only image on Apple silicon). Retrying +// can never succeed, so callers should treat it as a terminal failure rather +// than restarting. +var ErrUnsupportedPlatform = errors.New("image does not support this platform") + +// platformMismatchMarkers are substrings container runtimes emit when an image +// has no usable variant for the host architecture. Matched case-insensitively; +// best-effort, extend as runtime wording changes. Observed wording: +// - Apple `container` 1.1: "... does not support required platforms"; and +// "Exec format error" when a variant exists but its binaries can't run on +// the host (Rosetta can't help). +// - Docker: "no matching manifest for "; "does not match the +// specified platform". +var platformMismatchMarkers = []string{ + "does not support required platforms", + "no matching manifest", + "does not match the specified platform", + "exec format error", +} + +// isUnsupportedPlatformError reports whether container-runtime output indicates +// the image can't run on the host architecture. +func isUnsupportedPlatformError(output string) bool { + low := strings.ToLower(output) + for _, m := range platformMismatchMarkers { + if strings.Contains(low, m) { + return true + } + } + return false +} + // ContainerBackend abstracts the container runtime that workbench shells out // to. The Docker backend preserves workbench's original behavior exactly; the // Apple backend adapts to the `container` CLI's differences (no `wait` diff --git a/internal/runner/backend_test.go b/internal/runner/backend_test.go index 6763ea2..3c12840 100644 --- a/internal/runner/backend_test.go +++ b/internal/runner/backend_test.go @@ -130,6 +130,55 @@ func TestResolveBackend_Explicit(t *testing.T) { } } +func TestIsUnsupportedPlatformError(t *testing.T) { + tests := []struct { + name string + out string + want bool + }{ + { + // Real Apple `container` 1.1 output for an amd64-only image. + name: "apple no compatible variant", + out: "Error: image sha256:1178cdd375f7 does not support required platforms", + want: true, + }, + { + // Real Apple `container` 1.1 output when a variant exists but its + // binaries can't exec on the host. + name: "apple exec format error", + out: `failed to exec [echo hi] Error Domain=NSPOSIXErrorDomain Code=8 "Exec format error"`, + want: true, + }, + { + name: "docker no matching manifest", + out: "no matching manifest for linux/arm64/v8 in the manifest list entries", + want: true, + }, + { + name: "docker platform mismatch", + out: "image with reference X was found but does not match the specified platform", + want: true, + }, + { + name: "unrelated failure", + out: "Error: connection refused", + want: false, + }, + { + name: "empty", + out: "", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isUnsupportedPlatformError(tt.out); got != tt.want { + t.Errorf("isUnsupportedPlatformError(%q) = %v, want %v", tt.out, got, tt.want) + } + }) + } +} + func TestParseAppleInspect(t *testing.T) { tests := []struct { name string diff --git a/internal/runner/container.go b/internal/runner/container.go index 40e7465..e16b334 100644 --- a/internal/runner/container.go +++ b/internal/runner/container.go @@ -3,6 +3,7 @@ package runner import ( "fmt" "os/exec" + "runtime" "strings" "sync" "time" @@ -57,7 +58,13 @@ func (r *ContainerRunner) Start(env []string, logs *logbuf.Buffer, bus *events.B out, err := exec.Command(bin, r.backend.RunArgs(spec)...).Output() if err != nil { if exitErr, ok := err.(*exec.ExitError); ok { - return nil, fmt.Errorf("%s run failed: %s", bin, strings.TrimSpace(string(exitErr.Stderr))) + stderr := strings.TrimSpace(string(exitErr.Stderr)) + // An image with no usable variant for this architecture will never + // start — surface a terminal error so the supervisor stops retrying. + if isUnsupportedPlatformError(stderr) { + return nil, fmt.Errorf("%w: image %q on %s: %s", ErrUnsupportedPlatform, cc.Image, runtime.GOARCH, stderr) + } + return nil, fmt.Errorf("%s run failed: %s", bin, stderr) } return nil, fmt.Errorf("%s run: %w", bin, err) } diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index 9633589..7269213 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -2,6 +2,7 @@ package supervisor import ( "context" + "errors" "fmt" "os" "strings" @@ -347,6 +348,13 @@ func (s *Supervisor) runLoop(ms *managedService) { ms.info.Unlock() s.setStatus(ms, service.StatusFailed, err.Error()) + // A platform mismatch (e.g. an image with no arm64 variant) can + // never succeed on retry — fail terminally instead of looping + // through the restart policy. + if errors.Is(err, runner.ErrUnsupportedPlatform) { + return + } + // On start failure, check restart policy if !s.shouldRestart(ms, 1) { return From 03925d5b666662c45c8b046be90459b3328b8fbe Mon Sep 17 00:00:00 2001 From: Cameron Daniel Date: Wed, 29 Jul 2026 10:29:48 +1000 Subject: [PATCH 3/5] Add container_exec readiness probe kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A readiness probe that needs to run a command inside its own container had to be written as `kind: exec` with a hand-rolled `docker exec ...`. That hardcodes three things the config should not know: the runtime's CLI, the container prefix, and the service key. The CLI is the one that bites. `container_backend: auto` resolves to Apple `container` on Apple silicon, so an existing config's `docker exec` probe starts looking in the wrong runtime's namespace — every attempt reports "No such container" until max_attempts is exhausted, while the service it is probing is healthy the whole time. This silently contradicts the promise in docs/apple-container.md that a container service runs unchanged on either backend. Add a container_exec kind that runs its command inside the owning service's container, with workbench supplying the container and the CLI: readiness: kind: container_exec command: pg_isready -U bench -d bench Layered so each piece owns one decision: - ContainerBackend.ExecArgs(id, cmd) builds the invocation. Both backends spell it `exec ` today, but routing through the interface is what keeps the probe portable if that stops being true. - ContainerRunner.ExecCommand targets the container by name rather than id, so it is valid before Start() assigns an id and stays valid across restarts. - runner.ContainerExecer is deliberately not implemented by ProcessRunner, so the failed type assertion is how the probe reports "only valid for a container service" instead of hanging. It is resolved before the probe goroutine starts, so the probe never races the runLoop for ms.r. - Validation rejects container_exec on a service with no container block, since that can only ever be a config mistake. Verified end-to-end on the Apple backend: the probe execs into the VM and the service reaches ready, with pg_isready's own output visible in the service log buffer under the `probe` stream. Co-Authored-By: Claude Opus 5 (1M context) --- docs/apple-container.md | 20 +++++ docs/configuration.md | 32 +++++-- internal/cli/skill/SKILL.md | 8 +- internal/config/config_test.go | 52 +++++++++++ internal/config/validate.go | 13 ++- internal/runner/apple.go | 4 + internal/runner/backend.go | 5 ++ internal/runner/backend_test.go | 58 ++++++++++++ internal/runner/container.go | 10 +++ internal/runner/docker.go | 4 + internal/runner/runner.go | 11 +++ internal/supervisor/probe.go | 26 +++++- internal/supervisor/probe_test.go | 143 +++++++++++++++++++++++++----- internal/supervisor/supervisor.go | 7 +- 14 files changed, 361 insertions(+), 32 deletions(-) diff --git a/docs/apple-container.md b/docs/apple-container.md index e5083db..65e354d 100644 --- a/docs/apple-container.md +++ b/docs/apple-container.md @@ -50,6 +50,26 @@ container services automatically. If you've changed the `container` default subnet (in `~/.config/container/config.toml`), set `apple.gateway_ip` to the matching gateway address. +## Readiness probes that exec into a container + +Use `kind: container_exec` rather than `kind: exec` with a hand-written `docker +exec`. Workbench supplies the container and the backend's CLI, so the probe is +portable across backends: + +```yaml +readiness: + kind: container_exec + command: pg_isready -U bench -d bench +``` + +A probe written as `kind: exec` with `command: docker exec my-postgres +pg_isready …` keeps working on Docker but fails here — the container isn't in +Docker's namespace, so every attempt reports `No such container: my-postgres` +until `max_attempts` is exhausted. The service itself is usually healthy the +whole time; only the probe is broken. If a readiness failure appears right after +switching backends, check the log buffer's `probe` lines for `No such +container` — that's this, and `container_exec` is the fix. + ## Differences from Docker - **Isolation** — one lightweight VM per container, rather than shared-kernel diff --git a/docs/configuration.md b/docs/configuration.md index 7b482a5..65764d0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -232,15 +232,15 @@ Common noisy directories (`.git`, `node_modules`, `__pycache__`) are always excl | Field | Type | Description | | --------------- | -------------- | --------------------------------------------------------------------------------- | -| `kind` | string | `none`, `log_pattern`, `tcp`, `http`, `exec`, or `grpc` | +| `kind` | string | `none`, `log_pattern`, `tcp`, `http`, `exec`, `container_exec`, or `grpc` | | `pattern` | string | Go regular expression matched against log lines (for `log_pattern`) | | `address` | string | TCP address to dial, `host:port` (for `tcp` and `grpc`) | | `url` | string | HTTP URL to GET; any 2xx response means ready (for `http`) | -| `command` | string or list | Shell command or argv to run (for `exec`); exit 0 = ready | +| `command` | string or list | Shell command or argv to run (for `exec` and `container_exec`); exit 0 = ready | | `service` | string | gRPC service name (for `grpc`); empty = overall server health | | `timeout` | duration | Per-attempt probe timeout (default `2s`) | | `initial_delay` | duration | Delay before the first probe attempt | -| `interval` | duration | Sleep between failed attempts (default `500ms`); applies to `tcp`, `http`, `exec` | +| `interval` | duration | Sleep between failed attempts (default `500ms`); applies to `tcp`, `http`, `exec`, `container_exec` | | `max_attempts` | integer | Cap on probe attempts before giving up (default `0` = unlimited) | | `settle` | duration | Delay between probe-success and the Ready transition | @@ -261,9 +261,29 @@ dependents parked in Pending. successful connect wins. - **`http`** issues `GET url` using an `http.Client` with `timeout`. Any 2xx response marks the service Ready. -- **`exec`** runs `command` with a `timeout` deadline per attempt. Exit 0 = ready. - stdout/stderr from the probe is appended to the service's log buffer tagged - with stream `probe`, so you can see what the probe is observing. +- **`exec`** runs `command` on the **host** with a `timeout` deadline per attempt. + Exit 0 = ready. stdout/stderr from the probe is appended to the service's log + buffer tagged with stream `probe`, so you can see what the probe is observing. +- **`container_exec`** runs `command` **inside the service's own container**, + otherwise behaving exactly like `exec`. Only valid on a service with a + `container:` block; `bench validate` rejects it elsewhere. Workbench supplies + both the container and the runtime CLI, so the probe works unchanged on either + [container backend](apple-container.md) and does not depend on + `container_prefix` or the service key: + + ```yaml + services: + postgres: + container: + image: postgres:16-alpine + readiness: + kind: container_exec + command: pg_isready -U bench -d bench + ``` + + Prefer this over `exec` with a hand-written `docker exec …`, which + hardcodes both the runtime and the container name and so breaks when the + backend resolves to Apple `container` or the prefix changes. - **`grpc`** issues a `grpc.health.v1.Health/Check` call against `address`. Ready when the server responds with status `SERVING`. Set `service` to probe a specific gRPC service registered for health reporting; leave it empty to diff --git a/internal/cli/skill/SKILL.md b/internal/cli/skill/SKILL.md index 98954fe..a8ae383 100644 --- a/internal/cli/skill/SKILL.md +++ b/internal/cli/skill/SKILL.md @@ -114,8 +114,12 @@ common stuff, not an exhaustive reference. - Status flow: `pending → starting → running → [setup →] ready`. The optional `setup` step runs a per-service bootstrap command after the readiness probe passes; dependents wait for `ready`. -- Readiness probe kinds: `tcp`, `http`, `log_pattern`, `exec`, `grpc`. Probe - stdout/stderr appears in the service log buffer tagged with stream `probe`. +- Readiness probe kinds: `tcp`, `http`, `log_pattern`, `exec`, `container_exec`, + `grpc`. Probe stdout/stderr appears in the service log buffer tagged with + stream `probe`. `exec` runs on the host; `container_exec` runs inside the + service's own container and is the portable way to probe a container (it + supplies the container name and the backend CLI, so it needs no `docker exec` + prefix and works on both container backends). - Log buffers are ring buffers — old lines rotate out. - Unknown YAML fields are rejected; a typo like `expect_status` under `readiness:` fails validation rather than silently being ignored. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5a5ea05..bd1d0b8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2599,3 +2599,55 @@ func TestTransitiveDeps(t *testing.T) { t.Error("expected error for unknown root") } } + +func TestValidate_ContainerExecReadiness(t *testing.T) { + containerSvc := func(r ReadinessConfig) ServiceConfig { + return ServiceConfig{ + Container: &ContainerConfig{Image: "postgres:16"}, + Restart: RestartConfig{Policy: "never"}, + Readiness: r, + } + } + + t.Run("valid on a container service", func(t *testing.T) { + cfg := &Config{Version: 1, Services: map[string]ServiceConfig{ + "db": containerSvc(ReadinessConfig{ + Kind: "container_exec", + Command: &Command{Parts: []string{"pg_isready", "-U", "bench"}}, + }), + }} + if err := cfg.Validate(); err != nil { + t.Fatalf("expected container_exec to validate, got %v", err) + } + }) + + t.Run("requires a command", func(t *testing.T) { + cfg := &Config{Version: 1, Services: map[string]ServiceConfig{ + "db": containerSvc(ReadinessConfig{Kind: "container_exec"}), + }} + err := cfg.Validate() + if err == nil { + t.Fatal("expected validation error for missing command") + } + assertContains(t, err.Error(), "container_exec requires a command") + }) + + t.Run("rejects a process service", func(t *testing.T) { + cfg := &Config{Version: 1, Services: map[string]ServiceConfig{ + "app": { + Dir: ".", + Command: &Command{Parts: []string{"echo"}}, + Restart: RestartConfig{Policy: "never"}, + Readiness: ReadinessConfig{ + Kind: "container_exec", + Command: &Command{Parts: []string{"pg_isready"}}, + }, + }, + }} + err := cfg.Validate() + if err == nil { + t.Fatal("expected validation error for a non-container service") + } + assertContains(t, err.Error(), "container_exec requires a container service") + }) +} diff --git a/internal/config/validate.go b/internal/config/validate.go index 2704598..2a20e02 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -93,7 +93,7 @@ func (c *Config) Validate() error { } switch svc.Readiness.Kind { - case "", "none", "log_pattern", "tcp", "http", "exec", "grpc": + case "", "none", "log_pattern", "tcp", "http", "exec", "container_exec", "grpc": // valid default: errs = append(errs, fmt.Sprintf("%s: invalid readiness kind %q", prefix, svc.Readiness.Kind)) @@ -111,6 +111,17 @@ func (c *Config) Validate() error { if svc.Readiness.Kind == "exec" && (svc.Readiness.Command == nil || len(svc.Readiness.Command.Parts) == 0) { errs = append(errs, fmt.Sprintf("%s: readiness kind exec requires a command", prefix)) } + if svc.Readiness.Kind == "container_exec" { + if svc.Readiness.Command == nil || len(svc.Readiness.Command.Parts) == 0 { + errs = append(errs, fmt.Sprintf("%s: readiness kind container_exec requires a command", prefix)) + } + // The probe runs inside the service's own container, so there has to + // be one. Caught here rather than at runtime because it can only ever + // be a config mistake. + if !svc.IsContainer() { + errs = append(errs, fmt.Sprintf("%s: readiness kind container_exec requires a container service", prefix)) + } + } if svc.Readiness.Kind == "grpc" && svc.Readiness.Address == "" { errs = append(errs, fmt.Sprintf("%s: readiness kind grpc requires an address", prefix)) } diff --git a/internal/runner/apple.go b/internal/runner/apple.go index 2ff3354..a631d95 100644 --- a/internal/runner/apple.go +++ b/internal/runner/apple.go @@ -80,6 +80,10 @@ func (appleBackend) RemoveArgs(target string, force bool) []string { return append(args, target) } +func (appleBackend) ExecArgs(id string, cmd []string) []string { + return append([]string{"exec", id}, cmd...) +} + // WaitExit polls `container inspect` until the container leaves the running // state, then reports its exit code. `container` has no `wait` subcommand. // diff --git a/internal/runner/backend.go b/internal/runner/backend.go index 1773c6e..bee43e5 100644 --- a/internal/runner/backend.go +++ b/internal/runner/backend.go @@ -70,6 +70,11 @@ type ContainerBackend interface { KillArgs(id string) []string // RemoveArgs builds the args to remove a container by name or id. RemoveArgs(target string, force bool) []string + // ExecArgs builds the args to run a command inside a running container. + // Both current backends spell this the same way, but routing it through the + // interface is what lets the container_exec readiness probe stay portable: + // bench.yml names the command to run, never the CLI that runs it. + ExecArgs(id string, cmd []string) []string // WaitExit blocks until the container terminates and returns its exit code. // The strategy differs per backend (Docker `wait` vs polling `inspect`). WaitExit(id string) int diff --git a/internal/runner/backend_test.go b/internal/runner/backend_test.go index 3c12840..858ae66 100644 --- a/internal/runner/backend_test.go +++ b/internal/runner/backend_test.go @@ -255,3 +255,61 @@ func TestParseAppleInspect(t *testing.T) { }) } } + +func TestBackends_ExecArgs(t *testing.T) { + cmd := []string{"pg_isready", "-U", "bench"} + tests := []struct { + name string + backend ContainerBackend + want []string + }{ + {"docker", dockerBackend{}, []string{"exec", "bench-db", "pg_isready", "-U", "bench"}}, + {"apple", newAppleBackend(config.GlobalConfig{}), []string{"exec", "bench-db", "pg_isready", "-U", "bench"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.backend.ExecArgs("bench-db", cmd); !reflect.DeepEqual(got, tt.want) { + t.Errorf("ExecArgs = %v, want %v", got, tt.want) + } + }) + } + // The caller's slice must not be aliased or mutated by arg building. + if !reflect.DeepEqual(cmd, []string{"pg_isready", "-U", "bench"}) { + t.Errorf("ExecArgs mutated the caller's command slice: %v", cmd) + } +} + +func TestContainerRunner_ExecCommand(t *testing.T) { + tests := []struct { + name string + backend ContainerBackend + wantBin string + }{ + {"docker", dockerBackend{}, "docker"}, + {"apple", newAppleBackend(config.GlobalConfig{}), "container"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewContainerRunner(config.ServiceConfig{}, "postgres", "astrobot", tt.backend) + bin, args := r.ExecCommand([]string{"pg_isready"}) + if bin != tt.wantBin { + t.Errorf("bin = %q, want %q", bin, tt.wantBin) + } + // Targets the prefix-derived name, and works before Start() has + // assigned a container id. + want := []string{"exec", "astrobot-postgres", "pg_isready"} + if !reflect.DeepEqual(args, want) { + t.Errorf("args = %v, want %v", args, want) + } + }) + } +} + +// ContainerRunner must satisfy ContainerExecer for the container_exec probe to +// find it via type assertion; ProcessRunner must not. +func TestContainerExecerImplementations(t *testing.T) { + var _ ContainerExecer = (*ContainerRunner)(nil) + if _, ok := any(NewProcessRunner(config.ServiceConfig{})).(ContainerExecer); ok { + t.Error("ProcessRunner must not implement ContainerExecer") + } +} diff --git a/internal/runner/container.go b/internal/runner/container.go index e16b334..bbc8202 100644 --- a/internal/runner/container.go +++ b/internal/runner/container.go @@ -142,6 +142,16 @@ func (r *ContainerRunner) Stop(exitCh <-chan int, timeout time.Duration) { _ = exec.Command(bin, r.backend.RemoveArgs(r.containerID, false)...).Run() } +// ExecCommand returns the binary and arguments that run cmd inside this +// service's container, satisfying ContainerExecer. +// +// It targets the container by name rather than by id: the name is derived from +// the configured prefix and service key at construction, so it is available +// before Start() has assigned an id and stays valid across restarts. +func (r *ContainerRunner) ExecCommand(cmd []string) (string, []string) { + return r.backend.Binary(), r.backend.ExecArgs(r.name, cmd) +} + func (r *ContainerRunner) Info() RunnerInfo { shortID := r.containerID if len(shortID) > 12 { diff --git a/internal/runner/docker.go b/internal/runner/docker.go index 00d32e6..2f29d4a 100644 --- a/internal/runner/docker.go +++ b/internal/runner/docker.go @@ -50,6 +50,10 @@ func (dockerBackend) RemoveArgs(target string, force bool) []string { return append(args, "-v", target) } +func (dockerBackend) ExecArgs(id string, cmd []string) []string { + return append([]string{"exec", id}, cmd...) +} + func (dockerBackend) WaitExit(id string) int { // `docker wait` blocks until the container exits and prints the exit code. out, err := exec.Command("docker", "wait", id).Output() diff --git a/internal/runner/runner.go b/internal/runner/runner.go index cdc9982..5cfcef4 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -21,6 +21,17 @@ type Runner interface { Info() RunnerInfo } +// ContainerExecer is implemented by runners that can run a command inside the +// container they manage. The container_exec readiness probe uses it so a probe +// can target a service's own container without bench.yml naming either the +// container or the backend's CLI. ProcessRunner does not implement it, so a +// failed type assertion is how the probe detects a non-container service. +type ContainerExecer interface { + // ExecCommand returns the binary and arguments that run cmd inside the + // managed container. + ExecCommand(cmd []string) (bin string, args []string) +} + // RunnerInfo holds runtime details that differ between process and container runners. type RunnerInfo struct { Type string // "process" or "container" diff --git a/internal/supervisor/probe.go b/internal/supervisor/probe.go index 3c5203a..e68b561 100644 --- a/internal/supervisor/probe.go +++ b/internal/supervisor/probe.go @@ -17,6 +17,7 @@ import ( "github.com/ccakes/workbench/internal/config" "github.com/ccakes/workbench/internal/logbuf" + "github.com/ccakes/workbench/internal/runner" ) const ( @@ -37,7 +38,10 @@ const ( // exit codes. On success, the configured `settle` delay is observed before // returning true so dependents do not unblock during the gap between // "probe passed" and "service really ready." -func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buffer, baselineSeq uint64) bool { +// +// execer supplies the container-exec invocation for the container_exec kind and +// is nil for process services; every other kind ignores it. +func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buffer, baselineSeq uint64, execer runner.ContainerExecer) bool { kind := cfg.Kind if kind == "" || kind == "none" { return true @@ -88,6 +92,26 @@ func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buff ok = retryProbe(ctx, cfg.MaxAttempts, interval, func() bool { return probeExecOnce(ctx, parts, perAttempt, logs) }) + case "container_exec": + if cfg.Command == nil || len(cfg.Command.Parts) == 0 { + if logs != nil { + logs.Add("stderr", "readiness: container_exec kind requires a command") + } + return false + } + if execer == nil { + if logs != nil { + logs.Add("stderr", "readiness: container_exec is only valid for a container service") + } + return false + } + // Resolve the invocation once: both the container name and the backend + // CLI are fixed for the life of this probe. + bin, execArgs := execer.ExecCommand(cfg.Command.Parts) + parts := append([]string{bin}, execArgs...) + ok = retryProbe(ctx, cfg.MaxAttempts, interval, func() bool { + return probeExecOnce(ctx, parts, perAttempt, logs) + }) case "grpc": addr := cfg.Address svcName := cfg.Service diff --git a/internal/supervisor/probe_test.go b/internal/supervisor/probe_test.go index 4316192..7a01e19 100644 --- a/internal/supervisor/probe_test.go +++ b/internal/supervisor/probe_test.go @@ -5,6 +5,7 @@ import ( "net" "net/http" "net/http/httptest" + "reflect" "runtime" "strings" "testing" @@ -68,7 +69,7 @@ func TestProbeTCP_Ready(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if !runProbe(ctx, tcpReadiness(listenerAddr(l), 200*time.Millisecond, 0), nil, 0) { + if !runProbe(ctx, tcpReadiness(listenerAddr(l), 200*time.Millisecond, 0), nil, 0, nil) { t.Fatalf("expected TCP probe to succeed") } } @@ -100,7 +101,7 @@ func TestProbeTCP_RetriesThenReady(t *testing.T) { defer cancel() start := time.Now() - if !runProbe(ctx, tcpReadiness(addr, 200*time.Millisecond, 0), nil, 0) { + if !runProbe(ctx, tcpReadiness(addr, 200*time.Millisecond, 0), nil, 0, nil) { t.Fatalf("expected TCP probe to succeed after retry") } if elapsed := time.Since(start); elapsed < 50*time.Millisecond { @@ -114,7 +115,7 @@ func TestProbeTCP_CancelledBeforeReady(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) done := make(chan bool, 1) go func() { - done <- runProbe(ctx, tcpReadiness(addr, 100*time.Millisecond, 0), nil, 0) + done <- runProbe(ctx, tcpReadiness(addr, 100*time.Millisecond, 0), nil, 0, nil) }() time.Sleep(50 * time.Millisecond) @@ -139,7 +140,7 @@ func TestProbeHTTP_2xx(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if !runProbe(ctx, httpReadiness(srv.URL, 500*time.Millisecond), nil, 0) { + if !runProbe(ctx, httpReadiness(srv.URL, 500*time.Millisecond), nil, 0, nil) { t.Fatalf("expected HTTP probe to succeed on 200") } } @@ -153,7 +154,7 @@ func TestProbeHTTP_5xxNeverReady(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer cancel() - if runProbe(ctx, httpReadiness(srv.URL, 200*time.Millisecond), nil, 0) { + if runProbe(ctx, httpReadiness(srv.URL, 200*time.Millisecond), nil, 0, nil) { t.Fatalf("expected HTTP probe to return false (never reaches 2xx)") } } @@ -165,7 +166,7 @@ func TestProbeHTTP_NonDialable(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer cancel() - if runProbe(ctx, httpReadiness(url, 100*time.Millisecond), nil, 0) { + if runProbe(ctx, httpReadiness(url, 100*time.Millisecond), nil, 0, nil) { t.Fatalf("expected HTTP probe to return false against closed port") } } @@ -185,7 +186,7 @@ func TestProbeLogPattern_MatchAfterBaseline(t *testing.T) { done := make(chan bool, 1) go func() { - done <- runProbe(ctx, logPatternReadiness("listening on"), buf, baseline) + done <- runProbe(ctx, logPatternReadiness("listening on"), buf, baseline, nil) }() time.Sleep(50 * time.Millisecond) @@ -212,7 +213,7 @@ func TestProbeLogPattern_IgnoresPreBaseline(t *testing.T) { done := make(chan bool, 1) go func() { - done <- runProbe(ctx, logPatternReadiness("listening on"), buf, baseline) + done <- runProbe(ctx, logPatternReadiness("listening on"), buf, baseline, nil) }() select { @@ -238,7 +239,7 @@ func TestProbeInitialDelay(t *testing.T) { cfg := tcpReadiness(listenerAddr(l), 200*time.Millisecond, 200*time.Millisecond) start := time.Now() - if !runProbe(ctx, cfg, nil, 0) { + if !runProbe(ctx, cfg, nil, 0, nil) { t.Fatalf("expected probe to eventually succeed") } if elapsed := time.Since(start); elapsed < 180*time.Millisecond { @@ -255,7 +256,7 @@ func TestProbeBadRegex(t *testing.T) { goroutinesBefore := runtime.NumGoroutine() cfg := logPatternReadiness("[invalid(regex") // unclosed character class - result := runProbe(ctx, cfg, buf, 0) + result := runProbe(ctx, cfg, buf, 0, nil) if result { t.Fatalf("expected probe to return false on bad regex") } @@ -286,10 +287,10 @@ func TestRunProbe_NoneKindIsInstantReady(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - if !runProbe(ctx, config.ReadinessConfig{Kind: ""}, nil, 0) { + if !runProbe(ctx, config.ReadinessConfig{Kind: ""}, nil, 0, nil) { t.Error("empty kind should be instant-ready") } - if !runProbe(ctx, config.ReadinessConfig{Kind: "none"}, nil, 0) { + if !runProbe(ctx, config.ReadinessConfig{Kind: "none"}, nil, 0, nil) { t.Error("'none' kind should be instant-ready") } } @@ -302,7 +303,7 @@ func TestProbeExec_ExitZeroReady(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if !runProbe(ctx, cfg, nil, 0) { + if !runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected exec probe to succeed on exit 0") } } @@ -316,7 +317,7 @@ func TestProbeExec_StreamsOutputToLogs(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if !runProbe(ctx, cfg, buf, 0) { + if !runProbe(ctx, cfg, buf, 0, nil) { t.Fatal("expected exec probe to succeed") } // Give the streaming goroutine a moment to drain. @@ -345,7 +346,7 @@ func TestProbeExec_MaxAttemptsCap(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() start := time.Now() - if runProbe(ctx, cfg, nil, 0) { + if runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected exec probe to fail when command always exits non-zero") } elapsed := time.Since(start) @@ -366,7 +367,7 @@ func TestProbeTCP_MaxAttemptsCap(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if runProbe(ctx, cfg, nil, 0) { + if runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected tcp probe to give up after MaxAttempts") } } @@ -388,7 +389,7 @@ func TestProbeSettleDelaysReady(t *testing.T) { defer cancel() start := time.Now() - if !runProbe(ctx, cfg, nil, 0) { + if !runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected probe to succeed") } elapsed := time.Since(start) @@ -406,7 +407,7 @@ func TestProbeGRPC_Serving(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if !runProbe(ctx, cfg, nil, 0) { + if !runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected grpc probe to succeed when status=SERVING") } } @@ -427,7 +428,7 @@ func TestProbeGRPC_NotServingThenSucceeds(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if !runProbe(ctx, cfg, nil, 0) { + if !runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected grpc probe to succeed after server flips to SERVING") } } @@ -443,7 +444,7 @@ func TestProbeGRPC_MaxAttemptsCap(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if runProbe(ctx, cfg, nil, 0) { + if runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected grpc probe to give up after MaxAttempts when not SERVING") } } @@ -466,7 +467,107 @@ func TestProbeSettleCancellable(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - if runProbe(ctx, cfg, nil, 0) { + if runProbe(ctx, cfg, nil, 0, nil) { t.Fatal("expected runProbe to return false when settle is interrupted") } } + +// stubExecer stands in for a ContainerRunner: it records the command the probe +// asked to run inside the container and returns a fixed host invocation. +type stubExecer struct { + bin string + args []string + got []string +} + +func (s *stubExecer) ExecCommand(cmd []string) (string, []string) { + s.got = cmd + return s.bin, s.args +} + +func TestProbeContainerExec_ForwardsCommandAndSucceeds(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + ex := &stubExecer{bin: "sh", args: []string{"-c", "exit 0"}} + cfg := config.ReadinessConfig{ + Kind: "container_exec", + Command: &config.Command{Parts: []string{"pg_isready", "-U", "bench"}}, + Timeout: config.Duration{Duration: 2 * time.Second}, + MaxAttempts: 1, + } + + if !runProbe(ctx, cfg, nil, 0, ex) { + t.Fatal("expected container_exec probe to succeed on exit 0") + } + // The configured command must reach the backend verbatim — the probe adds + // the container and CLI, never rewrites what the user asked to run. + want := []string{"pg_isready", "-U", "bench"} + if !reflect.DeepEqual(ex.got, want) { + t.Errorf("ExecCommand got %v, want %v", ex.got, want) + } +} + +func TestProbeContainerExec_NonZeroExitFails(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + ex := &stubExecer{bin: "sh", args: []string{"-c", "exit 1"}} + cfg := config.ReadinessConfig{ + Kind: "container_exec", + Command: &config.Command{Parts: []string{"pg_isready"}}, + Timeout: config.Duration{Duration: time.Second}, + Interval: config.Duration{Duration: 10 * time.Millisecond}, + MaxAttempts: 2, + } + + if runProbe(ctx, cfg, nil, 0, ex) { + t.Fatal("expected container_exec probe to fail on non-zero exit") + } +} + +func TestProbeContainerExec_NilExecerFails(t *testing.T) { + buf := logbuf.New(100) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + cfg := config.ReadinessConfig{ + Kind: "container_exec", + Command: &config.Command{Parts: []string{"pg_isready"}}, + MaxAttempts: 1, + } + + // A process service yields a nil execer; the probe must fail loudly rather + // than hang or silently pass. + if runProbe(ctx, cfg, buf, 0, nil) { + t.Fatal("expected container_exec to fail for a non-container service") + } + if !logContains(buf, "only valid for a container service") { + t.Error("expected the non-container error to be logged") + } +} + +func TestProbeContainerExec_MissingCommandFails(t *testing.T) { + buf := logbuf.New(100) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + ex := &stubExecer{bin: "true"} + cfg := config.ReadinessConfig{Kind: "container_exec", MaxAttempts: 1} + + if runProbe(ctx, cfg, buf, 0, ex) { + t.Fatal("expected container_exec to fail with no command") + } + if !logContains(buf, "requires a command") { + t.Error("expected the missing-command error to be logged") + } +} + +func logContains(buf *logbuf.Buffer, substr string) bool { + for _, line := range buf.Lines() { + if strings.Contains(line.Text, substr) { + return true + } + } + return false +} diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index 7269213..c66458b 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -398,8 +398,13 @@ func (s *Supervisor) runLoop(ms *managedService) { if last := ms.logs.Last(1); len(last) == 1 { baseline = last[0].Seq } + // Resolved before the goroutine starts so the probe never races the + // runLoop for ms.r. Process runners don't implement ContainerExecer, + // which leaves execer nil and makes container_exec fail with a clear + // message rather than silently probing nothing. + execer, _ := ms.r.(runner.ContainerExecer) probeWG.Go(func() { - if !runProbe(probeCtx, ms.cfg.Readiness, ms.logs, baseline) { + if !runProbe(probeCtx, ms.cfg.Readiness, ms.logs, baseline, execer) { if probeCtx.Err() == nil { ms.mu.Lock() ms.startupErr = readinessFailureReason(ms.cfg.Readiness) From d112aa29a7aff2f329016309526340f16f2dce5d Mon Sep 17 00:00:00 2001 From: Cameron Daniel Date: Wed, 26 Aug 2026 07:58:54 +1000 Subject: [PATCH 4/5] Generalize service hook configuration Setup hooks need the same runtime choice as readiness commands. Preserve command-only configs as host exec hooks and report their deprecation. --- docs/apple-container.md | 6 +- docs/configuration.md | 18 ++- internal/cli/cli.go | 10 +- internal/cli/cli_test.go | 28 ++++ internal/cli/skill/SKILL.md | 4 +- internal/config/config.go | 95 +++++++------ internal/config/config_test.go | 177 ++++++++++++++++++++++--- internal/config/validate.go | 39 +++++- internal/runner/backend.go | 2 +- internal/runner/runner.go | 4 +- internal/supervisor/probe.go | 53 ++++---- internal/supervisor/probe_test.go | 44 +++--- internal/supervisor/setup.go | 40 +++--- internal/supervisor/setup_test.go | 37 ++++++ internal/supervisor/supervisor.go | 6 +- internal/supervisor/supervisor_test.go | 18 +-- 16 files changed, 425 insertions(+), 156 deletions(-) create mode 100644 internal/supervisor/setup_test.go diff --git a/docs/apple-container.md b/docs/apple-container.md index 65e354d..aee734f 100644 --- a/docs/apple-container.md +++ b/docs/apple-container.md @@ -50,11 +50,11 @@ container services automatically. If you've changed the `container` default subnet (in `~/.config/container/config.toml`), set `apple.gateway_ip` to the matching gateway address. -## Readiness probes that exec into a container +## Commands that exec into a container Use `kind: container_exec` rather than `kind: exec` with a hand-written `docker -exec`. Workbench supplies the container and the backend's CLI, so the probe is -portable across backends: +exec`. Workbench supplies the container and the backend's CLI, so readiness +probes and setup hooks are portable across backends: ```yaml readiness: diff --git a/docs/configuration.md b/docs/configuration.md index 65764d0..1f7b06c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -65,7 +65,7 @@ Unknown YAML fields are rejected at parse time. A typo such as `expect_status: 2 ``` error: parsing config bench.yml: parsing config: yaml: unmarshal errors: - line 9: field expect_status not found in type config.ReadinessConfig + line 9: field expect_status not found in type config.ServiceHookConfig ``` Run `bench validate` to surface these errors without starting any services. @@ -340,11 +340,12 @@ passes (and after any `settle` delay), before the service transitions to bootstrap that's logically part of bringing this service up — creating a dev environment in a flag service, seeding a default DB user, applying migrations. -| Field | Type | Description | -| --------- | -------------- | ------------------------------------------------------ | -| `command` | string or list | Shell command or argv to run; exit 0 = setup succeeded | -| `timeout` | duration | Cap on setup runtime (default `60s`) | -| `env` | map | Extra env applied on top of the service's env | +| Field | Type | Description | +| --------- | -------------- | ------------------------------------------------------------- | +| `kind` | string | `exec` (host) or `container_exec` (the service's container) | +| `command` | string or list | Shell command or argv to run; exit 0 = setup succeeded | +| `timeout` | duration | Cap on setup runtime (default `60s`) | +| `env` | map | Extra environment for `exec`; unsupported by `container_exec` | ```yaml services: @@ -354,10 +355,15 @@ services: kind: http url: http://localhost:4242/health setup: + kind: exec command: ./bin/flagman create-env development timeout: 30s ``` +`container_exec` is only valid for container services and uses the configured +container backend. A legacy setup block containing `command` without `kind` +still runs as `exec`, with a deprecation warning. + The status flow is `Running → Setup → Ready`. On non-zero exit or timeout the supervisor stops the service and marks it **Failed** with the setup error in `last_error`, so dependents cascade just as they would for any other failure. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 9b53f83..fa36cce 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -179,7 +179,15 @@ func loadConfig(configPath string) (*config.Config, error) { if err != nil { return nil, err } - return config.Load(path) + cfg, err := config.Load(path) + if err != nil { + return nil, err + } + for _, warning := range cfg.Warnings() { + fmt.Fprintf(os.Stderr, "warning: %s\n", warning) + } + + return cfg, nil } // connectToRunning attempts to connect to a running bench instance. diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 44e703e..0be80ca 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -104,6 +104,34 @@ services: } } +func TestRunValidateWarnsLegacySetup(t *testing.T) { + tmp := t.TempDir() + path := filepath.Join(tmp, "bench.yml") + data := []byte(` +version: 1 +services: + app: + dir: . + command: echo app + setup: + command: echo setup +`) + if err := os.WriteFile(path, data, 0644); err != nil { + t.Fatal(err) + } + + var code int + _, stderr := captureStdoutStderr(t, func() { + code = runValidate([]string{"-config", path}) + }) + if code != 0 { + t.Fatalf("runValidate returned %d: %s", code, stderr) + } + if !strings.Contains(stderr, "warning: service \"app\": setup.command without setup.kind is deprecated; using exec") { + t.Fatalf("missing deprecation warning: %s", stderr) + } +} + func TestApplyServiceSubset(t *testing.T) { cfg := &config.Config{ Services: map[string]config.ServiceConfig{ diff --git a/internal/cli/skill/SKILL.md b/internal/cli/skill/SKILL.md index a8ae383..379a9f7 100644 --- a/internal/cli/skill/SKILL.md +++ b/internal/cli/skill/SKILL.md @@ -112,8 +112,8 @@ common stuff, not an exhaustive reference. - Services are either processes (`command:`) or containers (`container:`); containers need Docker running. - Status flow: `pending → starting → running → [setup →] ready`. The optional - `setup` step runs a per-service bootstrap command after the readiness probe - passes; dependents wait for `ready`. + `setup` step runs a host `exec` or service `container_exec` bootstrap command + after the readiness probe passes; dependents wait for `ready`. - Readiness probe kinds: `tcp`, `http`, `log_pattern`, `exec`, `container_exec`, `grpc`. Probe stdout/stderr appears in the service log buffer tagged with stream `probe`. `exec` runs on the host; `container_exec` runs inside the diff --git a/internal/config/config.go b/internal/config/config.go index 33bbebf..d4158e4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -24,6 +24,7 @@ type Config struct { Extends string `yaml:"extends"` Global GlobalConfig `yaml:"global"` Services map[string]ServiceConfig `yaml:"services"` + warnings []string } type GlobalConfig struct { @@ -114,23 +115,23 @@ type ContainerConfig struct { } type ServiceConfig struct { - Name string `yaml:"name"` - Dir string `yaml:"dir"` - Command *Command `yaml:"command"` - Container *ContainerConfig `yaml:"container"` - Env map[string]string `yaml:"env"` - EnvFile string `yaml:"env_file"` - AutoStart *bool `yaml:"auto_start"` - DependsOn []string `yaml:"depends_on"` - Restart RestartConfig `yaml:"restart"` - Watch WatchConfig `yaml:"watch"` - Readiness ReadinessConfig `yaml:"readiness"` - Setup *SetupConfig `yaml:"setup"` - Profiles []string `yaml:"profiles"` - Group string `yaml:"group"` - Labels map[string]string `yaml:"labels"` - StopSignal string `yaml:"stop_signal"` - ShutdownTimeout *Duration `yaml:"shutdown_timeout"` + Name string `yaml:"name"` + Dir string `yaml:"dir"` + Command *Command `yaml:"command"` + Container *ContainerConfig `yaml:"container"` + Env map[string]string `yaml:"env"` + EnvFile string `yaml:"env_file"` + AutoStart *bool `yaml:"auto_start"` + DependsOn []string `yaml:"depends_on"` + Restart RestartConfig `yaml:"restart"` + Watch WatchConfig `yaml:"watch"` + Readiness ServiceHookConfig `yaml:"readiness"` + Setup *ServiceHookConfig `yaml:"setup"` + Profiles []string `yaml:"profiles"` + Group string `yaml:"group"` + Labels map[string]string `yaml:"labels"` + StopSignal string `yaml:"stop_signal"` + ShutdownTimeout *Duration `yaml:"shutdown_timeout"` } // HasProfile returns true if this service is tagged with the given profile. @@ -143,17 +144,6 @@ func (s *ServiceConfig) HasProfile(name string) bool { return false } -// SetupConfig configures a post-ready hook that runs after the service's -// readiness probe passes and before dependents are unblocked. Useful for -// per-service bootstrap steps (creating dev users, seeding flag environments, -// running migrations) that today require wrapping the service's command in a -// shell pipeline. -type SetupConfig struct { - Command Command `yaml:"command"` - Timeout Duration `yaml:"timeout"` - Env map[string]string `yaml:"env"` -} - // IsContainer returns true if this service is a container service. func (s *ServiceConfig) IsContainer() bool { return s.Container != nil @@ -216,18 +206,27 @@ func (w *WatchConfig) ShouldRestart() bool { return *w.Restart } -type ReadinessConfig struct { - Kind string `yaml:"kind"` - Pattern string `yaml:"pattern"` - Address string `yaml:"address"` - URL string `yaml:"url"` - Command *Command `yaml:"command"` - Service string `yaml:"service"` // gRPC service name; empty = overall server health - Timeout Duration `yaml:"timeout"` - InitialDelay Duration `yaml:"initial_delay"` - Interval Duration `yaml:"interval"` - Settle Duration `yaml:"settle"` - MaxAttempts int `yaml:"max_attempts"` +const ( + // ExecKind runs a command on the host. + ExecKind = "exec" + // ContainerExecKind runs a command inside the service container. + ContainerExecKind = "container_exec" +) + +// ServiceHookConfig configures readiness and setup hooks. +type ServiceHookConfig struct { + Kind string `yaml:"kind"` + Pattern string `yaml:"pattern"` + Address string `yaml:"address"` + URL string `yaml:"url"` + Command *Command `yaml:"command"` + Service string `yaml:"service"` // gRPC service name; empty = overall server health + Timeout Duration `yaml:"timeout"` + InitialDelay Duration `yaml:"initial_delay"` + Interval Duration `yaml:"interval"` + Settle Duration `yaml:"settle"` + MaxAttempts int `yaml:"max_attempts"` + Env map[string]string `yaml:"env"` // setup exec only } type TracingConfig struct { @@ -719,10 +718,26 @@ func (c *Config) applyDefaults() { if len(svc.Watch.Paths) == 0 && svc.Watch.IsEnabled() { svc.Watch.Paths = []string{"."} } + if svc.Setup != nil && svc.Setup.Kind == "" && svc.Setup.Command != nil { + svc.Setup.Kind = ExecKind + c.warnings = append(c.warnings, fmt.Sprintf( + "service %q: setup.command without setup.kind is deprecated; using %s", + key, + ExecKind, + )) + } c.Services[key] = svc } } +// Warnings returns non-fatal config diagnostics. +func (c *Config) Warnings() []string { + warnings := append([]string(nil), c.warnings...) + sort.Strings(warnings) + + return warnings +} + // FindConfig searches for bench.yml in the current and parent directories. func FindConfig() (string, error) { names := []string{"bench.yml", "bench.yaml"} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index bd1d0b8..5997cf3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -233,6 +234,71 @@ services: } } +func TestParse_SetupKinds(t *testing.T) { + yaml := []byte(` +version: 1 +services: + app: + dir: /tmp + command: echo app + setup: + kind: exec + command: echo setup + timeout: 3s + db: + container: + image: postgres:16 + setup: + kind: container_exec + command: [psql, -c, "select 1"] +`) + cfg, err := Parse(yaml, "/tmp") + if err != nil { + t.Fatalf("Parse: %v", err) + } + + app := cfg.Services["app"].Setup + if app == nil || app.Kind != ExecKind || app.Command.String() != "echo setup" { + t.Fatalf("unexpected exec setup: %#v", app) + } + if app.Timeout.Duration != 3*time.Second { + t.Fatalf("timeout = %v, want 3s", app.Timeout.Duration) + } + + db := cfg.Services["db"].Setup + if db == nil || db.Kind != ContainerExecKind { + t.Fatalf("unexpected container setup: %#v", db) + } + want := []string{"psql", "-c", "select 1"} + if !reflect.DeepEqual(db.Command.Parts, want) { + t.Fatalf("command = %v, want %v", db.Command.Parts, want) + } +} + +func TestParse_LegacySetupCommand(t *testing.T) { + yaml := []byte(` +version: 1 +services: + app: + dir: /tmp + command: echo app + setup: + command: echo setup +`) + cfg, err := Parse(yaml, "/tmp") + if err != nil { + t.Fatalf("Parse: %v", err) + } + + if got := cfg.Services["app"].Setup.Kind; got != ExecKind { + t.Fatalf("setup kind = %q, want %q", got, ExecKind) + } + warnings := cfg.Warnings() + if len(warnings) != 1 || !strings.Contains(warnings[0], "setup.command without setup.kind") { + t.Fatalf("warnings = %v", warnings) + } +} + func TestParse_InvalidDuration(t *testing.T) { yaml := []byte(` version: 1 @@ -868,25 +934,25 @@ func TestValidate_ReadinessKinds(t *testing.T) { dir := t.TempDir() tests := []struct { name string - readiness ReadinessConfig + readiness ServiceHookConfig wantErr string }{ - {name: "none", readiness: ReadinessConfig{Kind: "none"}, wantErr: ""}, - {name: "empty", readiness: ReadinessConfig{Kind: ""}, wantErr: ""}, - {name: "log_pattern valid", readiness: ReadinessConfig{Kind: "log_pattern", Pattern: "ready"}, wantErr: ""}, - {name: "log_pattern missing pattern", readiness: ReadinessConfig{Kind: "log_pattern"}, wantErr: "requires a pattern"}, - {name: "tcp valid", readiness: ReadinessConfig{Kind: "tcp", Address: ":8080"}, wantErr: ""}, - {name: "tcp missing address", readiness: ReadinessConfig{Kind: "tcp"}, wantErr: "requires an address"}, - {name: "http valid", readiness: ReadinessConfig{Kind: "http", URL: "http://localhost"}, wantErr: ""}, - {name: "http missing url", readiness: ReadinessConfig{Kind: "http"}, wantErr: "requires a url"}, - {name: "invalid kind", readiness: ReadinessConfig{Kind: "bogus"}, wantErr: "invalid readiness kind"}, - {name: "exec valid", readiness: ReadinessConfig{Kind: "exec", Command: &Command{Parts: []string{"echo", "ok"}}}, wantErr: ""}, - {name: "exec missing command", readiness: ReadinessConfig{Kind: "exec"}, wantErr: "requires a command"}, - {name: "grpc valid", readiness: ReadinessConfig{Kind: "grpc", Address: "localhost:50051"}, wantErr: ""}, - {name: "grpc missing address", readiness: ReadinessConfig{Kind: "grpc"}, wantErr: "requires an address"}, - {name: "negative max_attempts", readiness: ReadinessConfig{Kind: "none", MaxAttempts: -1}, wantErr: "max_attempts must be >= 0"}, - {name: "negative interval", readiness: ReadinessConfig{Kind: "none", Interval: Duration{Duration: -time.Second}}, wantErr: "interval must be >= 0"}, - {name: "negative settle", readiness: ReadinessConfig{Kind: "none", Settle: Duration{Duration: -time.Second}}, wantErr: "settle must be >= 0"}, + {name: "none", readiness: ServiceHookConfig{Kind: "none"}, wantErr: ""}, + {name: "empty", readiness: ServiceHookConfig{Kind: ""}, wantErr: ""}, + {name: "log_pattern valid", readiness: ServiceHookConfig{Kind: "log_pattern", Pattern: "ready"}, wantErr: ""}, + {name: "log_pattern missing pattern", readiness: ServiceHookConfig{Kind: "log_pattern"}, wantErr: "requires a pattern"}, + {name: "tcp valid", readiness: ServiceHookConfig{Kind: "tcp", Address: ":8080"}, wantErr: ""}, + {name: "tcp missing address", readiness: ServiceHookConfig{Kind: "tcp"}, wantErr: "requires an address"}, + {name: "http valid", readiness: ServiceHookConfig{Kind: "http", URL: "http://localhost"}, wantErr: ""}, + {name: "http missing url", readiness: ServiceHookConfig{Kind: "http"}, wantErr: "requires a url"}, + {name: "invalid kind", readiness: ServiceHookConfig{Kind: "bogus"}, wantErr: "invalid readiness kind"}, + {name: "exec valid", readiness: ServiceHookConfig{Kind: "exec", Command: &Command{Parts: []string{"echo", "ok"}}}, wantErr: ""}, + {name: "exec missing command", readiness: ServiceHookConfig{Kind: "exec"}, wantErr: "requires a command"}, + {name: "grpc valid", readiness: ServiceHookConfig{Kind: "grpc", Address: "localhost:50051"}, wantErr: ""}, + {name: "grpc missing address", readiness: ServiceHookConfig{Kind: "grpc"}, wantErr: "requires an address"}, + {name: "negative max_attempts", readiness: ServiceHookConfig{Kind: "none", MaxAttempts: -1}, wantErr: "max_attempts must be >= 0"}, + {name: "negative interval", readiness: ServiceHookConfig{Kind: "none", Interval: Duration{Duration: -time.Second}}, wantErr: "interval must be >= 0"}, + {name: "negative settle", readiness: ServiceHookConfig{Kind: "none", Settle: Duration{Duration: -time.Second}}, wantErr: "settle must be >= 0"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2601,7 +2667,7 @@ func TestTransitiveDeps(t *testing.T) { } func TestValidate_ContainerExecReadiness(t *testing.T) { - containerSvc := func(r ReadinessConfig) ServiceConfig { + containerSvc := func(r ServiceHookConfig) ServiceConfig { return ServiceConfig{ Container: &ContainerConfig{Image: "postgres:16"}, Restart: RestartConfig{Policy: "never"}, @@ -2611,7 +2677,7 @@ func TestValidate_ContainerExecReadiness(t *testing.T) { t.Run("valid on a container service", func(t *testing.T) { cfg := &Config{Version: 1, Services: map[string]ServiceConfig{ - "db": containerSvc(ReadinessConfig{ + "db": containerSvc(ServiceHookConfig{ Kind: "container_exec", Command: &Command{Parts: []string{"pg_isready", "-U", "bench"}}, }), @@ -2623,7 +2689,7 @@ func TestValidate_ContainerExecReadiness(t *testing.T) { t.Run("requires a command", func(t *testing.T) { cfg := &Config{Version: 1, Services: map[string]ServiceConfig{ - "db": containerSvc(ReadinessConfig{Kind: "container_exec"}), + "db": containerSvc(ServiceHookConfig{Kind: "container_exec"}), }} err := cfg.Validate() if err == nil { @@ -2638,7 +2704,7 @@ func TestValidate_ContainerExecReadiness(t *testing.T) { Dir: ".", Command: &Command{Parts: []string{"echo"}}, Restart: RestartConfig{Policy: "never"}, - Readiness: ReadinessConfig{ + Readiness: ServiceHookConfig{ Kind: "container_exec", Command: &Command{Parts: []string{"pg_isready"}}, }, @@ -2651,3 +2717,72 @@ func TestValidate_ContainerExecReadiness(t *testing.T) { assertContains(t, err.Error(), "container_exec requires a container service") }) } + +func TestValidate_SetupKinds(t *testing.T) { + command := &Command{Parts: []string{"echo", "setup"}} + processSvc := func(setup *ServiceHookConfig) ServiceConfig { + return ServiceConfig{ + Dir: ".", + Command: &Command{Parts: []string{"echo"}}, + Restart: RestartConfig{Policy: "never"}, + Setup: setup, + } + } + containerSvc := func(setup *ServiceHookConfig) ServiceConfig { + return ServiceConfig{ + Container: &ContainerConfig{Image: "postgres:16"}, + Restart: RestartConfig{Policy: "never"}, + Setup: setup, + } + } + setup := func(kind string) *ServiceHookConfig { + return &ServiceHookConfig{Kind: kind, Command: command} + } + + tests := []struct { + name string + service ServiceConfig + wantErr string + }{ + {name: "host exec", service: processSvc(setup(ExecKind))}, + {name: "container exec", service: containerSvc(setup(ContainerExecKind))}, + {name: "container exec on process", service: processSvc(setup(ContainerExecKind)), wantErr: "requires a container service"}, + {name: "invalid kind", service: processSvc(setup("http")), wantErr: "invalid setup kind"}, + {name: "missing command", service: processSvc(&ServiceHookConfig{Kind: ExecKind}), wantErr: "requires a command"}, + { + name: "probe option", + service: processSvc(&ServiceHookConfig{ + Kind: ExecKind, + Command: command, + Interval: Duration{Duration: time.Second}, + }), + wantErr: "setup only supports kind, command, timeout, and env", + }, + { + name: "container env", + service: containerSvc(&ServiceHookConfig{ + Kind: ContainerExecKind, + Command: command, + Env: map[string]string{"ROLE": "admin"}, + }), + wantErr: "env is only supported for kind exec", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &Config{Version: 1, Services: map[string]ServiceConfig{"app": tt.service}} + err := cfg.Validate() + if tt.wantErr == "" && err != nil { + t.Fatalf("Validate: %v", err) + } + if tt.wantErr == "" { + return + } + if err == nil { + t.Fatal("expected validation error") + } + assertContains(t, err.Error(), tt.wantErr) + }) + } +} diff --git a/internal/config/validate.go b/internal/config/validate.go index 2a20e02..37f09d0 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -93,7 +93,7 @@ func (c *Config) Validate() error { } switch svc.Readiness.Kind { - case "", "none", "log_pattern", "tcp", "http", "exec", "container_exec", "grpc": + case "", "none", "log_pattern", "tcp", "http", ExecKind, ContainerExecKind, "grpc": // valid default: errs = append(errs, fmt.Sprintf("%s: invalid readiness kind %q", prefix, svc.Readiness.Kind)) @@ -108,10 +108,10 @@ func (c *Config) Validate() error { if svc.Readiness.Kind == "http" && svc.Readiness.URL == "" { errs = append(errs, fmt.Sprintf("%s: readiness kind http requires a url", prefix)) } - if svc.Readiness.Kind == "exec" && (svc.Readiness.Command == nil || len(svc.Readiness.Command.Parts) == 0) { + if svc.Readiness.Kind == ExecKind && (svc.Readiness.Command == nil || len(svc.Readiness.Command.Parts) == 0) { errs = append(errs, fmt.Sprintf("%s: readiness kind exec requires a command", prefix)) } - if svc.Readiness.Kind == "container_exec" { + if svc.Readiness.Kind == ContainerExecKind { if svc.Readiness.Command == nil || len(svc.Readiness.Command.Parts) == 0 { errs = append(errs, fmt.Sprintf("%s: readiness kind container_exec requires a command", prefix)) } @@ -134,14 +134,32 @@ func (c *Config) Validate() error { if svc.Readiness.Settle.Duration < 0 { errs = append(errs, fmt.Sprintf("%s: readiness settle must be >= 0", prefix)) } + if len(svc.Readiness.Env) > 0 { + errs = append(errs, fmt.Sprintf("%s: readiness env is not supported", prefix)) + } if svc.Setup != nil { - if len(svc.Setup.Command.Parts) == 0 { - errs = append(errs, fmt.Sprintf("%s: setup requires a command", prefix)) + switch svc.Setup.Kind { + case ExecKind: + case ContainerExecKind: + if !svc.IsContainer() { + errs = append(errs, fmt.Sprintf("%s: setup kind container_exec requires a container service", prefix)) + } + if len(svc.Setup.Env) > 0 { + errs = append(errs, fmt.Sprintf("%s: setup env is only supported for kind exec", prefix)) + } + default: + errs = append(errs, fmt.Sprintf("%s: invalid setup kind %q (must be exec or container_exec)", prefix, svc.Setup.Kind)) + } + if svc.Setup.Command == nil || len(svc.Setup.Command.Parts) == 0 { + errs = append(errs, fmt.Sprintf("%s: setup kind %s requires a command", prefix, svc.Setup.Kind)) } if svc.Setup.Timeout.Duration < 0 { errs = append(errs, fmt.Sprintf("%s: setup timeout must be >= 0", prefix)) } + if hasProbeOnlyFields(svc.Setup) { + errs = append(errs, fmt.Sprintf("%s: setup only supports kind, command, timeout, and env", prefix)) + } } } @@ -185,6 +203,17 @@ func (c *Config) Validate() error { return nil } +func hasProbeOnlyFields(cfg *ServiceHookConfig) bool { + return cfg.Pattern != "" || + cfg.Address != "" || + cfg.URL != "" || + cfg.Service != "" || + cfg.InitialDelay.Duration != 0 || + cfg.Interval.Duration != 0 || + cfg.Settle.Duration != 0 || + cfg.MaxAttempts != 0 +} + func (c *Config) checkCycles() error { type color int const ( diff --git a/internal/runner/backend.go b/internal/runner/backend.go index bee43e5..3acaddc 100644 --- a/internal/runner/backend.go +++ b/internal/runner/backend.go @@ -72,7 +72,7 @@ type ContainerBackend interface { RemoveArgs(target string, force bool) []string // ExecArgs builds the args to run a command inside a running container. // Both current backends spell this the same way, but routing it through the - // interface is what lets the container_exec readiness probe stay portable: + // interface keeps container_exec hooks portable: // bench.yml names the command to run, never the CLI that runs it. ExecArgs(id string, cmd []string) []string // WaitExit blocks until the container terminates and returns its exit code. diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 5cfcef4..5c12a96 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -22,8 +22,8 @@ type Runner interface { } // ContainerExecer is implemented by runners that can run a command inside the -// container they manage. The container_exec readiness probe uses it so a probe -// can target a service's own container without bench.yml naming either the +// container they manage. Readiness and setup use it so commands can target a +// service's own container without bench.yml naming either the // container or the backend's CLI. ProcessRunner does not implement it, so a // failed type assertion is how the probe detects a non-container service. type ContainerExecer interface { diff --git a/internal/supervisor/probe.go b/internal/supervisor/probe.go index e68b561..8b93f53 100644 --- a/internal/supervisor/probe.go +++ b/internal/supervisor/probe.go @@ -41,7 +41,7 @@ const ( // // execer supplies the container-exec invocation for the container_exec kind and // is nil for process services; every other kind ignores it. -func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buffer, baselineSeq uint64, execer runner.ContainerExecer) bool { +func runProbe(ctx context.Context, cfg config.ServiceHookConfig, logs *logbuf.Buffer, baselineSeq uint64, execer runner.ContainerExecer) bool { kind := cfg.Kind if kind == "" || kind == "none" { return true @@ -81,34 +81,15 @@ func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buff ok = retryProbe(ctx, cfg.MaxAttempts, interval, func() bool { return probeHTTPOnce(ctx, cfg.URL, perAttempt) }) - case "exec": - if cfg.Command == nil || len(cfg.Command.Parts) == 0 { - if logs != nil { - logs.Add("stderr", "readiness: exec kind requires a command") - } - return false - } - parts := cfg.Command.Parts - ok = retryProbe(ctx, cfg.MaxAttempts, interval, func() bool { - return probeExecOnce(ctx, parts, perAttempt, logs) - }) - case "container_exec": - if cfg.Command == nil || len(cfg.Command.Parts) == 0 { - if logs != nil { - logs.Add("stderr", "readiness: container_exec kind requires a command") - } - return false - } - if execer == nil { + case config.ExecKind, config.ContainerExecKind: + bin, args, err := resolveExec(cfg, execer) + if err != nil { if logs != nil { - logs.Add("stderr", "readiness: container_exec is only valid for a container service") + logs.Add("stderr", "readiness: "+err.Error()) } return false } - // Resolve the invocation once: both the container name and the backend - // CLI are fixed for the life of this probe. - bin, execArgs := execer.ExecCommand(cfg.Command.Parts) - parts := append([]string{bin}, execArgs...) + parts := append([]string{bin}, args...) ok = retryProbe(ctx, cfg.MaxAttempts, interval, func() bool { return probeExecOnce(ctx, parts, perAttempt, logs) }) @@ -133,6 +114,28 @@ func runProbe(ctx context.Context, cfg config.ReadinessConfig, logs *logbuf.Buff return true } +// resolveExec maps a portable command to its host invocation. +func resolveExec(cfg config.ServiceHookConfig, execer runner.ContainerExecer) (string, []string, error) { + if cfg.Command == nil || len(cfg.Command.Parts) == 0 { + return "", nil, fmt.Errorf("%s kind requires a command", cfg.Kind) + } + + switch cfg.Kind { + case config.ExecKind: + return cfg.Command.Parts[0], cfg.Command.Parts[1:], nil + case config.ContainerExecKind: + if execer == nil { + return "", nil, fmt.Errorf("%s is only valid for a container service", cfg.Kind) + } + default: + return "", nil, fmt.Errorf("invalid exec kind %q", cfg.Kind) + } + + bin, args := execer.ExecCommand(cfg.Command.Parts) + + return bin, args, nil +} + // retryProbe repeatedly invokes attempt until it returns true, ctx is // cancelled, or maxAttempts is reached (0 = unlimited). It sleeps `interval` // between attempts. diff --git a/internal/supervisor/probe_test.go b/internal/supervisor/probe_test.go index 7a01e19..ac7cec0 100644 --- a/internal/supervisor/probe_test.go +++ b/internal/supervisor/probe_test.go @@ -17,9 +17,9 @@ import ( "github.com/ccakes/workbench/internal/logbuf" ) -// helper: build a ReadinessConfig with a given kind and useful defaults for tests. -func tcpReadiness(addr string, timeout, initialDelay time.Duration) config.ReadinessConfig { - return config.ReadinessConfig{ +// helper: build a ServiceHookConfig with a given kind and useful defaults for tests. +func tcpReadiness(addr string, timeout, initialDelay time.Duration) config.ServiceHookConfig { + return config.ServiceHookConfig{ Kind: "tcp", Address: addr, Timeout: config.Duration{Duration: timeout}, @@ -27,16 +27,16 @@ func tcpReadiness(addr string, timeout, initialDelay time.Duration) config.Readi } } -func httpReadiness(url string, timeout time.Duration) config.ReadinessConfig { - return config.ReadinessConfig{ +func httpReadiness(url string, timeout time.Duration) config.ServiceHookConfig { + return config.ServiceHookConfig{ Kind: "http", URL: url, Timeout: config.Duration{Duration: timeout}, } } -func logPatternReadiness(pattern string) config.ReadinessConfig { - return config.ReadinessConfig{ +func logPatternReadiness(pattern string) config.ServiceHookConfig { + return config.ServiceHookConfig{ Kind: "log_pattern", Pattern: pattern, } @@ -287,16 +287,16 @@ func TestRunProbe_NoneKindIsInstantReady(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - if !runProbe(ctx, config.ReadinessConfig{Kind: ""}, nil, 0, nil) { + if !runProbe(ctx, config.ServiceHookConfig{Kind: ""}, nil, 0, nil) { t.Error("empty kind should be instant-ready") } - if !runProbe(ctx, config.ReadinessConfig{Kind: "none"}, nil, 0, nil) { + if !runProbe(ctx, config.ServiceHookConfig{Kind: "none"}, nil, 0, nil) { t.Error("'none' kind should be instant-ready") } } func TestProbeExec_ExitZeroReady(t *testing.T) { - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "exec", Command: &config.Command{Parts: []string{"sh", "-c", "exit 0"}}, Timeout: config.Duration{Duration: 2 * time.Second}, @@ -310,7 +310,7 @@ func TestProbeExec_ExitZeroReady(t *testing.T) { func TestProbeExec_StreamsOutputToLogs(t *testing.T) { buf := logbuf.New(50) - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "exec", Command: &config.Command{Parts: []string{"sh", "-c", "echo hello-probe && exit 0"}}, Timeout: config.Duration{Duration: 2 * time.Second}, @@ -336,7 +336,7 @@ func TestProbeExec_StreamsOutputToLogs(t *testing.T) { func TestProbeExec_MaxAttemptsCap(t *testing.T) { // A command that always fails should give up after MaxAttempts and return false. - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "exec", Command: &config.Command{Parts: []string{"sh", "-c", "exit 1"}}, Timeout: config.Duration{Duration: 200 * time.Millisecond}, @@ -358,7 +358,7 @@ func TestProbeExec_MaxAttemptsCap(t *testing.T) { func TestProbeTCP_MaxAttemptsCap(t *testing.T) { // Closed port + MaxAttempts=2 should bail quickly instead of looping // until ctx cancellation. - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "tcp", Address: freeAddr(t), Timeout: config.Duration{Duration: 50 * time.Millisecond}, @@ -379,7 +379,7 @@ func TestProbeSettleDelaysReady(t *testing.T) { } defer func() { _ = l.Close() }() - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "tcp", Address: listenerAddr(l), Timeout: config.Duration{Duration: 200 * time.Millisecond}, @@ -400,7 +400,7 @@ func TestProbeSettleDelaysReady(t *testing.T) { func TestProbeGRPC_Serving(t *testing.T) { addr := startGRPCHealthServer(t, healthpb.HealthCheckResponse_SERVING, "") - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "grpc", Address: addr, Timeout: config.Duration{Duration: 500 * time.Millisecond}, @@ -420,7 +420,7 @@ func TestProbeGRPC_NotServingThenSucceeds(t *testing.T) { time.Sleep(200 * time.Millisecond) grpcHealthFlip(addr, healthpb.HealthCheckResponse_SERVING) }() - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "grpc", Address: addr, Timeout: config.Duration{Duration: 200 * time.Millisecond}, @@ -435,7 +435,7 @@ func TestProbeGRPC_NotServingThenSucceeds(t *testing.T) { func TestProbeGRPC_MaxAttemptsCap(t *testing.T) { addr := startGRPCHealthServer(t, healthpb.HealthCheckResponse_NOT_SERVING, "") - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "grpc", Address: addr, Timeout: config.Duration{Duration: 100 * time.Millisecond}, @@ -458,7 +458,7 @@ func TestProbeSettleCancellable(t *testing.T) { } defer func() { _ = l.Close() }() - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "tcp", Address: listenerAddr(l), Timeout: config.Duration{Duration: 200 * time.Millisecond}, @@ -490,7 +490,7 @@ func TestProbeContainerExec_ForwardsCommandAndSucceeds(t *testing.T) { defer cancel() ex := &stubExecer{bin: "sh", args: []string{"-c", "exit 0"}} - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "container_exec", Command: &config.Command{Parts: []string{"pg_isready", "-U", "bench"}}, Timeout: config.Duration{Duration: 2 * time.Second}, @@ -513,7 +513,7 @@ func TestProbeContainerExec_NonZeroExitFails(t *testing.T) { defer cancel() ex := &stubExecer{bin: "sh", args: []string{"-c", "exit 1"}} - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "container_exec", Command: &config.Command{Parts: []string{"pg_isready"}}, Timeout: config.Duration{Duration: time.Second}, @@ -531,7 +531,7 @@ func TestProbeContainerExec_NilExecerFails(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() - cfg := config.ReadinessConfig{ + cfg := config.ServiceHookConfig{ Kind: "container_exec", Command: &config.Command{Parts: []string{"pg_isready"}}, MaxAttempts: 1, @@ -553,7 +553,7 @@ func TestProbeContainerExec_MissingCommandFails(t *testing.T) { defer cancel() ex := &stubExecer{bin: "true"} - cfg := config.ReadinessConfig{Kind: "container_exec", MaxAttempts: 1} + cfg := config.ServiceHookConfig{Kind: "container_exec", MaxAttempts: 1} if runProbe(ctx, cfg, buf, 0, ex) { t.Fatal("expected container_exec to fail with no command") diff --git a/internal/supervisor/setup.go b/internal/supervisor/setup.go index 0c78ddf..501ca0f 100644 --- a/internal/supervisor/setup.go +++ b/internal/supervisor/setup.go @@ -3,11 +3,13 @@ package supervisor import ( "bufio" "context" - "errors" "fmt" "io" "os/exec" "time" + + "github.com/ccakes/workbench/internal/config" + "github.com/ccakes/workbench/internal/runner" ) const setupDefaultTimeout = 60 * time.Second @@ -15,16 +17,20 @@ const setupDefaultTimeout = 60 * time.Second // runSetupHook executes the configured setup command for a service after its // readiness probe has passed. The hook's stdout/stderr are appended to the // service's log buffer with a `setup` stream tag so the user can see what -// happened. Setup-hook env layers on top of the service's resolved env: +// happened. Host-exec env layers on top of the service's resolved env: // process env -> global env_file -> global env -> service env_file -> service -// env -> setup env. The hook runs in the service's working directory. +// env -> setup env. Container exec inherits the running container's env. // // Returns nil on exit 0. Returns a non-nil error on non-zero exit, timeout, // context cancellation, or if the env or working dir can't be resolved. -func (s *Supervisor) runSetupHook(ctx context.Context, ms *managedService) error { +func (s *Supervisor) runSetupHook(ctx context.Context, ms *managedService, execer runner.ContainerExecer) error { cfg := ms.cfg.Setup - if cfg == nil || len(cfg.Command.Parts) == 0 { - return errors.New("missing command") + if cfg == nil { + return fmt.Errorf("missing config") + } + bin, args, err := resolveExec(*cfg, execer) + if err != nil { + return err } timeout := cfg.Timeout.Duration @@ -34,18 +40,18 @@ func (s *Supervisor) runSetupHook(ctx context.Context, ms *managedService) error cmdCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - env, err := s.buildEnv(ms) - if err != nil { - return fmt.Errorf("building env: %w", err) - } - for k, v := range cfg.Env { - env = append(env, k+"="+v) - } - - parts := cfg.Command.Parts - cmd := exec.CommandContext(cmdCtx, parts[0], parts[1:]...) + cmd := exec.CommandContext(cmdCtx, bin, args...) cmd.Dir = ms.cfg.Dir - cmd.Env = env + if cfg.Kind == config.ExecKind { + env, err := s.buildEnv(ms) + if err != nil { + return fmt.Errorf("building env: %w", err) + } + for k, v := range cfg.Env { + env = append(env, k+"="+v) + } + cmd.Env = env + } outR, outW := io.Pipe() errR, errW := io.Pipe() diff --git a/internal/supervisor/setup_test.go b/internal/supervisor/setup_test.go new file mode 100644 index 0000000..7146402 --- /dev/null +++ b/internal/supervisor/setup_test.go @@ -0,0 +1,37 @@ +package supervisor + +import ( + "context" + "reflect" + "testing" + "time" + + "github.com/ccakes/workbench/internal/config" + "github.com/ccakes/workbench/internal/logbuf" +) + +func TestSetupContainerExec(t *testing.T) { + want := []string{"psql", "-c", "select 1"} + setup := &config.ServiceHookConfig{ + Kind: config.ContainerExecKind, + Command: &config.Command{Parts: want}, + Timeout: config.Duration{Duration: time.Second}, + } + svc := config.ServiceConfig{ + Container: &config.ContainerConfig{Image: "postgres:16"}, + Setup: setup, + } + ms := &managedService{cfg: svc, logs: logbuf.New(10)} + sup := &Supervisor{cfg: &config.Config{}} + execer := &stubExecer{bin: "sh", args: []string{"-c", "echo setup-complete"}} + + if err := sup.runSetupHook(context.Background(), ms, execer); err != nil { + t.Fatalf("runSetupHook: %v", err) + } + if !reflect.DeepEqual(execer.got, want) { + t.Fatalf("ExecCommand got %v, want %v", execer.got, want) + } + if !logContains(ms.logs, "setup-complete") { + t.Fatal("missing setup output") + } +} diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index c66458b..c361b5f 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -398,7 +398,7 @@ func (s *Supervisor) runLoop(ms *managedService) { if last := ms.logs.Last(1); len(last) == 1 { baseline = last[0].Seq } - // Resolved before the goroutine starts so the probe never races the + // Resolved before the goroutine starts so startup hooks never race the // runLoop for ms.r. Process runners don't implement ContainerExecer, // which leaves execer nil and makes container_exec fail with a clear // message rather than silently probing nothing. @@ -418,7 +418,7 @@ func (s *Supervisor) runLoop(ms *managedService) { } if ms.cfg.Setup != nil { s.setStatus(ms, service.StatusSetup, "running setup hook") - if err := s.runSetupHook(probeCtx, ms); err != nil { + if err := s.runSetupHook(probeCtx, ms, execer); err != nil { // Record the failure on the managed service. The runLoop's // stop path sees startupErr and finalises as Failed (not // Stopped) so the user can tell apart "I stopped this" @@ -530,7 +530,7 @@ func (s *Supervisor) runLoop(ms *managedService) { } } -func readinessFailureReason(cfg config.ReadinessConfig) string { +func readinessFailureReason(cfg config.ServiceHookConfig) string { if cfg.MaxAttempts > 0 { return fmt.Sprintf("readiness probe failed after %d attempts", cfg.MaxAttempts) } diff --git a/internal/supervisor/supervisor_test.go b/internal/supervisor/supervisor_test.go index 2a15638..95c7f6e 100644 --- a/internal/supervisor/supervisor_test.go +++ b/internal/supervisor/supervisor_test.go @@ -764,7 +764,7 @@ func TestReadiness_TCPReachesReady(t *testing.T) { l := openTCPListener(t) svc := longRunningSvc(dir) - svc.Readiness = config.ReadinessConfig{ + svc.Readiness = config.ServiceHookConfig{ Kind: "tcp", Address: l.Addr().String(), Timeout: config.Duration{Duration: 500 * time.Millisecond}, @@ -807,7 +807,7 @@ func TestReadiness_DependentWaitsForReady(t *testing.T) { addr := reservedAddr(t) depSvc := longRunningSvc(dir) - depSvc.Readiness = config.ReadinessConfig{ + depSvc.Readiness = config.ServiceHookConfig{ Kind: "tcp", Address: addr, Timeout: config.Duration{Duration: 200 * time.Millisecond}, @@ -933,7 +933,7 @@ func TestReadiness_ProbeGoroutineExitsOnStop(t *testing.T) { dir := t.TempDir() svc := longRunningSvc(dir) - svc.Readiness = config.ReadinessConfig{ + svc.Readiness = config.ServiceHookConfig{ Kind: "tcp", Address: reservedAddr(t), // never-listening Timeout: config.Duration{Duration: 100 * time.Millisecond}, @@ -986,7 +986,7 @@ func TestReadiness_MaxAttemptsMarksServiceFailed(t *testing.T) { dir := t.TempDir() svc := longRunningSvc(dir) - svc.Readiness = config.ReadinessConfig{ + svc.Readiness = config.ServiceHookConfig{ Kind: "tcp", Address: reservedAddr(t), Timeout: config.Duration{Duration: 50 * time.Millisecond}, @@ -1202,8 +1202,9 @@ func TestSetupHook_RunsBetweenProbeAndReady(t *testing.T) { marker := dir + "/setup-marker" svc := longRunningSvc(dir) - svc.Setup = &config.SetupConfig{ - Command: config.Command{Shell: true, Parts: []string{"sh", "-c", "touch " + marker}}, + svc.Setup = &config.ServiceHookConfig{ + Kind: config.ExecKind, + Command: &config.Command{Shell: true, Parts: []string{"sh", "-c", "touch " + marker}}, Timeout: config.Duration{Duration: 5 * time.Second}, } @@ -1242,8 +1243,9 @@ func TestSetupHook_FailureMarksServiceFailed(t *testing.T) { dir := t.TempDir() svc := longRunningSvc(dir) - svc.Setup = &config.SetupConfig{ - Command: config.Command{Shell: true, Parts: []string{"sh", "-c", "echo setup-bad >&2; exit 7"}}, + svc.Setup = &config.ServiceHookConfig{ + Kind: config.ExecKind, + Command: &config.Command{Shell: true, Parts: []string{"sh", "-c", "echo setup-bad >&2; exit 7"}}, Timeout: config.Duration{Duration: 5 * time.Second}, } From a78d169273901f617f7420905eb8a50660bb2397 Mon Sep 17 00:00:00 2001 From: Cameron Daniel Date: Wed, 26 Aug 2026 08:54:50 +1000 Subject: [PATCH 5/5] Use Docker as default backend Automatic selection can change existing projects when Apple Container is installed. Keep it opt-in so omitted configuration preserves Docker behavior. --- CHANGELOG.md | 23 +++++++++++++++++++++++ docs/apple-container.md | 8 ++++---- docs/configuration.md | 8 ++++---- internal/cli/skill/SKILL.md | 2 +- internal/config/config.go | 6 +++--- internal/config/config_test.go | 4 ++-- internal/config/validate.go | 2 +- internal/runner/backend.go | 14 ++++++-------- internal/runner/backend_test.go | 3 +++ 9 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d74edc..cd43e98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,29 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- Apple `container` is now supported as an alternative container backend on + Apple silicon (macOS 26+). Select it with the new global `container_backend` + setting (`docker`, `apple`, or `auto`). Docker remains the default; Apple and + automatic selection are opt-in. Container services run unchanged on either + backend. The TUI and `bench status` show the active backend. See + `docs/apple-container.md`. +- Container images with no usable variant for the host architecture (e.g. an + amd64-only image on Apple silicon) now fail terminally with a clear message + instead of looping through the restart policy. +- New readiness kind `container_exec` runs a command inside the service's own + container. Workbench supplies the container name and the backend's CLI, so + `command: pg_isready -U bench` works unchanged on either container backend — + unlike `kind: exec` with a hand-written `docker exec …`, which breaks + when the backend resolves to Apple `container` or `container_prefix` changes. + See `docs/configuration.md`. +- Setup hooks now support `kind: exec` and `kind: container_exec`, using the + same configuration as readiness hooks. Existing command-only setup hooks + remain host-side `exec` hooks and emit a deprecation warning. + ## [0.6.10] - 2026-07-07 ### Fixed diff --git a/docs/apple-container.md b/docs/apple-container.md index aee734f..42228c8 100644 --- a/docs/apple-container.md +++ b/docs/apple-container.md @@ -23,16 +23,16 @@ on either backend. Only the global `container_backend` setting differs. ```yaml global: - container_backend: auto # docker | apple | auto (default) + container_backend: apple # docker (default) | apple | auto apple: gateway_ip: 192.168.64.1 ``` -- **`auto`** (default) — use Apple `container` when running on Apple silicon with - the `container` binary installed; otherwise use Docker. -- **`docker`** — always use Docker. +- **`docker`** (default) — always use Docker. - **`apple`** — always use Apple `container`. Startup fails with a clear message if the host doesn't meet the requirements above. +- **`auto`** — use Apple `container` when running on Apple silicon with the + `container` binary installed; otherwise use Docker. The active backend is shown per container service in the TUI detail pane and in `bench status` (the `TYPE` column reads `container/apple` or `container/docker`, diff --git a/docs/configuration.md b/docs/configuration.md index 1f7b06c..9e2ef6a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -91,7 +91,7 @@ Run `bench validate` to surface these errors without starting any services. | `env` | map | | Global environment variables applied to all services | | `env_file` | path | | Global .env file loaded for all services | | `container_prefix` | string | dirname | Prefix for container names (e.g. `{prefix}-{service}`) | -| `container_backend`| string | `auto` | Container runtime: `docker`, `apple`, or `auto` | +| `container_backend`| string | `docker`| Container runtime: `docker`, `apple`, or `auto` | | `apple` | object | | Apple `container` backend settings | | `tracing` | object | | Tracing configuration | @@ -102,11 +102,11 @@ on [Apple's `container`](apple-container.md) tool instead. | Field | Type | Default | Description | | ------------------- | ------ | ------- | -------------------------------------------------------- | -| `container_backend` | string | `auto` | `docker`, `apple`, or `auto` (prefer Apple when present) | +| `container_backend` | string | `docker`| `docker`, `apple`, or `auto` (prefer Apple when present) | | `apple.gateway_ip` | string | `192.168.64.1` | Host IP an Apple container uses to reach the host | -`auto` selects the Apple backend when running on Apple silicon with the -`container` binary installed, otherwise Docker. See +`auto` is opt-in. It selects the Apple backend when running on Apple silicon +with the `container` binary installed, otherwise Docker. See [apple-container.md](apple-container.md) for requirements and caveats. #### Tracing diff --git a/internal/cli/skill/SKILL.md b/internal/cli/skill/SKILL.md index 379a9f7..6543e28 100644 --- a/internal/cli/skill/SKILL.md +++ b/internal/cli/skill/SKILL.md @@ -110,7 +110,7 @@ common stuff, not an exhaustive reference. ## Notes worth knowing - Services are either processes (`command:`) or containers (`container:`); - containers need Docker running. + containers use Docker by default, with Apple and `auto` available by config. - Status flow: `pending → starting → running → [setup →] ready`. The optional `setup` step runs a host `exec` or service `container_exec` bootstrap command after the readiness probe passes; dependents wait for `ready`. diff --git a/internal/config/config.go b/internal/config/config.go index d4158e4..949e0f8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -35,8 +35,8 @@ type GlobalConfig struct { EnvFile string `yaml:"env_file"` ContainerPrefix string `yaml:"container_prefix"` // ContainerBackend selects the runtime for container services: - // "docker", "apple", or "auto" (default). "auto" prefers Apple's - // `container` on Apple silicon when installed, otherwise Docker. + // "docker" (default), "apple", or "auto". "auto" prefers Apple's + // `container` on Apple silicon when installed. ContainerBackend string `yaml:"container_backend"` Apple AppleConfig `yaml:"apple"` Tracing TracingConfig `yaml:"tracing"` @@ -703,7 +703,7 @@ func (c *Config) applyDefaults() { c.Global.Tracing.BufferSize = ByteSize(500 * 1024 * 1024) } if c.Global.ContainerBackend == "" { - c.Global.ContainerBackend = BackendAuto + c.Global.ContainerBackend = BackendDocker } if c.Global.Apple.GatewayIP == "" { c.Global.Apple.GatewayIP = "192.168.64.1" diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5997cf3..78a6cd7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -70,8 +70,8 @@ services: } // Container backend defaults - if cfg.Global.ContainerBackend != BackendAuto { - t.Errorf("container_backend = %q, want %q", cfg.Global.ContainerBackend, BackendAuto) + if cfg.Global.ContainerBackend != BackendDocker { + t.Errorf("container_backend = %q, want %q", cfg.Global.ContainerBackend, BackendDocker) } if cfg.Global.Apple.GatewayIP != "192.168.64.1" { t.Errorf("apple.gateway_ip = %q, want 192.168.64.1", cfg.Global.Apple.GatewayIP) diff --git a/internal/config/validate.go b/internal/config/validate.go index 37f09d0..96ffca3 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -173,7 +173,7 @@ func (c *Config) Validate() error { switch c.Global.ContainerBackend { case "", BackendDocker, BackendApple, BackendAuto: - // valid (empty is defaulted to auto) + // valid (empty is defaulted to Docker) default: errs = append(errs, fmt.Sprintf("invalid container_backend %q (must be %q, %q, or %q)", c.Global.ContainerBackend, BackendDocker, BackendApple, BackendAuto)) } diff --git a/internal/runner/backend.go b/internal/runner/backend.go index 3acaddc..8a46865 100644 --- a/internal/runner/backend.go +++ b/internal/runner/backend.go @@ -101,22 +101,20 @@ type RunSpec struct { const containerPollInterval = 250 * time.Millisecond // ResolveBackend selects the container backend from global config. -// config.BackendDocker and config.BackendApple are explicit; config.BackendAuto -// (the default) prefers Apple's `container` on Apple silicon when the binary is -// installed, otherwise Docker. Selection is pure and side-effect-free — the -// environment/daemon health check happens later in Available(). +// Docker is the default. config.BackendAuto prefers Apple's `container` on +// Apple silicon when the binary is installed. Selection is pure and +// side-effect-free; the daemon health check happens later in Available(). func ResolveBackend(g config.GlobalConfig) ContainerBackend { switch g.ContainerBackend { - case config.BackendDocker: - return dockerBackend{} case config.BackendApple: return newAppleBackend(g) - default: // BackendAuto or unset + case config.BackendAuto: if isAppleSilicon() && appleContainerInstalled() { return newAppleBackend(g) } - return dockerBackend{} } + + return dockerBackend{} } // buildRunArgs assembles a detached-run argument list shared by both backends. diff --git a/internal/runner/backend_test.go b/internal/runner/backend_test.go index 858ae66..14d27b3 100644 --- a/internal/runner/backend_test.go +++ b/internal/runner/backend_test.go @@ -122,6 +122,9 @@ func TestBackends_OTELHost(t *testing.T) { } func TestResolveBackend_Explicit(t *testing.T) { + if got := ResolveBackend(config.GlobalConfig{}).Name(); got != "docker" { + t.Errorf("default => %q", got) + } if got := ResolveBackend(config.GlobalConfig{ContainerBackend: config.BackendDocker}).Name(); got != "docker" { t.Errorf("docker => %q", got) }