From 1cfbf8b74b3d291feb561bc7895e16a1cf8a63e6 Mon Sep 17 00:00:00 2001 From: MickLesk <47820557+MickLesk@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:27:43 +0200 Subject: [PATCH] VM silent: return the failure instead of exiting `$STD apt-get update || true` could not be handled: silent() exited the run itself, so a host whose apt-get update fails (enterprise repo without a subscription) ended every VM script that installs libguestfs with 100 before the image download. The trap it re-armed also dropped the script's line and command, which is why VM failures never said where. The report moves into the trap handlers and only applies to the line the record came from. --- .github/workflows/verify-split-files.yml | 4 ++ pve/vm-core.func | 69 +++++++++++---------- tests/vm-silent.sh | 76 ++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 32 deletions(-) create mode 100644 tests/vm-silent.sh diff --git a/.github/workflows/verify-split-files.yml b/.github/workflows/verify-split-files.yml index 644866f..b7d4e9c 100644 --- a/.github/workflows/verify-split-files.yml +++ b/.github/workflows/verify-split-files.yml @@ -39,6 +39,7 @@ on: - "tests/incus-target.sh" - "tests/incus-storage-space.sh" - "tests/release-stamp-pending.sh" + - "tests/vm-silent.sh" - ".github/workflows/verify-split-files.yml" push: branches: [main] @@ -69,6 +70,7 @@ on: - "tests/incus-target.sh" - "tests/incus-storage-space.sh" - "tests/release-stamp-pending.sh" + - "tests/vm-silent.sh" - ".github/workflows/verify-split-files.yml" workflow_dispatch: @@ -139,6 +141,8 @@ jobs: run: bash tests/incus-storage-space.sh - name: Release stamp pending marker tests run: bash tests/release-stamp-pending.sh + - name: VM silent helper tests + run: bash tests/vm-silent.sh - name: Prefetch lists cover every engine file run: | diff --git a/pve/vm-core.func b/pve/vm-core.func index 0d97867..f7e479f 100644 --- a/pve/vm-core.func +++ b/pve/vm-core.func @@ -314,14 +314,17 @@ get_active_logfile() { # silent() # # - Executes command with output redirected to active log file -# - On error: displays last 20 lines of log and exits with original exit code -# - Temporarily disables error trap to capture exit code correctly -# - Sources explain_exit_code() for detailed error messages +# - On error: records the command, line and log, and returns its exit code; +# the ERR trap reports it. `$STD cmd || true` used to end the run instead. +# - Temporarily disables error trap to capture exit code correctly, then +# restores the script's own trap # ------------------------------------------------------------------------------ silent() { local cmd="$*" local caller_line="${BASH_LINENO[0]:-unknown}" local logfile="$(get_active_logfile)" + local err_trap + err_trap="$(trap -p ERR)" set +Eeuo pipefail trap - ERR @@ -330,40 +333,33 @@ silent() { local rc=$? set -Eeuo pipefail - trap 'error_handler' ERR + eval "${err_trap:-trap - ERR}" if [[ $rc -ne 0 ]]; then - # Source explain_exit_code if needed - if ! declare -f explain_exit_code >/dev/null 2>&1; then - if declare -f _cs_source_func >/dev/null 2>&1; then - _cs_source_func "core/error_handler.func" 2>/dev/null || true - else - # shellcheck disable=SC1090 - source <(curl -fsSL "${_CS_CURL_RETRY[@]}" "${COMMUNITY_SCRIPTS_CORE_URL:-https://raw.githubusercontent.com/community-scripts/core/main}/core/error_handler.func") 2>/dev/null || true - fi - fi - - local explanation="" - if declare -f explain_exit_code >/dev/null 2>&1; then - explanation="$(explain_exit_code "$rc")" - fi - - printf "\e[?25h" - if [[ -n "$explanation" ]]; then - msg_error "in line ${caller_line}: exit code ${rc} (${explanation})" - else - msg_error "in line ${caller_line}: exit code ${rc}" - fi - msg_custom "→" "${YWB}" "${cmd}" + export _SILENT_FAILED_RC="$rc" _SILENT_FAILED_CMD="$cmd" _SILENT_FAILED_LINE="$caller_line" _SILENT_FAILED_LOG="$logfile" + return "$rc" + fi + unset _SILENT_FAILED_RC _SILENT_FAILED_CMD _SILENT_FAILED_LINE _SILENT_FAILED_LOG +} - if [[ -s "$logfile" ]]; then - echo -e "\n${TAB}--- Last 20 lines of log ---" - tail -n 20 "$logfile" - echo -e "${TAB}----------------------------\n" - fi +# The record silent() left behind only counts for the line it came from: a +# failure the caller handled must not be reported for a later, unrelated one. +_vm_show_silent_failure() { + local explanation="" + if ! declare -f explain_exit_code >/dev/null 2>&1 && declare -f _cs_source_func >/dev/null 2>&1; then + _cs_source_func "api/exitcodes.func" 2>/dev/null || true + fi + declare -f explain_exit_code >/dev/null 2>&1 && explanation="$(explain_exit_code "$_SILENT_FAILED_RC")" - exit "$rc" + printf "\e[?25h" + msg_error "in line ${_SILENT_FAILED_LINE}: exit code ${_SILENT_FAILED_RC}${explanation:+ (${explanation})}" + msg_custom "→" "${YWB:-}" "${_SILENT_FAILED_CMD}" + if [[ -s "${_SILENT_FAILED_LOG:-}" ]]; then + echo -e "\n${TAB}--- Last 20 lines of log ---" + tail -n 20 "$_SILENT_FAILED_LOG" + echo -e "${TAB}----------------------------\n" fi + unset _SILENT_FAILED_RC _SILENT_FAILED_CMD _SILENT_FAILED_LINE _SILENT_FAILED_LOG } # ------------------------------------------------------------------------------ @@ -679,6 +675,11 @@ error_handler() { local line_number="${1:-unknown}" local command="${2:-unknown}" + if [[ -n "${_SILENT_FAILED_RC:-}" && "${_SILENT_FAILED_LINE:-}" == "$line_number" ]]; then + command="$_SILENT_FAILED_CMD" + _vm_show_silent_failure + fi + if declare -f post_update_to_api >/dev/null 2>&1; then post_update_to_api "failed" "$exit_code" fi @@ -2310,6 +2311,10 @@ vm_mark_created() { vm_post_create_error() { local rc=$? line="${1:-unknown}" cmd="${2:-unknown}" + if [[ -n "${_SILENT_FAILED_RC:-}" && "${_SILENT_FAILED_LINE:-}" == "$line" ]]; then + cmd="$_SILENT_FAILED_CMD" + _vm_show_silent_failure + fi msg_warn "A step after VM creation failed (line ${line}, exit ${rc}): ${cmd}" msg_warn "VM ${VMID:-?} is left in place -- finish the remaining configuration by hand." declare -f post_update_to_api >/dev/null 2>&1 && post_update_to_api "failed" "$rc" diff --git a/tests/vm-silent.sh b/tests/vm-silent.sh new file mode 100644 index 0000000..6620293 --- /dev/null +++ b/tests/vm-silent.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# The VM engine's silent() hands a failure back to the caller. It used to exit +# the run itself, so `$STD cmd || true` ended every VM script whose host could +# not run apt-get update, and the trap it re-armed had lost the script's line. +set -uo pipefail + +COMMUNITY_SCRIPTS_CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +export COMMUNITY_SCRIPTS_CORE_DIR +pveversion() { :; } +# shellcheck disable=SC1091 +source "${COMMUNITY_SCRIPTS_CORE_DIR}/pve/vm-core.func" +set +e -uo pipefail +trap - ERR + +TEST_DIR="$(mktemp -d)" +trap 'rm -rf -- "$TEST_DIR"' EXIT +BUILD_LOG="$TEST_DIR/build.log" +CL="" TAB=" " YWB="" RD="" YW="" +VMID="" +passed=0 failed=0 + +msg_error() { echo "ERROR: $*"; } +msg_custom() { echo "CMD: $3"; } +msg_warn() { echo "WARN: $*"; } +post_update_to_api() { echo "POST $1 $2"; } +cleanup_vmid() { :; } +explain_exit_code() { echo "explained $1"; } + +expect() { + if [[ "$2" == "$3" ]]; then + echo "PASS $1" + passed=$((passed + 1)) + else + echo "FAIL $1: expected <$2>, got <$3>" >&2 + failed=$((failed + 1)) + fi +} + +# A failure the script handles stays handled. +out="$( (set -e; trap 'echo TRAPPED; exit 9' ERR; silent sh -c 'echo boom >&2; exit 100' || true; echo "carried on") 2>&1 )" +expect "a handled failure does not end the run" 1 "$(grep -c 'carried on' <<<"$out")" +expect "the ERR trap stays quiet for it" 0 "$(grep -c TRAPPED <<<"$out")" +expect "nothing is reported for it" 0 "$(grep -c 'ERROR:' <<<"$out")" + +# An unhandled one reaches the script's own trap with the command's code. +: >"$BUILD_LOG" +out="$( (set -e; trap 'error_handler $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'echo boom >&2; exit 100'; echo "never") 2>&1 )" +rc=$? +expect "the run ends with the command's code" 100 "$rc" +expect "the handler posts that code" 1 "$(grep -c 'POST failed 100' <<<"$out")" +expect "the command is the real one, not the wrapper" 1 "$(grep -c '^CMD: sh -c echo boom' <<<"$out")" +expect "the explanation is shown" 1 "$(grep -c 'exit code 100 (explained 100)' <<<"$out")" +expect "the log tail is shown" 1 "$(grep -c '^boom$' <<<"$out")" +expect "nothing runs after it" 0 "$(grep -c never <<<"$out")" + +# The script's trap survives the call. +out="$( (trap 'echo MINE' ERR; silent true; trap -p ERR) )" +expect "the script's ERR trap is kept" 1 "$(grep -c MINE <<<"$out")" + +# A handled failure is not pinned on the next, unrelated one. +out="$( (set -e; trap 'error_handler $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'exit 100' || true + false; echo "never") 2>&1 )" +rc=$? +expect "the later failure keeps its own code" 1 "$rc" +expect "the handled command is not blamed" 0 "$(grep -c 'CMD:' <<<"$out")" +expect "the later command is named" 1 "$(grep -c 'while executing command .*false' <<<"$out")" + +# The post-creation trap reports the same way. +out="$( (set -e; VM_CREATED=1; trap 'vm_post_create_error $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'echo late >&2; exit 3') 2>&1 )" +rc=$? +expect "a failure after creation keeps the code" 3 "$rc" +expect "and names the real command" 1 "$(grep -c '^CMD: sh -c echo late' <<<"$out")" +expect "and leaves the VM in place" 1 "$(grep -c 'left in place' <<<"$out")" + +echo "$passed passed, $failed failed" +((failed == 0))