[PATCH nft 1/2] tests/shell: kill running child processes when aborting "run-tests.sh"

[PATCH nft 1/2] tests/shell: kill running child processes when aborting "run-tests.sh"

[Thomas Haller]

When aborting "run-tests.sh", child processes were left running.  Kill
them. It's surprisingly complicated to get this somewhat right. Do it by
enabling monitor mode for each test call, so that they run in separate
process groups and we can kill the entire group.

Note that we cannot just `kill -- -$$`, because it's not clear who is in
this process group. Also, we don't want to kill the `tee` process which
handles our logging.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index f20a2bec9e9b..cf17302fdc19 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -424,12 +424,29 @@ if [ ! -x "$DIFF" ] ; then
 	DIFF=true
 fi
 
+declare -A JOBS_PIDLIST
+
 cleanup_on_exit() {
+	pids_search=''
+	for pid in "${!JOBS_PIDLIST[@]}" ; do
+		kill -- "-$pid" &>/dev/null
+		pids_search="$pids_search\\|\\<$pid\\>"
+	done
+	if [ -n "$pids_search" ] ; then
+		pids_search="${pids_search:2}"
+		for i in {1..100}; do
+			ps xh -o pgrp | grep -q "$pids_search" || break
+			sleep 0.01
+		done
+	fi
 	if [ "$NFT_TEST_KEEP_LOGS" != y -a -n "$NFT_TEST_TMPDIR" ] ; then
 		rm -rf "$NFT_TEST_TMPDIR"
 	fi
 }
-trap cleanup_on_exit EXIT
+
+trap 'exit 130' SIGINT
+trap 'exit 143' SIGTERM
+trap 'rc=$?; cleanup_on_exit; exit $rc' EXIT
 
 NFT_TEST_TMPDIR="$(mktemp --tmpdir="$_TMPDIR" -d "nft-test.$(date '+%Y%m%d-%H%M%S.%3N').XXXXXX")" ||
 	msg_error "Failure to create temp directory in \"$_TMPDIR\""
@@ -628,7 +645,6 @@ print_test_result() {
 }
 
 declare -A JOBS_TEMPDIR
