From 3bc6072142d0778d17a130c1f7eaf90b72156e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 18 Oct 2024 23:22:48 +0200 Subject: [PATCH 1/5] Fix the store choice in "podman pull image with additional store" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test got the stores RW status backwards. Before zstd:chunked, both image IDs should be the same, so this used to make no difference. Signed-off-by: Miloslav Trmač --- test/system/010-images.bats | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 1548615a71..8ef2456961 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -380,11 +380,11 @@ EOF # present in store, and if it is it will precede $IMAGE. CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Repository}}:{{.Tag}} {{.ReadOnly}}" assert "${#lines[*]}" -ge 2 "at least 2 lines from 'podman images'" - is "${lines[-2]}" "$IMAGE false" "image from readonly store" - is "${lines[-1]}" "$IMAGE true" "image from readwrite store" + is "${lines[-2]}" "$IMAGE false" "image from readwrite store" + is "${lines[-1]}" "$IMAGE true" "image from readonly store" CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Id}}" - id=${lines[-1]} + id=${lines[-2]} CONTAINERS_STORAGE_CONF=$sconf run_podman pull -q $IMAGE is "$output" "$id" "pull -q $IMAGE, using storage.conf" From bf8f2b55513898409745f012a5974cd24591e835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 17 Oct 2024 22:27:17 +0200 Subject: [PATCH 2/5] Simplify the additional store test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When looking up the current-store image ID, do that from the same output where we verify that the ID is from the current store, instead of listing images twice. Signed-off-by: Miloslav Trmač --- test/system/010-images.bats | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 8ef2456961..894c4e3aab 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -378,13 +378,11 @@ EOF # IMPORTANT! Use -2/-1 indices, not 0/1, because $SYSTEMD_IMAGE may be # present in store, and if it is it will precede $IMAGE. - CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Repository}}:{{.Tag}} {{.ReadOnly}}" + CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.ID}} {{.Repository}}:{{.Tag}} {{.ReadOnly}}" assert "${#lines[*]}" -ge 2 "at least 2 lines from 'podman images'" - is "${lines[-2]}" "$IMAGE false" "image from readwrite store" - is "${lines[-1]}" "$IMAGE true" "image from readonly store" - - CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Id}}" - id=${lines[-2]} + assert "${lines[-2]}" =~ ".*$IMAGE false" "image from readwrite store" + assert "${lines[-1]}" =~ ".*$IMAGE true" "image from readonly store" + id=${lines[-2]%% *} CONTAINERS_STORAGE_CONF=$sconf run_podman pull -q $IMAGE is "$output" "$id" "pull -q $IMAGE, using storage.conf" From 1d7ec1ef5fc778cf0a188f2605524f0169678fed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 17 Oct 2024 21:50:18 +0200 Subject: [PATCH 3/5] Use the config digest to compare images loaded/pulled using different methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Historically, non-schema1 images had a deterministic image ID == config digest. With zstd:chunked, we don't want to deduplicate layers pulled by consuming the full tarball and layers partially pulled based on TOC, because we can't cheaply ensure equivalence; so, image IDs for images where a TOC was used differ. To accommodate that, compare images using their configs digests, not using image IDs. Signed-off-by: Miloslav Trmač --- test/system/010-images.bats | 6 +++++- test/system/120-load.bats | 34 ++++++++++++++++++---------------- test/system/150-login.bats | 18 +++++++++--------- test/system/helpers.bash | 11 +++++++++++ 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 894c4e3aab..293145757c 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -383,9 +383,13 @@ EOF assert "${lines[-2]}" =~ ".*$IMAGE false" "image from readwrite store" assert "${lines[-1]}" =~ ".*$IMAGE true" "image from readonly store" id=${lines[-2]%% *} + local config_digest; config_digest=$(image_config_digest "@$id") # Without $sconf, i.e. from the read-write store. CONTAINERS_STORAGE_CONF=$sconf run_podman pull -q $IMAGE - is "$output" "$id" "pull -q $IMAGE, using storage.conf" + # This is originally a regression test, (podman pull) used to output multiple image IDs. Ensure it only prints one. + assert "${#lines[*]}" -le 1 "Number of output lines from podman pull" + local config_digest2; config_digest2=$(image_config_digest "@$output") + assert "$config_digest2" = "$config_digest" "pull -q $IMAGE, using storage.conf" run_podman --root $imstore/root rmi --all } diff --git a/test/system/120-load.bats b/test/system/120-load.bats index 3771825f17..e99d6cfef1 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -23,22 +23,24 @@ function teardown() { # Custom helpers for this test only. These just save us having to duplicate # the same thing four times (two tests, each with -i and stdin). # -# initialize, read image ID and name -get_iid_and_name() { +# initialize, read image ID, image config digest, and name +get_img_ids_and_name() { run_podman images -a --format '{{.ID}} {{.Repository}}:{{.Tag}}' read iid img_name <<<"$output" + img_config_digest=$(image_config_digest "@$iid") archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar } -# Simple verification of image ID and name -verify_iid_and_name() { +# Simple verification of image config digest and name +verify_img_config_digest_and_name() { run_podman images -a --format '{{.ID}} {{.Repository}}:{{.Tag}}' read new_iid new_img_name < <(echo "$output") + new_img_config_digest=$(image_config_digest "@$new_iid") # Verify - is "$new_iid" "$iid" "Image ID of loaded image == original" - is "$new_img_name" "$1" "Name & tag of restored image" + is "$new_img_config_digest" "$img_config_digest" "Image config digest of loaded image == original" + is "$new_img_name" "$1" "Name & tag of restored image" } @test "podman load invalid file" { @@ -178,7 +180,7 @@ verify_iid_and_name() { @test "podman load - by image ID" { # FIXME: how to build a simple archive instead? - get_iid_and_name + get_img_ids_and_name # Save image by ID, and remove it. run_podman save $iid -o $archive @@ -186,41 +188,41 @@ verify_iid_and_name() { # Load using -i; IID should be preserved, but name is not. run_podman load -i $archive - verify_iid_and_name ":" + verify_img_config_digest_and_name ":" # Same as above, using stdin run_podman rmi $iid run_podman load < $archive - verify_iid_and_name ":" + verify_img_config_digest_and_name ":" # Same as above, using stdin but with `podman image load` run_podman rmi $iid run_podman image load < $archive - verify_iid_and_name ":" + verify_img_config_digest_and_name ":" } @test "podman load - by image name" { - get_iid_and_name + get_img_ids_and_name run_podman save $img_name -o $archive run_podman rmi $iid # Load using -i; this time the image should be tagged. run_podman load -i $archive - verify_iid_and_name $img_name + verify_img_config_digest_and_name $img_name run_podman rmi $iid # Also make sure that `image load` behaves the same. run_podman image load -i $archive - verify_iid_and_name $img_name + verify_img_config_digest_and_name $img_name run_podman rmi $iid # Same as above, using stdin run_podman load < $archive - verify_iid_and_name $img_name + verify_img_config_digest_and_name $img_name } @test "podman load - from URL" { - get_iid_and_name + get_img_ids_and_name run_podman save $img_name -o $archive run_podman rmi $iid @@ -234,7 +236,7 @@ verify_iid_and_name() { $IMAGE /bin/busybox-extras httpd -f -p 80 run_podman load -i $SERVER/image.tar - verify_iid_and_name $img_name + verify_img_config_digest_and_name $img_name run_podman rm -f -t0 myweb } diff --git a/test/system/150-login.bats b/test/system/150-login.bats index b05d4e7cdb..49f24632b5 100644 --- a/test/system/150-login.bats +++ b/test/system/150-login.bats @@ -149,9 +149,8 @@ EOF } function _push_search_test() { - # Preserve image ID for later comparison against push/pulled image - run_podman inspect --format '{{.Id}}' $IMAGE - iid=$output + # Look up image config digest for later comparison against push/pulled image + local img_config_digest; img_config_digest=$(image_config_digest $IMAGE) destname=ok-$(random_string 10 | tr A-Z a-z)-ok # Use command-line credentials @@ -188,8 +187,8 @@ function _push_search_test() { localhost:${PODMAN_LOGIN_REGISTRY_PORT}/$destname # Compare to original image - run_podman inspect --format '{{.Id}}' $destname - is "$output" "$iid" "Image ID of pulled image == original IID" + local img_config_digest2; img_config_digest2=$(image_config_digest localhost:${PODMAN_LOGIN_REGISTRY_PORT}/$destname) + assert "$img_config_digest2" = "$img_config_digest" "config digest of pulled image == original digest" run_podman rmi $destname } @@ -345,12 +344,12 @@ function _test_skopeo_credential_sharing() { $image1 run_podman rmi $image1 - run_podman images $IMAGE --format {{.ID}} - local podman_image_id=$output + local podman_image_config_digest=$(image_config_digest $IMAGE) run_podman pull -q --retry 4 --retry-delay "0s" --authfile=$authfile \ --tls-verify=false $image1 - assert "${output:0:12}" = "$podman_image_id" "First pull (before stopping registry)" + local pulled_image_config_digest; pulled_image_config_digest=$(image_config_digest @$output) + assert "$pulled_image_config_digest" = "$podman_image_config_digest" "First pull (before stopping registry)" run_podman rmi $image1 # This actually STOPs the registry, so the port is unbound... @@ -360,7 +359,8 @@ function _test_skopeo_credential_sharing() { run_podman 0+w pull -q --retry 4 --retry-delay "5s" --authfile=$authfile \ --tls-verify=false $image1 assert "$output" =~ "Failed, retrying in 5s.*Error: initializing.* connection refused" - assert "${lines[-1]:0:12}" = "$podman_image_id" "push should succeed via retry" + local pulled_image_config_digest2; pulled_image_config_digest2=$(image_config_digest "@${lines[-1]}") + assert "$pulled_image_config_digest2" = "$podman_image_config_digest" "push should succeed via retry" unpause_registry run_podman rmi $image1 diff --git a/test/system/helpers.bash b/test/system/helpers.bash index 6f13394b7d..65ae66b9d3 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -847,6 +847,17 @@ function _ensure_container_running() { die "Timed out waiting for container $1 to enter state running=$2" } +# Return the config digest of an image in containers-storage. +# The input can be a named reference, or an @imageID (including shorter imageID prefixes) +# Historically, the image ID was a good indicator of “the same” image; +# with zstd:chunked, the same image might have different IDs depending on whether +# creating layers happened based on the TOC (and per-file operations) or the full layer tarball +function image_config_digest() { + local sha_output; sha_output=$(skopeo inspect --raw --config "containers-storage:$1" | sha256sum) + # strip filename ("-") from sha output + echo "${sha_output%% *}" +} + ########################### # _add_label_if_missing # make sure skip messages include rootless/remote ########################### From 77ef28c14f6f881d62a7904916f1e5fc096165e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 17 Oct 2024 21:51:44 +0200 Subject: [PATCH 4/5] Try to repair c/storage after removing an additional image store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The additional image store feature assumes that images / layers in the additional store never go away, while we do remove it after this test. Try to repair the store. Signed-off-by: Miloslav Trmač --- test/system/010-images.bats | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 293145757c..a0beda41f6 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -391,6 +391,12 @@ EOF local config_digest2; config_digest2=$(image_config_digest "@$output") assert "$config_digest2" = "$config_digest" "pull -q $IMAGE, using storage.conf" + # $IMAGE might now be reusing layers from the additional store; + # Removing the additional store underneath can result in dangling layer references. + # Try to fix that up. + CONTAINERS_STORAGE_CONF=$sconf run_podman rmi $IMAGE + _prefetch $IMAGE + run_podman --root $imstore/root rmi --all } From 6fd0e227b49426eba5678fa4ec3ac4393ec36beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 17 Oct 2024 22:30:29 +0200 Subject: [PATCH 5/5] Improve "podman load - from URL" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't assume that the loaded image will be deduplicated with the server image. Signed-off-by: Miloslav Trmač --- test/system/120-load.bats | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/system/120-load.bats b/test/system/120-load.bats index e99d6cfef1..09ef6c907b 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -234,11 +234,21 @@ verify_img_config_digest_and_name() { -v $archive:/var/www/image.tar:Z \ -w /var/www \ $IMAGE /bin/busybox-extras httpd -f -p 80 - + # We now have $IMAGE pointing at the image, possibly using a zstd:chunked (TOC-based) pull run_podman load -i $SERVER/image.tar - verify_img_config_digest_and_name $img_name + # This should move the $img_name tag ( = $IMAGE) to the result of loading the image; + # this is a non-TOC-based load, so it might or might not deduplicate the loaded image with + # the one for myweb. + # So, if we have an untagged image, it’s probably the one for myweb, and try to remove it. run_podman rm -f -t0 myweb + run_podman images -a --format '{{.ID}} {{.Repository}}:{{.Tag}}' + local myweb_iid=$(echo "$output" | sed -n '/:/s/ .*$//p') + if [[ -n "$myweb_iid" ]]; then + run_podman rmi $myweb_iid + fi + + verify_img_config_digest_and_name $img_name } @test "podman load - redirect corrupt payload" {