munge/0001-Sharness-Add-munged_kill_daemon-and-munged_cleanup.patch

From c593d4ff7b1fc37bb67bffaa1e0a896b136fdff6 Mon Sep 17 00:00:00 2001
From: Chris Dunlap <cdunlap@llnl.gov>
Date: Fri, 4 Dec 2020 17:00:06 -0800
Subject: [PATCH 1/4] Sharness: Add munged_kill_daemon and munged_cleanup

Add munged_kill_daemon() to kill an errant munged process left running
in the background from a previous test, and munged_cleanup() which
currently only calls munged_kill_daemon().

The situation of an errant munged process is most likely to occur when
a munged test is expected to fail and instead erroneously succeeds
since those tests do not include a corresponding munged_stop_daemon().

munged_cleanup() should be called at the end of any test script
that starts a munged process.  It is not necessary to call it or
munged_kill_daemon() after every munged process is supposedly stopped
since munged_kill_daemon() is now called by munged_start_daemon()
to kill any previous munged process named in the $(MUNGE_PIDFILE}
before starting a new one.  By only checking for a leftover munged
process named in the pidfile, munged_kill_daemon() will not interfere
with munged processes belonging to other tests or system use.

Tested:
- Arch Linux
- CentOS Stream 8, 8.3.2011, 7.9.2009, 6.10
- Debian sid, 10.8, 9.13, 8.11, 7.11, 6.0.10, 5.0.10, 4.0
- Fedora 33, 32, 31
- FreeBSD 12.2, 11.4
- NetBSD 9.1, 9.0, 8.1
- OpenBSD 6.8, 6.7, 6.6
- openSUSE 15.2, 15.1
- Raspberry Pi OS (Raspbian 10) [armv7l]
- Ubuntu 20.10, 20.04.2 LTS, 18.04.5 LTS, 16.04.7 LTS, 14.04.6 LTS, 12.04.5 LTS

Tested by calling munged_start_daemon() without a corresponding
munged_stop_daemon() in order to leave the munged process running in
the background.  The test suite was run with debug=t and verbose=t
to check for the test_debug() message for the killed munged pid.
---
 t/0010-basic.t                    |  4 +++
 t/0011-munged-cmdline.t           |  6 +++++
 t/0012-munge-cmdline.t            |  4 +++
 t/0013-unmunge-cmdline.t          |  4 +++
 t/0021-munged-valgrind.t          |  4 +++
 t/0022-munge-valgrind.t           |  4 +++
 t/0023-unmunge-valgrind.t         |  4 +++
 t/0100-munged-lock.t              |  8 +++++-
 t/0101-munged-security-socket.t   |  6 +++--
 t/0102-munged-security-keyfile.t  |  6 +++++
 t/0103-munged-security-logfile.t  |  6 +++++
 t/0104-munged-security-pidfile.t  |  6 +++++
 t/0105-munged-security-seedfile.t |  6 +++++
 t/0110-munged-origin-addr.t       |  6 +++++
 t/sharness.d/03-munged.sh         | 42 +++++++++++++++++++++++++++++--
 15 files changed, 111 insertions(+), 5 deletions(-)

