Repository navigation
feat(health): VM/CT health view and ntfy/e-mail alerts (v2.13.0) - #38
Merged
Merged
Conversation
- New allowlisted config keys: HEALTH_{CPU,MEM,DISK}_{WARN,CRIT},
HEALTH_CPU_RUNS, HEALTH_IGNORE_IDS, HEALTH_STATE_DIR, NTFY_URL,
NTFY_TOKEN_FILE, HEALTH_MAIL_TO, HEALTH_MAIL_FROM; inline NTFY_TOKEN
is ignored and rejected in notification modes.
- _validate_health_config runs only in health/check/test-notify modes.
- New flags --health, --check, --dry-run, --test-notify with mode
resolution and invalid-combination checks; fatal setup errors exit 3
in --check mode.
- Extract _fmt_duration and _owner_mode_ok helpers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Read local guests from `pvesh get /cluster/resources` (30 s timeout), parse JSON on stdin with python3 into TSV; templates and guests on other nodes are skipped, names sanitized. - _health_checks is the single source for cpu/mem/disk/down levels; stopped guests with onboot=1 are CRIT. - --health (boxed), --health --list (plain) with findings and totals, --health --json with null for n/a metrics; --filter/--name apply. - Main menu key "h" opens the health overview; the status action adds a "Health:" line and stays silent without pvesh/python3. - Tests: pvesh mock with fixtures, onboot support in qm/pct mocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- --check evaluates all local guests plus recent node tasks and alerts only on changes: new/escalated, improved, RESOLVED with duration. - CPU needs HEALTH_CPU_RUNS consecutive runs; mem/disk alert at once; a metric that is n/a keeps its previous state; a running guest that stops without a successful stop/shutdown/destroy/migrate task is WARN (latched), onboot=1 and stopped is CRIT. - Failed tasks since the last run are one-shot events (WARNINGS -> WARN, otherwise CRIT); the first run is a baseline; watermark by endtime+UPID. - Versioned TSV state (0600 in a 0700 dir), regex-validated on load, never sourced, written atomically; flock against parallel runs; INT/TERM and unexpected aborts exit 3. - Nagios-style summary; exit 0/1/2/3. --dry-run prints the composed message and writes neither state nor lock nor notifications. - _notify_all is a stub until the ntfy/sendmail senders land; with a channel configured it reports failure so state is kept for a retry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- _notify_ntfy: curl with URL, Title/Priority/Tags and the optional Bearer token passed only via the stdin config; body from a private temp file; no redirects; only the host is logged. A token is never sent over plain http (channel fails instead). - _notify_mail: local sendmail (fallback /usr/sbin/sendmail) with validated single-line headers, UTF-8 body and Auto-Submitted. - _notify_all tries every configured channel and fails only when all of them fail, so --check keeps its state and retries next run. - --test-notify sends a real test message. - A running or successful vzdump task explains a stopped guest (backup stop mode). - Tests: curl/sendmail mocks, delivery, dedup, recovery, partial and total failure, token handling, header injection, vzdump. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Version header 2.13.0 with changelog line; CHANGELOG [2.13.0] with Added, Changed and Security. - README: features and usage rows, new "Health monitoring & alerts" section (checks, thresholds, exit codes, state, ntfy/mail setup, cron example, limitations), full config key list with chmod 600 advice, updated security note, requirements, test count and banner version. - Bash/Zsh completions: --name, --health, --check, --dry-run, --test-notify. - CI runs tests/run.sh after shfmt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Metric alerts resolve when a guest stops (CPU counter restarts); an
n/a metric on a running guest keeps its state without being reported.
- Unexpected-stop detection uses the most recent lifecycle task per
guest (no time window, OK or WARNINGS; suspend, reboot in progress and
stop-mode backups included); a stop task clears an existing latch.
onboot=1 guests stopped by a task are WARN instead of CRIT.
- Tasks are read with --source all so running tasks are visible.
- --force warning goes to stderr for --json/--health --json/--check.
- --check: INT/TERM and usage errors exit 3 from the very start.
- Total delivery failure saves the state but keeps the unsent changes
(and the task watermark) pending, so they fire once on the next run.
- Lock on fd 9, closed for pvesh/python3/qm/pct/curl/sendmail.
- onboot read from /etc/pve/{qemu-server,lxc}/ID.conf (current section)
with qm/pct fallback; node name from `uname -n` up to the first dot.
- Findings shorten the guest name, never the text; narrow terminals drop
UPTIME/DISK%/TYPE; ignored guests are not counted; an empty node is
not an error for --health.
- Regression tests for each item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- http:// NTFY_URL together with NTFY_TOKEN_FILE is a config error (--check exits 3, --test-notify exits 1). - Token file: directory must be owned by root/current user and not group/world-writable; the token is read through one opened descriptor that is verified via /proc/self/fd (regular file, owner, mode 0600). - Lock file opened for append (never truncated); the parent of HEALTH_STATE_DIR must not be group/world-writable. - qm/pct config fallback runs with timeout 10. - curl: -q first, --proto =https when a token is used, no --retry. - json_escape escapes remaining control characters as \u00XX. - --test-notify removes its temp body file on exit and on INT/TERM. - _truncate keeps UTF-8 characters intact. - Tests for each item where practical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24d5812a2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2027
to
+2030
| qmstop | qmshutdown | vzstop | vzshutdown | qmsuspend | vzsuspend | qmdestroy | vzdestroy | \ | ||
| qmigrate | vzmigrate | vzdump) | ||
| if ((lc_end[$k] == 0)) || [[ "${lc_status[$k]}" == "OK" || "${lc_status[$k]}" == WARNINGS* ]]; then | ||
| excused["$k"]=1 |
There was a problem hiding this comment.
When a VM has completed a normal per-guest vzdump and later crashes before another lifecycle task occurs, this branch still marks the VM as excused because the successful backup remains its latest lifecycle task. Consequently, even if the previous check recorded the VM as running, _check_process_states suppresses the unexpected-stop warning indefinitely; completed backups should only excuse a stop when correlated with that transition, while a currently running backup can remain an unconditional excuse.
Useful? React with 👍 / 👎.
| # path between validation and use cannot leak another file; its directory must be trusted. | ||
| _read_ntfy_token() { | ||
| local fd info line='' | ||
| if [[ -L "$NTFY_TOKEN_FILE" ]] || ! _trusted_dir "$(dirname -- "$NTFY_TOKEN_FILE")"; then |
There was a problem hiding this comment.
When the token's immediate directory is private but one of its ancestors is non-sticky and world-writable, another user can rename that directory between this check and the subsequent open, then replace it with a symlink to a root-owned directory. If the resulting same-named target is a root-owned 0600 file whose first line matches the token syntax, the descriptor checks accept it and transmit that secret as the bearer token. Checking only dirname therefore does not establish the stated no-swap guarantee; verify the complete path chain or open it through trusted directory descriptors with no-follow semantics.
Useful? React with 👍 / 👎.
_term_cols is always called as $(_term_cols), where stdout is a pipe, so tput was never consulted and the width was fixed at 80. Also check stderr for a terminal. Found while verifying --health on a real PVE 9.2 node at 60 columns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds per-guest health monitoring and opt-in alerting to
pman(v2.13.0).--health— health overview for all VMs/CTs on the local node: status, CPU %, RAM %, root disk %,uptime and an OK/WARN/CRIT level with findings. Boxed table in a TTY, plain with
--list, JSON with--json.--filter/--nameapply. Interactive: keyhin the main menu, extraHealth:line inthe status action.
--check— cron-friendly evaluation with Nagios exit codes (0 OK / 1 WARN / 2 CRIT / 3 UNKNOWN),flocklock, versioned state file (/var/lib/pman/health.state, 0600 in 0700 dir), alerts only onstate changes (new / escalated / improved / RESOLVED).
HEALTH_{CPU,MEM,DISK}_{WARN,CRIT},0disables; CPU needsHEALTH_CPU_RUNSconsecutive hits)onboot=1, or running → stopped without an explaining lifecycletask such as shutdown, suspend, migrate or stop-mode vzdump)
/nodes/<node>/tasks --source all), deduplicated via watermark + UPID,baseline on first run
passed via stdin config, HTTPS enforced with token, no redirects,
-q) and e-mail (sendmail -t -oi).Unsent transitions are retried on the next run.
--test-notifysends a test message;--dry-runprints the message without sending or writing state.
NTFY_TOKENis rejected); guest names / task texts are sanitizedfor TTY, JSON, ntfy and mail.
(also adds the previously missing
--name). CI now runstests/run.sh.Parsing uses
python3(ships with PVE) like the existing IP lookup; no new dependencies.Type of Change
tests/run.sh)Checklist
shellcheck proxmox-manager.shpasses locally with no warningstests/run.sh— all greenCHANGELOG.mdupdated (as release section[2.13.0] - 2026-09-19)usage()/--helpoutputtype(scope): summaryconventionReviews
Implemented in work packages, reviewed by a logic reviewer (12 findings, all fixed: sticky metric alerts
after stop, false "unexpected stop" due to pvestatd delay, tasks
--source all, JSON with--force,exit code on early SIGTERM, …) and a security reviewer (no must-fix; 8 hardening items applied: token-file
TOCTOU, lock file open mode, timeouts,
curl -q,json_escapecontrol chars, …).Testing
shellcheck -x proxmox-manager.sh tests/run.sh tests/mock-bin/*— cleanshfmt -d proxmox-manager.sh— cleanbash tests/run.sh— 131 passed, 0 failed (41 before), new mocks:pvesh,curl,sendmail,unameand fixtures under
tests/fixtures/Verified on a real node (PVE 9.2.20, kernel 7.0.14-15-pve, 12 guests; script copied to a temp dir, installed pman untouched):
--health/--list/--json: values match rawpveshdata (e.g. VM 100 mem 5.70/6.44 GB → 89 %), VM disknull, ~1 s runtime_term_colsbug, see 73f60ae)--check: baseline, exit codes 0/1/2, state dir 0700 / files 0600,--dry-runwrites nothing, ~2 s runtime--test-notify, WARN push (prio 4) → no duplicate on repeat run → RESOLVED push (prio 2)poweroff→ WARN "stopped unexpectedly" (latched)qmsnapshotfailure from the node's task history reported as CRIT (exit 2)Health:lineNot covered live: end-to-end mail delivery (node has no working relay),
onboot=1CRIT path (mock-tested only).Screenshots in
docs/still show v2.12.1 and need regeneration on a real host.🤖 Generated with Claude Code