The rebuild path ran s.create then s.start on a project reduced to the
rebuilt services and their dependents. It now runs one plan with the
start phase scoped to that same subset: services outside it are left
alone, exactly as before, but ordering across recreation and restart
is carried by a single DAG.
Start-in-plan convergence, phase 1 (#14081).
Signed-off-by: Nicolas De Loof <[email protected]>
Detached up (Attach == nil) now runs one plan carrying creation and
the start phase, instead of chaining s.create with the second engine;
the per-dependency waits inherit --wait-timeout. The --wait barrier
stays a post-plan verification, extracted from s.start into a shared
waitStarted helper.
`compose start` (and every listener-less s.start call) goes through
the start-only plan profile: waits, pre_start hooks and starts over
the containers as they exist, never touching resources or recreating
diverged containers — preserving its long-standing semantics,
including the 'service X has no container to start' error when the
project has no containers at all.
The listener-driven s.start path remains for the interactive up until
the session moves to executor hooks (phase 2): it is now that path's
only caller.
converge takes *startPhaseOptions instead of a bool.
Start-in-plan convergence, phase 1 (#14081).
Signed-off-by: Nicolas De Loof <[email protected]>
Three capabilities the remaining consumers of the second engine need:
- startPhaseOptions.Services scopes which services get start nodes
(watch only restarts the rebuilt services and their dependents; the
rest of the project is left alone, as the legacy engine did by
starting a reduced project).
- WaitCondition nodes are deduplicated by (dependency, condition,
required): dependents sharing an edge share one polling node.
required stays in the key because Optional is a node property — a
required and an optional edge to the same dependency do not fail the
same way.
- startPhaseOptions.Only plans the start phase alone — the
`compose start` profile: no network/volume reconciliation, no
creation, recreation or removal; existing containers are started as
they are, whatever their config divergence.
Each capability is pinned by a plan-shape unit test (scope, dedup with
mixed required-ness, start-only including the diverged-not-recreated
guarantee).
Start-in-plan convergence, phase 1 (#14081).
Signed-off-by: Nicolas De Loof <[email protected]>
The progress-event stream is a de-facto public surface: the TTY
renderer, --progress plain (grepped by user scripts), --progress json
(consumed by tools) and SDK EventProcessors all read it. Converging
the two lifecycle engines without a written contract is how silent
divergences happen.
api.EventProcessor now documents the contract: ordering is per
resource (global interleaving is explicitly not guaranteed), the
typical per-resource progressions
(Creating→Created, Starting→Started, Waiting→Healthy|Exited, ...),
early termination through Error or Skipped-with-reason, and status
texts as stable identifiers.
An executor test records the stream through a real plan (wait node +
enriched start) and asserts the per-resource progressions — the same
kind of assertion the wider phase 1 will lean on.
Validated design decision for #14081 (event parity contract).
Signed-off-by: Nicolas De Loof <[email protected]>
waitDependencies reports an optional dependency's failure as one
Skipped event per container of the dependency, with the reason — and a
required condition failure as per-container Error events. The plan
walker's generic single Skipped on the wait node's resource ID would
have silently changed that granularity, exactly the kind of
divergence the event contract exists to prevent.
Wait failures now flow through reportWaitFailure, which emits the
historical per-container events (Skipped with reason when optional,
Error otherwise) before handing the error to the walker; the walker
skips its generic event for wait nodes.
Validated design decision for #14081 (event parity contract).
Signed-off-by: Nicolas De Loof <[email protected]>
The 'once per service, only when no replica is running' rule is
decided at plan time from the observed state — the placement of the
RunPreStart node stays a plan decision. But between observation and
execution a replica can start (the general observe-to-execute drift
window the plan design accepts): for pre_start that drift would re-run
hooks the rule says must not run. The operation now re-checks the
daemon at execution time and skips itself when a replica is already
running — plan-time placement, runtime guard.
Validated design decision for #14081 (start-candidates model).
Signed-off-by: Nicolas De Loof <[email protected]>
Prototype of #14081 phase 1 on its smallest consumer: Scale now runs
one create+start plan (s.converge with PlanStart) instead of chaining
s.create with the separate s.start engine — one DAG orders the whole
operation, and startService/waitDependencies are no longer involved.
create() keeps its exact behavior via converge(..., false).
Validated by the plan-shape unit tests and the full e2e scale family,
plus the pre_start/post_start/pre_stop hook suites as control groups.
Signed-off-by: Nicolas De Loof <[email protected]>
- OpWaitCondition polls the dependency's containers from the live view
(populated by earlier create nodes) until the declared condition
holds, the deadline expires, or the context ends — the plan-side
equivalent of one waitDependencies edge, with the same events
(Waiting/Healthy/Exited) and error messages.
- OpStartContainer, when it carries a Service (start-phase nodes),
performs the full service start: secret/config injection before
ContainerStart, post_start hooks after — mirroring
startServiceContainer. Bare operations (paused/dead restarts) keep
the plain start. Containers created by the plan are resolved through
the create node's result (CreateNodeID), the same mechanism rename
already uses.
- OpRunPreStart runs the service's pre_start hooks on the replica the
plan selected.
- Event emission handles start-phase nodes whose container does not
exist yet (named from the operation's target name).
Phase 1 groundwork for the start-in-plan convergence (#14081).
Signed-off-by: Nicolas De Loof <[email protected]>
Behind the new ReconcileOptions.PlanStart, the reconciler extends the
plan past container creation: per-dependency WaitCondition nodes
(service_started stays a plain DAG edge; required:false maps to
Optional nodes), a once-per-service RunPreStart node — its
'only when no replica is running' rule decided at plan time from the
observed state, on the lowest replica number — and one enriched
StartContainer node per non-running replica, chained after its create
(or reconnect) nodes, its dependencies' start nodes and its wait nodes.
Start candidates are collected while walking existing containers,
which surfaces a case the plan used to leave dangling: containers the
plan itself stops to recreate a diverged network were never restarted
by the plan — the second engine picked them up. With PlanStart they
are start candidates chained after their reconnects.
With PlanStart unset nothing changes: candidates are discarded and the
plan stops at creation, as before (all existing plan snapshots
unchanged). Covered by plan-shape unit tests: dependency wiring,
Optional propagation, and both pre_start planning rules.
Phase 1 groundwork for the start-in-plan convergence (#14081).
Signed-off-by: Nicolas De Loof <[email protected]>
The executor aborted the whole plan on any node failure, which rules
out modeling optional semantics — a dependency declared with
required: false must be reported as skipped, not fail the operation,
and its dependents must still run. This is a prerequisite for planning
the start phase (dependency-condition waits) inside the reconciler.
Operation gains Optional: at the walker level, a failing Optional node
emits a Skipped event plus a warning and completes successfully, so
dependent nodes proceed. It is documented against the pre-existing
BestEffort flag, which is narrower (one specific expected error,
tolerated inside the operation itself) — the two docs now
cross-reference each other so they cannot be confused.
Covered by an executor unit test asserting a failing Optional node
does not fail the plan and its dependent still executes.
Groundwork for the start-in-plan convergence.
Signed-off-by: Nicolas De Loof <[email protected]>
waitDependencies returned nil when its context expired: every polling
goroutine hit the ctx.Done branch and reported success, so the
'timeout waiting for dependencies' translation after eg.Wait() was
unreachable in practice, and up --wait / start --wait could succeed
silently while nothing was healthy. The error path was only ever taken
by a race, when a health probe happened to be in flight at expiry.
The ctx.Done branch now distinguishes the two reasons the context can
end: a deadline is exactly the failure this function is asked to
detect and is surfaced (callers translate it into their own messages:
'timeout waiting for dependencies', 'application not healthy after X');
a plain cancellation (user interruption) keeps returning nil so
Ctrl-C does not masquerade as a dependency failure — preserving the
behavior of the restart/run/up callers that pass no timeout.
Both paths are now pinned by unit tests; neither message was covered
before.
Part of #14074 (C) and groundwork for the start-in-plan convergence.
Signed-off-by: Nicolas De Loof <[email protected]>
The e2e suite runs with the Go default 10-minute test timeout (no
-timeout flag). Its cumulative duration on the slowest matrix leg
(standalone, oldstable, graphdriver) now reaches ~11 minutes of
in-binary time on slower runners, so the whole suite gets killed at
600s with whatever tests were in flight reported as failed — observed
repeatedly across unrelated PRs. Give the suite explicit headroom;
the surrounding CI job timeout still bounds the total.
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Nicolas De Loof <[email protected]>
Show an ENGINE column in `compose ps` default table output when the
com.docker.compose.engine label is present. Expose {{.Engine}} for custom
--format templates.
Rename api.EngineLabel to api.ContainerEngineLabel to avoid the
name collision with desktop.EngineLabel (com.docker.desktop.address).
Signed-off-by: Nick Sieger <[email protected]>
validateNavigationMenu validated nothing: it resolves the TTY /
COMPOSE_MENU / --menu precedence and mutates opts.navigationMenu —
rename to resolveNavigationMenu.
runScale duplicated setServiceScale's GetService/SetScale/write-back
dance inline; move the helper next to its natural home in scale.go and
use it from both callers.
Signed-off-by: Nicolas De Loof <[email protected]>
Inline small functions that were extracted from their single call site
without a real boundary to justify it — no reuse, no dedicated test, no
responsibility of their own — so each caller now tells its whole story
top-down:
- getExecTarget, attachContainer, logContainer: one-line trampolines to
getSpecifiedContainer / doAttachContainer / doLogContainer, which
other call sites already use directly
- removeImage: single-statement wrapper, unlike its removeVolume
sibling which has actual logic
- checkSelectedServices: named like a validation, actually a filter;
the subtle rule (an unknown service is only an error with an explicit
compose file) now reads where options.Services is rewritten
- prepareLabels: mutated the map it received while looking pure at the
call site; the label writes are now visible in getCreateConfigs
- setDefaultTarget: mutation-by-pointer of the loop copy, now visible
in the loop of injectFileReferences
- buildVolumeOptions/buildTmpfsOptions/buildImageOptions: nil-guard +
field copies; the buildMountOptions switch now shows side by side
what each mount type propagates (buildBindOption keeps real logic and
keeps buildMountOptions under the gocyclo limit)
- displayDryRunBuildEvent: was longer than its only caller
- hasMore: read like a predicate, was a one-line spinner restart
- escapeDollarSign: wrapped a single bytes.ReplaceAll
- extractEnvCLIDefined: replaced by the canonical compose-go helper
types.NewMappingWithEquals().ToMapping(), as run.go already does
- isPullPolicyValid: rebuilt the valid-values slice on every call; now
a package-level list checked at the call site
- viewFromStackList: projection now sits next to the render closure
that consumes exactly its three fields
No behavior change; single gocyclo threshold untouched.
Signed-off-by: Nicolas De Loof <[email protected]>
A running container may reference an image record that no longer
exists: under the containerd image store, `up --build` with identical
content moves the tag to a new index digest (provenance attestation
churn) and the daemon drops the old index the container was created
from — without compose recreating the container, by design (#13636).
`docker rmi -f` of a running container's image produces the same state.
`compose images` used to fail the whole listing on the resulting
NotFound. Degrade to what the container itself knows (image ID, and
repository/tag when the reference is not a raw ID) instead of failing.
Fixes#14014
Signed-off-by: Nicolas De Loof <[email protected]>
docker-agent-action v2.0.3 raised the review job's required permission
from actions:read to actions:write (cache delete for review-lock
release, feedback artifact management). GitHub refuses to start a
reusable workflow requesting more permissions than the caller grants,
so every PR Review run since the v2.0.3 bump ended in startup_failure
and docker-agent stopped launching automatically.
Signed-off-by: Guillaume Lours <[email protected]>
The 'builder does not support multi-arch' subtest expects the docker
driver to reject multi-platform builds, and skipped itself by checking
that buildx lists both linux/amd64 and linux/arm64. That heuristic
misses containerd-store hosts without binfmt emulation (plain CI
runners): only native platforms are listed even though the driver
happily cross-builds, so the build succeeds and the test fails with
'ExitCode was 0 expected 1'. Skip on the containerd image store
directly, detected from the daemon's DriverStatus.
Signed-off-by: Nicolas De Loof <[email protected]>
Several image-identity bugs only manifest on one image store backend:
the graphdriver and containerd stores report different digest kinds and
resolve them differently (#14005, #14007, #14014 all reproduce only
under the containerd store, which no CI job covered). Cross the stable
e2e jobs with a store axis (graphdriver/containerd) so both backends
are exercised in plugin and standalone modes; oldstable jobs stay on
the default graphdriver.
Signed-off-by: Nicolas De Loof <[email protected]>
Regression test for #14005, pulled-image scenario: with the source image
of a type=image volume already present in the local store, compose used
to rewrite the mount source to a digest the daemon can't resolve as a
mount source under the containerd image store (No such image).
TestImageVolume only exercises this path when a previous test happens to
have left the image locally; pre-pulling makes it deterministic. Also
asserts a second unchanged up doesn't recreate the service.
The test fails on main under the containerd image store and passes with
the fix from #14011.
Signed-off-by: Nicolas De Loof <[email protected]>
Regression test for #14007: with the containerd image store, a local
multi-platform image holding the requested non-native variant must be
used as-is by the default missing pull policy — compose used to inspect
the image without a platform, compare the host variant's platform fields
to the requested one, conclude the image is missing and try to pull the
(unpublished) tag.
The test fails on main and passes with #14011, whose platform check
resolves the requested platform against the locally available manifests.
Signed-off-by: Nicolas De Loof <[email protected]>
Compose manipulates two digest kinds that must never be conflated:
the platform-specific content digest (localContentDigest) is ONLY an
identity to compare a running container with a fresh build/pull, while
pinning image references in a reproducible compose model (publish /
config --resolve-image-digests) must keep the registry descriptor
digest — the multi-platform index — through ImageDigestResolver, or
the published file would be bound to the platform of whoever resolved
it. Document the distinction on both producers and lock it with a test
that fails if the resolver ever goes through a local image inspect.
Signed-off-by: Nicolas De Loof <[email protected]>
- dry-run up with a missing image must not hit the real daemon
- create twice with a non-native DOCKER_DEFAULT_PLATFORM is idempotent
- two services sharing an image with mixed platform pinning get their
own platform's digest label, idempotently, whatever pull ordering
- explicit pull refreshes ahead of a daily refresh window
Signed-off-by: Nicolas De Loof <[email protected]>
- pullRequiredImages resolves each distinct pulled image once, after
all pulls completed, for the host default platform — the exact way
getLocalImagesDigests resolves already-local images. Resolving from
each pull's goroutine, for the pulled platform, let the recorded
digest depend on pull completion order when several services pull
the same tag for different platforms (last-writer-wins), and made
the first up after a pull recreate containers. Platform-pinned
services keep getting their own platform's digest from
serviceImageDigest. In dry-run nothing was actually pulled, so the
local inspect is skipped — it reached the real daemon through a
DryRunClient dispatch that no longer matched and failed with 'No
such image'.
- explicit 'compose pull' treats daily/weekly/every_N windows as due:
it is the only way to force a refresh ahead of the window.
- 'compose pull' no longer silently skips services declaring both
provider: and image:.
- pre_start hook images go through the same mustPull interpreter on the
up path as on the pull path, so refresh windows apply consistently.
- matchLocalManifest falls back to the inspect's flat platform fields
when the lone available manifest carries no ImageData (locally built
images), instead of reporting the platform unsatisfied and pulling a
possibly local-only image.
- scale resolves DOCKER_DEFAULT_PLATFORM without validating
build.platforms: a conflict on a service that isn't being built must
not abort the command. run needs nothing: createOptions.Apply
resolves and validates platforms on its path already.
- resolveImageVolumes documents the accepted one-time-recreate and
retag-race tradeoffs.
Signed-off-by: Nicolas De Loof <[email protected]>
compose pull switched on the raw pull_policy string: daily/weekly/every_N
never matched a case and fell through to an unconditional re-pull, and
the hook-image loop was a second interpreter that ignored the refresh
window entirely. Delegate the decision to the exact interpreter the up
path uses (mustPull), with hook images routed through the same decision
(build mapped to missing — a hook image can't be built as a fallback).
Two deliberate differences with up are kept and documented in
shouldPullImage: a service without an explicit pull_policy is always
refreshed (skipping it would turn an explicit compose pull into a no-op
once images exist), and a present latest tag is still refreshed under
missing/if_not_present — the tag is expected to move, and triggering the
pull lets the daemon negotiate with the registry, a manifest check with
no download when the local image is already current.
User-visible change (changelog): compose pull now honors
daily/weekly/every_N refresh windows instead of always re-pulling.
Signed-off-by: Guillaume Lours <[email protected]>
scale and run were the only container-creating commands that never
called applyPlatforms, yet both go through the regular create path and
its config-hash comparison (run for the dependencies it starts). With
DOCKER_DEFAULT_PLATFORM set, they hashed an empty service Platform where
up had hashed the resolved one, so every invocation recreated the
affected containers. run's project preparation is extracted to a helper
to keep runCommand under the complexity threshold.
No unit test: neither command has a test harness and the fix is the one
missing call, aligned on create/watch; the config-hash equality is
covered by the reconciler tests.
Signed-off-by: Guillaume Lours <[email protected]>
Image identities recorded for staleness detection were produced by
several independent paths yielding different digest kinds for the same
image: the platform check compared flat inspect fields while the digest
picked a manifest with the host matcher (never the service's pinned
platform), a wrong-platform summary just discarded still leaked its
digest into the label, bake substituted digests host-side in batch, and
the classic builder recorded the raw build-stream ID as-is. Any of those
mismatches makes the next up see a phantom image change and recreate
containers.
Converge every producer on one selection (matchLocalManifest /
localContentDigest): the shared parallel inspect feeds both the digest
and the platform check, platform-pinned services resolve THEIR platform's
manifest in-process (no extra API call), and both builders route through
canonicalBuiltDigest. Registry-only builds (push-only, multi-platform
without load) keep the builder-reported digest — volatile but honest, an
actual rebuild is still detected, where a stable placeholder would hide
real image changes. ensureImagesExists' final loop becomes the label's
single writer so the pinned resolution can't be overwritten, superseded
only by pull/build results already platform-resolved by their producers —
and when a pull or build refreshed the shared entry mid-run (a digest
resolved for whichever service triggered it), a service pinned on another
platform re-resolves its own with one extra inspect, in that case only.
With every producer converged, TestUpIdempotentContainerdStore is
un-skipped here.
Signed-off-by: Guillaume Lours <[email protected]>
pullServiceImage returned the pulled image's raw inspect ID, while
getImageSummaries resolves already-local images through contentDigest
(the platform image-manifest digest). Both values feed the
com.docker.compose.image label that mustRecreate compares to detect
image changes, so the two paths disagreeing made the first 'up' after
the pulling 'up' see a phantom image change and recreate every
container once, with no change anywhere. Under the containerd image
store a tag@digest reference triggers this: the raw inspect ID is the
index digest, while contentDigest picks the platform manifest digest.
Resolve the pulled image through the same manifests-aware inspect and
contentDigest call getImageSummaries uses, so both sides of the
staleness comparison speak the same scheme.
Verified against a fresh docker:dind (29.7.0, containerd store) with a
tag@digest service: unpatched v5.4.0 recreates the container on the
second 'up'; with this fix the container survives repeated 'up' runs.
Existing behavior is preserved for engines without manifest support
(contentDigest falls back to the plain ID).
(Squashed with the follow-up lint cleanup from the same PR.)
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Max Malm <[email protected]>
Signed-off-by: Guillaume Lours <[email protected]>
The e2e suite only ran on graphdriver daemons, where the different kinds
of image digests coincide — the blind spot that let #13636, #13998 and
#14005 through. Add one matrix entry enabling the containerd image store,
plus TestUpIdempotentContainerdStore: two consecutive `up` runs with no
change must not recreate any container. The test is red on this
configuration (the com.docker.compose.image label is written from the
index digest on the pulling run, then compared against the per-platform
manifest digest on the next run) and skipped until the next commit
resolves the pull-path digest.
Signed-off-by: Guillaume Lours <[email protected]>
With the containerd image store, ImageSummary.ID holds the digest of the
platform-specific manifest so ServiceHash stays stable across attested
rebuilds (see contentDigest). resolveImageVolumes reused that same value
as the `type: image` mount Source, but the daemon only resolves a mount
Source by name/tag or top-level image ID, not by manifest digest — so
`compose up` failed with "No such image" whenever the volume's source
image was already present locally (always for a built image; on a
second run for a pulled one).
Keep Source as the resolved image name, and track the digest separately
via a new com.docker.compose.image-volume-digest label so mustRecreate
can still detect a rebuilt/updated source image independently of Source.
Fixes#14005
Signed-off-by: Ricardo Branco <[email protected]>
Signed-off-by: Guillaume Lours <[email protected]>
`docker compose bridge transformations create` panicked with an index
out of range when invoked without its PATH argument, as RunE indexed
args[0] without any validator. Add cli.ExactArgs(1) so the CLI reports
a proper usage error instead.
Also add cobra.NoArgs to `bridge convert` and `bridge transformations
list`, which silently ignored stray arguments.
Signed-off-by: Guillaume Lours <[email protected]>
The disclosure file added by AI agents states the change may not have
been independently reviewed or tested by its human submitter. It is
meant as a checkpoint, not something to merge: fail a PR check while
the file is present, telling the contributor to review the code their
agent produced and remove the file to confirm that manual review.
Signed-off-by: Nicolas De Loof <[email protected]>
`up --abort-on-container-exit` tore down the project when a one-off
container created by `docker compose run` exited. The container list
built in monitor.Start already excludes one-off containers via
oneOffFilter(false), but the event subscription did not, and the only
guard on incoming events is the service label, which a one-off
container inherits from the service it was run from.
Regression since v2.39.0 (last unaffected release is v2.38.2), and a
recurrence of docker/compose-cli#1955, fixed there by
docker/compose-cli#1987.
Signed-off-by: Branislav Osif <[email protected]>
CI runs the tests as root, where the unreadable-directory test is always
skipped. Extract the WalkDir callbacks and inject the watch registration
so a synthetic permission error can drive those branches instead.
Signed-off-by: Endika Iglesias <[email protected]>
Containers are created from a project whose service environment is
resolved (env_file merged into environment), but since v2.22.0 `config
--hash` skipped that resolution, so hashes diverged from the
com.docker.compose.config-hash label for services using env_file.
Resolve the environment of the hashed services only, so a broken
env_file or platforms on an unrelated service still doesn't prevent
hashing, and honor --no-env-resolution as an escape hatch.
Fixes#14001
Signed-off-by: Guillaume Lours <[email protected]>
pruneDanglingImagesOnRebuild probed the name-keyed built-images map with
an image ID, so the spare-check never matched and every dangling image
carrying the project label was removed on each rebuild with --prune —
not just the superseded ones. Match dangling IDs against the map's
values (the freshly built image IDs) instead.
Signed-off-by: Guillaume Lours <[email protected]>
The official nginx images set STOPSIGNAL to SIGQUIT which dumps core.
Set it to SIGTERM to avoid dumping core on e2e tests when containers
running "sleep infinity" are stopped.
Signed-off-by: Ricardo Branco <[email protected]>