From f65276db357090269cef0474193f3e13cdeb0db7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 14 Jul 2021 22:33:50 +0000 Subject: [PATCH 1/2] tests/int/helpers: rm $bundle handling It is not used since PR 2757, as all tests are run with cd to bundle directory. runc_spec argument count checking is removed since otherwise shellcheck complains: > SC2120: runc_spec references arguments, but none are ever passed. Signed-off-by: Kir Kolyshkin --- tests/integration/helpers.bash | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index fe3a324b62f..b5d634709d0 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -47,44 +47,34 @@ function __runc() { "$RUNC" ${RUNC_USE_SYSTEMD+--systemd-cgroup} --root "$ROOT/state" "$@" } -# Wrapper for runc spec, which takes only one argument (the bundle path). +# Wrapper for runc spec. function runc_spec() { - ! [[ "$#" > 1 ]] - local args=() - local bundle="" - if [ "$ROOTLESS" -ne 0 ]; then args+=("--rootless") fi - if [ "$#" -ne 0 ]; then - bundle="$1" - args+=("--bundle" "$bundle") - fi runc spec "${args[@]}" # Always add additional mappings if we have idmaps. if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"idmap"* ]]; then - runc_rootless_idmap "$bundle" + runc_rootless_idmap fi } # Helper function to reformat config.json file. Input uses jq syntax. function update_config() { - bundle="${2:-.}" - jq "$1" "$bundle/config.json" | awk 'BEGIN{RS="";getline<"-";print>ARGV[1]}' "$bundle/config.json" + jq "$1" "./config.json" | awk 'BEGIN{RS="";getline<"-";print>ARGV[1]}' "./config.json" } # Shortcut to add additional uids and gids, based on the values set as part of # a rootless configuration. function runc_rootless_idmap() { - bundle="${1:-.}" update_config ' .mounts |= map((select(.type == "devpts") | .options += ["gid=5"]) // .) | .linux.uidMappings += [{"hostID": '"$ROOTLESS_UIDMAP_START"', "containerID": 1000, "size": '"$ROOTLESS_UIDMAP_LENGTH"'}] | .linux.gidMappings += [{"hostID": '"$ROOTLESS_GIDMAP_START"', "containerID": 100, "size": 1}] | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}] - | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' $bundle + | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' } # Returns systemd version as a number (-1 if systemd is not enabled/supported). @@ -141,7 +131,6 @@ function init_cgroup_paths() { # Randomize cgroup path(s), and update cgroupsPath in config.json. # This function sets a few cgroup-related variables. function set_cgroups_path() { - bundle="${1:-.}" init_cgroup_paths local rnd="$RANDOM" @@ -165,7 +154,7 @@ function set_cgroups_path() { CGROUP_PATH=${CGROUP_BASE_PATH}${REL_CGROUPS_PATH} fi - update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"' "$bundle" + update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"' } # Get a value from a cgroup file. @@ -259,15 +248,12 @@ function check_cpu_weight() { # Helper function to set a resources limit function set_resources_limit() { - bundle="${1:-.}" - update_config '.linux.resources.pids.limit |= 100' $bundle + update_config '.linux.resources.pids.limit |= 100' } # Helper function to make /sys/fs/cgroup writable function set_cgroup_mount_writable() { - bundle="${1:-.}" - update_config '.mounts |= map((select(.type == "cgroup") | .options -= ["ro"]) // .)' \ - $bundle + update_config '.mounts |= map((select(.type == "cgroup") | .options -= ["ro"]) // .)' } # Fails the current test, providing the error given. From fb629db693e82a32c0a6ccb03e120b39652cdc90 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 20 Jul 2021 16:16:36 -0700 Subject: [PATCH 2/2] tests/int/helpers: fix shellcheck warnings ... and add the file to be checked by shellcheck. The warnings fixed are: In tests/integration/helpers.bash line 10: INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")") ^----------^ SC2128: Expanding an array without an index only gives the first element. In tests/integration/helpers.bash line 22: TESTDATA="${INTEGRATION_ROOT}/testdata" ^------^ SC2034: TESTDATA appears unused. Verify use (or export if used externally). In tests/integration/helpers.bash line 42: echo "runc $@ (status=$status):" >&2 ^-- SC2145: Argument mixes string and array. Use * or separate argument. ^-----^ SC2154: status is referenced but not assigned. In tests/integration/helpers.bash line 43: echo "$output" >&2 ^-----^ SC2154: output is referenced but not assigned. In tests/integration/helpers.bash line 77: | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}] ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables. In tests/integration/helpers.bash line 78: | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables. ^---------------------^ SC2004: $/${} is unnecessary on arithmetic variables. In tests/integration/helpers.bash line 125: base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'${g}'\>/ { print $5; exit }' /proc/self/mountinfo) ^--^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'"${g}"'\>/ { print $5; exit }' /proc/self/mountinfo) In tests/integration/helpers.bash line 127: eval CGROUP_${g^^}_BASE_PATH="${base_path}" ^----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: eval CGROUP_"${g^^}"_BASE_PATH="${base_path}" In tests/integration/helpers.bash line 229: if [ "x$CGROUP_UNIFIED" = "xyes" ]; then ^----------------^ SC2268: Avoid x-prefix in comparisons as it no longer serves a purpose. Did you mean: if [ "$CGROUP_UNIFIED" = "yes" ]; then In tests/integration/helpers.bash line 234: eval cgroup=\$${var}${REL_CGROUPS_PATH} ^----^ SC2086: Double quote to prevent globbing and word splitting. ^-----------------^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: eval cgroup=\$"${var}""${REL_CGROUPS_PATH}" In tests/integration/helpers.bash line 236: cat $cgroup/$source ^-----^ SC2086: Double quote to prevent globbing and word splitting. ^-----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: cat "$cgroup"/"$source" In tests/integration/helpers.bash line 242: current="$(get_cgroup_value $1)" ^-- SC2086: Double quote to prevent globbing and word splitting. Did you mean: current="$(get_cgroup_value "$1")" In tests/integration/helpers.bash line 245: echo "current" $current "!?" "$expected" ^------^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: echo "current" "$current" "!?" "$expected" In tests/integration/helpers.bash line 257: [ $(id -u) != "0" ] && user="--user" ^------^ SC2046: Quote this to prevent word splitting. In tests/integration/helpers.bash line 259: current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}') ^-----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: current=$(systemctl show $user --property "$source" $SD_UNIT_NAME | awk -F= '{print $2}') In tests/integration/helpers.bash line 261: [ "$current" = "$expected" ] || [ -n "$expected2" -a "$current" = "$expected2" ] ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. In tests/integration/helpers.bash line 309: check_cgroup_value "cpu.weight" $weight ^-----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: check_cgroup_value "cpu.weight" "$weight" In tests/integration/helpers.bash line 310: check_systemd_value "CPUWeight" $weight ^-----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: check_systemd_value "CPUWeight" "$weight" In tests/integration/helpers.bash line 383: if [ $CGROUP_UNIFIED = "no" -a ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. In tests/integration/helpers.bash line 412: local cpu_count=$(grep -c '^processor' /proc/cpuinfo) ^-------^ SC2155: Declare and assign separately to avoid masking return values. In tests/integration/helpers.bash line 450: sleep $delay ^----^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: sleep "$delay" In tests/integration/helpers.bash line 453: echo "Command \"$@\" failed $attempts times. Output: $output" ^-- SC2145: Argument mixes string and array. Use * or separate argument. In tests/integration/helpers.bash line 471: runc state $1 ^-- SC2086: Double quote to prevent globbing and word splitting. Did you mean: runc state "$1" In tests/integration/helpers.bash line 472: if [ $2 == "checkpointed" ]; then ^-- SC2086: Double quote to prevent globbing and word splitting. Did you mean: if [ "$2" == "checkpointed" ]; then In tests/integration/helpers.bash line 484: mkdir $dir ^--^ SC2086: Double quote to prevent globbing and word splitting. Did you mean: mkdir "$dir" In tests/integration/helpers.bash line 497: kill -9 $(cat "$dir/pid") ^---------------^ SC2046: Quote this to prevent word splitting. In tests/integration/helpers.bash line 508: export ROOT=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX") ^--^ SC2155: Declare and assign separately to avoid masking return values. In tests/integration/helpers.bash line 512: cd "$ROOT/bundle" ^---------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. Did you mean: cd "$ROOT/bundle" || exit In tests/integration/helpers.bash line 535: cd "$INTEGRATION_ROOT" ^--------------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. Did you mean: cd "$INTEGRATION_ROOT" || exit For more information: https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ... https://www.shellcheck.net/wiki/SC2034 -- TESTDATA appears unused. Verify u... https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt... Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- tests/integration/helpers.bash | 62 ++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index d386048df41..4c117a9322c 100644 --- a/Makefile +++ b/Makefile @@ -118,7 +118,7 @@ cfmt: indent -linux -l120 -il0 -ppi2 -cp1 -T size_t -T jmp_buf $(C_SRC) shellcheck: - shellcheck tests/integration/*.bats tests/integration/*.sh tests/*.sh script/release.sh + shellcheck tests/integration/*.bats tests/integration/*.sh tests/integration/*.bash tests/*.sh script/release.sh # TODO: add shellcheck for more sh files shfmt: diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index b5d634709d0..b4544185e4d 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -7,7 +7,7 @@ if [ -z "$BATS_RUN_TMPDIR" ]; then fi # Root directory of integration tests. -INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")") +INTEGRATION_ROOT=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")") # Download images, get *_IMAGE variables. IMAGES=$("${INTEGRATION_ROOT}"/get-images.sh) @@ -18,6 +18,7 @@ RUNC="${INTEGRATION_ROOT}/../../runc" RECVTTY="${INTEGRATION_ROOT}/../../contrib/cmd/recvtty/recvtty" # Test data path. +# shellcheck disable=SC2034 TESTDATA="${INTEGRATION_ROOT}/testdata" # CRIU PATH @@ -38,7 +39,9 @@ function runc() { # Some debug information to make life easier. bats will only print it if the # test failed, in which case the output is useful. - echo "runc $@ (status=$status):" >&2 + # shellcheck disable=SC2154 + echo "runc $* (status=$status):" >&2 + # shellcheck disable=SC2154 echo "$output" >&2 } @@ -73,8 +76,8 @@ function runc_rootless_idmap() { update_config ' .mounts |= map((select(.type == "devpts") | .options += ["gid=5"]) // .) | .linux.uidMappings += [{"hostID": '"$ROOTLESS_UIDMAP_START"', "containerID": 1000, "size": '"$ROOTLESS_UIDMAP_LENGTH"'}] | .linux.gidMappings += [{"hostID": '"$ROOTLESS_GIDMAP_START"', "containerID": 100, "size": 1}] - | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}] - | .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' + | .linux.gidMappings += [{"hostID": '"$((ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}] + | .linux.gidMappings += [{"hostID": '"$((ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$((ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' } # Returns systemd version as a number (-1 if systemd is not enabled/supported). @@ -121,9 +124,9 @@ function init_cgroup_paths() { CGROUP_SUBSYSTEMS=$(awk '!/^#/ {print $1}' /proc/cgroups) local g base_path for g in ${CGROUP_SUBSYSTEMS}; do - base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'${g}'\>/ { print $5; exit }' /proc/self/mountinfo) + base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'"${g}"'\>/ { print $5; exit }' /proc/self/mountinfo) test -z "$base_path" && continue - eval CGROUP_${g^^}_BASE_PATH="${base_path}" + eval CGROUP_"${g^^}"_BASE_PATH="${base_path}" done fi } @@ -162,39 +165,39 @@ function get_cgroup_value() { local source=$1 local cgroup var current - if [ "x$CGROUP_UNIFIED" = "xyes" ]; then + if [ "$CGROUP_UNIFIED" = "yes" ]; then cgroup=$CGROUP_PATH else var=${source%%.*} # controller name (e.g. memory) var=CGROUP_${var^^}_BASE_PATH # variable name (e.g. CGROUP_MEMORY_BASE_PATH) - eval cgroup=\$${var}${REL_CGROUPS_PATH} + eval cgroup=\$"${var}${REL_CGROUPS_PATH}" fi - cat $cgroup/$source + cat "$cgroup/$source" } # Helper to check a if value in a cgroup file matches the expected one. function check_cgroup_value() { local current - current="$(get_cgroup_value $1)" + current="$(get_cgroup_value "$1")" local expected=$2 - echo "current" $current "!?" "$expected" + echo "current $current !? $expected" [ "$current" = "$expected" ] } # Helper to check a value in systemd. function check_systemd_value() { [ -z "${RUNC_USE_SYSTEMD}" ] && return - local source=$1 + local source="$1" [ "$source" = "unsupported" ] && return local expected="$2" local expected2="$3" local user="" - [ $(id -u) != "0" ] && user="--user" + [ "$(id -u)" != "0" ] && user="--user" - current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}') + current=$(systemctl show $user --property "$source" "$SD_UNIT_NAME" | awk -F= '{print $2}') echo "systemd $source: current $current !? $expected $expected2" - [ "$current" = "$expected" ] || [ -n "$expected2" -a "$current" = "$expected2" ] + [ "$current" = "$expected" ] || [[ -n "$expected2" && "$current" = "$expected2" ]] } function check_cpu_quota() { @@ -242,8 +245,8 @@ function check_cpu_shares() { function check_cpu_weight() { local weight=$1 - check_cgroup_value "cpu.weight" $weight - check_systemd_value "CPUWeight" $weight + check_cgroup_value "cpu.weight" "$weight" + check_systemd_value "CPUWeight" "$weight" } # Helper function to set a resources limit @@ -316,7 +319,7 @@ function requires() { ;; cgroups_swap) init_cgroup_paths - if [ $CGROUP_UNIFIED = "no" -a ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then + if [ $CGROUP_UNIFIED = "no" ] && [ ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then skip_me=1 fi ;; @@ -345,8 +348,9 @@ function requires() { fi ;; smp) - local cpu_count=$(grep -c '^processor' /proc/cpuinfo) - if [ "$cpu_count" -lt 2 ]; then + local cpus + cpus=$(grep -c '^processor' /proc/cpuinfo) + if [ "$cpus" -lt 2 ]; then skip_me=1 fi ;; @@ -383,10 +387,10 @@ function retry() { if [[ "$status" -eq 0 ]]; then return 0 fi - sleep $delay + sleep "$delay" done - echo "Command \"$@\" failed $attempts times. Output: $output" + echo "Command \"$*\" failed $attempts times. Output: $output" false } @@ -404,8 +408,8 @@ function wait_for_container() { function testcontainer() { # test state of container - runc state $1 - if [ $2 == "checkpointed" ]; then + runc state "$1" + if [ "$2" == "checkpointed" ]; then [ "$status" -eq 1 ] return fi @@ -417,7 +421,7 @@ function setup_recvtty() { [ -z "$ROOT" ] && return 1 # must not be called without ROOT set local dir="$ROOT/tty" - mkdir $dir + mkdir "$dir" export CONSOLE_SOCKET="$dir/sock" # We need to start recvtty in the background, so we double fork in the shell. @@ -430,7 +434,7 @@ function teardown_recvtty() { # When we kill recvtty, the container will also be killed. if [ -f "$dir/pid" ]; then - kill -9 $(cat "$dir/pid") + kill -9 "$(cat "$dir/pid")" fi # Clean up the files that might be left over. @@ -441,11 +445,11 @@ function setup_bundle() { local image="$1" # Root for various container directories (state, tty, bundle). - export ROOT=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX") + ROOT=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX") mkdir -p "$ROOT/state" "$ROOT/bundle/rootfs" setup_recvtty - cd "$ROOT/bundle" + cd "$ROOT/bundle" || return tar --exclude './dev/*' -C rootfs -xf "$image" @@ -468,7 +472,7 @@ function setup_debian() { function teardown_bundle() { [ -z "$ROOT" ] && return 0 # nothing to teardown - cd "$INTEGRATION_ROOT" + cd "$INTEGRATION_ROOT" || return teardown_recvtty local ct for ct in $(__runc list -q); do