fix(observer): map containers_not_running to unhealthy status + incident — dead/crash-looping containers created no incident, so supervisor never alerted
node-agent (and stability-agent) emit containers_not_running for exited/dead
and crash-looping containers, but the observer's event->status/incident map
only handled service_recovered/service_healthy/service_unhealthy/
healthcheck_failed. containers_not_running fell through: status stayed at its
last "healthy" value and no incident was opened, so the supervisor saw no drift
and generated no action — a dead container produced ZERO operator alerts
(matches the "action queue empty despite failures" symptom).
- containers_not_running now sets status=unhealthy and opens an incident whose
trigger_type ("containers_not_running") is already in the supervisor's
CONTAINER_RESTART_TRIGGERS, so remediation (container_restart) fires with no
supervisor change. Recovery is unchanged: service_healthy resolves the
incident via the existing svc_key->incident_id link.
- container_restarting / container_state_unexpected (added in 4746ebe) are kept
intentionally observational — no incident, status not flipped to unhealthy
(would cause a false redeploy for a transient blip) — but leave a
last_observation trace so they don't vanish. A real crash-loop still escalates
via node-agent re-emitting containers_not_running.
Tests: services/control-plane/tests/test_observer_container_events.py — status
+ incident + trigger_type, end-to-end reconcile -> container_restart, recovery
auto-resolve, observational no-incident/trace, idempotent (no incident
multiplication). Full control-plane suite: 117 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c6b186eca7
commit
4bfd6c4429
|
|
@ -750,9 +750,50 @@ class Observer:
|
||||||
# unhealthy/crashing is now confirmed healthy, the incident is over.
|
# unhealthy/crashing is now confirmed healthy, the incident is over.
|
||||||
self.world_state["services"][svc_key]["status"] = "healthy"
|
self.world_state["services"][svc_key]["status"] = "healthy"
|
||||||
self._resolve_incident(svc_key, timestamp)
|
self._resolve_incident(svc_key, timestamp)
|
||||||
elif etype in ["service_unhealthy", "healthcheck_failed"]:
|
elif etype in ["service_unhealthy", "healthcheck_failed", "containers_not_running"]:
|
||||||
|
# containers_not_running: node-agent (node_agent.py) — and the
|
||||||
|
# stability-agent — report a *managed* container that has exited /
|
||||||
|
# dead or is crash-looping (restarting past RestartCount threshold).
|
||||||
|
# Treat it exactly like the other hard-failure signals: mark the
|
||||||
|
# service unhealthy and open an incident. The incident's
|
||||||
|
# trigger_type is the event type ("containers_not_running"), which
|
||||||
|
# the supervisor already recognises in CONTAINER_RESTART_TRIGGERS and
|
||||||
|
# remediates with a low-risk container_restart (vs. a full redeploy).
|
||||||
|
#
|
||||||
|
# Before this branch existed, containers_not_running fell through
|
||||||
|
# this if/elif chain entirely: node-agent saw the dead container and
|
||||||
|
# emitted the event, the observer bumped last_check but left status at
|
||||||
|
# its last "healthy" value and created NO incident. The supervisor
|
||||||
|
# then saw no drift and generated no action — so a dead / crash-looping
|
||||||
|
# container produced ZERO operator alerts. The monitoring loop was
|
||||||
|
# silently broken (matches the "action queue empty despite failures"
|
||||||
|
# symptom). (node_agent.py's own comment claims this event "rides the
|
||||||
|
# existing, supervisor-wired remediation path" — that path only exists
|
||||||
|
# once the observer opens the incident here.)
|
||||||
self.world_state["services"][svc_key]["status"] = "unhealthy"
|
self.world_state["services"][svc_key]["status"] = "unhealthy"
|
||||||
self._handle_incident(svc_key, event)
|
self._handle_incident(svc_key, event)
|
||||||
|
elif etype in ["container_restarting", "container_state_unexpected"]:
|
||||||
|
# Intentionally observational — parity with node_agent.check_containers,
|
||||||
|
# which emits these as low/medium severity and explicitly does NOT wire
|
||||||
|
# them to any supervisor trigger. Causes: a transient post-deploy
|
||||||
|
# restart still below the crash-loop threshold, an operator `docker
|
||||||
|
# pause`, or an unhandled-but-not-fault docker state.
|
||||||
|
#
|
||||||
|
# We deliberately do NOT set status=unhealthy (that would make the
|
||||||
|
# supervisor generate remediation for a transient blip — a redeploy,
|
||||||
|
# since there is no CONTAINER_RESTART_TRIGGERS incident) and do NOT
|
||||||
|
# open an incident (noise). But the event must not vanish, so we
|
||||||
|
# record a lightweight observational trace on the service entry; the
|
||||||
|
# raw event also stays visible in the /events feed. If a restart is a
|
||||||
|
# real crash-loop it escalates on its own: node-agent re-emits it as
|
||||||
|
# containers_not_running once RestartCount crosses the threshold, and
|
||||||
|
# the branch above then alarms.
|
||||||
|
self.world_state["services"][svc_key]["last_observation"] = {
|
||||||
|
"type": etype,
|
||||||
|
"severity": severity,
|
||||||
|
"timestamp": timestamp,
|
||||||
|
"message": event.get("message"),
|
||||||
|
}
|
||||||
|
|
||||||
# 3. Update Deployment State
|
# 3. Update Deployment State
|
||||||
if etype.startswith("deployment_") and cid:
|
if etype.startswith("deployment_") and cid:
|
||||||
|
|
|
||||||
253
services/control-plane/tests/test_observer_container_events.py
Normal file
253
services/control-plane/tests/test_observer_container_events.py
Normal file
|
|
@ -0,0 +1,253 @@
|
||||||
|
"""Tests for observer handling of node-agent container-state events.
|
||||||
|
|
||||||
|
Regression suite for the "dead/crash-looping container = silent" bug: the
|
||||||
|
observer's process_event event→status/incident map handled service_recovered,
|
||||||
|
service_healthy, service_unhealthy and healthcheck_failed but NOT
|
||||||
|
containers_not_running (emitted by node-agent for exited/dead/crash-loop
|
||||||
|
containers, and by stability-agent). A dead container therefore left the
|
||||||
|
service at its last "healthy" status with no incident, so the supervisor saw no
|
||||||
|
drift and produced no action — the operator was never alerted.
|
||||||
|
|
||||||
|
Covers:
|
||||||
|
1. containers_not_running → status=unhealthy + incident with the right trigger_type
|
||||||
|
2. the incident's trigger_type is one the supervisor already routes to a
|
||||||
|
container_restart (end-to-end reconcile, no supervisor change required)
|
||||||
|
3. service_healthy after containers_not_running → incident auto-resolved
|
||||||
|
4. container_restarting / container_state_unexpected → observational: no
|
||||||
|
incident, status not flipped to unhealthy, but a trace is left behind
|
||||||
|
5. idempotency: reprocessing containers_not_running does not multiply incidents
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
import sys
|
||||||
|
import time
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
# Observer lives outside the control-plane package; add scripts/ to path.
|
||||||
|
sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / "scripts"))
|
||||||
|
sys.path.insert(0, str(Path(__file__).parent.parent / "src"))
|
||||||
|
|
||||||
|
from observer.observer import Observer # noqa: E402
|
||||||
|
import supervisor as supervisor_module # noqa: E402
|
||||||
|
from supervisor import Supervisor, CONTAINER_RESTART_TRIGGERS # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Observer fixture — redirect every runtime path into a per-test tmp_path.
|
||||||
|
# Mirrors the fixture in test_incident_lifecycle.py (incl. OBSERVER_STATE_FILE,
|
||||||
|
# which is derived from STATE_DIR at import time).
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def observer(tmp_path, monkeypatch) -> Observer:
|
||||||
|
import observer.observer as obs_mod
|
||||||
|
|
||||||
|
world = tmp_path / "world"
|
||||||
|
state = tmp_path / "state"
|
||||||
|
events = tmp_path / "events"
|
||||||
|
logs = tmp_path / "logs"
|
||||||
|
repo = tmp_path / "repo"
|
||||||
|
for d in (world, state, events, logs, repo / "inventory", repo / "hosts"):
|
||||||
|
d.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
|
(repo / "inventory" / "topology.yaml").write_text(
|
||||||
|
"nodes:\n piha:\n roles: [infra]\n connectivity: {}\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
monkeypatch.setattr(obs_mod, "WORLD_DIR", world)
|
||||||
|
monkeypatch.setattr(obs_mod, "STATE_DIR", state)
|
||||||
|
monkeypatch.setattr(obs_mod, "EVENTS_DIR", events)
|
||||||
|
monkeypatch.setattr(obs_mod, "LOGS_DIR", logs)
|
||||||
|
monkeypatch.setattr(obs_mod, "INVENTORY_TOPOLOGY", repo / "inventory" / "topology.yaml")
|
||||||
|
monkeypatch.setattr(obs_mod, "REPO_ROOT", repo)
|
||||||
|
monkeypatch.setattr(obs_mod, "FAILED_EVENTS_DIR", state / "observer_failed_events")
|
||||||
|
monkeypatch.setattr(obs_mod, "OBSERVER_STATE_FILE", state / "observer_checkpoint.json")
|
||||||
|
return Observer()
|
||||||
|
|
||||||
|
|
||||||
|
def _container_event(etype, *, node="piha", service="paperless",
|
||||||
|
severity="high", ts=None, payload=None):
|
||||||
|
return {
|
||||||
|
"id": f"evt-{node}-{ts or int(time.time())}-{etype}-{service}",
|
||||||
|
"type": etype,
|
||||||
|
"node": node,
|
||||||
|
"service": service,
|
||||||
|
"severity": severity,
|
||||||
|
"timestamp": ts or int(time.time()),
|
||||||
|
"message": f"Container '{service}' {etype}",
|
||||||
|
"payload": payload or {"container": service, "status": "exited"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 1. containers_not_running → unhealthy + incident
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_containers_not_running_sets_unhealthy(observer):
|
||||||
|
observer.process_event(_container_event("containers_not_running"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
assert svc["status"] == "unhealthy"
|
||||||
|
assert svc["incident_id"] is not None
|
||||||
|
|
||||||
|
|
||||||
|
def test_containers_not_running_creates_incident_with_trigger_type(observer):
|
||||||
|
ev = _container_event("containers_not_running")
|
||||||
|
observer.process_event(ev)
|
||||||
|
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
inc = observer.world_state["incidents"][svc["incident_id"]]
|
||||||
|
assert inc["status"] == "active"
|
||||||
|
assert inc["node"] == "piha"
|
||||||
|
assert inc["service"] == "paperless"
|
||||||
|
# trigger_type is the event type, which the supervisor routes to container_restart.
|
||||||
|
assert inc["trigger_type"] == "containers_not_running"
|
||||||
|
|
||||||
|
|
||||||
|
def test_incident_trigger_type_is_recognized_by_supervisor(observer):
|
||||||
|
"""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"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
inc = observer.world_state["incidents"][svc["incident_id"]]
|
||||||
|
assert inc["trigger_type"] in CONTAINER_RESTART_TRIGGERS
|
||||||
|
|
||||||
|
|
||||||
|
def test_containers_not_running_after_healthy_transitions_to_unhealthy(observer):
|
||||||
|
"""A container that was healthy and then dies must flip to unhealthy — the
|
||||||
|
exact real-world sequence that used to stay stuck at 'healthy'."""
|
||||||
|
observer.process_event(_container_event("service_healthy", severity="info"))
|
||||||
|
assert observer.world_state["services"]["piha/paperless"]["status"] == "healthy"
|
||||||
|
|
||||||
|
observer.process_event(_container_event("containers_not_running"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
assert svc["status"] == "unhealthy"
|
||||||
|
assert svc["incident_id"] is not None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 2. End-to-end: observer output → supervisor generates container_restart
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_supervisor_generates_container_restart_from_observer_output(
|
||||||
|
observer, tmp_path, monkeypatch
|
||||||
|
):
|
||||||
|
"""Integration: run the real observer to produce world state for a dead
|
||||||
|
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"))
|
||||||
|
observer._save_world()
|
||||||
|
|
||||||
|
# 2. Point the supervisor at a fresh tmp tree; copy observer world output in.
|
||||||
|
actions = tmp_path / "sup_actions"
|
||||||
|
events = tmp_path / "sup_events"
|
||||||
|
world = tmp_path / "sup_world"
|
||||||
|
repo = tmp_path / "sup_repo"
|
||||||
|
for d in (actions, events, world, repo / "hosts" / "piha"):
|
||||||
|
d.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
|
import observer.observer as obs_mod
|
||||||
|
for name in ("services.json", "incidents.json", "nodes.json"):
|
||||||
|
src = obs_mod.WORLD_DIR / name
|
||||||
|
if src.exists():
|
||||||
|
(world / name).write_text(src.read_text())
|
||||||
|
|
||||||
|
# Declare paperless as a desired service on piha so the drift loop considers it.
|
||||||
|
(repo / "hosts" / "piha" / "services.yaml").write_text(
|
||||||
|
"host: piha\nservices:\n paperless:\n role: kb\n exposure: lan\n"
|
||||||
|
)
|
||||||
|
|
||||||
|
monkeypatch.setattr(supervisor_module, "ACTIONS_DIR", actions)
|
||||||
|
monkeypatch.setattr(supervisor_module, "EVENTS_DIR", events)
|
||||||
|
monkeypatch.setattr(supervisor_module, "WORLD_DIR", world)
|
||||||
|
monkeypatch.setattr(supervisor_module, "REPO_ROOT", repo)
|
||||||
|
|
||||||
|
sup = Supervisor()
|
||||||
|
sup.reconcile()
|
||||||
|
|
||||||
|
action_id = "container-restart-piha-paperless"
|
||||||
|
action_path = actions / "pending" / f"{action_id}.json"
|
||||||
|
assert action_path.exists(), "supervisor did not generate a container_restart"
|
||||||
|
action = json.loads(action_path.read_text())
|
||||||
|
assert action["type"] == "container_restart"
|
||||||
|
assert action["node"] == "piha"
|
||||||
|
assert action["service"] == "paperless"
|
||||||
|
assert action["payload"]["reason"] == "containers_not_running"
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 3. Recovery: service_healthy after containers_not_running auto-resolves
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_service_healthy_resolves_containers_not_running_incident(observer):
|
||||||
|
observer.process_event(_container_event("containers_not_running"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
inc_id = svc["incident_id"]
|
||||||
|
assert observer.world_state["incidents"][inc_id]["status"] == "active"
|
||||||
|
|
||||||
|
observer.process_event(_container_event("service_healthy", severity="info"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
assert svc["status"] == "healthy"
|
||||||
|
assert svc["incident_id"] is None
|
||||||
|
assert observer.world_state["incidents"][inc_id]["status"] == "resolved"
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 4. Observational events — no incident, no false remediation, but a trace
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("etype", ["container_restarting", "container_state_unexpected"])
|
||||||
|
def test_observational_events_do_not_create_incident(observer, etype):
|
||||||
|
observer.process_event(_container_event(etype, severity="low"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
# No incident, and status is NOT flipped to unhealthy (would trigger a redeploy).
|
||||||
|
assert svc.get("incident_id") is None
|
||||||
|
assert svc["status"] != "unhealthy"
|
||||||
|
assert not observer.world_state["incidents"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("etype", ["container_restarting", "container_state_unexpected"])
|
||||||
|
def test_observational_events_leave_a_trace(observer, etype):
|
||||||
|
"""They must be visible in world state, not vanish into the void."""
|
||||||
|
observer.process_event(_container_event(etype, severity="medium"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
obs_note = svc.get("last_observation")
|
||||||
|
assert obs_note is not None
|
||||||
|
assert obs_note["type"] == etype
|
||||||
|
|
||||||
|
|
||||||
|
def test_container_restarting_does_not_clear_existing_unhealthy(observer):
|
||||||
|
"""A container that is already unhealthy (dead) and then reports a transient
|
||||||
|
'restarting' must NOT be downgraded to healthy — the incident stands until a
|
||||||
|
real service_healthy arrives."""
|
||||||
|
observer.process_event(_container_event("containers_not_running"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
assert svc["status"] == "unhealthy"
|
||||||
|
inc_id = svc["incident_id"]
|
||||||
|
|
||||||
|
observer.process_event(_container_event("container_restarting", severity="low"))
|
||||||
|
svc = observer.world_state["services"]["piha/paperless"]
|
||||||
|
assert svc["status"] == "unhealthy"
|
||||||
|
assert svc["incident_id"] == inc_id
|
||||||
|
assert observer.world_state["incidents"][inc_id]["status"] == "active"
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# 5. Idempotency — reprocessing must not multiply incidents
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_repeated_containers_not_running_does_not_multiply_incidents(observer):
|
||||||
|
ev = _container_event("containers_not_running", ts=1784000000)
|
||||||
|
observer.process_event(ev)
|
||||||
|
observer.process_event(ev)
|
||||||
|
observer.process_event(ev)
|
||||||
|
|
||||||
|
active = [i for i in observer.world_state["incidents"].values()
|
||||||
|
if i["status"] == "active"]
|
||||||
|
assert len(active) == 1
|
||||||
|
# Correlation bumps occurrence_count rather than opening new incidents.
|
||||||
|
assert active[0]["occurrence_count"] == 3
|
||||||
Loading…
Reference in a new issue