From f1283c2a6fe7375140eb957b16eae4526bd1705a Mon Sep 17 00:00:00 2001 From: MickLesk <47820557+MickLesk@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:36:01 +0100 Subject: [PATCH] Run the host checks' probes during load, and report nothing a check stopped The checks before the settings menu waited on pveversion, two pvesm status calls, pvesh and, on clusters, pvecm, one after the other. core/build.func now starts them in the background before it downloads the engine; the checks read the results through _cs_probe_read and run a command themselves only when its probe failed or never ran. The storage check trusts a probe's yes and asks a no again live. Telemetry starts with the install record the build sends. A run that ends before it, in the checks or a dialog, reports nothing: a host that is not ready is not a failed install, and these runs made up a third of the failures since 27 October. Tools and add-ons have no such point, so their "requirement not met" codes send nothing either. root_check now runs before pve_check. As another user /etc/pve/storage.cfg is unreadable, and the cluster check reported a damaged cluster. log_msg and log_section no longer end a run when the log cannot be written. --- .github/workflows/verify-split-files.yml | 5 +++ api/API.txt | 1 + api/telemetry.func | 26 ++++++++------ core/build.func | 33 +++++++++++++++++- core/core.func | 37 +++++++++++++++----- docs/api.md | 13 +++++-- pve/backend.func | 12 +++++-- pve/vm-core.func | 2 +- tests/host-probes.sh | 44 ++++++++++++++++++++++++ tests/telemetry-trace.sh | 20 ++++++++--- ui/API.txt | 3 ++ ui/menu.func | 10 ++++-- 12 files changed, 174 insertions(+), 32 deletions(-) create mode 100644 tests/host-probes.sh diff --git a/.github/workflows/verify-split-files.yml b/.github/workflows/verify-split-files.yml index 3fa8997..e37088a 100644 --- a/.github/workflows/verify-split-files.yml +++ b/.github/workflows/verify-split-files.yml @@ -37,6 +37,7 @@ on: - "tests/stale-silent-record.sh" - "tests/exit-code-hints.sh" - "tests/telemetry-trace.sh" + - "tests/host-probes.sh" - "tests/forge-release-json.sh" - "tests/install-cleanup.sh" - "tests/console-hangup.sh" @@ -73,6 +74,7 @@ on: - "tests/stale-silent-record.sh" - "tests/exit-code-hints.sh" - "tests/telemetry-trace.sh" + - "tests/host-probes.sh" - "tests/forge-release-json.sh" - "tests/install-cleanup.sh" - "tests/console-hangup.sh" @@ -146,6 +148,9 @@ jobs: - name: Telemetry trace tests run: bash tests/telemetry-trace.sh + - name: Host probe tests + run: bash tests/host-probes.sh + - name: Forge release lookup tests run: bash tests/forge-release-json.sh diff --git a/api/API.txt b/api/API.txt index d708af0..951e9eb 100644 --- a/api/API.txt +++ b/api/API.txt @@ -14,6 +14,7 @@ _tm_post_progress _tm_post_run _tm_post_update _tm_quiet +_tm_requirement_code _tm_send _tm_step_output build_error_string diff --git a/api/telemetry.func b/api/telemetry.func index d27ab91..7652c48 100644 --- a/api/telemetry.func +++ b/api/telemetry.func @@ -259,16 +259,10 @@ _tm_post_update() { ;; esac - # Unset: no install was reported yet. A host run that failed in its checks - # (storage, ID, template) reports one now; an update run inside a container - # or a cancelled dialog does not. "false" means that send failed. - if [[ -z "${POST_TO_API_DONE:-}" ]]; then - if [[ "$pb_status" == "failed" && -n "${NSAPP:-}" ]] && - ! { declare -f _is_container_context >/dev/null 2>&1 && _is_container_context; }; then - post_to_api - fi - [[ -z "${POST_TO_API_DONE:-}" ]] && return 0 - fi + # Unset: the run ended before its build reported the install, in the checks + # or a dialog. Those report nothing; a host that is not ready is no failed + # install. "false" means the install was due and its send failed. + [[ -z "${POST_TO_API_DONE:-}" ]] && return 0 if [[ "$pb_status" == "failed" || "$pb_status" == "unknown" ]]; then _tm_error_context "$exit_code" @@ -302,6 +296,17 @@ _tm_post_update() { return 0 } +# Tools and add-ons have no build step that starts their telemetry, so their +# checks are known by the code they exit with: wrong host or OS, nothing to work +# on, a prerequisite missing. Like the checks before a container build, those +# report nothing. +_tm_requirement_code() { + case "$1" in + 10 | 103 | 104 | 106 | 232 | 233 | 234 | 236 | 238) return 0 ;; + esac + return 1 +} + # ------------------------------------------------------------------------------ # _tm_post_run - tool and add-on runs # @@ -318,6 +323,7 @@ _tm_post_run() { uuid=$(cat /proc/sys/kernel/random/uuid 2>/dev/null || uuidgen 2>/dev/null || echo "${type}-$(date +%s)") [[ "$status" == "done" ]] && status="success" [[ "$exit_code" =~ ^[0-9]+$ ]] || exit_code=199 + [[ "$status" == "failed" ]] && _tm_requirement_code "$exit_code" && return 0 if [[ "$status" == "failed" ]]; then _tm_error_context "$exit_code" diff --git a/core/build.func b/core/build.func index bf748a4..b675f07 100644 --- a/core/build.func +++ b/core/build.func @@ -268,7 +268,7 @@ _cs_prefetch_engine() { [[ -d "$base" && -w "$base" ]] || continue # An EXIT trap would be the obvious cleanup, but core.func and api.func # install their own later and the last one wins. Sweep instead. - find "$base" -maxdepth 1 \( -name 'cs-engine.*' -o -name 'cs-pveam.*' -o -name 'cs-pveversion.*' \) -mmin +60 -exec rm -rf {} + 2>/dev/null || true + find "$base" -maxdepth 1 \( -name 'cs-engine.*' -o -name 'cs-pveam.*' -o -name 'cs-pveversion.*' -o -name 'cs-probe.*' \) -mmin +60 -exec rm -rf {} + 2>/dev/null || true dir="$(mktemp -d "${base}/cs-engine.XXXXXX" 2>/dev/null)" && break dir="" done @@ -305,6 +305,37 @@ _cs_prefetch_engine() { return 0 } +# The checks before the settings menu wait on pveversion, pvesm and pvesh, each +# a Perl start of up to a second. Start them now, while the engine downloads and +# the header loads; _cs_probe_read hands the checks what they printed. +_cs_probe() { + local dir="$1" name="$2" + shift 2 + : >"${dir}/${name}.out" + ("$@" >"${dir}/${name}.out" 2>/dev/null + echo "$?" >"${dir}/${name}.tmp" && mv -f "${dir}/${name}.tmp" "${dir}/${name}.rc") /dev/null 2>&1 || return 0 + [[ "$(id -u)" == "0" ]] || return 0 + local base dir="" + for base in /dev/shm "${TMPDIR:-/tmp}" /tmp; do + [[ -d "$base" && -w "$base" ]] || continue + dir="$(mktemp -d "${base}/cs-probe.XXXXXX" 2>/dev/null)" && break + dir="" + done + [[ -n "$dir" ]] || return 0 + _cs_probe "$dir" pveversion pveversion + _cs_probe "$dir" vztmpl pvesm status -content vztmpl + _cs_probe "$dir" rootdir pvesm status -content rootdir + _cs_probe "$dir" nextid pvesh get /cluster/nextid + [[ -f /etc/pve/corosync.conf ]] && _cs_probe "$dir" pvecm pvecm status + CS_PROBE_DIR="$dir" + return 0 +} + +_cs_start_host_probes _cs_prefetch_engine || true _cs_source_func "lxc/platform.func" diff --git a/core/core.func b/core/core.func index 0e18236..6459856 100644 --- a/core/core.func +++ b/core/core.func @@ -549,16 +549,34 @@ _cs_runtime_cache_dir() { return 1 } +# _cs_probe_read - output of a host probe that core/build.func started at +# load, once it has finished; fails when there is none or it failed, and the +# caller runs the command itself. Polls a file, so it works inside $( ) too. +_cs_probe_read() { + local dir="${CS_PROBE_DIR:-}" n=0 + [[ -n "$dir" && -f "${dir}/$1.out" ]] || return 1 + while [[ ! -f "${dir}/$1.rc" ]]; do + ((n++ < 1200)) || return 1 + sleep 0.05 + done + [[ "$(<"${dir}/$1.rc")" == "0" ]] || return 1 + cat "${dir}/$1.out" +} + +_cs_pveversion_out() { + _cs_probe_read pveversion || pveversion 2>/dev/null +} + _pve_version() { [[ -n "${PVEVERSION:-}" ]] && { printf '%s' "$PVEVERSION"; return 0; } local dir cache dir="$(_cs_runtime_cache_dir)" || { - pveversion 2>/dev/null | awk -F'/' '{print $2}' | awk -F'-' '{print $1}' - return + _cs_pveversion_out | awk -F'/' '{print $2}' | awk -F'-' '{print $1}' + return 0 } cache="${dir}/cs-pveversion.$$" [[ -s "$cache" ]] || - pveversion 2>/dev/null | awk -F'/' '{print $2}' | awk -F'-' '{print $1}' >"$cache" + _cs_pveversion_out | awk -F'/' '{print $2}' | awk -F'-' '{print $1}' >"$cache" cat "$cache" 2>/dev/null } @@ -735,7 +753,7 @@ pmxcfs_check() { exit 228 fi - if [[ -f /etc/pve/corosync.conf ]] && ! pvecm status 2>/dev/null | grep -qE '^Quorate:[[:space:]]+Yes'; then + if [[ -f /etc/pve/corosync.conf ]] && ! { _cs_probe_read pvecm || pvecm status 2>/dev/null; } | grep -qE '^Quorate:[[:space:]]+Yes'; then msg_error "This cluster node has no quorum, so /etc/pve is read-only and nothing can be created." msg_custom "🔧" "${YW}" "Bring enough cluster nodes online, then check: pvecm status" exit 210 @@ -899,8 +917,8 @@ log_msg() { local logfile logfile="$(get_active_logfile)" - [[ -z "$msg" ]] && return - [[ -z "$logfile" ]] && return + [[ -z "$msg" ]] && return 0 + [[ -z "$logfile" ]] && return 0 # Ensure log directory exists mkdir -p "$(dirname "$logfile")" 2>/dev/null || true @@ -908,7 +926,8 @@ log_msg() { # Strip ANSI codes and write with timestamp local clean_msg clean_msg=$(strip_ansi "$msg") - echo "[$(date '+%Y-%m-%d %H:%M:%S')] $clean_msg" >>"$logfile" + # A log that cannot be written (full disk, removed directory) must not end the run. + echo "[$(date '+%Y-%m-%d %H:%M:%S')] $clean_msg" >>"$logfile" 2>/dev/null || true } # ------------------------------------------------------------------------------ @@ -923,7 +942,7 @@ log_section() { local logfile logfile="$(get_active_logfile)" - [[ -z "$logfile" ]] && return + [[ -z "$logfile" ]] && return 0 mkdir -p "$(dirname "$logfile")" 2>/dev/null || true { @@ -931,7 +950,7 @@ log_section() { echo "================================================================================" echo "[$(date '+%Y-%m-%d %H:%M:%S')] $section" echo "================================================================================" - } >>"$logfile" + } >>"$logfile" 2>/dev/null || true } # ------------------------------------------------------------------------------ diff --git a/docs/api.md b/docs/api.md index c1fef0a..708559c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -15,9 +15,16 @@ terminal status: it writes `/root/.install-.failed` and an `.errinfo` capture, which the host picks up after `lxc-attach` returns. See [`core/error_handler.func`](../core/error_handler.func). -A host run that fails before its install record went out (storage, ID or -template checks) sends that record first, then the failure. Cancelled dialogs -and update runs inside a container send nothing. +Telemetry starts with the install record (`post_to_api`, `post_to_api_vm`), +which the build sends once the checks and dialogs are behind it. A run that +ends before that sends nothing: a host that is not ready is not a failed +install. Tools and add-ons have no such point, so a failure with a +"requirement not met" code (10, 103, 104, 106, 232, 233, 234, 236, 238) sends +nothing either. Update runs inside a container send nothing. + +The checks wait on `pveversion`, `pvesm status` and `pvesh`. `core/build.func` +starts these in the background before it downloads the engine, and the checks +read their output through `_cs_probe_read`. ## What a failure carries (payload version 3) diff --git a/pve/backend.func b/pve/backend.func index 390579b..804e81e 100644 --- a/pve/backend.func +++ b/pve/backend.func @@ -2245,8 +2245,16 @@ _offer_storage_content() { # content switched off, which can be switched on in place. # ------------------------------------------------------------------------------ storage_content_check() { - check_storage_support "vztmpl" || _offer_storage_content "vztmpl" || exit 120 - check_storage_support "rootdir" || _offer_storage_content "rootdir" || exit 119 + _probe_has_storage "vztmpl" || check_storage_support "vztmpl" || _offer_storage_content "vztmpl" || exit 120 + _probe_has_storage "rootdir" || check_storage_support "rootdir" || _offer_storage_content "rootdir" || exit 119 +} + +# The probe started at load answers yes; a no is asked again live, since +# _offer_storage_content may change it and nothing else reads the probe. +_probe_has_storage() { + local out + out="$(_cs_probe_read "$1")" || return 1 + awk 'NR > 1 && $1 != "" { found = 1 } END { exit !found }' <<<"$out" } select_storage() { diff --git a/pve/vm-core.func b/pve/vm-core.func index 011af99..670ecc0 100644 --- a/pve/vm-core.func +++ b/pve/vm-core.func @@ -787,7 +787,7 @@ cleanup() { if [[ -n "${TEMP_DIR:-}" && -d "$TEMP_DIR" ]]; then rm -rf "$TEMP_DIR" fi - # The final status nobody sent yet; post_update_to_api decides whether an early failure counts. + # The final status nobody sent yet; post_update_to_api drops it when the run never reached post_to_api_vm. if [[ "${POST_UPDATE_DONE:-}" != "true" ]] && declare -f post_update_to_api >/dev/null 2>&1; then if [[ $exit_code -ne 0 ]]; then post_update_to_api "failed" "$exit_code" diff --git a/tests/host-probes.sh b/tests/host-probes.sh new file mode 100644 index 0000000..5e43c56 --- /dev/null +++ b/tests/host-probes.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# The host checks read pveversion, pvesm and pvesh output that core/build.func +# collects in the background while the engine loads. A reader must wait for an +# unfinished probe, and fall back to the real command when a probe failed or +# never ran. +set -uo pipefail + +CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +eval "$(awk '/^_cs_probe[(][)] [{]/,/^}/' "$CORE_DIR/core/build.func")" +eval "$(awk '/^_cs_probe_read[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" +eval "$(awk '/^_probe_has_storage[(][)] [{]/,/^}/' "$CORE_DIR/pve/backend.func")" + +CS_PROBE_DIR="$(mktemp -d)" +trap 'rm -rf -- "$CS_PROBE_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 +} + +_cs_probe "$CS_PROBE_DIR" pveversion sh -c 'sleep 0.4; echo pve-manager/9.1.4/5ac30304265fbd8e' +_cs_probe "$CS_PROBE_DIR" broken sh -c 'echo partial; exit 2' +_cs_probe "$CS_PROBE_DIR" rootdir printf 'Name Type Status Total Used Available %%\nlocal-lvm lvmthin active 1 1 1 1%%\n' +_cs_probe "$CS_PROBE_DIR" vztmpl printf 'Name Type Status Total Used Available %%\n' + +expect "a reader waits for a probe still running" "pve-manager/9.1.4/5ac30304265fbd8e" "$(_cs_probe_read pveversion)" +expect "a failed probe gives nothing" "fail" "$(_cs_probe_read broken >/dev/null && echo ok || echo fail)" +expect "a probe never started gives nothing" "fail" "$(_cs_probe_read nextid >/dev/null && echo ok || echo fail)" +expect "a storage listed by the probe counts" yes "$(_probe_has_storage rootdir && echo yes || echo no)" +expect "a header without storages does not" no "$(_probe_has_storage vztmpl && echo yes || echo no)" + +saved="$CS_PROBE_DIR" +CS_PROBE_DIR="" +expect "without a probe dir the caller runs the command" "fail" "$(_cs_probe_read pveversion >/dev/null && echo ok || echo fail)" +CS_PROBE_DIR="$saved" + +printf '%s passed, %s failed\n' "$passed" "$failed" +((failed == 0)) diff --git a/tests/telemetry-trace.sh b/tests/telemetry-trace.sh index ff78b8b..4a0739a 100644 --- a/tests/telemetry-trace.sh +++ b/tests/telemetry-trace.sh @@ -117,7 +117,12 @@ unset POST_TO_API_DONE : >"$TEST_DIR/statuses" CS_ERROR_MSG="Not enough space on 'local'. Required: 500GB, Available: 23.2GB" post_update_to_api failed 214 -expect "an early host failure is reported with its install record" "installing failed" "$(paste -sd' ' "$TEST_DIR/statuses")" +expect "a failed check sends nothing" "" "$(paste -sd' ' "$TEST_DIR/statuses")" +reset +POST_TO_API_DONE=false +: >"$TEST_DIR/statuses" +post_update_to_api failed 214 +expect "a failure after an install record that did not go out still does" failed "$(paste -sd' ' "$TEST_DIR/statuses")" reset unset POST_TO_API_DONE : >"$TEST_DIR/statuses" @@ -133,11 +138,18 @@ expect "message with quotes and backslashes round-trips" 'quote " backslash \ ta # --- tools and add-ons carry the same fields --- reset -CS_ERROR_MSG="No LXC containers found" CS_STEP="Scanning containers" -post_tool_to_api demo-tool failed 234 +CS_ERROR_MSG="Backup of CT 105 failed" CS_STEP="Backing up containers" +post_tool_to_api demo-tool failed 235 expect "tool failures stay failures" failed "$(json_field "$SENT" status)" expect "tool type" pve "$(json_field "$SENT" type)" -expect "tool message" "No LXC containers found" "$(json_field "$SENT" error_message)" +expect "tool message" "Backup of CT 105 failed" "$(json_field "$SENT" error_message)" +reset +CS_ERROR_MSG="No LXC containers found" +post_tool_to_api demo-tool failed 234 +expect "a tool without anything to work on sends nothing" no "$([[ -f "$SENT" ]] && echo yes || echo no)" +reset +post_addon_to_api demo-addon failed 10 +expect "an add-on whose prerequisite is missing sends nothing" no "$([[ -f "$SENT" ]] && echo yes || echo no)" printf '%s passed, %s failed\n' "$passed" "$failed" ((failed == 0)) diff --git a/ui/API.txt b/ui/API.txt index 7e91610..356652e 100644 --- a/ui/API.txt +++ b/ui/API.txt @@ -10,6 +10,8 @@ _cs_host_arch _cs_host_version_line _cs_live_base _cs_os_family +_cs_probe_read +_cs_pveversion_out _cs_reattach_output _cs_ref_line _cs_run_os_section @@ -73,6 +75,7 @@ _tm_post_progress _tm_post_run _tm_post_update _tm_quiet +_tm_requirement_code _tm_send _tm_step_output _vars_file_value diff --git a/ui/menu.func b/ui/menu.func index d33eddd..8463747 100644 --- a/ui/menu.func +++ b/ui/menu.func @@ -189,8 +189,10 @@ install_script() { # pvesh spawns a Perl client and waits on pvedaemon — the slowest thing here # by far, and it needs nothing the checks below produce. Start it now and # collect it when it is actually needed, so it runs while they do. + # core/build.func usually started it at load already (_cs_start_host_probes). local _nextid_file="" _nextid_pid="" - if ! declare -f get_next_container_identifier &>/dev/null && command -v pvesh &>/dev/null; then + if ! declare -f get_next_container_identifier &>/dev/null && [[ ! -f "${CS_PROBE_DIR:-/nonexistent}/nextid.out" ]] && + command -v pvesh &>/dev/null; then _nextid_file="$(mktemp 2>/dev/null)" || _nextid_file="" if [[ -n "$_nextid_file" ]]; then pvesh get /cluster/nextid >"$_nextid_file" 2>/dev/null & @@ -198,9 +200,11 @@ install_script() { fi fi - pve_check + # Root first: for any other user /etc/pve/storage.cfg is unreadable, and the + # cluster check in pve_check took that for a damaged cluster. shell_check root_check + pve_check ensure_whiptail arch_check arm64_notice @@ -215,6 +219,8 @@ install_script() { if declare -f get_next_container_identifier &>/dev/null; then NEXTID="$(get_next_container_identifier)" + elif NEXTID="$(_cs_probe_read nextid | tr -d '[:space:]')" && [[ -n "$NEXTID" ]]; then + : elif [[ -n "$_nextid_pid" ]]; then wait "$_nextid_pid" 2>/dev/null || true NEXTID="$(tr -d '[:space:]' <"$_nextid_file" 2>/dev/null)"