image prune: remove unused images only with --all

Fix a regression in `podman image prune` where unused images were
accidentally removed even when `--all=false`.  Extend and partially
rewrite the e2e tests to make sure we're not regressing again in the
future.

Fixing the aforementioned issue revealed another issue in the default
prune filter.  While prune should remove all "dangling" images (i.e.,
those without tag), it removed only "intermediate" ones; dangling images
without children.  Remove the mistaken comment from the libimage
migration.

Also clarify the help message and man page.

Fixes: #10350
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

cmd/podman/images/prune.go

@@ -15,9 +15,7 @@ import (
 )
 
 var (
-	pruneDescription = `Removes all unnamed images from local storage.
-
-  If an image is not being used by a container, it will be removed from the system.`
+	pruneDescription = `Removes dangling or unused images from local storage.`
 	pruneCmd         = &cobra.Command{
 		Use:               "prune [options]",
 		Args:              validate.NoArgs,
@@ -41,7 +39,7 @@ func init() {
 	})
 
 	flags := pruneCmd.Flags()
-	flags.BoolVarP(&pruneOpts.All, "all", "a", false, "Remove all unused images, not just dangling ones")
+	flags.BoolVarP(&pruneOpts.All, "all", "a", false, "Remove all images not in use by containers, not just dangling ones")
 	flags.BoolVarP(&force, "force", "f", false, "Do not prompt for confirmation")
 
 	filterFlagName := "filter"

docs/source/markdown/podman-image-prune.1.md

@@ -8,8 +8,7 @@ podman-image-prune - Remove all unused images from the local store
 
 ## DESCRIPTION
 **podman image prune** removes all dangling images from local storage. With the `all` option,
-you can delete all unused images.  Unused images are dangling images as well as any image that
-does not have any containers based on it.
+you can delete all unused images (i.e., images not in use by any container).
 
 The image prune command does not prune cache images that only use layers that are necessary for other images.
 

pkg/domain/infra/abi/images.go

@@ -40,25 +40,13 @@ func (ir *ImageEngine) Exists(_ context.Context, nameOrID string) (*entities.Boo
 }
 
 func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOptions) ([]*reports.PruneReport, error) {
-	// NOTE: the terms "dangling" and "intermediate" are not used
-	// consistently across our code base.  In libimage, "dangling" means
-	// that an image has no tags.  "intermediate" means that an image is
-	// dangling and that no other image depends on it (i.e., has no
-	// children).
-	//
-	// While pruning usually refers to "dangling" images, it has always
-	// removed "intermediate" ones.
-	defaultOptions := &libimage.RemoveImagesOptions{
-		Filters:  append(opts.Filter, "intermediate=true", "containers=false", "readonly=false"),
+	pruneOptions := &libimage.RemoveImagesOptions{
+		Filters:  append(opts.Filter, "containers=false", "readonly=false"),
 		WithSize: true,
 	}
 
-	// `image prune --all` means to *also* remove images which are not in
-	// use by any container.  Since image filters are chained, we need to
-	// do two look ups since the default ones are a subset of all.
-	unusedOptions := &libimage.RemoveImagesOptions{
-		Filters:  append(opts.Filter, "containers=false", "readonly=false"),
-		WithSize: true,
+	if !opts.All {
+		pruneOptions.Filters = append(pruneOptions.Filters, "dangling=true")
 	}
 
 	var pruneReports []*reports.PruneReport
@@ -66,16 +54,12 @@ func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOption
 	// Now prune all images until we converge.
 	numPreviouslyRemovedImages := 1
 	for {
-		removedDefault, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, nil, defaultOptions)
-		if rmErrors != nil {
-			return nil, errorhandling.JoinErrors(rmErrors)
-		}
-		removedUnused, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, nil, unusedOptions)
+		removedImages, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, nil, pruneOptions)
 		if rmErrors != nil {
 			return nil, errorhandling.JoinErrors(rmErrors)
 		}
 
-		for _, rmReport := range append(removedDefault, removedUnused...) {
+		for _, rmReport := range removedImages {
 			r := *rmReport
 			pruneReports = append(pruneReports, &reports.PruneReport{
 				Id:   r.ID,
@@ -83,7 +67,7 @@ func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOption
 			})
 		}
 
-		numRemovedImages := len(removedDefault) + len(removedUnused)
+		numRemovedImages := len(removedImages)
 		if numRemovedImages+numPreviouslyRemovedImages == 0 {
 			break
 		}

test/e2e/prune_test.go

@@ -88,6 +88,53 @@ var _ = Describe("Podman prune", func() {
 		Expect(podmanTest.NumberOfContainers()).To(Equal(0))
 	})
 
+	It("podman image prune - remove only dangling images", func() {
+		session := podmanTest.Podman([]string{"images", "-a"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		hasNone, _ := session.GrepString("<none>")
+		Expect(hasNone).To(BeFalse())
+		numImages := len(session.OutputToStringArray())
+
+		// Since there's no dangling image, none should be removed.
+		session = podmanTest.Podman([]string{"image", "prune", "-f"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(len(session.OutputToStringArray())).To(Equal(0))
+
+		// Let's be extra sure that the same number of images is
+		// reported.
+		session = podmanTest.Podman([]string{"images", "-a"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(len(session.OutputToStringArray())).To(Equal(numImages))
+
+		// Now build a new image with dangling intermediate images.
+		podmanTest.BuildImage(pruneImage, "alpine_bash:latest", "true")
+
+		session = podmanTest.Podman([]string{"images", "-a"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		hasNone, _ = session.GrepString("<none>")
+		Expect(hasNone).To(BeTrue()) // ! we have dangling ones
+		numImages = len(session.OutputToStringArray())
+
+		// Since there's at least one dangling image, prune should
+		// remove them.
+		session = podmanTest.Podman([]string{"image", "prune", "-f"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		numPrunedImages := len(session.OutputToStringArray())
+		Expect(numPrunedImages >= 1).To(BeTrue())
+
+		// Now make sure that exactly the number of pruned images has
+		// been removed.
+		session = podmanTest.Podman([]string{"images", "-a"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(len(session.OutputToStringArray())).To(Equal(numImages - numPrunedImages))
+	})
+
 	It("podman image prune skip cache images", func() {
 		podmanTest.BuildImage(pruneImage, "alpine_bash:latest", "true")