Merge pull request #9887 from edsantiago/test_buildah_bud_with_remote

buildah bud tests under podman-remote

7 files changed, 147 insertions, 34 deletions

diff --git a/.cirrus.yml b/.cirrus.yml
index 8a79ade78..7218e3e9a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -601,7 +601,11 @@ buildah_bud_test_task:
         CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
         # ID for re-use of build output
         _BUILD_CACHE_HANDLE: ${FEDORA_NAME}-build-${CIRRUS_BUILD_ID}
-        PODBIN_NAME: podman
+    matrix:
+        - env:
+            PODBIN_NAME: podman
+        - env:
+            PODBIN_NAME: remote
     gce_instance: *standardvm
     timeout_in: 45m
     clone_script: *noop
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index f44b0ea42..91c0fef87 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -4,8 +4,15 @@ repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks.git
     rev: v3.4.0
     hooks:
+      # buildah-tests.diff is generated by 'git format-patch' and includes
+      # trailing whitespace as part of its format. We can work around that,
+      # but unfortunately the buildah repo has some files with tabs, which
+      # git-diff formats as '[+/-]<space><tab>', which these hooks choke on.
+      # Just disable checks on this diff file as a special case.
       - id: end-of-file-fixer
+        exclude: test/buildah-bud/buildah-tests.diff
       - id: trailing-whitespace
+        exclude: test/buildah-bud/buildah-tests.diff
       - id: mixed-line-ending
       - id: check-byte-order-marker
       - id: check-executables-have-shebangs
diff --git a/Makefile b/Makefile
index 832671337..32d2f44b7 100644
--- a/Makefile
+++ b/Makefile
@@ -214,7 +214,7 @@ endif
 .PHONY: .gitvalidation
 .gitvalidation: .gopathok
 	@echo "Validating vs commit '$(call err_if_empty,EPOCH_TEST_COMMIT)'"
-	GIT_CHECK_EXCLUDE="./vendor:docs/make.bat" $(GOBIN)/git-validation -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD)
+	GIT_CHECK_EXCLUDE="./vendor:docs/make.bat:test/buildah-bud/buildah-tests.diff" $(GOBIN)/git-validation -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD)
 
 .PHONY: lint
 lint: golangci-lint
diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas
index 41c84257f..18b3d56f9 100755
--- a/test/buildah-bud/apply-podman-deltas
+++ b/test/buildah-bud/apply-podman-deltas
@@ -56,22 +56,31 @@ function errmsg() {
     done
 }
 