-declare -A JOBS_PIDLIST
 
 job_start() {
 	local testfile="$1"
@@ -656,7 +672,8 @@ job_wait()
 	while [ "$JOBS_N_RUNNING" -gt 0 -a "$JOBS_N_RUNNING" -ge "$num_jobs" ] ; do
 		wait -n -p JOBCOMPLETED
 		local rc_got="$?"
-		testfile2="${JOBS_PIDLIST[$JOBCOMPLETED]}"
+		local testfile2="${JOBS_PIDLIST[$JOBCOMPLETED]}"
+		unset JOBS_PIDLIST[$JOBCOMPLETED]
 		print_test_result "${JOBS_TEMPDIR["$testfile2"]}" "$testfile2" "$rc_got"
 		((JOBS_N_RUNNING--))
 		check_kmemleak
@@ -677,8 +694,12 @@ for testfile in "${TESTS[@]}" ; do
 	chmod 755 "$NFT_TEST_TESTTMPDIR"
 	JOBS_TEMPDIR["$testfile"]="$NFT_TEST_TESTTMPDIR"
 
-	job_start "$testfile" "$TESTIDX" &
-	JOBS_PIDLIST[$!]="$testfile"
+	[[ -o monitor ]] && set_old_state='set -m' || set_old_state='set +m'
+	set -m
+	( job_start "$testfile" "$TESTIDX" ) &
+	pid=$!
+	eval "$set_old_state"
+	JOBS_PIDLIST[$pid]="$testfile"
 	((JOBS_N_RUNNING++))
 done
 
-- 
2.41.0

[PATCH nft 2/2] tests/shell: ensure vgdb-pipe files are deleted from "nft-valgrind-wrapper.sh"

[Thomas Haller]

When the valgrind process gets killed, those files can be left over.
They are located in the original $TMPDIR (usually /tmp). They should be
cleaned up.

I tried to cleanup the files from withing "nft-valgrind-wrapper.sh"
itself via a `trap`, but it doesn't work. Instead, let "run-tests.sh"
delete all files with a matching pattern.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/nft-valgrind-wrapper.sh |  4 ++--
 tests/shell/run-tests.sh                    | 10 +++++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/shell/helpers/nft-valgrind-wrapper.sh b/tests/shell/helpers/nft-valgrind-wrapper.sh
index 6125dd0f3089..98bbdf43570e 100755
--- a/tests/shell/helpers/nft-valgrind-wrapper.sh
+++ b/tests/shell/helpers/nft-valgrind-wrapper.sh
@@ -1,6 +1,6 @@
 #!/bin/bash -e
 
-SUFFIX="$(date "+%Y%m%d-%H%M%S.%6N.$$")"
+SUFFIX="$(date "+%H%M%S.%6N").$$"
 
 rc=0
 libtool \
@@ -12,7 +12,7 @@ libtool \
 		--show-leak-kinds=all \
 		--num-callers=100 \
 		--error-exitcode=122 \
-		--vgdb-prefix="$NFT_TEST_TMPDIR_ORIG/vgdb-pipe-nft-test-$SUFFIX" \
+		--vgdb-prefix="$_NFT_TEST_VALGRIND_VGDB_PREFIX-$SUFFIX" \
 		$NFT_TEST_VALGRIND_OPTS \
 		"$NFT_REAL" \
 		"$@" \
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index cf17302fdc19..7ac6202ca43c 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -426,6 +426,8 @@ fi
 
 declare -A JOBS_PIDLIST
 
+_NFT_TEST_VALGRIND_VGDB_PREFIX=
+
 cleanup_on_exit() {
 	pids_search=''
 	for pid in "${!JOBS_PIDLIST[@]}" ; do
@@ -442,13 +444,17 @@ cleanup_on_exit() {
 	if [ "$NFT_TEST_KEEP_LOGS" != y -a -n "$NFT_TEST_TMPDIR" ] ; then
 		rm -rf "$NFT_TEST_TMPDIR"
 	fi
+	if [ -n "$_NFT_TEST_VALGRIND_VGDB_PREFIX" ] ; then
+		rm -rf "$_NFT_TEST_VALGRIND_VGDB_PREFIX"* &>/dev/null
+	fi
 }
 
 trap 'exit 130' SIGINT
 trap 'exit 143' SIGTERM
 trap 'rc=$?; cleanup_on_exit; exit $rc' EXIT
 
-NFT_TEST_TMPDIR="$(mktemp --tmpdir="$_TMPDIR" -d "nft-test.$(date '+%Y%m%d-%H%M%S.%3N').XXXXXX")" ||
+TIMESTAMP=$(date '+%Y%m%d-%H%M%S.%3N')
+NFT_TEST_TMPDIR="$(mktemp --tmpdir="$_TMPDIR" -d "nft-test.$TIMESTAMP.XXXXXX")" ||
 	msg_error "Failure to create temp directory in \"$_TMPDIR\""
 chmod 755 "$NFT_TEST_TMPDIR"
 
@@ -493,6 +499,8 @@ msg_info "info: NFT_TEST_TMPDIR=$(printf '%q' "$NFT_TEST_TMPDIR")"
 if [ "$VALGRIND" == "y" ]; then
 	NFT="$NFT_TEST_BASEDIR/helpers/nft-valgrind-wrapper.sh"
 	msg_info "info: NFT=$(printf '%q' "$NFT")"
+	_NFT_TEST_VALGRIND_VGDB_PREFIX="$NFT_TEST_TMPDIR_ORIG/vgdb-pipe-nft-test-$TIMESTAMP.$$.$RANDOM"
+	export _NFT_TEST_VALGRIND_VGDB_PREFIX
 fi
 
 kernel_cleanup() {
-- 
2.41.0