summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNalin Dahyabhai <nalin@redhat.com>2021-04-14 13:35:43 -0400
committerNalin Dahyabhai <nalin@redhat.com>2021-04-20 15:44:39 -0400
commitfd9dd7065d440f9d44d51ddccbda20c01ccdc1aa (patch)
tree70ff833a098443b7993195c41b67b3741c7b6876
parent2a32fc3e403e4b70fb68fda564cbdf33b7dd5326 (diff)
downloadpodman-fd9dd7065d440f9d44d51ddccbda20c01ccdc1aa.tar.gz
podman-fd9dd7065d440f9d44d51ddccbda20c01ccdc1aa.tar.bz2
podman-fd9dd7065d440f9d44d51ddccbda20c01ccdc1aa.zip
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>
-rw-r--r--libpod/image/image.go3
-rw-r--r--libpod/runtime_img.go3
-rw-r--r--pkg/domain/infra/abi/images.go4
-rw-r--r--test/system/330-corrupt-images.bats134
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