Skip to content

Commit

Permalink
buildah bud tests under podman-remote
Browse files Browse the repository at this point in the history
A huge ton of skips: podman-remote build just doesn't
really work at all. Lots of issues filed, a few more
left to go. Submitting this just to see how it does in CI.

Signed-off-by: Ed Santiago <[email protected]>
  • Loading branch information
edsantiago authored and vrothberg committed Jul 28, 2021
1 parent a5de831 commit f1aa663
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 36 deletions.
10 changes: 7 additions & 3 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ validate_task:
bindings_task:
name: "Test Bindings"
alias: bindings
only_if: &not_docs $CIRRUS_CHANGE_TITLE !=~ '.*CI:DOCS.*'
only_if: &not_docs $CIRRUS_CHANGE_TITLE =~ '.*CI:DOCS.*'
skip: *branches_and_tags
depends_on:
- build
Expand Down Expand Up @@ -590,7 +590,7 @@ buildah_bud_test_task:
name: *std_name_fmt
alias: buildah_bud_test
skip: *tags
only_if: *not_docs
# only_if: *not_docs
depends_on:
- local_integration_test
env:
Expand All @@ -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
73 changes: 60 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,43 @@ 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.

#skip_if_remote "FIXME: podman #10907 (complicated .dockerignore file)" \
# "bud with .dockerignore"
#
#skip_if_remote "FIXME: podman #10908 (.containerignore + .dockerignore)" \
# "bud with .containerignore"

# Warning messages go to server, not client
# FIXME FIXME FIXME: this is now buildah #3285, which merged 2021-06-15.
# FIXME FIXME FIXME: remove this once we vendor in a buildah > v1.21.1
skip_if_remote "FIXME: podman #10616 - cpp stderr shown on server end" \
"bud with preprocessor error"

###############################################################################
# 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 dc44d1c82c87bf49183b23a99dc61272f934d6d6 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 f1aa663

Please sign in to comment.