diff --git a/t/0010-basic.t b/t/0010-basic.t
index 9294bab..1709c3f 100755
--- a/t/0010-basic.t
+++ b/t/0010-basic.t
@@ -84,4 +84,8 @@ test_expect_unstable 'check logfile for replay' '
     grep "Replayed credential" "${MUNGE_LOGFILE}"
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0011-munged-cmdline.t b/t/0011-munged-cmdline.t
index 95a1dc7..2566ce0 100755
--- a/t/0011-munged-cmdline.t
+++ b/t/0011-munged-cmdline.t
@@ -65,4 +65,10 @@ test_expect_failure 'finish writing tests' '
     false
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0012-munge-cmdline.t b/t/0012-munge-cmdline.t
index 53d542a..57394f2 100755
--- a/t/0012-munge-cmdline.t
+++ b/t/0012-munge-cmdline.t
@@ -623,4 +623,8 @@ test_expect_success 'stop munged' '
     munged_stop_daemon
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0013-unmunge-cmdline.t b/t/0013-unmunge-cmdline.t
index c034109..c532123 100755
--- a/t/0013-unmunge-cmdline.t
+++ b/t/0013-unmunge-cmdline.t
@@ -245,4 +245,8 @@ test_expect_success 'stop munged' '
     munged_stop_daemon
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0021-munged-valgrind.t b/t/0021-munged-valgrind.t
index fb0dc56..071be97 100755
--- a/t/0021-munged-valgrind.t
+++ b/t/0021-munged-valgrind.t
@@ -40,4 +40,8 @@ test_expect_success 'check valgrind log for errors in munged' '
     valgrind_check_log
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0022-munge-valgrind.t b/t/0022-munge-valgrind.t
index 9e62bdb..ed9a7d2 100755
--- a/t/0022-munge-valgrind.t
+++ b/t/0022-munge-valgrind.t
@@ -32,4 +32,8 @@ test_expect_success 'check valgrind log for errors in munge' '
     valgrind_check_log
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0023-unmunge-valgrind.t b/t/0023-unmunge-valgrind.t
index e54fbbd..6788ee4 100755
--- a/t/0023-unmunge-valgrind.t
+++ b/t/0023-unmunge-valgrind.t
@@ -36,4 +36,8 @@ test_expect_success 'check valgrind log for errors in unmunge' '
     valgrind_check_log
 '
 
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0100-munged-lock.t b/t/0100-munged-lock.t
index a1ab934..117e848 100755
--- a/t/0100-munged-lock.t
+++ b/t/0100-munged-lock.t
@@ -53,7 +53,7 @@ test_expect_success 'check lockfile permissions' '
 #   The lockfile should prevent this.
 ##
 test_expect_success 'start munged with in-use socket' '
-    test_must_fail munged_start_daemon &&
+    test_must_fail munged_start_daemon t-keep-process &&
     egrep "Error:.* Failed to lock \"${MUNGE_LOCKFILE}\"" "${MUNGE_LOGFILE}"
 '
 
@@ -201,4 +201,10 @@ test_expect_success SUDO 'stop unprivileged munged as root' '
     fi
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0101-munged-security-socket.t b/t/0101-munged-security-socket.t
index 560e4ac..532dc19 100755
--- a/t/0101-munged-security-socket.t
+++ b/t/0101-munged-security-socket.t
@@ -213,11 +213,13 @@ test_expect_success 'socket dir inaccessible by all override' '
             "${MUNGE_LOGFILE}"
 '
 
-# Cleanup detritus from testing.
+# Clean up detritus from testing.  This may include an errant munged process
+#   that has not terminated.
 ##
 test_expect_success 'cleanup' '
     rmdir "${MUNGE_SOCKETDIR}" &&
-    if rmdir "$(dirname "${MUNGE_SOCKETDIR}")" 2>/dev/null; then :; fi
+    if rmdir "$(dirname "${MUNGE_SOCKETDIR}")" 2>/dev/null; then :; fi &&
+    munged_cleanup
 '
 
 test_done
diff --git a/t/0102-munged-security-keyfile.t b/t/0102-munged-security-keyfile.t
index 25e7ce2..5cc1808 100755
--- a/t/0102-munged-security-keyfile.t
+++ b/t/0102-munged-security-keyfile.t
@@ -358,4 +358,10 @@ test_expect_success 'keyfile dir writable by other with sticky bit' '
     chmod 0755 "${MUNGE_KEYDIR}"
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0103-munged-security-logfile.t b/t/0103-munged-security-logfile.t
index c9887fd..fafd973 100755
--- a/t/0103-munged-security-logfile.t
+++ b/t/0103-munged-security-logfile.t
@@ -358,4 +358,10 @@ test_expect_success 'logfile failure writes single message to stderr' '
     test "${NUM}" -eq 1 2>/dev/null
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0104-munged-security-pidfile.t b/t/0104-munged-security-pidfile.t
index dbe5825..0c2a505 100755
--- a/t/0104-munged-security-pidfile.t
+++ b/t/0104-munged-security-pidfile.t
@@ -42,4 +42,10 @@ test_expect_failure 'finish writing tests' '
     false
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0105-munged-security-seedfile.t b/t/0105-munged-security-seedfile.t
index 31debc2..5008239 100755
--- a/t/0105-munged-security-seedfile.t
+++ b/t/0105-munged-security-seedfile.t
@@ -50,4 +50,10 @@ test_expect_failure 'finish writing tests' '
     false
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/0110-munged-origin-addr.t b/t/0110-munged-origin-addr.t
index 3b4d369..7d0589c 100755
--- a/t/0110-munged-origin-addr.t
+++ b/t/0110-munged-origin-addr.t
@@ -139,4 +139,10 @@ test_expect_success 'munged --origin non-interface IP address metadata' '
     egrep "^ENCODE_HOST:.* 192\.0\.0\.255\>" meta.$$
 '
 
