From fbf165fbeabf60d2ca066e5a16e06b40f5aa0ddc Mon Sep 17 00:00:00 2001 From: oskar Date: Wed, 29 Jul 2026 19:24:01 +0200 Subject: [PATCH] fix(supervisor): route healthcheck_failed to container_restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit healthcheck_failed incidents fell through to redeploy, which is broken as wired (executor calls deploy-node.sh with arguments it ignores, at a path that does not exist in the container) — so 3376 healthcheck_failed events dead-ended with no working remediation (recon D14/D15). A container restart plausibly heals a failing healthcheck and rides the executor path that actually works; redeploy returns to the map once etap 2 fixes the executor. service_unhealthy / deployment_failed / missing_service stay on redeploy — theoretical until etap 2, kept so drift remains visible in pending actions (noted in comments). CLAUDE.md routing table updated to match; stale mqtt_unreachable example in the observer's trigger_type comment refreshed. Tests: trigger-type recognition and the end-to-end observer→supervisor reconcile test parametrized over both container_restart triggers, with an assertion that no redeploy action is also generated. Full control-plane suite: 147 passed. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 3 +- scripts/observer/observer.py | 2 +- services/control-plane/src/supervisor.py | 20 ++++++++++-- .../tests/test_observer_container_events.py | 31 +++++++++++++------ 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fc02256..0e2932d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -108,7 +108,8 @@ Normalized event types: `deployment_started/completed/failed`, `service_unhealth | Event type | Source | Action generated | Cooldown | |---|---|---|---| | `containers_not_running` | stability-agent | `container_restart` | dedup via stable ID | -| `service_unhealthy` / other | stability-agent | `redeploy` | dedup via stable ID | +| `healthcheck_failed` | node-agent | `container_restart` | dedup via stable ID | +| `service_unhealthy` / other | stability-agent | `redeploy` (broken until etap 2 executor fix) | dedup via stable ID | | `disk_pressure` (high) | stability-agent | `disk_cleanup` | dedup via stable ID | | `ha_websocket_dead` | ha-diag-agent | `container_restart` (homeassistant) | 30 min after completion | | `ha_websocket_recovered` | ha-diag-agent | cancels matching restart | — | diff --git a/scripts/observer/observer.py b/scripts/observer/observer.py index 9f7402b..27784c1 100644 --- a/scripts/observer/observer.py +++ b/scripts/observer/observer.py @@ -863,7 +863,7 @@ class Observer: "severity": event.get("severity"), # trigger_type records the event type that opened this incident so that # the supervisor can choose the appropriate remediation action - # (e.g. container_restart for containers_not_running / mqtt_unreachable + # (container_restart for containers_not_running / healthcheck_failed # vs. a full redeploy for other causes). "trigger_type": event.get("type"), "started_at": event.get("timestamp"), diff --git a/services/control-plane/src/supervisor.py b/services/control-plane/src/supervisor.py index bfadb8e..93e066d 100644 --- a/services/control-plane/src/supervisor.py +++ b/services/control-plane/src/supervisor.py @@ -34,12 +34,21 @@ except Exception: NODE_ALIAS_MAP = {} # Incident trigger types that should result in a lightweight container_restart -# rather than a full redeploy: the container is present but not running. +# rather than a full redeploy: the container is present but not running, or +# running with a failing health check — a restart plausibly heals both. +# healthcheck_failed added 2026-07-29 (recon D14/D15): it used to route to +# redeploy, which is broken as wired (executor calls deploy-node.sh with +# arguments it ignores, at a path that does not exist in the container), so +# 3376 healthcheck_failed events dead-ended. redeploy returns to the map only +# once it actually works (etap 2). Everything not listed here +# (service_unhealthy, deployment_failed, missing_service) still falls through +# to redeploy — theoretical until etap 2 fixes the executor, kept as-is so the +# drift stays visible in pending actions. # mqtt_unreachable was removed 2026-07-28: the observer never creates incidents # with that trigger_type, so the branch was dead code (recon # docs/architecture/RECON-multiagent-2026-07-27.md, D15). stability-agent still # emits the mqtt_unreachable *event*; it just never becomes an incident. -CONTAINER_RESTART_TRIGGERS = {"containers_not_running"} +CONTAINER_RESTART_TRIGGERS = {"containers_not_running", "healthcheck_failed"} # Nodes where automatic disk_cleanup actions must NOT be generated. # On chelsty nodes disk fullness is overwhelmingly caused by Frigate recordings @@ -412,7 +421,8 @@ class Supervisor: if trigger_type in CONTAINER_RESTART_TRIGGERS: # Lightweight remediation: the container exists but is not running - # (containers_not_running). A docker restart is sufficient and low-risk. + # (containers_not_running) or is up with a failing health check + # (healthcheck_failed). A docker restart is sufficient and low-risk. container_name = self._get_container_name(service) action = { "action_id": action_id, @@ -436,6 +446,10 @@ class Supervisor: else: # Full redeploy: container is running but service is broken, # or the cause is unknown / not a simple restart candidate. + # NOTE: the redeploy action type is currently theoretical — the + # executor's redeploy path is broken until etap 2 (see + # CONTAINER_RESTART_TRIGGERS comment). Generated anyway so the + # drift is visible to the operator in pending actions. action = { "action_id": action_id, "timestamp": time.time(), diff --git a/services/control-plane/tests/test_observer_container_events.py b/services/control-plane/tests/test_observer_container_events.py index 9329b6f..c2e80a0 100644 --- a/services/control-plane/tests/test_observer_container_events.py +++ b/services/control-plane/tests/test_observer_container_events.py @@ -106,12 +106,15 @@ def test_containers_not_running_creates_incident_with_trigger_type(observer): assert inc["trigger_type"] == "containers_not_running" -def test_incident_trigger_type_is_recognized_by_supervisor(observer): +@pytest.mark.parametrize("etype", ["containers_not_running", "healthcheck_failed"]) +def test_incident_trigger_type_is_recognized_by_supervisor(observer, etype): """The trigger_type the observer stamps MUST be one the supervisor routes to - a container_restart — otherwise remediation silently never fires.""" - observer.process_event(_container_event("containers_not_running")) + a container_restart — otherwise remediation silently never fires (or, for + healthcheck_failed pre-etap-1, dead-ended in the broken redeploy path).""" + observer.process_event(_container_event(etype)) svc = observer.world_state["services"]["piha/paperless"] inc = observer.world_state["incidents"][svc["incident_id"]] + assert inc["trigger_type"] == etype assert inc["trigger_type"] in CONTAINER_RESTART_TRIGGERS @@ -131,15 +134,21 @@ def test_containers_not_running_after_healthy_transitions_to_unhealthy(observer) # 2. End-to-end: observer output → supervisor generates container_restart # --------------------------------------------------------------------------- +@pytest.mark.parametrize("etype", ["containers_not_running", "healthcheck_failed"]) def test_supervisor_generates_container_restart_from_observer_output( - observer, tmp_path, monkeypatch + observer, tmp_path, monkeypatch, etype ): - """Integration: run the real observer to produce world state for a dead + """Integration: run the real observer to produce world state for a failing container, then run the real supervisor.reconcile() against it and assert a - container_restart action lands in pending/. Proves the whole loop works with - NO change to the supervisor.""" - # 1. Observer ingests the dead-container event and writes world state to disk. - observer.process_event(_container_event("containers_not_running")) + container_restart action lands in pending/. + + healthcheck_failed routes to container_restart since etap 1 (2026-07-29, + recon D14/D15): it used to fall through to redeploy, which is broken as + wired in the executor, so those events dead-ended. A restart plausibly + heals a failing healthcheck; redeploy returns once the executor works + (etap 2).""" + # 1. Observer ingests the failure event and writes world state to disk. + observer.process_event(_container_event(etype)) observer._save_world() # 2. Point the supervisor at a fresh tmp tree; copy observer world output in. @@ -176,7 +185,9 @@ def test_supervisor_generates_container_restart_from_observer_output( assert action["type"] == "container_restart" assert action["node"] == "piha" assert action["service"] == "paperless" - assert action["payload"]["reason"] == "containers_not_running" + assert action["payload"]["reason"] == etype + # Must not ALSO fall through to the (broken until etap 2) redeploy path. + assert not (actions / "pending" / "redeploy-piha-paperless.json").exists() # ---------------------------------------------------------------------------