Compare commits
2 commits
ff8412b565
...
74ff3ee1e2
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
74ff3ee1e2 | ||
|
|
91db6829f4 |
|
|
@ -469,15 +469,27 @@ class Supervisor:
|
|||
# life of one incident (observer._handle_incident only bumps
|
||||
# last_occurrence/occurrence_count on repeat occurrences) and changes
|
||||
# only when a new incident is opened for that service — exactly the
|
||||
# cases we want "same id" and "different id" for, respectively. Falls
|
||||
# back to time.time() if the incident record is missing/malformed so
|
||||
# a restart action is still generated (never block remediation on
|
||||
# this being unavailable).
|
||||
# cases we want "same id" and "different id" for, respectively.
|
||||
#
|
||||
# No incident record → bare id (the pre-fix format, no suffix at
|
||||
# all), NOT time.time(). A missing/unlinked incident_id is not just
|
||||
# a malformed-data edge case: observer._prune_stale_world Case 3
|
||||
# (commit 71a7af5) clears service.incident_id after 24h of event
|
||||
# silence even while the underlying drift is still ongoing, so this
|
||||
# path is hit by a live, still-restarting service. time.time() would
|
||||
# mint a new action_id — and a new pending file — on every single
|
||||
# reconcile() tick, defeating the dedup check below entirely. The
|
||||
# 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 {}
|
||||
suffix_ts = int(_parse_ts(incident.get("started_at"))) or int(time.time())
|
||||
action_id = f"container-restart-{node}-{service}-{suffix_ts}"
|
||||
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}"
|
||||
else:
|
||||
# redeploy IDs stay bare (node-service) — out of scope for this
|
||||
# fix (see commit message: no observed collision here yet), and
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
|
@ -128,10 +127,15 @@ def test_new_incident_after_old_completed_gets_different_action_id(sup, tmp_path
|
|||
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):
|
||||
def test_fallback_to_bare_id_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."""
|
||||
(non-crashing) action_id, not block remediation. The fallback must be the
|
||||
bare pre-fix id — NOT a time.time() suffix — since this path is hit
|
||||
naturally (not just on malformed data): observer._prune_stale_world
|
||||
Case 3 (commit 71a7af5) clears service.incident_id after 24h of event
|
||||
silence while the drift is still ongoing, so a time.time() suffix would
|
||||
mint a new id on every reconcile() tick forever."""
|
||||
svc_key = "piha/paperless"
|
||||
sup.actual_state["services"][svc_key] = {
|
||||
"node": "piha", "service": "paperless", "status": "unhealthy",
|
||||
|
|
@ -140,16 +144,32 @@ def test_fallback_to_now_when_incident_record_missing(sup, tmp_path):
|
|||
# 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
|
||||
assert pending[0].name == "container-restart-piha-paperless.json"
|
||||
|
||||
|
||||
def test_fallback_bare_id_stable_across_repeated_calls(sup, tmp_path):
|
||||
"""Same missing-incident-record scenario, but simulating reconcile()
|
||||
calling _generate_recommendation() on every loop iteration while the
|
||||
drift persists: must not spam a new pending action each time, exactly
|
||||
like the has-an-incident-record case above."""
|
||||
svc_key = "piha/paperless"
|
||||
sup.actual_state["services"][svc_key] = {
|
||||
"node": "piha", "service": "paperless", "status": "unhealthy",
|
||||
"incident_id": "inc-missing",
|
||||
}
|
||||
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.json"
|
||||
|
||||
|
||||
def test_redeploy_action_id_stays_bare(sup, tmp_path):
|
||||
|
|
|
|||
Loading…
Reference in a new issue