Address glours review on #14258: TestExecutePlugin_GetRelayInfo and
TestExecutePlugin_GetRelayInfoDesktop only exercised the happy path.
Add TestExecutePlugin_GetRelayInfoUnresolvedGateway to cover the
best-effort contract relay.go omitempty is built around: a
NetworkInspect failure, an IPv6-only gateway, and no gateway at all
must each still list the network with Gateway left empty.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A provider running its service LOCALLY has to pick the address its
published endpoint binds, and on a standalone Linux engine no address
is both relay-reachable and off the LAN (#14257):
host.docker.internal resolves there to the bridge gateway, which a
loopback-only listener cannot accept, while the wildcard exposes the
port on every host interface. The bundled example provider worked
around it by binding 0.0.0.0.
The new get-relay-info message closes that gap, with compose owning
the platform knowledge so the provider just binds what is announced.
Compose answers with one JSON line listing the networks the relay
would join — the dependents' networks, exactly as selected for the
relay deployment — each with the address a locally-run endpoint should
bind to be reachable from the relay: the network's engine-assigned
gateway on a standalone engine (an address the host owns on that
bridge, reachable from the relay but not from the LAN), and 127.0.0.1
under Docker Desktop — the networks live inside the VM there, and the
host's own loopback is, factually, where a host process is reached
through the Desktop proxy. Resolution is lazy (Desktop detection and
network inspects run only when a provider asks — a remote-resource
provider never does) and best-effort: a missing gateway means "bind
elsewhere".
Compose also announces the message types it accepts in the
COMPOSE_PROVIDER_MESSAGES environment variable of the provider
process: an unknown message stays a fatal protocol error by design
(a provider REQUIRING an unsupported message must fail loudly), and
the announcement is how a provider adapts instead of failing. The
example provider binds the first announced gateway, falling back to
the wildcard only when none is announced; through it,
TestProviderPublishEndpoint exercises the gateway path on Linux CI
and the loopback path on Docker Desktop.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The plan learns the start vocabulary — inert until a caller opts in
(ReconcileOptions.Scope, zero value keeps today's create-only plans
byte-identical):
- OpWaitCondition, one node per (awaited service, condition),
deduplicated across dependents like networkNodes deduplicates
networks; required:false marks the shared node best-effort, one
required dependent upgrades it. service_started needs no node — a
plain DAG edge to the dependency's chain end expresses it. Health is
deliberately re-observed at execution time: the plan encodes what to
wait for, never a stale observation.
- OpRunPreStart, emitted at plan time only when no replica was running
at observation — the imperative gating — targeting the
lowest-numbered replica.
- OpRunPostStart per container, after its start.
- replica chains: inject+start+post_start of replica n+1 depends on
the end of replica n's chain, today's sequential start order made
visible in golden plans; startChainEnds points at the chain end so a
service_started dependent waits for the whole service, matching
InDependencyOrder semantics.
- scope Start plans starting observed exited/created containers
without converging them (the future compose start); scope
CreateStart appends the start phase to the create plan, start nodes
resolving their target from the create node that materializes the
replica (CreateNodeID, the mechanism OpRenameContainer already
uses).
Lifecycle parity with the imperative engine is load-bearing and
golden-locked:
- dependency conditions are evaluated even when nothing has to start
(waitDependencies runs for every visited service before looking at
what to start), so an up with everything running still fails on an
unhealthy required dependency;
- an exceptional-state replica takes NO start-phase node: its bare
create-phase restart leaves it running when the start phase looks, so
the imperative engine neither re-starts nor injects — and it gates
pre_start like any running replica;
- startChainEnds carries the end-of-visit node set (waits included when
nothing started), so a service_started dependent begins only once the
dependency's whole visit completed, matching InDependencyOrder;
- under scope Start, a scale>0 service with no container at all fails
the plan with startService's exact error.
The "service:<name>:<number>" resource-ID format is built and parsed in
one place (serviceReplicaID/serviceReplicaPrefix/startGroupID), and the
replica sort deliberately carries the plan's determinism over the
unordered containerNodes iteration.
Golden tests only; no executor support yet and no caller passes the
scope. Epic #14081, Lot 1 — reconciler (first item).
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The exact pull-failure wording is daemon-backend dependent (classic
graphdriver: "pull access denied for ..."; containerd image store:
"...: not found"), and OutputMatches only checks stdout while the
daemon's progress line is written to stderr. Assert the image name
appears on stderr instead, which holds regardless of backend or
wording.
Signed-off-by: Ricardo Branco <rbranco@suse.de>
wantAbs was just wantSource under a different name, left over from an
earlier revision. Use wantSource directly.
(glours review on #14234)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
BindMountSource kept iterating after locating the bind mount at the
expected destination, so a hypothetical second bind entry at the same
target would trip a spurious mismatch even though the first entry
already confirmed the source. Break out of the loop once a match is
found, per docker-agent review feedback on #14234.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
.Mounts also lists named volumes and tmpfs entries at the same
Destination; matching on Destination alone let those shadow the
intended bind mount, either failing with a confusing volume Source or
silently passing against an empty tmpfs Source. Filter to Type ==
"bind" before comparing, as the function's name and doc promise.
Reported by docker-agent.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
filepath.Abs resolves a relative wantSource against the test process
cwd, not any project directory -- silently comparing against the
wrong base for a future caller passing a relative path, even though
the doc comment frames this as a general check. Reject a relative
wantSource explicitly instead.
(docker-agent review on #14234)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
TestOciRemoteProjectDirectory publishes a project whose only service
mounts a relative volume, then runs `up` on the oci:// artifact with an
explicit --project-directory pointing elsewhere. The relative volume
must resolve against that directory.
It currently fails against the pinned compose-go: LoadConfigFiles
defaults the working dir to the downloaded artifact's own cache
directory whenever a remote resource loader is involved, silently
overriding the explicit --project-directory the same way it would a
mere default -- docker/compose#14224. On Docker Desktop this surfaces
as a hard failure (the cache directory isn't a shared mount); on Linux
it silently mounts the wrong directory, matching the original report.
New BindMountSource check (pkg/e2e/checks.go) pins a service's bind
mount source to an exact expected path, for tests that need to verify
which working directory a relative volume path resolved against.
Needs the compose-go fix (github.com/ndeloof/compose-go@a626c70,
branch 14224-working-dir) to pass -- not yet reflected in go.mod here.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
glours' review on this PR (pullrequestreview-5277692679): stopLateStarter
only ever got armed inside stopOnFirstExit's own closure, so it caught a
service racing the on-exit cascade's sweep but not one racing
gracefulTeardown's (Ctrl+C/SIGTERM) identical one-shot stopApplication
sweep -- same root cause, same file, just the other trigger. Worse for
that path: Ctrl+C can land at t~=0, before monitor.Start even starts and
while s.start() (its own uncancelable context) is still climbing the
dependency graph, so most services may still be created, not running.
The only recovery today is a second manual Ctrl+C.
u.isTerminated already flags "termination is underway" and was already
set by gracefulTeardown (just not consulted for late-starter catching,
and set after stopApplication rather than before it). Extract the
late-starter watch out of stopOnFirstExit into its own listener,
stopLateStarters, armed on u.isTerminated regardless of which path set
it, and register it unconditionally (Ctrl+C works regardless of the
on-exit policy, unlike stopOnFirstExit's own listener). stopOnFirstExit
now sets isTerminated itself before its sweep, symmetric with
gracefulTeardown (also reordered to set it before stopApplication, not
after).
isLateStarter is extracted as a pure predicate, following the existing
shouldFollowStartEvent precedent, and unit-tested (TestIsLateStarter):
the previous fix (ca867482a) shipped only against a flaky test hitting
the on-exit case by chance, with no dedicated regression test at all.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
TestUpExitCodeFromContainerKilled hung for its full 2-minute budget on
CI, and its event transcript shows why: 'Aborting on container exit'
sweeps the application while the start phase — which runs on a
deliberately uncancelable context for SIGTERM management — was still
starting services; test-1 came up AFTER its stop and stayed up, so the
monitor never drained and up never returned.
The abort listener now watches the events stream past its trigger: any
container started after the sweep is a late starter from that race, and
gets stopped as it appears. Event-driven, so there is no listing window
to miss; idempotent stops make duplicates harmless. The exit code
semantics are preserved: the late starter's own 143 flows through
captureExitCodeFrom exactly as when the sweep wins the race.
TestUpExitCodeFrom* pass 5/5 locally.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
TestWatch/debian flakes on loaded runners during 'writing to a file
until Compose watch is up and running': that bootstrap loop ran under
poll.WaitOn's default 10s budget, which must absorb image pull/build,
container start and watcher initialization. Only the bootstrap gets the
2-minute budget — every later step keeps the sharp default so a real
sync regression still fails fast.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
TestAttachRestart kept failing residually after the re-attach anchoring
fix, and its new daemon-view dump told exactly why: the daemon holds
all three 'world' lines while compose printed two. The monitor returns
on the final die event — an events-channel fact — and up canceled the
global context on the spot, killing the re-attach log streams with the
last run's line still in flight: the exit notice outran the output that
preceded it.
Re-attach streams are now counted in a WaitGroup, and the monitor
wrapper waits for them to reach their natural EOF — guaranteed once the
containers exited — before canceling, under a bound that only protects
against a wedged daemon and is skipped entirely when the context is
already down (Ctrl-C). AttachRestart passes 5/5 locally with this
drain; it failed within 2 CI attempts without it.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Wait listed running containers only, so a service that finished between
up and the listing — a fast run, or a service long done by the time the
user types the command — failed with 'no containers for project' in a
few milliseconds instead of returning its recorded exit code. This is
also the root cause of the TestWaitAndDrop flake: its 'faster' service
sleeps 2 seconds, less than the harness latency between the two steps.
The condition wait observes (container no longer running) already holds
for such a target: fall back to a full listing only when no container
is running, and let ContainerWait return the recorded status
immediately. Scoping the fallback to the previously-erroring path keeps
every other semantics intact — in particular a stale exited one-off can
never short-circuit a wait that has live containers to observe.
Exit-code propagation verified end to end: wait on an already-failed
service returns its code (7), not an error.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Review finding on runEndTracker.Observe: recording nothing for an exit
event without a timestamp is the deliberate choice — the anchor is
evaluated by the daemon against its own log clock, so substituting the
local clock would introduce real skew mis-anchoring to paper over a
hypothetical daemon quirk, while dropping only degrades that container
to the pre-tracker fallback.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The FinishedAt anchor did not cure the third-run loss on CI (still 2
worlds for 3 exit notices after a full minute, oldstable runner). The
remaining suspects are on both sides of the API: a line the daemon
never captured (copier torn down before a millisecond-lived run's
output) or a line compose still fails to relay. On timeout the test
now dumps `docker logs` for the container — ground truth that
discriminates the two on the next CI occurrence.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The CI hardening in the previous commit turned TestAttachRestart's
flake into a reliable detector, and what it detected is a real loss:
re-attaching with since=StartedAt drops a fast run's first lines
forever, because the daemon starts copying stdout before it records
StartedAt.
Anchoring on the inspected FinishedAt fixes the common case but leaves
a narrower race CI still caught: when the new run itself finishes
before compose reacts to its start event, the inspected FinishedAt is
already the NEW run's own end, and the log window drops everything the
run printed — two worlds for three exit notices, the third never
arriving no matter how long you wait.
Both re-attach sites (attached up, logs --follow) therefore anchor on
the session's own record of the container's previous exit
(runEndTracker): the monitor delivers events in order, so the anchor
captured synchronously at start-event time is necessarily the previous
run's end — nanosecond-precise, immune to how fast the new run dies.
The inspected FinishedAt remains the fallback for a container the
session never saw exit, and a fresh container keeps no lower bound. A
unit test pins the ordering contract, including the fast-run sequence
CI caught.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Three tests failed 9 CI runs across 5 branches this week, all on
asynchronous-observation races, none reproducible locally:
- RequireServiceState asserted on a single `compose ps` snapshot; the
daemon reports state transitions asynchronously from everything else
a test observes (TestUpDependenciesNotStopped saw 'created' while
the container's logs were already flowing). It now polls until the
state converges (15s bound).
- TestAttachRestart counted restart log lines in a snapshot taken as
soon as the third exit notice appeared; exit notices come from the
events monitor while log lines come from the re-attached logs
stream — two channels with no ordering between them. The count is
now awaited like the exit notices already were; a genuinely lost
line still fails, by timeout.
- TestUpExitCodeFromContainerKilled ran a full up+abort cycle under a
60s ceiling, once exceeded on a loaded oldstable runner; raised to
120s.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The plan documented in both PRs had #14215 land first, then #14093's
compose-go bump delete the transient TestServiceHashContinuity. Merged
in the opposite order, main's pkg/compose tests no longer even compile:
richServiceFixture sets fields the container-spec layering turned into
promoted embeds (struct literals require go1.27 for that), and once
compilable two tests fail by construction.
- richServiceFixture is built by assignment;
- TestServiceHashContinuity is deleted, exactly as its own comment
prescribes: the layered compose-go is the reorder it existed to
outlive, its proof duty (pinned == historical bytes) is done, and
TestHashGoldenValues carries the continuity contract alone — it
PASSES on the layered compose-go, confirming the pinning preserved
every historical hash through the layering;
- pull_refresh_after, newly exposed at the service root, is appended
to serviceHashKeyOrder: the fallback already emitted it in that
exact position, so no hash moves — listing it only freezes the
layout and satisfies the coverage walker.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The trimming of hash-excluded fields becomes trimServiceHashFields,
shared between ServiceHash and the continuity test so the exclusion set
cannot silently drift; pinRootKeyOrder reuses the package's sortedKeys
helper; the key-order coverage walker now follows encoding/json
promotion rules for non-struct embeds and unexported fields.
Golden hashes unchanged.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ServiceHash digests json.Marshal of types.ServiceConfig, which couples
every recorded config-hash to the DECLARATION ORDER of compose-go
struct fields: encoding/json emits struct fields in that order and
flattens embedded structs at their embedding position. Any compose-go
refactoring that moves a field — such as the upcoming container-spec
layering, which regroups the whole struct — would change the bytes, and
with them the hash, of configurations that did not change at all: every
container recreated on the first `up` after an upgrade.
The hash now re-emits the marshaled object with its ROOT keys in a
frozen list reproducing the historical order (generated by reflection
over the last pre-layering compose-go), values byte-verbatim. For
today, the output is byte-identical to the direct marshal — proven by a
continuity test — so every existing container stamp stays valid: no
migration, no recreation, full backward compatibility. From the first
struct reorder on, the frozen list alone carries that continuity,
locked by golden-value tests; a root attribute added later is appended
in sorted order and, thanks to omitempty, only moves the hash of
configurations that use it — exactly like a field addition always did.
A reflection test fails when compose-go grows a root attribute missing
from the list, so extending the hash surface stays a reviewed decision.
Nested objects keep their own struct marshal: a reorder inside one of
them would still move hashes — the golden tests exist to turn that into
a caught, reviewed event rather than a silent side effect. Network and
volume hashes are unchanged (their structs are not being reordered) and
gain the same golden locks.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent review: without --build, an image tagged
e2e-additional-context-base:latest left over from a previous run would
let up skip the build entirely, passing the test without exercising
the additional_contexts-to-a-disabled-service path it exists to lock
in.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A service reachable only as another service's build additional_contexts
must still build even when its own profile is inactive: it's referenced
for its image, not started as a workload -- the same tolerance the
depends_on consistency check already grants a disabled, non-required
dependency, and the same case addBuildDependencies already special-cases
on the docker/compose side.
compose-go's own consistency check doesn't grant that tolerance yet: it
rejects the reference outright, failing the whole project load before
addBuildDependencies (or anything else, up included) ever runs -- for
up, build and up --no-build alike, exactly as reported.
This is expected to fail until compose-go bumps to include
compose-spec/compose-go#931.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The previous fix only covered create/start. projectOrName backs every
other service-targeting command too (stop, kill, pause, unpause, logs,
rm, down, ps, events), so they still surfaced the same two problems: a
raw "no such service" error for a job name, and -- when
COMPOSE_PROJECT_NAME is set -- a silent no-op, since projectOrName's
label-driven fallback swallows the load error entirely.
Fold the check into projectOrName so every caller is fixed at once,
instead of duplicating the wiring per command. start.go's own
job-handling code, now redundant, is removed; jobTargetErr and
unselectedJobs (cmd/compose/run.go) are unchanged, just called from one
more place.
Adds unit coverage for projectOrName itself (including the
COMPOSE_PROJECT_NAME path and the typo-correlation edge case) and e2e
coverage for two more callers (stop, down) to confirm the fix lives in
the shared helper rather than being re-implemented per command.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
projectOrName falls back to a label-driven, file-less project on any
load failure when COMPOSE_PROJECT_NAME is set -- swallowing the load
error entirely, including "no such service" for a target that's
actually a declared job. Since that branch returns err == nil,
jobTargetErr (gated on err != nil) and rejectScheduledJobs (gated on
project != nil) were both skipped: `docker compose start migrate`
fell through to a label-driven start that finds no container for a
job that was never run, and exited 0 having silently done nothing.
Refuse a named job explicitly on that fallback path too, reusing
unselectedJobs from the previous commit.
Also adds the test coverage this and the previous fix (job-target
error messages on create/start) were entirely missing: create.go and
start.go had no unit tests at all, and pkg/e2e/jobs_test.go only
exercised up/run.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
materializeJobClosure walked job-typed depends_on edges without
re-checking triggers.manual: false on anything but the top-level run
target, so `docker compose run A` where A depends_on job B (manual:
false) silently ran B anyway.
manual: false declares a job harmful to trigger outside its
schedule; depends_on doesn't change who caused the execution or when
-- pulling B in to satisfy A's dependency is still the run command
causing that out-of-schedule execution, one hop removed. Refuse it
the same way the top-level check already does, before anything in the
closure is created.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Rebasing onto main pulled in the relay work (added after this branch
was cut), whose tests build ServiceConfig/PreStart literals against
the flat, pre-layering shape: DependsOn/Networks set directly, and
PreStart typed as []ServiceHook. Both are incompatible with the
ContainerSpec/WorkloadSpec split this branch adopts -- the promoted
DependsOn/Networks fields need go1.27 to set via a flat literal (this
module stays on 1.26.3), and PreStart is now []PreStartHook, a full
container specification, not a plain hook.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Every publish safeguard and image-pinning path used to walk only
project.Services, leaving jobs invisible:
- the sensitive-data checks (literal environment values, env_file
scans, bind-mount warnings, build-only rejection) now cover jobs — a
job declaring AWS_SECRET_ACCESS_KEY=... was published without the
guard rail a service gets;
- the image-digest override pins job images too (jobs dressed as
services run through the exact WithImagesResolved semantics), so the
published artifact is reproducible for jobs as well;
- the application index references job images;
- `config --images` lists them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
compose-go resolves each hook against its service at load time: the
model itself carries the full container specification a hook runs with,
and the runtime consumes it as-is through the standard create path —
the runtime-side merge helper goes away.
Consuming the full spec means honoring ALL of it:
- service references it may carry — volumes_from entries, and
service:-scoped network_mode/ipc/pid, inherited or declared — resolve
to live container IDs exactly like the service create path does; the
daemon knows nothing about service names and rejected them, failing
up for any service combining volumes_from with a pre_start hook
(locked by an e2e scenario).
- hook labels — declared or inherited — merge into the container's
labels, the runtime identification set winning on conflicts.
- the <API 1.44 network-connect fallback joins the HOOK's networks,
not the parent service's: a hook overriding networks was connected
to the wrong ones.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Per the spec, any job can be triggered manually by an explicit run
command, its automated triggers notwithstanding — so run now accepts
scheduled jobs too. The exception is a job explicitly declared with
triggers.manual: false, which run rejects: meant for scheduled jobs
whose out-of-schedule execution would be harmful.
compose-go is bumped to the jobs-branch commit making Manual tri-state
(*bool) and allowing manual and schedule to be combined.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Scenario-DSL coverage for the jobs entry points:
- up refuses a project declaring an active scheduled job before creating
any resource;
- run executes a manual job like a service, starting its depends_on
services first, and refuses a schedule-only job;
- a job's own env_file feeds its environment through run — the
materialization happens before environment resolution;
- a job depending on another job runs the dependency to completion
first, through the exact machinery a service dependency uses.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
pre_start hooks carry the full container specification, but the
init-container runner only consumed a handful of attributes (image,
command, user, env, workdir) — everything else was silently dropped.
Instead of wiring attributes one by one, the hook's specification is
merged over the service's ContainerSpec through compose-go's own file
merge machinery (override.Merge on the canonical yaml tree): command
and entrypoint replace, environment merges per key with the hook
winning, extra_hosts and dns accumulate entries, ulimits merge — the
exact per-attribute rules of multi-file compose, maintained in one
place. Every ContainerSpec attribute inherits this way, current and
future, with zero attribute-specific code.
The merged spec then runs through the standard create path
(getCreateConfigs) as a service-shaped one-off, so resources,
capabilities, dns, sysctls, logging... materialize exactly as they
would for a service container. Hook containers keep their minimal
labels and carry no container-number, so tooling telling replicas
apart does not count them.
The only deliberate exception is volumes: mounts inherit at runtime
through volumes_from — the only mechanism that shares the service's
anonymous and image volumes — and the hook's own volume declarations,
materialized by the standard path, take precedence per target. This is
what lets an init container get read-write access to a volume the
service mounts read-only (fixes: see PR).
e2e scenarios lock extra_hosts inheritance and accumulation, volume
override and completion, and the unit tests pin the merge rules.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
compose-spec/compose-go#866 is merged: jobs top-level element, container
specification layered as ContainerSpec/WorkloadSpec, pre_start hooks
carrying the full container specification resolved at load time. Bump
to the merged head and adapt in the same movement — composite literals
setting moved fields wrap them into the embedded ContainerSpec /
WorkloadSpec (promoted field access was already source-compatible, so
this is literal-only, no behavior change), and pre_start handling is
typed against PreStartHook.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The relay only dials an upstream and forwards bytes: it needs none of
Docker's default Linux capabilities. Run it with CapDrop: ["ALL"],
keeping only NET_BIND_SERVICE back via CapAdd since a route commonly
targets a privileged port (e.g. 80, 443) that the relay -- running
unprivileged as UID 65532 -- must still be able to listen on inside
its own container.
Validated end-to-end against a real provider project: a container on
the relay's network reaches the provider through its published, low
port with the hardened capability set in place.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
glours' review flagged the same bug class this PR already fixes for
start/restart: stopContainer (down.go) runs PreStop hooks
unconditionally, and runHook execs into the container -- which fails
against a shell-less relay. Guarded it the same way, with a regression
test.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent review (via glours) flagged that startService's own
pre_start call site had the same gap this PR fixes for start/restart:
lowestNumberedContainer can resolve to the relay container, and
runPreStart shares its VolumesFrom with whatever container it is
given -- reachable via plain start/up whenever a provider service also
declares pre_start. Guarded it the same way, with a regression test.
Also, while touching this: mirror the TOCTOU/StateRemoving tolerance
already needed in removeServiceRelay onto ensureServiceRelay own
removal branch, which had the identical latent race; dedupe
isRelayContainer against the pre-existing isRelay(labels) in
monitor.go instead of reimplementing the same label check; and collapse
runPlugin two separate command == "up" checks into one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent review: removeServiceRelay called ContainerRemove
unconditionally, unlike the existing ensureServiceRelay which already
guards against two concurrency issues — a relay stuck in StateRemoving
(a concurrent ContainerRemove fails with "removal already in
progress") and a TOCTOU window between findRelayContainer and
ContainerRemove where another goroutine removes the relay first. Both
now degrade to the already-achieved outcome (relay gone) instead of
failing the whole up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A provider that stops publishing endpoints on a later `up` left its
previously created relay running: deployRelay was only evaluated when
endpoints were non-empty, so a zero-endpoint up fell through with no
cleanup. The generic reconciler is no help either — it returns early
for any service.Provider != nil, before the recreate logic that would
otherwise catch this. runPlugin now removes any existing relay when a
provider publishes no endpoint on an up, whether it never did or
stopped doing so since a previous run.
Separately, startServiceContainer and restartContainer had no
relay-awareness at all, unlike exec/cp which already refuse to target
one via checkRelayTarget. A relay is a shell-less, FROM-scratch binary
standing in for the service on the network: it has no filesystem or
process for secrets/configs injection or PreStart/PostStart/PreStop
hooks to act on. Nothing in the compose-spec schema forbids declaring
secrets, configs or hooks on a provider: service, so this was reachable
in practice, not just in theory. Both now skip those steps for a relay
container while still starting/restarting the container itself
normally.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ensureServiceRelay runs concurrently per provider service (the shared
project mutex is released before this Docker API work). Two goroutines
reconciling relays that happen to share a consumer network can both
decide, from their own ContainerList snapshot, that the same relay
still needs connecting to it. Whichever runs second got the daemon's
"endpoint already exists" back from NetworkConnect, wrapped and
returned as a fatal error even though the relay was already in the
desired state.
Tolerate errdefs.IsConflict the same way the rest of the relay/network
reconciliation already does, instead of failing the run over a race
that already resolved to the right outcome.
(docker-agent review on #14236)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
relayIdentity only hashes image+routes, never network topology. When
a dependent service is added after the relay is already up, on a
network the relay isn't attached to, the relay's identity is
unaffected -- so ensureServiceRelay's running-container fast path
returned nil without ever looking at whether networkKeys had grown.
The new consumer could never reach the provider through it, silently:
no error, just an unreachable compose-native address.
ensureRelayNetworks now runs on every reuse path (running, stopped,
paused) before the state-specific branch decides what to do with the
container, connecting whichever networks in networkKeys the relay
isn't already on -- reusing NetworkSettings from the same
ContainerList call findRelayContainer already made, no extra inspect.
Networks the relay is already connected to are left untouched.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
`compose stop` on a provider service short-circuited straight into
runPlugin(..., "stop"), never touching stopContainers -- so the relay
container this PR gives a provider service (RestartPolicyUnlessStopped,
a real persistent container) was left running and still Up in
`compose ps`, contradicting docs/extension.md's claim that stop
treats the relay like the service.
downService already stops/removes a provider's containers before
invoking the plugin; stop.go gets the same treatment: the service's
containers are stopped unconditionally, then the plugin's own stop
hook (if any) runs for provider services. start/restart needed no
change -- neither special-cases providers, so they already operate on
whatever containers match the service filter, relay included.
Verified locally: reverting just this change makes the new e2e steps
below fail on "stop halts the relay container".
Extends TestProviderPublishEndpoint with stop/start/restart coverage
of the relay container.
(glours review on #14193)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
runPlugin held the global mutex — there to guard concurrent writes to
project.Services — across ensureServiceRelay, whose Docker API work
(list, create, image pull, start, a 30s removal wait) forced every
concurrent provider to wait on the slowest one.
The mutex now covers only the shared-state work: env-var injection plus
the relay's network selection, which reads project.Services and must
not race with another provider's writes. The relay is deployed after
the lock is released, taking the pre-computed network keys — simply
unlocking around the call, as first suggested, would have traded the
serialization for a data race on the services map.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
down handled a provider service by running the plugin alone — but the
service may own a project container, the relay deployed when it
published endpoints. Left running, it kept the project network in use
and `down -v` failed with "Resource is still in use".
The relay is now part of the service's deprovisioning: its containers
are removed first, then the plugin removes the provider's resource —
mirroring up, which provisions the resource before deploying the relay.
The example provider's down used to answer with a hardcoded error (a
leftover no test relied on): it now succeeds, with the failure
simulation kept behind PROVIDER_DOWN_FAILURE.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Providers push messages on their own initiative — setenv, info,
publish-endpoint… — they do not respond to anything. "invalid response
from plugin" sent users looking for a request that never existed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A provider legitimately expresses its endpoints from the host's
perspective — "localhost:5734" is where its resource listens, on the
machine compose runs on. But the relay dials from its own network
namespace, where loopback names the relay container itself: routes were
passed verbatim, so every connection died on the relay's own empty
loopback while the host.docker.internal ExtraHosts mapping provisioned
for exactly this purpose sat unused.
relayRoutesSpec now rewrites host-relative upstreams (localhost, any
loopback IP, unspecified or empty host) to host.docker.internal before
rendering; LAN IPs and DNS names still pass verbatim. The rewrite
happens before the identity hash, so existing relays carrying the old
routes are recreated on the next up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A provider's resource lives outside the compose network: consumers could
only reach it through injected variables carrying a host-published
address — nothing like the compose-native experience of addressing a
service by name at its well-known port.
A provider may now publish where each endpoint of its resource actually
listens:
{"type": "publish-endpoint", "message": "80=localhost:49152"}
The endpoint is announced as seen from the provider's host: the relay —
the component that knows it runs inside a container — rewrites loopback
or unspecified upstream hosts to host.docker.internal (resolved through
its injected host-gateway extra_host); routable addresses pass through.
When at least one endpoint is published, compose deploys a relay
container in place of the service: a minimal TCP forwarder (new relay/
directory, published as docker/compose-relay, overridable with
COMPOSE_RELAY_IMAGE for internal registries) joining the networks of the
services that depend on the provider service, aliased with the service
name. Consumers then use http://<service>:<port> as if the service were
a regular container.
The relay is a first-class project container — canonical name, standard
compose labels including config-hash (label-driven commands run without
the compose file keep seeing the service: ps, logs, stop, down) — plus
the com.docker.compose.relay label declaring its role:
- the reconciler already leaves provider services' containers alone, and
the relay's identity hash (image + routes) makes up idempotent: kept
when routes are unchanged, recreated otherwise;
- process-level commands (exec, cp) refuse a relay — there is no service
process in it to act on;
- the up monitor excludes relays from the containers whose termination
ends an attached up: they are long-lived infrastructure and would
otherwise keep 'up' waiting forever.
The example provider demonstrates the flow behind PROVIDER_DEMO_ENDPOINT
(a detached helper serving a fixed HTTP response), backed by an e2e
scenario asserting the compose-native address works and exec is refused.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
git init -b requires git >= 2.28; init followed by symbolic-ref names
the initial branch on any version. Branch() now returns the working
tree to main so successive calls cut from the same base instead of
stacking on the previous branch.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The remote loaders had almost no end-to-end coverage: one test consumed
an OCI artifact (config only, plus an up declined at the prompt) and
nothing exercised the git loader at all.
Git scenarios run against a throwaway repository served over the smart
HTTP protocol by an in-process server — git http-backend as a CGI per
httptest request: no daemon, no container, no fixed port. Smart HTTP is
a hard requirement: the loader ls-remotes the ref then shallow-fetches
the raw commit, which the dumb protocol cannot serve (no shallow
capability) and which needs uploadpack.allowAnySHA1InWant. Covered:
deploying from the default branch with repository-relative files,
selecting a #branch, selecting a #ref:subdir project.
OCI scenarios publish the fixture to a throwaway local registry (the
TestPublish pattern) then deploy from oci://: a full up consuming the
bundled env-file layer, and tag selection between two published
revisions.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
FromRemote switches a scenario's -f to a reference handled by compose's
remote loaders (git URL, oci:// artifact) with optional root flags such
as --insecure-registry, while the anchored testdata copy keeps serving
as the local content the remote is built from. ContainerEnv joins the
check vocabulary: the container-config environment is the natural
observable that a remote project's bundled files (env_file) were
consumed, whatever the source of the model.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
RequireEventuallyServiceState succeeded on the first NDJSON entry for
the service matching the expected state, without checking any
remaining entries. During a recreation window compose ps can list two
containers for the same (scale: 1) service simultaneously (old and
new); the old one could still read "running" while the new one is
"starting", passing the poll prematurely on stale state.
Track whether any entry for the service was seen, and only succeed
once every one of them matches -- fail (poll.Continue) on the first
mismatch instead.
(docker-agent review on #14228)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
