Skip to content

Commit

Permalink
buildah bud tests under podman-remote
Browse files Browse the repository at this point in the history
New functionality -- mostly in the diffs we apply to
buildah's helpers.bash -- to enable running buildah-bud
tests under podman-remote. The gist of it is, we start
a 'podman system service' before each test, and clean
it up on test exit.

Design decision: the diff file for helpers.bash is no
longer trailing-whitespace-clean: that ended up producing
diffs that git wouldn't apply, because in some cases
the whitespace is actually important. In order to pass CI,
we need to exclude this file from some checks.

Signed-off-by: Ed Santiago <[email protected]>
  • Loading branch information
edsantiago committed Jul 28, 2021
1 parent f9395dd commit ec9dad7
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 34 deletions.
6 changes: 5 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
61 changes: 48 additions & 13 deletions test/buildah-bud/apply-podman-deltas
Original file line number Diff line number Diff line change
Expand Up @@ -56,37 +56,46 @@ 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
#
# 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
Expand Down Expand Up @@ -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"

Expand All @@ -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)" \
Expand All @@ -163,6 +173,31 @@ skip "FIXME FIXME FIXME - requires updated CI images (#10829)" \
# BEGIN tests which are skipped due to actual podman bugs.


###############################################################################
# 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.

Expand Down
93 changes: 81 additions & 12 deletions test/buildah-bud/buildah-tests.diff
Original file line number Diff line number Diff line change
@@ -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 <[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/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.
Expand All @@ -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
Expand All @@ -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

8 changes: 3 additions & 5 deletions test/buildah-bud/make-new-buildah-diffs
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions test/buildah-bud/run-buildah-bud-tests
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit ec9dad7

Please sign in to comment.