From dde6bcbca3149a0fb214d4079fa45fd7b7add727 Mon Sep 17 00:00:00 2001
From: Ed Santiago <santiago@redhat.com>
Date: Thu, 22 Jun 2023 11:54:16 -0600
Subject: [PATCH] system tests: add and use _prefetch

Add new _prefetch helper for fetching and caching images.
Use it in a few places, most importantly 120-load.bats
where our teardown() now runs 'rmi -af'.

Reason: in #17911 we discovered that podman save + load do
not actually preserve the image: annotations and other metadata
are lost. This means that a test which runs after 120-load.bats
is operating on a different $IMAGE than a test which runs before.

This is not a problem except in very obscure corner cases, like
one fixed in #18542, but it seems irresponsible to just handwave
that issue away

The _prefetch function uses skopeo for fetching and saving
images, because skopeo preserves digests and metadata.

[Side note for posterity: I tried amending basic_setup() to
always rmi -a + prefetch, instead of the current images -a +
rmi unwanted ones. That slowed down system tests by 10 minutes,
presumably because loads are much slower than queries. I reverted
that change and am documenting it as a reminder of why we do things
the way we do.]

Signed-off-by: Ed Santiago <santiago@redhat.com>
---
 test/system/001-basic.bats       |  4 +-
 test/system/010-images.bats      |  4 ++
 test/system/011-image.bats       |  8 +++-
 test/system/120-load.bats        | 25 +++++++----
 test/system/255-auto-update.bats |  3 ++
 test/system/260-sdnotify.bats    | 10 +----
 test/system/710-kube.bats        |  7 ++--
 test/system/helpers.bash         | 71 +++++++++++++++++++++++++++-----
 8 files changed, 99 insertions(+), 33 deletions(-)

