Merge pull request #9296 from edsantiago/test_buildah_bud

run buildah bud tests using podman
containers · Mar 23, 2021 · 4d3e71a · 4d3e71a
2 parents 60c90c3 + 3baa9da
commit 4d3e71a
Show file tree

Hide file tree

Showing 7 changed files with 532 additions and 0 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -579,6 +579,29 @@ remote_system_test_task:
         TEST_FLAVOR: sys
         PODBIN_NAME: remote
 
+buildah_bud_test_task:
+    name: *std_name_fmt
+    alias: buildah_bud_test
+    skip: *tags
+    only_if: *not_docs
+    depends_on:
+      - local_integration_test
+    env:
+        TEST_FLAVOR: bud
+        DISTRO_NV: ${FEDORA_NAME}
+        # Not used here, is used in other tasks
+        VM_IMAGE_NAME: ${FEDORA_CACHE_IMAGE_NAME}
+        CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
+        # ID for re-use of build output
+        _BUILD_CACHE_HANDLE: ${FEDORA_NAME}-build-${CIRRUS_BUILD_ID}
+        PODBIN_NAME: podman
+    gce_instance: *standardvm
+    timeout_in: 45m
+    clone_script: *noop
+    gopath_cache: *ro_gopath_cache
+    setup_script: *setup
+    main_script: *main
+    always: *int_logs_artifacts
 
 rootless_system_test_task:
     name: *std_name_fmt
@@ -687,6 +710,7 @@ success_task:
         - remote_system_test
         - rootless_system_test
         - upgrade_test
+        - buildah_bud_test
         - meta
     container: *smallcontainer
     env:

diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh
@@ -74,6 +74,10 @@ function _run_upgrade_test() {
     bats test/upgrade |& logformatter
 }
 
+function _run_bud() {
+    ./test/buildah-bud/run-buildah-bud-tests |& logformatter
+}
+
 function _run_bindings() {
     # shellcheck disable=SC2155
     export PATH=$PATH:$GOSRC/hack

diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh
@@ -201,6 +201,7 @@ case "$TEST_FLAVOR" in
     int) ;&
     sys) ;&
     upgrade_test) ;&
+    bud) ;&
     bindings) ;&
     endpoint)
         # Use existing host bits when testing is to happen inside a container

