rmi: don't break when the image is missing a manifest

In libpod/image.Image.Remove(), if the attempt to find the image's
parent fails for any reason, log a warning and proceed as though it
didn't have one instead of failing, which would leave us unable to
remove the image without resetting everything.

In libpod/Runtime.RemoveImage(), if we can't determine if an image has
children, log a warning, and assume that it doesn't have any instead of
failing, which would leave us unable to remove the image without
resetting everything.

In pkg/domain/infra/abi.ImageEngine.Remove(), when attempting to remove
all images, if we encounter an error checking if a given image has
children, log a warning, and assume that it doesn't have any instead of
failing, which would leave us unable to remove the image without
resetting everything.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

4 files changed, 140 insertions, 4 deletions

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 12dc22360..3c9fb3a37 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -617,7 +617,8 @@ func (i *Image) TopLayer() string {
 func (i *Image) Remove(ctx context.Context, force bool) error {
 	parent, err := i.GetParent(ctx)
 	if err != nil {
-		return err
+		logrus.Warnf("error determining parent of image: %v, ignoring the error", err)
+		parent = nil
 	}
 	if _, err := i.imageruntime.store.DeleteImage(i.ID(), true); err != nil {
 		return err
diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go
index 3588467a5..2b101c01f 100644
--- a/libpod/runtime_img.go
+++ b/libpod/runtime_img.go
@@ -66,7 +66,8 @@ func (r *Runtime) RemoveImage(ctx context.Context, img *image.Image, force bool)
 
 	hasChildren, err := img.IsParent(ctx)
 	if err != nil {
-		return nil, err
+		logrus.Warnf("error determining if an image is a parent: %v, ignoring the error", err)
+		hasChildren = false
 	}
 
 	if (len(img.Names()) > 1 && !img.InputIsID()) || hasChildren {
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index c02eb2bfc..84c7ebecd 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/pkg/domain/infra/abi/images.go
@@ -620,8 +620,8 @@ func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entitie
 			for _, img := range storageImages {
 				isParent, err := img.IsParent(ctx)
 				if err != nil {
-					rmErrors = append(rmErrors, err)
-					continue
+					logrus.Warnf("%v, ignoring the error", err)
+					isParent = false
 				}
 				// Skip parent images.
 				if isParent {
diff --git a/test/system/330-corrupt-images.bats b/test/system/330-corrupt-images.bats
new file mode 100644
index 000000000..9836de363
--- /dev/null
+++ b/test/system/330-corrupt-images.bats
@@ -0,0 +1,134 @@
+#!/usr/bin/env bats   -*- bats -*-
+#
+# All tests in here perform nasty manipulations on image storage.
+#
+
+load helpers
+
+###############################################################################
+# BEGIN setup/teardown
+
+# Create a scratch directory; this is what we'll use for image store and cache
+if [ -z "${PODMAN_CORRUPT_TEST_WORKDIR}" ]; then
+    export PODMAN_CORRUPT_TEST_WORKDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} podman_corrupt_test.XXXXXX)
+fi
+
+PODMAN_CORRUPT_TEST_IMAGE_FQIN=quay.io/libpod/alpine@sha256:634a8f35b5f16dcf4aaa0822adc0b1964bb786fca12f6831de8ddc45e5986a00
+PODMAN_CORRUPT_TEST_IMAGE_ID=961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4
+
+# All tests in this file (and ONLY in this file) run with a custom rootdir
+function setup() {
+    skip_if_remote "none of these tests run under podman-remote"
+    _PODMAN_TEST_OPTS="--root ${PODMAN_CORRUPT_TEST_WORKDIR}/root"
+}
+
+function teardown() {
+    # No other tests should ever run with this custom rootdir
+    unset _PODMAN_TEST_OPTS
+
+    is_remote && return
+
+    # Clean up
+    umount ${PODMAN_CORRUPT_TEST_WORKDIR}/root/overlay || true
+    if is_rootless; then
+        run_podman unshare rm -rf ${PODMAN_CORRUPT_TEST_WORKDIR}/root
+    else
+        rm -rf ${PODMAN_CORRUPT_TEST_WORKDIR}/root
+    fi
+}
+
+# END   setup/teardown
+###############################################################################
+# BEGIN primary test helper
+
+# This is our main action, invoked by every actual test. It:
+#    - creates a new empty rootdir
+#    - populates it with our crafted test image
+#    - removes [ manifest, blob ]
+#    - confirms that "podman images" throws an error
+#    - runs the specified command (rmi -a -f, prune, reset, etc)
+#    - confirms that it succeeds, and also emits expected warnings
+function _corrupt_image_test() {
+    # Run this test twice: once removing manifest, once removing blob
+    for what_to_rm in manifest blob; do
+        # I have no idea, but this sometimes remains mounted
+        umount ${PODMAN_CORRUPT_TEST_WORKDIR}/root/overlay || true
+        # Start with a fresh storage root, load prefetched image into it.
+        /bin/rm -rf ${PODMAN_CORRUPT_TEST_WORKDIR}/root
+        mkdir -p ${PODMAN_CORRUPT_TEST_WORKDIR}/root
+        run_podman load -i ${PODMAN_CORRUPT_TEST_WORKDIR}/img.tar
+        # "podman load" restores it without a tag, which (a) causes rmi-by-name
+        # to fail, and (b) causes "podman images" to exit 0 instead of 125
+        run_podman tag ${PODMAN_CORRUPT_TEST_IMAGE_ID} ${PODMAN_CORRUPT_TEST_IMAGE_FQIN}
+
+        # shortcut variable name
+        local id=${PODMAN_CORRUPT_TEST_IMAGE_ID}
+
+        case "$what_to_rm" in
+            manifest)  rm_path=manifest ;;
+            blob)      rm_path="=$(echo -n "sha256:$id" | base64 -w0)" ;;
+            *)         die "Internal error: unknown action '$what_to_rm'" ;;
+        esac
+
+        # Corruptify, and confirm that 'podman images' throws an error
+        rm -v ${PODMAN_CORRUPT_TEST_WORKDIR}/root/*-images/$id/${rm_path}
+        run_podman 125 images
+        is "$output" "Error: error retrieving label for image \"$id\": you may need to remove the image to resolve the error"
+
+        # Run the requested command. Confirm it succeeds, with suitable warnings
+        run_podman $*
+        is "$output" ".*error determining parent of image" \
+           "$* with missing $what_to_rm"
+
+        run_podman images -a --noheading
+        is "$output" "" "podman images -a, after $*, is empty"
+    done
+}
+
+# END   primary test helper
+###############################################################################
+# BEGIN first "test" does a one-time pull of our desired image
+
+@test "podman corrupt images - initialize" {
+    # Pull once, save cached copy.
+    run_podman pull $PODMAN_CORRUPT_TEST_IMAGE_FQIN
+    run_podman save -o ${PODMAN_CORRUPT_TEST_WORKDIR}/img.tar \
+               $PODMAN_CORRUPT_TEST_IMAGE_FQIN
+}
+
+# END   first "test" does a one-time pull of our desired image
+###############################################################################
+# BEGIN actual tests
+
+@test "podman corrupt images - rmi -f <image-id>" {
+    _corrupt_image_test "rmi -f ${PODMAN_CORRUPT_TEST_IMAGE_ID}"
+}
+
+@test "podman corrupt images - rmi -f <image-name>" {
+    _corrupt_image_test "rmi -f ${PODMAN_CORRUPT_TEST_IMAGE_FQIN}"
+}
+
+@test "podman corrupt images - rmi -f -a" {
+    _corrupt_image_test "rmi -f -a"
+}
+
+@test "podman corrupt images - image prune" {
+    _corrupt_image_test "image prune -a -f"
+}
+
+@test "podman corrupt images - system reset" {
+    _corrupt_image_test "image prune -a -f"
+}
+
+# END   actual tests
+###############################################################################
+# BEGIN final cleanup
+
+@test "podman corrupt images - cleanup" {
+    rm -rf ${PODMAN_CORRUPT_TEST_WORKDIR}
+}
+
+# END   final cleanup
+###############################################################################
+
+# vim: filetype=sh