From b4f56c4395995ad56463702fd4c705871a96ac16 Mon Sep 17 00:00:00 2001 From: bilby91 <2201079+bilby91@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:09:47 +0000 Subject: [PATCH 1/5] refactor!: remove the per-backend capability gating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runtime.Capabilities existed to describe where a backend diverged from Docker. Every field's only false case was the Apple backend, and runtime/docker reported all six true (compose_primitives.go). With Docker the only backend the struct was unconditionally all-true, so everything it gated became unreachable — the same situation that retired selfHealthProber in #124 when its only implementor left. Removed from the public API: runtime.Capabilities, Runtime.Capabilities() compose.Plan.Validate(backendName, caps) -> Validate() compose.UnsupportedFeatureOnBackendError compose.VolumeSharedAcrossServicesError Orchestrator.BackendName + the NewOrchestrator parameter that set it runtime.BuilderUnavailableError, runtime.UnsupportedOptionError runtime.ExecFailedError BackendName's only reader was UnsupportedFeatureOnBackendError's message, and the Engine already passed "" at all three call sites, so the field carried no information in production. BuilderUnavailableError and UnsupportedOptionError were constructed only by the Apple backend; ExecFailedError has had no producer for far longer. Also removes the code the flags gated: Plan.refuseBackendGated and its helpers (needsNamespaceSharing, refuseSharedVolumes) — Docker supports health-gated depends_on, namespace sharing and shared volumes, so no refusal could fire Orchestrator's /etc/hosts post-start patch (patchHostsFiles, containerIP, renderHostsBlock, appendHostsBlock) — reached only when ServiceNameDNS was false, which only Apple reported; Docker has DNS aliases on the project network, and the path's only coverage was the deleted Apple integration suite compose/graph.go's isServiceNetworkMode, orphaned once needsNamespaceSharing went (serviceRefTarget, its callee, stays) The .dap review directive naming Capabilities() as the way to encode backend divergence goes too; design/compose-native.md keeps its capability sections as the historical record, per design/README.md. Behavior on Docker is unchanged: every removed branch was either all-true-gated or a no-op on this backend. Co-Authored-By: Claude Opus 5 --- .dap/review/engineering.md | 3 - CHANGELOG.md | 22 +++ compose/errors.go | 43 ----- compose/graph.go | 8 - compose/orchestrator.go | 138 +------------- compose/orchestrator_test.go | 56 +++--- compose/plan.go | 171 +----------------- compose/plan_test.go | 131 +------------- down.go | 2 +- engine_test.go | 14 -- runtime/compose_primitives.go | 66 ------- runtime/docker/compose_primitives.go | 14 -- runtime/docker/compose_primitives_test.go | 20 -- runtime/errors.go | 45 ----- runtime/runtime.go | 15 +- .../compose_native_orchestrator_test.go | 8 +- up.go | 4 +- 17 files changed, 74 insertions(+), 686 deletions(-) diff --git a/.dap/review/engineering.md b/.dap/review/engineering.md index 02846a1..d628efd 100644 --- a/.dap/review/engineering.md +++ b/.dap/review/engineering.md @@ -51,9 +51,6 @@ 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. ## R3. Destructive recreate diff --git a/CHANGELOG.md b/CHANGELOG.md index ec311ac..0234b9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +- **BREAKING — the per-backend capability gating is removed.** `runtime.Capabilities` + and `Runtime.Capabilities()` existed to describe where a backend diverged from + Docker: every field's only `false` case was the Apple backend, and `runtime/docker` + reported all six `true`. With Docker the only backend the struct was + unconditionally all-true, so the code it gated was unreachable. Removed: + `runtime.Capabilities`, `Runtime.Capabilities()`, `compose.Plan.Validate`'s + `backendName` and `caps` parameters (now `Validate()`), + `compose.UnsupportedFeatureOnBackendError`, + `compose.VolumeSharedAcrossServicesError`, and `Orchestrator.BackendName` with + the `NewOrchestrator` parameter that set it (the Engine already passed `""`; the + field's only reader was the deleted error). +- **compose (native)** — the `/etc/hosts` post-start patch is removed with it. It + existed only for backends without service-name DNS (`ServiceNameDNS: false`, Apple + only); Docker has built-in DNS aliases on the project network, so the branch was a + no-op there and its only coverage was the deleted Apple integration suite. + `Plan.Validate` no longer refuses health-gated `depends_on`, namespace-sharing + modes, or volumes shared across services — Docker supports all of them, so those + refusals could not fire. +- **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 diff --git a/compose/errors.go b/compose/errors.go index d4f9c18..977e351 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 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..3d91bde 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(); err != nil { return UpResult{}, err } @@ -216,135 +210,11 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { if err := o.gateLevel(ctx, plan, level, res.ContainerIDs, keep); err != nil { 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. - if !o.rt.Capabilities().ServiceNameDNS { - if err := o.patchHostsFiles(ctx, plan, res.ContainerIDs); err != nil { - return res, err - } - } } 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 { diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index 6591f40..b63f77b 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -20,14 +20,9 @@ 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 @@ -64,7 +59,6 @@ type mockContainer struct { func newMockRuntime() *mockRuntime { return &mockRuntime{ - Caps: runtime.Capabilities{Healthchecks: true, ExitCodes: true, NamespaceSharing: true, RestartPolicies: true, SharedVolumes: true, ServiceNameDNS: true}, networks: map[string]map[string]string{}, volumes: map[string]map[string]string{}, containers: map[string]*mockContainer{}, @@ -250,10 +244,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 +275,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 +324,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 +357,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 +376,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 +410,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 +445,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 +496,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 +523,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 +565,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 +594,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 +633,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 +697,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 +744,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 +838,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 +861,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 +885,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 +901,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 +936,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 +966,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 +1004,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 +1041,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 +1072,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 +1110,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", diff --git a/compose/plan.go b/compose/plan.go index 40b3c7a..74af730 100644 --- a/compose/plan.go +++ b/compose/plan.go @@ -2,11 +2,8 @@ package compose import ( "fmt" - "sort" composetypes "github.com/compose-spec/compose-go/v2/types" - - "github.com/crunchloop/devcontainer/runtime" ) // Plan describes a compose-project Up request in a runtime-neutral @@ -65,38 +62,16 @@ 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. -// -// 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 { +// Validate inspects the Plan against the refused-feature list, +// returning a typed UnsupportedFieldError that lists every offending +// (service, field) site so the user can fix them in a single edit. +// Calls are side-effect-free; safe to invoke before any backend +// interaction. Returns nil when the project uses nothing we refuse. +func (p *Plan) Validate() 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 - } - - // Pass 2: backend-gated features. - if err := refuseBackendGated(backendName, caps, p.Project); err != nil { - return err - } - - return nil + return refuseUnsupportedFields(p.Project) } // refuseUnsupportedFields walks the project and collects every use @@ -172,138 +147,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..7a8ede2 100644 --- a/compose/plan_test.go +++ b/compose/plan_test.go @@ -5,28 +5,11 @@ import ( "testing" composetypes "github.com/compose-spec/compose-go/v2/types" - - "github.com/crunchloop/devcontainer/runtime" ) -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{} -} - func TestValidate_NilProject(t *testing.T) { p := &Plan{} - if err := p.Validate("docker", dockerCaps()); err == nil { + if err := p.Validate(); err == nil { t.Fatal("want error on nil project") } } @@ -38,7 +21,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(); err != nil { t.Errorf("Validate: %v", err) } } @@ -57,7 +40,7 @@ func TestValidate_RefusesSwarmFields(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("docker", dockerCaps()) + err := p.Validate() var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -75,7 +58,7 @@ func TestValidate_RefusesScaleMulti(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate("docker", dockerCaps()) + err := p.Validate() var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -90,58 +73,14 @@ 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(); err != nil { t.Errorf("Validate: %v", err) } } -func TestValidate_RefusesHealthyOnLimitedCaps(t *testing.T) { - proj := &composetypes.Project{ - Services: composetypes.Services{ - "app": composetypes.ServiceConfig{ - Name: "app", Image: "alpine", - DependsOn: composetypes.DependsOnConfig{ - "db": composetypes.ServiceDependency{Condition: "service_healthy"}, - }, - }, - }, - } - 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) - } -} - -func TestValidate_RefusesCompletedSuccessfullyOnLimitedCaps(t *testing.T) { - proj := &composetypes.Project{ - Services: composetypes.Services{ - "app": composetypes.ServiceConfig{ - Name: "app", Image: "alpine", - DependsOn: composetypes.DependsOnConfig{ - "setup": composetypes.ServiceDependency{Condition: "service_completed_successfully"}, - }, - }, - }, - } - 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) - } -} - -func TestValidate_AcceptsServiceStartedOnLimitedCaps(t *testing.T) { +func TestValidate_AcceptsServiceStarted(t *testing.T) { // service_started is the v1 / default condition — no health - // gate, just "exists." Limited caps must allow it. + // gate, just "exists." proj := &composetypes.Project{ Services: composetypes.Services{ "app": composetypes.ServiceConfig{ @@ -154,63 +93,11 @@ func TestValidate_AcceptsServiceStartedOnLimitedCaps(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("limited", limitedCaps()); err != nil { + if err := p.Validate(); 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()) - 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 len(bad.Services) != 2 { - t.Errorf("want 2 services, got %v", bad.Services) - } -} - func TestValidate_AcceptsSingleServiceVolume(t *testing.T) { proj := &composetypes.Project{ Volumes: composetypes.Volumes{ @@ -226,7 +113,7 @@ func TestValidate_AcceptsSingleServiceVolume(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate("limited", limitedCaps()); err != nil { + if err := p.Validate(); err != nil { t.Errorf("single-service volume must be accepted: %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..622c0b3 100644 --- a/engine_test.go +++ b/engine_test.go @@ -205,20 +205,6 @@ func (f *fakeRuntime) RemoveImage(ctx context.Context, ref string) error { return runtime.ErrNotImplemented } -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, - } -} - func (f *fakeRuntime) FindContainerByLabel(ctx context.Context, key, value string) (*runtime.Container, error) { f.mu.Lock() defer f.mu.Unlock() diff --git a/runtime/compose_primitives.go b/runtime/compose_primitives.go index 1173ad9..b759c97 100644 --- a/runtime/compose_primitives.go +++ b/runtime/compose_primitives.go @@ -67,69 +67,3 @@ type LabelFilter struct { // client-side after enumeration. 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. -// -// 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. -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 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 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 bool -} diff --git a/runtime/docker/compose_primitives.go b/runtime/docker/compose_primitives.go index 0d111b4..7adde5c 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. diff --git a/runtime/docker/compose_primitives_test.go b/runtime/docker/compose_primitives_test.go index 68fae85..89ea848 100644 --- a/runtime/docker/compose_primitives_test.go +++ b/runtime/docker/compose_primitives_test.go @@ -46,23 +46,3 @@ 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. -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 { - 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..3ea56f9 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 @@ -180,13 +178,6 @@ type Runtime interface { // RemoveImage removes a local image by ID or reference. No-op // 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() Capabilities } // ImageRef identifies an image by digest and any associated tags. @@ -362,9 +353,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 { From 95b4ff7de1af6b8113e9ddd9c7375b26f942832f Mon Sep 17 00:00:00 2001 From: bilby91 <2201079+bilby91@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:22:22 +0000 Subject: [PATCH 2/5] fix(compose): refuse service_healthy when the backend reports no health MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review findings on #128. [HIGH] The capability removal dropped a guarantee for third-party backends. runtime.HealthStatus documents HealthNone as ambiguous — either the image declared no HEALTHCHECK, or the backend does not surface health at all — and waitFor reads it as "no healthcheck", so HealthNone + State=Running satisfied a service_healthy gate. Until this branch, Capabilities.Healthchecks refused those plans before any side effect; without it, a Runtime whose InspectContainer cannot report health silently passed the gate and its dependents started before the check ever succeeded. The guarantee now lives at the gate instead of in a self-reported capability flag: gateLevel tells waitFor whether the service declares an active healthcheck, and when it does, HealthNone no longer means "no healthcheck". waitFor keeps polling and reports an explicit error at the deadline rather than passing. This is strictly stronger than what it replaces — the old flag only protected backends that self-reported honestly, while this covers any implementation, including one that claims Healthchecks: true and doesn't deliver. Docker is unaffected: it reports starting/healthy/unhealthy for any container with a healthcheck, so the new branch is unreachable there. Two tests pin both sides of the boundary, and the refusal test fails without the fix. [CRITICAL] Restores the R2 directive this branch deleted. Removing the rule that forbids silently assuming Docker semantics, in the same change that removed the capability guard, was wrong regardless of the rule citing a now-deleted API: it withdrew the standard that catches the regression above. The rule is back with its substance intact and its mechanism updated to name explicit refusal rather than Capabilities(), and the code above now satisfies it. [LOW] README's runtime package inventory no longer advertises the removed capabilities API. Co-Authored-By: Claude Opus 5 --- .dap/review/engineering.md | 4 +++ CHANGELOG.md | 18 +++++++++- README.md | 2 +- compose/orchestrator.go | 32 +++++++++++++++-- compose/orchestrator_test.go | 68 ++++++++++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 4 deletions(-) diff --git a/.dap/review/engineering.md b/.dap/review/engineering.md index d628efd..04541d6 100644 --- a/.dap/review/engineering.md +++ b/.dap/review/engineering.md @@ -51,6 +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 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 0234b9d..0f83f42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING — the per-backend capability gating is removed.** `runtime.Capabilities` and `Runtime.Capabilities()` existed to describe where a backend diverged from Docker: every field's only `false` case was the Apple backend, and `runtime/docker` - reported all six `true`. With Docker the only backend the struct was + reported all six remaining fields `true` (`Checkpoint` went with the Podman backend + in the entry below). With Docker the only backend the struct was unconditionally all-true, so the code it gated was unreachable. Removed: `runtime.Capabilities`, `Runtime.Capabilities()`, `compose.Plan.Validate`'s `backendName` and `caps` parameters (now `Validate()`), @@ -80,6 +81,21 @@ 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 a healthcheck. + `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 declares a healthcheck itself, and previously + `Capabilities.Healthchecks` refused such plans up front. The guarantee now lives at + the gate: `Orchestrator.waitFor` keeps polling and fails with an explicit error + rather than starting dependents before the check ever succeeded. Docker is + unaffected — it reports `starting` / `healthy` / `unhealthy` for any container with + a healthcheck — but the check now covers every `runtime.Runtime` implementation, + including one that misreports its own capabilities. + ## [0.4.2] - 2026-08-23 ### Fixed diff --git a/README.md b/README.md index 1dfc040..c5534f8 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ func (*Engine) Down(ctx, *Workspace, DownOptions) error Sub-packages: - `config` — devcontainer.json parsing, merging, host-context substitution -- `runtime` — container backend abstraction (`Runtime`, `ComposeRuntime`, capabilities, network/volume/list primitives) +- `runtime` — container backend abstraction (`Runtime`, `ComposeRuntime`, network/volume/list primitives) - `runtime/docker` — Docker Engine API implementation (uses `moby/moby/client`) - `feature` — feature resolution (OCI / HTTPS / local), DAG ordering, dockerfile generation - `compose` — `dockerComposeFile` parsing via `compose-spec/compose-go`, plus a runtime-agnostic in-process orchestrator (`Orchestrator`, `Plan`, topological + health gating) used when `ComposeBackendNative` is selected diff --git a/compose/orchestrator.go b/compose/orchestrator.go index 3d91bde..1db0383 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -504,7 +504,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 "image has no HEALTHCHECK" from "backend does + // not surface health" below. + declaresHealthcheck := false + if cfg, ok := plan.Project.Services[svcName]; ok { + declaresHealthcheck = cfg.HealthCheck != nil && !cfg.HealthCheck.Disable + } + 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 @@ -522,8 +529,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 @@ -542,6 +551,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 } @@ -564,6 +586,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, diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index b63f77b..0228de4 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "sort" + "strings" "sync" "testing" "time" @@ -1127,3 +1128,70 @@ 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) + } +} From 6b893c995eee857d8b744b9a9c0ba1b91b2a8f2f Mon Sep 17 00:00:00 2001 From: bilby91 <2201079+bilby91@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:33:16 +0000 Subject: [PATCH 3/5] fix(compose): do not treat disabled healthchecks as declared Follow-up to 95b4ff7, which was too eager about what counts as a declared healthcheck and would have blocked valid Docker projects. compose lets a service disable its healthcheck inline with `test: ["NONE"]`. compose-go's validator accepts that verbatim (loader/validate.go allows CMD, CMD-SHELL and NONE) rather than folding it into Disable, and runtime/docker's toHealthcheck forwards Test unchanged, so docker disables the check and reports no health. The previous predicate (HealthCheck != nil && !Disable) called that an active healthcheck, so the service_healthy gate refused a container that was working as configured and Up failed at the health timeout. An empty test has the same problem from the other direction: the image's own HEALTHCHECK applies, and the compose file cannot tell us whether the image declares one, so HealthNone there is not evidence that the backend failed to report. declaresActiveHealthcheck now requires an explicit, non-NONE test command. Table test covers test:["NONE"], disable:true, an empty test and a nil config; the NONE and empty-test cases both fail against the old predicate. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 +++++-- compose/orchestrator.go | 30 ++++++++++++++++++++++-- compose/orchestrator_test.go | 44 ++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f83f42..be9bdd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,17 +84,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **compose (native)** — the `service_healthy` gate no longer passes when the backend - reports no health status for a service that declares a healthcheck. + 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 declares a healthcheck itself, and previously + wrong when the service names a real test command itself, and previously `Capabilities.Healthchecks` refused such plans up front. The guarantee now lives at the gate: `Orchestrator.waitFor` keeps polling and fails with an explicit error rather than starting dependents before the check ever succeeded. Docker is unaffected — it reports `starting` / `healthy` / `unhealthy` for any container with a healthcheck — but the check now covers every `runtime.Runtime` implementation, including one that misreports its own capabilities. + 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 diff --git a/compose/orchestrator.go b/compose/orchestrator.go index 1db0383..1b61e59 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -505,11 +505,11 @@ func (o *Orchestrator) gateLevel( continue } // Whether the service itself declares an active healthcheck. - // Distinguishes "image has no HEALTHCHECK" from "backend does + // Distinguishes "no healthcheck to report on" from "backend does // not surface health" below. declaresHealthcheck := false if cfg, ok := plan.Project.Services[svcName]; ok { - declaresHealthcheck = cfg.HealthCheck != nil && !cfg.HealthCheck.Disable + declaresHealthcheck = declaresActiveHealthcheck(cfg.HealthCheck) } if err := o.waitFor(ctx, svcName, cid, req.condition, declaresHealthcheck, deadline); err != nil { if req.optional { @@ -931,3 +931,29 @@ 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" +} diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index 0228de4..c70a0ff 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -1195,3 +1195,47 @@ func TestUp_HealthGatePassesWithoutDeclaredHealthcheck(t *testing.T) { 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) + } + }) + } +} From 1f85c68c825b59267339734cc35122ef5b66bc51 Mon Sep 17 00:00:00 2001 From: bilby91 <2201079+bilby91@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:53:17 +0000 Subject: [PATCH 4/5] refactor!: keep the two capability flags that cannot be checked at the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Narrows this branch in response to review findings 21 and 22. Deleting runtime.Capabilities outright removed three guarantees that README promises for any runtime.Runtime implementation, not one. Health is enforceable at the gate — waitFor can tell "no health reported" from "no healthcheck declared" — but the other two are not: ExitCodes: a backend that cannot surface an exit code reports zero, which is indistinguishable from a clean exit, so a failed setup job would satisfy service_completed_successfully. ServiceNameDNS: nothing observable at Up time separates working DNS from broken DNS; the failure appears inside the container later. Both are restored, and nothing else is: runtime.Capabilities keeps ExitCodes + ServiceNameDNS (six -> two) Runtime.Capabilities() is back on the interface, docker reports both Plan.Validate(caps) refuses service_completed_successfully without ExitCodes, via UnsupportedFeatureOnBackendError (Backend field dropped — the Engine never set one) Orchestrator's /etc/hosts fallback returns behind ServiceNameDNS Still removed, on the criteria the flags themselves failed: Healthchecks — the gate enforces it directly, and covers a backend that claims the capability and doesn't deliver NamespaceSharing, SharedVolumes — the primitive fails loudly when the backend can't honour the request, so plan-time refusal was UX, not correctness RestartPolicies — gated a WarnRestartPolicyIgnoredOnBackend event that was never implemented BackendName + the NewOrchestrator parameter, ExecFailedError, BuilderUnavailableError, UnsupportedOptionError, VolumeSharedAcrossServicesError Tests: the ExitCodes refusal, the hosts patch firing without ServiceNameDNS and not firing on the docker baseline, and docker's two-field capability baseline. Apple references in the restored /etc/hosts helpers are rewritten — the mechanism is generic, that backend is gone. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 41 ++++---- README.md | 2 +- compose/errors.go | 26 +++++ compose/orchestrator.go | 122 +++++++++++++++++++++- compose/orchestrator_test.go | 63 +++++++++++ compose/plan.go | 49 +++++++-- compose/plan_test.go | 53 ++++++++-- engine_test.go | 5 + runtime/compose_primitives.go | 27 +++++ runtime/docker/compose_primitives.go | 10 ++ runtime/docker/compose_primitives_test.go | 13 +++ runtime/runtime.go | 5 + 12 files changed, 381 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be9bdd4..65b2289 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,25 +9,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- **BREAKING — the per-backend capability gating is removed.** `runtime.Capabilities` - and `Runtime.Capabilities()` existed to describe where a backend diverged from - Docker: every field's only `false` case was the Apple backend, and `runtime/docker` - reported all six remaining fields `true` (`Checkpoint` went with the Podman backend - in the entry below). With Docker the only backend the struct was - unconditionally all-true, so the code it gated was unreachable. Removed: - `runtime.Capabilities`, `Runtime.Capabilities()`, `compose.Plan.Validate`'s - `backendName` and `caps` parameters (now `Validate()`), - `compose.UnsupportedFeatureOnBackendError`, - `compose.VolumeSharedAcrossServicesError`, and `Orchestrator.BackendName` with - the `NewOrchestrator` parameter that set it (the Engine already passed `""`; the - field's only reader was the deleted error). -- **compose (native)** — the `/etc/hosts` post-start patch is removed with it. It - existed only for backends without service-name DNS (`ServiceNameDNS: false`, Apple - only); Docker has built-in DNS aliases on the project network, so the branch was a - no-op there and its only coverage was the deleted Apple integration suite. - `Plan.Validate` no longer refuses health-gated `depends_on`, namespace-sharing - modes, or volumes shared across services — Docker supports all of them, so those - refusals could not fire. +- **BREAKING — `runtime.Capabilities` is cut from six fields to two.** 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 two behaviours whose absence the + orchestrator cannot detect at the point of use: `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: `Healthchecks`, now enforced at the gate instead — see + Changed below; `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` now refuses exactly one backend-gated feature — + `depends_on.condition: service_completed_successfully` against a backend 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 diff --git a/README.md b/README.md index c5534f8..1dfc040 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ func (*Engine) Down(ctx, *Workspace, DownOptions) error Sub-packages: - `config` — devcontainer.json parsing, merging, host-context substitution -- `runtime` — container backend abstraction (`Runtime`, `ComposeRuntime`, network/volume/list primitives) +- `runtime` — container backend abstraction (`Runtime`, `ComposeRuntime`, capabilities, network/volume/list primitives) - `runtime/docker` — Docker Engine API implementation (uses `moby/moby/client`) - `feature` — feature resolution (OCI / HTTPS / local), DAG ordering, dockerfile generation - `compose` — `dockerComposeFile` parsing via `compose-spec/compose-go`, plus a runtime-agnostic in-process orchestrator (`Orchestrator`, `Plan`, topological + health gating) used when `ComposeBackendNative` is selected diff --git a/compose/errors.go b/compose/errors.go index 977e351..50b3797 100644 --- a/compose/errors.go +++ b/compose/errors.go @@ -113,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/orchestrator.go b/compose/orchestrator.go index 1b61e59..9648be8 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -106,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(); err != nil { + if err := plan.Validate(o.rt.Capabilities()); err != nil { return UpResult{}, err } @@ -210,6 +210,16 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { if err := o.gateLevel(ctx, plan, level, res.ContainerIDs, keep); err != nil { return res, err } + + // 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 + } + } } return res, nil @@ -957,3 +967,113 @@ func declaresActiveHealthcheck(hc *composetypes.HealthCheckConfig) bool { } 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 c70a0ff..e37cc07 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -30,6 +30,10 @@ type mockRuntime struct { 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 @@ -60,6 +64,7 @@ type mockContainer struct { func newMockRuntime() *mockRuntime { return &mockRuntime{ + Caps: runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true}, networks: map[string]map[string]string{}, volumes: map[string]map[string]string{}, containers: map[string]*mockContainer{}, @@ -161,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() @@ -1239,3 +1248,57 @@ func TestUp_HealthGateAcceptsDisabledHealthchecks(t *testing.T) { }) } } + +// 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{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 74af730..0c4a93e 100644 --- a/compose/plan.go +++ b/compose/plan.go @@ -3,6 +3,8 @@ package compose import ( "fmt" + "github.com/crunchloop/devcontainer/runtime" + composetypes "github.com/compose-spec/compose-go/v2/types" ) @@ -62,16 +64,49 @@ type DownPlan struct { Project *composetypes.Project } -// Validate inspects the Plan against the refused-feature list, -// returning a typed UnsupportedFieldError that lists every offending -// (service, field) site so the user can fix them in a single edit. -// Calls are side-effect-free; safe to invoke before any backend -// interaction. Returns nil when the project uses nothing we refuse. -func (p *Plan) Validate() error { +// 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. depends_on.condition: service_completed_successfully against a +// backend that does not surface exit codes: one +// UnsupportedFeatureOnBackendError. This is refused here rather +// than at the gate because a backend that reports no exit code +// reports zero, which is indistinguishable from a clean exit. +func (p *Plan) Validate(caps runtime.Capabilities) error { if p == nil || p.Project == nil { return fmt.Errorf("compose.Plan.Validate: nil plan or project") } - return refuseUnsupportedFields(p.Project) + if err := refuseUnsupportedFields(p.Project); err != nil { + return err + } + return refuseUnsupportedConditions(caps, p.Project) +} + +// 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 { + if caps.ExitCodes { + return nil + } + for name, svc := range proj.Services { + for _, dep := range svc.DependsOn { + if dep.Condition == "service_completed_successfully" { + return &UnsupportedFeatureOnBackendError{ + Capability: "ExitCodes", + Service: name, + Detail: "depends_on.condition: service_completed_successfully requires backend exit-code surfacing", + } + } + } + } + return nil } // refuseUnsupportedFields walks the project and collects every use diff --git a/compose/plan_test.go b/compose/plan_test.go index 7a8ede2..6c210b2 100644 --- a/compose/plan_test.go +++ b/compose/plan_test.go @@ -4,12 +4,18 @@ import ( "errors" "testing" + "github.com/crunchloop/devcontainer/runtime" + composetypes "github.com/compose-spec/compose-go/v2/types" ) +func dockerCaps() runtime.Capabilities { + return runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} +} + func TestValidate_NilProject(t *testing.T) { p := &Plan{} - if err := p.Validate(); err == nil { + if err := p.Validate(dockerCaps()); err == nil { t.Fatal("want error on nil project") } } @@ -21,7 +27,7 @@ func TestValidate_Clean(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate(); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("Validate: %v", err) } } @@ -40,7 +46,7 @@ func TestValidate_RefusesSwarmFields(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate() + err := p.Validate(dockerCaps()) var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -58,7 +64,7 @@ func TestValidate_RefusesScaleMulti(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - err := p.Validate() + err := p.Validate(dockerCaps()) var unsup *UnsupportedFieldError if !errors.As(err, &unsup) { t.Fatalf("want *UnsupportedFieldError, got %T: %v", err, err) @@ -73,7 +79,7 @@ func TestValidate_AcceptsScaleOne(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate(); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("Validate: %v", err) } } @@ -93,7 +99,7 @@ func TestValidate_AcceptsServiceStarted(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate(); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("service_started must be accepted: %v", err) } } @@ -113,7 +119,40 @@ func TestValidate_AcceptsSingleServiceVolume(t *testing.T) { }, } p := &Plan{Project: proj, ProjectName: "dc-x"} - if err := p.Validate(); err != nil { + if err := p.Validate(dockerCaps()); err != nil { t.Errorf("single-service volume must be accepted: %v", err) } } + +// 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{ + "setup": composetypes.ServiceDependency{Condition: "service_completed_successfully"}, + }, + }, + }, + } + p := &Plan{Project: proj, ProjectName: "dc-x"} + + 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 != "ExitCodes" { + t.Errorf("Capability = %q, want ExitCodes", bad.Capability) + } + // 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) + } +} diff --git a/engine_test.go b/engine_test.go index 622c0b3..44d1bc0 100644 --- a/engine_test.go +++ b/engine_test.go @@ -205,6 +205,11 @@ func (f *fakeRuntime) RemoveImage(ctx context.Context, ref string) error { return runtime.ErrNotImplemented } +func (f *fakeRuntime) Capabilities() runtime.Capabilities { + // The docker baseline; non-compose tests never read this. + return runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} +} + func (f *fakeRuntime) FindContainerByLabel(ctx context.Context, key, value string) (*runtime.Container, error) { f.mu.Lock() defer f.mu.Unlock() diff --git a/runtime/compose_primitives.go b/runtime/compose_primitives.go index b759c97..fa74d20 100644 --- a/runtime/compose_primitives.go +++ b/runtime/compose_primitives.go @@ -67,3 +67,30 @@ type LabelFilter struct { // client-side after enumeration. Match map[string]string } + +// 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. +// +// Backends self-describe; the docker baseline is both true. The +// returned value should be constant for the lifetime of the Runtime. +type Capabilities struct { + // 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 + + // 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 7adde5c..4c7fe3c 100644 --- a/runtime/docker/compose_primitives.go +++ b/runtime/docker/compose_primitives.go @@ -229,3 +229,13 @@ 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{ + ExitCodes: true, + ServiceNameDNS: true, + } +} diff --git a/runtime/docker/compose_primitives_test.go b/runtime/docker/compose_primitives_test.go index 89ea848..c2cfe7b 100644 --- a/runtime/docker/compose_primitives_test.go +++ b/runtime/docker/compose_primitives_test.go @@ -46,3 +46,16 @@ func TestMapContainerState(t *testing.T) { } } } + +// 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{} + want := runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} + if got := r.Capabilities(); got != want { + t.Errorf("Capabilities = %+v, want %+v", got, want) + } +} diff --git a/runtime/runtime.go b/runtime/runtime.go index 3ea56f9..4f07e9e 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -178,6 +178,11 @@ type Runtime interface { // RemoveImage removes a local image by ID or reference. No-op // if missing. RemoveImage(ctx context.Context, ref string) error + + // 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 } // ImageRef identifies an image by digest and any associated tags. From 1420791edd3b06381474dfecf7c700c007107f11 Mon Sep 17 00:00:00 2001 From: bilby91 <2201079+bilby91@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:19:07 +0000 Subject: [PATCH 5/5] fix(compose): restore the Healthchecks capability alongside the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last gap between this branch and pre-branch behavior. waitFor's gate catches a backend that cannot report health only when compose declares the healthcheck test itself. A service inheriting its healthcheck from the image is indistinguishable at the gate from one with no healthcheck at all — declaresActiveHealthcheck returns false and HealthNone + Running satisfies the condition. Capabilities.Healthchecks refused every service_healthy plan regardless of where the check was declared, so dropping it narrowed the guarantee for that sub-case. Healthchecks is back as a third capability field and Plan.Validate refuses service_healthy when it is false. The gate hardening stays: the capability covers a backend that honestly reports it cannot do healthchecks, the gate covers one that claims the capability and then reports nothing. Neither existed in both forms before this branch, so health is now strictly better guarded than it was. runtime.Capabilities ends at three fields — Healthchecks, ExitCodes, ServiceNameDNS — the three whose absence the orchestrator cannot fully detect at the point of use. NamespaceSharing and SharedVolumes stay removed (the primitive fails loudly), as does RestartPolicies (gated an event that was never implemented). Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 35 +++++++++++----------- compose/orchestrator_test.go | 4 +-- compose/plan.go | 36 +++++++++++++++-------- compose/plan_test.go | 36 ++++++++++++++++++++++- engine_test.go | 2 +- runtime/compose_primitives.go | 8 +++++ runtime/docker/compose_primitives.go | 1 + runtime/docker/compose_primitives_test.go | 2 +- 8 files changed, 89 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65b2289..bba9aef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,15 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed -- **BREAKING — `runtime.Capabilities` is cut from six fields to two.** The struct +- **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 two behaviours whose absence the - orchestrator cannot detect at the point of use: `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: `Healthchecks`, now enforced at the gate instead — see - Changed below; `NamespaceSharing` and `SharedVolumes`, where a backend that cannot + 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. @@ -25,9 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `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` now refuses exactly one backend-gated feature — - `depends_on.condition: service_completed_successfully` against a backend without - `ExitCodes`. `compose.UnsupportedFeatureOnBackendError` loses its `Backend` field + `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. @@ -91,13 +92,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `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, and previously - `Capabilities.Healthchecks` refused such plans up front. The guarantee now lives at - the gate: `Orchestrator.waitFor` keeps polling and fails with an explicit error - rather than starting dependents before the check ever succeeded. Docker is - unaffected — it reports `starting` / `healthy` / `unhealthy` for any container with - a healthcheck — but the check now covers every `runtime.Runtime` implementation, - including one that misreports its own capabilities. + 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 diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index e37cc07..01515d4 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -64,7 +64,7 @@ type mockContainer struct { func newMockRuntime() *mockRuntime { return &mockRuntime{ - Caps: runtime.Capabilities{ExitCodes: 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{}, @@ -1257,7 +1257,7 @@ func TestUp_HealthGateAcceptsDisabledHealthchecks(t *testing.T) { // stays a declared capability rather than a check at the gate. func TestUp_PatchesHostsWithoutServiceNameDNS(t *testing.T) { rt := newMockRuntime() - rt.Caps = runtime.Capabilities{ExitCodes: true, ServiceNameDNS: false} + 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 { diff --git a/compose/plan.go b/compose/plan.go index 0c4a93e..6faf91e 100644 --- a/compose/plan.go +++ b/compose/plan.go @@ -72,11 +72,13 @@ type DownPlan struct { // Validation order: // 1. Hard refusals (§2.2 fields we never implement): one // UnsupportedFieldError listing every offending site. -// 2. depends_on.condition: service_completed_successfully against a -// backend that does not surface exit codes: one -// UnsupportedFeatureOnBackendError. This is refused here rather -// than at the gate because a backend that reports no exit code -// reports zero, which is indistinguishable from a clean exit. +// 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") @@ -92,16 +94,24 @@ func (p *Plan) Validate(caps runtime.Capabilities) error { // 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 { - if caps.ExitCodes { - return nil - } for name, svc := range proj.Services { for _, dep := range svc.DependsOn { - if dep.Condition == "service_completed_successfully" { - return &UnsupportedFeatureOnBackendError{ - Capability: "ExitCodes", - Service: name, - Detail: "depends_on.condition: service_completed_successfully requires backend exit-code surfacing", + 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", + } } } } diff --git a/compose/plan_test.go b/compose/plan_test.go index 6c210b2..f05cd20 100644 --- a/compose/plan_test.go +++ b/compose/plan_test.go @@ -10,7 +10,7 @@ import ( ) func dockerCaps() runtime.Capabilities { - return runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} + return runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true} } func TestValidate_NilProject(t *testing.T) { @@ -156,3 +156,37 @@ func TestValidate_RefusesCompletedSuccessfullyWithoutExitCodes(t *testing.T) { t.Errorf("want accepted on a backend with ExitCodes: %v", err) } } + +// 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{ + 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"}, + }, + }, + }, + } + p := &Plan{Project: proj, ProjectName: "dc-x"} + + 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/engine_test.go b/engine_test.go index 44d1bc0..e3cb445 100644 --- a/engine_test.go +++ b/engine_test.go @@ -207,7 +207,7 @@ func (f *fakeRuntime) RemoveImage(ctx context.Context, ref string) error { func (f *fakeRuntime) Capabilities() runtime.Capabilities { // The docker baseline; non-compose tests never read this. - return runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} + 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 fa74d20..ec0c294 100644 --- a/runtime/compose_primitives.go +++ b/runtime/compose_primitives.go @@ -76,6 +76,14 @@ type LabelFilter struct { // 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: 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 a meaningful exit code for // a stopped container (ContainerDetails.ExitCode is real for // state=exited, not a zero placeholder). Required for compose's diff --git a/runtime/docker/compose_primitives.go b/runtime/docker/compose_primitives.go index 4c7fe3c..9604e18 100644 --- a/runtime/docker/compose_primitives.go +++ b/runtime/docker/compose_primitives.go @@ -235,6 +235,7 @@ func mapContainerState(s string) runtime.State { // 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 c2cfe7b..47633e8 100644 --- a/runtime/docker/compose_primitives_test.go +++ b/runtime/docker/compose_primitives_test.go @@ -54,7 +54,7 @@ func TestMapContainerState(t *testing.T) { // patch. func TestCapabilities(t *testing.T) { r := &Runtime{} - want := runtime.Capabilities{ExitCodes: true, ServiceNameDNS: true} + want := runtime.Capabilities{Healthchecks: true, ExitCodes: true, ServiceNameDNS: true} if got := r.Capabilities(); got != want { t.Errorf("Capabilities = %+v, want %+v", got, want) }