build: label built images for reliable cleanup on `down`

When running `compose down`, the `--rmi` flag can be passed,
which currently supports two values:
 * `local`: remove any _implicitly-named_ images that Compose
            built
 * `all`  : remove any named images (locally-built or fetched
            from a remote repo)

Removing images in the `local` case can be problematic, as it's
historically been done via a fair amount of inference over the
Compose model. Additionally, when using the "project-model"
(by passing `--project-name` instead of using a Compose file),
we're even more limited: if no containers for the project are
running, there's nothing to derive state from to perform the
inference on.

As a first pass, we started labeling _containers_ with the name
of the locally-built image associated with it (if any) in #9715.
Unfortunately, this still suffers from the aforementioned problems
around using actual state (i.e. the containers might no longer
exist) and meant that when operating in file mode (the default),
things did not behave as expected: the label is not available
in the project since it only exists at runtime.

Now, with these changes, Compose will label any images it builds
with project metadata. Upon cleanup during `down`, the engine
image API is queried for related images and matched up with the
services for the project. As a fallback for images built with
prior versions of Compose, the previous approach is still taken.

See also:
 * https://github.com/docker/compose/issues/9655
 * https://github.com/docker/compose/pull/9715

Signed-off-by: Milas Bowman <milas.bowman@docker.com>

pkg/api/labels.go

@@ -47,14 +47,14 @@ const (
 	OneoffLabel = "com.docker.compose.oneoff"
 	// SlugLabel stores unique slug used for one-off container identity
 	SlugLabel = "com.docker.compose.slug"
-	// ImageNameLabel stores the content of the image section in the compose file
-	ImageNameLabel = "com.docker.compose.image_name"
 	// ImageDigestLabel stores digest of the container image used to run service
 	ImageDigestLabel = "com.docker.compose.image"
 	// DependenciesLabel stores service dependencies
 	DependenciesLabel = "com.docker.compose.depends_on"
-	// VersionLabel stores the compose tool version used to run application
+	// VersionLabel stores the compose tool version used to build/run application
 	VersionLabel = "com.docker.compose.version"
+	// ImageBuilderLabel stores the builder (classic or BuildKit) used to produce the image.
+	ImageBuilderLabel = "com.docker.compose.image.builder"
 )
 
 // ComposeVersion is the compose tool version as declared by label VersionLabel

pkg/compose/build.go

@@ -139,7 +139,6 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types.
 				project.Services[i].Labels = types.Labels{}
 			}
 			project.Services[i].CustomLabels.Add(api.ImageDigestLabel, digest)
-			project.Services[i].CustomLabels.Add(api.ImageNameLabel, service.Image)
 		}
 	}
 	return nil
@@ -192,7 +191,6 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ
 		digest, ok := images[imgName]
 		if ok {
 			project.Services[i].CustomLabels.Add(api.ImageDigestLabel, digest)
-			project.Services[i].CustomLabels.Add(api.ImageNameLabel, project.Services[i].Image)
 		}
 	}
 
@@ -263,6 +261,8 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 		tags = append(tags, service.Build.Tags...)
 	}
 
