diff options
author | Ed Santiago <santiago@redhat.com> | 2021-02-09 14:33:19 -0700 |
---|---|---|
committer | Ed Santiago <santiago@redhat.com> | 2021-03-21 06:59:01 -0600 |
commit | 3baa9da4edec72a70aec4aa25a29b4afd82ed689 (patch) | |
tree | 8b7c1c6259c4f041518e7fce271b46dd21639fb5 /test/buildah-bud | |
parent | ebc9871c9358b41daefc37e5db8119f596770cb7 (diff) | |
download | podman-3baa9da4edec72a70aec4aa25a29b4afd82ed689.tar.gz podman-3baa9da4edec72a70aec4aa25a29b4afd82ed689.tar.bz2 podman-3baa9da4edec72a70aec4aa25a29b4afd82ed689.zip |
WIP: run buildah bud tests using podman
Set of scripts to run buildah's bud.bats test using
podman build in podman CI.
podman build is not 100% compatible with buildah bud.
In particular:
* podman defaults to --layers=true; buildah to false
* podman defaults to --force-rm=true; buildah to false
* podman error exit status is 125; buildah is 2
* differences in error messages, command-line arguments
Some of the above can be dealt with programmatically,
by tweaking the buildah helpers.bash (BATS helpers).
Some need to be tweaked by patching bud.bats itself.
This PR includes a patch that will, I fear, need to
be periodically maintained over time.
There will likely be failures when vendoring in a
new buildah, possibly because new tests were added
for new features that don't exist in podman, possibly
(I hope unlikely) if existing tests are changed in
ways that make the patch file fail to apply. I've
tried to write good instructions and to write the run
script in such a way that it will offer helpful hints
on failure. My instructions and code will be imperfect;
I hope they will be good enough to merit continued use
of this test (possibly with improvements to the instructions
as we learn more about real-world failures).
Signed-off-by: Ed Santiago <santiago@redhat.com>
Diffstat (limited to 'test/buildah-bud')
-rw-r--r-- | test/buildah-bud/README.md | 82 | ||||
-rw-r--r-- | test/buildah-bud/buildah-tests.diff | 192 | ||||
-rw-r--r-- | test/buildah-bud/make-new-buildah-diffs | 63 | ||||
-rwxr-xr-x | test/buildah-bud/run-buildah-bud-tests | 166 |
4 files changed, 503 insertions, 0 deletions
diff --git a/test/buildah-bud/README.md b/test/buildah-bud/README.md new file mode 100644 index 000000000..88e4bbc3c --- /dev/null +++ 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 new file mode 100644 index 000000000..92adabcac --- /dev/null +++ b/test/buildah-bud/buildah-tests.diff @@ -0,0 +1,192 @@ +From c85882a8f7fb6efbf4d59dfe8340bfbef57ccd48 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/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 new file mode 100644 index 000000000..1191f4597 --- /dev/null +++ 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 diff --git a/test/buildah-bud/run-buildah-bud-tests b/test/buildah-bud/run-buildah-bud-tests new file mode 100755 index 000000000..67c8fdfa4 --- /dev/null +++ b/test/buildah-bud/run-buildah-bud-tests @@ -0,0 +1,166 @@ +#!/bin/bash + +ME=$(basename $0) + +############################################################################### +# BEGIN user-customizable section + +# Buildah main repository; unlikely to change often +BUILDAH_REPO=github.com/containers/buildah + +# Tag name used to identify the base checkout +BASE_TAG=buildah-bud-in-podman + +# END user-customizable section +############################################################################### + +usage="Usage: $ME [--help] [--no-checkout] [--no-test] +" + +# Parse command-line options (used in development only, not in CI) +do_checkout=y +do_test=y +for i; do + case "$i" in + --no-checkout) do_checkout= ; shift;; + --no-test) do_test= ; shift;; + -h|--help) echo "$usage"; exit 0;; + *) echo "$ME: Unrecognized option '$i'" >&2; exit 1;; + esac +done + +# Patches helpers.bash and potentially other files (bud.bats? Dockerfiles?) +# +# The patch file is horrible to generate: +# 1) cd to the checked-out buildah/tests directory +# 2) make your edits +# 3) git commit -asm 'blah blah blah' +# 3a) if checked-out directory already includes earlier patches, +# you may need to 'git commit --amend' instead +# 4) git format-patch HEAD^ +# 5) sed -e 's/ \+$//' 0001* >../PATCH-FILE-PATH +# 6) vim that file, remove trailing empty newlines +# 7) cd back out of buildah directory, and git-commit this new patch file +# +# FIXME: this makes me nervous. The diff will probably need tweaking +# over time. I don't think we need to version it, because we +# *have* to be in lockstep with a specific buildah version, +# so problems should only arise when we re-vendor. +# But I'm still nervous and can't put my finger on the reason. +# +# Complicated invocation needed because we 'cd' down below. +BUD_TEST_DIR=$(realpath $(dirname ${BASH_SOURCE[0]})) +PATCHES=${BUD_TEST_DIR}/buildah-tests.diff + +# Friendlier relative path to our buildah-tests dir +BUD_TEST_DIR_REL=$(dirname $(git ls-files --full-name ${BASH_SOURCE[0]})) +# Path to podman binary; again, do it before we cd +PODMAN_BINARY=$(pwd)/bin/podman +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 +fi + +function die() { + failhint= + echo "$ME: $*" >&2 + exit 1 +} + +# From here on out, any unexpected abort will try to offer helpful hints +failhint= +trap 'if [[ $? != 0 ]]; then if [[ -n $failhint ]]; then echo;echo "***************************************";echo $failhint;echo;echo "Please see $BUD_TEST_DIR_REL/README.md for advice";fi;fi' 0 + +# Find the version of buildah we've vendored in, so we can run the right tests +buildah_version=$(awk "\$1 == \"$BUILDAH_REPO\" { print \$2 }" <go.mod) + +if [[ -z "$buildah_version" ]]; then + # This should not happen + die "Did not find '$BUILDAH_REPO' in go.mod" +fi + +# From here on out, any error is fatal +set -e + +# Before pulling buildah (while still cd'ed to podman repo), try to determine +# if this is a PR, and if so if it's a revendoring of buildah. We use this to +# try to offer a helpful hint on failure. +is_revendor= +if [[ -n $CIRRUS_CHANGE_IN_REPO ]]; then + if [[ -n $DEST_BRANCH ]]; then + head=${CIRRUS_CHANGE_IN_REPO} + # Base of this PR. + base=$(set -x;git merge-base ${DEST_BRANCH} $head) + changes=$(set -x;git diff --name-status $base $head) + if [[ -n $changes ]]; then + if [[ $changes =~ vendor/$BUILDAH_REPO ]]; then + is_revendor=y + fi + fi + fi +fi + +# Pull buildah, including tests +buildah_dir=test-buildah-$buildah_version +if [[ -n $do_checkout ]]; then + if [[ -d $buildah_dir ]]; then + die "Directory already exists: $buildah_dir" + fi + + failhint="'git clone' failed - this should never happen!" + (set -x;git clone -q --branch $buildah_version https://$BUILDAH_REPO $buildah_dir) + + cd $buildah_dir + + # Give it a recognizable tag; this will be useful if we need to update + # the set of patches + (set -x;git tag $BASE_TAG) + + # Build buildah + failhint="error building buildah. This should never happen." + (set -x;make bin/buildah) + + # Apply custom patches. We do this _after_ building, although it shouldn't + # matter because these patches should only apply to test scripts. + failhint=" +Error applying patch file. This can happen when you vendor in a new buildah." + (set -x;git am <$PATCHES) + + failhint= + sed -e "s,\[BASETAG\],${BASE_TAG},g" \ + -e "s,\[BUILDAHREPO\],${BUILDAH_REPO},g" \ + < ${BUD_TEST_DIR}/make-new-buildah-diffs \ + > make-new-buildah-diffs + chmod 755 make-new-buildah-diffs +else + # Called with --no-checkout + test -d $buildah_dir || die "Called with --no-checkout, but $buildah_dir does not exist" + + cd $buildah_dir +fi + +if [[ -n $do_test ]]; then + failhint="Error running buildah bud tests under podman." + if [[ -n $is_revendor ]]; then + failhint+=" + +It looks like you're vendoring in a new buildah. The likely failure +here is that there's a new test in bud.bats that uses functionality +not (yet) in podman build. You will likely need to 'skip' that test. +" + else + failhint+=" + +Is it possible that your PR breaks podman build in some way? Please +review the test failure and double-check your changes. +" + fi + + (set -x;sudo env TMPDIR=/var/tmp \ + PODMAN_BINARY=$PODMAN_BINARY \ + BUILDAH_BINARY=$(pwd)/bin/buildah \ + bats tests/bud.bats) +fi |