diff --git a/.dap/review/engineering.md b/.dap/review/engineering.md index 02846a1..04541d6 100644 --- a/.dap/review/engineering.md +++ b/.dap/review/engineering.md @@ -51,9 +51,10 @@ Refines `D1`. This repository implements the same behaviour more than once by de interface. Shared orchestration (engine, compose) must reach it through that interface; a diff that leaks Docker-specific behaviour into shared code is a finding, because the interface is what keeps a second backend possible. -- A capability flag on `Capabilities()` (`ServiceNameDNS`, for instance) is the - legitimate way to encode divergence. A silent assumption that all backends behave like - Docker is not. +- A backend that cannot satisfy a compose feature must be refused explicitly — at plan + time, or at the gate that needs it. A silent assumption that all backends behave like + Docker is not the legitimate way to encode divergence, and neither is silently + degrading to a weaker guarantee than the compose condition asks for. ## R3. Destructive recreate diff --git a/CHANGELOG.md b/CHANGELOG.md index ec311ac..bba9aef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +- **BREAKING — `runtime.Capabilities` is cut from six fields to three.** The struct + described where a backend diverged from Docker; every field's only `false` case was + the Apple backend and `runtime/docker` reported all of them `true`, so most of what + it gated was unreachable. What survives is the three behaviours whose absence the + orchestrator cannot fully detect at the point of use: `Healthchecks` (a service may + inherit its healthcheck from the image, which the compose file cannot see), + `ExitCodes` (a backend that reports no exit code reports zero, which is + indistinguishable from a clean exit) and `ServiceNameDNS` (broken name resolution + surfaces inside the container, never at `Up` time). Removed fields: + `NamespaceSharing` and `SharedVolumes`, where a backend that cannot + honour the request fails loudly from the primitive itself, so the plan-time refusal + was UX rather than correctness; and `RestartPolicies`, which gated a + `WarnRestartPolicyIgnoredOnBackend` event that was never implemented. +- **BREAKING — `compose.Plan.Validate(caps runtime.Capabilities)`** drops its + `backendName` parameter, and `Orchestrator.BackendName` and the `NewOrchestrator` + parameter that set it are gone: the Engine passed `""` at all three call sites and + the field's only reader was an error message, so it never carried information. + `Validate` refuses the two backend-gated `depends_on` conditions — + `service_healthy` without `Healthchecks` and `service_completed_successfully` + without `ExitCodes`. `compose.UnsupportedFeatureOnBackendError` loses its `Backend` field + and `compose.VolumeSharedAcrossServicesError` is removed with the refusal that + produced it. The `/etc/hosts` post-start patch behind `ServiceNameDNS` is unchanged + and still runs on a backend that reports no service-name DNS. +- **BREAKING — three `runtime` error types with no remaining producer are removed:** + `runtime.BuilderUnavailableError` and `runtime.UnsupportedOptionError` (constructed + only by the Apple backend) and `runtime.ExecFailedError`, which has had no producer + since well before the backend removals. - **BREAKING — the Apple Containers backend is removed.** `runtime/applecontainer` and the `applecontainer-bridge` Swift package (reached through a cgo shim) are deleted, along with the `--runtime applecontainer` CLI value: `--runtime` now @@ -58,6 +85,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `design/compose-native-health.md`; all three remain readable in git history at tag `v0.4.3`. +### Changed + +- **compose (native)** — the `service_healthy` gate no longer passes when the backend + reports no health status for a service that declares an explicit healthcheck test. + `runtime.HealthStatus` documents `HealthNone` as ambiguous — the image declared no + `HEALTHCHECK`, or the backend does not surface health at all — and the gate reads it + as "no healthcheck" so healthcheck-less projects still come up. That reading is + wrong when the service names a real test command itself. `Orchestrator.waitFor` + keeps polling and fails with an explicit error rather than starting dependents + before the check ever succeeded. This is a second line of defence, not a + replacement for `Capabilities.Healthchecks`: the capability refuses a backend that + honestly reports it cannot do healthchecks, while the gate also catches one that + claims the capability and then reports nothing. Docker is unaffected — it reports + `starting` / `healthy` / `unhealthy` for any container with a healthcheck. + Only an explicit, non-`NONE` test command counts as declaring one: `disable: true`, + compose's inline `test: ["NONE"]`, and an empty test (where the image's own + `HEALTHCHECK` applies) all keep the permissive fallback, so no valid project blocks + on its own gate. + ## [0.4.2] - 2026-08-23 ### Fixed diff --git a/compose/errors.go b/compose/errors.go index d4f9c18..50b3797 100644 --- a/compose/errors.go +++ b/compose/errors.go @@ -62,49 +62,6 @@ func sortFields(in []UnsupportedField) []UnsupportedField { return out } -// UnsupportedFeatureOnBackendError is returned by Plan.Validate when -// the project uses a compose feature the active backend cannot -// satisfy — e.g. depends_on.condition: service_healthy against a -// backend whose Capabilities().Healthchecks is false. -// -// Distinct from UnsupportedFieldError (which lists fields we never -// implement) because the gating is backend-specific and may flip if -// the backend gains the capability later. -type UnsupportedFeatureOnBackendError struct { - Backend string // backend display name (e.g. "docker") - Capability string // Capabilities struct field name (e.g. "Healthchecks") - Service string // service that triggered the refusal - Detail string // one-sentence explanation -} - -func (e *UnsupportedFeatureOnBackendError) Error() string { - if e.Service != "" { - return fmt.Sprintf( - "compose: service %q uses %s, which the %s backend does not support: %s", - e.Service, e.Capability, e.Backend, e.Detail, - ) - } - return fmt.Sprintf( - "compose: project uses %s, which the %s backend does not support: %s", - e.Capability, e.Backend, e.Detail, - ) -} - -// VolumeSharedAcrossServicesError is returned by Plan.Validate when -// the project mounts a single named volume into 2+ services and the -// active backend's Capabilities().SharedVolumes is false. -type VolumeSharedAcrossServicesError struct { - Volume string - Services []string // sorted -} - -func (e *VolumeSharedAcrossServicesError) Error() string { - return fmt.Sprintf( - "compose: volume %q is mounted into %d services (%s); the active backend does not allow shared volumes", - e.Volume, len(e.Services), strings.Join(e.Services, ", "), - ) -} - // PartialUpError signals that Up brought some services online and // then failed before completing. Returned with the names of the // services that did and didn't start so the caller (Engine.Up) can @@ -156,3 +113,29 @@ type CycleError struct { func (e *CycleError) Error() string { return fmt.Sprintf("compose: depends_on cycle: %s", strings.Join(e.Cycle, " -> ")) } + +// UnsupportedFeatureOnBackendError is returned by Plan.Validate when +// the project uses a compose feature the active backend cannot +// satisfy, as advertised by runtime.Capabilities. +// +// Distinct from UnsupportedFieldError (which lists fields we never +// implement) because the gating is backend-specific and may flip if +// the backend gains the capability later. +type UnsupportedFeatureOnBackendError struct { + Capability string // Capabilities struct field name (e.g. "ExitCodes") + Service string // service that triggered the refusal + Detail string // one-sentence explanation +} + +func (e *UnsupportedFeatureOnBackendError) Error() string { + if e.Service != "" { + return fmt.Sprintf( + "compose: service %q uses %s, which the active backend does not support: %s", + e.Service, e.Capability, e.Detail, + ) + } + return fmt.Sprintf( + "compose: project uses %s, which the active backend does not support: %s", + e.Capability, e.Detail, + ) +} diff --git a/compose/graph.go b/compose/graph.go index cb551dd..ba0c15a 100644 --- a/compose/graph.go +++ b/compose/graph.go @@ -133,14 +133,6 @@ func findCycle(deps map[string]map[string]struct{}, remaining map[string]struct{ } } -// isServiceNetworkMode reports whether the value of `network_mode:` -// references another service's namespace (`service:`). The -// orchestrator surfaces the dep edge here so topo-sort respects the -// ordering even though compose-go doesn't model it under DependsOn. -func isServiceNetworkMode(nm string) bool { - return serviceRefTarget(nm) != "" -} - // serviceRefTarget returns the service name a `service:` // namespace-mode value points at, or "" when the value is anything // else (empty, "host", "none", "container:", ...). diff --git a/compose/orchestrator.go b/compose/orchestrator.go index 7c4dcc6..9648be8 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -15,9 +15,8 @@ // - Down: list by project label -> stop + remove containers -> // remove network -> optionally remove volumes / images. // - service_healthy / service_completed_successfully gating: the -// polling loop is in place but only reads InspectContainer -// fields the runtime already exposes; once Apple gains health -// and exit-code surfacing the orchestrator code does not change. +// polling loop reads InspectContainer fields the runtime +// already exposes. // // Out of scope here, picked up in later PRs: // - Port bindings (RunSpec doesn't carry them yet). @@ -71,10 +70,6 @@ const DefaultHealthTimeout = 60 * time.Second type Orchestrator struct { rt runtime.Runtime - // BackendName identifies the backend in error messages. Empty - // is allowed but reduces error-message clarity. - BackendName string - // HealthTimeout overrides DefaultHealthTimeout. Applied per // depends_on edge, not for the whole Up. HealthTimeout time.Duration @@ -85,10 +80,9 @@ type Orchestrator struct { } // NewOrchestrator constructs an Orchestrator with sane defaults. -func NewOrchestrator(rt runtime.Runtime, backendName string) *Orchestrator { +func NewOrchestrator(rt runtime.Runtime) *Orchestrator { return &Orchestrator{ rt: rt, - BackendName: backendName, HealthTimeout: DefaultHealthTimeout, PollInterval: 500 * time.Millisecond, } @@ -112,7 +106,7 @@ type UpResult struct { // already started; the already-running services are NOT torn down // (debuggability matters more than tidiness — see design §5.3). func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { - if err := plan.Validate(o.BackendName, o.rt.Capabilities()); err != nil { + if err := plan.Validate(o.rt.Capabilities()); err != nil { return UpResult{}, err } @@ -217,11 +211,10 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { return res, err } - // Backends without service-name DNS (apple) need a manual - // /etc/hosts patch in every running container with the - // service→IP map known so far. Docker has built-in DNS - // aliases on the project network — this is a no-op there - // because Capabilities().ServiceNameDNS is true. + // Backends without service-name DNS need a manual /etc/hosts + // patch in every running container with the service→IP map + // known so far. Docker has built-in DNS aliases on the project + // network, so this is a no-op there. if !o.rt.Capabilities().ServiceNameDNS { if err := o.patchHostsFiles(ctx, plan, res.ContainerIDs); err != nil { return res, err @@ -232,119 +225,6 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { return res, nil } -// patchHostsFiles appends the project's service→IP map to /etc/hosts -// of every running container in res.ContainerIDs. Used on backends -// like apple/container 0.12.x where the project network has no -// built-in service-name DNS resolution (probe 3 in -// design/compose-native.md). Issues are best-effort: a service that -// already has the entries (re-runs of Up on an unchanged project) -// is fine because the patch is append-only with a sentinel marker -// that we check for to avoid duplicate lines. -func (o *Orchestrator) patchHostsFiles( - ctx context.Context, plan *Plan, containerIDs map[string]string, -) error { - // Build the service → IP map by inspecting each running - // container. Apple's inspect surfaces the network IP under - // ContainerDetails.Labels via the dev.containers.network-ip - // key — we read it through generic Inspect output rather than - // adding a typed field, keeping the runtime.Runtime surface - // stable. If the backend doesn't expose the IP at all, we - // skip silently and rely on lazy-DNS in the container's - // userland (most app code resolves on first request). - ips := map[string]string{} - for svc, id := range containerIDs { - ip, err := o.containerIP(ctx, id) - if err != nil || ip == "" { - continue - } - ips[svc] = ip - } - if len(ips) == 0 { - return nil - } - - hostsBlock := renderHostsBlock(ips) - for _, id := range containerIDs { - // Best-effort: hosts patching failure should not fail the - // whole Up (the user might still get working resolution - // via lazy DNS retries). Log via the orchestrator's - // future event channel; today we swallow. - _ = o.appendHostsBlock(ctx, id, hostsBlock) - } - return nil -} - -// containerIP reads the network IP a backend assigned to the -// given container. Apple's inspect emits ipv4Address strings in the -// form "192.168.66.2/24" under networks[].ipv4Address; we don't -// surface that as a typed field on runtime.ContainerDetails yet, -// so this is a string-parse over a side channel. -// -// On backends with built-in DNS (docker, ServiceNameDNS=true) the -// orchestrator never calls this — the hosts-patch path is gated. -func (o *Orchestrator) containerIP(ctx context.Context, id string) (string, error) { - d, err := o.rt.InspectContainer(ctx, id) - if err != nil || d == nil { - return "", err - } - // Backends report the IP via the labels map under a documented - // key when they can't widen ContainerDetails. Empty = "no IP - // surfaced" — caller skips the entry. - if ip := d.Labels["dev.containers.network-ip"]; ip != "" { - return ip, nil - } - return "", nil -} - -// renderHostsBlock formats a service→IP map into the block we -// append to /etc/hosts. Includes a sentinel comment so re-runs of -// Up can detect "already patched" by grepping for the marker. -func renderHostsBlock(ips map[string]string) string { - names := make([]string, 0, len(ips)) - for n := range ips { - names = append(names, n) - } - sort.Strings(names) - var b []byte - b = append(b, "# devcontainer-go compose hosts patch\n"...) - for _, n := range names { - b = append(b, ips[n]...) - b = append(b, '\t') - b = append(b, n...) - b = append(b, '\n') - } - return string(b) -} - -// appendHostsBlock runs as root inside the container and appends -// the given block to /etc/hosts. Idempotent via a sentinel-marker -// grep: if the marker is already present, the existing block is -// replaced with the new one (covers Up-on-changed-project), then -// the block is appended. Uses busybox-friendly sh syntax so it -// works on alpine + debian-slim equally. -func (o *Orchestrator) appendHostsBlock(ctx context.Context, id, block string) error { - const marker = "# devcontainer-go compose hosts patch" - script := fmt.Sprintf( - // 1) Strip any prior block (lines from marker to next blank - // or EOF). Uses sed with start-of-marker pattern. - // 2) Append the new block. - `set -e -if grep -qF %q /etc/hosts 2>/dev/null; then - sed -i.bak '/^%s$/,/^$/d' /etc/hosts || true - rm -f /etc/hosts.bak -fi -cat >> /etc/hosts <<'EOF' -%sEOF -`, - marker, marker, block, - ) - _, err := o.rt.ExecContainer(ctx, id, runtime.ExecOptions{ - Cmd: []string{"sh", "-c", script}, - User: "0", - }) - return err -} - // Down tears down a project. Idempotent: missing resources are // no-ops; missing project leaves no observable state change. func (o *Orchestrator) Down(ctx context.Context, plan *DownPlan) error { @@ -634,7 +514,14 @@ func (o *Orchestrator) gateLevel( if cid == "" { continue } - if err := o.waitFor(ctx, svcName, cid, req.condition, deadline); err != nil { + // Whether the service itself declares an active healthcheck. + // Distinguishes "no healthcheck to report on" from "backend does + // not surface health" below. + declaresHealthcheck := false + if cfg, ok := plan.Project.Services[svcName]; ok { + declaresHealthcheck = declaresActiveHealthcheck(cfg.HealthCheck) + } + if err := o.waitFor(ctx, svcName, cid, req.condition, declaresHealthcheck, deadline); err != nil { if req.optional { // Per compose spec: a non-required dependency that // fails to satisfy its condition does not block the @@ -652,8 +539,10 @@ func (o *Orchestrator) gateLevel( // conditions read container state from the backend's inspect; no native // healthcheck is required either way. func (o *Orchestrator) waitFor( - ctx context.Context, svc, id, cond string, deadline time.Time, + ctx context.Context, svc, id, cond string, + declaresHealthcheck bool, deadline time.Time, ) error { + healthUnreported := false for { if err := ctx.Err(); err != nil { return err @@ -672,6 +561,19 @@ func (o *Orchestrator) waitFor( case runtime.HealthHealthy: return nil case runtime.HealthNone: + // HealthNone is ambiguous per + // runtime.HealthStatus: either the image + // declared no HEALTHCHECK, or the backend + // does not surface health at all. When the + // service declares one itself, "no status" + // cannot mean "no healthcheck" — passing the + // gate here would start dependents before the + // check ever succeeded. Keep waiting and + // report it at the deadline instead. + if declaresHealthcheck { + healthUnreported = details.State == runtime.StateRunning + break + } if details.State == runtime.StateRunning { return nil } @@ -694,6 +596,12 @@ func (o *Orchestrator) waitFor( } } if time.Now().After(deadline) { + if healthUnreported { + return fmt.Errorf( + "compose: service %q declares a healthcheck but the backend never reported a health status; service_healthy cannot be honored on this backend", + svc, + ) + } return &HealthTimeoutError{ Service: svc, Condition: cond, @@ -1033,3 +941,139 @@ func serviceLabelOf(c runtime.Container) string { } return c.Name } + +// declaresActiveHealthcheck reports whether a compose service declares +// a healthcheck that the backend is expected to produce a status for. +// Only an explicit, non-disabled test command counts: +// +// - nil / `disable: true` — no healthcheck. +// - `test: ["NONE"]` — compose's inline way to disable one. +// compose-go's validator accepts NONE verbatim rather than folding +// it into Disable, and runtime/docker forwards it to docker's NONE +// sentinel, so the container reports HealthNone by design. +// - an empty test — the image's own HEALTHCHECK (if any) applies. We +// cannot tell from the compose file whether one exists, so this +// stays permissive. +// +// Only when the service names a real test command does HealthNone +// unambiguously mean "the backend did not report", which is what +// waitFor's service_healthy gate keys off. +func declaresActiveHealthcheck(hc *composetypes.HealthCheckConfig) bool { + if hc == nil || hc.Disable { + return false + } + if len(hc.Test) == 0 { + return false + } + return hc.Test[0] != "NONE" +} + +// patchHostsFiles appends the project's service→IP map to /etc/hosts +// of every running container in res.ContainerIDs. Used on backends +// whose project network has no built-in service-name DNS resolution +// (Capabilities.ServiceNameDNS false). Issues are best-effort: a +// service that +// already has the entries (re-runs of Up on an unchanged project) +// is fine because the patch is append-only with a sentinel marker +// that we check for to avoid duplicate lines. +func (o *Orchestrator) patchHostsFiles( + ctx context.Context, plan *Plan, containerIDs map[string]string, +) error { + // Build the service → IP map by inspecting each running + // container. The IP is read out of generic Inspect output rather + // than a typed field, keeping the runtime.Runtime surface stable. + // If the backend doesn't expose the IP at all we skip silently and + // rely on lazy-DNS in the container's userland (most app code + // resolves on first request). + ips := map[string]string{} + for svc, id := range containerIDs { + ip, err := o.containerIP(ctx, id) + if err != nil || ip == "" { + continue + } + ips[svc] = ip + } + if len(ips) == 0 { + return nil + } + + hostsBlock := renderHostsBlock(ips) + for _, id := range containerIDs { + // Best-effort: hosts patching failure should not fail the + // whole Up (the user might still get working resolution + // via lazy DNS retries). Log via the orchestrator's + // future event channel; today we swallow. + _ = o.appendHostsBlock(ctx, id, hostsBlock) + } + return nil +} + +// containerIP reads the network IP a backend assigned to the given +// container. Backends may report it as a bare address or in CIDR form +// ("192.168.66.2/24"); runtime.ContainerDetails has no typed field for +// it, so this is a string-parse over a side channel. +// +// On backends with built-in DNS (docker, ServiceNameDNS=true) the +// orchestrator never calls this — the hosts-patch path is gated. +func (o *Orchestrator) containerIP(ctx context.Context, id string) (string, error) { + d, err := o.rt.InspectContainer(ctx, id) + if err != nil || d == nil { + return "", err + } + // Backends report the IP via the labels map under a documented + // key when they can't widen ContainerDetails. Empty = "no IP + // surfaced" — caller skips the entry. + if ip := d.Labels["dev.containers.network-ip"]; ip != "" { + return ip, nil + } + return "", nil +} + +// renderHostsBlock formats a service→IP map into the block we +// append to /etc/hosts. Includes a sentinel comment so re-runs of +// Up can detect "already patched" by grepping for the marker. +func renderHostsBlock(ips map[string]string) string { + names := make([]string, 0, len(ips)) + for n := range ips { + names = append(names, n) + } + sort.Strings(names) + var b []byte + b = append(b, "# devcontainer-go compose hosts patch\n"...) + for _, n := range names { + b = append(b, ips[n]...) + b = append(b, '\t') + b = append(b, n...) + b = append(b, '\n') + } + return string(b) +} + +// appendHostsBlock runs as root inside the container and appends +// the given block to /etc/hosts. Idempotent via a sentinel-marker +// grep: if the marker is already present, the existing block is +// replaced with the new one (covers Up-on-changed-project), then +// the block is appended. Uses busybox-friendly sh syntax so it +// works on alpine + debian-slim equally. +func (o *Orchestrator) appendHostsBlock(ctx context.Context, id, block string) error { + const marker = "# devcontainer-go compose hosts patch" + script := fmt.Sprintf( + // 1) Strip any prior block (lines from marker to next blank + // or EOF). Uses sed with start-of-marker pattern. + // 2) Append the new block. + `set -e +if grep -qF %q /etc/hosts 2>/dev/null; then + sed -i.bak '/^%s$/,/^$/d' /etc/hosts || true + rm -f /etc/hosts.bak +fi +cat >> /etc/hosts <<'EOF' +%sEOF +`, + marker, marker, block, + ) + _, err := o.rt.ExecContainer(ctx, id, runtime.ExecOptions{ + Cmd: []string{"sh", "-c", script}, + User: "0", + }) + return err +} diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index 6591f40..01515d4 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "sort" + "strings" "sync" "testing" "time" @@ -20,20 +21,19 @@ import ( // (container exit codes, label-stored hashes), and supports // concurrent access from the orchestrator's within-level parallel // service starts. -// -// Capabilities default to docker-baseline (all true). Override via -// the Caps field per test. type mockRuntime struct { mu sync.Mutex - Caps runtime.Capabilities - // Resources networks map[string]map[string]string // name -> labels volumes map[string]map[string]string // name -> labels containers map[string]*mockContainer // id -> container // Call log for assertions + // Caps is what Capabilities() reports. newMockRuntime defaults to + // the docker baseline; tests exercising the fallback paths set it. + Caps runtime.Capabilities + createNetworkCalls int createVolumeCalls int runCalls int @@ -64,7 +64,7 @@ type mockContainer struct { func newMockRuntime() *mockRuntime { return &mockRuntime{ - Caps: runtime.Capabilities{Healthchecks: true, ExitCodes: true, NamespaceSharing: true, RestartPolicies: true, SharedVolumes: true, ServiceNameDNS: true}, + Caps: runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true}, networks: map[string]map[string]string{}, volumes: map[string]map[string]string{}, containers: map[string]*mockContainer{}, @@ -166,6 +166,10 @@ func (m *mockRuntime) InspectContainer(ctx context.Context, id string) (*runtime return d, nil } +func (m *mockRuntime) Capabilities() runtime.Capabilities { + return m.Caps +} + func (m *mockRuntime) InspectImage(ctx context.Context, ref string) (*runtime.ImageDetails, error) { m.mu.Lock() defer m.mu.Unlock() @@ -250,10 +254,6 @@ func (m *mockRuntime) RemoveImage(ctx context.Context, ref string) error { return nil } -func (m *mockRuntime) Capabilities() runtime.Capabilities { - return m.Caps -} - // labelsSuperset is the same predicate as docker's labelsMatch, kept // local so test mocks don't import runtime/docker. func labelsSuperset(have, want map[string]string) bool { @@ -285,7 +285,7 @@ func newProject(t *testing.T, deps map[string][]string) *composetypes.Project { func TestUp_SingleService(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) res, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}) @@ -334,7 +334,7 @@ func TestServiceToRunSpec_CarriesSecurityFields(t *testing.T) { func TestUp_DependencyOrder(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{ "db": nil, "api": {"db"}, @@ -367,7 +367,7 @@ func TestUp_DependencyOrder(t *testing.T) { func TestUp_IdempotentReuseOnHashMatch(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) plan := &Plan{Project: proj, ProjectName: "dc-x"} @@ -386,7 +386,7 @@ func TestUp_IdempotentReuseOnHashMatch(t *testing.T) { func TestUp_RecreateOnHashChange(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) if _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}); err != nil { @@ -420,7 +420,7 @@ func TestUp_RecreateOnHashChange(t *testing.T) { // a `docker pull`. func TestUp_RecreateOnImageDigestChange(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) plan := &Plan{Project: proj, ProjectName: "dc-x"} @@ -455,7 +455,7 @@ func TestUp_RecreateOnImageDigestChange(t *testing.T) { // (issue #71). func TestUp_StartsStoppedContainerOnConfigMatch(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) plan := &Plan{Project: proj, ProjectName: "dc-x"} @@ -506,7 +506,7 @@ func TestUp_PartialFailureSurfacesPartialError(t *testing.T) { } return nil, nil } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{ "db": nil, "api": {"db"}, @@ -533,7 +533,7 @@ func TestUp_HealthGateTimesOut(t *testing.T) { base.State = runtime.StateCreated return base } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) orch.HealthTimeout = 100 * time.Millisecond orch.PollInterval = 20 * time.Millisecond @@ -575,7 +575,7 @@ func TestUp_OptionalDependencySkipsOnTimeout(t *testing.T) { base.State = runtime.StateCreated return base } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) orch.HealthTimeout = 50 * time.Millisecond orch.PollInterval = 10 * time.Millisecond @@ -604,7 +604,7 @@ func TestUp_OptionalDependencySkipsOnTimeout(t *testing.T) { func TestUp_RefusesUnsupportedFields(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := &composetypes.Project{ Services: composetypes.Services{ "app": composetypes.ServiceConfig{Name: "app", Image: "alpine", Deploy: &composetypes.DeployConfig{Mode: "global"}}, @@ -643,7 +643,7 @@ func TestDown_RemovesProjectContainers(t *testing.T) { labels: map[string]string{LabelComposeProject: "other"}, } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) if err := orch.Down(context.Background(), &DownPlan{ProjectName: "dc-x"}); err != nil { t.Fatalf("Down: %v", err) } @@ -707,7 +707,7 @@ func TestDown_ReverseTopoOrder(t *testing.T) { stopFunc: origStop, } - orch := NewOrchestrator(wrapped, "docker") + orch := NewOrchestrator(wrapped) proj := newProject(t, map[string][]string{ "db": nil, "api": {"db"}, @@ -754,7 +754,7 @@ func TestUp_AnonymousVolumesFlowThrough(t *testing.T) { } return nil, nil } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) svc := composetypes.ServiceConfig{ Name: "app", Image: "alpine", @@ -848,7 +848,7 @@ func TestUp_ResourceLimitsTranslate(t *testing.T) { seen = spec return nil, nil } - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) svc := composetypes.ServiceConfig{Name: "app", Image: "alpine"} tc.mut(&svc) proj := &composetypes.Project{Services: composetypes.Services{"app": svc}} @@ -871,7 +871,7 @@ func TestUp_ResourceLimitsTranslate(t *testing.T) { // teardown would leak the project network. func TestDown_RemovesProjectNetwork(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) plan := &Plan{Project: proj, ProjectName: "dc-x"} if _, err := orch.Up(context.Background(), plan); err != nil { @@ -895,7 +895,7 @@ func TestDown_RemovesProjectNetwork(t *testing.T) { func TestDown_Idempotent(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) // Project never up; Down must be a clean no-op. if err := orch.Down(context.Background(), &DownPlan{ProjectName: "dc-x"}); err != nil { t.Errorf("Down on empty: %v", err) @@ -911,7 +911,7 @@ func TestDown_Idempotent(t *testing.T) { // adoption only ever applies to reattach/resume flows. func TestUp_AdoptsForeignContainerWithoutOurLabels(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) rt.containers["legacy-1"] = &mockContainer{ @@ -946,7 +946,7 @@ func TestUp_AdoptsForeignContainerWithoutOurLabels(t *testing.T) { // hands-off. func TestUp_AdoptsRunningForeignContainerWithoutStarting(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) rt.containers["legacy-2"] = &mockContainer{ @@ -976,7 +976,7 @@ func TestUp_AdoptsRunningForeignContainerWithoutStarting(t *testing.T) { // namespace targets) come up too. func TestUp_RestrictedServicesStartDependencyClosure(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{ "db": nil, "app": {"db"}, @@ -1014,7 +1014,7 @@ func TestServiceClosure_FollowsNamespaceEdges(t *testing.T) { // the project network — `none` in particular is an isolation request. func TestUp_NetworkModeCarriedAndProjectNetworkSkipped(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil, "sandboxed": nil}) sandboxed := proj.Services["sandboxed"] sandboxed.NetworkMode = "none" @@ -1051,7 +1051,7 @@ func TestUp_NetworkModeCarriedAndProjectNetworkSkipped(t *testing.T) { // topo order guarantees exists by the time the dependent is created. func TestUp_ServiceNetworkModeResolvesToContainer(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"proxy": nil, "app": nil}) app := proj.Services["app"] app.NetworkMode = "service:proxy" @@ -1082,7 +1082,7 @@ func TestUp_ServiceNetworkModeResolvesToContainer(t *testing.T) { // keep the container (and its upperdir + anon volumes) anyway. func TestUp_AdoptExistingReusesDespiteHashDrift(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) // A prior orchestrator-made container whose stored hash is stale. @@ -1120,7 +1120,7 @@ func TestUp_AdoptExistingReusesDespiteHashDrift(t *testing.T) { // Without AdoptExisting, hash drift still recreates (guards the default path). func TestUp_NoAdoptRecreatesOnHashDrift(t *testing.T) { rt := newMockRuntime() - orch := NewOrchestrator(rt, "docker") + orch := NewOrchestrator(rt) proj := newProject(t, map[string][]string{"app": nil}) rt.containers["old-1"] = &mockContainer{ id: "old-1", name: "dc-x-app-1", image: "alpine", @@ -1137,3 +1137,168 @@ func TestUp_NoAdoptRecreatesOnHashDrift(t *testing.T) { t.Error("expected recreate (remove) on hash drift without AdoptExisting") } } + +// TestUp_HealthGateRefusesUnreportedHealth pins the contract for a +// backend that does not surface health status. runtime.HealthStatus +// documents HealthNone as ambiguous — either the image declared no +// HEALTHCHECK, or the backend reports no health at all — and the gate +// treats it as satisfied so healthcheck-less projects come up. When +// the service declares a healthcheck of its own, that reading is +// unavailable: passing the gate would start dependents before the +// check ever succeeded. Until #128 removed the capability gating, +// Plan.Validate refused these plans up front via +// Capabilities.Healthchecks; this asserts the guarantee survives at +// the gate that needs it, for any Runtime implementation. +func TestUp_HealthGateRefusesUnreportedHealth(t *testing.T) { + // mockRuntime's inspect reports State=Running with the zero + // HealthStatus, which IS runtime.HealthNone. + rt := newMockRuntime() + orch := NewOrchestrator(rt) + orch.HealthTimeout = 100 * time.Millisecond + orch.PollInterval = 20 * time.Millisecond + + proj := &composetypes.Project{Services: composetypes.Services{ + "db": composetypes.ServiceConfig{ + Name: "db", Image: "alpine", + HealthCheck: &composetypes.HealthCheckConfig{Test: []string{"CMD", "true"}}, + }, + "app": composetypes.ServiceConfig{ + Name: "app", Image: "alpine", + DependsOn: composetypes.DependsOnConfig{ + "db": composetypes.ServiceDependency{Condition: "service_healthy", Required: true}, + }, + }, + }} + + _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}) + if err == nil { + t.Fatal("want an error: the backend never reported health for a service that declares one") + } + if !strings.Contains(err.Error(), "never reported a health status") { + t.Errorf("error = %v, want it to name the unreported health status", err) + } +} + +// TestUp_HealthGatePassesWithoutDeclaredHealthcheck locks the other +// side of that boundary: with no healthcheck declared, HealthNone +// keeps meaning "no healthcheck" and State=Running satisfies the +// gate. This is compose v2's behavior for healthcheck-less services +// and the reason HealthNone is permissive in the first place. +func TestUp_HealthGatePassesWithoutDeclaredHealthcheck(t *testing.T) { + rt := newMockRuntime() + orch := NewOrchestrator(rt) + orch.HealthTimeout = 100 * time.Millisecond + orch.PollInterval = 20 * time.Millisecond + + proj := &composetypes.Project{Services: composetypes.Services{ + "db": composetypes.ServiceConfig{Name: "db", Image: "alpine"}, + "app": composetypes.ServiceConfig{ + Name: "app", Image: "alpine", + DependsOn: composetypes.DependsOnConfig{ + "db": composetypes.ServiceDependency{Condition: "service_healthy", Required: true}, + }, + }, + }} + + if _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}); err != nil { + t.Fatalf("Up: %v", err) + } +} + +// TestUp_HealthGateAcceptsDisabledHealthchecks covers the ways a +// compose service can have a HealthCheckConfig that is nonetheless not +// an active healthcheck. Each must keep the permissive HealthNone + +// State=Running fallback, or a valid project would block on its own +// gate until the health timeout: +// +// - test: ["NONE"] — compose's inline disable. compose-go accepts it +// verbatim (loader/validate.go allows CMD, CMD-SHELL and NONE) and +// does not fold it into Disable, and runtime/docker forwards it to +// docker's NONE sentinel, so docker reports no health for it. +// - disable: true — the explicit form. +// - no test at all — the image's HEALTHCHECK applies, and the +// compose file cannot tell us whether the image declares one. +func TestUp_HealthGateAcceptsDisabledHealthchecks(t *testing.T) { + cases := map[string]*composetypes.HealthCheckConfig{ + "test_none": {Test: []string{"NONE"}}, + "disable": {Test: []string{"CMD", "true"}, Disable: true}, + "no_test": {}, + "nil_config": nil, + } + for name, hc := range cases { + t.Run(name, func(t *testing.T) { + rt := newMockRuntime() + orch := NewOrchestrator(rt) + orch.HealthTimeout = 100 * time.Millisecond + orch.PollInterval = 20 * time.Millisecond + + proj := &composetypes.Project{Services: composetypes.Services{ + "db": composetypes.ServiceConfig{Name: "db", Image: "alpine", HealthCheck: hc}, + "app": composetypes.ServiceConfig{ + Name: "app", Image: "alpine", + DependsOn: composetypes.DependsOnConfig{ + "db": composetypes.ServiceDependency{Condition: "service_healthy", Required: true}, + }, + }, + }} + + if _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}); err != nil { + t.Fatalf("Up: %v", err) + } + }) + } +} + +// TestUp_PatchesHostsWithoutServiceNameDNS pins the service-name DNS +// fallback: on a backend that does not resolve peers by service name, +// Up patches /etc/hosts inside every started container. Nothing +// observable at Up time distinguishes working DNS from broken DNS — +// the failure shows up inside the container later — which is why this +// stays a declared capability rather than a check at the gate. +func TestUp_PatchesHostsWithoutServiceNameDNS(t *testing.T) { + rt := newMockRuntime() + rt.Caps = runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: false} + + // Report an IP for every container so the patch has a map to write. + rt.OnInspect = func(id string, base *runtime.ContainerDetails) *runtime.ContainerDetails { + base.Labels["dev.containers.network-ip"] = "192.168.66.2" + return base + } + var patched []string + rt.OnExec = func(id string, opts runtime.ExecOptions) (runtime.ExecResult, error) { + if len(opts.Cmd) > 0 && strings.Contains(strings.Join(opts.Cmd, " "), "/etc/hosts") { + patched = append(patched, id) + } + return runtime.ExecResult{}, nil + } + + orch := NewOrchestrator(rt) + proj := &composetypes.Project{Services: composetypes.Services{ + "db": composetypes.ServiceConfig{Name: "db", Image: "alpine"}, + "app": composetypes.ServiceConfig{Name: "app", Image: "alpine"}, + }} + + if _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}); err != nil { + t.Fatalf("Up: %v", err) + } + if len(patched) == 0 { + t.Error("want /etc/hosts patched on a backend without service-name DNS") + } + + // And the inverse: the docker baseline never touches /etc/hosts. + rt2 := newMockRuntime() + var patched2 []string + rt2.OnExec = func(id string, opts runtime.ExecOptions) (runtime.ExecResult, error) { + if len(opts.Cmd) > 0 && strings.Contains(strings.Join(opts.Cmd, " "), "/etc/hosts") { + patched2 = append(patched2, id) + } + return runtime.ExecResult{}, nil + } + orch2 := NewOrchestrator(rt2) + if _, err := orch2.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-y"}); err != nil { + t.Fatalf("Up: %v", err) + } + if len(patched2) != 0 { + t.Errorf("docker baseline must not patch /etc/hosts, got %v", patched2) + } +} diff --git a/compose/plan.go b/compose/plan.go index 40b3c7a..6faf91e 100644 --- a/compose/plan.go +++ b/compose/plan.go @@ -2,11 +2,10 @@ package compose import ( "fmt" - "sort" - - composetypes "github.com/compose-spec/compose-go/v2/types" "github.com/crunchloop/devcontainer/runtime" + + composetypes "github.com/compose-spec/compose-go/v2/types" ) // Plan describes a compose-project Up request in a runtime-neutral @@ -65,37 +64,58 @@ type DownPlan struct { Project *composetypes.Project } -// Validate inspects the Plan against the active backend's -// Capabilities and the refused-feature list, returning a typed -// error on the first kind of refusal encountered. Calls are -// side-effect-free; safe to invoke before any backend interaction. +// Validate inspects the Plan against the refused-feature list and the +// backend's Capabilities, returning a typed error on the first refusal +// found. Calls are side-effect-free; safe to invoke before any backend +// interaction. // // Validation order: // 1. Hard refusals (§2.2 fields we never implement): one // UnsupportedFieldError listing every offending site. -// 2. Backend-gated features (depends_on conditions, namespace -// sharing, restart policies, shared volumes): one -// UnsupportedFeatureOnBackendError per offending feature, or -// a typed VolumeSharedAcrossServicesError for the volume case. -// -// Each kind returns the FIRST error of that kind found; if no -// refusals trigger, Validate returns nil. -func (p *Plan) Validate(backendName string, caps runtime.Capabilities) error { +// 2. depends_on conditions the backend cannot honour, per its +// Capabilities: one UnsupportedFeatureOnBackendError. Refused +// here rather than at the gate because neither absence is +// detectable there — a backend reporting no exit code reports +// zero, indistinguishable from a clean exit, and a service may +// inherit its healthcheck from the image, which the compose file +// cannot see. +func (p *Plan) Validate(caps runtime.Capabilities) error { if p == nil || p.Project == nil { return fmt.Errorf("compose.Plan.Validate: nil plan or project") } - - // Pass 1: hard refusals. Collect every offending field across - // the project so the user can fix them in a single edit. if err := refuseUnsupportedFields(p.Project); err != nil { return err } + return refuseUnsupportedConditions(caps, p.Project) +} - // Pass 2: backend-gated features. - if err := refuseBackendGated(backendName, caps, p.Project); err != nil { - return err +// refuseUnsupportedConditions refuses depends_on conditions the active +// backend cannot honour. Only service_completed_successfully is gated: +// service_healthy is enforced by Orchestrator.waitFor, which can tell +// "no health reported" from "no healthcheck declared" at the gate. +func refuseUnsupportedConditions(caps runtime.Capabilities, proj *composetypes.Project) error { + for name, svc := range proj.Services { + for _, dep := range svc.DependsOn { + switch dep.Condition { + case "service_healthy": + if !caps.Healthchecks { + return &UnsupportedFeatureOnBackendError{ + Capability: "Healthchecks", + Service: name, + Detail: "depends_on.condition: service_healthy requires backend healthcheck support", + } + } + case "service_completed_successfully": + if !caps.ExitCodes { + return &UnsupportedFeatureOnBackendError{ + Capability: "ExitCodes", + Service: name, + Detail: "depends_on.condition: service_completed_successfully requires backend exit-code surfacing", + } + } + } + } } - return nil } @@ -172,138 +192,6 @@ func refuseUnsupportedFields(proj *composetypes.Project) error { return &UnsupportedFieldError{Fields: sortFields(found)} } -// refuseBackendGated checks features whose support flips with -// Capabilities. Returns the first error encountered. -func refuseBackendGated(backendName string, caps runtime.Capabilities, proj *composetypes.Project) error { - for name, svc := range proj.Services { - // depends_on conditions - for _, dep := range svc.DependsOn { - switch dep.Condition { - case "service_healthy": - if !caps.Healthchecks { - return &UnsupportedFeatureOnBackendError{ - Backend: backendName, - Capability: "Healthchecks", - Service: name, - Detail: "depends_on.condition: service_healthy requires backend healthcheck support", - } - } - case "service_completed_successfully": - if !caps.ExitCodes { - return &UnsupportedFeatureOnBackendError{ - Backend: backendName, - Capability: "ExitCodes", - Service: name, - Detail: "depends_on.condition: service_completed_successfully requires backend exit-code surfacing", - } - } - } - } - // network_mode: service: / host / none — all require - // kernel namespace sharing this backend doesn't model. - if needsNamespaceSharing(svc.NetworkMode) && !caps.NamespaceSharing { - return &UnsupportedFeatureOnBackendError{ - Backend: backendName, - Capability: "NamespaceSharing", - Service: name, - Detail: fmt.Sprintf("network_mode %q requires kernel namespace sharing this backend lacks", svc.NetworkMode), - } - } - // pid: service: / host - if needsNamespaceSharing(svc.Pid) && !caps.NamespaceSharing { - return &UnsupportedFeatureOnBackendError{ - Backend: backendName, - Capability: "NamespaceSharing", - Service: name, - Detail: fmt.Sprintf("pid %q requires kernel namespace sharing this backend lacks", svc.Pid), - } - } - // ipc: service: / host - if needsNamespaceSharing(svc.Ipc) && !caps.NamespaceSharing { - return &UnsupportedFeatureOnBackendError{ - Backend: backendName, - Capability: "NamespaceSharing", - Service: name, - Detail: fmt.Sprintf("ipc %q requires kernel namespace sharing this backend lacks", svc.Ipc), - } - } - } - - // Shared volumes: any single named volume mounted into 2+ - // services. Anonymous and bind mounts are not affected. - if !caps.SharedVolumes { - if err := refuseSharedVolumes(proj); err != nil { - return err - } - } - return nil -} - -// needsNamespaceSharing returns true when a network/pid/ipc field -// value refers to another container's namespace. -func needsNamespaceSharing(v string) bool { - switch v { - case "host", "none": - return true - } - if isServiceNetworkMode(v) { - return true - } - const p = "container:" - return len(v) > len(p) && v[:len(p)] == p -} - -// refuseSharedVolumes returns the first volume mounted by 2+ -// services as a VolumeSharedAcrossServicesError. Walks every -// service's `volumes:` field looking for `type: volume` entries -// against the project's top-level `volumes:`. -func refuseSharedVolumes(proj *composetypes.Project) error { - users := make(map[string]map[string]struct{}) // volume -> set(service) - for svcName, svc := range proj.Services { - for _, vol := range svc.Volumes { - if vol.Type != composetypes.VolumeTypeVolume { - continue - } - // vol.Source is the top-level volume name. Sanity-check - // it actually maps to one — compose-go normalizes this - // during Load, so the lookup is just defensive. - if _, ok := proj.Volumes[vol.Source]; !ok { - continue - } - set, ok := users[vol.Source] - if !ok { - set = map[string]struct{}{} - users[vol.Source] = set - } - set[svcName] = struct{}{} - } - } - // Iterate volume names in sorted order so that when multiple - // volumes are shared, the error consistently reports the same - // one (test stability + better user experience on repeat runs). - volNames := make([]string, 0, len(users)) - for volName := range users { - volNames = append(volNames, volName) - } - sort.Strings(volNames) - for _, volName := range volNames { - set := users[volName] - if len(set) < 2 { - continue - } - services := make([]string, 0, len(set)) - for s := range set { - services = append(services, s) - } - sort.Strings(services) - return &VolumeSharedAcrossServicesError{ - Volume: volName, - Services: services, - } - } - return nil -} - // deployUnsupported collects refusals for sub-fields of deploy: that // this orchestrator can't honor. We accept deploy when it only carries // resources.limits with memory/cpus — that's how compose v3+ users diff --git a/compose/plan_test.go b/compose/plan_test.go index 5756fba..f05cd20 100644 --- a/compose/plan_test.go +++ b/compose/plan_test.go @@ -4,29 +4,18 @@ import ( "errors" "testing" - composetypes "github.com/compose-spec/compose-go/v2/types" - "github.com/crunchloop/devcontainer/runtime" + + composetypes "github.com/compose-spec/compose-go/v2/types" ) func dockerCaps() runtime.Capabilities { - return runtime.Capabilities{ - Healthchecks: true, - ExitCodes: true, - NamespaceSharing: true, - RestartPolicies: true, - SharedVolumes: true, - ServiceNameDNS: true, - } -} - -func limitedCaps() runtime.Capabilities { - return runtime.Capabilities{} + return runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true} } func TestValidate_NilProject(t *testing.T) { p := &Plan{} - if err := p.Validate("docker", dockerCaps()); err == nil { + if err := p.Validate(dockerCaps()); err == nil { t.Fatal("want error on nil project") } } @@ -38,7 +27,7 @@ func TestValidate_Clean(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("docker", dockerCaps()); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("Validate: %v", err) } } @@ -57,7 +46,7 @@ func TestValidate_RefusesSwarmFields(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("docker", dockerCaps()) + err := p.Validate(dockerCaps()) var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -75,7 +64,7 @@ func TestValidate_RefusesScaleMulti(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("docker", dockerCaps()) + err := p.Validate(dockerCaps()) var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -90,143 +79,114 @@ func TestValidate_AcceptsScaleOne(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("docker", dockerCaps()); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("Validate: %v", err) } } -func TestValidate_RefusesHealthyOnLimitedCaps(t *testing.T) { +func TestValidate_AcceptsServiceStarted(t *testing.T) { + // service_started is the v1 / default condition — no health + // gate, just "exists." proj := &composetypes.Project{ Services: composetypes.Services{ "app": composetypes.ServiceConfig{ Name: "app", Image: "alpine", DependsOn: composetypes.DependsOnConfig{ - "db": composetypes.ServiceDependency{Condition: "service_healthy"}, + "db": composetypes.ServiceDependency{Condition: "service_started"}, }, }, + "db": composetypes.ServiceConfig{Name: "db", Image: "postgres"}, }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("limited", limitedCaps()) - var bad *UnsupportedFeatureOnBackendError - if !errors.As(err, &bad) { - t.Fatalf("want *UnsupportedFeatureOnBackendError, got %T: %v", err, err) - } - if bad.Capability != "Healthchecks" { - t.Errorf("capability = %q, want Healthchecks", bad.Capability) + if err := p.Validate(dockerCaps()); err != nil { + t.Errorf("service_started must be accepted: %v", err) } } -func TestValidate_RefusesCompletedSuccessfullyOnLimitedCaps(t *testing.T) { +func TestValidate_AcceptsSingleServiceVolume(t *testing.T) { proj := &composetypes.Project{ + Volumes: composetypes.Volumes{ + "data": composetypes.VolumeConfig{Name: "data"}, + }, Services: composetypes.Services{ "app": composetypes.ServiceConfig{ Name: "app", Image: "alpine", - DependsOn: composetypes.DependsOnConfig{ - "setup": composetypes.ServiceDependency{Condition: "service_completed_successfully"}, + Volumes: []composetypes.ServiceVolumeConfig{ + {Type: composetypes.VolumeTypeVolume, Source: "data", Target: "/data"}, }, }, }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("limited", limitedCaps()) - var bad *UnsupportedFeatureOnBackendError - if !errors.As(err, &bad) { - t.Fatalf("want *UnsupportedFeatureOnBackendError, got %T: %v", err, err) - } - if bad.Capability != "ExitCodes" { - t.Errorf("capability = %q, want ExitCodes", bad.Capability) + if err := p.Validate(dockerCaps()); err != nil { + t.Errorf("single-service volume must be accepted: %v", err) } } -func TestValidate_AcceptsServiceStartedOnLimitedCaps(t *testing.T) { - // service_started is the v1 / default condition — no health - // gate, just "exists." Limited caps must allow it. +// TestValidate_RefusesCompletedSuccessfullyWithoutExitCodes pins the +// one condition still refused at plan time. A backend that does not +// surface exit codes reports the zero value for a stopped container, +// which is indistinguishable from a clean exit — so the gate cannot +// tell a failed job from a successful one and the plan is refused +// before any side effect. +func TestValidate_RefusesCompletedSuccessfullyWithoutExitCodes(t *testing.T) { proj := &composetypes.Project{ Services: composetypes.Services{ "app": composetypes.ServiceConfig{ Name: "app", Image: "alpine", DependsOn: composetypes.DependsOnConfig{ - "db": composetypes.ServiceDependency{Condition: "service_started"}, + "setup": composetypes.ServiceDependency{Condition: "service_completed_successfully"}, }, }, - "db": composetypes.ServiceConfig{Name: "db", Image: "postgres"}, }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("limited", limitedCaps()); err != nil { - t.Errorf("service_started must be accepted: %v", err) - } -} -func TestValidate_RefusesNamespaceSharingOnLimitedCaps(t *testing.T) { - proj := &composetypes.Project{ - Services: composetypes.Services{ - "app": composetypes.ServiceConfig{Name: "app", Image: "alpine", NetworkMode: "service:primary"}, - "primary": composetypes.ServiceConfig{Name: "primary", Image: "alpine"}, - }, - } - p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("limited", limitedCaps()) + err := p.Validate(runtime.Capabilities{ServiceNameDNS: true}) var bad *UnsupportedFeatureOnBackendError if !errors.As(err, &bad) { t.Fatalf("want *UnsupportedFeatureOnBackendError, got %T: %v", err, err) } - if bad.Capability != "NamespaceSharing" { - t.Errorf("capability = %q, want NamespaceSharing", bad.Capability) - } -} - -func TestValidate_RefusesSharedVolumeOnLimitedCaps(t *testing.T) { - proj := &composetypes.Project{ - Volumes: composetypes.Volumes{ - "data": composetypes.VolumeConfig{Name: "data"}, - }, - Services: composetypes.Services{ - "reader": composetypes.ServiceConfig{ - Name: "reader", Image: "alpine", - Volumes: []composetypes.ServiceVolumeConfig{ - {Type: composetypes.VolumeTypeVolume, Source: "data", Target: "/data"}, - }, - }, - "writer": composetypes.ServiceConfig{ - Name: "writer", Image: "alpine", - Volumes: []composetypes.ServiceVolumeConfig{ - {Type: composetypes.VolumeTypeVolume, Source: "data", Target: "/data"}, - }, - }, - }, - } - p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("limited", limitedCaps()) - var bad *VolumeSharedAcrossServicesError - if !errors.As(err, &bad) { - t.Fatalf("want *VolumeSharedAcrossServicesError, got %T: %v", err, err) - } - if bad.Volume != "data" { - t.Errorf("volume = %q, want data", bad.Volume) + if bad.Capability != "ExitCodes" { + t.Errorf("Capability = %q, want ExitCodes", bad.Capability) } - if len(bad.Services) != 2 { - t.Errorf("want 2 services, got %v", bad.Services) + // The same plan is accepted when the backend does surface them. + if err := p.Validate(dockerCaps()); err != nil { + t.Errorf("want accepted on a backend with ExitCodes: %v", err) } } -func TestValidate_AcceptsSingleServiceVolume(t *testing.T) { +// TestValidate_RefusesHealthyWithoutHealthchecks pins the plan-time +// refusal of service_healthy. Orchestrator.waitFor also guards this at +// the gate, but only when compose declares the test itself: a service +// inheriting its healthcheck from the image is indistinguishable there +// from one with no healthcheck at all. The plan-time refusal does not +// depend on where the healthcheck is declared, so it covers both. +func TestValidate_RefusesHealthyWithoutHealthchecks(t *testing.T) { + // No healthcheck declared in compose — the case waitFor cannot see. proj := &composetypes.Project{ - Volumes: composetypes.Volumes{ - "data": composetypes.VolumeConfig{Name: "data"}, - }, Services: composetypes.Services{ + "db": composetypes.ServiceConfig{Name: "db", Image: "alpine"}, "app": composetypes.ServiceConfig{ Name: "app", Image: "alpine", - Volumes: []composetypes.ServiceVolumeConfig{ - {Type: composetypes.VolumeTypeVolume, Source: "data", Target: "/data"}, + DependsOn: composetypes.DependsOnConfig{ + "db": composetypes.ServiceDependency{Condition: "service_healthy"}, }, }, }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("limited", limitedCaps()); err != nil { - t.Errorf("single-service volume must be accepted: %v", err) + + err := p.Validate(runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true}) + var bad *UnsupportedFeatureOnBackendError + if !errors.As(err, &bad) { + t.Fatalf("want *UnsupportedFeatureOnBackendError, got %T: %v", err, err) + } + if bad.Capability != "Healthchecks" { + t.Errorf("Capability = %q, want Healthchecks", bad.Capability) + } + if err := p.Validate(dockerCaps()); err != nil { + t.Errorf("want accepted on a backend with Healthchecks: %v", err) } } diff --git a/down.go b/down.go index 8a2a01c..6c61994 100644 --- a/down.go +++ b/down.go @@ -124,7 +124,7 @@ func (e *Engine) downCompose(ctx context.Context, ws *Workspace, opts DownOption switch e.opts.ComposeBackend { case ComposeBackendNative: - orch := compose.NewOrchestrator(e.runtime, "") + orch := compose.NewOrchestrator(e.runtime) if err := orch.Down(ctx, &compose.DownPlan{ ProjectName: projectName, RemoveVolumes: opts.RemoveVolumes, diff --git a/engine_test.go b/engine_test.go index b477682..e3cb445 100644 --- a/engine_test.go +++ b/engine_test.go @@ -206,17 +206,8 @@ func (f *fakeRuntime) RemoveImage(ctx context.Context, ref string) error { } func (f *fakeRuntime) Capabilities() runtime.Capabilities { - // fakeRuntime advertises the docker baseline; non-compose tests - // never read this. Compose orchestrator tests live in compose/ - // with their own purpose-built fake. - return runtime.Capabilities{ - Healthchecks: true, - ExitCodes: true, - NamespaceSharing: true, - RestartPolicies: true, - SharedVolumes: true, - ServiceNameDNS: true, - } + // The docker baseline; non-compose tests never read this. + return runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true} } func (f *fakeRuntime) FindContainerByLabel(ctx context.Context, key, value string) (*runtime.Container, error) { diff --git a/runtime/compose_primitives.go b/runtime/compose_primitives.go index 1173ad9..ec0c294 100644 --- a/runtime/compose_primitives.go +++ b/runtime/compose_primitives.go @@ -68,68 +68,37 @@ type LabelFilter struct { Match map[string]string } -// Capabilities advertises optional features a backend implements. -// The compose orchestrator's plan validator (compose.Plan.Validate) -// keys feature gates off this struct so per-backend conditionals -// stay out of the validator. Backends self-describe; defaults are -// the docker baseline. +// Capabilities advertises the two compose behaviours a backend cannot +// be assumed to provide and whose absence the orchestrator cannot +// detect at the point of use. Everything else a backend can't do +// surfaces as an error from the primitive itself. // -// Each field documents the upstream issue (or status note) governing -// it so future contributors can tell at a glance which capabilities -// might flip true on the apple backend in the future. See -// design/compose-native.md §11.5 for the full provenance. +// Backends self-describe; the docker baseline is both true. The +// returned value should be constant for the lifetime of the Runtime. type Capabilities struct { - // Healthchecks: backend honors HEALTHCHECK directives on - // RunSpec/BuildSpec, and InspectContainer surfaces - // State.Health.Status. Required for compose's - // depends_on..condition: service_healthy gating. - // - // Apple 0.12.x: false (apple/container #1502). + // Healthchecks: InspectContainer surfaces State.Health.Status. + // Required for compose's depends_on condition: service_healthy. + // Refused at plan time rather than at the gate because a service + // may inherit its healthcheck from the image, which the compose + // file cannot see — Orchestrator.waitFor catches only the case + // where compose declares the test itself. Healthchecks bool - // ExitCodes: InspectContainer returns the container's exit code - // after Stop (ContainerDetails.ExitCode is meaningful for - // state=exited). Required for compose's depends_on condition: - // service_completed_successfully. - // - // Apple 0.12.x: false (apple/container #1501). + // ExitCodes: InspectContainer returns a meaningful exit code for + // a stopped container (ContainerDetails.ExitCode is real for + // state=exited, not a zero placeholder). Required for compose's + // depends_on condition: service_completed_successfully — a + // backend that reports zero regardless would make a failed job + // look successful, and zero is indistinguishable from a clean + // exit, so this cannot be checked at the gate. ExitCodes bool - // NamespaceSharing: backend supports network_mode / pid / ipc - // set to service: (Linux namespace sharing within one - // kernel). - // - // Apple: architectural false — one VM per container means - // separate kernels; namespace sharing is not implementable. - NamespaceSharing bool - - // RestartPolicies: backend enforces compose's `restart:` field - // via RunSpec or backend-equivalent. When false, the - // orchestrator emits a single WarnRestartPolicyIgnoredOnBackend - // event per Plan rather than refusing the project. - // - // Apple 0.12.x: false (apple/container #286). - RestartPolicies bool - - // SharedVolumes: a single named volume can be concurrently - // mounted into 2+ running containers. Apple's - // ext4-on-disk-image volumes refuse multi-attach with - // VZErrorDomain Code=2; Plan.Validate refuses such projects on - // backends where this is false. - // - // Apple 0.12.x: false (apple/container #889). - SharedVolumes bool - - // ServiceNameDNS: containers on the project network can resolve - // peers by service name out of the box (compose's default - // behavior). When false, the orchestrator falls back to a - // post-start /etc/hosts patch driven by InspectContainer + - // ExecContainer to seed the service→IP map. - // - // Apple 0.12.x: false (probe 3; apple/container #856 / 856 - // resolution upstream is open). The hosts-patch workaround - // covers depends_on-declared edges; intra-level peers without a - // depends_on edge race and may miss the patch on first DNS - // lookup — documented limitation on this backend. + // ServiceNameDNS: containers on the project network resolve peers + // by service name out of the box, which is compose's default + // contract. When false the orchestrator patches /etc/hosts in + // every started container from the service→IP map. Nothing + // observable at Up time distinguishes working DNS from broken + // DNS — the failure appears inside the container later — so this + // cannot be checked at the gate either. ServiceNameDNS bool } diff --git a/runtime/docker/compose_primitives.go b/runtime/docker/compose_primitives.go index 0d111b4..9604e18 100644 --- a/runtime/docker/compose_primitives.go +++ b/runtime/docker/compose_primitives.go @@ -190,20 +190,6 @@ func (r *Runtime) RemoveImage(ctx context.Context, ref string) error { return nil } -// Capabilities advertises the docker backend's compose feature set. -// All flags true: docker has been the compose reference target -// since v2 shipped, so every gated feature is available. -func (r *Runtime) Capabilities() runtime.Capabilities { - return runtime.Capabilities{ - Healthchecks: true, - ExitCodes: true, - NamespaceSharing: true, - RestartPolicies: true, - SharedVolumes: true, - ServiceNameDNS: true, - } -} - // labelsMatch returns true if `have` is a superset of `want`: every // (k,v) in `want` is present and equal in `have`. Used by // CreateNetwork / CreateVolume idempotency checks. @@ -243,3 +229,14 @@ func mapContainerState(s string) runtime.State { } return runtime.State(s) } + +// Capabilities advertises the docker backend's compose behaviours. +// Both true: docker surfaces real exit codes on stopped containers and +// provides service-name DNS aliases on user-defined networks. +func (r *Runtime) Capabilities() runtime.Capabilities { + return runtime.Capabilities{ + Healthchecks: true, + ExitCodes: true, + ServiceNameDNS: true, + } +} diff --git a/runtime/docker/compose_primitives_test.go b/runtime/docker/compose_primitives_test.go index 68fae85..47633e8 100644 --- a/runtime/docker/compose_primitives_test.go +++ b/runtime/docker/compose_primitives_test.go @@ -47,22 +47,15 @@ func TestMapContainerState(t *testing.T) { } } -// TestCapabilities locks in docker's all-true compose feature set. -// Flipping any of these to false silently could let the compose -// orchestrator's Plan validator accept a project Docker can run -// but our other backends can't, eroding parity guarantees. +// TestCapabilities locks in the docker backend's compose baseline. +// Flipping either to false would send the orchestrator down a fallback +// path docker doesn't need: ExitCodes gates plan-time refusal of +// service_completed_successfully, ServiceNameDNS gates the /etc/hosts +// patch. func TestCapabilities(t *testing.T) { r := &Runtime{} - got := r.Capabilities() - want := runtime.Capabilities{ - Healthchecks: true, - ExitCodes: true, - NamespaceSharing: true, - RestartPolicies: true, - SharedVolumes: true, - ServiceNameDNS: true, - } - if got != want { + want := runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true} + if got := r.Capabilities(); got != want { t.Errorf("Capabilities = %+v, want %+v", got, want) } } diff --git a/runtime/errors.go b/runtime/errors.go index 9816194..4570e34 100644 --- a/runtime/errors.go +++ b/runtime/errors.go @@ -42,17 +42,6 @@ func (e *ContainerNotFoundError) Error() string { func (e *ContainerNotFoundError) Unwrap() error { return e.Err } -// ExecFailedError indicates an exec call completed with a non-zero -// exit code. Captured stderr is included for diagnostics. -type ExecFailedError struct { - ExitCode int - Stderr string -} - -func (e *ExecFailedError) Error() string { - return fmt.Sprintf("exec failed (exit=%d): %s", e.ExitCode, e.Stderr) -} - // ComposeUnavailableError indicates the `docker compose` v2 plugin is // not installed / not on PATH. Returned by ComposeRuntime methods on // first attempted use; cached on the runtime so subsequent calls @@ -91,37 +80,3 @@ func (e *DaemonUnavailableError) Error() string { } func (e *DaemonUnavailableError) Unwrap() error { return e.Err } - -// BuilderUnavailableError indicates the container engine's image-build -// component is missing or not running. Distinct from -// DaemonUnavailableError because the build engine is typically a -// separate process / VM that can be started independently (e.g. -// Docker's BuildKit daemon). -type BuilderUnavailableError struct { - // Hint is a backend-specific message telling the user how to - // remediate (e.g. "start the BuildKit daemon"). - Hint string - Err error -} - -func (e *BuilderUnavailableError) Error() string { - if e.Hint != "" { - return fmt.Sprintf("image build engine unavailable (%s): %v", e.Hint, e.Err) - } - return fmt.Sprintf("image build engine unavailable: %v", e.Err) -} - -func (e *BuilderUnavailableError) Unwrap() error { return e.Err } - -// UnsupportedOptionError indicates a RunSpec / BuildSpec field that -// the chosen backend cannot honor. Returned at the boundary instead -// of silently dropping the option, so callers fail fast rather than -// observing apparent success with missing behavior. -type UnsupportedOptionError struct { - Backend string - Option string -} - -func (e *UnsupportedOptionError) Error() string { - return fmt.Sprintf("%s: option %q is not supported on this backend", e.Backend, e.Option) -} diff --git a/runtime/runtime.go b/runtime/runtime.go index 667781f..4f07e9e 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -141,9 +141,7 @@ type Runtime interface { // orchestrator under compose/ (see design/compose-native.md §4). // Types live in runtime/compose_primitives.go. A backend that // returns ErrNotImplemented from any of these effectively opts - // out of compose source — Plan.Validate(Capabilities()) catches - // such projects at validation time and refuses with a typed - // error before any side effect. + // out of compose source. // CreateNetwork creates a network with the given name and // labels. Returns the backend's network ID for later @@ -181,11 +179,9 @@ type Runtime interface { // if missing. RemoveImage(ctx context.Context, ref string) error - // Capabilities advertises optional features this backend - // supports. compose.Plan.Validate keys feature gates off this - // struct so per-backend conditionals stay out of the validator. - // The returned value should be a constant for the lifetime of - // the Runtime; callers may cache it. + // Capabilities advertises the compose behaviours this backend + // provides that the orchestrator cannot verify itself. See + // runtime.Capabilities for why these two and no others. Capabilities() Capabilities } @@ -362,9 +358,7 @@ type RunSpec struct { // "container:"). Empty means the backend default. // The compose orchestrator translates `network_mode:` / `pid:` / // `ipc:` directives here, resolving `service:` references to - // the dependency's container first. Backends without namespace - // sharing never see these: compose.Plan.Validate refuses such - // projects via Capabilities.NamespaceSharing. + // the dependency's container first. NetworkMode string PidMode string IpcMode string diff --git a/test/integration/compose_native_orchestrator_test.go b/test/integration/compose_native_orchestrator_test.go index 0489805..f8c782f 100644 --- a/test/integration/compose_native_orchestrator_test.go +++ b/test/integration/compose_native_orchestrator_test.go @@ -85,7 +85,7 @@ services: `) projectName := "dc-it-native-twosvc" - orch := compose.NewOrchestrator(rt, "docker") + orch := compose.NewOrchestrator(rt) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() @@ -185,7 +185,7 @@ services: `) projectName := "dc-it-native-idem" - orch := compose.NewOrchestrator(rt, "docker") + orch := compose.NewOrchestrator(rt) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() @@ -247,7 +247,7 @@ services: `) projectName := "dc-it-native-ports" - orch := compose.NewOrchestrator(rt, "docker") + orch := compose.NewOrchestrator(rt) orch.HealthTimeout = 45 * time.Second ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) @@ -305,7 +305,7 @@ services: command: ["sh", "-c", "while sleep 1000; do :; done"] `) projectName := "dc-it-native-depson" - orch := compose.NewOrchestrator(rt, "docker") + orch := compose.NewOrchestrator(rt) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() diff --git a/up.go b/up.go index d56bb87..03bcec0 100644 --- a/up.go +++ b/up.go @@ -725,7 +725,7 @@ func (e *Engine) upComposeNative( return nil, err } - orch := compose.NewOrchestrator(e.runtime, "") + orch := compose.NewOrchestrator(e.runtime) res, err := orch.Up(ctx, &compose.Plan{ Project: project, ProjectName: projectName, @@ -969,7 +969,7 @@ func (e *Engine) composeDownExisting(ctx context.Context, existing *runtime.Cont } if e.opts.ComposeBackend == ComposeBackendNative { - orch := compose.NewOrchestrator(e.runtime, "") + orch := compose.NewOrchestrator(e.runtime) if err := orch.Down(ctx, &compose.DownPlan{ ProjectName: projectName, }); err != nil {