diff --git a/.github/workflows/verify-split-files.yml b/.github/workflows/verify-split-files.yml index 85a3153..bd55c94 100644 --- a/.github/workflows/verify-split-files.yml +++ b/.github/workflows/verify-split-files.yml @@ -33,6 +33,7 @@ on: - "tests/sudo-env.sh" - "tests/deb822-local-keyring.sh" - "tests/gpu-pci-class.sh" + - "tests/hangup-host-commands.sh" - "tests/nvidia-version-pin.sh" - "tests/external-installer.sh" - "tests/forge-version-key.sh" @@ -94,6 +95,7 @@ on: - "tests/sudo-env.sh" - "tests/deb822-local-keyring.sh" - "tests/gpu-pci-class.sh" + - "tests/hangup-host-commands.sh" - "tests/nvidia-version-pin.sh" - "tests/external-installer.sh" - "tests/forge-version-key.sh" @@ -185,6 +187,9 @@ jobs: - name: GPU detection by PCI class tests run: bash tests/gpu-pci-class.sh + - name: Hangup during host commands tests + run: bash tests/hangup-host-commands.sh + - name: NVIDIA userspace version pin tests run: bash tests/nvidia-version-pin.sh diff --git a/core/core.func b/core/core.func index 05312ce..2e07fe8 100644 --- a/core/core.func +++ b/core/core.func @@ -1013,6 +1013,13 @@ _cs_reattach_output() { # subshell sets the signal to ignore, which exec carries into the program, so # it finishes even though the terminal is gone. _cs_shield_hangup() { + # Inside $(...) the engine's trap is already reset, so that subshell died of the + # hangup and returned 129 for a command that finished (update-apps: "Backup failed"). + local _sig="" + if [[ "$BASHPID" != "$$" && -r "/proc/$BASHPID/status" ]]; then + while read -r _sig; do [[ "$_sig" == SigCgt:* ]] && break; done <"/proc/$BASHPID/status" + ((16#${_sig##*[[:space:]]} & 1)) || trap '' HUP + fi ( trap '' HUP # apt restores the terminal it was given and exits 100 when it is gone. diff --git a/pve/backend.func b/pve/backend.func index da4bd73..ffbee09 100644 --- a/pve/backend.func +++ b/pve/backend.func @@ -177,8 +177,8 @@ _pct_start_failed() { # also return 0 and leave the container stopped, so ask for the real state. _pct_restart() { local ctid="$1" what="$2" err - err="$(pct start "$ctid" 2>&1)" || true - [[ "$(pct status "$ctid" 2>/dev/null)" == "status: running" ]] && return 0 + err="$(_cs_shield_hangup pct start "$ctid" 2>&1)" || true + [[ "$(_cs_shield_hangup pct status "$ctid" 2>/dev/null)" == "status: running" ]] && return 0 msg_error "Container ${ctid} did not start after ${what}" _pct_start_failed "$ctid" "$err" return 1 @@ -268,6 +268,11 @@ _pve_wait_for_detached_install() { done } +# A closed console killed this pipeline, and a downloaded template read as missing. +_template_in_storage() { + pveam list "$TEMPLATE_STORAGE" 2>/dev/null | grep -qF "$1" +} + # Where a template lands in $TEMPLATE_STORAGE, or the default dir layout when # pvesm cannot say. _template_path() { @@ -1080,7 +1085,8 @@ EOF # the container simply would not start. # pct can also fail and leave it running ("CT 104 already running"). local _start_out - if ! _start_out="$(pct start "$CTID" 2>&1)" && [[ "$(pct status "$CTID" 2>/dev/null)" != "status: running" ]]; then + # A closed console killed pct start halfway, and a container that came up was reported as one that would not. + if ! _start_out="$(_cs_shield_hangup pct start "$CTID" 2>&1)" && [[ "$(_cs_shield_hangup pct status "$CTID" 2>/dev/null)" != "status: running" ]]; then msg_error "Container ${CTID} was created but would not start" _pct_start_failed "$CTID" "$_start_out" exit 227 @@ -3586,7 +3592,7 @@ create_lxc_container() { printf '%s\n' "$_dl_out" >>"${BUILD_LOG:-/dev/null}" # pveam can report a failed download and still exit 0, so only the # template in storage counts as success. - if pveam list "$TEMPLATE_STORAGE" 2>/dev/null | grep -qF "$TEMPLATE"; then + if _cs_shield_hangup _template_in_storage "$TEMPLATE"; then msg_ok "Template download successful." break fi @@ -3608,7 +3614,7 @@ create_lxc_container() { done fi - if ! pveam list "$TEMPLATE_STORAGE" 2>/dev/null | grep -q "$TEMPLATE"; then + if ! _cs_shield_hangup _template_in_storage "$TEMPLATE"; then msg_error "Template $TEMPLATE not available in storage $TEMPLATE_STORAGE after download." exit 223 fi diff --git a/tests/hangup-host-commands.sh b/tests/hangup-host-commands.sh new file mode 100644 index 0000000..b8f53d1 --- /dev/null +++ b/tests/hangup-host-commands.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +# A closed console sends SIGHUP to the whole foreground group. Since the run +# survives that, an unshielded pct start or template check failed instead: +# 10 runs in three days reported "would not start" for a container that came +# up, 5 "not available after download" for a template on disk. +set -uo pipefail + +CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +BACKEND="$CORE_DIR/pve/backend.func" +TEST_DIR="$(mktemp -d)" +trap 'rm -rf -- "$TEST_DIR"' EXIT +passed=0 failed=0 + +expect() { + if [[ "$2" == "$3" ]]; then + echo "PASS $1" + passed=$((passed + 1)) + else + echo "FAIL $1: expected <$2>, got <$3>" >&2 + failed=$((failed + 1)) + fi +} + +if ! command -v setsid >/dev/null || [[ ! -r /proc/self/stat ]]; then + echo "SKIP needs setsid and /proc" + exit 0 +fi + +# Each stub hangs up its whole process group, as a closing console does, then does its work. +mkdir -p "$TEST_DIR/bin" +cat >"$TEST_DIR/bin/pct" <<'EOF' +#!/usr/bin/env bash +case "$1" in +start) + kill -HUP -- "-$(awk '{print $5}' /proc/$$/stat)" + sleep 0.3 + : >"$MARK/started" + echo "Started pve-container@$2.service" + ;; +status) [[ -f "$MARK/started" ]] && echo "status: running" || echo "status: stopped" ;; +esac +EOF +cat >"$TEST_DIR/bin/pveam" <<'EOF' +#!/usr/bin/env bash +kill -HUP -- "-$(awk '{print $5}' /proc/$$/stat)" +sleep 0.3 +echo "local:vztmpl/debian-13-standard_13.6-1_amd64.tar.zst 129.66MB" +EOF +cat >"$TEST_DIR/bin/vzdump" <<'EOF' +#!/usr/bin/env bash +kill -HUP -- "-$(awk '{print $5}' /proc/$$/stat)" +sleep 0.3 +echo "INFO: notified via target mail-to-root" +EOF +chmod +x "$TEST_DIR/bin/pct" "$TEST_DIR/bin/pveam" "$TEST_DIR/bin/vzdump" + +{ + awk '/^_cs_shield_hangup[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func" + awk '/^(_template_in_storage|_pct_restart)[(][)] [{]/,/^}/' "$BACKEND" +} >"$TEST_DIR/funcs.sh" + +# In its own session, with the HUP trap the engine has, the way a console runs it. +run() { + rm -f "$TEST_DIR/started" + MARK="$TEST_DIR" PATH="$TEST_DIR/bin:$PATH" setsid -w bash -c ' + source "$1" + trap ":" HUP + msg_error() { :; } + _pct_start_failed() { :; } + TEMPLATE_STORAGE=local + eval "$2"' _ "$TEST_DIR/funcs.sh" "$1" 2>/dev/null +} +started() { [[ -f "$TEST_DIR/started" ]] && echo started || echo aborted; } + +run 'pct start 101 >/dev/null 2>&1' +expect "the stub's hangup kills an unshielded pct start (the old code)" aborted "$(started)" + +out="$(run 'out="$(_cs_shield_hangup pct start 101 2>&1)"; echo "$out"')" +expect "a shielded pct start finishes" started "$(started)" +expect "and its output arrives in full" "Started pve-container@101.service" "$out" + +# update-apps runs vzdump this way: the backup finished, yet the subshell returned 129. +expect "a shielded command inside \$(...) keeps its own exit status" "0 INFO: notified via target mail-to-root" "$(run 'out="$(_cs_shield_hangup vzdump 101 2>&1)"; echo "$? $out"')" +expect "called directly, it leaves the engine's own HUP trap alone" 1 "$(run '_cs_shield_hangup true; trap -p HUP | grep -c "trap -- '"':'"'"')" + +expect "_pct_restart sees the container running" 0 "$(run '_pct_restart 101 "the config change"; echo $?')" + +expect "the old template check loses a template on disk" 1 \ + "$(run 'pveam list local 2>/dev/null | grep -q debian-13-standard_13.6-1; echo $(($? != 0))')" +expect "the shielded check finds it" 0 \ + "$(run '_cs_shield_hangup _template_in_storage debian-13-standard_13.6-1_amd64.tar.zst; echo $?')" + +expect "the start step uses the shield" 1 "$(grep -c '_start_out="$(_cs_shield_hangup pct start "$CTID" 2>&1)"' "$BACKEND")" +expect "both template checks use it" 2 "$(grep -c '_cs_shield_hangup _template_in_storage "$TEMPLATE"' "$BACKEND")" +expect "the pveam list | grep pipeline lives only in the helper" 1 "$(grep -c 'pveam list "$TEMPLATE_STORAGE" 2>/dev/null | grep -q' "$BACKEND" | tr -d ' ')" + +printf '%s passed, %s failed\n' "$passed" "$failed" +((failed == 0))