From bc175ba423e174981ca97331a95097f073fb1507 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 14 Jan 2021 23:44:47 -0800 Subject: [PATCH 1/4] tests/helpers.bash: rm GOPATH It is not used anywhere since commit 30601efa81df06d3. Signed-off-by: Kir Kolyshkin --- tests/integration/helpers.bash | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index cea6deda6a3..8b12aa0c933 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -7,7 +7,6 @@ INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")") RUNC="${INTEGRATION_ROOT}/../../runc" RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty" -GOPATH="$(mktemp -d --tmpdir runc-integration-gopath.XXXXXX)" # Test data path. TESTDATA="${INTEGRATION_ROOT}/testdata" From 5ab05884329244744f4c10d3a479f2ef764af373 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Jan 2021 00:11:01 -0800 Subject: [PATCH 2/4] ci: untangle getting test images This simplifies and optimizes getting container images used for tests. Currently, we have three different ways of getting images: 1. (for hello-world) the image is in this repo under tests/integration/testdata. 2. (for busybox) download it from github (the repo that is used for preparing official Docker image) using curl. 3. (for debian) download from Docker hub, using skopeo and umoci. To further complicate things, we have to do this downloading in multiple scenarios (at least 4): locally, in github CI, from Dockefile, inside a Vagrant VM. For each scenario, we have to install skopeo and umoci, and those two are not yet universally available for all the distros that we use. Yet another complication is those images are used for tests/integration (bats-driven tests) as well as for libcontainer/integration (go tests). The tests in libcontainer/integration rely on busybox being available from /busybox, and the bats tests just download the images to a temporary location during every run. It is also hard to support CI for other architectures, because all the machinery for preparing images is so complicated. This commit is an attempt to simplify and optimize getting images, mostly by getting rid of skopeo and umoci dependencies, but also by moving the download logic into one small shell script, which is used from all the places. Benefits: - images (if not present) are only downloaded once; - same images are used for both kind of tests (go and bats); - same images are used for local and inside-docker tests (because source directory is mounted into container); - the download logic is located within 1 simple shell script. [v2: fix eval; more doc to get-images; print URL if curl failed] [v3: use "slim" debian, twice as small] [v4: fix not using $image in setup_bundle] [v5: don't remove TESTDATA from helpers.bash] [v6: add i386 support] Signed-off-by: Kir Kolyshkin --- .github/workflows/test.yml | 20 +---- Dockerfile | 21 ------ Makefile | 4 +- Vagrantfile.centos7 | 12 +-- Vagrantfile.fedora33 | 13 +--- libcontainer/integration/utils_test.go | 35 ++++++++- tests/integration/get-images.sh | 70 ++++++++++++++++++ tests/integration/helpers.bash | 62 +++++----------- tests/integration/hooks.bats | 1 + tests/integration/multi-arch.bash | 44 ----------- tests/integration/testdata/.gitignore | 2 + ...{hello-world.tar => hello-world-amd64.tar} | Bin ...orld-aarch64.tar => hello-world-arm64.tar} | Bin tests/rootless.sh | 4 + 14 files changed, 135 insertions(+), 153 deletions(-) create mode 100755 tests/integration/get-images.sh delete mode 100644 tests/integration/multi-arch.bash create mode 100644 tests/integration/testdata/.gitignore rename tests/integration/testdata/{hello-world.tar => hello-world-amd64.tar} (100%) rename tests/integration/testdata/{hello-world-aarch64.tar => hello-world-arm64.tar} (100%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 484c5b24e9c..fae239821f1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,13 +29,10 @@ jobs: - name: install deps run: | - # skopeo repo - echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_10/ /' | sudo tee /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list - wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_10/Release.key -O- | sudo apt-key add - # criu repo sudo add-apt-repository -y ppa:criu/ppa # apt-add-repository runs apt update so we don't have to - sudo apt -q install libseccomp-dev criu skopeo + sudo apt -q install libseccomp-dev criu - name: install go ${{ matrix.go-version }} uses: actions/setup-go@v2 @@ -46,26 +43,11 @@ jobs: - name: build run: sudo -E PATH="$PATH" make all - - name: install umoci - run: | - mkdir ~/bin - echo "PATH=$HOME/bin:$PATH" >> $GITHUB_ENV - curl -o ~/bin/umoci -fsSL https://github.com/opencontainers/umoci/releases/download/v0.4.6/umoci.amd64 - chmod +x ~/bin/umoci - - name: install bats uses: mig4/setup-bats@v1 with: bats-version: 1.2.1 - - name: get images for libcontainer/integration unit test - # no rootless unit tests - if: matrix.rootless != 'rootless' - run: | - sudo mkdir /busybox /debian - . tests/integration/multi-arch.bash - curl -fsSL `get_busybox` | sudo tar xfJC - /busybox - - name: unit test if: matrix.rootless != 'rootless' run: sudo -E PATH="$PATH" -- make localunittest diff --git a/Dockerfile b/Dockerfile index f5ce90c0efe..1d8a27dcb6a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,14 +1,11 @@ ARG GO_VERSION=1.15 ARG BATS_VERSION=v1.2.1 -ARG UMOCI_VERSION=v0.4.6 FROM golang:${GO_VERSION}-buster ARG DEBIAN_FRONTEND=noninteractive RUN echo 'deb https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_10/ /' > /etc/apt/sources.list.d/criu.list \ && wget -nv https://download.opensuse.org/repositories/devel:/tools:/criu/Debian_10/Release.key -O- | apt-key add - \ - && echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_10/ /' > /etc/apt/sources.list.d/skopeo.list \ - && wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_10/Release.key -O- | apt-key add - \ && dpkg --add-architecture armel \ && dpkg --add-architecture armhf \ && dpkg --add-architecture arm64 \ @@ -35,7 +32,6 @@ RUN echo 'deb https://download.opensuse.org/repositories/devel:/tools:/criu/Debi libseccomp2 \ pkg-config \ python-minimal \ - skopeo \ sudo \ uidmap \ && apt-get clean \ @@ -56,21 +52,4 @@ RUN cd /tmp \ && ./install.sh /usr/local \ && rm -rf /tmp/bats-core -# install umoci -ARG UMOCI_VERSION -RUN curl -o /usr/local/bin/umoci -fsSL https://github.com/opencontainers/umoci/releases/download/${UMOCI_VERSION}/umoci.amd64 \ - && chmod +x /usr/local/bin/umoci - WORKDIR /go/src/github.com/opencontainers/runc - -# setup a playground for us to spawn containers in -COPY tests/integration/multi-arch.bash tests/integration/ -ENV ROOTFS /busybox -RUN mkdir -p "${ROOTFS}" -RUN . tests/integration/multi-arch.bash \ - && curl -fsSL `get_busybox` | tar xfJC - "${ROOTFS}" - -ENV DEBIAN_ROOTFS /debian -RUN mkdir -p "${DEBIAN_ROOTFS}" -RUN . tests/integration/multi-arch.bash \ - && get_and_extract_debian "$DEBIAN_ROOTFS" diff --git a/Makefile b/Makefile index eaa3a903eed..423df4b23dd 100644 --- a/Makefile +++ b/Makefile @@ -123,8 +123,8 @@ validate: $(GO) vet $(MOD_VENDOR) ./... shellcheck: - shellcheck tests/integration/*.bats - # TODO: add shellcheck for sh files + shellcheck tests/integration/*.bats tests/integration/*.sh + # TODO: add shellcheck for more sh files shfmt: shfmt -ln bats -d -w tests/integration/*.bats diff --git a/Vagrantfile.centos7 b/Vagrantfile.centos7 index 4dcfa4bca61..559f8dd397e 100644 --- a/Vagrantfile.centos7 +++ b/Vagrantfile.centos7 @@ -17,21 +17,16 @@ Vagrant.configure("2") do |config| # configuration GO_VERSION="1.15" BATS_VERSION="v1.2.1" - UMOCI_VERSION="v0.4.6" # install yum packages yum install -y -q epel-release (cd /etc/yum.repos.d && curl -O https://copr.fedorainfracloud.org/coprs/adrian/criu-el7/repo/epel-7/adrian-criu-el7-epel-7.repo) - yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make skopeo criu + yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make criu yum clean all # install Go curl -fsSL "https://dl.google.com/go/go${GO_VERSION}.linux-amd64.tar.gz" | tar Cxz /usr/local - # Install umoci - curl -o /usr/local/bin/umoci -fsSL https://github.com/opencontainers/umoci/releases/download/${UMOCI_VERSION}/umoci.amd64 - chmod +x /usr/local/bin/umoci - # install bats git clone https://github.com/bats-core/bats-core cd bats-core @@ -53,10 +48,5 @@ EOF # Add a user for rootless tests useradd -u2000 -m -d/home/rootless -s/bin/bash rootless - - # Add busybox for libcontainer/integration tests - . /vagrant/tests/integration/multi-arch.bash \ - && mkdir /busybox \ - && curl -fsSL $(get_busybox) | tar xfJC - /busybox SHELL end diff --git a/Vagrantfile.fedora33 b/Vagrantfile.fedora33 index dc5fc30c9c6..7e0dac608e1 100644 --- a/Vagrantfile.fedora33 +++ b/Vagrantfile.fedora33 @@ -21,7 +21,7 @@ Vagrant.configure("2") do |config| config exclude kernel,kernel-core config install_weak_deps false update -install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu skopeo +install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu ts run EOF done @@ -36,17 +36,6 @@ EOF cat /root/rootless.key.pub >> /home/rootless/.ssh/authorized_keys chown -R rootless.rootless /home/rootless - # Install umoci - UMOCI_VERSION=v0.4.6 - curl -o /usr/local/bin/umoci -fsSL https://github.com/opencontainers/umoci/releases/download/${UMOCI_VERSION}/umoci.amd64 - chmod +x /usr/local/bin/umoci - - # Add busybox for libcontainer/integration tests - . /vagrant/tests/integration/multi-arch.bash \ - && mkdir /busybox /debian \ - && curl -fsSL $(get_busybox) | tar xfJC - /busybox \ - && get_and_extract_debian /debian - # Delegate cgroup v2 controllers to rootless user via --systemd-cgroup mkdir -p /etc/systemd/system/user@.service.d cat > /etc/systemd/system/user@.service.d/delegate.conf << EOF diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go index 8b2d714e235..59db2778df3 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "runtime" "strings" "syscall" @@ -19,6 +20,36 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) +var busyboxTar string + +// init makes sure the container images are downloaded, +// and initializes busyboxTar. If images can't be downloaded, +// we are unable to run any tests, so panic. +func init() { + // Figure out path to get-images.sh. Note it won't work + // in case the compiled test binary is moved elsewhere. + _, ex, _, _ := runtime.Caller(0) + getImages, err := filepath.Abs(filepath.Join(filepath.Dir(ex), "..", "..", "tests", "integration", "get-images.sh")) + if err != nil { + panic(err) + } + // Call it to make sure images are downloaded, and to get the paths. + out, err := exec.Command(getImages).CombinedOutput() + if err != nil { + panic(fmt.Errorf("getImages error %s (output: %s)", err, out)) + } + // Extract the value of BUSYBOX_IMAGE. + found := regexp.MustCompile(`(?m)^BUSYBOX_IMAGE=(.*)$`).FindSubmatchIndex(out) + if len(found) < 4 { + panic(fmt.Errorf("unable to find BUSYBOX_IMAGE= in %q", out)) + } + busyboxTar = string(out[found[2]:found[3]]) + // Finally, check the file is present + if _, err := os.Stat(busyboxTar); err != nil { + panic(err) + } +} + func ptrInt(v int) *int { return &v } @@ -114,9 +145,9 @@ func remove(dir string) { // copyBusybox copies the rootfs for a busybox container created for the test image // into the new directory for the specific test func copyBusybox(dest string) error { - out, err := exec.Command("sh", "-c", fmt.Sprintf("cp -a /busybox/* %s/", dest)).CombinedOutput() + out, err := exec.Command("sh", "-c", fmt.Sprintf("tar --exclude './dev/*' -C %q -xf %q", dest, busyboxTar)).CombinedOutput() if err != nil { - return fmt.Errorf("copy error %q: %q", err, out) + return fmt.Errorf("untar error %q: %q", err, out) } return nil } diff --git a/tests/integration/get-images.sh b/tests/integration/get-images.sh new file mode 100755 index 00000000000..4a5c92b4c14 --- /dev/null +++ b/tests/integration/get-images.sh @@ -0,0 +1,70 @@ +#!/bin/bash + +# This script checks if container images needed for tests (currently +# busybox and Debian 10 "Buster") are available locally, and downloads +# them to testdata directory if not. +# +# The script is self-contained/standalone and is used from a few places +# that need to ensure the images are downloaded. Its output is suitable +# for consumption by shell via eval (see helpers.bash). +# +# XXX: Latest available images are fetched. Theoretically, +# this can bring some instability in case of a broken image. +# In this case, images will need to be pinned to a checksum +# on a per-image and per-architecture basis. + +set -e -u -o pipefail + +# Root directory of integration tests. +INTEGRATION_ROOT=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")") +# Test data path. +TESTDATA="${INTEGRATION_ROOT}/testdata" +# Sanity check: $TESTDATA directory must exist. +if [ ! -d "$TESTDATA" ]; then + echo "Bad TESTDATA directory: $TESTDATA. Aborting" >&2 + exit 1 +fi + +function get() { + local dest="$1" url="$2" + + [ -e "$dest" ] && return + + # Sanity check: $TESTDATA directory must be writable. + if [ ! -w "$TESTDATA" ]; then + echo "TESTDATA directory ($TESTDATA) not writable. Aborting" >&2 + exit 1 + fi + + if ! curl -o "$dest" -fsSL "$url"; then + echo "Failed to get $url" 1>&2 + exit 1 + fi +} + +arch=$(go env GOARCH) +# Convert from GOARCH to whatever the URLs below are using. +case $arch in +arm64) + arch=arm64v8 + ;; +386) + arch=i386 + ;; +esac + +# busybox +BUSYBOX_IMAGE="$TESTDATA/busybox-${arch}.tar.xz" +get "$BUSYBOX_IMAGE" \ + "https://github.com/docker-library/busybox/raw/dist-${arch}/stable/glibc/busybox.tar.xz" +echo "BUSYBOX_IMAGE=$BUSYBOX_IMAGE" + +# debian +DEBIAN_IMAGE="$TESTDATA/debian-${arch}.tar.xz" +get "$DEBIAN_IMAGE" \ + "https://github.com/debuerreotype/docker-debian-artifacts/raw/dist-${arch}/buster/slim/rootfs.tar.xz" +echo "DEBIAN_IMAGE=$DEBIAN_IMAGE" + +# hello-world is local, no need to download. +HELLO_IMAGE="$TESTDATA/hello-world-${arch}.tar" +echo "HELLO_IMAGE=$HELLO_IMAGE" diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index 8b12aa0c933..59905e1b471 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -3,7 +3,10 @@ # Root directory of integration tests. INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")") -. ${INTEGRATION_ROOT}/multi-arch.bash +# Download images, get *_IMAGE variables. +IMAGES=$("${INTEGRATION_ROOT}"/get-images.sh) +eval "$IMAGES" +unset IMAGES RUNC="${INTEGRATION_ROOT}/../../runc" RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty" @@ -11,16 +14,9 @@ RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty" # Test data path. TESTDATA="${INTEGRATION_ROOT}/testdata" -# Busybox image -BUSYBOX_IMAGE="$BATS_TMPDIR/busybox.tar" +# Destinations for test containers. BUSYBOX_BUNDLE="$BATS_TMPDIR/busyboxtest" - -# hello-world in tar format -HELLO_FILE=$(get_hello) -HELLO_IMAGE="$TESTDATA/$HELLO_FILE" HELLO_BUNDLE="$BATS_TMPDIR/hello-world" - -# debian image DEBIAN_BUNDLE="$BATS_TMPDIR/debiantest" # CRIU PATH @@ -475,48 +471,30 @@ function teardown_recvtty() { rm -f "$CONSOLE_SOCKET" } -function setup_busybox() { +function setup_bundle() { + local image="$1" + local bundle="$2" + setup_recvtty - mkdir -p "$BUSYBOX_BUNDLE"/rootfs - if [ -e "/testdata/busybox.tar" ]; then - BUSYBOX_IMAGE="/testdata/busybox.tar" - fi - if [ ! -e $BUSYBOX_IMAGE ]; then - curl -o $BUSYBOX_IMAGE -sSL $(get_busybox) - fi - tar --exclude './dev/*' -C "$BUSYBOX_BUNDLE"/rootfs -xf "$BUSYBOX_IMAGE" - cd "$BUSYBOX_BUNDLE" + mkdir -p "$bundle"/rootfs + cd "$bundle" + + tar --exclude './dev/*' -C rootfs -xf "$image" + runc_spec } +function setup_busybox() { + setup_bundle "$BUSYBOX_IMAGE" "$BUSYBOX_BUNDLE" +} + function setup_hello() { - setup_recvtty - mkdir -p "$HELLO_BUNDLE"/rootfs - tar --exclude './dev/*' -C "$HELLO_BUNDLE"/rootfs -xf "$HELLO_IMAGE" - cd "$HELLO_BUNDLE" - runc_spec + setup_bundle "$HELLO_IMAGE" "$HELLO_BUNDLE" update_config '(.. | select(.? == "sh")) |= "/hello"' } function setup_debian() { - # skopeo and umoci are not installed on the travis runner - if [ -n "${RUNC_USE_SYSTEMD}" ]; then - return - fi - - setup_recvtty - mkdir -p "$DEBIAN_BUNDLE" - - if [ ! -d "$DEBIAN_ROOTFS/rootfs" ]; then - get_and_extract_debian "$DEBIAN_BUNDLE" - fi - - # Use the cached version - if [ ! -d "$DEBIAN_BUNDLE/rootfs" ]; then - cp -r "$DEBIAN_ROOTFS"/* "$DEBIAN_BUNDLE/" - fi - - cd "$DEBIAN_BUNDLE" + setup_bundle "$DEBIAN_IMAGE" "$DEBIAN_BUNDLE" } function teardown_running_container() { diff --git a/tests/integration/hooks.bats b/tests/integration/hooks.bats index 7f1401f6503..b7d0ff4cb05 100644 --- a/tests/integration/hooks.bats +++ b/tests/integration/hooks.bats @@ -48,6 +48,7 @@ function teardown() { .hooks |= . + {"createRuntime": [{"path": "/bin/sh", "args": ["/bin/sh", "-c", $create_runtime_hook]}]} | .hooks |= . + {"createContainer": [{"path": "/bin/sh", "args": ["/bin/sh", "-c", $create_container_hook]}]} | .hooks |= . + {"startContainer": [{"path": "/bin/sh", "args": ["/bin/sh", "-c", "ldconfig"]}]} | + .root.readonly |= false | .process.args = ["/bin/sh", "-c", "ldconfig -p | grep librunc"]' "$DEBIAN_BUNDLE"/config.json) echo "${CONFIG}" >config.json diff --git a/tests/integration/multi-arch.bash b/tests/integration/multi-arch.bash deleted file mode 100644 index 1dd751bbd3b..00000000000 --- a/tests/integration/multi-arch.bash +++ /dev/null @@ -1,44 +0,0 @@ -#!/bin/bash -get_busybox() { - case $(go env GOARCH) in - arm64) - echo 'https://github.com/docker-library/busybox/raw/dist-arm64v8/stable/glibc/busybox.tar.xz' - ;; - *) - echo 'https://github.com/docker-library/busybox/raw/dist-amd64/stable/glibc/busybox.tar.xz' - ;; - esac -} - -get_hello() { - case $(go env GOARCH) in - arm64) - echo 'hello-world-aarch64.tar' - ;; - *) - echo 'hello-world.tar' - ;; - esac -} - -get_and_extract_debian() { - tmp=$(mktemp -d) - cd "$tmp" - - debian="debian:3.11.6" - - case $(go env GOARCH) in - arm64) - skopeo copy docker://arm64v8/debian:buster "oci:$debian" - ;; - *) - skopeo copy docker://amd64/debian:buster "oci:$debian" - ;; - esac - - args="$([ -z "${ROOTLESS_TESTPATH+x}" ] && echo "--rootless")" - umoci unpack $args --image "$debian" "$1" - - cd - - rm -rf "$tmp" -} diff --git a/tests/integration/testdata/.gitignore b/tests/integration/testdata/.gitignore new file mode 100644 index 00000000000..70f55824adf --- /dev/null +++ b/tests/integration/testdata/.gitignore @@ -0,0 +1,2 @@ +busybox-*.tar.xz +debian-*.tar.xz diff --git a/tests/integration/testdata/hello-world.tar b/tests/integration/testdata/hello-world-amd64.tar similarity index 100% rename from tests/integration/testdata/hello-world.tar rename to tests/integration/testdata/hello-world-amd64.tar diff --git a/tests/integration/testdata/hello-world-aarch64.tar b/tests/integration/testdata/hello-world-arm64.tar similarity index 100% rename from tests/integration/testdata/hello-world-aarch64.tar rename to tests/integration/testdata/hello-world-arm64.tar diff --git a/tests/rootless.sh b/tests/rootless.sh index 929fec2c2f6..70623659300 100755 --- a/tests/rootless.sh +++ b/tests/rootless.sh @@ -144,6 +144,10 @@ function powerset() { } features_powerset="$(powerset "${ALL_FEATURES[@]}")" +# Make sure we have container images downloaded, as otherwise +# rootless user won't be able to write to $TESTDATA. +"$ROOT"/tests/integration/get-images.sh >/dev/null + # Iterate over the powerset of all features. IFS=: for enabled_features in $features_powerset; do From 1143759338d0ccea9ce40dd699676eeb08298494 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Jan 2021 11:23:49 -0800 Subject: [PATCH 3/4] tests/rootless.sh: use set -e -u -o pipefail Currently, set -e ("exit on error") is only set before running tests. The bad consequence is the script may fail to set up test environment but will still run the tests. Use "set -e -u -o pipefail" for the whole script to catch potential issues with test setup and/or the script itself. A few caveats: 1. We have to set RUNC_USE_SYSTEMD to an empty value if it is unset, so that the subsequent checks like [ -z "$RUNC_USE_SYSTEMD" ] won't trigger a "unbound variable" error. Same for ROOTLESS_TESTPATH. 2. Functions that have code like [ -f $file ] && do_something towards the end may return 1 in case $file does not exist (as test aka [ was the last operation performed, and its exit code is returned. This is why we had to add return 0. Signed-off-by: Kir Kolyshkin --- tests/rootless.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/rootless.sh b/tests/rootless.sh index 70623659300..09214c5f937 100755 --- a/tests/rootless.sh +++ b/tests/rootless.sh @@ -19,6 +19,10 @@ # a new feature, please match the existing style. Add an entry to $ALL_FEATURES, # and add an enable_* and disable_* hook. +set -e -u -o pipefail +: "${RUNC_USE_SYSTEMD:=}" +: "${ROOTLESS_TESTPATH:=}" + ALL_FEATURES=("idmap" "cgroup") # cgroup is managed by systemd when RUNC_USE_SYSTEMD is set if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then @@ -54,14 +58,12 @@ function enable_idmap() { # Create a directory owned by $AUX_UID inside container, to be used # by a test case in cwd.bats. This setup can't be done by the test itself, # as it needs root for chown. - set -e export AUX_UID=1024 AUX_DIR="$(mktemp -d)" # 1000 is linux.uidMappings.containerID value, # as set by runc_rootless_idmap chown "$((ROOTLESS_UIDMAP_START - 1000 + AUX_UID))" "$AUX_DIR" export AUX_DIR - set +e } function disable_idmap() { @@ -75,10 +77,12 @@ function disable_idmap() { # Deactivate new{uid,gid}map helpers. setuid is preserved with mv(1). [ -e /usr/bin/newuidmap ] && mv /usr/bin/{,unused-}newuidmap [ -e /usr/bin/newgidmap ] && mv /usr/bin/{,unused-}newgidmap + + return 0 } function cleanup() { - if [ -n "$AUX_DIR" ]; then + if [ -v AUX_DIR ]; then rmdir "$AUX_DIR" unset AUX_DIX fi @@ -133,6 +137,8 @@ function disable_cgroup() { done # cgroup v2 [ -d "$CGROUP_MOUNT/$CGROUP_PATH" ] && rmdir "$CGROUP_MOUNT/$CGROUP_PATH" + + return 0 } # Create a powerset of $ALL_FEATURES (the set of all subsets of $ALL_FEATURES). @@ -150,6 +156,7 @@ features_powerset="$(powerset "${ALL_FEATURES[@]}")" # Iterate over the powerset of all features. IFS=: +idx=0 for enabled_features in $features_powerset; do idx="$(($idx + 1))" echo "[$(printf '%.2d' "$idx")] run rootless tests ... (${enabled_features%%+})" @@ -162,7 +169,6 @@ for enabled_features in $features_powerset; do done # Run the test suite! - set -e echo path: $PATH export ROOTLESS_FEATURES="$enabled_features" if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then @@ -173,6 +179,5 @@ for enabled_features in $features_powerset; do else sudo -HE -u rootless PATH="$PATH" $(which bats) -t "$ROOT/tests/integration$ROOTLESS_TESTPATH" fi - set +e cleanup done From c348b98203a8f04dda33e8ea4f8a6f87c6a36160 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 15 Jan 2021 12:29:07 -0800 Subject: [PATCH 4/4] tests/rootless.sh: fix/ignore shellcheck warnings Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- tests/rootless.sh | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 423df4b23dd..e13d8f1d47f 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,7 @@ validate: $(GO) vet $(MOD_VENDOR) ./... shellcheck: - shellcheck tests/integration/*.bats tests/integration/*.sh + shellcheck tests/integration/*.bats tests/integration/*.sh tests/*.sh # TODO: add shellcheck for more sh files shfmt: diff --git a/tests/rootless.sh b/tests/rootless.sh index 09214c5f937..bacea49d649 100755 --- a/tests/rootless.sh +++ b/tests/rootless.sh @@ -28,7 +28,7 @@ ALL_FEATURES=("idmap" "cgroup") if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then ALL_FEATURES=("idmap") fi -ROOT="$(readlink -f "$(dirname "${BASH_SOURCE}")/..")" +ROOT="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")" # FEATURE: Opportunistic new{uid,gid}map support, allowing a rootless container # to be set up with the usage of helper setuid binaries. @@ -94,7 +94,8 @@ function cleanup() { # List of cgroups. We handle name= cgroups as well as combined # (comma-separated) cgroups and correctly split and/or strip them. -ALL_CGROUPS=($(cat /proc/self/cgroup | cut -d: -f2 | sed -E '{s/^name=//;s/,/\n/;/^$/D}')) +# shellcheck disable=SC2207 +ALL_CGROUPS=($(cut -d: -f2 "$CGROUP_MOUNT/cgroup.subtree_control"; done + # shellcheck disable=SC2013 + for f in $(cat "$CGROUP_MOUNT/cgroup.controllers"); do echo "+$f" >"$CGROUP_MOUNT/cgroup.subtree_control"; done set +x # Create the cgroup. mkdir -p "$CGROUP_MOUNT/$CGROUP_PATH" @@ -146,7 +148,7 @@ function disable_cgroup() { # feature knobs this shouldn't take too long -- but the number of tested # combinations is O(2^n)). function powerset() { - eval printf '%s' $(printf '{,%s+}' "$@"): + eval printf '%s' "$(printf '{,%s+}' "$@")": } features_powerset="$(powerset "${ALL_FEATURES[@]}")" @@ -158,26 +160,26 @@ features_powerset="$(powerset "${ALL_FEATURES[@]}")" IFS=: idx=0 for enabled_features in $features_powerset; do - idx="$(($idx + 1))" - echo "[$(printf '%.2d' "$idx")] run rootless tests ... (${enabled_features%%+})" + ((++idx)) + printf "[%.2d] run rootless tests ... (${enabled_features%%+})\n" "$idx" unset IFS for feature in "${ALL_FEATURES[@]}"; do hook_func="disable_$feature" - grep -E "(^|\+)$feature(\+|$)" <<<$enabled_features &>/dev/null && hook_func="enable_$feature" + grep -E "(^|\+)$feature(\+|$)" <<<"$enabled_features" &>/dev/null && hook_func="enable_$feature" "$hook_func" done # Run the test suite! - echo path: $PATH + echo "path: $PATH" export ROOTLESS_FEATURES="$enabled_features" if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then # We use `ssh rootless@localhost` instead of `sudo -u rootless` for creating systemd user session. # Alternatively we could use `machinectl shell`, but it is known not to work well on SELinux-enabled hosts as of April 2020: # https://bugzilla.redhat.com/show_bug.cgi?id=1788616 - ssh -t -t -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i $HOME/rootless.key rootless@localhost -- PATH="$PATH" RUNC_USE_SYSTEMD="$RUNC_USE_SYSTEMD" bats -t "$ROOT/tests/integration$ROOTLESS_TESTPATH" + ssh -t -t -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i "$HOME/rootless.key" rootless@localhost -- PATH="$PATH" RUNC_USE_SYSTEMD="$RUNC_USE_SYSTEMD" bats -t "$ROOT/tests/integration$ROOTLESS_TESTPATH" else - sudo -HE -u rootless PATH="$PATH" $(which bats) -t "$ROOT/tests/integration$ROOTLESS_TESTPATH" + sudo -HE -u rootless PATH="$PATH" "$(which bats)" -t "$ROOT/tests/integration$ROOTLESS_TESTPATH" fi cleanup done