162 lines
6.8 KiB
Python
162 lines
6.8 KiB
Python
|
|
"""action_id uniqueness for container_restart (2026-08-26 fix).
|
||
|
|
|
||
|
|
Before this fix, _generate_recommendation() built container_restart action
|
||
|
|
ids as the bare `container-restart-<node>-<service>` — no timestamp, no
|
||
|
|
incident reference. Two DIFFERENT incidents for the same node+service (e.g. a
|
||
|
|
generic containers_not_running restart followed, after recovery and a later
|
||
|
|
recurrence, by another restart) produced the identical id. Once the first
|
||
|
|
action reached cancelled/completed/failed, the second action's move into that
|
||
|
|
same directory overwrote the first one's history file outright — this is
|
||
|
|
exactly what happened on 2026-08-26 to a shadow-mode HA-websocket restart
|
||
|
|
colliding with an unrelated 08-06 entry (docs/sessions/2026-08-26.md), worked
|
||
|
|
around manually that session by hand-renaming the file.
|
||
|
|
|
||
|
|
The fix anchors the id's suffix to the triggering incident's `started_at`
|
||
|
|
instead of wall-clock time-of-generation, so:
|
||
|
|
- repeated reconcile() calls for the SAME ongoing incident keep producing
|
||
|
|
the SAME action_id (required for the pending/approved/running dedup
|
||
|
|
check in _generate_recommendation to still work — see its comment), and
|
||
|
|
- a NEW incident (after the old one resolved) gets a NEW id, so its
|
||
|
|
eventual cancelled/completed/failed file cannot collide with the old
|
||
|
|
incident's.
|
||
|
|
"""
|
||
|
|
from __future__ import annotations
|
||
|
|
|
||
|
|
import json
|
||
|
|
import sys
|
||
|
|
import time
|
||
|
|
from pathlib import Path
|
||
|
|
|
||
|
|
import pytest
|
||
|
|
|
||
|
|
sys.path.insert(0, str(Path(__file__).parent.parent / "src"))
|
||
|
|
import supervisor as supervisor_module # noqa: E402
|
||
|
|
from supervisor import Supervisor, CONTAINER_RESTART_TRIGGERS # noqa: E402
|
||
|
|
|
||
|
|
|
||
|
|
@pytest.fixture
|
||
|
|
def sup(tmp_path, monkeypatch):
|
||
|
|
actions = tmp_path / "actions"
|
||
|
|
events = tmp_path / "events"
|
||
|
|
world = tmp_path / "world"
|
||
|
|
repo = tmp_path / "repo"
|
||
|
|
for d in (actions, events, world, repo / "inventory", repo / "hosts"):
|
||
|
|
d.mkdir(parents=True, exist_ok=True)
|
||
|
|
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)
|
||
|
|
s = Supervisor()
|
||
|
|
# _generate_recommendation only reads actual_state — bypass disk-backed
|
||
|
|
# _load_desired_state()/_load_actual_state() and set it directly.
|
||
|
|
s.actual_state = {"services": {}, "nodes": {}, "incidents": {}}
|
||
|
|
return s
|
||
|
|
|
||
|
|
|
||
|
|
def _drift(node, service, trigger_type="containers_not_running"):
|
||
|
|
return {
|
||
|
|
"type": "unhealthy_service",
|
||
|
|
"svc_key": f"{node}/{service}",
|
||
|
|
"node": node,
|
||
|
|
"service": service,
|
||
|
|
"status": "unhealthy",
|
||
|
|
"trigger_type": trigger_type,
|
||
|
|
}
|
||
|
|
|
||
|
|
|
||
|
|
def _seed_incident(sup, node, service, incident_id, started_at, status="active"):
|
||
|
|
svc_key = f"{node}/{service}"
|
||
|
|
sup.actual_state["services"][svc_key] = {
|
||
|
|
"node": node, "service": service, "status": "unhealthy",
|
||
|
|
"incident_id": incident_id,
|
||
|
|
}
|
||
|
|
sup.actual_state["incidents"][incident_id] = {
|
||
|
|
"id": incident_id, "status": status,
|
||
|
|
"trigger_type": "containers_not_running",
|
||
|
|
"started_at": started_at,
|
||
|
|
"last_occurrence": started_at,
|
||
|
|
}
|
||
|
|
|
||
|
|
|
||
|
|
def _pending(tmp_path):
|
||
|
|
return list((tmp_path / "actions" / "pending").glob("*.json"))
|
||
|
|
|
||
|
|
|
||
|
|
def test_repeated_calls_for_same_incident_produce_same_action_id_no_duplicate(sup, tmp_path):
|
||
|
|
"""Simulates reconcile() calling _generate_recommendation() on every loop
|
||
|
|
iteration while the drift persists: must not spam a new pending action
|
||
|
|
each time."""
|
||
|
|
_seed_incident(sup, "piha", "paperless", "inc-1000-piha-paperless", started_at=1000)
|
||
|
|
drift = _drift("piha", "paperless")
|
||
|
|
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
|
||
|
|
pending = _pending(tmp_path)
|
||
|
|
assert len(pending) == 1, f"expected exactly one pending action, got {[p.name for p in pending]}"
|
||
|
|
assert pending[0].name == "container-restart-piha-paperless-1000.json"
|
||
|
|
|
||
|
|
|
||
|
|
def test_new_incident_after_old_completed_gets_different_action_id(sup, tmp_path):
|
||
|
|
"""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", "paperless", "inc-1000-piha-paperless", started_at=1000)
|
||
|
|
drift = _drift("piha", "paperless")
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
|
||
|
|
first_action_path = tmp_path / "actions" / "pending" / "container-restart-piha-paperless-1000.json"
|
||
|
|
assert first_action_path.exists()
|
||
|
|
|
||
|
|
# First incident's action ran to completion (moved out of pending/ by the
|
||
|
|
# executor in real life; simulate that here).
|
||
|
|
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()
|
||
|
|
|
||
|
|
# A NEW incident recurs later for the same node+service (old one resolved
|
||
|
|
# in between — new incident_id, new started_at).
|
||
|
|
_seed_incident(sup, "piha", "paperless", "inc-2000-piha-paperless", started_at=2000)
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
|
||
|
|
second_action_path = tmp_path / "actions" / "pending" / "container-restart-piha-paperless-2000.json"
|
||
|
|
assert second_action_path.exists()
|
||
|
|
# The first incident's completed record must be untouched — not overwritten.
|
||
|
|
assert json.loads((completed_dir / "container-restart-piha-paperless-1000.json").read_text())["status"] == "completed"
|
||
|
|
|
||
|
|
|
||
|
|
def test_fallback_to_now_when_incident_record_missing(sup, tmp_path):
|
||
|
|
"""Malformed/missing incident data (incident_id set on the service but no
|
||
|
|
matching record in incidents.json) must still produce a usable
|
||
|
|
(non-crashing) action_id, not block remediation."""
|
||
|
|
svc_key = "piha/paperless"
|
||
|
|
sup.actual_state["services"][svc_key] = {
|
||
|
|
"node": "piha", "service": "paperless", "status": "unhealthy",
|
||
|
|
"incident_id": "inc-missing",
|
||
|
|
}
|
||
|
|
# Deliberately no matching entry in sup.actual_state["incidents"].
|
||
|
|
drift = _drift("piha", "paperless")
|
||
|
|
|
||
|
|
before = int(time.time())
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
after = int(time.time())
|
||
|
|
|
||
|
|
pending = _pending(tmp_path)
|
||
|
|
assert len(pending) == 1
|
||
|
|
name = pending[0].stem
|
||
|
|
assert name.startswith("container-restart-piha-paperless-")
|
||
|
|
suffix_ts = int(name.rsplit("-", 1)[-1])
|
||
|
|
assert before <= suffix_ts <= after
|
||
|
|
|
||
|
|
|
||
|
|
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."""
|
||
|
|
drift = _drift("piha", "outline", trigger_type="service_unhealthy")
|
||
|
|
sup._generate_recommendation(drift)
|
||
|
|
|
||
|
|
assert (tmp_path / "actions" / "pending" / "redeploy-piha-outline.json").exists()
|