diff --git a/test/system/001-basic.bats b/test/system/001-basic.bats
index e0c7d1a5e9..b6a88495c1 100644
--- a/test/system/001-basic.bats
+++ b/test/system/001-basic.bats
@@ -72,7 +72,9 @@ function setup() {
 }
 
 @test "podman can pull an image" {
-    run_podman rmi -a
+    run_podman rmi -a -f
+
+    # This is a risk point: it will fail if the registry or network are flaky
     run_podman pull $IMAGE
 
     # Regression test for https://github.com/containers/image/pull/1615
diff --git a/test/system/010-images.bats b/test/system/010-images.bats
index ca3fc78dbc..33818194a9 100644
--- a/test/system/010-images.bats
+++ b/test/system/010-images.bats
@@ -126,6 +126,10 @@ Labels.created_at | 20[0-9-]\\\+T[0-9:]\\\+Z
 # Regression test for https://github.com/containers/podman/issues/7651
 # in which "podman pull image-with-sha" causes "images -a" to crash
 @test "podman images -a, after pulling by sha " {
+    # This test requires that $IMAGE be 100% the same as the registry one
+    run_podman rmi -a -f
+    _prefetch $IMAGE
+
     # Get a baseline for 'images -a'
     run_podman images -a
     local images_baseline="$output"
diff --git a/test/system/011-image.bats b/test/system/011-image.bats
index 5150e875ea..455c8e9b4f 100644
--- a/test/system/011-image.bats
+++ b/test/system/011-image.bats
@@ -30,8 +30,12 @@ EOF
 }
 
 function check_signature() {
+    # This test requires that $IMAGE be 100% the same as the registry one
+    run_podman rmi -a -f
+    _prefetch $IMAGE
+
     local sigfile=$1
-    ls -laR $PODMAN_TMPDIR/signatures
+    find $PODMAN_TMPDIR/signatures -print
     run_podman inspect --format '{{.Digest}}' $PODMAN_TEST_IMAGE_FQN
     local repodigest=${output/:/=}
 
@@ -47,7 +51,7 @@ function check_signature() {
 
 
 @test "podman image - sign with no sigfile" {
-    GNUPGHOME=$_GNUPGHOME_TMP run_podman image sign --sign-by foo@bar.com --directory $PODMAN_TMPDIR/signatures  "docker://$PODMAN_TEST_IMAGE_FQN"
+    GNUPGHOME=$_GNUPGHOME_TMP run_podman image sign --sign-by foo@bar.com --directory $PODMAN_TMPDIR/signatures  "containers-storage:$PODMAN_TEST_IMAGE_FQN"
     check_signature "signature-1"
 }
 
diff --git a/test/system/120-load.bats b/test/system/120-load.bats
index 904b5da49d..b1ba7e457b 100644
--- a/test/system/120-load.bats
+++ b/test/system/120-load.bats
@@ -6,6 +6,19 @@
 load helpers
 load helpers.network
 
+function teardown() {
+    # Destroy all images, to make sure we don't leave garbage behind.
+    #
+    # The tests in here do funky things with image store, including
+    # reloading the default $IMAGE in a way that appears normal but
+    # is not actually the same as what is normally pulled, e.g.,
+    # annotations and image digests may be different. See
+    # https://github.com/containers/podman/discussions/17911
+    run_podman rmi -a -f
+
+    basic_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).
 #
@@ -174,10 +187,6 @@ verify_iid_and_name() {
     run_podman rmi $iid
     run_podman image load < $archive
     verify_iid_and_name "<none>:<none>"
-
-    # Cleanup: since load-by-iid doesn't preserve name, re-tag it;
-    # otherwise our global teardown will rmi and re-pull our standard image.
-    run_podman tag $iid $img_name
 }
 
 @test "podman load - by image name" {
@@ -240,8 +249,8 @@ verify_iid_and_name() {
     img2="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_TEST_IMAGE_NAME:multiimage"
     archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar
 
-    run_podman pull $img1
-    run_podman pull $img2
+    _prefetch $img1
+    _prefetch $img2
 
     run_podman save -m -o $archive $img1 $img2
     run_podman rmi -f $img1 $img2
@@ -258,8 +267,8 @@ verify_iid_and_name() {
     img2="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_TEST_IMAGE_NAME:multiimage"
     archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar
 
-    run_podman pull $img1
-    run_podman pull $img2
+    _prefetch $img1
+    _prefetch $img2
 
     # We can't use run_podman because that uses the BATS 'run' function
     # which redirects stdout and stderr. Here we need to guarantee
diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats
index 08dfa03243..7fef945299 100644
--- a/test/system/255-auto-update.bats
+++ b/test/system/255-auto-update.bats
@@ -289,6 +289,8 @@ function _confirm_update() {
         skip "this test only works with crun, not $runtime"
     fi
 
+    _prefetch $SYSTEMD_IMAGE
+
     dockerfile1=$PODMAN_TMPDIR/Dockerfile.1
     cat >$dockerfile1 <<EOF
 FROM $SYSTEMD_IMAGE
@@ -468,6 +470,7 @@ EOF
         skip "this test only works with crun, not $runtime"
     fi
 
+    _prefetch $SYSTEMD_IMAGE
     install_kube_template
 
     dockerfile1=$PODMAN_TMPDIR/Dockerfile.1
diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats
index 1452e68898..71894ed1cb 100644
--- a/test/system/260-sdnotify.bats
+++ b/test/system/260-sdnotify.bats
@@ -142,10 +142,7 @@ READY=1" "sdnotify sent MAINPID and READY"
 # These tests can fail in dev. environment because of SELinux.
 # quick fix: chcon -t container_runtime_exec_t ./bin/podman
 @test "sdnotify : container" {
-    # Pull our systemd image. Retry in case of flakes.
-    run_podman pull $SYSTEMD_IMAGE || \
-        run_podman pull $SYSTEMD_IMAGE || \
-        run_podman pull $SYSTEMD_IMAGE
+    _prefetch $SYSTEMD_IMAGE
 
     export NOTIFY_SOCKET=$PODMAN_TMPDIR/container.sock
     _start_socat
@@ -261,10 +258,7 @@ READY=1" "sdnotify sent MAINPID and READY"
 @test "sdnotify : play kube - with policies" {
     skip_if_journald_unavailable
 
-    # Pull that image. Retry in case of flakes.
-    run_podman pull $SYSTEMD_IMAGE || \
-        run_podman pull $SYSTEMD_IMAGE || \
-        run_podman pull $SYSTEMD_IMAGE
+    _prefetch $SYSTEMD_IMAGE
 
     # Create the YAMl file
     yaml_source="$PODMAN_TMPDIR/test.yaml"
diff --git a/test/system/710-kube.bats b/test/system/710-kube.bats
index 2c3082f100..6e367432f5 100644
--- a/test/system/710-kube.bats
+++ b/test/system/710-kube.bats
@@ -8,12 +8,9 @@ load helpers
 # capability drop list
 capabilities='{"drop":["CAP_FOWNER","CAP_SETFCAP"]}'
 
-# Warning that is emitted once on containers, multiple times on pods
-kubernetes_63='Truncation Annotation: .* Kubernetes only allows 63 characters'
-
 # filter: convert yaml to json, because bash+yaml=madness
 function yaml2json() {
-    grep -E -v "$kubernetes_63" | python3 -c 'import yaml
+    python3 -c 'import yaml
 import json
 import sys
 json.dump(yaml.safe_load(sys.stdin), sys.stdout)'
@@ -33,6 +30,8 @@ json.dump(yaml.safe_load(sys.stdin), sys.stdout)'
     cname=c$(random_string 15)
     run_podman container create --cap-drop fowner --cap-drop setfcap --name $cname $IMAGE top
     run_podman kube generate $cname
+
+    # As of #18542, we must never see this message again.
     assert "$output" !~ "Kubernetes only allows 63 characters"
     # Convert yaml to json, and dump to stdout (to help in case of errors)
     json=$(yaml2json <<<"$output")
diff --git a/test/system/helpers.bash b/test/system/helpers.bash
index 64176c1e18..8cbefa2f3a 100644
--- a/test/system/helpers.bash
+++ b/test/system/helpers.bash
@@ -10,7 +10,6 @@ PODMAN_TEST_IMAGE_USER=${PODMAN_TEST_IMAGE_USER:-"libpod"}
 PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"testimage"}
 PODMAN_TEST_IMAGE_TAG=${PODMAN_TEST_IMAGE_TAG:-"20221018"}
 PODMAN_TEST_IMAGE_FQN="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/$PODMAN_TEST_IMAGE_NAME:$PODMAN_TEST_IMAGE_TAG"
-PODMAN_TEST_IMAGE_ID=
 
 # Larger image containing systemd tools.
 PODMAN_SYSTEMD_IMAGE_NAME=${PODMAN_SYSTEMD_IMAGE_NAME:-"systemd-image"}
@@ -36,6 +35,55 @@ if [ $(id -u) -eq 0 ]; then
     _LOG_PROMPT='#'
 fi
 
+###############################################################################
+# BEGIN tools for fetching & caching test images
+#
+# Registries are flaky: any time we have to pull an image, that's a risk.
+#
+
+# Store in a semipermanent location. Not important for CI, but nice for
+# developers so test restarts don't hang fetching images.
+export PODMAN_IMAGECACHE=${BATS_TMPDIR:-/tmp}/podman-systest-imagecache-$(id -u)
+mkdir -p ${PODMAN_IMAGECACHE}
+
+function _prefetch() {
+     local want=$1
+
+     # Do we already have it in image store?
+     run_podman '?' image exists "$want"
+     if [[ $status -eq 0 ]]; then
+         return
+     fi
+
+    # No image. Do we have it already cached? (Replace / and : with --)
+    local cachename=$(sed -e 's;[/:];--;g' <<<"$want")
+    local cachepath="${PODMAN_IMAGECACHE}/${cachename}.tar"
+    if [[ ! -e "$cachepath" ]]; then
+        # Not cached. Fetch it and cache it. Retry twice, because of flakes.
+        cmd="skopeo copy --preserve-digests docker://$want oci-archive:$cachepath"
+        echo "$_LOG_PROMPT $cmd"
+        run $cmd
+        echo "$output"
+        if [[ $status -ne 0 ]]; then
+            echo "# 'pull $want' failed, will retry..." >&3
+            sleep 5
+
+            run $cmd
+            echo "$output"
+            if [[ $status -ne 0 ]]; then
+                echo "# 'pull $want' failed again, will retry one last time..." >&3
+                sleep 30
+                $cmd
+            fi
+        fi
+    fi
+
+    # Cached image is now guaranteed to exist. Be sure to load it
+    # with skopeo, not podman, in order to preserve metadata
+    skopeo copy --all oci-archive:$cachepath containers-storage:$want
+}
+
+# END   tools for fetching & caching test images
 ###############################################################################
 # BEGIN setup/teardown tools
 
@@ -72,7 +120,11 @@ function basic_setup() {
         fi
     done
 
-    # Clean up all images except those desired
+    # Clean up all images except those desired.
+    # 2023-06-26 REMINDER: it is tempting to think that this is clunky,
+    # wouldn't it be safer/cleaner to just 'rmi -a' then '_prefetch $IMAGE'?
+    # Yes, but it's also tremendously slower: 29m for a CI run, to 39m.
+    # Image loads are slow.
     found_needed_image=
     run_podman '?' images --all --format '{{.Repository}}:{{.Tag}} {{.ID}}'
     # FIXME FIXME FIXME: temporary hack for #17216. If we see the unlinkat-busy
@@ -116,16 +168,15 @@ function basic_setup() {
         fi
     done
 
-    # Make sure desired images are present
-    if [ -z "$found_needed_image" ]; then
-        run_podman pull "$PODMAN_TEST_IMAGE_FQN"
+    # Make sure desired image is present
+    if [[ -z "$found_needed_image" ]]; then
+        _prefetch $PODMAN_TEST_IMAGE_FQN
     fi
 
-    # Argh. Although BATS provides $BATS_TMPDIR, it's just /tmp!
-    # That's bloody worthless. Let's make our own, in which subtests
-    # can write whatever they like and trust that it'll be deleted
-    # on cleanup.
-    # TODO: do this outside of setup, so it carries across tests?
+    # Temporary subdirectory, in which tests can write whatever they like
+    # and trust that it'll be deleted on cleanup.
+    # (BATS v1.3 and above provide $BATS_TEST_TMPDIR, but we still use
+    # ancient BATS (v1.1) in RHEL gating tests.)
     PODMAN_TMPDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-/tmp} podman_bats.XXXXXX)
 
     # In the unlikely event that a test runs is() before a run_podman()