diff --git a/test/buildah-bud/README.md b/test/buildah-bud/README.md
@@ -0,0 +1,82 @@
+buildah-bud tests under podman
+==============================
+
+This directory contains tools for running 'buildah bud' tests
+under podman. The key concept of the workflow is:
+
+* Pull buildah @ version specified in go.mod
+* Apply a small set of patches to buildah's tests directory, such that
+  * BATS will use 'podman build' instead of 'buildah bud'; and
+  * some not-applicable-under-podman tests are skipped
+
+It's a teeny bit more complicated than that, but that's really most of
+what you need to know for most purposes. The tests run in podman CI,
+and for the most part are expected to just pass.
+
+Troubleshooting
+---------------
+
+If you're reading this, it's probably because something went wrong.
+At the time of this writing (March 2021, initial commit) it is
+impossible to foresee what typical failures will look like, but
+my prediction is that they will fit one of two categories:
+
+* Failure when vendoring new buildah (e.g., by dependabot)
+* Other failure
+
+Let's examine those in reverse order:
+
+Failure when not vendoring
+--------------------------
+
+Aside from flakes, my only guess here is that you broke 'podman build'.
+If this is the case, it is very likely that you are aware of what you
+did; and if this is the case, your change likely falls into one of
+these two categories:
+
+* "OOPS! I didn't mean to break that". Solution: fix it!
+* "Uh, yeah, this is deliberate, and we choose to be incompatible with buildah". In this case, you'll need to skip or edit the failing test(s); see below.
+
+If neither of those is the case, then I'm sorry, you're on your own.
+When you figure it out, please remember to update these instructions.
+
+
+Failure when vendoring new buildah
+----------------------------------
+
+This is what I predict will be the usual case; and I predict that
+failures will fall into one of two bins:
+
+* failure to apply the patch
+* failure because there are new buildah tests for functionality not in podman
+
+In either case, the process for solving is the same:
+
+* Start with a checked-out podman tree with the failing PR applied
+* run `./test/buildah-bud/run-buildah-bud-tests`
+
+Presumably, something will fail here. Whatever the failure, your next step is:
+
+* `cd test-buildah-v<TAB>` (this is a new directory created by the script)
+
+If the failure was in `git am`, solve it (left as exercise for the reader).
+
+If the failure was in tests run, solve it (either by adding `skip`s to
+failing tests in bud.bats, or less preferably, by making other tweaks
+to the test code).
+
+You now have modified files. THOSE SHOULD ONLY BE test/bud.bats or
+test/helpers.bash! If you changed any other file, that is a sign that
+something is very wrong!
+
+Commit your changes: `git commit --all --amend`
+
+Push those changes to the podman repo: `./make-new-buildah-diffs`
+
+cd back up to the podman repo
+
+As necessary, rerun `run-buildah-bud-tests`. You can use `--no-checkout`
+to run tests immediately, without rerunning the git checkout.
+
+If you're happy with the diffs, `git add` the modified `.diff` file
+and submit it as part of your PR.
diff --git a/test/buildah-bud/buildah-tests.diff b/test/buildah-bud/buildah-tests.diff
@@ -0,0 +1,192 @@
+From c85882a8f7fb6efbf4d59dfe8340bfbef57ccd48 Mon Sep 17 00:00:00 2001
+From: Ed Santiago <[email protected]>
+Date: Tue, 9 Feb 2021 17:28:05 -0700
+Subject: [PATCH] tweaks for running buildah tests under podman
+
+Signed-off-by: Ed Santiago <[email protected]>
+---
+ tests/bud.bats     | 26 ++++++++++++++++----------
+ tests/helpers.bash | 28 ++++++++++++++++++++++++----
+ 2 files changed, 40 insertions(+), 14 deletions(-)
+
+diff --git a/tests/bud.bats b/tests/bud.bats
+index 1efc3c58..9a39d594 100644
+--- a/tests/bud.bats
++++ b/tests/bud.bats
+@@ -4,7 +4,7 @@ load helpers
+
+ @test "bud with a path to a Dockerfile (-f) containing a non-directory entry" {
+   run_buildah 125 bud -f ${TESTSDIR}/bud/non-directory-in-path/non-directory/Dockerfile
+-  expect_output --substring "non-directory/Dockerfile: not a directory"
++  expect_output --substring "Error: context must be a directory:"
+ }
+
+ @test "bud with --dns* flags" {
+@@ -95,6 +95,7 @@ symlink(subdir)"
+ }
+
+ @test "bud-flags-order-verification" {
++  skip "N/A under podman"
+   run_buildah 125 bud /tmp/tmpdockerfile/ -t blabla
+   check_options_flag_err "-t"
+
+@@ -1324,13 +1325,13 @@ function _test_http() {
+ @test "bud with dir for file but no Dockerfile in dir" {
+   target=alpine-image
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/empty-dir ${TESTSDIR}/bud/empty-dir
+-  expect_output --substring "no such file or directory"
++  expect_output --substring "Error: context must be a directory:"
+ }
+
+ @test "bud with bad dir Dockerfile" {
+   target=alpine-image
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/baddirname ${TESTSDIR}/baddirname
+-  expect_output --substring "no such file or directory"
++  expect_output --substring "Error: context must be a directory:"
+ }
+
+ @test "bud with ARG before FROM default value" {
+@@ -1742,7 +1743,9 @@ _EOF
+   run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t test-img-2 --build-arg TEST=foo -f Dockerfile4 ${TESTSDIR}/bud/build-arg
+   run_buildah inspect -f '{{.FromImageID}}' test-img-2
+   argsid="$output"
+-  [[ "$argsid" != "$initialid" ]]
++  if [[ "$argsid" == "$initialid" ]]; then
++      die ".FromImageID of test-img-2 ($argsid) == same as test-img, it should be different"
++  fi
+
+   # Set the build-arg via an ENV in the local environment and verify that the cached layers are not used
+   export TEST=bar
+@@ -1795,6 +1798,7 @@ _EOF
+ }
+
+ @test "bud without any arguments should succeed" {
++  skip "does not work under podman"
+   cd ${TESTSDIR}/bud/from-scratch
+   run_buildah bud --signature-policy ${TESTSDIR}/policy.json
+ }
+@@ -1802,7 +1806,7 @@ _EOF
+ @test "bud without any arguments should fail when no Dockerfile exist" {
+   cd $(mktemp -d)
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json
+-  expect_output --substring "no such file or directory"
++  expect_output "Error: no context directory and no Containerfile specified"
+ }
+
+ @test "bud with specified context should fail if directory contains no Dockerfile" {
+@@ -1815,16 +1819,17 @@ _EOF
+   DIR=$(mktemp -d)
+   mkdir -p "$DIR"/Dockerfile
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"
+-  expect_output --substring "is not a file"
++  expect_output --substring "Error: open .*: no such file or directory"
+ }
+
+ @test "bud with specified context should fail if context contains not-existing Dockerfile" {
+   DIR=$(mktemp -d)
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
+-  expect_output --substring "no such file or directory"
++  expect_output --substring "context must be a directory"
+ }
+
+ @test "bud with specified context should succeed if context contains existing Dockerfile" {
++  skip "podman requires a directory, not a Dockerfile"
+   DIR=$(mktemp -d)
+   echo "FROM alpine" > "$DIR"/Dockerfile
+   run_buildah 0 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
+@@ -1876,7 +1881,7 @@ _EOF
+
+ @test "bud-squash-hardlinks" {
+   _prefetch busybox
+-  run_buildah bud --signature-policy ${TESTSDIR}/policy.json --squash ${TESTSDIR}/bud/layers-squash/Dockerfile.hardlinks
++  run_buildah bud --signature-policy ${TESTSDIR}/policy.json --squash -f Dockerfile.hardlinks ${TESTSDIR}/bud/layers-squash
+ }
+
+ @test "bud with additional directory of devices" {
+@@ -2023,6 +2028,7 @@ _EOF
+ }
+
+ @test "bud pull never" {
++  skip "FIXME: podman issue #9573"
+   target=pull
+   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} --pull-never ${TESTSDIR}/bud/pull
+   expect_output --substring "pull policy is \"never\" but \""
+@@ -2042,6 +2048,7 @@ _EOF
+ }
+
+ @test "bud with Containerfile should fail with nonexistent authfile" {
++  skip "FIXME: podman issue #9572"
+   target=alpine-image
+   run_buildah 125 bud --authfile /tmp/nonexistent --signature-policy ${TESTSDIR}/policy.json -t ${target} ${TESTSDIR}/bud/containerfile
+ }
+@@ -2169,6 +2176,7 @@ EOM
+ }
+
+ @test "bud with encrypted FROM image" {
++  skip "Too much effort to spin up a local registry"
+   _prefetch busybox
+   mkdir ${TESTDIR}/tmp
+   openssl genrsa -out ${TESTDIR}/tmp/mykey.pem 1024
+@@ -2241,8 +2249,6 @@ EOM
+   _prefetch alpine
+   run_buildah bud --timestamp=0 --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json -t timestamp -f Dockerfile.1 ${TESTSDIR}/bud/cache-stages
+   cid=$output
+-  run_buildah inspect --format '{{ .Docker.Created }}' timestamp
+-  expect_output --substring "1970-01-01"
+   run_buildah inspect --format '{{ .OCIv1.Created }}' timestamp
+   expect_output --substring "1970-01-01"
+   run_buildah inspect --format '{{ .History }}' timestamp
+diff --git a/tests/helpers.bash b/tests/helpers.bash
+index 5623a0e7..9683360f 100644
+--- a/tests/helpers.bash
++++ b/tests/helpers.bash
+@@ -70,7 +70,7 @@ function _prefetch() {
+             mkdir -p ${_BUILDAH_IMAGE_CACHEDIR}
+         fi
+
+-        local _podman_opts="--root ${TESTDIR}/root --storage-driver ${STORAGE_DRIVER}"
++        local _podman_opts="--root ${TESTDIR}/root --runroot ${TESTDIR}/runroot --storage-driver ${STORAGE_DRIVER}"
+
+         for img in "$@"; do
+             echo "# [checking for: $img]" >&2
+@@ -138,15 +138,35 @@ function run_buildah() {
+         --retry)         retry=3;        shift;;  # retry network flakes
+     esac
+
++    local podman_or_buildah=${BUILDAH_BINARY}
++    if [[ $1 == "bud" || $1 == "build-using-dockerfile" ]]; then
++        shift
++        # podman defaults to --layers=true; buildah to --false.
++        # If command line includes explicit --layers, leave it untouched,
++        # but otherwise update command line so podman mimics buildah default.
++        if [[ "$*" =~ --layers || "$*" =~ --squash ]]; then
++            set "build" "--force-rm=false" "$@"
++        else
++            set "build" "--force-rm=false" "--layers=false" "$@"
++        fi
++        podman_or_buildah=${PODMAN_BINARY}
++
++        # podman always exits 125 where buildah exits 1 or 2
++        case $expected_rc in
++            1|2)   expected_rc=125 ;;
++        esac
++    fi
++    local cmd_basename=$(basename ${podman_or_buildah})
++
+     # 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 timeout --foreground --kill=10 $BUILDAH_TIMEOUT ${BUILDAH_BINARY} --registries-conf ${TESTSDIR}/registries.conf --root ${TESTDIR}/root --runroot ${TESTDIR}/runroot --storage-driver ${STORAGE_DRIVER} "$@"
++        echo "\$ $cmd_basename $*"
++        run timeout --foreground --kill=10 $BUILDAH_TIMEOUT ${podman_or_buildah} --registries-conf ${TESTSDIR}/registries.conf --root ${TESTDIR}/root --runroot ${TESTDIR}/runroot --storage-driver ${STORAGE_DRIVER} "$@"
+         # without "quotes", multiple lines are glommed together into one
+         if [ -n "$output" ]; then
+             echo "$output"
+--
+2.30.2
diff --git a/test/buildah-bud/make-new-buildah-diffs b/test/buildah-bud/make-new-buildah-diffs
@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# This script is intended to help developers get buildah-tests-under-podman
+# working again in case of failure.
+#
+ME=$(basename $0)
+
+die() {
+    echo "$ME: $*" >&2
+    exit 1
+}
+
+# Confirm that we're in a test-buildah* subdir of podman
+whereami=$(basename $(pwd))
+if [[ ! $whereami =~ test-buildah-v ]]; then
+    die "Please run me while cd'ed to a test-buildah-vN.M directory"
+fi
+
+# FIXME: check that git repo is buildah
+git remote -v | grep -q [BUILDAHREPO] \
+    || die "This does not look like a buildah repo (git remote -v)"
+
+# We could do the commit automatically, but it's prudent to require human
+# involvement.
+modified=$(git status --untracked=no --porcelain)
+if [[ -n "$modified" ]]; then
+    echo $modified
+    die "Please commit your changes: git commit --amend --all"
+fi
+
+# Remove any 00??-*.patch files
+rm -f 0001-*.patch
+
+# Check count of commits, barf if need to squash
+n_commits=$(git log --pretty=format:%h [BASETAG]..HEAD | wc -l)
+if [[ $n_commits -gt 1 ]]; then
+    die "Please squash your commits"
+fi
+
+# Scope check: make sure the only files changed are under tests/
+changes=$(git diff --name-status [BASETAG]..HEAD | egrep -v '\stests/')
+if [[ -n "$changes" ]]; then
+    echo $changes
+    die "Found modified files other than under 'tests/'"
+fi
+
+###############################################################################
+# All right - things look good. Generate the patch, and copy it into place.
+
+git format-patch [BASETAG]
+
+# Once again, make sure there's exactly one and only one commit
+shopt -s nullglob
+patch2=$(echo 0002-*.patch)
+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