+	imageLabels := getImageBuildLabels(project, service)
+
 	return build.Options{
 		Inputs: build.Inputs{
 			ContextPath:    service.Build.Context,
@@ -277,7 +277,7 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 		Target:      service.Build.Target,
 		Exports:     []bclient.ExportEntry{{Type: "image", Attrs: map[string]string{}}},
 		Platforms:   plats,
-		Labels:      service.Build.Labels,
+		Labels:      imageLabels,
 		NetworkMode: service.Build.Network,
 		ExtraHosts:  service.Build.ExtraHosts.AsList(),
 		Session:     sessionConfig,
@@ -327,7 +327,6 @@ func sshAgentProvider(sshKeys types.SSHConfig) (session.Attachable, error) {
 }
 
 func addSecretsConfig(project *types.Project, service types.ServiceConfig) (session.Attachable, error) {
-
 	var sources []secretsprovider.Source
 	for _, secret := range service.Build.Secrets {
 		config := project.Secrets[secret.Source]
@@ -352,3 +351,17 @@ func addSecretsConfig(project *types.Project, service types.ServiceConfig) (sess
 	}
 	return secretsprovider.NewSecretProvider(store), nil
 }
+
+func getImageBuildLabels(project *types.Project, service types.ServiceConfig) types.Labels {
+	ret := make(types.Labels)
+	if service.Build != nil {
+		for k, v := range service.Build.Labels {
+			ret.Add(k, v)
+		}
+	}
+
+	ret.Add(api.VersionLabel, api.ComposeVersion)
+	ret.Add(api.ProjectLabel, project.Name)
+	ret.Add(api.ServiceLabel, service.Name)
+	return ret
+}

pkg/compose/build_buildkit.go

@@ -25,6 +25,8 @@ import (
 	"github.com/docker/buildx/build"
 	"github.com/docker/buildx/driver"
 	xprogress "github.com/docker/buildx/util/progress"
+
+	"github.com/docker/compose/v2/pkg/api"
 )
 
 func (s *composeService) doBuildBuildkit(ctx context.Context, project *types.Project, opts map[string]build.Options, mode string) (map[string]string, error) {
@@ -47,6 +49,15 @@ func (s *composeService) doBuildBuildkit(ctx context.Context, project *types.Pro
 	defer cancel()
 	w := xprogress.NewPrinter(progressCtx, s.stdout(), os.Stdout, mode)
 
+	for k := range opts {
+		if opts[k].Labels == nil {
+			opt := opts[k]
+			opt.Labels = make(map[string]string)
+			opts[k] = opt
+		}
+		opts[k].Labels[api.ImageBuilderLabel] = "buildkit"
+	}
+
 	// We rely on buildx "docker" builder integrated in docker engine, so don't need a DockerAPI here
 	response, err := build.Build(ctx, driverInfo, opts, nil, filepath.Dir(s.configFile().Filename), w)
 	errW := w.Wait()

pkg/compose/build_classic.go

@@ -89,6 +89,11 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
 		}
 	}
 
+	if options.Labels == nil {
+		options.Labels = make(map[string]string)
+	}
+	options.Labels[api.ImageBuilderLabel] = "classic"
+
 	switch {
 	case isLocalDir(specifiedContext):
 		contextDir, relDockerfile, err = build.GetContextFromLocalDir(specifiedContext, dockerfileName)

pkg/compose/compose.go

@@ -30,11 +30,12 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/streams"
-	"github.com/docker/compose/v2/pkg/api"
 	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/client"
 	"github.com/pkg/errors"
+
+	"github.com/docker/compose/v2/pkg/api"
 )
 
 // NewComposeService create a local implementation of the compose.Service API
@@ -130,13 +131,9 @@ func (s *composeService) projectFromName(containers Containers, projectName stri
 		serviceLabel := c.Labels[api.ServiceLabel]
 		_, ok := set[serviceLabel]
 		if !ok {
-			serviceImage := c.Image
-			if serviceNameFromLabel, ok := c.Labels[api.ImageNameLabel]; ok {
-				serviceImage = serviceNameFromLabel
-			}
 			set[serviceLabel] = &types.ServiceConfig{
 				Name:   serviceLabel,
-				Image:  serviceImage,
+				Image:  c.Image,
 				Labels: c.Labels,
 			}
 		}

pkg/compose/down.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/compose-spec/compose-go/types"
+	"github.com/distribution/distribution/v3/reference"
 	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/errdefs"
@@ -31,6 +32,7 @@ import (
 
 	"github.com/docker/compose/v2/pkg/api"
 	"github.com/docker/compose/v2/pkg/progress"
+	"github.com/docker/compose/v2/pkg/utils"
 )
 
 type downOp func() error
@@ -86,7 +88,11 @@ func (s *composeService) down(ctx context.Context, projectName string, options a
 	ops := s.ensureNetworksDown(ctx, project, w)
 
 	if options.Images != "" {
-		ops = append(ops, s.ensureImagesDown(ctx, project, options, w)...)
+		imgOps, err := s.ensureImagesDown(ctx, project, options, w)
+		if err != nil {
+			return err
+		}
+		ops = append(ops, imgOps...)
 	}
 
 	if options.Volumes {
@@ -118,15 +124,20 @@ func (s *composeService) ensureVolumesDown(ctx context.Context, project *types.P
 	return ops
 }
 
-func (s *composeService) ensureImagesDown(ctx context.Context, project *types.Project, options api.DownOptions, w progress.Writer) []downOp {
+func (s *composeService) ensureImagesDown(ctx context.Context, project *types.Project, options api.DownOptions, w progress.Writer) ([]downOp, error) {
+	images, err := s.getServiceImagesToRemove(ctx, options, project)
+	if err != nil {
+		return nil, err
+	}
+
 	var ops []downOp
-	for image := range s.getServiceImagesToRemove(options, project) {
-		image := image
+	for i := range images {
+		img := images[i]
 		ops = append(ops, func() error {
-			return s.removeImage(ctx, image, w)
+			return s.removeImage(ctx, img, w)
 		})
 	}
-	return ops
+	return ops, nil
 }
 
 func (s *composeService) ensureNetworksDown(ctx context.Context, project *types.Project, w progress.Writer) []downOp {
@@ -190,17 +201,108 @@ func (s *composeService) removeNetwork(ctx context.Context, name string, w progr
 	return nil
 }
 
-func (s *composeService) getServiceImagesToRemove(options api.DownOptions, project *types.Project) map[string]struct{} {
-	images := map[string]struct{}{}
+//nolint:gocyclo
+func (s *composeService) getServiceImagesToRemove(ctx context.Context, options api.DownOptions, project *types.Project) ([]string, error) {
+	if options.Images == "" {
+		return nil, nil
+	}
+
+	var localServiceImages []string
+	var imagesToRemove []string
+	addImageToRemove := func(img string, checkExistence bool) {
+		// since some references come from user input (service.image) and some
+		// come from the engine API, we standardize them, opting for the
+		// familiar name format since they'll also be displayed in the CLI
+		ref, err := reference.ParseNormalizedNamed(img)
+		if err != nil {
+			return
+		}
+		ref = reference.TagNameOnly(ref)
+		img = reference.FamiliarString(ref)
+		if utils.StringContains(imagesToRemove, img) {
+			return
+		}
+
+		if checkExistence {
+			_, _, err := s.apiClient().ImageInspectWithRaw(ctx, img)
+			if errdefs.IsNotFound(err) {
+				// err on the side of caution: only skip if we successfully
+				// queried the API and got back a definitive "not exists"
+				return
+			}
+		}
+
+		imagesToRemove = append(imagesToRemove, img)
+	}
+
+	imageListOpts := moby.ImageListOptions{
+		Filters: filters.NewArgs(
+			projectFilter(project.Name),
+			// TODO(milas): we should really clean up the dangling images as
+			// well (historically we have NOT); need to refactor this to handle
+			// it gracefully without producing confusing CLI output, i.e. we
+			// do not want to print out a bunch of untagged/dangling image IDs,
+			// they should be grouped into a logical operation for the relevant
+			// service
+			filters.Arg("dangling", "false"),
+		),
+	}
+	projectImages, err := s.apiClient().ImageList(ctx, imageListOpts)
+	if err != nil {
+		return nil, err
+	}
+
+	// 1. Remote / custom-named images - only deleted on `--rmi="all"`
 	for _, service := range project.Services {
-		image, ok := service.Labels[api.ImageNameLabel] // Information on the compose file at the creation of the container
-		if !ok || (options.Images == "local" && image != "") {
+		if service.Image == "" {
+			localServiceImages = append(localServiceImages, service.Name)
 			continue
 		}
-		image = api.GetImageNameOrDefault(service, project.Name)
-		images[image] = struct{}{}
+
+		if options.Images == "all" {
+			addImageToRemove(service.Image, true)
+		}
 	}
-	return images
+
+	// 2. *LABELED* Locally-built images with implicit image names
+	//
+	// If `--remove-orphans` is being used, then ALL images for the project
+	// will be selected for removal. Otherwise, only those that match a known
+	// service based on the loaded project will be included.
+	for _, img := range projectImages {
+		if len(img.RepoTags) == 0 {
+			// currently, we're only removing the tagged references, but
+			// if we start removing the dangling images and grouping by
+			// service, we can remove this (and should rely on `Image::ID`)
+			continue
+		}
+
+		shouldRemove := options.RemoveOrphans
+		for _, service := range localServiceImages {
+			if img.Labels[api.ServiceLabel] == service {
+				shouldRemove = true
+				break
+			}
+		}
+
+		if shouldRemove {
+			addImageToRemove(img.RepoTags[0], false)
+		}
+	}
+
+	// 3. *UNLABELED* Locally-built images with implicit image names
+	//
+	// This is a fallback for (2) to handle images built by previous
+	// versions of Compose, which did not label their built images.
+	for _, serviceName := range localServiceImages {
+		service, err := project.GetService(serviceName)
+		if err != nil || service.Image != "" {
+			continue
+		}
+		imgName := api.GetImageNameOrDefault(service, project.Name)
+		addImageToRemove(imgName, true)
+	}
+	return imagesToRemove, nil
 }
 
 func (s *composeService) removeImage(ctx context.Context, image string, w progress.Writer) error {

pkg/compose/down_test.go

@@ -18,12 +18,15 @@ package compose
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 
+	"github.com/compose-spec/compose-go/types"
 	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/volume"
+	"github.com/docker/docker/errdefs"
 	"github.com/golang/mock/gomock"
 	"gotest.tools/v3/assert"
 
@@ -143,38 +146,109 @@ func TestDownRemoveVolumes(t *testing.T) {
 	assert.NilError(t, err)
 }
 
-func TestDownRemoveImageLocal(t *testing.T) {
+func TestDownRemoveImages(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
+	opts := compose.DownOptions{
+		Project: &types.Project{
+			Name: strings.ToLower(testProject),
+			Services: types.Services{
+				{Name: "local-anonymous"},
+				{Name: "local-named", Image: "local-named-image"},
+				{Name: "remote", Image: "remote-image"},
+				{Name: "remote-tagged", Image: "registry.example.com/remote-image-tagged:v1.0"},
+				{Name: "no-images-anonymous"},
+				{Name: "no-images-named", Image: "missing-named-image"},
+			},
+		},
+	}
+
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
 	tested.dockerCli = cli
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
-	container := testContainer("service1", "123", false)
-	container.Labels[compose.ImageNameLabel] = ""
+	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).
+		Return([]moby.Container{
+			testContainer("service1", "123", false),
+		}, nil).
+		AnyTimes()
 
-	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
-		[]moby.Container{container}, nil)
+	api.EXPECT().ImageList(gomock.Any(), moby.ImageListOptions{
+		Filters: filters.NewArgs(
+			projectFilter(strings.ToLower(testProject)),
+			filters.Arg("dangling", "false"),
+		),
+	}).Return([]moby.ImageSummary{
+		{
+			Labels:   types.Labels{compose.ServiceLabel: "local-anonymous"},
+			RepoTags: []string{"testproject-local-anonymous:latest"},
+		},
+		{
+			Labels:   types.Labels{compose.ServiceLabel: "local-named"},
+			RepoTags: []string{"local-named-image:latest"},
+		},
+	}, nil).AnyTimes()
 
-	api.EXPECT().VolumeList(gomock.Any(), filters.NewArgs(projectFilter(strings.ToLower(testProject)))).
-		Return(volume.VolumeListOKBody{
-			Volumes: []*moby.Volume{{Name: "myProject_volume"}},
-		}, nil)
-	api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}).
-		Return(nil, nil)
+	imagesToBeInspected := map[string]bool{
+		"local-named-image:latest":               true,
+		"remote-image:latest":                    true,
+		"testproject-no-images-anonymous:latest": false,
+		"missing-named-image:latest":             false,
+	}
+	for img, exists := range imagesToBeInspected {
+		var resp moby.ImageInspect
+		var err error
+		if exists {
+			resp.RepoTags = []string{img}
+		} else {
+			err = errdefs.NotFound(fmt.Errorf("test specified that image %q should not exist", img))
+		}
 
-	api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
-	api.EXPECT().ContainerRemove(gomock.Any(), "123", moby.ContainerRemoveOptions{Force: true}).Return(nil)
+		api.EXPECT().ImageInspectWithRaw(gomock.Any(), img).
+			Return(resp, nil, err).
+			AnyTimes()
+	}
 
-	api.EXPECT().ImageRemove(gomock.Any(), "testproject-service1", moby.ImageRemoveOptions{}).Return(nil, nil)
+	api.EXPECT().ImageInspectWithRaw(gomock.Any(), "registry.example.com/remote-image-tagged:v1.0").
+		Return(moby.ImageInspect{RepoTags: []string{"registry.example.com/remote-image-tagged:v1.0"}}, nil, nil).
+		AnyTimes()
 
-	err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "local"})
+	localImagesToBeRemoved := []string{
+		"testproject-local-anonymous:latest",
+	}
+	for _, img := range localImagesToBeRemoved {
+		// test calls down --rmi=local then down --rmi=all, so local images
+		// get "removed" 2x, while other images are only 1x
+		api.EXPECT().ImageRemove(gomock.Any(), img, moby.ImageRemoveOptions{}).
+			Return(nil, nil).
+			Times(2)
+	}
+
+	t.Log("-> docker compose down --rmi=local")
+	opts.Images = "local"
+	err := tested.Down(context.Background(), strings.ToLower(testProject), opts)
+	assert.NilError(t, err)
+
+	otherImagesToBeRemoved := []string{
+		"local-named-image:latest",
+		"remote-image:latest",
+		"registry.example.com/remote-image-tagged:v1.0",
+	}
+	for _, img := range otherImagesToBeRemoved {
+		api.EXPECT().ImageRemove(gomock.Any(), img, moby.ImageRemoveOptions{}).
+			Return(nil, nil).
+			Times(1)
+	}
+
+	t.Log("-> docker compose down --rmi=all")
+	opts.Images = "all"
+	err = tested.Down(context.Background(), strings.ToLower(testProject), opts)
 	assert.NilError(t, err)
 }
 
-func TestDownRemoveImageLocalNoLabel(t *testing.T) {
+func TestDownRemoveImages_NoLabel(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
@@ -195,37 +269,23 @@ func TestDownRemoveImageLocalNoLabel(t *testing.T) {
 	api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}).
 		Return(nil, nil)
 
+	// ImageList returns no images for the project since they were unlabeled
+	// (created by an older version of Compose)
+	api.EXPECT().ImageList(gomock.Any(), moby.ImageListOptions{
+		Filters: filters.NewArgs(
+			projectFilter(strings.ToLower(testProject)),
+			filters.Arg("dangling", "false"),
+		),
+	}).Return(nil, nil)
+
+	api.EXPECT().ImageInspectWithRaw(gomock.Any(), "testproject-service1:latest").
+		Return(moby.ImageInspect{}, nil, nil)
+
 	api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
 	api.EXPECT().ContainerRemove(gomock.Any(), "123", moby.ContainerRemoveOptions{Force: true}).Return(nil)
 
+	api.EXPECT().ImageRemove(gomock.Any(), "testproject-service1:latest", moby.ImageRemoveOptions{}).Return(nil, nil)
+
 	err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "local"})
 	assert.NilError(t, err)
 }
-
-func TestDownRemoveImageAll(t *testing.T) {
-	mockCtrl := gomock.NewController(t)
-	defer mockCtrl.Finish()
-
-	api := mocks.NewMockAPIClient(mockCtrl)
-	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
-	cli.EXPECT().Client().Return(api).AnyTimes()
-
-	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
-		[]moby.Container{testContainer("service1", "123", false)}, nil)
-
-	api.EXPECT().VolumeList(gomock.Any(), filters.NewArgs(projectFilter(strings.ToLower(testProject)))).
-		Return(volume.VolumeListOKBody{
-			Volumes: []*moby.Volume{{Name: "myProject_volume"}},
-		}, nil)
-	api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}).
-		Return(nil, nil)
-
-	api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
-	api.EXPECT().ContainerRemove(gomock.Any(), "123", moby.ContainerRemoveOptions{Force: true}).Return(nil)
-
-	api.EXPECT().ImageRemove(gomock.Any(), "service1-img", moby.ImageRemoveOptions{}).Return(nil, nil)
-
-	err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "all"})
-	assert.NilError(t, err)
-}

pkg/compose/kill_test.go

@@ -109,7 +109,6 @@ func containerLabels(service string, oneOff bool) map[string]string {
 	composefile := filepath.Join(workingdir, "compose.yaml")
 	labels := map[string]string{
 		compose.ServiceLabel:     service,
-		compose.ImageNameLabel:   service + "-img",
 		compose.ConfigFilesLabel: composefile,
 		compose.WorkingDirLabel:  workingdir,
 		compose.ProjectLabel:     strings.ToLower(testProject)}

pkg/e2e/fixtures/build-dependencies/compose.yaml

@@ -10,3 +10,5 @@ services:
     build:
       context: .
       dockerfile: service.dockerfile
+  nginx:
+    image: nginx