diff --git a/CLAUDE.md b/CLAUDE.md index 7a090b8..62951fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,7 +109,7 @@ Agents must never execute destructive actions (restarts, deploys, config changes ## Event System -Events are append-only JSON lines at `/opt/homelab/events/YYYY-MM-DD//events.jsonl`. +Events are append-only, one JSON file per event, flat under `/opt/homelab/events//evt---[-].json`. Emit via `scripts/lib/events.sh` (shell) or `scripts/lib/events.py` (Python). diff --git a/scripts/monitor/health-monitor.sh b/scripts/monitor/health-monitor.sh index 684c5f4..0027f0f 100755 --- a/scripts/monitor/health-monitor.sh +++ b/scripts/monitor/health-monitor.sh @@ -1,17 +1,17 @@ #!/usr/bin/env bash -# health-monitor.sh - Homelab node health monitor and safe disk cleanup +# health-monitor.sh - Homelab node health monitor # # Designed to run standalone on the host (cron or direct) or to be called by -# the node-agent Python daemon. All cleanup decisions follow the conservative -# policy agreed in the design review: +# the node-agent Python daemon. # -# lte_node (chelsty-infra, chelsty-ha) : NO cleanup at all -# sd_card (piha, saturn) : dangling images + stopped containers, -# rate-limited to once per 24 h -# ai_node (solaria) : dangling images + stopped containers -# + build cache (NEVER -a) -# standard (vps) : dangling images + stopped containers -# + build cache +# Docker cleanup (image/container/build-cache prune) does NOT live here. +# node_agent.py (R1) is the sole owner of that cleanup, with a filtered +# container prune that respects restart policy and compose ownership — +# see kb/incidents/2026-07-30-ollama-solaria-vanish.md. This script used to +# carry its own unfiltered `docker container prune -f` (never wired into +# cron/systemd on any node, verified 2026-08-06); it was removed rather than +# backporting the R1 filter here too, to avoid two independent copies of +# cleanup logic drifting apart. # # VPS additionally rotates control-plane filesystem artefacts: # actions/completed + failed > 7 days @@ -43,15 +43,6 @@ DISK_CRIT_PCT=85 MEM_WARN_PCT=85 MEM_CRIT_PCT=95 -# Rate-limit file for SD-card nodes (max one Docker cleanup per 24 h) -CLEANUP_LOCK="${STATE_DIR}/last-docker-cleanup" -CLEANUP_INTERVAL=86400 # seconds - -# Node classifications -LTE_NODES="chelsty-infra chelsty-ha" -SD_CARD_NODES="piha saturn" -AI_NODES="solaria" - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -60,20 +51,6 @@ log() { echo "$(date -u +%H:%M:%S) [INFO] $*"; } warn() { echo "$(date -u +%H:%M:%S) [WARN] $*" >&2; } err() { echo "$(date -u +%H:%M:%S) [ERROR] $*" >&2; } -contains() { - local word="$1"; shift - for w in "$@"; do [[ "$w" == "$word" ]] && return 0; done - return 1 -} - -get_node_type() { - # shellcheck disable=SC2086 - if contains "$NODE_NAME" $LTE_NODES; then echo "lte_node"; return; fi - if contains "$NODE_NAME" $SD_CARD_NODES; then echo "sd_card"; return; fi - if contains "$NODE_NAME" $AI_NODES; then echo "ai_node"; return; fi - echo "standard" -} - # --------------------------------------------------------------------------- # Event emission # --------------------------------------------------------------------------- @@ -195,69 +172,6 @@ check_containers() { --format "{{.Names}}" 2>/dev/null || true) } -# --------------------------------------------------------------------------- -# Safe Docker cleanup (per policy) -# --------------------------------------------------------------------------- - -_sd_card_rate_ok() { - if [[ -f "${CLEANUP_LOCK}" ]]; then - local last_ts elapsed - last_ts=$(cat "${CLEANUP_LOCK}" 2>/dev/null || echo 0) - elapsed=$(( TIMESTAMP - last_ts )) - if [[ "${elapsed}" -lt "${CLEANUP_INTERVAL}" ]]; then - log "Docker cleanup skipped: last run ${elapsed}s ago (limit ${CLEANUP_INTERVAL}s)" - return 1 - fi - fi - return 0 -} - -_mark_cleanup_done() { - echo "${TIMESTAMP}" > "${CLEANUP_LOCK}" -} - -run_safe_cleanup() { - command -v docker &>/dev/null || return - local node_type - node_type=$(get_node_type) - - case "${node_type}" in - lte_node) - # NO cleanup on LTE nodes. Any docker operation risks triggering - # a pull over a metered/intermittent connection. - log "Skipping Docker cleanup: LTE node (${NODE_NAME})" - ;; - - sd_card) - # Dangling images + stopped containers only. - # Rate-limited to once per 24 hours to protect SD card write endurance. - _sd_card_rate_ok || return - log "Running rate-limited Docker cleanup (SD card node)" - docker image prune -f >/dev/null 2>&1 || true - docker container prune -f >/dev/null 2>&1 || true - _mark_cleanup_done - ;; - - ai_node) - # Dangling images + stopped containers + build cache. - # NEVER docker image prune -a (would remove Ollama runtime images, - # requiring a multi-hour re-pull of model weights). - log "Running AI-node Docker cleanup (dangling images + containers + build cache)" - docker image prune -f >/dev/null 2>&1 || true - docker container prune -f >/dev/null 2>&1 || true - docker builder prune -f >/dev/null 2>&1 || true - ;; - - standard) - # VPS and other standard nodes: full safe cleanup. - log "Running standard Docker cleanup" - docker image prune -f >/dev/null 2>&1 || true - docker container prune -f >/dev/null 2>&1 || true - docker builder prune -f >/dev/null 2>&1 || true - ;; - esac -} - # --------------------------------------------------------------------------- # VPS-specific: control-plane filesystem rotation # --------------------------------------------------------------------------- @@ -315,15 +229,13 @@ except Exception: mkdir -p "${EVENTS_DIR}/${NODE_NAME}" "${STATE_DIR}" -log "Health check starting on ${NODE_NAME} (type=$(get_node_type))" +log "Health check starting on ${NODE_NAME}" disk_pct=$(check_disk || echo 0) mem_pct=$(check_memory || echo 0) cpu_pct=$(check_cpu || echo 0) check_containers -run_safe_cleanup - # VPS: also rotate control-plane filesystem artefacts if [[ "${NODE_NAME}" == "vps" ]]; then cleanup_control_plane_fs diff --git a/scripts/observer/observer.py b/scripts/observer/observer.py index 4d349d4..967b26c 100644 --- a/scripts/observer/observer.py +++ b/scripts/observer/observer.py @@ -141,6 +141,11 @@ FAILED_EVENTS_DIR = STATE_DIR / "observer_failed_events" # "operator drops a file, the owning process consumes it" pattern the actions # pending/approved queue already uses. RESOLVE_REQUESTS_DIR = WORLD_DIR / "resolve-requests" +# mkdir(mode=...) is masked by the process umask, so the SSH operator (group +# aerbot) cannot drop a flag file into a dir created at the default 0o755 — +# same defect/fix as executor.py's INBOX_DIR_MODE (2026-08-06): an explicit, +# idempotent os.chmod after mkdir, applied on the same path the dir is used. +RESOLVE_REQUESTS_DIR_MODE = 0o775 # Time-based fallback for incidents that can never receive the service_healthy # event _resolve_incident() waits for (service removed/renamed/decommissioned @@ -277,6 +282,13 @@ class Observer: LOGS_DIR.mkdir(parents=True, exist_ok=True) FAILED_EVENTS_DIR.mkdir(parents=True, exist_ok=True) RESOLVE_REQUESTS_DIR.mkdir(parents=True, exist_ok=True) + try: + os.chmod(RESOLVE_REQUESTS_DIR, RESOLVE_REQUESTS_DIR_MODE) + except OSError as e: + logger.warning( + f"Could not set mode {oct(RESOLVE_REQUESTS_DIR_MODE)} on " + f"{RESOLVE_REQUESTS_DIR}: {e}" + ) def _quarantine_event_file(self, file_path: str, node_dir: str, exc: Exception) -> None: """Move an unreadable/unprocessable event out of the hot path.""" diff --git a/services/control-plane/src/supervisor.py b/services/control-plane/src/supervisor.py index b97aa77..8eb0a69 100644 --- a/services/control-plane/src/supervisor.py +++ b/services/control-plane/src/supervisor.py @@ -452,13 +452,15 @@ class Supervisor: # Choose action type first so we can build the ID. # - # container_restart IDs carry a suffix so two DIFFERENT incidents for - # the same node+service never collide in cancelled/completed/failed - # (2026-08-26: a generic containers_not_running restart and a later - # ha-diag-agent shadow-mode restart both used the bare + # Both container_restart and redeploy IDs carry a suffix so two + # DIFFERENT incidents for the same node+service never collide in + # cancelled/completed/failed (2026-08-26: a generic + # containers_not_running restart and a later ha-diag-agent + # shadow-mode restart both used the bare # container-restart-piha-homeassistant id and overwrote each other's # history — worked around manually that session, see - # docs/sessions/2026-08-26.md). + # docs/sessions/2026-08-26.md). redeploy carries the same latent risk + # via the same code path and got the same fix on 2026-08-27. # # The suffix is the triggering incident's started_at, NOT time.time() # at generation time: reconcile() calls _generate_recommendation on @@ -482,20 +484,22 @@ class Supervisor: # bare id has no incident to distinguish "same" from "different" # occurrences by, but it is at least stable across calls, which is # what idempotency here actually requires. - if trigger_type in CONTAINER_RESTART_TRIGGERS: - incident_id = self.actual_state["services"].get(drift["svc_key"], {}).get("incident_id") - incident = self.actual_state["incidents"].get(incident_id, {}) if incident_id else {} - started_ts = int(_parse_ts(incident.get("started_at"))) - if started_ts: - action_id = f"container-restart-{node}-{service}-{started_ts}" - else: - action_id = f"container-restart-{node}-{service}" + # + # Safe to add the suffix to redeploy ids: verified no code anywhere + # else reconstructs `redeploy-{node}-{service}` for an exact-match + # lookup. _cancel_resolved_pending_actions matches on the node/service + # *fields* inside each pending file, not on the id string; executor.py, + # node_agent.py and deploy-runner.sh all treat action_id as an opaque + # string read back from the action's own JSON/marker file, never + # reconstructed from node+service. + action_prefix = "container-restart" if trigger_type in CONTAINER_RESTART_TRIGGERS else "redeploy" + incident_id = self.actual_state["services"].get(drift["svc_key"], {}).get("incident_id") + incident = self.actual_state["incidents"].get(incident_id, {}) if incident_id else {} + started_ts = int(_parse_ts(incident.get("started_at"))) + if started_ts: + action_id = f"{action_prefix}-{node}-{service}-{started_ts}" else: - # redeploy IDs stay bare (node-service) — out of scope for this - # fix (see commit message: no observed collision here yet), and - # _cancel_resolved_pending_actions/_ha_action_recently_completed - # do not key off redeploy ids so nothing here depends on it. - action_id = f"redeploy-{node}-{service}" + action_id = f"{action_prefix}-{node}-{service}" # Skip if an action for this ID is already live in any active state # (pending → approved → running). This prevents re-creation after diff --git a/services/control-plane/tests/test_incident_lifecycle.py b/services/control-plane/tests/test_incident_lifecycle.py index f740648..4f44e65 100644 --- a/services/control-plane/tests/test_incident_lifecycle.py +++ b/services/control-plane/tests/test_incident_lifecycle.py @@ -2,6 +2,7 @@ from __future__ import annotations import json +import os import sys import time from pathlib import Path @@ -915,3 +916,19 @@ def test_resolve_request_flag_for_already_resolved_incident_is_removed(tmp_path) assert obs.world_state["incidents"][inc_id]["status"] == "resolved" assert obs.world_state["incidents"][inc_id]["resolved_reason"] == "manual_operator" assert not flag.exists() + + +def test_resolve_requests_dir_is_group_writable(tmp_path, monkeypatch): + """world/resolve-requests/ must be group-writable so an SSH operator + (group aerbot, not the observer's own user) can drop a resolve flag file + without docker exec — mkdir(mode=...) alone is masked by the process + umask, same defect/fix as executor.py's INBOX_DIR_MODE (2026-08-06).""" + old_umask = os.umask(0o022) + try: + obs = _make_observer_simple(tmp_path) + finally: + os.umask(old_umask) + import observer.observer as obs_mod + + mode = obs_mod.RESOLVE_REQUESTS_DIR.stat().st_mode & 0o777 + assert mode == 0o775 diff --git a/services/control-plane/tests/test_supervisor_action_id_uniqueness.py b/services/control-plane/tests/test_supervisor_action_id_uniqueness.py index 703b62e..a63371b 100644 --- a/services/control-plane/tests/test_supervisor_action_id_uniqueness.py +++ b/services/control-plane/tests/test_supervisor_action_id_uniqueness.py @@ -1,4 +1,5 @@ -"""action_id uniqueness for container_restart (2026-08-26 fix). +"""action_id uniqueness for container_restart (2026-08-26) and redeploy +(2026-08-27, same latent risk, same fix). Before this fix, _generate_recommendation() built container_restart action ids as the bare `container-restart--` — no timestamp, no @@ -172,10 +173,51 @@ def test_fallback_bare_id_stable_across_repeated_calls(sup, tmp_path): assert pending[0].name == "container-restart-piha-paperless.json" -def test_redeploy_action_id_stays_bare(sup, tmp_path): - """Non-container_restart drift (redeploy path) is out of scope for this - fix and keeps its existing bare node-service id.""" +def test_redeploy_action_id_falls_back_to_bare_without_incident(sup, tmp_path): + """Non-container_restart drift (redeploy path) with no linked incident + record keeps the pre-fix bare node-service id, same fallback as + container_restart.""" drift = _drift("piha", "outline", trigger_type="service_unhealthy") sup._generate_recommendation(drift) assert (tmp_path / "actions" / "pending" / "redeploy-piha-outline.json").exists() + + +def test_redeploy_action_id_carries_incident_started_at_suffix(sup, tmp_path): + """Domknięcie COMMIT 2 z task/incident-resolve-fix (2026-08-27): redeploy + ids carry the same started_at suffix as container_restart, closing the + same latent node+service collision risk.""" + _seed_incident(sup, "piha", "outline", "inc-3000-piha-outline", started_at=3000) + drift = _drift("piha", "outline", trigger_type="service_unhealthy") + + sup._generate_recommendation(drift) + + pending = _pending(tmp_path) + assert len(pending) == 1 + assert pending[0].name == "redeploy-piha-outline-3000.json" + + +def test_redeploy_new_incident_after_old_completed_gets_different_action_id(sup, tmp_path): + """Same collision scenario as the container_restart case, for redeploy: + a second, later incident for the same node/service must not collide + with the first incident's already-completed action file.""" + _seed_incident(sup, "piha", "outline", "inc-3000-piha-outline", started_at=3000) + drift = _drift("piha", "outline", trigger_type="service_unhealthy") + sup._generate_recommendation(drift) + + first_action_path = tmp_path / "actions" / "pending" / "redeploy-piha-outline-3000.json" + assert first_action_path.exists() + + completed_dir = tmp_path / "actions" / "completed" + completed_dir.mkdir(parents=True, exist_ok=True) + first_action = json.loads(first_action_path.read_text()) + first_action["status"] = "completed" + (completed_dir / first_action_path.name).write_text(json.dumps(first_action)) + first_action_path.unlink() + + _seed_incident(sup, "piha", "outline", "inc-4000-piha-outline", started_at=4000) + sup._generate_recommendation(drift) + + second_action_path = tmp_path / "actions" / "pending" / "redeploy-piha-outline-4000.json" + assert second_action_path.exists() + assert json.loads((completed_dir / "redeploy-piha-outline-3000.json").read_text())["status"] == "completed"