Fix image digest reconciliation - #14011
Conversation
… digest 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 docker#14005 Signed-off-by: Ricardo Branco <rbranco@suse.de> Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The e2e suite only ran on graphdriver daemons, where the different kinds of image digests coincide — the blind spot that let docker#13636, docker#13998 and docker#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 <glours@users.noreply.github.com>
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 <noreply@anthropic.com> Signed-off-by: Max Malm <benjick@dumfan.net> Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
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 <glours@users.noreply.github.com>
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 <glours@users.noreply.github.com>
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 <glours@users.noreply.github.com>
|
Thanks for picking this up and generalizing it — the single-producer framing is much better than what I had, and the containerd CI job is the part I'm most glad to see. I went looking for whether the class is fully closed and I think one case remains: The pull side resolves an effective platform ( platform := service.Platform
if platform == "" {
platform = defaultPlatform
}
...
id, _, err := s.inspectLocalContent(ctx, service.Image, platform)The local side doesn't: I reproduced it against a fresh services:
app:
image: alpine:3.20
command: ["sleep", "infinity"]
Run 2 is stable because amd64 is the only available manifest at that point, so Worth noting the container in run 3 is still running x86_64 ( To be clear this isn't a regression from this PR — the same trace on the base commit recreates at run 2 and run 3 ( If you agree it's in scope, it looks like a small fix: Keeping up the tradition, my dog: |
4 participants
What I did
Compose records image identities for staleness detection (the
com.docker.compose.imagelabel compared bymustRecreate) through several independent code paths — pull, bake, classic builder, already-local inspect — and they didn't all produce the same kind of digest for the same image: top-level index digest, per-platform manifest digest, or config digest, depending on the engine (graphdriver vs containerd image store), the API version, the builder, and how the image arrived locally. Whenever two runs resolved the same image through different paths, the comparison failed and containers were recreated with no actual change. This mismatch class is behind #13636, #14005, and the phantom-recreate-after-pull fixed by #13998 — plus several latent cases (platform-pinned services resolved with the host platform, wrong-platform digests leaking into labels, bake push-only builds recreating on everyup).This PR fixes the class, not just the instances:
matchLocalManifest/localContentDigest): every path — pull, bake, classic builder, local inspect — resolves the recorded identity the same way. Digest selection and the platform-mismatch check now share the same manifest resolution, so the digest recorded and the platform validated always refer to the same manifest.ensureImagesExistsis the only writer ofcom.docker.compose.image. Platform-pinned services get the digest of their platform's manifest, resolved in-process from the already-fetched inspects (zero extra API calls in steady state).type: imagevolumes mount by resolvable name and track their source-image digest in a dedicatedcom.docker.compose.image-volume-digestlabel (fixes the [BUG] type=image volumes fail with "No such image" when the source image is already present locally #14005 regression).TestUpIdempotentContainerdStorelocks the invariant: two consecutiveupruns with no change must not recreate anything.scaleandrunnow resolveDOCKER_DEFAULT_PLATFORMlikeupdoes (they recreated containers on every invocation otherwise), andcompose pullinterpretspull_policythrough the same interpreter asup(two deliberate, documented differences remain: an unset policy always refreshes, and a presentlatesttag is still refreshed undermissing).This PR supersedes and includes #13998 (@benjick) and #14006 (@ricardobranco777) — both commits are integrated with their original authorship preserved. Thanks to both for the investigations that narrowed this down.
User-visible changes
compose pullnow honorsdaily/weekly/every_Nrefresh windows instead of always re-pulling.upafter upgrading: label values change kind (and image-volume users transition from digest-as-mount-source to the new label). Subsequent runs are stable.Each commit is independently CI-green and reviewable on its own: image-volume mount fix → containerd-store CI job (test skipped) → pulled-image content digest (test enabled) → canonical producer/single writer → scale/run platform resolution → pull policy alignment.
Related issue
Fixes #14005
Supersedes #13998, #14006
(not mandatory) A picture of a cute animal, if possible in relation to what you did