+# Clean up after a munged process that may not have terminated.
+##
+test_expect_success 'cleanup' '
+    munged_cleanup
+'
+
 test_done
diff --git a/t/sharness.d/03-munged.sh b/t/sharness.d/03-munged.sh
index 0168326..6f6e975 100644
--- a/t/sharness.d/03-munged.sh
+++ b/t/sharness.d/03-munged.sh
@@ -73,19 +73,22 @@ munged_create_key()
 }
 
 ##
-# Start munged, removing an existing logfile (from a previous run) if present.
+# Start munged, removing an existing logfile or killing an errant munged
+#   process (from a previous run) if needed.
 # The following leading args are recognized:
 #   t-exec=ARG - use ARG to exec munged.
 #   t-keep-logfile - do not remove logfile before starting munged.
+#   t-keep-process - do not kill previous munged process.
 # Remaining args will be appended to the munged command-line.
 ##
 munged_start_daemon()
 {
-    local EXEC= KEEP_LOGFILE= &&
+    local EXEC= KEEP_LOGFILE= KEEP_PROCESS= &&
     while true; do
         case $1 in
             t-exec=*) EXEC=$(echo "$1" | sed 's/^[^=]*=//');;
             t-keep-logfile) KEEP_LOGFILE=1;;
+            t-keep-process) KEEP_PROCESS=1;;
             *) break;;
         esac
         shift
@@ -93,6 +96,9 @@ munged_start_daemon()
     if test "${KEEP_LOGFILE}" != 1; then
         rm -f "${MUNGE_LOGFILE}"
     fi &&
+    if test "${KEEP_PROCESS}" != 1; then
+        munged_kill_daemon
+    fi &&
     test_debug "echo ${EXEC} \"${MUNGED}\" \
             --socket=\"${MUNGE_SOCKET}\" \
             --key-file=\"${MUNGE_KEYFILE}\" \
@@ -136,3 +142,35 @@ munged_stop_daemon()
             --stop \
             "$@"
 }
+
+##
+# Kill an errant munged process running in the background from a previous test.
+# This situation is most likely to occur if a munged test is expected to fail
+#   and instead erroneously succeeds.
+# Only check for the pid named in ${MUNGE_PIDFILE} to avoid intefering with
+#   munged processes belonging to other tests or system use.  And check that
+#   the named pid is a munged process and not one recycled by the system for
+#   some other running process.
+# A SIGTERM is used here instead of "munged --stop" in case the latter has a
+#   bug introduced that prevents cleanup from occurring.
+# A SIGKILL would prevent the munged process from cleaning up which could cause
+#   other tests to inadvertently fail.
+##
+munged_kill_daemon()
+{
+    local PID
+    PID=$(cat "${MUNGE_PIDFILE}" 2>/dev/null)
+    if ps -p "${PID}" -ww 2>/dev/null | grep munged; then
+        kill "${PID}"
+        test_debug "echo \"Killed munged pid ${PID}\""
+    fi
+}
+
+##
+#  Perform any housekeeping to clean up after munged.  This should be called
+#    at the end of any test script that starts a munged process.
+##
+munged_cleanup()
+{
+    munged_kill_daemon
+}
-- 
2.30.0