fix(supervisor): apply started_at suffix to redeploy action_id too
Domknięcie COMMIT 2 z task/incident-resolve-fix (2026-08-26):
redeploy-<node>-<service> carried the same latent collision risk as
container-restart-<node>-<service> before that fix — two different
incidents for the same node+service can still overwrite each other's
cancelled/completed/failed history.
Verified before applying the same fix: no code anywhere reconstructs
`redeploy-{node}-{service}` for an exact-match lookup.
_cancel_resolved_pending_actions matches on the node/service *fields*
inside each pending file, not 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. Safe to extend the suffix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VMn76yx2CNuHKFVMrYcKWA
This commit is contained in:
parent
40d78ce60d
commit
1dca438015
|
|
@ -452,13 +452,15 @@ class Supervisor:
|
||||||
|
|
||||||
# Choose action type first so we can build the ID.
|
# Choose action type first so we can build the ID.
|
||||||
#
|
#
|
||||||
# container_restart IDs carry a suffix so two DIFFERENT incidents for
|
# Both container_restart and redeploy IDs carry a suffix so two
|
||||||
# the same node+service never collide in cancelled/completed/failed
|
# DIFFERENT incidents for the same node+service never collide in
|
||||||
# (2026-08-26: a generic containers_not_running restart and a later
|
# cancelled/completed/failed (2026-08-26: a generic
|
||||||
# ha-diag-agent shadow-mode restart both used the bare
|
# 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
|
# container-restart-piha-homeassistant id and overwrote each other's
|
||||||
# history — worked around manually that session, see
|
# 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()
|
# The suffix is the triggering incident's started_at, NOT time.time()
|
||||||
# at generation time: reconcile() calls _generate_recommendation on
|
# at generation time: reconcile() calls _generate_recommendation on
|
||||||
|
|
@ -482,20 +484,22 @@ class Supervisor:
|
||||||
# bare id has no incident to distinguish "same" from "different"
|
# bare id has no incident to distinguish "same" from "different"
|
||||||
# occurrences by, but it is at least stable across calls, which is
|
# occurrences by, but it is at least stable across calls, which is
|
||||||
# what idempotency here actually requires.
|
# what idempotency here actually requires.
|
||||||
if trigger_type in CONTAINER_RESTART_TRIGGERS:
|
#
|
||||||
|
# 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_id = self.actual_state["services"].get(drift["svc_key"], {}).get("incident_id")
|
||||||
incident = self.actual_state["incidents"].get(incident_id, {}) if incident_id else {}
|
incident = self.actual_state["incidents"].get(incident_id, {}) if incident_id else {}
|
||||||
started_ts = int(_parse_ts(incident.get("started_at")))
|
started_ts = int(_parse_ts(incident.get("started_at")))
|
||||||
if started_ts:
|
if started_ts:
|
||||||
action_id = f"container-restart-{node}-{service}-{started_ts}"
|
action_id = f"{action_prefix}-{node}-{service}-{started_ts}"
|
||||||
else:
|
else:
|
||||||
action_id = f"container-restart-{node}-{service}"
|
action_id = f"{action_prefix}-{node}-{service}"
|
||||||
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}"
|
|
||||||
|
|
||||||
# Skip if an action for this ID is already live in any active state
|
# Skip if an action for this ID is already live in any active state
|
||||||
# (pending → approved → running). This prevents re-creation after
|
# (pending → approved → running). This prevents re-creation after
|
||||||
|
|
|
||||||
|
|
@ -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
|
Before this fix, _generate_recommendation() built container_restart action
|
||||||
ids as the bare `container-restart-<node>-<service>` — no timestamp, no
|
ids as the bare `container-restart-<node>-<service>` — 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"
|
assert pending[0].name == "container-restart-piha-paperless.json"
|
||||||
|
|
||||||
|
|
||||||
def test_redeploy_action_id_stays_bare(sup, tmp_path):
|
def test_redeploy_action_id_falls_back_to_bare_without_incident(sup, tmp_path):
|
||||||
"""Non-container_restart drift (redeploy path) is out of scope for this
|
"""Non-container_restart drift (redeploy path) with no linked incident
|
||||||
fix and keeps its existing bare node-service id."""
|
record keeps the pre-fix bare node-service id, same fallback as
|
||||||
|
container_restart."""
|
||||||
drift = _drift("piha", "outline", trigger_type="service_unhealthy")
|
drift = _drift("piha", "outline", trigger_type="service_unhealthy")
|
||||||
sup._generate_recommendation(drift)
|
sup._generate_recommendation(drift)
|
||||||
|
|
||||||
assert (tmp_path / "actions" / "pending" / "redeploy-piha-outline.json").exists()
|
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"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue