diff --git a/pkg/compose/image_pruner.go b/pkg/compose/image_pruner.go index 0d332fa82..6babb43f6 100644 --- a/pkg/compose/image_pruner.go +++ b/pkg/compose/image_pruner.go @@ -62,7 +62,6 @@ func (p *ImagePruner) ImagesToPrune(ctx context.Context, opts ImagePruneOptions) } else if opts.Mode != ImagePruneLocal && opts.Mode != ImagePruneAll { return nil, fmt.Errorf("unsupported image prune mode: %s", opts.Mode) } - var images []string if opts.Mode == ImagePruneAll { @@ -73,28 +72,38 @@ func (p *ImagePruner) ImagesToPrune(ctx context.Context, opts ImagePruneOptions) images = append(images, namedImages...) } - projectImages, err := p.builtImagesForProject(ctx) + projectImages, err := p.labeledLocalImages(ctx) if err != nil { return nil, err } - for _, img := range projectImages { if len(img.RepoTags) == 0 { - // currently, we're only removing the tagged references, but + // currently, we're only pruning 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 } - removeImage := opts.RemoveOrphans - if !removeImage { + var shouldPrune bool + if opts.RemoveOrphans { + // indiscriminately prune all project images even if they're not + // referenced by the current Compose state (e.g. the service was + // removed from YAML) + shouldPrune = true + } else { + // only prune the image if it belongs to a known service for the + // project AND is either an implicitly-named, locally-built image + // or `--rmi=all` has been specified. + // TODO(milas): now that Compose labels the images it builds, this + // makes less sense; arguably, locally-built but explicitly-named + // images should be removed with `--rmi=local` as well. service, err := p.project.GetService(img.Labels[api.ServiceLabel]) if err == nil && (opts.Mode == ImagePruneAll || service.Image == "") { - removeImage = true + shouldPrune = true } } - if removeImage { + if shouldPrune { images = append(images, img.RepoTags[0]) } } @@ -109,7 +118,28 @@ func (p *ImagePruner) ImagesToPrune(ctx context.Context, opts ImagePruneOptions) return images, nil } -func (p *ImagePruner) builtImagesForProject(ctx context.Context) ([]moby.ImageSummary, error) { +// namedImages are those that are explicitly named in the service config. +// +// These could be registry-only images (no local build), hybrid (support build +// as a fallback if cannot pull), or local-only (image does not exist in a +// registry). +func (p *ImagePruner) namedImages(ctx context.Context) ([]string, error) { + var images []string + for _, service := range p.project.Services { + if service.Image == "" { + continue + } + images = append(images, service.Image) + } + return p.filterImagesByExistence(ctx, images) +} + +// labeledLocalImages are images that were locally-built by a current version of +// Compose (it did not always label built images). +// +// The image name could either have been defined by the user or implicitly +// created from the project + service name. +func (p *ImagePruner) labeledLocalImages(ctx context.Context) ([]moby.ImageSummary, error) { imageListOpts := moby.ImageListOptions{ Filters: filters.NewArgs( projectFilter(p.project.Name), @@ -129,17 +159,33 @@ func (p *ImagePruner) builtImagesForProject(ctx context.Context) ([]moby.ImageSu return projectImages, nil } -func (p *ImagePruner) namedImages(ctx context.Context) ([]string, error) { +// unlabeledLocalImages are images that match the implicit naming convention +// for locally-built images but did not get labeled, presumably because they +// were produced by an older version of Compose. +// +// This is transitional to ensure `down` continues to work as expected on +// projects built/launched by previous versions of Compose. It can safely +// be removed after some time. +func (p *ImagePruner) unlabeledLocalImages(ctx context.Context) ([]string, error) { var images []string for _, service := range p.project.Services { - if service.Image == "" { + if service.Image != "" { continue } - images = append(images, service.Image) + img := api.GetImageNameOrDefault(service, p.project.Name) + images = append(images, img) } return p.filterImagesByExistence(ctx, images) } +// filterImagesByExistence returns the subset of images that exist in the +// engine store. +// +// NOTE: Any transient errors communicating with the API will result in an +// image being returned as "existing", as this method is exclusively used to +// find images to remove, so the worst case of being conservative here is an +// attempt to remove an image that doesn't exist, which will cause a warning +// but is otherwise harmless. func (p *ImagePruner) filterImagesByExistence(ctx context.Context, imageNames []string) ([]string, error) { var mu sync.Mutex var ret []string @@ -168,18 +214,7 @@ func (p *ImagePruner) filterImagesByExistence(ctx context.Context, imageNames [] return ret, nil } -func (p *ImagePruner) unlabeledLocalImages(ctx context.Context) ([]string, error) { - var images []string - for _, service := range p.project.Services { - if service.Image != "" { - continue - } - img := api.GetImageNameOrDefault(service, p.project.Name) - images = append(images, img) - } - return p.filterImagesByExistence(ctx, images) -} - +// normalizeAndDedupeImages returns the unique set of images after normalization. func normalizeAndDedupeImages(images []string) []string { seen := make(map[string]struct{}, len(images)) for _, img := range images { @@ -197,6 +232,7 @@ func normalizeAndDedupeImages(images []string) []string { for v := range seen { ret = append(ret, v) } + // ensure a deterministic return result - the actual ordering is not useful sort.Strings(ret) return ret } diff --git a/pkg/e2e/fixtures/build-dependencies/compose.yaml b/pkg/e2e/fixtures/build-dependencies/compose.yaml index c974b7241..7de1960b4 100644 --- a/pkg/e2e/fixtures/build-dependencies/compose.yaml +++ b/pkg/e2e/fixtures/build-dependencies/compose.yaml @@ -10,5 +10,3 @@ services: build: context: . dockerfile: service.dockerfile - nginx: - image: nginx