Shield pct start and the template check from a closed console

.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
 

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.

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

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))