From d458b28fcf930ec306669d70c04b309c70f1130b Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 17 Jun 2021 09:45:53 +0200 Subject: [PATCH] [0.38] libimage: force remove: only untag on multi tag image When removing an image by name, do not remove the image and all its tags, even if force is set. Instead, just untag the specified name. Note: adjust the load test to preserve the order in the untagged field. Also vendor in the latest HEAD in containers/image to fix a bug revealed in Podman CI. Backport of commit c6578d76fb0da6f6a87d62ac33306945dc616205. Context: containers/podman/issues/10685 Signed-off-by: Valentin Rothberg --- libimage/image.go | 16 ++++++++------- libimage/load_test.go | 2 +- libimage/remove_test.go | 44 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 libimage/remove_test.go diff --git a/libimage/image.go b/libimage/image.go index 1f76d4ae5..c9ebb4a03 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -329,17 +329,19 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport, // an `rmi foo` will not untag "foo" but instead attempt to remove the // entire image. If there's a container using "foo", we should get an // error. - if options.Force || referencedBy == "" || numNames == 1 { + if referencedBy == "" || numNames == 1 { // DO NOTHING, the image will be removed } else { byID := strings.HasPrefix(i.ID(), referencedBy) byDigest := strings.HasPrefix(referencedBy, "sha256:") - if byID && numNames > 1 { - return errors.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names()) - } else if byDigest && numNames > 1 { - // FIXME - Docker will remove the digest but containers storage - // does not support that yet, so our hands are tied. - return errors.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names()) + if !options.Force { + if byID && numNames > 1 { + return errors.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names()) + } else if byDigest && numNames > 1 { + // FIXME - Docker will remove the digest but containers storage + // does not support that yet, so our hands are tied. + return errors.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names()) + } } // Only try to untag if we know it's not an ID or digest. diff --git a/libimage/load_test.go b/libimage/load_test.go index 0a6d95d3d..8464a3655 100644 --- a/libimage/load_test.go +++ b/libimage/load_test.go @@ -55,7 +55,7 @@ func TestLoad(t *testing.T) { } // Now remove the image. - rmReports, rmErrors := runtime.RemoveImages(ctx, loadedImages, &RemoveImagesOptions{Force: true}) + rmReports, rmErrors := runtime.RemoveImages(ctx, ids, &RemoveImagesOptions{Force: true}) require.Len(t, rmErrors, 0) require.Len(t, rmReports, test.numImages) diff --git a/libimage/remove_test.go b/libimage/remove_test.go new file mode 100644 index 000000000..3500dd2ea --- /dev/null +++ b/libimage/remove_test.go @@ -0,0 +1,44 @@ +package libimage + +import ( + "context" + "os" + "testing" + + "github.com/containers/common/pkg/config" + "github.com/stretchr/testify/require" +) + +func TestRemoveImages(t *testing.T) { + // Note: this will resolve pull from the GCR registry (see + // testdata/registries.conf). + busyboxLatest := "docker.io/library/busybox:latest" + + runtime, cleanup := testNewRuntime(t) + defer cleanup() + ctx := context.Background() + + pullOptions := &PullOptions{} + pullOptions.Writer = os.Stdout + pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyAlways, pullOptions) + require.NoError(t, err) + require.Len(t, pulledImages, 1) + + err = pulledImages[0].Tag("foobar") + require.NoError(t, err) + + // containers/podman/issues/10685 - force removal on image with + // multiple tags will only untag but not remove all tags including the + // image. + rmReports, rmErrors := runtime.RemoveImages(ctx, []string{"foobar"}, &RemoveImagesOptions{Force: true}) + require.Nil(t, rmErrors) + require.Len(t, rmReports, 1) + require.Equal(t, pulledImages[0].ID(), rmReports[0].ID) + require.False(t, rmReports[0].Removed) + require.Equal(t, []string{"localhost/foobar:latest"}, rmReports[0].Untagged) + + // The busybox image is still present even if foobar was force removed. + exists, err := runtime.Exists(busyboxLatest) + require.NoError(t, err) + require.True(t, exists) +}