diff --git a/.github/workflows/verify-split-files.yml b/.github/workflows/verify-split-files.yml index d5bb1eb..30a98a5 100644 --- a/.github/workflows/verify-split-files.yml +++ b/.github/workflows/verify-split-files.yml @@ -40,6 +40,7 @@ on: - "tests/incus-storage-space.sh" - "tests/release-stamp-pending.sh" - "tests/pct-start-reason.sh" + - "tests/vm-silent.sh" - ".github/workflows/verify-split-files.yml" push: branches: [main] @@ -71,6 +72,7 @@ on: - "tests/incus-storage-space.sh" - "tests/release-stamp-pending.sh" - "tests/pct-start-reason.sh" + - "tests/vm-silent.sh" - ".github/workflows/verify-split-files.yml" workflow_dispatch: @@ -143,6 +145,8 @@ jobs: run: bash tests/release-stamp-pending.sh - name: pct start reason tests run: bash tests/pct-start-reason.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))