CI: test overlay and vfs

We're only testing vfs in CI. That's bad. containers#18822 tried to
remedy that but that only worked on system tests, not e2e.

Here we introduce CI_DESIRED_STORAGE, to be set in .cirrus.yml
in the same vein as all the other CI_DESIRED_X. Since it's 2023
we default to overlay, testing vfs only in priorfedora.

Fixes required:
 - in cleanup, umount ROOT/overlay to avoid leaking mounts

 - e2e:
   - Skip the "split imagestore" test: it doesn't work,
     and nobody knows why (containers#19748)

 - system tests:
   - fix a few badly-written tests that assumed/hardcoded overlay
   - buildx test: add weird exception to device-number test
   - mount tests: add special case code for vfs
   - unprivileged test: disable one section that is N/A on vfs

Signed-off-by: Ed Santiago <santiago@redhat.com>

.cirrus.yml

@@ -59,6 +59,7 @@ env:
     VM_IMAGE_NAME:           # One of the "Google-cloud VM Images" (above)
     CTR_FQIN:                # One of the "Container FQIN's" (above)
     CI_DESIRED_DATABASE: sqlite # One of "", "sqlite", "boltdb"
+    CI_DESIRED_STORAGE: overlay # overlay or vfs
 
     # Curl-command prefix for downloading task artifacts, simply add the
     # the url-encoded task name, artifact name, and path as a suffix.
@@ -120,6 +121,7 @@ build_task:
               CTR_FQIN: ${PRIOR_FEDORA_CONTAINER_FQIN}
               CI_DESIRED_RUNTIME: crun
               CI_DESIRED_NETWORK: cni
+              CI_DESIRED_STORAGE: vfs
               # Catch invalid "TMPDIR == /tmp" assumptions; PR #19281
               TMPDIR: /var/tmp
         - env:

contrib/cirrus/lib.sh

@@ -95,7 +95,7 @@ EPOCH_TEST_COMMIT="$CIRRUS_BASE_SHA"
 # contexts, such as host->container or root->rootless user
 #
 # List of envariables which must be EXACT matches
-PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS|EC2_INST_TYPE|PODMAN_DB'
+PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS|EC2_INST_TYPE|PODMAN_DB|STORAGE_FS'
 
 # List of envariable patterns which must match AT THE BEGINNING of the name.
 PASSTHROUGH_ENV_ATSTART='CI|LANG|LC_|TEST'

contrib/cirrus/setup_environment.sh

@@ -103,21 +103,19 @@ esac
 # shellcheck disable=SC2154
 printf "[engine]\ndatabase_backend=\"$CI_DESIRED_DATABASE\"\n" > /etc/containers/containers.conf.d/92-db.conf
 
-# For debian envs pre-configure storage driver as overlay.
-# See: Discussion here https://github.com/containers/podman/pull/18510#discussion_r1189812306
-# for more details.
-# TODO: remove this once all CI VM have newer buildah version. (i.e where buildah
-# does not defaults to using `vfs` as storage driver)
+# Force the requested storage driver; this applies only to system tests.
 # shellcheck disable=SC2154
-if [[ "$OS_RELEASE_ID" == "debian" ]]; then
-    showrun echo "conditional setup for debian"
-    conf=/etc/containers/storage.conf
-    if [[ -e $conf ]]; then
-        die "FATAL! INTERNAL ERROR! Cannot override $conf"
-    fi
-    msg "Overriding $conf, setting overlay (was: $buildah_storage)"
-    printf '[storage]\ndriver = "overlay"\nrunroot = "/run/containers/storage"\ngraphroot = "/var/lib/containers/storage"\n' >$conf
+showrun echo "Setting up containers.conf for CI_DESIRED_STORAGE=$CI_DESIRED_STORAGE"
+conf=/etc/containers/storage.conf
+if [[ -e $conf ]]; then
+    die "FATAL! INTERNAL ERROR! Cannot override $conf"
 fi
+cat <<EOF >$conf
+[storage]
+driver = "$CI_DESIRED_STORAGE"
+runroot = "/run/containers/storage"
+graphroot = "/var/lib/containers/storage"
+EOF
 
 if ((CONTAINER==0)); then  # Not yet running inside a container
     showrun echo "conditional setup for CONTAINER == 0"
@@ -199,6 +197,10 @@ case "$CI_DESIRED_DATABASE" in
         ;;
 esac
 
+# shellcheck disable=SC2154
+showrun echo "about to set up for CI_DESIRED_STORAGE [=$CI_DESIRED_STORAGE]"
+echo "STORAGE_FS=$CI_DESIRED_STORAGE" >>/etc/ci_environment
+
 # Required to be defined by caller: The environment where primary testing happens
 # shellcheck disable=SC2154
 showrun echo "about to set up for TEST_ENVIRON [=$TEST_ENVIRON]"

test/e2e/common_test.go

@@ -983,13 +983,25 @@ func populateCache(podman *PodmanTestIntegration) {
 // rmAll removes the directory and its content, when running rootless we use
 // podman unshare to prevent any subuid/gid problems
 func rmAll(podmanBin string, path string) {
+	// When using overlay, podman leaves a stray mount behind. This
+	// leak causes remote tests to take a loooooong time, and time out.
+	overlayPath := path + "/root/overlay"
+	if _, err := os.Stat(overlayPath); err == nil {
+		umount := exec.Command("umount", overlayPath)
+		umount.Stdout = GinkgoWriter
+		umount.Stderr = GinkgoWriter
+		if err = umount.Run(); err != nil {
+			GinkgoWriter.Printf("Error umounting %s: %v\n", overlayPath, err)
+		}
+	}
+
 	// Remove cache dirs
 	if isRootless() {
 		// If rootless, os.RemoveAll() is failed due to permission denied
 		cmd := exec.Command(podmanBin, "unshare", "rm", "-rf", path)
 		cmd.Stdout = GinkgoWriter
 		cmd.Stderr = GinkgoWriter
-		if err := cmd.Run(); err != nil {
+		if err = cmd.Run(); err != nil {
 			GinkgoWriter.Printf("%v\n", err)
 		}
 	} else {

test/e2e/config_amd64.go

@@ -1,10 +1,10 @@
 package integration
 
 var (
-	STORAGE_FS               = "vfs"                                                                                                                                                     //nolint:revive,stylecheck
-	STORAGE_OPTIONS          = "--storage-driver vfs"                                                                                                                                    //nolint:revive,stylecheck
-	ROOTLESS_STORAGE_FS      = "vfs"                                                                                                                                                     //nolint:revive,stylecheck
-	ROOTLESS_STORAGE_OPTIONS = "--storage-driver vfs"                                                                                                                                    //nolint:revive,stylecheck
+	STORAGE_FS               = "overlay"                                                                                                                                                 //nolint:revive,stylecheck
+	STORAGE_OPTIONS          = "--storage-driver overlay"                                                                                                                                //nolint:revive,stylecheck
+	ROOTLESS_STORAGE_FS      = "overlay"                                                                                                                                                 //nolint:revive,stylecheck
+	ROOTLESS_STORAGE_OPTIONS = "--storage-driver overlay"                                                                                                                                //nolint:revive,stylecheck
 	CACHE_IMAGES             = []string{ALPINE, BB, fedoraMinimal, NGINX_IMAGE, REDIS_IMAGE, REGISTRY_IMAGE, INFRA_IMAGE, CITEST_IMAGE, HEALTHCHECK_IMAGE, SYSTEMD_IMAGE, fedoraToolbox} //nolint:revive,stylecheck
 	NGINX_IMAGE              = "quay.io/libpod/alpine_nginx:latest"                                                                                                                      //nolint:revive,stylecheck
 	BB_GLIBC                 = "docker.io/library/busybox:glibc"                                                                                                                         //nolint:revive,stylecheck

test/e2e/config_arm64.go

@@ -1,10 +1,10 @@
 package integration
 
 var (
-	STORAGE_FS               = "vfs"                                                                                                                                                     //nolint:revive,stylecheck
-	STORAGE_OPTIONS          = "--storage-driver vfs"                                                                                                                                    //nolint:revive,stylecheck
-	ROOTLESS_STORAGE_FS      = "vfs"                                                                                                                                                     //nolint:revive,stylecheck
-	ROOTLESS_STORAGE_OPTIONS = "--storage-driver vfs"                                                                                                                                    //nolint:revive,stylecheck
+	STORAGE_FS               = "overlay"                                                                                                                                                 //nolint:revive,stylecheck
+	STORAGE_OPTIONS          = "--storage-driver overlay"                                                                                                                                //nolint:revive,stylecheck
+	ROOTLESS_STORAGE_FS      = "overlay"                                                                                                                                                 //nolint:revive,stylecheck
+	ROOTLESS_STORAGE_OPTIONS = "--storage-driver overlay"                                                                                                                                //nolint:revive,stylecheck
 	CACHE_IMAGES             = []string{ALPINE, BB, fedoraMinimal, NGINX_IMAGE, REDIS_IMAGE, REGISTRY_IMAGE, INFRA_IMAGE, CITEST_IMAGE, HEALTHCHECK_IMAGE, SYSTEMD_IMAGE, fedoraToolbox} //nolint:revive,stylecheck
 	NGINX_IMAGE              = "quay.io/lsm5/alpine_nginx-aarch64:latest"                                                                                                                //nolint:revive,stylecheck
 	BB_GLIBC                 = "docker.io/library/busybox:glibc"                                                                                                                         //nolint:revive,stylecheck

test/e2e/config_ppc64le.go

@@ -3,8 +3,8 @@ package integration
 var (
 	STORAGE_FS               = "overlay"
 	STORAGE_OPTIONS          = "--storage-driver overlay"
-	ROOTLESS_STORAGE_FS      = "vfs"
-	ROOTLESS_STORAGE_OPTIONS = "--storage-driver vfs"
+	ROOTLESS_STORAGE_FS      = "overlay"
+	ROOTLESS_STORAGE_OPTIONS = "--storage-driver overlay"
 	CACHE_IMAGES             = []string{ALPINE, BB, fedoraMinimal, NGINX_IMAGE, REDIS_IMAGE, INFRA_IMAGE, CITEST_IMAGE}
 	NGINX_IMAGE              = "quay.io/libpod/alpine_nginx-ppc64le:latest"
 	BB_GLIBC                 = "docker.io/ppc64le/busybox:glibc"

test/e2e/info_test.go

@@ -216,6 +216,21 @@ var _ = Describe("Podman Info", func() {
 		Expect(session.ErrorToString()).To(Equal("Error: unsupported database backend: \"bogus\""))
 	})
 
+	It("Podman info: check desired storage driver", func() {
+		// defined in .cirrus.yml
+		want := os.Getenv("CI_DESIRED_STORAGE")
+		if want == "" {
+			if os.Getenv("CIRRUS_CI") == "" {
+				Skip("CI_DESIRED_STORAGE is not set--this is OK because we're not running under Cirrus")
+			}
+			Fail("CIRRUS_CI is set, but CI_DESIRED_STORAGE is not! See #20161")
+		}
+		session := podmanTest.Podman([]string{"info", "--format", "{{.Store.GraphDriverName}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).To(ExitCleanly())
+		Expect(session.OutputToString()).To(Equal(want), ".Store.GraphDriverName from podman info")
+	})
+
 	It("Podman info: check lock count", Serial, func() {
 		// This should not run on architectures and OSes that use the file locks backend.
 		// Which, for now, is Linux + RISCV and FreeBSD, neither of which are in CI - so

test/system/001-basic.bats

@@ -53,6 +53,7 @@ function setup() {
         'Cgroups:{{.Host.CgroupsVersion}}+{{.Host.CgroupManager}}'
         'Net:{{.Host.NetworkBackend}}'
         'DB:{{.Host.DatabaseBackend}}'
+        'Store:{{.Store.GraphDriverName}}'
     )
     run_podman info --format "$(IFS='/' echo ${want[@]})"
     echo "# $output" >&3

test/system/005-info.bats

@@ -113,6 +113,22 @@ host.slirp4netns.executable | $expr_path
     is "$output" "$CI_DESIRED_DATABASE" "CI_DESIRED_DATABASE (from .cirrus.yml)"
 }
 
+@test "podman info - confirm desired storage driver" {
+    if [[ -z "$CI_DESIRED_STORAGE" ]]; then
+        # When running in Cirrus, CI_DESIRED_STORAGE *must* be defined
+        # in .cirrus.yml so we can double-check that all CI VMs are
+        # using overlay or vfs as desired.
+        if [[ -n "$CIRRUS_CI" ]]; then
+            die "CIRRUS_CI is set, but CI_DESIRED_STORAGE is not! See #20161"
+        fi
+
+        # Not running under Cirrus (e.g., gating tests, or dev laptop).
+        # Totally OK to skip this test.
+        skip "CI_DESIRED_STORAGE is unset--OK, because we're not in Cirrus"
+    fi
+
+    is "$(podman_storage_driver)" "$CI_DESIRED_STORAGE" "podman storage driver is not CI_DESIRED_STORAGE (from .cirrus.yml)"
+}
 
 # 2021-04-06 discussed in watercooler: RHEL must never use crun, even if
 # using cgroups v2.
@@ -159,7 +175,7 @@ host.slirp4netns.executable | $expr_path
 @test "podman --root PATH info - basic output" {
     if ! is_remote; then
         run_podman --storage-driver=vfs --root ${PODMAN_TMPDIR}/nothing-here-move-along info --format '{{ .Store.GraphOptions }}'
-        is "$output" "map\[\]" "'podman --root should reset Graphoptions to []"
+        is "$output" "map\[\]" "'podman --root should reset GraphOptions to []"
     fi
 }
 

test/system/010-images.bats

@@ -337,28 +337,34 @@ Deleted: $pauseID"
 @test "podman pull image with additional store" {
     skip_if_remote "only works on local"
 
+    # overlay or vfs
+    local storagedriver="$(podman_storage_driver)"
+
     local imstore=$PODMAN_TMPDIR/imagestore
     local sconf=$PODMAN_TMPDIR/storage.conf
     cat >$sconf <<EOF
 [storage]
-driver="overlay"
+driver="$storagedriver"
 
 [storage.options]
 additionalimagestores = [ "$imstore/root" ]
 EOF
 
     skopeo copy containers-storage:$IMAGE \
-           containers-storage:\[overlay@$imstore/root+$imstore/runroot\]$IMAGE
+           containers-storage:\[${storagedriver}@${imstore}/root+${imstore}/runroot\]$IMAGE
 
+    # IMPORTANT! Use -2/-1 indices, not 0/1, because $SYSTEMD_IMAGE may be
+    # present in store; if it is, it will precede $IMAGE.
     CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Repository}}:{{.Tag}} {{.ReadOnly}}"
-    is "${lines[0]}" "$IMAGE false" "image from readonly store"
-    is "${lines[1]}" "$IMAGE true" "image from readwrite store"
+    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"
 
     CONTAINERS_STORAGE_CONF=$sconf run_podman images -a -n --format "{{.Id}}"
-    id=${lines[0]}
+    id=${lines[-1]}
 
     CONTAINERS_STORAGE_CONF=$sconf run_podman pull -q $IMAGE
-    is "$output" "$id" "Should only print one line"
+    is "$output" "$id" "pull -q $IMAGE, using storage.conf"
 
     run_podman --root $imstore/root rmi --all
 }

test/system/060-mount.bats

@@ -30,7 +30,10 @@ load helpers
     # umount, and make sure files are gone
     run_podman umount $c_name
     if [ -e "$mount_path/$f_path" ]; then
-        die "Mounted file exists even after umount: $mount_path/$f_path"
+        # Overlay must actually unmount; vfs only removes when image is removed
+        if [[ "$(podman_storage_driver)" != "vfs" ]]; then
+            die "Mounted file exists even after umount: $mount_path/$f_path"
+        fi
     fi
 }
 
@@ -242,7 +245,10 @@ EOF
     # umount, and make sure files are gone
     run_podman umount $external_cid
     if [ -d "$mount_path" ]; then
-        die "'podman umount' did not umount"
+        # Under VFS, mountpoint always exists even despite umount
+        if [[ "$(podman_storage_driver)" != "vfs" ]]; then
+            die "'podman umount' did not umount $mount_path"
+        fi
     fi
     buildah rm $external_cid
 }

test/system/070-build.bats

@@ -77,9 +77,14 @@ EOF
     assert "${lines[0]}" = "${lines[5]}" "devnum( / ) = devnum( /[etc )"
     assert "${lines[0]}" = "${lines[7]}" "devnum( / ) = devnum( /etc )"
     assert "${lines[6]}" = "${lines[8]}" "devnum( /[etc/foo, ) = devnum( /etc/bar] )"
-    # ...then, each volume should be different
-    assert "${lines[0]}" != "${lines[3]}" "devnum( / ) != devnum( volume0 )"
-    assert "${lines[0]}" != "${lines[6]}" "devnum( / ) != devnum( volume1 )"
+    # ...then, check volumes; these differ between overlay and vfs.
+    # Under Overlay (usual case), these will be different. On VFS, they're the same.
+    local op="!="
+    if [[ "$(podman_storage_driver)" == "vfs" ]]; then
+        op="="
+    fi
+    assert "${lines[0]}" $op "${lines[3]}" "devnum( / ) $op devnum( volume0 )"
+    assert "${lines[0]}" $op "${lines[6]}" "devnum( / ) $op devnum( volume1 )"
 
     # FIXME: is this expected? I thought /a/b/c and /[etc/foo, would differ
     assert "${lines[3]}" = "${lines[6]}" "devnum( volume0 ) = devnum( volume1 )"

test/system/400-unprivileged-access.bats

@@ -50,6 +50,13 @@ if chmod +w "$path"; then
     die "Able to chmod $path"
 fi
 
+EOF
+
+    # Under VFS, if anything has been run with --uidmap, all images
+    # become world-accessible. On overlay (and future drivers?) we
+    # want to run things a little more tightly.
+    if [[ $(podman_storage_driver) != "vfs" ]]; then
+        cat >>$test_script <<EOF
 if [ -d "$path" ]; then
     if ls "$path" >/dev/null; then
         die "Able to run 'ls $path' without error"
@@ -67,8 +74,9 @@ else
     fi
 fi
 
-exit 0
 EOF
+    fi
+    echo "exit 0" >>$test_script
     chmod 755 $PODMAN_TMPDIR $test_script
 
     # get podman image and container storage directories

test/system/helpers.bash

@@ -575,6 +575,18 @@ function podman_runtime() {
     basename "${output:-[null]}"
 }
 
+# Returns the storage driver, 'overlay' or 'vfs'
+function podman_storage_driver() {
+    run_podman info --format '{{.Store.GraphDriverName}}' >/dev/null
+    # Should there ever be a new driver
+    case "$output" in
+        overlay) ;;
+        vfs)     ;;
+        *)       die "Unknown storage driver '$output'; if this is a new driver, please review uses of this function in tests." ;;
+    esac
+    echo "$output"
+}
+
 # rhbz#1895105: rootless journald is unavailable except to users in
 # certain magic groups; which our testuser account does not belong to
 # (intentional: that is the RHEL default, so that's the setup we test).