tests: fix checks for non-existent variables

Audit all checks for non-empty variables (i.e. ' -z ', ' -n ',
' != ""' and '= ""'), and fix those cases where a variable might be
unset. Those variables (that might not be set) are

 - RUNC_USE_SYSTEMD
 - BATS_RUN_TMPDIR
 - AUX_UID
 - AUX_DIR
 - SD_PARENT_NAME
 - REL_PARENT_PATH
 - ROOT
 - HAVE_CRIU
 - ROOTLESS_FEATURES
 - and a few test-specific or file-specific variables

This should allow us to enable set -u.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2022-02-07 10:07:43 -08:00
parent 99d5c0231f
commit c8c3e8526d
7 changed files with 42 additions and 38 deletions

View File

@@ -45,7 +45,7 @@ function setup() {
runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions
[ "$status" -eq 0 ]
if [ "$CGROUP_UNIFIED" != "no" ]; then
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
if [ -v RUNC_USE_SYSTEMD ]; then
if [ "$(id -u)" = "0" ]; then
check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/machine.slice/cgroup.controllers)"
else

View File

@@ -252,7 +252,7 @@ function simple_cr() {
# in time when the last page is lazily transferred to the destination.
# Killing the CRIU on the checkpoint side will let the container
# continue to run if the migration failed at some point.
[ -n "$RUNC_USE_SYSTEMD" ] && set_cgroups_path
[ -v RUNC_USE_SYSTEMD ] && set_cgroups_path
runc_restore_with_pipes ./image-dir test_busybox_restore --lazy-pages
wait $cpt_pid

View File

@@ -35,7 +35,7 @@ function teardown() {
# Some setup for this test (AUX_DIR and AUX_UID) is done
# by rootless.sh. Check that setup is done...
if [[ ! -d "$AUX_DIR" || -z "$AUX_UID" ]]; then
if [[ ! -v AUX_UID || ! -v AUX_DIR || ! -d "$AUX_DIR" ]]; then
skip "bad/unset AUX_DIR/AUX_UID"
fi
# ... and is correct, i.e. the current user

View File

@@ -1,7 +1,7 @@
#!/bin/bash
# bats-core v1.2.1 defines BATS_RUN_TMPDIR
if [ -z "$BATS_RUN_TMPDIR" ]; then
# bats-core v1.2.1 defines BATS_RUN_TMPDIR.
if [ ! -v BATS_RUN_TMPDIR ]; then
echo "bats >= v1.2.1 is required. Aborting." >&2
exit 1
fi
@@ -19,6 +19,10 @@ RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty"
SD_HELPER="${INTEGRATION_ROOT}/../../contrib/cmd/sd-helper/sd-helper"
SECCOMP_AGENT="${INTEGRATION_ROOT}/../../contrib/cmd/seccompagent/seccompagent"
# Some variables may not always be set. Set those to empty value,
# if unset, to avoid "unbound variable" error.
: "${ROOTLESS_FEATURES:=}"
# Test data path.
# shellcheck disable=SC2034
TESTDATA="${INTEGRATION_ROOT}/testdata"
@@ -66,7 +70,7 @@ function runc_spec() {
runc spec $rootless
# Always add additional mappings if we have idmaps.
if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"idmap"* ]]; then
if [[ "$ROOTLESS" -ne 0 && "$ROOTLESS_FEATURES" == *"idmap"* ]]; then
runc_rootless_idmap
fi
}
@@ -88,7 +92,7 @@ function runc_rootless_idmap() {
# Returns systemd version as a number (-1 if systemd is not enabled/supported).
function systemd_version() {
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
if [ -v RUNC_USE_SYSTEMD ]; then
systemctl --version | awk '/^systemd / {print $2; exit}'
return
fi
@@ -98,7 +102,7 @@ function systemd_version() {
function init_cgroup_paths() {
# init once
test -n "$CGROUP_UNIFIED" && return
[ -v CGROUP_UNIFIED ] && return
if stat -f -c %t /sys/fs/cgroup | grep -qFw 63677270; then
CGROUP_UNIFIED=yes
@@ -106,7 +110,7 @@ function init_cgroup_paths() {
# For rootless + systemd case, controllers delegation is required,
# so check the controllers that the current user has, not the top one.
# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04).
if [[ "$ROOTLESS" -ne 0 && -n "$RUNC_USE_SYSTEMD" ]]; then
if [[ "$ROOTLESS" -ne 0 && -v RUNC_USE_SYSTEMD ]]; then
controllers="/sys/fs/cgroup/user.slice/user-$(id -u).slice/user@$(id -u).service/cgroup.controllers"
fi
@@ -141,11 +145,11 @@ function init_cgroup_paths() {
}
function create_parent() {
if [ -n "$RUNC_USE_SYSTEMD" ]; then
[ -z "$SD_PARENT_NAME" ] && return
if [ -v RUNC_USE_SYSTEMD ]; then
[ ! -v SD_PARENT_NAME ] && return
"$SD_HELPER" --parent machine.slice start "$SD_PARENT_NAME"
else
[ -z "$REL_PARENT_PATH" ] && return
[ ! -v REL_PARENT_PATH ] && return
if [ "$CGROUP_UNIFIED" == "yes" ]; then
mkdir "/sys/fs/cgroup$REL_PARENT_PATH"
else
@@ -161,11 +165,11 @@ function create_parent() {
}
function remove_parent() {
if [ -n "$RUNC_USE_SYSTEMD" ]; then
[ -z "$SD_PARENT_NAME" ] && return
if [ -v RUNC_USE_SYSTEMD ]; then
[ ! -v SD_PARENT_NAME ] && return
"$SD_HELPER" --parent machine.slice stop "$SD_PARENT_NAME"
else
[ -z "$REL_PARENT_PATH" ] && return
[ ! -v REL_PARENT_PATH ] && return
if [ "$CGROUP_UNIFIED" == "yes" ]; then
rmdir "/sys/fs/cgroup/$REL_PARENT_PATH"
else
@@ -180,8 +184,8 @@ function remove_parent() {
}
function set_parent_systemd_properties() {
[ -z "$SD_PARENT_NAME" ] && return
local user
[ ! -v SD_PARENT_NAME ] && return
local user=""
[ "$(id -u)" != "0" ] && user="--user"
systemctl set-property $user "$SD_PARENT_NAME" "$@"
}
@@ -194,7 +198,7 @@ function set_parent_systemd_properties() {
# refer to it.
function set_cgroups_path() {
init_cgroup_paths
local pod dash_pod slash_pod pod_slice
local pod dash_pod="" slash_pod="" pod_slice=""
if [ "$#" -ne 0 ] && [ "$1" != "" ]; then
# Set up a parent/pod cgroup.
pod="$1"
@@ -205,7 +209,7 @@ function set_cgroups_path() {
fi
local rnd="$RANDOM"
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
if [ -v RUNC_USE_SYSTEMD ]; then
SD_UNIT_NAME="runc-cgroups-integration-test-${rnd}.scope"
if [ "$(id -u)" = "0" ]; then
REL_PARENT_PATH="/machine.slice${pod_slice}"
@@ -227,7 +231,7 @@ function set_cgroups_path() {
CGROUP_PATH=${CGROUP_BASE_PATH}${REL_CGROUPS_PATH}
fi
[ -n "$pod" ] && create_parent
[ -v pod ] && create_parent
update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"'
}
@@ -259,11 +263,11 @@ function check_cgroup_value() {
# Helper to check a value in systemd.
function check_systemd_value() {
[ -z "${RUNC_USE_SYSTEMD}" ] && return
[ ! -v RUNC_USE_SYSTEMD ] && return
local source="$1"
[ "$source" = "unsupported" ] && return
local expected="$2"
local expected2="$3"
local expected2="${3:-}"
local user=""
[ "$(id -u)" != "0" ] && user="--user"
@@ -339,7 +343,7 @@ function fail() {
# Check whether rootless runc can use cgroups.
function rootless_cgroup() {
[[ "$ROOTLESS_FEATURES" == *"cgroup"* || -n "$RUNC_USE_SYSTEMD" ]]
[[ "$ROOTLESS_FEATURES" == *"cgroup"* || -v RUNC_USE_SYSTEMD ]]
}
# Allows a test to specify what things it requires. If the environment can't
@@ -349,7 +353,7 @@ function requires() {
local skip_me
case $var in
criu)
if [ -n "$HAVE_CRIU" ]; then
if [ ! -v HAVE_CRIU ]; then
skip_me=1
fi
;;
@@ -379,7 +383,7 @@ function requires() {
fi
;;
rootless_no_features)
if [ "$ROOTLESS_FEATURES" != "" ]; then
if [ -n "$ROOTLESS_FEATURES" ]; then
skip_me=1
fi
;;
@@ -414,7 +418,7 @@ function requires() {
;;
cgroups_hybrid)
init_cgroup_paths
if [ "$CGROUP_HYBRID" != "yes" ]; then
if [ ! -v CGROUP_HYBRID ]; then
skip_me=1
fi
;;
@@ -433,12 +437,12 @@ function requires() {
fi
;;
systemd)
if [ -z "${RUNC_USE_SYSTEMD}" ]; then
if [ ! -v RUNC_USE_SYSTEMD ]; then
skip_me=1
fi
;;
no_systemd)
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
if [ -v RUNC_USE_SYSTEMD ]; then
skip_me=1
fi
;;
@@ -451,7 +455,7 @@ function requires() {
fail "BUG: Invalid requires $var."
;;
esac
if [ -n "$skip_me" ]; then
if [ -v skip_me ]; then
skip "test requires $var"
fi
done
@@ -501,7 +505,7 @@ function testcontainer() {
}
function setup_recvtty() {
[ -z "$ROOT" ] && return 1 # must not be called without ROOT set
[ ! -v ROOT ] && return 1 # must not be called without ROOT set
local dir="$ROOT/tty"
mkdir "$dir"
@@ -512,7 +516,7 @@ function setup_recvtty() {
}
function teardown_recvtty() {
[ -z "$ROOT" ] && return 0 # nothing to teardown
[ ! -v ROOT ] && return 0 # nothing to teardown
local dir="$ROOT/tty"
# When we kill recvtty, the container will also be killed.
@@ -571,7 +575,7 @@ function setup_debian() {
}
function teardown_bundle() {
[ -z "$ROOT" ] && return 0 # nothing to teardown
[ ! -v ROOT ] && return 0 # nothing to teardown
cd "$INTEGRATION_ROOT" || return
teardown_recvtty

View File

@@ -13,10 +13,11 @@ function setup() {
}
function teardown() {
if [ -n "$LIBPATH" ]; then
if [ -v LIBPATH ]; then
umount "$LIBPATH"/$HOOKLIBCR.1.0.0 &>/dev/null || true
umount "$LIBPATH"/$HOOKLIBCC.1.0.0 &>/dev/null || true
rm -f $HOOKLIBCR.1.0.0 $HOOKLIBCC.1.0.0
unset LIBPATH HOOKLIBCR HOOKLIBCC
fi
teardown_bundle
}

View File

@@ -399,7 +399,7 @@ EOF
set_cgroups_path "pod_${RANDOM}"
# Set parent/pod CPU quota limit to 50%.
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
if [ -v RUNC_USE_SYSTEMD ]; then
set_parent_systemd_properties CPUQuota="50%"
else
echo 50000 >"/sys/fs/cgroup/cpu/$REL_PARENT_PATH/cpu.cfs_quota_us"

View File

@@ -20,12 +20,11 @@
# 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
# cgroup is managed by systemd when RUNC_USE_SYSTEMD is set.
if [ -v RUNC_USE_SYSTEMD ]; then
ALL_FEATURES=("idmap")
fi
ROOT="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")"
@@ -182,7 +181,7 @@ for enabled_features in $features_powerset; do
# Run the test suite!
echo "path: $PATH"
export ROOTLESS_FEATURES="$enabled_features"
if [[ -n "${RUNC_USE_SYSTEMD}" ]]; then
if [ -v 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