From 8113bad42ed73779bbe860e965019b20b118ae17 Mon Sep 17 00:00:00 2001 From: Ricardo Branco Date: Wed, 5 Aug 2026 21:42:09 +0200 Subject: [PATCH 1/6] fix(build): resolve image volumes to a mountable name, not a manifest digest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Guillaume Lours --- pkg/api/labels.go | 5 ++++ pkg/compose/build.go | 44 ++++++++++++++++++++++------------- pkg/compose/observed_state.go | 14 ++++++----- pkg/compose/reconcile.go | 3 +++ 4 files changed, 44 insertions(+), 22 deletions(-) diff --git a/pkg/api/labels.go b/pkg/api/labels.go index 3a0f684b98b..bc9d1f4a1b9 100644 --- a/pkg/api/labels.go +++ b/pkg/api/labels.go @@ -47,6 +47,11 @@ const ( SlugLabel = "com.docker.compose.slug" // ImageDigestLabel stores digest of the container image used to run service ImageDigestLabel = "com.docker.compose.image" + // ImageVolumeDigestLabel stores the content digest of each `type: image` + // volume's source image, as "target=digest" pairs joined by ",", so + // mustRecreate can detect a rebuilt/updated source image independently of + // the mount Source (which must stay a resolvable name, not a digest). + ImageVolumeDigestLabel = "com.docker.compose.image-volume-digest" // DependenciesLabel stores service dependencies DependenciesLabel = "com.docker.compose.depends_on" // VersionLabel stores the compose tool version used to build/run application diff --git a/pkg/compose/build.go b/pkg/compose/build.go index 2a861659922..e38be84ef80 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -19,6 +19,7 @@ package compose import ( "context" "fmt" + "sort" "strings" "time" @@ -167,25 +168,36 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. } func resolveImageVolumes(service *types.ServiceConfig, images map[string]api.ImageSummary, projectName string) { + var digests []string for i, vol := range service.Volumes { - if vol.Type == types.VolumeTypeImage { - imgName := vol.Source - if _, ok := images[vol.Source]; !ok { - // check if source is another service in the project - imgName = api.GetImageNameOrDefault(types.ServiceConfig{Name: vol.Source}, projectName) - // If we still can't find it, it might be an external image that wasn't pulled yet or doesn't exist - if _, ok := images[imgName]; !ok { - continue - } - } - if img, ok := images[imgName]; ok { - // Use Image ID directly as source. - // Using name@digest format (via reference.WithDigest) fails for local-only images - // that don't have RepoDigests (e.g. built locally in CI). - // Image ID (sha256:...) is always valid and ensures ServiceHash changes on rebuild. - service.Volumes[i].Source = img.ID + if vol.Type != types.VolumeTypeImage { + continue + } + imgName := vol.Source + if _, ok := images[vol.Source]; !ok { + // check if source is another service in the project + imgName = api.GetImageNameOrDefault(types.ServiceConfig{Name: vol.Source}, projectName) + // If we still can't find it, it might be an external image that wasn't pulled yet or doesn't exist + if _, ok := images[imgName]; !ok { + continue } } + img, ok := images[imgName] + if !ok { + continue + } + // The daemon only resolves a `type=image` mount Source that is a name/tag + // or a top-level image ID, not a per-platform manifest digest (which is + // what ImageSummary.ID holds to stay stable across attested rebuilds, see + // contentDigest()). Keep Source as the resolved name so mounting always + // works, and track the digest separately so mustRecreate can still detect + // a changed source image. + service.Volumes[i].Source = imgName + digests = append(digests, vol.Target+"="+img.ID) + } + if len(digests) > 0 { + sort.Strings(digests) + service.CustomLabels.Add(api.ImageVolumeDigestLabel, strings.Join(digests, ",")) } } diff --git a/pkg/compose/observed_state.go b/pkg/compose/observed_state.go index d2e187e0b67..aaf44269a34 100644 --- a/pkg/compose/observed_state.go +++ b/pkg/compose/observed_state.go @@ -103,12 +103,13 @@ func (s *ObservedState) selectVolume(key, desiredName string) (ObservedVolume, [ // ObservedContainer holds the relevant state extracted from a running or stopped // container, with label values pre-parsed for efficient comparison. type ObservedContainer struct { - ID string - Name string - State container.ContainerState // "running", "exited", "created", "restarting", etc. - ConfigHash string // label com.docker.compose.config-hash - ImageDigest string // label com.docker.compose.image - Number int // label com.docker.compose.container-number + ID string + Name string + State container.ContainerState // "running", "exited", "created", "restarting", etc. + ConfigHash string // label com.docker.compose.config-hash + ImageDigest string // label com.docker.compose.image + ImageVolumeDigest string // label com.docker.compose.image-volume-digest + Number int // label com.docker.compose.container-number // ConnectedNetworks maps network IDs found in the container's network // settings. Key is the network name as seen by Docker, value is the @@ -333,6 +334,7 @@ func toObservedContainer(c container.Summary) ObservedContainer { State: c.State, ConfigHash: c.Labels[api.ConfigHashLabel], ImageDigest: c.Labels[api.ImageDigestLabel], + ImageVolumeDigest: c.Labels[api.ImageVolumeDigestLabel], Number: number, ConnectedNetworks: networks, Summary: c, diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go index 56eaced0f55..a065969bc1b 100644 --- a/pkg/compose/reconcile.go +++ b/pkg/compose/reconcile.go @@ -770,6 +770,9 @@ func (r *reconciler) mustRecreate(expected types.ServiceConfig, expectedHash str if oc.ImageDigest != expected.CustomLabels[api.ImageDigestLabel] { return true } + if oc.ImageVolumeDigest != expected.CustomLabels[api.ImageVolumeDigestLabel] { + return true + } if oc.State == container.StateRunning && r.hasNetworkMismatch(expected, oc) { return true } From 9a157e145a81c08dafad4985a9641a6445bff265 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 6 Aug 2026 15:42:26 +0200 Subject: [PATCH 2/6] ci(e2e): run e2e against the containerd image store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/ci.yml | 29 ++++++- pkg/e2e/fixtures/image-identity/compose.yaml | 4 + pkg/e2e/image_identity_test.go | 81 ++++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 pkg/e2e/fixtures/image-identity/compose.yaml create mode 100644 pkg/e2e/image_identity_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0637850993c..cebf6a497bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -165,7 +165,7 @@ jobs: e2e: runs-on: ubuntu-latest - name: e2e (${{ matrix.mode }}, ${{ matrix.channel }}) + name: e2e (${{ matrix.mode }}, ${{ matrix.channel }}${{ matrix.store && format(', {0}', matrix.store) || '' }}) strategy: fail-fast: false matrix: @@ -185,12 +185,23 @@ jobs: - mode: standalone engine: 28 channel: oldstable + + # containerd image store (non-graphdriver digest behavior) + - mode: plugin + engine: 29 + channel: stable + store: containerd steps: - name: Prepare run: | mode=${{ matrix.mode }} engine=${{ matrix.engine }} - echo "MODE_ENGINE_PAIR=${mode}-${engine}" >> $GITHUB_ENV + store=${{ matrix.store }} + pair="${mode}-${engine}" + if [ -n "$store" ]; then + pair="${pair}-${store}" + fi + echo "MODE_ENGINE_PAIR=${pair}" >> $GITHUB_ENV - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -206,6 +217,20 @@ jobs: - name: Check Docker Version run: docker --version + - name: Enable containerd image store + if: ${{ matrix.store == 'containerd' }} + run: | + sudo mkdir -p /etc/docker + if [ -s /etc/docker/daemon.json ]; then + jq -s '.[0] * .[1]' /etc/docker/daemon.json <(echo '{"features": {"containerd-snapshotter": true}}') \ + | sudo tee /etc/docker/daemon.json.new > /dev/null + sudo mv /etc/docker/daemon.json.new /etc/docker/daemon.json + else + echo '{"features": {"containerd-snapshotter": true}}' | sudo tee /etc/docker/daemon.json > /dev/null + fi + sudo systemctl restart docker.service + docker info -f '{{json .DriverStatus}}' | grep -q io.containerd.snapshotter.v1 + - name: Set up Docker Buildx uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0 diff --git a/pkg/e2e/fixtures/image-identity/compose.yaml b/pkg/e2e/fixtures/image-identity/compose.yaml new file mode 100644 index 00000000000..11a73d4fdce --- /dev/null +++ b/pkg/e2e/fixtures/image-identity/compose.yaml @@ -0,0 +1,4 @@ +services: + app: + image: alpine:3.20 + command: ["sleep", "infinity"] diff --git a/pkg/e2e/image_identity_test.go b/pkg/e2e/image_identity_test.go new file mode 100644 index 00000000000..bf7dc897a36 --- /dev/null +++ b/pkg/e2e/image_identity_test.go @@ -0,0 +1,81 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "strings" + "testing" + + "gotest.tools/v3/assert" +) + +// TestUpIdempotentContainerdStore reproduces the scenario described in +// https://github.com/docker/compose/pull/13998 : under the containerd image +// store, a multi-platform image pulled by the first `up` gets its +// `com.docker.compose.image` label set from the raw digest returned by +// `image inspect` (the manifest-list/index digest). On the next `up`, the +// image is now local, so compose recomputes the label from the per-platform +// manifest digest instead. Those two digests differ for the very same image, +// so compose believes the image changed and recreates the container even +// though nothing did. +// +// `up` MUST be idempotent: running it twice in a row without any change +// must not recreate any container. +func TestUpIdempotentContainerdStore(t *testing.T) { + // TODO(image-identity): temporary skip — this test is red by design until + // the canonical content-digest producer lands (next commit of this PR). + // Remove this skip in that commit; the test is the acceptance criterion. + t.Skip("skipped until the canonical image content-digest producer lands (see PR commits)") + + c := NewCLI(t) + requireContainerdStore(t, c) + + const projectName = "compose-e2e-image-identity-idempotent" + const image = "alpine:3.20" + const composeFile = "./fixtures/image-identity/compose.yaml" + + t.Cleanup(func() { + c.cleanupWithDown(t, projectName) + c.RunDockerOrExitError(t, "rmi", "-f", image) + }) + + // Start from an image that was never pulled locally: the first `up` + // below will pull it, exercising the multi-platform pull path. + c.RunDockerOrExitError(t, "rmi", "-f", image) + + c.RunDockerComposeCmd(t, "-f", composeFile, "--project-name", projectName, "up", "-d") + containerID := c.RunDockerCmd(t, "inspect", fmt.Sprintf("%s-app-1", projectName), "-f", "{{.Id}}").Stdout() + + res := c.RunDockerComposeCmd(t, "-f", composeFile, "--project-name", projectName, "up", "-d") + assert.Check(t, !strings.Contains(res.Combined(), "Recreate"), "second `up` should not recreate anything, got: %s", res.Combined()) + + newContainerID := c.RunDockerCmd(t, "inspect", fmt.Sprintf("%s-app-1", projectName), "-f", "{{.Id}}").Stdout() + assert.Equal(t, containerID, newContainerID, "container should not have been recreated by an idempotent `up`") +} + +// requireContainerdStore skips the test unless the docker daemon backing the +// CLI instance uses the containerd image store (`io.containerd.snapshotter.v1` +// driver), which is a prerequisite for the image identity bug this test +// covers. +func requireContainerdStore(t *testing.T, c *CLI) { + t.Helper() + res := c.RunDockerCmd(t, "info", "-f", "{{json .DriverStatus}}") + if !strings.Contains(res.Stdout(), "io.containerd.snapshotter.v1") { + t.Skip("Skipping test: daemon is not using the containerd image store") + } +} From 0a050b2f80b6be6f478548d8c0c3d8ae149df104 Mon Sep 17 00:00:00 2001 From: Max Malm Date: Thu, 6 Aug 2026 15:56:17 +0200 Subject: [PATCH 3/6] Use content digest for pulled service images 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 Signed-off-by: Max Malm Signed-off-by: Guillaume Lours --- pkg/compose/images.go | 21 +++++++++++++ pkg/compose/pull.go | 13 ++++---- pkg/compose/pull_test.go | 65 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/pkg/compose/images.go b/pkg/compose/images.go index 6502f4cf53a..2894641d158 100644 --- a/pkg/compose/images.go +++ b/pkg/compose/images.go @@ -185,6 +185,27 @@ func (s *composeService) manifestsSupported(ctx context.Context) (bool, error) { return versions.GreaterThanOrEqualTo(version, apiVersion148), nil } +// inspectContentDigest inspects ref, requesting per-manifest data on engines +// that support it, and returns the digest identifying the image's runnable +// content for the default platform. Callers that record an image identity +// compose later compares for staleness must go through this, so every such +// identity is computed the same way — see contentDigest. +func (s *composeService) inspectContentDigest(ctx context.Context, ref string) (string, error) { + withManifests, err := s.manifestsSupported(ctx) + if err != nil { + return "", err + } + var opts []client.ImageInspectOption + if withManifests { + opts = append(opts, client.ImageInspectWithManifests(true)) + } + inspected, err := s.apiClient().ImageInspect(ctx, ref, opts...) + if err != nil { + return "", err + } + return contentDigest(inspected.InspectResponse, platforms.Default()), nil +} + // contentDigest returns the digest identifying an image's runnable content // (config + layers) for the given platform. With BuildKit provenance // attestations enabled (the default since recent Buildx/BuildKit), the image is diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 62aa57da6ec..57175026ca5 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -286,11 +286,14 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser } s.events.On(newEvent(resource, api.Done, api.StatusPulled)) - inspected, err := s.apiClient().ImageInspect(ctx, service.Image) - if err != nil { - return "", err - } - return inspected.ID, nil + // Resolve the pulled image's identity exactly the way getImageSummaries + // does for already-local images: both values feed the + // com.docker.compose.image label used to detect stale containers, so they + // must be computed identically. Returning the raw inspect ID here (the + // index digest, under the containerd store with a tag@digest ref) while + // later ups resolve the platform manifest digest via contentDigest made + // the first up after a pull recreate every container despite no change. + return s.inspectContentDigest(ctx, service.Image) } // ImageDigestResolver creates a func able to resolve image digest from a docker ref, diff --git a/pkg/compose/pull_test.go b/pkg/compose/pull_test.go index c1af0caa596..a120bc727bd 100644 --- a/pkg/compose/pull_test.go +++ b/pkg/compose/pull_test.go @@ -17,10 +17,18 @@ package compose import ( + "context" + "io" + "iter" "sort" "testing" "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/config/configfile" + "github.com/moby/moby/api/types/image" + "github.com/moby/moby/api/types/jsonstream" + "github.com/moby/moby/client" + "go.uber.org/mock/gomock" "gotest.tools/v3/assert" "github.com/docker/compose/v5/pkg/api" @@ -46,10 +54,10 @@ func scheduledHookImages(t *testing.T, project *types.Project, present map[strin return images } -func serviceWithHook(name, image, policy string) types.ServiceConfig { +func serviceWithHook(name, img, policy string) types.ServiceConfig { return types.ServiceConfig{ Name: name, - Image: image, + Image: img, PullPolicy: policy, PreStart: []types.ServiceHook{{Image: "init:latest"}}, } @@ -102,6 +110,59 @@ func TestAddPreStartHookPulls_NeverSkips(t *testing.T) { assert.Equal(t, len(scheduledHookImages(t, project, map[string]api.ImageSummary{})), 0) } +// fakePullResponse is an empty, already-complete pull stream. +type fakePullResponse struct{} + +func (fakePullResponse) Read([]byte) (int, error) { return 0, io.EOF } +func (fakePullResponse) Close() error { return nil } +func (fakePullResponse) Wait(context.Context) error { + return nil +} + +func (fakePullResponse) JSONMessages(context.Context) iter.Seq2[jsonstream.Message, error] { + return func(func(jsonstream.Message, error) bool) {} +} + +// TestPullServiceImageUsesContentDigest verifies the pull path resolves the +// pulled image's identity with the same contentDigest scheme +// getImageSummaries uses for already-local images. Both values feed the +// com.docker.compose.image label that detects stale containers, so when the +// pull path returned the raw inspect ID instead (the index digest, under the +// containerd store with a tag@digest ref), the first up after the pulling up +// saw a phantom image change and recreated every container once. +func TestPullServiceImageUsesContentDigest(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockAPI, cli := prepareMocks(mockCtrl) + cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() + tested, err := NewComposeService(cli) + assert.NilError(t, err) + mockAPI.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). + Return(client.PingResult{APIVersion: "1.48"}, nil).AnyTimes() + mockAPI.EXPECT().ClientVersion().Return("1.48").AnyTimes() + + ref := "foo:1@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + mockAPI.EXPECT(). + ImagePull(anyCancellableContext(), ref, gomock.Any()). + Return(fakePullResponse{}, nil) + inspect := image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{ + imageManifest("sha256:image", "amd64", true), + attestationManifest(), + }, + } + mockAPI.EXPECT(). + ImageInspect(anyCancellableContext(), ref, gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: inspect}, nil) + + id, err := tested.(*composeService). + pullServiceImage(t.Context(), types.ServiceConfig{Name: "web", Image: ref}, true, "") + assert.NilError(t, err) + assert.Equal(t, id, "sha256:image") +} + // TestAddPreStartHookPulls_DedupsSharedHookImage verifies a hook image shared by // several services is scheduled at most once. func TestAddPreStartHookPulls_DedupsSharedHookImage(t *testing.T) { From 3d304f536c961e5d3616138641b821b702bf4d67 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 6 Aug 2026 16:36:31 +0200 Subject: [PATCH 4/6] fix(build): canonical content-digest producer, single image-label writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/compose/build.go | 98 +++++----- pkg/compose/build_bake.go | 30 +-- pkg/compose/build_classic.go | 6 +- pkg/compose/build_test.go | 75 +++++++- pkg/compose/images.go | 219 +++++++++++++++------ pkg/compose/images_test.go | 298 +++++++++++++++++++++++++++-- pkg/compose/observed_state_test.go | 12 +- pkg/compose/pull.go | 5 +- pkg/compose/pull_test.go | 2 +- pkg/e2e/image_identity_test.go | 5 - 10 files changed, 585 insertions(+), 165 deletions(-) diff --git a/pkg/compose/build.go b/pkg/compose/build.go index e38be84ef80..9bac1cb7c5b 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -24,8 +24,6 @@ import ( "time" "github.com/compose-spec/compose-go/v2/types" - "github.com/containerd/platforms" - specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "github.com/docker/compose/v5/internal/tracing" @@ -115,7 +113,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. } } - images, err := s.getLocalImagesDigests(ctx, project) + images, pinnedDigests, err := s.getLocalImagesDigests(ctx, project) if err != nil { return err } @@ -152,12 +150,17 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. } } - // set digest as com.docker.compose.image label so we can detect outdated containers + // set digest as com.docker.compose.image label so we can detect outdated + // containers — the single writer of that label, so the platform-pinned + // resolution below can't be overwritten by another code path for name, service := range project.Services { image := api.GetImageNameOrDefault(service, project.Name) img, ok := images[image] if ok { - service.CustomLabels.Add(api.ImageDigestLabel, img.ID) + // a platform-pinned service is labelled with the digest of ITS + // platform's manifest — see serviceImageDigest + digest := s.serviceImageDigest(ctx, service, image, img, pinnedDigests) + service.CustomLabels = service.CustomLabels.Add(api.ImageDigestLabel, digest) } resolveImageVolumes(&service, images, project.Name) @@ -174,22 +177,19 @@ func resolveImageVolumes(service *types.ServiceConfig, images map[string]api.Ima continue } imgName := vol.Source - if _, ok := images[vol.Source]; !ok { + img, ok := images[imgName] + if !ok { // check if source is another service in the project imgName = api.GetImageNameOrDefault(types.ServiceConfig{Name: vol.Source}, projectName) // If we still can't find it, it might be an external image that wasn't pulled yet or doesn't exist - if _, ok := images[imgName]; !ok { + if img, ok = images[imgName]; !ok { continue } } - img, ok := images[imgName] - if !ok { - continue - } // The daemon only resolves a `type=image` mount Source that is a name/tag // or a top-level image ID, not a per-platform manifest digest (which is // what ImageSummary.ID holds to stay stable across attested rebuilds, see - // contentDigest()). Keep Source as the resolved name so mounting always + // localContentDigest). Keep Source as the resolved name so mounting always // works, and track the digest separately so mustRecreate can still detect // a changed source image. service.Volumes[i].Source = imgName @@ -197,11 +197,22 @@ func resolveImageVolumes(service *types.ServiceConfig, images map[string]api.Ima } if len(digests) > 0 { sort.Strings(digests) - service.CustomLabels.Add(api.ImageVolumeDigestLabel, strings.Join(digests, ",")) + service.CustomLabels = service.CustomLabels.Add(api.ImageVolumeDigestLabel, strings.Join(digests, ",")) } } -func (s *composeService) getLocalImagesDigests(ctx context.Context, project *types.Project) (map[string]api.ImageSummary, error) { +// pinnedImageDigest is the content digest of a platform-pinned service's +// image, resolved for the service's platform rather than the host's. +type pinnedImageDigest struct { + digest string + // from is the shared summary digest at resolution time: once a pull or + // build refreshed the summary, this resolution is stale and + // serviceImageDigest re-resolves the pinned platform, keeping the + // refreshed shared value only as fallback. + from string +} + +func (s *composeService) getLocalImagesDigests(ctx context.Context, project *types.Project) (map[string]api.ImageSummary, map[string]pinnedImageDigest, error) { imageNames := utils.Set[string]{} for _, s := range project.Services { imageNames.Add(api.GetImageNameOrDefault(s, project.Name)) @@ -214,48 +225,45 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ imageNames.Add(img) } } - imgs, err := s.getImageSummaries(ctx, imageNames.Elements()) + inspections, err := s.inspectLocalImages(ctx, imageNames.Elements()) if err != nil { - return nil, err + return nil, nil, err + } + imgs := make(map[string]api.ImageSummary, len(inspections)) + for repoTag, inspect := range inspections { + imgs[repoTag] = imageSummary(repoTag, inspect) } - for i, service := range project.Services { + pinnedDigests := map[string]pinnedImageDigest{} + for name, service := range project.Services { + if service.Platform == "" { + continue + } imgName := api.GetImageNameOrDefault(service, project.Name) img, ok := imgs[imgName] if !ok { continue } - if service.Platform != "" { - platform, err := platforms.Parse(service.Platform) - if err != nil { - return nil, err - } - // inspect by name, not img.ID: img.ID now holds a content-manifest - // digest (see contentDigest) which is not necessarily inspectable, - // whereas the image name always resolves. - inspect, err := s.apiClient().ImageInspect(ctx, imgName) - if err != nil { - return nil, err - } - actual := specs.Platform{ - Architecture: inspect.Architecture, - OS: inspect.Os, - Variant: inspect.Variant, - } - if !platforms.NewMatcher(platform).Match(actual) { - logrus.Debugf("local image %s doesn't match expected platform %s", service.Image, service.Platform) - // there is a local image, but it's for the wrong platform, so - // pretend it doesn't exist so that we can pull/build an image - // for the correct platform instead - delete(imgs, imgName) - } + // digest selection and platform validation share the same manifest + // resolution (matchLocalManifest) on the inspect we already hold, + // otherwise the digest recorded and the platform validated could + // refer to different manifests of the same image + digest, satisfied, err := localContentDigest(inspections[imgName], service.Platform) + if err != nil { + return nil, nil, err } - - project.Services[i].CustomLabels.Add(api.ImageDigestLabel, img.ID) - + if !satisfied { + logrus.Debugf("local image %s doesn't match expected platform %s", service.Image, service.Platform) + // there is a local image, but it's for the wrong platform, so + // pretend it doesn't exist so that we can pull/build an image + // for the correct platform instead + delete(imgs, imgName) + continue + } + pinnedDigests[name] = pinnedImageDigest{digest: digest, from: img.ID} } - return imgs, nil + return imgs, pinnedDigests, nil } // resolveAndMergeBuildArgs returns the final set of build arguments to use for the service image build. diff --git a/pkg/compose/build_bake.go b/pkg/compose/build_bake.go index fe5b9b4355b..8ec6a272ee9 100644 --- a/pkg/compose/build_bake.go +++ b/pkg/compose/build_bake.go @@ -403,38 +403,24 @@ func (s *composeService) doBuildBake(ctx context.Context, project *types.Project return nil, err } + // Bake reports the top-level attested image/index digest, which changes on + // every build when provenance attestations are enabled — even for a fully + // cached build (see https://github.com/docker/compose/issues/13636). For + // images loaded into the local engine (the common non-push case), resolve + // the canonical content digest — with the service's pinned platform when + // set — so unchanged rebuilds don't recreate containers. results := map[string]string{} - var builtImages []string - for name := range serviceToBeBuild { + for name, service := range serviceToBeBuild { image := expectedImages[name] target := targets[name] built, ok := md[target] if !ok { return nil, fmt.Errorf("build result not found in Bake metadata for service %s", name) } - results[image] = built.Digest - builtImages = append(builtImages, image) + results[image] = s.canonicalBuiltDigest(ctx, image, service.Platform, built.Digest) s.events.On(builtEvent(image)) } - // Bake reports the top-level attested image/index digest, which changes on - // every build when provenance attestations are enabled — even for a fully - // cached build (see https://github.com/docker/compose/issues/13636). For - // images loaded into the local engine (the common non-push case), substitute - // the content digest so unchanged rebuilds don't recreate containers. - // Registry-only images (push/multi-platform) aren't inspectable locally, so - // they keep the Bake-reported digest. - // Best effort: if the built images can't be inspected, keep the - // Bake-reported digests rather than failing an already-successful build. - summaries, err := s.getImageSummaries(ctx, builtImages) - if err != nil { - logrus.Debugf("unable to inspect built images for content digest, keeping bake digests: %v", err) - } else { - for image, summary := range summaries { - results[image] = summary.ID - } - } - return results, nil } diff --git a/pkg/compose/build_classic.go b/pkg/compose/build_classic.go index a5f750ae2dc..dba0e956ac7 100644 --- a/pkg/compose/build_classic.go +++ b/pkg/compose/build_classic.go @@ -94,7 +94,11 @@ func (s *composeService) doBuildClassic(ctx context.Context, project *types.Proj return err } s.events.On(builtEvent(image)) - builtDigests[getServiceIndex(name)] = id + // the classic builder reports the raw image ID from the build stream; + // resolve the canonical content digest instead so the recorded + // identity matches what later runs compute for the same local image + // (resolved here to inherit the build traversal's concurrency) + builtDigests[getServiceIndex(name)] = s.canonicalBuiltDigest(ctx, image, service.Platform, id) if options.Push { return s.push(ctx, project, api.PushOptions{}) diff --git a/pkg/compose/build_test.go b/pkg/compose/build_test.go index 00f80dc82c6..a7fbf2e2c33 100644 --- a/pkg/compose/build_test.go +++ b/pkg/compose/build_test.go @@ -25,6 +25,8 @@ import ( "github.com/moby/moby/client" "go.uber.org/mock/gomock" "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" ) func Test_dockerFilePath(t *testing.T) { @@ -134,8 +136,79 @@ func TestGetLocalImagesDigests_PreStartHook(t *testing.T) { apiClient.EXPECT().ImageInspect(gomock.Any(), "alpine:3.19"). Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ID: "sha256:hook"}}, nil) - images, err := tested.getLocalImagesDigests(t.Context(), project) + images, _, err := tested.getLocalImagesDigests(t.Context(), project) assert.NilError(t, err) assert.Equal(t, images["alpine:3.20"].ID, "sha256:service") assert.Equal(t, images["alpine:3.19"].ID, "sha256:hook") } + +func TestResolveImageVolumes(t *testing.T) { + images := map[string]api.ImageSummary{ + "content:1": {ID: "sha256:content"}, + "p-source": {ID: "sha256:built"}, + "assets:2": {ID: "sha256:assets"}, + "p-web": {ID: "sha256:web"}, + } + imageVolume := func(source, target string) types.ServiceVolumeConfig { + return types.ServiceVolumeConfig{Type: types.VolumeTypeImage, Source: source, Target: target} + } + + t.Run("source is an image name", func(t *testing.T) { + service := types.ServiceConfig{ + Name: "web", + CustomLabels: types.Labels{}, + Volumes: []types.ServiceVolumeConfig{imageVolume("content:1", "/data")}, + } + resolveImageVolumes(&service, images, "p") + assert.Equal(t, service.Volumes[0].Source, "content:1") + assert.Equal(t, service.CustomLabels[api.ImageVolumeDigestLabel], "/data=sha256:content") + }) + + t.Run("source is another service resolves to its image name", func(t *testing.T) { + service := types.ServiceConfig{ + Name: "web", + CustomLabels: types.Labels{}, + Volumes: []types.ServiceVolumeConfig{imageVolume("source", "/data")}, + } + resolveImageVolumes(&service, images, "p") + // the mount Source must stay a daemon-resolvable name, never a digest + assert.Equal(t, service.Volumes[0].Source, "p-source") + assert.Equal(t, service.CustomLabels[api.ImageVolumeDigestLabel], "/data=sha256:built") + }) + + t.Run("unresolvable source is left untouched and unlabelled", func(t *testing.T) { + service := types.ServiceConfig{ + Name: "web", + CustomLabels: types.Labels{}, + Volumes: []types.ServiceVolumeConfig{imageVolume("ghost:1", "/data")}, + } + resolveImageVolumes(&service, images, "p") + assert.Equal(t, service.Volumes[0].Source, "ghost:1") + _, labelled := service.CustomLabels[api.ImageVolumeDigestLabel] + assert.Assert(t, !labelled) + }) + + t.Run("several volumes produce a deterministic sorted label", func(t *testing.T) { + service := types.ServiceConfig{ + Name: "web", + CustomLabels: types.Labels{}, + Volumes: []types.ServiceVolumeConfig{ + imageVolume("assets:2", "/b"), + imageVolume("content:1", "/a"), + }, + } + resolveImageVolumes(&service, images, "p") + assert.Equal(t, service.CustomLabels[api.ImageVolumeDigestLabel], "/a=sha256:content,/b=sha256:assets") + }) + + t.Run("no image volumes writes no label", func(t *testing.T) { + service := types.ServiceConfig{ + Name: "web", + CustomLabels: types.Labels{}, + Volumes: []types.ServiceVolumeConfig{{Type: types.VolumeTypeVolume, Source: "vol", Target: "/data"}}, + } + resolveImageVolumes(&service, images, "p") + _, labelled := service.CustomLabels[api.ImageVolumeDigestLabel] + assert.Assert(t, !labelled) + }) +} diff --git a/pkg/compose/images.go b/pkg/compose/images.go index 2894641d158..cdb05cd9b67 100644 --- a/pkg/compose/images.go +++ b/pkg/compose/images.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/compose-spec/compose-go/v2/types" "github.com/containerd/errdefs" "github.com/containerd/platforms" "github.com/distribution/reference" @@ -31,6 +32,8 @@ import ( "github.com/moby/moby/api/types/image" "github.com/moby/moby/client" "github.com/moby/moby/client/pkg/versions" + specs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" "github.com/docker/compose/v5/pkg/api" @@ -126,22 +129,19 @@ func (s *composeService) Images(ctx context.Context, projectName string, options return summary, err } -func (s *composeService) getImageSummaries(ctx context.Context, repoTags []string) (map[string]api.ImageSummary, error) { - summary := map[string]api.ImageSummary{} - l := sync.Mutex{} - - withManifests, err := s.manifestsSupported(ctx) +// inspectLocalImages inspects the given references in parallel, requesting +// per-manifest data on engines that support it. References not found locally +// are simply absent from the result. +func (s *composeService) inspectLocalImages(ctx context.Context, repoTags []string) (map[string]client.ImageInspectResult, error) { + opts, err := s.imageInspectOptions(ctx) if err != nil { return nil, err } - + inspections := map[string]client.ImageInspectResult{} + l := sync.Mutex{} eg, ctx := errgroup.WithContext(ctx) for _, repoTag := range repoTags { eg.Go(func() error { - var opts []client.ImageInspectOption - if withManifests { - opts = append(opts, client.ImageInspectWithManifests(true)) - } inspect, err := s.apiClient().ImageInspect(ctx, repoTag, opts...) if err != nil { if errdefs.IsNotFound(err) { @@ -149,29 +149,47 @@ func (s *composeService) getImageSummaries(ctx context.Context, repoTags []strin } return fmt.Errorf("unable to get image '%s': %w", repoTag, err) } - tag := "" - repository := "" - ref, err := reference.ParseDockerRef(repoTag) - if err == nil { - // ParseDockerRef will reject a local image ID - repository = reference.FamiliarName(ref) - if tagged, ok := ref.(reference.Tagged); ok { - tag = tagged.Tag() - } - } l.Lock() - summary[repoTag] = api.ImageSummary{ - ID: contentDigest(inspect.InspectResponse, platforms.Default()), - Repository: repository, - Tag: tag, - Size: inspect.Size, - LastTagTime: inspect.Metadata.LastTagTime, - } + inspections[repoTag] = inspect l.Unlock() return nil }) } - return summary, eg.Wait() + return inspections, eg.Wait() +} + +// imageInspectOptions requests per-manifest data when the engine supports it +// (see manifestsSupported). +func (s *composeService) imageInspectOptions(ctx context.Context) ([]client.ImageInspectOption, error) { + withManifests, err := s.manifestsSupported(ctx) + if err != nil { + return nil, err + } + if !withManifests { + return nil, nil + } + return []client.ImageInspectOption{client.ImageInspectWithManifests(true)}, nil +} + +func imageSummary(repoTag string, inspect client.ImageInspectResult) api.ImageSummary { + tag := "" + repository := "" + ref, err := reference.ParseDockerRef(repoTag) + if err == nil { + // ParseDockerRef will reject a local image ID + repository = reference.FamiliarName(ref) + if tagged, ok := ref.(reference.Tagged); ok { + tag = tagged.Tag() + } + } + id, _, _ := localContentDigest(inspect, "") + return api.ImageSummary{ + ID: id, + Repository: repository, + Tag: tag, + Size: inspect.Size, + LastTagTime: inspect.Metadata.LastTagTime, + } } // manifestsSupported reports whether the engine can return per-manifest data on @@ -185,45 +203,107 @@ func (s *composeService) manifestsSupported(ctx context.Context) (bool, error) { return versions.GreaterThanOrEqualTo(version, apiVersion148), nil } -// inspectContentDigest inspects ref, requesting per-manifest data on engines -// that support it, and returns the digest identifying the image's runnable -// content for the default platform. Callers that record an image identity -// compose later compares for staleness must go through this, so every such -// identity is computed the same way — see contentDigest. -func (s *composeService) inspectContentDigest(ctx context.Context, ref string) (string, error) { - withManifests, err := s.manifestsSupported(ctx) - if err != nil { - return "", err +// serviceImageDigest returns the digest to record in a service's +// com.docker.compose.image label. A platform-pinned service uses the digest +// resolved in-process from the pre-pull inspect while it is still current; +// once the shared summary entry was refreshed by a pull or build during the +// run, that resolution is stale AND the refreshed digest was resolved for the +// platform of whichever service triggered the refresh — services sharing the +// image with a different pinned platform re-resolve theirs with one extra +// inspect (only in that refresh case; steady-state runs never get here). Best +// effort: the shared digest is kept when the pinned platform can't be +// satisfied or inspected. +func (s *composeService) serviceImageDigest(ctx context.Context, service types.ServiceConfig, imgName string, img api.ImageSummary, pinnedDigests map[string]pinnedImageDigest) string { + if service.Platform == "" { + return img.ID } - var opts []client.ImageInspectOption - if withManifests { - opts = append(opts, client.ImageInspectWithManifests(true)) + if pinned, ok := pinnedDigests[service.Name]; ok && pinned.from == img.ID { + return pinned.digest } - inspected, err := s.apiClient().ImageInspect(ctx, ref, opts...) + digest, satisfied, err := s.inspectLocalContent(ctx, imgName, service.Platform) + if err != nil || !satisfied { + logrus.Debugf("unable to resolve %s for pinned platform %s, keeping shared digest: satisfied=%v err=%v", imgName, service.Platform, satisfied, err) + return img.ID + } + return digest +} + +// canonicalBuiltDigest resolves the canonical content digest of a just-built +// image so the recorded identity matches what later runs compute for the same +// local image. Registry-only builds (push-only, multi-platform without load) +// are not locally inspectable and keep the builder-reported digest: a volatile +// but honest value — an actual rebuild is still detected — preferred over a +// stable marker that would hide real image changes. Best effort: never fails +// an already-successful build. +func (s *composeService) canonicalBuiltDigest(ctx context.Context, imageRef, platform, builderDigest string) string { + id, _, err := s.inspectLocalContent(ctx, imageRef, platform) if err != nil { - return "", err + logrus.Debugf("unable to resolve content digest for built image %s, keeping builder-reported digest: %v", imageRef, err) + return builderDigest } - return contentDigest(inspected.InspectResponse, platforms.Default()), nil + return id } -// contentDigest returns the digest identifying an image's runnable content -// (config + layers) for the given platform. With BuildKit provenance -// attestations enabled (the default since recent Buildx/BuildKit), the image is -// stored as an index whose top-level digest (inspect.ID) also covers the -// attestation manifest, so it changes on every build even when the runnable -// image is unchanged — making compose recreate containers needlessly (see +// localContentDigest returns the digest identifying an inspected image's +// runnable content (config + layers) for the requested platform (empty means +// the host default), plus whether the local image satisfies that platform. +// Note the satisfied bool is only meaningful for an explicitly requested +// platform: with platform empty, the manifests path reports host-default +// satisfaction while the manifest-less path always reports true. +// +// With BuildKit provenance attestations enabled (the default since recent +// Buildx/BuildKit), the image is stored as an index whose top-level digest +// (inspect.ID) also covers the attestation manifest, so it changes on every +// build even when the runnable image is unchanged — making compose recreate +// containers needlessly (see // https://github.com/docker/compose/issues/13636). The digest of the "image" -// kind manifest reflects only the image content, which is what compose needs to -// detect staleness. +// kind manifest reflects only the image content, which is what compose needs +// to detect staleness. Images inspected without manifest data (engines that +// can't report them) keep the plain image ID — already the config digest — +// with platform satisfaction from the inspect's flat platform fields, the +// pre-manifest behavior. Every image identity compose records for staleness +// comparison must be computed through here, whatever the image's provenance +// (pulled, built, already local), so any two runs produce comparable values. +func localContentDigest(inspect client.ImageInspectResult, platform string) (string, bool, error) { + var matcher platforms.Matcher = platforms.Default() + pinned := platform != "" + if pinned { + p, err := platforms.Parse(platform) + if err != nil { + return "", false, err + } + matcher = platforms.NewMatcher(p) + } + if len(inspect.Manifests) > 0 { + id, ok := matchLocalManifest(inspect.InspectResponse, matcher) + return id, ok, nil + } + ok := true + if pinned { + ok = matcher.Match(specs.Platform{ + Architecture: inspect.Architecture, + OS: inspect.Os, + Variant: inspect.Variant, + }) + } + return inspect.ID, ok, nil +} + +// matchLocalManifest selects, among the locally available image manifests of +// inspect, the digest identifying the runnable content for the requested +// platform, and reports whether the local content actually satisfies that +// platform. Selection is platform-aware and deterministic so the same image +// always maps to the same digest across rebuilds: the available manifest +// matching the requested platform wins (satisfied); a lone available image +// manifest keeps its digest — single-platform images stay identifiable +// whatever their platform — without satisfying a different requested +// platform; otherwise fall back to inspect.ID, which never satisfies the +// request. // -// Selection is platform-aware and deterministic so the same image always maps -// to the same digest across rebuilds: the available manifest matching the -// requested platform wins; a lone available image manifest is used as-is -// (single-platform images, whatever their platform); otherwise we fall back to -// inspect.ID (engines that don't report manifests — where inspect.ID is already -// the config digest — or a multi-platform image whose requested platform isn't -// available locally, which the caller then treats as a platform miss). -func contentDigest(inspect image.InspectResponse, platform platforms.MatchComparer) string { +// Both the digest producers and the platform checks must go through this +// single selection so the digest recorded and the platform validated always +// refer to the same manifest. +func matchLocalManifest(inspect image.InspectResponse, platform platforms.Matcher) (string, bool) { var available []image.ManifestSummary for _, m := range inspect.Manifests { if m.Kind == image.ManifestKindImage && m.Available { @@ -232,11 +312,26 @@ func contentDigest(inspect image.InspectResponse, platform platforms.MatchCompar } for _, m := range available { if m.ImageData != nil && platform.Match(m.ImageData.Platform) { - return m.ID + return m.ID, true } } if len(available) == 1 { - return available[0].ID + return available[0].ID, false + } + return inspect.ID, false +} + +// inspectLocalContent inspects ref and returns its canonical content digest +// for the requested platform (empty means the host default), plus whether the +// local image satisfies that platform — see localContentDigest. +func (s *composeService) inspectLocalContent(ctx context.Context, ref string, platform string) (string, bool, error) { + opts, err := s.imageInspectOptions(ctx) + if err != nil { + return "", false, err + } + inspected, err := s.apiClient().ImageInspect(ctx, ref, opts...) + if err != nil { + return "", false, err } - return inspect.ID + return localContentDigest(inspected, platform) } diff --git a/pkg/compose/images_test.go b/pkg/compose/images_test.go index 5d26231aed8..2121db5fe96 100644 --- a/pkg/compose/images_test.go +++ b/pkg/compose/images_test.go @@ -22,8 +22,10 @@ import ( "testing" "time" + "github.com/compose-spec/compose-go/v2/types" "github.com/containerd/errdefs" "github.com/containerd/platforms" + "github.com/docker/cli/cli/config/configfile" "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/image" "github.com/moby/moby/client" @@ -119,13 +121,19 @@ func attestationManifest() image.ManifestSummary { return image.ManifestSummary{ID: "sha256:att", Kind: image.ManifestKindAttestation, Available: true} } -func TestContentDigest(t *testing.T) { +func TestMatchLocalManifest(t *testing.T) { amd64 := platforms.Only(specs.Platform{OS: "linux", Architecture: "amd64"}) arm64 := platforms.Only(specs.Platform{OS: "linux", Architecture: "arm64"}) - t.Run("no manifests falls back to the plain image ID", func(t *testing.T) { - inspect := image.InspectResponse{ID: "sha256:top"} - assert.Equal(t, contentDigest(inspect, amd64), "sha256:top") + match := func(t *testing.T, inspect image.InspectResponse, platform platforms.Matcher, wantID string, wantOK bool) { + t.Helper() + id, ok := matchLocalManifest(inspect, platform) + assert.Equal(t, id, wantID) + assert.Equal(t, ok, wantOK) + } + + t.Run("no manifests falls back to the plain image ID, unsatisfied", func(t *testing.T) { + match(t, image.InspectResponse{ID: "sha256:top"}, amd64, "sha256:top", false) }) t.Run("attested image ignores the attestation manifest", func(t *testing.T) { @@ -138,16 +146,17 @@ func TestContentDigest(t *testing.T) { attestationManifest(), }, } - assert.Equal(t, contentDigest(inspect, amd64), "sha256:amd64") + match(t, inspect, amd64, "sha256:amd64", true) }) - t.Run("single image manifest is used even when the platform does not match", func(t *testing.T) { + t.Run("single image manifest keeps its digest but does not satisfy another platform", func(t *testing.T) { // single-platform image built for a non-host platform stays resolvable inspect := image.InspectResponse{ ID: "sha256:index", Manifests: []image.ManifestSummary{imageManifest("sha256:arm64", "arm64", true)}, } - assert.Equal(t, contentDigest(inspect, amd64), "sha256:arm64") + match(t, inspect, amd64, "sha256:arm64", false) + match(t, inspect, arm64, "sha256:arm64", true) }) t.Run("multi-platform picks the matching platform manifest", func(t *testing.T) { @@ -159,8 +168,8 @@ func TestContentDigest(t *testing.T) { attestationManifest(), }, } - assert.Equal(t, contentDigest(inspect, amd64), "sha256:amd64") - assert.Equal(t, contentDigest(inspect, arm64), "sha256:arm64") + match(t, inspect, amd64, "sha256:amd64", true) + match(t, inspect, arm64, "sha256:arm64", true) }) t.Run("unavailable manifests are skipped", func(t *testing.T) { @@ -172,7 +181,7 @@ func TestContentDigest(t *testing.T) { imageManifest("sha256:arm64", "arm64", true), }, } - assert.Equal(t, contentDigest(inspect, amd64), "sha256:arm64") + match(t, inspect, amd64, "sha256:arm64", false) }) t.Run("only attestation manifests falls back to the plain image ID", func(t *testing.T) { @@ -180,7 +189,7 @@ func TestContentDigest(t *testing.T) { ID: "sha256:top", Manifests: []image.ManifestSummary{attestationManifest()}, } - assert.Equal(t, contentDigest(inspect, amd64), "sha256:top") + match(t, inspect, amd64, "sha256:top", false) }) t.Run("ambiguous multi-platform with no match falls back to the plain image ID", func(t *testing.T) { @@ -192,7 +201,7 @@ func TestContentDigest(t *testing.T) { }, } windows := platforms.Only(specs.Platform{OS: "windows", Architecture: "amd64"}) - assert.Equal(t, contentDigest(inspect, windows), "sha256:index") + match(t, inspect, windows, "sha256:index", false) }) } @@ -204,10 +213,11 @@ func newTestComposeService(t *testing.T, mockCtrl *gomock.Controller, apiVersion api.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). Return(client.PingResult{APIVersion: apiVersion}, nil).AnyTimes() api.EXPECT().ClientVersion().Return(apiVersion).AnyTimes() + cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() return api, tested.(*composeService) } -func TestGetImageSummariesUsesContentDigest(t *testing.T) { +func TestInspectLocalImagesUsesContentDigest(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() api, tested := newTestComposeService(t, mockCtrl, "1.48") @@ -223,12 +233,12 @@ func TestGetImageSummariesUsesContentDigest(t *testing.T) { ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). Return(client.ImageInspectResult{InspectResponse: inspect}, nil) - summaries, err := tested.getImageSummaries(t.Context(), []string{"foo:1"}) + inspections, err := tested.inspectLocalImages(t.Context(), []string{"foo:1"}) assert.NilError(t, err) - assert.Equal(t, summaries["foo:1"].ID, "sha256:image") + assert.Equal(t, imageSummary("foo:1", inspections["foo:1"]).ID, "sha256:image") } -func TestGetImageSummariesLegacyEngineUsesPlainID(t *testing.T) { +func TestInspectLocalImagesLegacyEngineUsesPlainID(t *testing.T) { // Engine < 28.0 (API < 1.48) can't report manifests, so we keep the plain ID. mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -239,12 +249,12 @@ func TestGetImageSummariesLegacyEngineUsesPlainID(t *testing.T) { ImageInspect(anyCancellableContext(), "foo:1"). Return(client.ImageInspectResult{InspectResponse: inspect}, nil) - summaries, err := tested.getImageSummaries(t.Context(), []string{"foo:1"}) + inspections, err := tested.inspectLocalImages(t.Context(), []string{"foo:1"}) assert.NilError(t, err) - assert.Equal(t, summaries["foo:1"].ID, "sha256:plain") + assert.Equal(t, imageSummary("foo:1", inspections["foo:1"]).ID, "sha256:plain") } -func TestGetImageSummariesSkipsMissingImages(t *testing.T) { +func TestInspectLocalImagesSkipsMissingImages(t *testing.T) { // Registry-only images (push/multi-platform) aren't inspectable locally; // they must be omitted so the caller keeps the Bake-reported digest. mockCtrl := gomock.NewController(t) @@ -255,12 +265,258 @@ func TestGetImageSummariesSkipsMissingImages(t *testing.T) { ImageInspect(anyCancellableContext(), "missing:1", gomock.Any()). Return(client.ImageInspectResult{}, errdefs.ErrNotFound) - summaries, err := tested.getImageSummaries(t.Context(), []string{"missing:1"}) + inspections, err := tested.inspectLocalImages(t.Context(), []string{"missing:1"}) assert.NilError(t, err) - _, ok := summaries["missing:1"] + _, ok := inspections["missing:1"] assert.Assert(t, !ok) } +func TestInspectLocalContent(t *testing.T) { + t.Run("manifests path selects the pinned platform", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{ + imageManifest("sha256:amd64", "amd64", true), + imageManifest("sha256:arm64", "arm64", true), + }, + }}, nil) + + id, ok, err := tested.inspectLocalContent(t.Context(), "foo:1", "linux/arm64") + assert.NilError(t, err) + assert.Equal(t, id, "sha256:arm64") + assert.Assert(t, ok) + }) + + t.Run("manifests path reports an unavailable pinned platform", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{imageManifest("sha256:amd64", "amd64", true)}, + }}, nil) + + _, ok, err := tested.inspectLocalContent(t.Context(), "foo:1", "linux/arm64") + assert.NilError(t, err) + assert.Assert(t, !ok) + }) + + t.Run("legacy engine falls back to flat platform fields", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.47") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1"). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:plain", Os: "linux", Architecture: "amd64", + }}, nil).Times(2) + + id, ok, err := tested.inspectLocalContent(t.Context(), "foo:1", "linux/amd64") + assert.NilError(t, err) + assert.Equal(t, id, "sha256:plain") + assert.Assert(t, ok) + + _, ok, err = tested.inspectLocalContent(t.Context(), "foo:1", "linux/arm64") + assert.NilError(t, err) + assert.Assert(t, !ok) + }) +} + +// TestPlatformPinnedDigest covers two historic defects around +// `platform:`-pinned services: +// - the com.docker.compose.image label must hold the digest of the PINNED +// platform manifest, not the host's — all the way through +// ensureImagesExists, whose final loop is the label's single writer; +// - when the local image cannot satisfy the pinned platform, no label at all +// must be written (the summary was just discarded as "wrong platform"). +func TestPlatformPinnedDigest(t *testing.T) { + // platforms are synthetic so neither can match the machine running the + // tests: the host-side summary must fall back to the index digest while + // the pinned resolution picks the service's platform manifest + multiPlatform := image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{ + imageManifest("sha256:s390x", "s390x", true), + imageManifest("sha256:riscv64", "riscv64", true), + }, + } + + newProject := func() *types.Project { + return &types.Project{ + Name: "p", + Services: types.Services{ + "app": { + Name: "app", + Image: "foo:1", + Platform: "linux/s390x", + CustomLabels: types.Labels{}, + }, + }, + } + } + + t.Run("pinned platform digest is resolved from the shared inspect", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: multiPlatform}, nil) // a single inspect serves both the summary and the platform check + + project := newProject() + imgs, pinned, err := tested.getLocalImagesDigests(t.Context(), project) + assert.NilError(t, err) + assert.Equal(t, imgs["foo:1"].ID, "sha256:index", "shared summary stays host-resolved") + assert.Equal(t, pinned["app"], pinnedImageDigest{digest: "sha256:s390x", from: "sha256:index"}) + }) + + t.Run("pinned platform digest lands in the label through ensureImagesExists", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: multiPlatform}, nil) + + project := newProject() + assert.NilError(t, tested.ensureImagesExists(t.Context(), project, nil, true)) + assert.Equal(t, project.Services["app"].CustomLabels[compose.ImageDigestLabel], "sha256:s390x") + }) + + t.Run("platform mismatch discards the image and writes no label", func(t *testing.T) { + amd64Only := image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{imageManifest("sha256:riscv64", "riscv64", true)}, + } + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: amd64Only}, nil) + + project := newProject() + imgs, pinned, err := tested.getLocalImagesDigests(t.Context(), project) + assert.NilError(t, err) + _, present := imgs["foo:1"] + assert.Assert(t, !present) + assert.Equal(t, len(pinned), 0) + _, labelled := project.Services["app"].CustomLabels[compose.ImageDigestLabel] + assert.Assert(t, !labelled) + }) +} + +// TestServiceImageDigest covers the label-digest decision for platform-pinned +// services, notably when the shared summary entry was refreshed by a pull or +// build during the run: the refreshed digest was resolved for the platform of +// whichever service triggered it, so services sharing the image with another +// pinned platform must re-resolve theirs. +func TestServiceImageDigest(t *testing.T) { + pinnedService := types.ServiceConfig{Name: "app", Platform: "linux/s390x"} + + t.Run("unpinned service uses the shared digest, no inspect", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + _, tested := newTestComposeService(t, mockCtrl, "1.48") + + got := tested.serviceImageDigest(t.Context(), types.ServiceConfig{Name: "app"}, "foo:1", + compose.ImageSummary{ID: "sha256:shared"}, nil) + assert.Equal(t, got, "sha256:shared") + }) + + t.Run("valid pinned resolution is used without any inspect", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + _, tested := newTestComposeService(t, mockCtrl, "1.48") + + got := tested.serviceImageDigest(t.Context(), pinnedService, "foo:1", + compose.ImageSummary{ID: "sha256:index"}, + map[string]pinnedImageDigest{"app": {digest: "sha256:s390x", from: "sha256:index"}}) + assert.Equal(t, got, "sha256:s390x") + }) + + t.Run("refreshed entry re-resolves the pinned platform", func(t *testing.T) { + // the image was pulled/built during the run for ANOTHER service's + // platform: the stale pre-pull resolution must not be used, and the + // shared digest is not this service's platform either + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:refreshed", + Manifests: []image.ManifestSummary{ + imageManifest("sha256:riscv64", "riscv64", true), + imageManifest("sha256:s390x", "s390x", true), + }, + }}, nil) + + got := tested.serviceImageDigest(t.Context(), pinnedService, "foo:1", + compose.ImageSummary{ID: "sha256:refreshed"}, + map[string]pinnedImageDigest{"app": {digest: "sha256:stale", from: "sha256:index"}}) + assert.Equal(t, got, "sha256:s390x") + }) + + t.Run("unsatisfied pinned platform falls back to the shared digest", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "foo:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:refreshed", + Manifests: []image.ManifestSummary{imageManifest("sha256:riscv64", "riscv64", true)}, + }}, nil) + + got := tested.serviceImageDigest(t.Context(), pinnedService, "foo:1", + compose.ImageSummary{ID: "sha256:refreshed"}, nil) + assert.Equal(t, got, "sha256:refreshed") + }) +} + +func TestCanonicalBuiltDigest(t *testing.T) { + t.Run("locally inspectable build resolves to the content digest", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "built:1", gomock.Any()). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ + ID: "sha256:index", + Manifests: []image.ManifestSummary{ + imageManifest("sha256:image", "amd64", true), + attestationManifest(), + }, + }}, nil) + + got := tested.canonicalBuiltDigest(t.Context(), "built:1", "", "sha256:bakeindex") + assert.Equal(t, got, "sha256:image") + }) + + t.Run("registry-only build keeps the builder-reported digest", func(t *testing.T) { + // push-only / multi-platform-only builds never land in the local + // store: keep the builder digest — volatile but honest (a real + // rebuild is detected) rather than a stable marker hiding changes. + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + api, tested := newTestComposeService(t, mockCtrl, "1.48") + api.EXPECT(). + ImageInspect(anyCancellableContext(), "pushed:1", gomock.Any()). + Return(client.ImageInspectResult{}, errdefs.ErrNotFound) + + got := tested.canonicalBuiltDigest(t.Context(), "pushed:1", "", "sha256:bakeindex") + assert.Equal(t, got, "sha256:bakeindex") + }) +} + func containerDetail(service string, id string, status container.ContainerState, imageName string) container.Summary { return container.Summary{ ID: id, diff --git a/pkg/compose/observed_state_test.go b/pkg/compose/observed_state_test.go index 5e965ba7b99..60055664853 100644 --- a/pkg/compose/observed_state_test.go +++ b/pkg/compose/observed_state_test.go @@ -40,11 +40,12 @@ func TestToObservedContainer(t *testing.T) { Names: []string{"/testProject-web-1"}, State: container.StateRunning, Labels: map[string]string{ - api.ServiceLabel: "web", - api.ConfigHashLabel: "sha256:aaa", - api.ImageDigestLabel: "sha256:bbb", - api.ContainerNumberLabel: "1", - api.ProjectLabel: "testproject", + api.ServiceLabel: "web", + api.ConfigHashLabel: "sha256:aaa", + api.ImageDigestLabel: "sha256:bbb", + api.ImageVolumeDigestLabel: "/data=sha256:ccc", + api.ContainerNumberLabel: "1", + api.ProjectLabel: "testproject", }, NetworkSettings: &container.NetworkSettingsSummary{ Networks: map[string]*network.EndpointSettings{ @@ -60,6 +61,7 @@ func TestToObservedContainer(t *testing.T) { assert.Equal(t, oc.State, container.StateRunning) assert.Equal(t, oc.ConfigHash, "sha256:aaa") assert.Equal(t, oc.ImageDigest, "sha256:bbb") + assert.Equal(t, oc.ImageVolumeDigest, "/data=sha256:ccc") assert.Equal(t, oc.Number, 1) assert.Equal(t, oc.ConnectedNetworks["mynet"], "net123") assert.Equal(t, oc.Summary.ID, "abc123") diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 57175026ca5..d80bfc42130 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -51,7 +51,7 @@ func (s *composeService) Pull(ctx context.Context, project *types.Project, optio } func (s *composeService) pull(ctx context.Context, project *types.Project, opts api.PullOptions) error { //nolint:gocyclo - images, err := s.getLocalImagesDigests(ctx, project) + images, _, err := s.getLocalImagesDigests(ctx, project) if err != nil { return err } @@ -293,7 +293,8 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser // index digest, under the containerd store with a tag@digest ref) while // later ups resolve the platform manifest digest via contentDigest made // the first up after a pull recreate every container despite no change. - return s.inspectContentDigest(ctx, service.Image) + id, _, err := s.inspectLocalContent(ctx, service.Image, platform) + return id, err } // ImageDigestResolver creates a func able to resolve image digest from a docker ref, diff --git a/pkg/compose/pull_test.go b/pkg/compose/pull_test.go index a120bc727bd..b6ace581d1f 100644 --- a/pkg/compose/pull_test.go +++ b/pkg/compose/pull_test.go @@ -125,7 +125,7 @@ func (fakePullResponse) JSONMessages(context.Context) iter.Seq2[jsonstream.Messa // TestPullServiceImageUsesContentDigest verifies the pull path resolves the // pulled image's identity with the same contentDigest scheme -// getImageSummaries uses for already-local images. Both values feed the +// getLocalImagesDigests uses for already-local images. Both values feed the // com.docker.compose.image label that detects stale containers, so when the // pull path returned the raw inspect ID instead (the index digest, under the // containerd store with a tag@digest ref), the first up after the pulling up diff --git a/pkg/e2e/image_identity_test.go b/pkg/e2e/image_identity_test.go index bf7dc897a36..4839e4a6738 100644 --- a/pkg/e2e/image_identity_test.go +++ b/pkg/e2e/image_identity_test.go @@ -37,11 +37,6 @@ import ( // `up` MUST be idempotent: running it twice in a row without any change // must not recreate any container. func TestUpIdempotentContainerdStore(t *testing.T) { - // TODO(image-identity): temporary skip — this test is red by design until - // the canonical content-digest producer lands (next commit of this PR). - // Remove this skip in that commit; the test is the acceptance criterion. - t.Skip("skipped until the canonical image content-digest producer lands (see PR commits)") - c := NewCLI(t) requireContainerdStore(t, c) From e988c2956e721b560088deae784ece083919951b Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 6 Aug 2026 16:36:52 +0200 Subject: [PATCH 5/6] fix(scale, run): resolve DOCKER_DEFAULT_PLATFORM like up does 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 --- cmd/compose/run.go | 28 ++++++++++++++++++++++------ cmd/compose/scale.go | 7 +++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/cmd/compose/run.go b/cmd/compose/run.go index ecb3f9f1bb3..3bbec0e8239 100644 --- a/cmd/compose/run.go +++ b/cmd/compose/run.go @@ -204,12 +204,7 @@ func runCommand(p *ProjectOptions, dockerCli command.Cli, backendOptions *Backen return err } - project, _, err := p.ToProject(ctx, dockerCli, backend, []string{options.Service}, composecli.WithoutEnvironmentResolution) - if err != nil { - return err - } - - project, err = project.WithServicesEnvironmentResolved(true) + project, err := runProject(ctx, dockerCli, backend, p, options.Service) if err != nil { return err } @@ -269,6 +264,27 @@ func normalizeRunFlags(f *pflag.FlagSet, name string) pflag.NormalizedName { return pflag.NormalizedName(name) } +// runProject loads and prepares the project for a one-off run: environment +// resolved after service selection (so env_file of unrelated services doesn't +// need to exist) and DOCKER_DEFAULT_PLATFORM resolved into service.Platform +// exactly like `up`/`create` do — Platform feeds the config-hash of the +// dependencies started by run, so hashing a different value would recreate +// their containers. +func runProject(ctx context.Context, dockerCli command.Cli, backend api.Compose, p *ProjectOptions, service string) (*types.Project, error) { + project, _, err := p.ToProject(ctx, dockerCli, backend, []string{service}, composecli.WithoutEnvironmentResolution) + if err != nil { + return nil, err + } + project, err = project.WithServicesEnvironmentResolved(true) + if err != nil { + return nil, err + } + if err := applyPlatforms(project, true); err != nil { + return nil, err + } + return project, nil +} + func runRun(ctx context.Context, backend api.Compose, project *types.Project, options runOptions, createOpts createOptions, buildOpts buildOptions, dockerCli command.Cli) error { project, err := options.apply(project) if err != nil { diff --git a/cmd/compose/scale.go b/cmd/compose/scale.go index ac1e25469ba..02af0b8d4cc 100644 --- a/cmd/compose/scale.go +++ b/cmd/compose/scale.go @@ -80,6 +80,13 @@ func runScale(ctx context.Context, dockerCli command.Cli, backendOptions *Backen return err } + // resolve DOCKER_DEFAULT_PLATFORM into service.Platform exactly like + // `up`/`create` do: Platform feeds the service config-hash, so scale + // hashing a different value would recreate every container + if err := applyPlatforms(project, true); err != nil { + return err + } + if opts.noDeps { if project, err = project.WithSelectedServices(services, types.IgnoreDependencies); err != nil { return err From 31b7ff881e60b8c455086dc4b2d5274680d0a956 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 6 Aug 2026 15:45:33 +0200 Subject: [PATCH 6/6] fix(pull): interpret pull_policy like up does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/compose/pull.go | 104 +++++++++++++++++++++++++-------------- pkg/compose/pull_test.go | 81 +++++++++++++++++++++++++----- 2 files changed, 138 insertions(+), 47 deletions(-) diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index d80bfc42130..1471e71214d 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -77,24 +77,20 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts continue } - switch service.PullPolicy { - case types.PullPolicyNever, types.PullPolicyBuild: + pullRequired, skipReason, err := shouldPullImage(service, images) + if err != nil { + // join already-scheduled pulls before returning: bailing out with + // goroutines still in flight would leak them past pull()'s return + return errors.Join(err, eg.Wait()) + } + if !pullRequired { s.events.On(api.Resource{ - ID: "Image " + service.Image, - Status: api.Done, - Text: "Skipped", + ID: "Image " + service.Image, + Status: api.Done, + Text: "Skipped", + Details: skipReason, }) continue - case types.PullPolicyMissing, types.PullPolicyIfNotPresent: - if imageAlreadyPresent(service.Image, images) { - s.events.On(api.Resource{ - ID: "Image " + service.Image, - Status: api.Done, - Text: "Skipped", - Details: "Image is already present locally", - }) - continue - } } if service.Build != nil && opts.IgnoreBuildable { @@ -137,25 +133,31 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts // pre_start hook images run as ephemeral init containers with their own // registry image. They have no pull policy of their own, so we inherit the - // parent service's policy for skip decisions. Unlike the service image, a - // hook image can't be built, so `build` does not exempt it from pulling — - // only `never` does (consistent with the `up`/create path). + // parent service's policy for skip decisions — through the same + // shouldPullImage decision as the service image. Unlike the service + // image, a hook image can't be built, so `build` falls back to + // pull-if-missing instead of exempting it from pulling. for name, service := range project.Services { - if service.PullPolicy == types.PullPolicyNever { - continue + hookPolicy := service.PullPolicy + if hookPolicy == types.PullPolicyBuild { + hookPolicy = types.PullPolicyMissing } for _, img := range api.GetDependentImages(service, project.Name) { - switch service.PullPolicy { - case types.PullPolicyMissing, types.PullPolicyIfNotPresent, types.PullPolicyBuild: - if imageAlreadyPresent(img, images) { + pullRequired, skipReason, err := shouldPullImage(types.ServiceConfig{Name: name, Image: img, PullPolicy: hookPolicy}, images) + if err != nil { + // same as the service loop: never leave scheduled pulls unjoined + return errors.Join(err, eg.Wait()) + } + if !pullRequired { + if skipReason != "" { s.events.On(api.Resource{ ID: "Image " + img, Status: api.Done, Text: "Skipped", - Details: "Image is already present locally", + Details: skipReason, }) - continue } + continue } if _, ok := imagesBeingPulled[img]; ok { continue @@ -188,19 +190,49 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts return errors.Join(pullErrors...) } -func imageAlreadyPresent(serviceImage string, localImages map[string]api.ImageSummary) bool { - normalizedImage, err := reference.ParseDockerRef(serviceImage) +// shouldPullImage decides whether `compose pull` refreshes a service's image, +// delegating to the exact pull_policy interpreter the up path uses (mustPull) +// so both commands honor never/build, skip-if-present and the +// daily/weekly/every_N refresh window identically. The command keeps two +// deliberate differences: +// - a service without an explicit pull_policy is always refreshed — an +// unset policy resolves to "missing" for `up`, but skipping it would turn +// an explicit `compose pull` into a no-op once images exist; +// - 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, no download, when the +// local image is already up to date. +func shouldPullImage(service types.ServiceConfig, images map[string]api.ImageSummary) (bool, string, error) { + if service.PullPolicy == "" { + return true, "", nil + } + pull, err := mustPull(service, images) + if err != nil || pull { + return pull, "", err + } + policy, _, _ := service.GetPullPolicy() + switch policy { + case types.PullPolicyRefresh: + return false, "Image is not due for refresh", nil + case types.PullPolicyMissing, types.PullPolicyIfNotPresent: + if isLatestTag(service.Image) { + return true, "", nil + } + return false, "Image is already present locally", nil + default: // never, build — and provider services short-circuited by mustPull + return false, "", nil + } +} + +// isLatestTag reports whether ref points at a `latest` tag, including bare +// references that normalize to it. +func isLatestTag(ref string) bool { + named, err := reference.ParseDockerRef(ref) if err != nil { return false } - switch refType := normalizedImage.(type) { - case reference.NamedTagged: - _, ok := localImages[serviceImage] - return ok && refType.Tag() != "latest" - default: - _, ok := localImages[serviceImage] - return ok - } + tagged, ok := named.(reference.Tagged) + return ok && tagged.Tag() == "latest" } func getUnwrappedErrorMessage(err error) string { @@ -286,7 +318,7 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser } s.events.On(newEvent(resource, api.Done, api.StatusPulled)) - // Resolve the pulled image's identity exactly the way getImageSummaries + // Resolve the pulled image's identity exactly the way getLocalImagesDigests // does for already-local images: both values feed the // com.docker.compose.image label used to detect stale containers, so they // must be computed identically. Returning the raw inspect ID here (the diff --git a/pkg/compose/pull_test.go b/pkg/compose/pull_test.go index b6ace581d1f..7588ac46aaa 100644 --- a/pkg/compose/pull_test.go +++ b/pkg/compose/pull_test.go @@ -22,9 +22,9 @@ import ( "iter" "sort" "testing" + "time" "github.com/compose-spec/compose-go/v2/types" - "github.com/docker/cli/cli/config/configfile" "github.com/moby/moby/api/types/image" "github.com/moby/moby/api/types/jsonstream" "github.com/moby/moby/client" @@ -133,14 +133,7 @@ func (fakePullResponse) JSONMessages(context.Context) iter.Seq2[jsonstream.Messa func TestPullServiceImageUsesContentDigest(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - - mockAPI, cli := prepareMocks(mockCtrl) - cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() - tested, err := NewComposeService(cli) - assert.NilError(t, err) - mockAPI.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). - Return(client.PingResult{APIVersion: "1.48"}, nil).AnyTimes() - mockAPI.EXPECT().ClientVersion().Return("1.48").AnyTimes() + mockAPI, tested := newTestComposeService(t, mockCtrl, "1.48") ref := "foo:1@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" mockAPI.EXPECT(). @@ -157,8 +150,7 @@ func TestPullServiceImageUsesContentDigest(t *testing.T) { ImageInspect(anyCancellableContext(), ref, gomock.Any()). Return(client.ImageInspectResult{InspectResponse: inspect}, nil) - id, err := tested.(*composeService). - pullServiceImage(t.Context(), types.ServiceConfig{Name: "web", Image: ref}, true, "") + id, err := tested.pullServiceImage(t.Context(), types.ServiceConfig{Name: "web", Image: ref}, true, "") assert.NilError(t, err) assert.Equal(t, id, "sha256:image") } @@ -176,3 +168,70 @@ func TestAddPreStartHookPulls_DedupsSharedHookImage(t *testing.T) { assert.DeepEqual(t, scheduledHookImages(t, project, map[string]api.ImageSummary{}), []string{"init:latest"}) } + +func TestShouldPullImage(t *testing.T) { + present := map[string]api.ImageSummary{ + "web:1": {LastTagTime: time.Now()}, + "web:latest": {LastTagTime: time.Now()}, + "old:1": {LastTagTime: time.Now().Add(-48 * time.Hour)}, + } + svc := func(image, policy string) types.ServiceConfig { + return types.ServiceConfig{Name: "web", Image: image, PullPolicy: policy} + } + + t.Run("no explicit policy always refreshes", func(t *testing.T) { + pull, _, err := shouldPullImage(svc("web:1", ""), present) + assert.NilError(t, err) + assert.Assert(t, pull) + }) + + t.Run("never and build skip", func(t *testing.T) { + for _, policy := range []string{types.PullPolicyNever, types.PullPolicyBuild} { + pull, _, err := shouldPullImage(svc("web:1", policy), present) + assert.NilError(t, err) + assert.Assert(t, !pull) + } + }) + + t.Run("missing skips a present image", func(t *testing.T) { + pull, _, err := shouldPullImage(svc("web:1", types.PullPolicyMissing), present) + assert.NilError(t, err) + assert.Assert(t, !pull) + + pull, _, err = shouldPullImage(svc("absent:1", types.PullPolicyMissing), present) + assert.NilError(t, err) + assert.Assert(t, pull) + }) + + t.Run("missing still refreshes a present latest tag", func(t *testing.T) { + // deliberate exception: `latest` is expected to move, so the pull is + // triggered anyway and the daemon's registry negotiation decides + // (a no-op when the local image is already up to date) + for _, image := range []string{"web:latest", "web"} { + pull, _, err := shouldPullImage(svc(image, types.PullPolicyMissing), map[string]api.ImageSummary{ + image: {LastTagTime: time.Now()}, + }) + assert.NilError(t, err) + assert.Assert(t, pull, "present %s must still be refreshed", image) + } + }) + + t.Run("refresh policies honor the same window as up", func(t *testing.T) { + pull, _, err := shouldPullImage(svc("web:1", "daily"), present) + assert.NilError(t, err) + assert.Assert(t, !pull, "recently tagged image is not due for refresh") + + pull, _, err = shouldPullImage(svc("old:1", "daily"), present) + assert.NilError(t, err) + assert.Assert(t, pull, "image older than the window must be refreshed") + + pull, _, err = shouldPullImage(svc("absent:1", "weekly"), present) + assert.NilError(t, err) + assert.Assert(t, pull, "absent image must be pulled") + }) + + t.Run("invalid refresh spec errors", func(t *testing.T) { + _, _, err := shouldPullImage(svc("web:1", "every_bogus"), present) + assert.Assert(t, err != nil) + }) +}