From 416c916d59e47d85887350a29722e4371535e458 Mon Sep 17 00:00:00 2001 From: Max Malm Date: Mon, 3 Aug 2026 19:43:17 +0200 Subject: [PATCH 1/2] 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). Co-Authored-By: Claude Fable 5 Signed-off-by: Max Malm --- pkg/compose/pull.go | 19 +++++++++++-- pkg/compose/pull_test.go | 61 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 62aa57da6e..00039c7731 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -286,11 +286,26 @@ 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) + // 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. + withManifests, err := s.manifestsSupported(ctx) if err != nil { return "", err } - return inspected.ID, nil + var opts []client.ImageInspectOption + if withManifests { + opts = append(opts, client.ImageInspectWithManifests(true)) + } + inspected, err := s.apiClient().ImageInspect(ctx, service.Image, opts...) + if err != nil { + return "", err + } + return contentDigest(inspected.InspectResponse, platforms.Default()), nil } // 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 c1af0caa59..365cb7f825 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" @@ -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 54b0ad72820c5fe14a2e08319b5f5698d7ce3ee9 Mon Sep 17 00:00:00 2001 From: Max Malm Date: Tue, 4 Aug 2026 15:24:41 +0200 Subject: [PATCH 2/2] Fix lint issues in pull digest resolution Extract the inspect-with-manifests + contentDigest sequence from pullServiceImage into inspectContentDigest, next to the contentDigest and manifestsSupported helpers it belongs with. pullServiceImage was over the gocyclo limit of 16 with the resolve block inlined, and getImageSummaries already ran the same sequence, so this names it once. Rename the image parameter of the serviceWithHook test helper to img: it shadowed the moby image package, newly imported by pull_test.go. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Max Malm --- pkg/compose/images.go | 21 +++++++++++++++++++++ pkg/compose/pull.go | 14 +------------- pkg/compose/pull_test.go | 4 ++-- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/compose/images.go b/pkg/compose/images.go index 6502f4cf53..2894641d15 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 00039c7731..57175026ca 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -293,19 +293,7 @@ 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. - 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, service.Image, opts...) - if err != nil { - return "", err - } - return contentDigest(inspected.InspectResponse, platforms.Default()), nil + 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 365cb7f825..a120bc727b 100644 --- a/pkg/compose/pull_test.go +++ b/pkg/compose/pull_test.go @@ -54,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"}}, }