Merge pull request #1864 from mtrmac/storage-big-hammer

Fix storage.conf overrides in test-system in CI, update c/storage

Makefile

@@ -197,7 +197,11 @@ shell:
 check: validate test-unit test-integration test-system
 
 test-integration:
-	$(CONTAINER_RUN) $(MAKE) test-integration-local
+# This is intended to be equal to $(CONTAINER_RUN), but with --cap-add=cap_mknod.
+# --cap-add=cap_mknod is important to allow skopeo to use containers-storage: directly as it exists in the callers’ environment, without
+# creating a nested user namespace (which requires /etc/subuid and /etc/subgid to be set up)
+	$(CONTAINER_CMD) --security-opt label=disable --cap-add=cap_mknod -v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) $(SKOPEO_CIDEV_CONTAINER_FQIN) \
+		$(MAKE) test-integration-local
 
 
 # Intended for CI, assumed to be running in quay.io/libpod/skopeo_cidev container.

contrib/cirrus/runner.sh

@@ -115,15 +115,6 @@ _run_unit() {
 _podman_reset() {
     # Ensure we start with a clean-slate
     showrun podman system reset --force
-    # WARNING WARNING WARNING WARNING
-    # Without running a container, the system tests will inexplicably
-    # fail with obscure errors/warning messages.  I have no idea why
-    # running a container after a `system reset` fixes/prevents the
-    # problem.  The failures do not reproduce when tests are run manually.
-    # So unless or until /until somebody develops a better understanding,
-    # this fix is JFM - just fakking magic.
-    # WARNING WARNING WARNING WARNING
-    showrun podman run -it --rm --entrypoint /bin/true quay.io/libpod/alpine:latest
 }
 
 _run_integration() {

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/containers/common v0.50.1
 	github.com/containers/image/v5 v5.23.1-0.20230113185223-cf9ccfb4d9b1
 	github.com/containers/ocicrypt v1.1.7
-	github.com/containers/storage v1.45.1
+	github.com/containers/storage v1.45.3
 	github.com/docker/distribution v2.8.1+incompatible
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.1.0-rc2

go.sum

@@ -935,8 +935,8 @@ github.com/containers/ocicrypt v1.1.7 h1:thhNr4fu2ltyGz8aMx8u48Ae0Pnbip3ePP9/mzk
 github.com/containers/ocicrypt v1.1.7/go.mod h1:7CAhjcj2H8AYp5YvEie7oVSK2AhBY8NscCYRawuDNtw=
 github.com/containers/storage v1.43.0/go.mod h1:uZ147thiIFGdVTjMmIw19knttQnUCl3y9zjreHrg11s=
 github.com/containers/storage v1.45.0/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g=
-github.com/containers/storage v1.45.1 h1:hsItObigGLm77Dn4ebUxQ68EfE6nMrwGcIdMRqzgclI=
-github.com/containers/storage v1.45.1/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g=
+github.com/containers/storage v1.45.3 h1:GbtTvTtp3GW2/tcFg5VhgHXcYMwVn2KfZKiHjf9FAOM=
+github.com/containers/storage v1.45.3/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=

hack/make/test-system

@@ -5,16 +5,36 @@ set -e
 # not all storage drivers are supported in a container
 # environment.  Detect this and setup storage when
 # running in a container.
-if ((SKOPEO_CONTAINER_TESTS)) && [[ -r /etc/containers/storage.conf ]]; then
-    sed -i \
-        -e 's/^driver\s*=.*/driver = "vfs"/' \
-        -e 's/^mountopt/#mountopt/' \
-        /etc/containers/storage.conf
-elif ((SKOPEO_CONTAINER_TESTS)); then
-    cat >> /etc/containers/storage.conf << EOF
+#
+# Paradoxically (FIXME: clean this up), SKOPEO_CONTAINER_TESTS is set
+# both inside a container and without a container (in a CI VM); it actually means
+# "it is safe to desctructively modify the system for tests".
+#
+# On a CI VM, we can just use Podman as it is already configured; the changes below,
+# to use VFS, are necessary only inside a container, because overlay-inside-overlay
+# does not work. So, make these changes conditional on both
+# SKOPEO_CONTAINER_TESTS (for acceptability to do destructive modification) and !CI
+# (for necessity to adjust for in-container operation)
+if ((SKOPEO_CONTAINER_TESTS)) && [[ "$CI" != true ]]; then
+    if [[ -r /etc/containers/storage.conf ]]; then
+        echo "MODIFYING existing storage.conf"
+        sed -i \
+            -e 's/^driver\s*=.*/driver = "vfs"/' \
+            -e 's/^mountopt/#mountopt/' \
+            /etc/containers/storage.conf
+    else
+        echo "CREATING NEW storage.conf"
+        cat >> /etc/containers/storage.conf << EOF
 [storage]
 driver = "vfs"
+runroot = "/run/containers/storage"
+graphroot = "/var/lib/containers/storage"
 EOF
+    fi
+    # The logic of finding the relevant storage.conf file is convoluted
+    # and in effect differs between Skopeo and Podman, at least in some versions;
+    # explicitly point at the file we want to use to hopefully avoid that.
+    export CONTAINERS_STORAGE_CONF=/etc/containers/storage.conf
 fi
 
 # Build skopeo, install into /usr/bin

vendor/github.com/containers/storage/VERSION

@@ -1 +1 @@
-1.45.1
+1.45.3

vendor/github.com/containers/storage/types/utils.go

@@ -173,6 +173,9 @@ func DefaultConfigFile(rootless bool) (string, error) {
 		return path, nil
 	}
 	if !rootless {
+		if _, err := os.Stat(defaultOverrideConfigFile); err == nil {
+			return defaultOverrideConfigFile, nil
+		}
 		return defaultConfigFile, nil
 	}
 

vendor/github.com/containers/storage/userns.go

@@ -78,6 +78,10 @@ func (s *store) getAvailableIDs() (*idSet, *idSet, error) {
 	return u, g, nil
 }
 
+// nobodyUser returns the UID and GID of the "nobody" user.  Hardcode its value
+// for simplicity.
+const nobodyUser = 65534
+
 // parseMountedFiles returns the maximum UID and GID found in the /etc/passwd and
 // /etc/group files.
 func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
@@ -98,10 +102,10 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
 			if u.Name == "nobody" {
 				continue
 			}
-			if u.Uid > size {
+			if u.Uid > size && u.Uid != nobodyUser {
 				size = u.Uid
 			}
-			if u.Gid > size {
+			if u.Gid > size && u.Gid != nobodyUser {
 				size = u.Gid
 			}
 		}
@@ -113,7 +117,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
 			if g.Name == "nobody" {
 				continue
 			}
-			if g.Gid > size {
+			if g.Gid > size && g.Gid != nobodyUser {
 				size = g.Gid
 			}
 		}

vendor/modules.txt

@@ -149,7 +149,7 @@ github.com/containers/ocicrypt/keywrap/pkcs7
 github.com/containers/ocicrypt/spec
 github.com/containers/ocicrypt/utils
 github.com/containers/ocicrypt/utils/keyprovider
-# github.com/containers/storage v1.45.1
+# github.com/containers/storage v1.45.3
 ## explicit; go 1.17
 github.com/containers/storage
 github.com/containers/storage/drivers