provider services take part in the image phase, with image distribution on demand by ndeloof · Pull Request #14252 · docker/compose

ndeloof · 2026-09-22T07:49:17Z

What this PR does, in one sentence

Provider-backed services take part in the image phase: compose hands the provider the identity of the service image and — on request — streams it the image bytes, so a build: or image: declared on a provider service actually reaches the provider's runtime.

Context

A provider service can declare build: today: compose builds the image… into the local daemon, where the provider never sees it. Providers whose runtime lives elsewhere (a sandbox VM with its own daemon, a cloud backend) have no way to receive a locally-built image, and no say in the image phase at all — docker compose pull skips them entirely. The only workarounds are out-of-band: push to a registry the provider can reach, or manual docker save | load plumbing.

What the PR brings

Providers opt in by declaring a pull block in their metadata output — the presence of the command block is the declaration of support, the same convention stop already uses (now documented as the general rule). For opted-in providers, compose invokes <provider> compose pull during the image phase of up (after any build) and on docker compose pull, passing:

the image identity (--image) and the state of the local daemon cache (--digest/--created, when present) — facts to persist as bookkeeping keys, not instructions: digest as identity test, created as the ordering fallback for backends that cannot preserve digests (a rebuilt local image must be detectable as newer even by a backend that transforms what it ingests);
two verdicts compose alone can compute, so providers never re-implement its arbitration: --source (local: the daemon's image is the desired state — build-only service, pull_policy: build, or just built by this run; registry: resolve upstream, which covers the build-as-CI-recipe workflow where consumers never build) and --policy (missing on up: a usable version suffices; always on pull: ensure freshness).

When the provider needs the local bytes, it sends a get-image request and compose answers on its stdin with one JSON line then the image tar as an HTTP/1.1 chunked body (RFC 9112): length-prefixed blocks need no in-band delimiter in binary data, the zero-length chunk marks a complete transfer (a truncated stream must be discarded), and any language's stock chunked reader consumes it. Export failures are announced in-band so the provider is never left waiting.

Guardrails: providers without the metadata block see zero behavior change; the wire format is locked by unit tests (byte-identical round-trip on delimiter-hostile payloads, error announce, end-to-end against a fake provider process); an e2e scenario drives the full path through the example provider (up builds and streams under the local verdict, compose pull re-invokes under the freshness contract). The PR also fixes a pre-existing gap the scenario exposed: a project made only of provider services failed the start phase with "no container to start" — softened for provider services only, the endpoint-relay still going through the regular start path.

Why this is the right next brick

This is deliberately the provider-initiated design: the image phase is where compose already makes images available to where services run, and the digest/created negotiation gives idempotence for free (no re-streaming on every up). A side-channel transport for very large images and the pull-fails→build fallback chain can layer on later without breaking the announced-answer protocol (encoding field).

🤖 Generated with Claude Code

docker-agent

Assessment: 🟡 NEEDS ATTENTION

Lower-confidence findings (not posted inline)

[medium→low] pkg/compose/plugins.go:395 — get-image request handled for any plugin command without command-context guard (confidence: weak 52/100)

codecov · 2026-09-22T09:15:37Z

Codecov Report

❌ Patch coverage is 78.90625% with 27 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/compose/plugins.go	78.12%	10 Missing and 4 partials ⚠️
pkg/compose/provider_images.go	78.72%	6 Missing and 4 partials ⚠️
pkg/compose/build.go	80.00%	1 Missing and 1 partial ⚠️
pkg/compose/run.go	0.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

docker-agent

This review covers only the commits pushed since d7fc6a1 (the previous completed review).

Assessment: 🟢 APPROVE

The incremental diff (92 lines across 3 files) makes two targeted fixes:

plugins.go — handlePluginMessage gains a command parameter so get-image requests are rejected with a protocol error when sent outside the pull command, preventing image-export stalls in up/down/stop. The command string is passed by value and is thread-safe.
pull.go — ensureProviderImages now runs unconditionally before the err != nil check (fixing the previously reported finding): a regular-service pull failure no longer silently skips provider image distribution. When both fail, errors.Join(err, providerErr) surfaces both. Under IgnoreFailures=true, providerErr is suppressed symmetrically with regular service pull errors — consistent with the semantics documented in the prior review thread resolution.
provider_images_test.go — New unit test TestExecutePlugin_GetImageRefusedOutsidePull validates the protocol restriction. The assert.ErrorContains substring ("get-image is only supported during the pull command") correctly matches the actual error message from plugins.go ("invalid message from plugin: get-image is only supported during the pull command").

No new bugs found in the + lines.

docker-agent

Assessment: 🟡 NEEDS ATTENTION

docker-agent

Assessment: 🟡 NEEDS ATTENTION

Provider image-phase integration is well-structured overall and the chunked-streaming protocol is sound. Two correctness issues were found in the new code.

docker-agent

Assessment: 🟡 NEEDS ATTENTION

docker-agent

Assessment: 🟢 APPROVE

docker-agent

Assessment: 🟢 APPROVE

The provider image distribution implementation is solid. The chunked-transfer protocol, mutex discipline, and error propagation are all well-handled. The two drafter hypotheses about a trailing CRLF mismatch between ChunkedWriter.Close() and the documented protocol were investigated and found to be false positives: Go's httputil.ChunkedWriter.Close() writes only "0\r\n" (not "0\r\n\r\n") — confirmed from the Go 1.26.3 source — and the code comment and docs/extension.md are correct. ChunkedReader returns EOF when it reads the zero-length chunk without consuming any additional bytes, so the wire format is internally consistent and the test assertion ("no byte may follow the terminating zero chunk") is valid.

Lower-confidence findings (not posted inline)

[low] pkg/compose/pull.go:~86–93 — ensureProviderImages unreachable when pullServiceImages/pullHookImages returns a fatal scheduling error (unverified low severity)
The comment at line 102 states providers run "even when a regular pull failed", which is accurate for per-image pull failures accumulated in p.pullErrors. However, if pullServiceImages or pullHookImages returns a non-nil scheduling error (e.g. a shouldPullImage parse failure on a bad pull_policy value), the early-exit at line 93 skips ensureProviderImages entirely. In practice this edge case only triggers on configuration errors that would abort the command anyway, so real-world impact is minimal.
[low] pkg/compose/provider_images.go:97–107 — providerImageSource always returns "local" for build-only services during compose pull --pull=always (unverified low severity)
For a service with build: declared and no image: name, providerImageSource always returns providerImageSourceLocal regardless of whether the image was actually rebuilt in this run. On compose pull, built is nil, so justBuilt is always false — and since service.Image == "", line 101 returns "local". This is arguably correct design (build-only services have no registry authority), but a provider receiving source=local, policy=always may re-ingest the same unchanged local image on every compose pull, giving false freshness signals. A brief doc comment clarifying this intent would help provider authors.

docker-agent

Assessment: 🟢 APPROVE

The image-distribution feature is well-structured. The chunked HTTP/1.1 streaming protocol is correct, the stdinMu contract is intentionally documented, and the errgroup non-canceling pattern is appropriate. The prior review findings (mutex duration, mid-transfer abort via stdin.Close, error propagation in pull.go) have all been addressed in the current code.

Lower-confidence findings (not posted inline)

These low-severity observations were drafted but not formally verified (low findings skip the verification step). They are surfaced here for author awareness only.

[low] docs/examples/provider.go:155 — error message omits announce.Error when compose sends an error JSON
When json.Unmarshal succeeds but announce.Error != "" (compose sent an error response), the emitted message is image stream unavailable: <line> (err <nil>) — the server-reported reason in announce.Error is not included. As example/documentation code this has no production impact, but would mislead readers implementing their own provider. A clearer message would be: image stream unavailable: <announce.Error>.
[low] pkg/compose/plugins.go:361,368 — announce write failures silently discarded on error paths in streamImageTo
On the invalid platform and ImageSave error paths, _ = announce(...) discards any write failure. If the provider pipe is already broken at that point, the provider receives EOF without an error JSON line. The impact is minimal: the provider already sees EOF and discards, and the original error is still returned to the caller and logged. No hang or data corruption is possible.

docker-agent

Assessment: 🟢 APPROVE

Lower-confidence findings (not posted inline)

[low] pkg/compose/pull.go:110 — ensureProviderImages nil return appended to pre-allocated pullErrors slice with leading nil slots (confidence: not scored — low severity, not verified)

pullErrors is initialized as make([]error, len(project.Services)) — a slice of N nil entries where runServicePull writes by index. The provider error is appended after those N entries. errors.Join skips nils so there's no user-visible impact today, but a future caller assuming pullErrors[i] corresponds to project.Services[i] would find the provider error at index len(project.Services) outside that mapping.
[low] pkg/compose/plugins.go:476 — stdin.Close() may be called twice concurrently on the non-processExited error path (confidence: not scored — low severity, not verified)

When streamImageTo fails after the success announce is sent, the GetImageType goroutine calls stdin.Close() while holding stdinMu. On the non-processExited path the executePlugin defer also calls stdin.Close(). Go's os.File.Close() is thread-safe so there's no crash, but a future refactor making the WriteCloser less robust could surface the double-close. A sync.Once would make the intent clearer.
[low] pkg/compose/provider_images.go:74 — providerImageSource returns local for a build-only service on docker compose pull when no local image may exist (confidence: not scored — low severity, not verified)

When docker compose pull is invoked (built=nil), a provider service with a build: directive but no image: field gets --source=local with no --digest/--created cache facts (because the image was never built). A provider implementing the documented contract might then issue get-image for a ref that doesn't exist in the local daemon, receiving an error. Consider returning providerImageSourceRegistry for build-only services when built is nil, or explicitly documenting that --source=local with no cache facts means "no local copy exists."

glours

Reviewed the provider image-distribution mechanism (the new pull lifecycle command + chunked get-image transfer). Requesting changes on 3 points below, each with a concrete minimal fix. Everything else checked out correctly under direct tracing: the providerImageSource truth table against the 5 reachable combinations, goroutine/stdin lifecycle (no leak, no mutual deadlock between the stdout read loop and the image-streaming goroutine), event-ID disjointness, SkipProviders scoping (only used by watch.go's rebuild path), and the pull.go error-joining logic. Build (go build ./...) and the existing unit tests pass against d10d7ea1b, but none of them exercise the 3 gaps below, which is why they weren't caught.

…ution Providers opt into image distribution by declaring a pull block in their metadata — the presence of the command block is the declaration of support, generalizing the stop precedent. During its pull command a provider can request the service image from the local daemon with a get-image message (optionally platform-narrowed); compose answers on stdin with one image-stream JSON line then the tar as HTTP/1.1 chunked data (RFC 9112): length-prefixed blocks need no in-band delimiter in binary data, the zero-length chunk marks a COMPLETE transfer, and any stock chunked reader consumes it. There is deliberately no trailer nor final CRLF after the zero chunk — a stock reader stops there without consuming further bytes, so the next stdin answer starts clean for a provider requesting several images. An export failure is announced in-band through the error field so the provider is never left waiting; a failure after the announce closes the answer channel so the truncation is observable as EOF mid-chunk instead of a stream nobody will finish. The stream is exclusive on stdin for its whole duration — the framing invariant behind the lock. get-image is scoped to the pull command: emitted during up, down or stop it is a protocol error rather than served, so an image export can never stall another lifecycle command. Message dispatch moves out of executePlugin into handlePluginMessage, which also keeps the function under the complexity threshold. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

ensureImagesExists and docker compose pull now give provider-backed services their turn: compose invokes the provider's pull command with the image identity and the state of the local daemon cache (--image/--digest/--created), plus two verdicts compose alone can compute so providers never re-implement that arbitration: - --source: the authority for this invocation — local (the daemon's image is the desired state: build-only service, pull_policy build, or an image the current run just built) or registry (resolve the reference upstream — including the CI workflow where build is only the recipe used to publish the image consumers pull); - --policy: missing on the up path (a usable version suffices) or always on compose pull (ensure freshness of the authority). digest/created describe the cache, they are not instructions: the provider persists them as the bookkeeping keys of what it ingested — digest as identity test, created as the ordering fallback for backends that cannot preserve digests. Providers without the metadata block are skipped: behavior unchanged. Providers are independent, so their pulls run without fail-fast cancellation — the first failure no longer kills its siblings mid-run and mangles their reports into context-canceled noise; every provider runs to completion and every failure is reported, prefixed by service. On the pull path the provider phase runs unconditionally — skipping it because an unrelated service failed to pull would silently leave provider runtimes stale — and its error is a per-service pull error, following their exact regime: reported with them, suppressed by IgnoreFailures. A fatal errgroup error is reported joined with the accumulated per-service context instead of dropping it. The provider runs under the root context: the errgroup context is canceled once Wait returns and would kill the provider process on the spot. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

startService errored with 'no container to start' whenever the project-wide container listing came back empty, before the per-service filtering that lets provider services pass through. A project whose services are all provider-backed legitimately reaches the start phase with zero containers. The empty-list error is now softened for provider services only — a relay container deployed for published endpoints still goes through the regular start path. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

extension.md gains the Image distribution section: invocation contract (--image/--digest/--created/--source/--policy), the get-image request and its chunked image-stream answer (zero-chunk terminated, stdin exclusivity and drain-before-next-answer contract), the two-branch decision guidance (digest as identity, created as ordering fallback, reproducible builds caveat), and the support-by-presence metadata convention is now stated as the general rule. The example provider implements pull end to end — request, stock chunked reader, hash recorded at PROVIDER_PULL_MARKER — and the e2e scenario locks the whole path: up builds the service image and streams it to the provider under the local verdict, compose pull re-invokes it under the freshness contract. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

A service declaring only build: (no image:) answered get-service-config with an empty "image" field: service.Image is the YAML-declared value, and compose-go never fills it in for this case (every other caller of GetImageNameOrDefault in this package exists precisely because of that). The provider only ever sees this response, never the model compose builds internally, so an sbx-style provider building its own runtime image rejected the service outright with "defines no image". The response now carries the resolved name (api.GetImageNameOrDefault) instead - the same one the image phase already built and tagged by the time a provider asks. The build directive itself is cleared before marshaling: a provider has no builder to run it against, only an image identity to run. Pinned by a regression test using the existing fake-provider harness, verified by ablation to fail on either half of the fix reverted. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

Three fixes from glours' review: - ensureProviderImages now skips a provider service declaring neither image: nor build: (legal for a pure resource provider, e.g. one backing a remote Amazon RDS instance). Without the guard, such a service reached pull with a fabricated "<project>-<service>" reference (GetImageNameOrDefault) under source=registry (providerImageSource treats Build == nil as registry-authoritative) -- a name that was never real. Already reachable through existing e2e fixtures (TestDependsOnMultipleProviders' provider1/provider2), only a no-op today because the demo pull() short-circuits on an unset env var, not because of any guard here. - localImageFacts now resolves --digest through the manifest-aware localContentDigest instead of the raw ImageInspect ID. Under BuildKit provenance attestations, the raw ID covers the attestation manifest too and changes on every build even when the runnable image content didn't (docker#13636), which would make the provider re-sync on every up for no reason -- defeating docs/extension.md's own "identity test" contract for --digest. Threads service.Platform through from runProviderPull so the resolved manifest matches what the provider will actually run. - streamImageTo now distinguishes a failure before the success announce (invalid platform, ImageSave error -- a clean, well-formed error line, channel intact) from one after it (the chunked body truncated, channel unresynchronizable) via a new streamAbortedError type. Only the latter closes stdin; the provider protocol supports several sequential get-image requests per pull invocation, and closing stdin on a clean pre-stream error was killing any other independent request in the same invocation for no reason. Each fix is pinned by a regression test, verified by reverting the fix locally and confirming the new test catches it. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

docker-agent

Assessment: 🟡 NEEDS ATTENTION

docker-agent found that streamImageTo returned a plain error (channel intact) when the announce write itself failed -- for the success announce, and for the error announce on a platform-parse/ImageSave failure. In both cases the provider never received anything to read and was left blocked on ReadString, while the caller, seeing a plain error, never closed stdin either -- a deadlock. Both paths now return streamAbortedError when the announce write fails, matching the existing mid-stream/close failure handling so the caller's errors.As check closes stdin and unblocks the provider. Pinned by two new regression tests using a writer that always errors, verified by reverting the fix locally and confirming both fail with a plain error instead of streamAbortedError. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

docker-agent

This review covers only the commits pushed since 220e69341281d71b5cf5bd8828a4422e2d6ba99a.