Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: label built images for reliable cleanup on down #9819

Merged
merged 5 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/api/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 17 additions & 4 deletions pkg/compose/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,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
Expand Down Expand Up @@ -207,7 +206,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)
}
}

Expand Down Expand Up @@ -267,6 +265,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,
Expand All @@ -281,7 +281,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,
Expand Down Expand Up @@ -331,7 +331,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]
Expand Down Expand Up @@ -379,6 +378,20 @@ func addPlatforms(project *types.Project, service types.ServiceConfig) ([]specs.
return plats, 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
}

func useDockerDefaultPlatform(project *types.Project, platformList types.StringList) ([]specs.Platform, error) {
var plats []specs.Platform
if platform, ok := project.Environment["DOCKER_DEFAULT_PLATFORM"]; ok {
Expand Down
5 changes: 5 additions & 0 deletions pkg/compose/build_classic.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
return "", errors.Errorf("this builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use multi-arch builder")
}

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)
Expand Down
9 changes: 3 additions & 6 deletions pkg/compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,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
Expand Down Expand Up @@ -115,13 +116,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,
}
}
Expand Down
39 changes: 20 additions & 19 deletions pkg/compose/down.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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 {
Expand Down Expand Up @@ -118,15 +122,25 @@ 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) {
imagePruner := NewImagePruner(s.apiClient(), project)
pruneOpts := ImagePruneOptions{
Mode: ImagePruneMode(options.Images),
RemoveOrphans: options.RemoveOrphans,
}
images, err := imagePruner.ImagesToPrune(ctx, pruneOpts)
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 {
Expand Down Expand Up @@ -190,19 +204,6 @@ 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{}{}
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 != "") {
continue
}
image = api.GetImageNameOrDefault(service, project.Name)
images[image] = struct{}{}
}
return images
}

func (s *composeService) removeImage(ctx context.Context, image string, w progress.Writer) error {
id := fmt.Sprintf("Image %s", image)
w.Event(progress.NewEvent(id, progress.Working, "Removing"))
Expand Down
147 changes: 103 additions & 44 deletions pkg/compose/down_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -149,40 +152,112 @@ 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 := composeService{
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().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()

imagesToBeInspected := map[string]bool{
"testproject-local-anonymous": true,
"local-named-image": true,
"remote-image": true,
"testproject-no-images-anonymous": false,
"missing-named-image": 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().ImageInspectWithRaw(gomock.Any(), img).
Return(resp, nil, err).
AnyTimes()
}

api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
[]moby.Container{container}, 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()

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)
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)
}

api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
api.EXPECT().ContainerRemove(gomock.Any(), "123", moby.ContainerRemoveOptions{Force: true}).Return(nil)
t.Log("-> docker compose down --rmi=local")
opts.Images = "local"
err := tested.Down(context.Background(), strings.ToLower(testProject), opts)
assert.NilError(t, err)

api.EXPECT().ImageRemove(gomock.Any(), "testproject-service1", moby.ImageRemoveOptions{}).Return(nil, nil)
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)
}

err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "local"})
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()

Expand All @@ -205,39 +280,23 @@ func TestDownRemoveImageLocalNoLabel(t *testing.T) {
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)

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 := composeService{
dockerCli: cli,
}
cli.EXPECT().Client().Return(api).AnyTimes()

api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
[]moby.Container{testContainer("service1", "123", false)}, 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().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().ImageInspectWithRaw(gomock.Any(), "testproject-service1").
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(), "service1-img", moby.ImageRemoveOptions{}).Return(nil, 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: "all"})
err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{Images: "local"})
assert.NilError(t, err)
}
Loading