From 884117471c97efa1d2ef9fd6ffd1d4e12de3b581 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sun, 3 Dec 2023 16:36:33 +1100 Subject: [PATCH] tests: integration: fix spurious SC203[01] shellcheck errors We had ignore lines for these warnings, but it turns out this is most likely a bug in shellcheck and we can work around it by moving the helper function definition before any of the functions that use the helper function. Hopefully it'll be fixed soon. Ref: https://github.com/koalaman/shellcheck/issues/2873 Signed-off-by: Aleksa Sarai --- tests/integration/delete.bats | 135 +++++++++++++++++----------------- tests/integration/events.bats | 69 +++++++++-------- tests/integration/kill.bats | 55 +++++++------- tests/integration/mounts.bats | 27 +++---- 4 files changed, 141 insertions(+), 145 deletions(-) diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats index a1a217485..a7ade1082 100644 --- a/tests/integration/delete.bats +++ b/tests/integration/delete.bats @@ -10,6 +10,72 @@ function teardown() { teardown_bundle } +# See also: "kill KILL [host pidns + init gone]" test in kill.bats. +# +# This needs to be placed at the top of the bats file to work around +# a shellcheck bug. See . +function test_runc_delete_host_pidns() { + requires cgroups_freezer + + update_config ' .linux.namespaces -= [{"type": "pid"}]' + set_cgroups_path + if [ $EUID -ne 0 ]; then + requires rootless_cgroup + # Apparently, for rootless test, when using systemd cgroup manager, + # newer versions of systemd clean up the container as soon as its init + # process is gone. This is all fine and dandy, except it prevents us to + # test this case, thus we skip the test. + # + # It is not entirely clear which systemd version got this feature: + # v245 works fine, and v249 does not. + if [ -v RUNC_USE_SYSTEMD ] && [ "$(systemd_version)" -gt 245 ]; then + skip "rootless+systemd conflicts with systemd > 245" + fi + # Can't mount real /proc when rootless + no pidns, + # so change it to a bind-mounted one from the host. + update_config ' .mounts |= map((select(.type == "proc") + | .type = "none" + | .source = "/proc" + | .options = ["rbind", "nosuid", "nodev", "noexec"] + ) // .)' + fi + + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + cgpath=$(get_cgroup_path "pids") + init_pid=$(cat "$cgpath"/cgroup.procs) + + # Start a few more processes. + for _ in 1 2 3 4 5; do + __runc exec -d test_busybox sleep 1h + done + + # Now kill the container's init process. Since the container do + # not have own PID ns, its init is no special and the container + # will still be up and running. + kill -9 "$init_pid" + + # Get the list of all container processes. + pids=$(cat "$cgpath"/cgroup.procs) + echo "pids: $pids" + # Sanity check -- make sure all processes exist. + for p in $pids; do + kill -0 "$p" + done + + # Must kill those processes and remove container. + runc delete "$@" test_busybox + [ "$status" -eq 0 ] + + runc state test_busybox + [ "$status" -ne 0 ] # "Container does not exist" + + # Make sure all processes are gone. + pids=$(cat "$cgpath"/cgroup.procs) || true # OK if cgroup is gone + echo "pids: $pids" + [ -z "$pids" ] +} + @test "runc delete" { # Need a permission to create a cgroup. # XXX(@kolyshkin): currently this test does not handle rootless when @@ -68,78 +134,10 @@ function teardown() { } # Issue 4047, case "runc delete --force" (different code path). -# shellcheck disable=SC2030 @test "runc delete --force [host pidns + init gone]" { test_runc_delete_host_pidns --force } -# See also: "kill KILL [host pidns + init gone]" test in kill.bats. -function test_runc_delete_host_pidns() { - requires cgroups_freezer - - update_config ' .linux.namespaces -= [{"type": "pid"}]' - set_cgroups_path - if [ $EUID -ne 0 ]; then - requires rootless_cgroup - # Apparently, for rootless test, when using systemd cgroup manager, - # newer versions of systemd clean up the container as soon as its init - # process is gone. This is all fine and dandy, except it prevents us to - # test this case, thus we skip the test. - # - # It is not entirely clear which systemd version got this feature: - # v245 works fine, and v249 does not. - if [ -v RUNC_USE_SYSTEMD ] && [ "$(systemd_version)" -gt 245 ]; then - skip "rootless+systemd conflicts with systemd > 245" - fi - # Can't mount real /proc when rootless + no pidns, - # so change it to a bind-mounted one from the host. - update_config ' .mounts |= map((select(.type == "proc") - | .type = "none" - | .source = "/proc" - | .options = ["rbind", "nosuid", "nodev", "noexec"] - ) // .)' - fi - - runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox - # shellcheck disable=SC2031 - [ "$status" -eq 0 ] - cgpath=$(get_cgroup_path "pids") - init_pid=$(cat "$cgpath"/cgroup.procs) - - # Start a few more processes. - for _ in 1 2 3 4 5; do - __runc exec -d test_busybox sleep 1h - done - - # Now kill the container's init process. Since the container do - # not have own PID ns, its init is no special and the container - # will still be up and running. - kill -9 "$init_pid" - - # Get the list of all container processes. - pids=$(cat "$cgpath"/cgroup.procs) - echo "pids: $pids" - # Sanity check -- make sure all processes exist. - for p in $pids; do - kill -0 "$p" - done - - # Must kill those processes and remove container. - # shellcheck disable=SC2031 - runc delete "$@" test_busybox - # shellcheck disable=SC2031 - [ "$status" -eq 0 ] - - runc state test_busybox - # shellcheck disable=SC2031 - [ "$status" -ne 0 ] # "Container does not exist" - - # Make sure all processes are gone. - pids=$(cat "$cgpath"/cgroup.procs) || true # OK if cgroup is gone - echo "pids: $pids" - [ -z "$pids" ] -} - @test "runc delete --force [paused container]" { runc run -d --console-socket "$CONSOLE_SOCKET" ct1 [ "$status" -eq 0 ] @@ -251,7 +249,6 @@ EOF requires systemd_v244 # Older systemd lacks RuntimeMaxSec support. set_cgroups_path - # shellcheck disable=SC2016 update_config ' .annotations += { "org.systemd.property.RuntimeMaxSec": "2", "org.systemd.property.TimeoutStopSec": "1" diff --git a/tests/integration/events.bats b/tests/integration/events.bats index 707fe4e73..cf42eaf55 100644 --- a/tests/integration/events.bats +++ b/tests/integration/events.bats @@ -10,7 +10,40 @@ function teardown() { teardown_bundle } -# shellcheck disable=SC2030 +# This needs to be placed at the top of the bats file to work around +# a shellcheck bug. See . +function test_events() { + # XXX: currently cgroups require root containers. + requires root + init_cgroup_paths + + local status interval retry_every=1 + if [ $# -eq 2 ]; then + interval="$1" + retry_every="$2" + fi + + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + # Spawn two subshels: + # 1. Event logger that sends stats events to events.log. + (__runc events ${interval:+ --interval "$interval"} test_busybox >events.log) & + # 2. Waits for an event that includes test_busybox then kills the + # test_busybox container which causes the event logger to exit. + ( + retry 10 "$retry_every" grep -q test_busybox events.log + __runc delete -f test_busybox + ) & + wait # for both subshells to finish + + [ -e events.log ] + + output=$(head -1 events.log) + [[ "$output" == [\{]"\"type\""[:]"\"stats\""[,]"\"id\""[:]"\"test_busybox\""[,]* ]] + [[ "$output" == *"data"* ]] +} + @test "events --stats" { # XXX: currently cgroups require root containers. requires root @@ -27,7 +60,6 @@ function teardown() { [[ "${lines[0]}" == *"data"* ]] } -# shellcheck disable=SC2030 @test "events --stats with psi data" { requires root cgroups_v2 psi init_cgroup_paths @@ -56,39 +88,6 @@ function teardown() { done } -function test_events() { - # XXX: currently cgroups require root containers. - requires root - init_cgroup_paths - - local status interval retry_every=1 - if [ $# -eq 2 ]; then - interval="$1" - retry_every="$2" - fi - - runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox - # shellcheck disable=SC2031 - [ "$status" -eq 0 ] - - # Spawn two subshels: - # 1. Event logger that sends stats events to events.log. - (__runc events ${interval:+ --interval "$interval"} test_busybox >events.log) & - # 2. Waits for an event that includes test_busybox then kills the - # test_busybox container which causes the event logger to exit. - ( - retry 10 "$retry_every" grep -q test_busybox events.log - __runc delete -f test_busybox - ) & - wait # for both subshells to finish - - [ -e events.log ] - - output=$(head -1 events.log) - [[ "$output" == [\{]"\"type\""[:]"\"stats\""[,]"\"id\""[:]"\"test_busybox\""[,]* ]] - [[ "$output" == *"data"* ]] -} - @test "events --interval default" { test_events } diff --git a/tests/integration/kill.bats b/tests/integration/kill.bats index 3124bb0e5..d1e0de19d 100644 --- a/tests/integration/kill.bats +++ b/tests/integration/kill.bats @@ -10,32 +10,8 @@ function teardown() { teardown_bundle } -# shellcheck disable=SC2030 -@test "kill detached busybox" { - # run busybox detached - runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox - [ "$status" -eq 0 ] - - # check state - testcontainer test_busybox running - - runc kill test_busybox KILL - [ "$status" -eq 0 ] - wait_for_container 10 1 test_busybox stopped - - # Check that kill errors on a stopped container. - runc kill test_busybox 0 - [ "$status" -ne 0 ] - [[ "$output" == *"container not running"* ]] - - # Check that -a (now obsoleted) makes kill return no error for a stopped container. - runc kill -a test_busybox 0 - [ "$status" -eq 0 ] - - runc delete test_busybox - [ "$status" -eq 0 ] -} - +# This needs to be placed at the top of the bats file to work around +# a shellcheck bug. See . test_host_pidns_kill() { requires cgroups_freezer @@ -53,7 +29,6 @@ test_host_pidns_kill() { fi runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox - # shellcheck disable=SC2031 [ "$status" -eq 0 ] cgpath=$(get_cgroup_path "pids") init_pid=$(cat "$cgpath"/cgroup.procs) @@ -82,7 +57,6 @@ test_host_pidns_kill() { done runc kill test_busybox KILL - # shellcheck disable=SC2031 [ "$status" -eq 0 ] wait_for_container 10 1 test_busybox stopped @@ -92,6 +66,31 @@ test_host_pidns_kill() { [ -z "$pids" ] } +@test "kill detached busybox" { + # run busybox detached + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + # check state + testcontainer test_busybox running + + runc kill test_busybox KILL + [ "$status" -eq 0 ] + wait_for_container 10 1 test_busybox stopped + + # Check that kill errors on a stopped container. + runc kill test_busybox 0 + [ "$status" -ne 0 ] + [[ "$output" == *"container not running"* ]] + + # Check that -a (now obsoleted) makes kill return no error for a stopped container. + runc kill -a test_busybox 0 + [ "$status" -eq 0 ] + + runc delete test_busybox + [ "$status" -eq 0 ] +} + # This is roughly the same as TestPIDHostInitProcessWait in libcontainer/integration. # The differences are: # diff --git a/tests/integration/mounts.bats b/tests/integration/mounts.bats index 3cd01da84..a4d997ca7 100644 --- a/tests/integration/mounts.bats +++ b/tests/integration/mounts.bats @@ -10,6 +10,20 @@ function teardown() { teardown_bundle } +# https://github.com/opencontainers/runc/security/advisories/GHSA-m8cg-xc2p-r3fc +# +# This needs to be placed at the top of the bats file to work around +# a shellcheck bug. See . +function test_ro_cgroup_mount() { + local lines status + # shellcheck disable=SC2016 + update_config '.process.args |= ["sh", "-euc", "for f in `grep /sys/fs/cgroup /proc/mounts | awk \"{print \\\\$2}\"| uniq`; do test -e $f && grep -w $f /proc/mounts | tail -n1; done"]' + runc run test_busybox + [ "$status" -eq 0 ] + [ "${#lines[@]}" -ne 0 ] + for line in "${lines[@]}"; do [[ "${line}" == *'ro,'* ]]; done +} + # https://github.com/opencontainers/runc/issues/3991 @test "runc run [tmpcopyup]" { mkdir -p rootfs/dir1/dir2 @@ -88,22 +102,9 @@ function teardown() { test_ro_cgroup_mount } -# shellcheck disable=SC2030 @test "runc run [ro /sys/fs/cgroup mounts + cgroupns]" { requires cgroupns # With cgroup namespace. update_config '.linux.namespaces |= if index({"type": "cgroup"}) then . else . + [{"type": "cgroup"}] end' test_ro_cgroup_mount } - -# https://github.com/opencontainers/runc/security/advisories/GHSA-m8cg-xc2p-r3fc -# shellcheck disable=SC2031 -function test_ro_cgroup_mount() { - local lines status - # shellcheck disable=SC2016 - update_config '.process.args |= ["sh", "-euc", "for f in `grep /sys/fs/cgroup /proc/mounts | awk \"{print \\\\$2}\"| uniq`; do test -e $f && grep -w $f /proc/mounts | tail -n1; done"]' - runc run test_busybox - [ "$status" -eq 0 ] - [ "${#lines[@]}" -ne 0 ] - for line in "${lines[@]}"; do [[ "${line}" == *'ro,'* ]]; done -}