diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0637850993..cebf6a497b 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/cmd/compose/run.go b/cmd/compose/run.go index ecb3f9f1bb..3bbec0e823 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 ac1e25469b..02af0b8d4c 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 diff --git a/pkg/api/labels.go b/pkg/api/labels.go index 3a0f684b98..bc9d1f4a1b 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 2a86165992..9bac1cb7c5 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -19,12 +19,11 @@ package compose import ( "context" "fmt" + "sort" "strings" "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" @@ -114,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 } @@ -151,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) @@ -167,29 +171,48 @@ 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 + 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 img, ok = images[imgName]; !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 + // 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 + digests = append(digests, vol.Target+"="+img.ID) + } + if len(digests) > 0 { + sort.Strings(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)) @@ -202,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 fe5b9b4355..8ec6a272ee 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 a5f750ae2d..dba0e956ac 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 00f80dc82c..a7fbf2e2c3 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 6502f4cf53..cdb05cd9b6 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,24 +203,107 @@ func (s *composeService) manifestsSupported(ctx context.Context) (bool, error) { return versions.GreaterThanOrEqualTo(version, apiVersion148), 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 -// 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 +// 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 + } + if pinned, ok := pinnedDigests[service.Name]; ok && pinned.from == img.ID { + return pinned.digest + } + 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 { + logrus.Debugf("unable to resolve content digest for built image %s, keeping builder-reported digest: %v", imageRef, err) + return builderDigest + } + return id +} + +// 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 { @@ -211,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 5d26231aed..2121db5fe9 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.go b/pkg/compose/observed_state.go index d2e187e0b6..aaf44269a3 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/observed_state_test.go b/pkg/compose/observed_state_test.go index 5e965ba7b9..6005566485 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 62aa57da6e..1471e71214 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 } @@ -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,11 +318,15 @@ 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 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 + // 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. + 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 c1af0caa59..7588ac46aa 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" + "time" "github.com/compose-spec/compose-go/v2/types" + "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,51 @@ 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 +// 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 +// saw a phantom image change and recreated every container once. +func TestPullServiceImageUsesContentDigest(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + mockAPI, tested := newTestComposeService(t, mockCtrl, "1.48") + + 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.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) { @@ -115,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) + }) +} diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go index 56eaced0f5..a065969bc1 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 } diff --git a/pkg/e2e/fixtures/image-identity/compose.yaml b/pkg/e2e/fixtures/image-identity/compose.yaml new file mode 100644 index 0000000000..11a73d4fdc --- /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 0000000000..4839e4a673 --- /dev/null +++ b/pkg/e2e/image_identity_test.go @@ -0,0 +1,76 @@ +/* + 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) { + 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") + } +}