-# skip: used to add a 'skip' to one specific test
-function skip() {
+# _skip: used to add a 'skip' or 'skip_if_remote' to one specific test
+function _skip() {
+    local skip=$1;   shift
     local reason=$1; shift
 
     # All further arguments are test names
     for t in "$@"; do
         if fgrep -qx "@test \"$t\" {" $BUD; then
-            $ECHO "@test \"$t\" : skip \"$reason\""
+            $ECHO "@test \"$t\" : $skip \"$reason\""
             t=${t//\//\\/}
-            sed -i -e "/^\@test \"$t\" {/ a \ \ skip \"$reason\"" $BUD
+            sed -i -e "/^\@test \"$t\" {/ a \ \ $skip \"$reason\"" $BUD
         else
-            warn "[skip] Did not find test \"$t\" in $BUD"
+            warn "[$skip] Did not find test \"$t\" in $BUD"
         fi
     done
 }
 
+function skip() {
+    _skip "skip" "$@"
+}
+
+function skip_if_remote() {
+    _skip "skip_if_remote" "$@"
+}
+
 # END   handlers
 ###############################################################################
 # BEGIN user-customizable section
@@ -79,14 +88,14 @@ function skip() {
 # These are the hand-maintained exceptions. This is what you want to edit
 # or update as needed.
 #
-# There are two directives you can use below:
+# There are three directives you can use below:
 #
 #    errmsg "old-message" "new-message" "test name" ["test name"...]
 #
 # This replaced "old-message" with "new-message" in @test "test name".
 # It is used when a podman error message differs from buildah's.
 #
-#    skip "reason" "test name" ["test name"...]
+#    [skip | skip_if_remote] "reason" "test name" ["test name"...]
 #
 # This adds a 'skip' statement as the first line of @test "test name".
 # It is used when a test does not work in podman, either for permanent
@@ -126,6 +135,7 @@ errmsg "no such file or directory" \
 
 ###############################################################################
 # BEGIN tests that don't make sense under podman due to fundamental differences
+
 skip "N/A under podman" \
      "bud-flags-order-verification"
 
@@ -147,13 +157,13 @@ skip "Too much effort to spin up a local registry" \
 skip "FIXME FIXME FIXME: argument-order incompatible with podman" \
      "bud-squash-hardlinks"
 
-skip "FIXME FIXME FIXME we'll figure these out later" \
-     "bud-multi-stage-nocache-nocommit" \
-     "bud with --cgroup-parent"
+skip "FIXME FIXME FIXME: this passes on Ed's laptop, fails in CI??" \
+     "bud-multi-stage-nocache-nocommit"
 
-# see https://github.com/containers/podman/pull/10147#issuecomment-832503633
-skip "FIXME FIXME FIXME podman save/load has been fixed (but not yet used in Buildah CI)" \
-     "bud with --layers and --no-cache flags"
+# This will probably never work: buildah and podman have incompatible defaults
+# Documented in https://github.com/containers/podman/issues/10412
+skip "buildah runs with --cgroup-manager=cgroupfs, podman with systemd" \
+     "bud with --cgroup-parent"
 
 # see https://github.com/containers/podman/pull/10829
 skip "FIXME FIXME FIXME - requires updated CI images (#10829)" \
@@ -164,6 +174,31 @@ skip "FIXME FIXME FIXME - requires updated CI images (#10829)" \
 
 
 ###############################################################################
+# BEGIN tests which are skipped because they make no sense under podman-remote
+
+skip_if_remote "--target does not work with podman-remote" \
+               "bud-target"
+
+skip_if_remote "--runtime not meaningful under podman-remote" \
+               "bud with --runtime and --runtime-flag"
+
+skip_if_remote "secret files not implemented under podman-remote" \
+               "bud with containerfile secret" \
+               "bud with containerfile secret accessed on second RUN" \
+               "bud with containerfile secret options"
+
+skip_if_remote "volumes don't work with podman-remote" \
+               "buildah bud --volume" \
+               "buildah-bud-policy"
+
+# See podman #9890 for discussion
+skip_if_remote "--stdin option will not be implemented in podman-remote" \
+               "bud test no --stdin"
+
+###############################################################################
+# BEGIN tests which are skipped due to actual podman-remote bugs.
+
+###############################################################################
 # Done.
 
 exit $RC
diff --git a/test/buildah-bud/buildah-tests.diff b/test/buildah-bud/buildah-tests.diff
index 1c8592f7f..66d470648 100644
--- a/test/buildah-bud/buildah-tests.diff
+++ b/test/buildah-bud/buildah-tests.diff
@@ -1,23 +1,75 @@
-From a00508656599b24776982996fdb44d4874338fd4 Mon Sep 17 00:00:00 2001
+From d684753d6f00ee95720d8fb2e09c7ac19b37b01e Mon Sep 17 00:00:00 2001
 From: Ed Santiago <santiago@redhat.com>
 Date: Tue, 9 Feb 2021 17:28:05 -0700
 Subject: [PATCH] tweaks for running buildah tests under podman
 
 Signed-off-by: Ed Santiago <santiago@redhat.com>
 ---
- tests/helpers.bash | 28 +++++++++++++++++++++++++---
- 1 file changed, 25 insertions(+), 3 deletions(-)
+ tests/helpers.bash | 71 +++++++++++++++++++++++++++++++++++++++++++---
+ 1 file changed, 67 insertions(+), 4 deletions(-)
 
 diff --git a/tests/helpers.bash b/tests/helpers.bash
-index 11deb367..08e73954 100644
+index 11deb367..44c71dad 100644
 --- a/tests/helpers.bash
 +++ b/tests/helpers.bash
-@@ -164,15 +164,37 @@ function run_buildah() {
+@@ -34,6 +34,23 @@ function setup() {
+     ROOTDIR_OPTS="--root ${TESTDIR}/root --runroot ${TESTDIR}/runroot --storage-driver ${STORAGE_DRIVER}"
+     BUILDAH_REGISTRY_OPTS="--registries-conf ${TESTSDIR}/registries.conf --registries-conf-dir ${TESTDIR}/registries.d --short-name-alias-conf ${TESTDIR}/cache/shortnames.conf"
+     PODMAN_REGISTRY_OPTS="--registries-conf ${TESTSDIR}/registries.conf"
++
++    PODMAN_SERVER_PID=
++    PODMAN_NATIVE="${PODMAN_BINARY} ${ROOTDIR_OPTS} ${PODMAN_REGISTRY_OPTS}"
++    if [[ -n "$REMOTE" ]]; then
++        PODMAN_NATIVE="${PODMAN_BINARY%%-remote} ${ROOTDIR_OPTS} ${PODMAN_REGISTRY_OPTS}"
++        # static CONTAINERS_CONF needed for capabilities test. As of 2021-07-01
++        # no tests in bud.bats override this; if at some point any test does
++        # so, it will probably need to be skip_if_remote()d.
++        env CONTAINERS_CONF=${CONTAINERS_CONF:-$(dirname ${BASH_SOURCE})/containers.conf} $PODMAN_NATIVE system service --timeout=0  &
++        PODMAN_SERVER_PID=$!
++        local timeout=10
++        while ((timeout > 0)); do
++            test -S /run/podman/podman.sock && return
++            sleep 0.2
++        done
++        die "podman server never came up"
++    fi
+ }
+ 
+ function starthttpd() {
+@@ -57,6 +74,12 @@ function stophttpd() {
+ function teardown() {
+     stophttpd
+ 
++    if [[ -n "$PODMAN_SERVER_PID" ]]; then
++        kill $PODMAN_SERVER_PID
++        wait $PODMAN_SERVER_PID
++        rm -f /run/podman/podman.sock
++    fi
++
+     # Workaround for #1991 - buildah + overlayfs leaks mount points.
+     # Many tests leave behind /var/tmp/.../root/overlay and sub-mounts;
+     # let's find those and clean them up, otherwise 'rm -rf' fails.
+@@ -129,7 +152,13 @@ function copy() {
+ }
+ 
+ function podman() {
+-    command podman ${PODMAN_REGISTRY_OPTS} ${ROOTDIR_OPTS} "$@"
++    echo "# ... podman $*" >&3
++    ${PODMAN_BINARY} ${PODMAN_REGISTRY_OPTS} ${ROOTDIR_OPTS} "$@"
++}
++
++function podman-remote() {
++    echo "# ... podman-remote $*" >&3
++    ${PODMAN_BINARY} ${ROOTDIR_OPTS} "$@"
+ }
+ 
+ #################
+@@ -164,15 +193,40 @@ function run_buildah() {
          --retry)         retry=3;        shift;;  # retry network flakes
      esac
-
+ 
 +    local podman_or_buildah=${BUILDAH_BINARY}
-+    local registry_opts=${BUILDAH_REGISTRY_OPTS}
++    local _opts="${ROOTDIR_OPTS} ${BUILDAH_REGISTRY_OPTS}"
 +    if [[ $1 == "bud" || $1 == "build-using-dockerfile" ]]; then
 +        shift
 +        # podman defaults to --layers=true; buildah to --false.
@@ -29,7 +81,10 @@ index 11deb367..08e73954 100644
 +            set "build" "--force-rm=false" "--layers=false" "$@"
 +        fi
 +        podman_or_buildah=${PODMAN_BINARY}
-+        registry_opts=${PODMAN_REGISTRY_OPTS}
++        _opts="${ROOTDIR_OPTS} ${PODMAN_REGISTRY_OPTS}"
++        if [[ -n "$REMOTE" ]]; then
++            _opts=
++        fi
 +
 +        # podman always exits 125 where buildah exits 1 or 2
 +        case $expected_rc in
@@ -41,17 +96,31 @@ index 11deb367..08e73954 100644
      # Remember command args, for possible use in later diagnostic messages
 -    MOST_RECENT_BUILDAH_COMMAND="buildah $*"
 +    MOST_RECENT_BUILDAH_COMMAND="$cmd_basename $*"
-
+ 
      while [ $retry -gt 0 ]; do
          retry=$(( retry - 1 ))
-
+ 
          # stdout is only emitted upon error; this echo is to help a debugger
 -        echo "\$ $BUILDAH_BINARY $*"
 -        run env CONTAINERS_CONF=${CONTAINERS_CONF:-$(dirname ${BASH_SOURCE})/containers.conf} timeout --foreground --kill=10 $BUILDAH_TIMEOUT ${BUILDAH_BINARY} ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} "$@"
 +        echo "\$ $cmd_basename $*"
-+        run env CONTAINERS_CONF=${CONTAINERS_CONF:-$(dirname ${BASH_SOURCE})/containers.conf} timeout --foreground --kill=10 $BUILDAH_TIMEOUT ${podman_or_buildah} ${registry_opts} ${ROOTDIR_OPTS} "$@"
++        run env CONTAINERS_CONF=${CONTAINERS_CONF:-$(dirname ${BASH_SOURCE})/containers.conf} timeout --foreground --kill=10 $BUILDAH_TIMEOUT ${podman_or_buildah} ${_opts} "$@"
          # without "quotes", multiple lines are glommed together into one
          if [ -n "$output" ]; then
              echo "$output"
---
+@@ -396,3 +450,12 @@ function skip_if_no_docker() {
+     skip "this test needs actual docker, not podman-docker"
+   fi
+ }
++
++####################
++#  skip_if_remote  #  (only applicable for podman)
++####################
++function skip_if_remote() {
++    if [[ -n "$REMOTE" ]]; then
++        skip "${1:-test does not work with podman-remote}"
++    fi
++}
+-- 
 2.31.1
+
diff --git a/test/buildah-bud/make-new-buildah-diffs b/test/buildah-bud/make-new-buildah-diffs
index 11987e376..3d0a77008 100644
--- a/test/buildah-bud/make-new-buildah-diffs
+++ b/test/buildah-bud/make-new-buildah-diffs
@@ -56,8 +56,6 @@ if [[ -n "$patch2" ]]; then
     die "Internal error: I thought I checked for squashed commits, but still see $patch2"
 fi
 
-# All looks good. Now write that patch into its proper place in the
-# podman repo. The sed and tac mess strips trailing whitespace and
-# empty lines; we need to do this to pass github CI checks.
-sed -e 's/ \+$//' <0001-*.patch |\
-    tac | sed -e '/./,$!d' | tac >| ../test/buildah-bud/buildah-tests.diff
+# All looks good. We can now copy that patch into its proper place in the
+# podman repo.
+cp 0001-*.patch ../test/buildah-bud/buildah-tests.diff
diff --git a/test/buildah-bud/run-buildah-bud-tests b/test/buildah-bud/run-buildah-bud-tests
index a37e90dc4..eb8de5618 100755
--- a/test/buildah-bud/run-buildah-bud-tests
+++ b/test/buildah-bud/run-buildah-bud-tests
@@ -69,8 +69,7 @@ REMOTE=
 # If remote, start server & change path
 if [[ "${PODBIN_NAME:-}" = "remote" ]]; then
     REMOTE=1
-    echo "$ME: remote tests are not working yet" >&2
-    exit 1
+    PODMAN_BINARY+="-remote"
 fi
 
 function die() {
@@ -214,6 +213,7 @@ review the test failure and double-check your changes.
 
     (set -x;sudo env TMPDIR=/var/tmp \
                  PODMAN_BINARY=$PODMAN_BINARY \
+                 REMOTE=$REMOTE \
                  BUILDAH_BINARY=$(pwd)/bin/buildah \
                  COPY_BINARY=$(pwd)/bin/copy \
                  bats "${bats_filter[@]}" tests/bud.bats)