diff --git a/ariadne/services/hermes_autotriage.py b/ariadne/services/hermes_autotriage.py index 213bf95..a4252aa 100644 --- a/ariadne/services/hermes_autotriage.py +++ b/ariadne/services/hermes_autotriage.py @@ -355,7 +355,13 @@ def _execute_action( def _repair_demo_fixture(storage: Any, base: dict[str, Any], action_id: str) -> dict[str, Any]: - """Run the demo-fixture repair Job and request the verification rebuild.""" + """Patch the demo fixture healthy and request the verification rebuild. + + The repair runs in-process against the cluster API rather than as a + Kubernetes Job, so the executed action carries the patched target instead + of a Job name; the one-action-per-incident budget upstream is still what + stops the repair from repeating. + """ hermes_events.record_action(storage, base, action_id, "requested", None) hermes_events.record_action(storage, base, action_id, "accepted", None) @@ -370,18 +376,10 @@ def _repair_demo_fixture(storage: Any, base: dict[str, Any], action_id: str) -> rebuild = hermes_autotriage_repair.trigger_rebuild(settings, str(base["job"])) if not rebuild.get("requested"): return _fail_action(storage, base, action_id, str(rebuild.get("error") or "rebuild trigger failed")) - hermes_events.record_action(storage, base, action_id, "executed", {"repair_job": repair.get("job_name")}) - hermes_events.record_incident( - storage, - base, - "awaiting_rebuild", - {"action": action_id, "repair_job": repair.get("job_name")}, - ) - return { - "status": "awaiting_rebuild", - "incident_id": str(base["incident_id"]), - "repair_job": repair.get("job_name"), - } + detail = {"repair": repair.get("action"), "target": repair.get("target")} + hermes_events.record_action(storage, base, action_id, "executed", detail) + hermes_events.record_incident(storage, base, "awaiting_rebuild", {"action": action_id, **detail}) + return {"status": "awaiting_rebuild", "incident_id": str(base["incident_id"]), **detail} def _retry_transient_infra(storage: Any, base: dict[str, Any], marker: str | None) -> dict[str, Any]: @@ -480,12 +478,11 @@ def _decision_config() -> dict[str, Any]: def _repair_config() -> dict[str, Any]: - """Build the config passed to the repair Job executor.""" + """Build the config naming the ConfigMap the repair action patches.""" return { "namespace": settings.hermes_demo_namespace, "fixture_configmap": settings.hermes_demo_fixture_configmap, - "image": settings.hermes_repair_image, } diff --git a/ariadne/services/hermes_autotriage_repair.py b/ariadne/services/hermes_autotriage_repair.py index 13c22d3..2cd9b4e 100644 --- a/ariadne/services/hermes_autotriage_repair.py +++ b/ariadne/services/hermes_autotriage_repair.py @@ -1,54 +1,55 @@ +"""Remediation executors for the Hermes auto-triage demo incident flow. + +The fixture repair is an in-process Ariadne action, not a Kubernetes Job: it +merge-patches the demo fixture ConfigMap back to healthy over the cluster API +using Ariadne's own service account, which needs get/patch on that one +ConfigMap. Because the patch always writes the same terminal value, the +repair is naturally idempotent and needs no duplicate guard of its own; the +one-action-per-incident budget upstream (the prior-action-count check in the +authorization chain) remains the only limit on how often it runs. +""" + from __future__ import annotations -import json -import time from typing import Any import httpx -from ..k8s.client import get_json, post_json +from ..k8s.client import patch_json from ..utils.logging import get_logger logger = get_logger(__name__) HTTP_CREATED = 201 -HTTP_CONFLICT = 409 -_DEFAULT_WAIT_TIMEOUT_SECONDS = 120.0 -_POLL_INTERVAL_SECONDS = 2.0 -_JOB_TTL_SECONDS = 3600 -_REPAIR_MESSAGE = "fixture state reset to healthy" +_REPAIR_ACTION = "configmap_patch" _PARAMETERIZED_ENDPOINT = "buildWithParameters" _PLAIN_ENDPOINT = "build" _BUILD_PARAMETERS = {"SEED_FAILURE": "false"} def execute_repair(cfg: dict, incident_id: str, build_number: int) -> dict[str, Any]: - """Run the hardcoded demo-fixture repair Job and wait for its outcome. + """Patch the demo fixture ConfigMap back to healthy in one API call. - Inputs: `cfg` with namespace, fixture_configmap, and image (plus optional - wait_timeout_seconds); the incident id and the failed build number that - names the Job. Outputs: {"job_name", "succeeded", "error"}; a 409 on - creation is reported as a "duplicate job" failure. Never raises. + Inputs: `cfg` with namespace and fixture_configmap, the incident id used + for the log line, and the failed build number (kept for the executor + signature; the patch is the same whatever built). Outputs: {"action", + "target", "succeeded", "error"}. No Job, no polling: a non-2xx response, + a transport error, or an incomplete target returns succeeded False with a + short reason. Never raises. """ - namespace = str(cfg.get("namespace") or "") - job_name = f"hermes-demo-repair-{build_number}" + namespace = str(cfg.get("namespace") or "").strip() + name = str(cfg.get("fixture_configmap") or "").strip() + target = f"{namespace}/{name}" + if not namespace or not name: + return _repair_result(target, False, "fixture target incomplete", incident_id) try: - post_json( - f"/apis/batch/v1/namespaces/{namespace}/jobs", - _job_payload(cfg, job_name, incident_id), - ) + patch_json(f"/api/v1/namespaces/{namespace}/configmaps/{name}", {"data": {"state": "healthy"}}) except Exception as exc: - error = "duplicate job" if _status_code(exc) == HTTP_CONFLICT else f"job create failed: {exc}" - logger.info( - "hermes demo repair job creation failed", - extra={"event": "hermes_autotriage_repair", "status": "error", "job": job_name, "detail": error}, - ) - return {"job_name": job_name, "succeeded": False, "error": error} - timeout_seconds = _number(cfg.get("wait_timeout_seconds"), _DEFAULT_WAIT_TIMEOUT_SECONDS) - return _wait_for_completion(namespace, job_name, timeout_seconds) + return _repair_result(target, False, _patch_error(exc), incident_id) + return _repair_result(target, True, None, incident_id) def trigger_rebuild(config: Any, job: str) -> dict[str, Any]: @@ -99,73 +100,28 @@ def _post_build( return {"requested": False, "error": f"{label} http {response.status_code}"} -def _job_payload(cfg: dict, job_name: str, incident_id: str) -> dict[str, Any]: - """Build the hardcoded batch Job manifest that resets the demo fixture.""" +def _repair_result(target: str, succeeded: bool, error: str | None, incident_id: str) -> dict[str, Any]: + """Log one repair outcome line and return the executor's result dict.""" - marker = json.dumps( - {"event": "hermes_demo_repair", "incident_id": incident_id, "message": _REPAIR_MESSAGE}, - separators=(",", ":"), - ) - namespace = str(cfg.get("namespace") or "") - fixture = str(cfg.get("fixture_configmap") or "hermes-triage-demo-fixture") - patch = '{"data":{"state":"healthy"}}' - script = ( - f"kubectl -n {namespace} patch configmap {fixture} " - f"--type merge -p '{patch}' && echo '{marker}'" - ) - return { - "apiVersion": "batch/v1", - "kind": "Job", - "metadata": { - "name": job_name, - "namespace": str(cfg.get("namespace") or ""), - "labels": { - "atlas.bstein.dev/trigger": "ariadne", - "app.kubernetes.io/part-of": "hermes-triage-demo", - }, + logger.info( + "hermes demo fixture repair", + extra={ + "event": "hermes_autotriage_repair", + "status": "ok" if succeeded else "error", + "target": target, + "incident_id": incident_id, }, - "spec": { - "backoffLimit": 0, - "ttlSecondsAfterFinished": _JOB_TTL_SECONDS, - "template": { - "metadata": {"labels": {"app.kubernetes.io/part-of": "hermes-triage-demo"}}, - "spec": { - "restartPolicy": "Never", - "serviceAccountName": "hermes-demo-repair", - "nodeSelector": {"node-role.kubernetes.io/worker": "true"}, - "containers": [ - { - "name": "repair", - "image": str(cfg.get("image") or ""), - "command": ["sh", "-c", script], - } - ], - }, - }, - }, - } + ) + return {"action": _REPAIR_ACTION, "target": target, "succeeded": succeeded, "error": error} -def _wait_for_completion(namespace: str, job_name: str, timeout_seconds: float) -> dict[str, Any]: - """Poll the repair Job until it succeeds, fails, or times out.""" +def _patch_error(exc: Exception) -> str: + """Render a short reason for a failed ConfigMap patch.""" - deadline = time.time() + timeout_seconds - while time.time() < deadline: - try: - job = get_json(f"/apis/batch/v1/namespaces/{namespace}/jobs/{job_name}") - except Exception as exc: - return {"job_name": job_name, "succeeded": False, "error": f"job status read failed: {exc}"} - status = job.get("status") if isinstance(job.get("status"), dict) else {} - if _int_value(status.get("succeeded")) > 0: - return {"job_name": job_name, "succeeded": True, "error": None} - if _int_value(status.get("failed")) > 0: - return {"job_name": job_name, "succeeded": False, "error": "repair job failed"} - time.sleep(_POLL_INTERVAL_SECONDS) - return { - "job_name": job_name, - "succeeded": False, - "error": f"repair job timeout after {timeout_seconds}s", - } + code = _status_code(exc) + if code is not None: + return f"configmap patch http {code}" + return f"configmap patch failed: {exc}" def _jenkins_client_kwargs(config: Any) -> dict[str, Any]: @@ -196,12 +152,3 @@ def _number(value: Any, default: float) -> float: return float(value) except (TypeError, ValueError): return default - - -def _int_value(value: Any) -> int: - """Coerce a value to int, defaulting to zero.""" - - try: - return int(value) - except (TypeError, ValueError): - return 0 diff --git a/ariadne/settings.py b/ariadne/settings.py index 862cf3e..5117f06 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -195,7 +195,6 @@ class Settings: hermes_run_timeout_seconds: float hermes_demo_namespace: str hermes_demo_fixture_configmap: str - hermes_repair_image: str hermes_code_enabled: bool hermes_code_job: str hermes_code_owner: str diff --git a/ariadne/settings_sections.py b/ariadne/settings_sections.py index 32827c9..769c0e3 100644 --- a/ariadne/settings_sections.py +++ b/ariadne/settings_sections.py @@ -298,7 +298,6 @@ def _hermes_autotriage_config() -> dict[str, Any]: "hermes_run_timeout_seconds": _env_float("ARIADNE_HERMES_RUN_TIMEOUT_SECONDS", 420.0), "hermes_demo_namespace": _env("ARIADNE_HERMES_DEMO_NAMESPACE", "hermes-triage-demo"), "hermes_demo_fixture_configmap": _env("ARIADNE_HERMES_DEMO_FIXTURE_CONFIGMAP", "hermes-triage-demo-fixture"), - "hermes_repair_image": _env("ARIADNE_HERMES_REPAIR_IMAGE", "bitnami/kubectl@sha256:554ab88b1858e8424c55de37ad417b16f2a0e65d1607aa0f3fe3ce9b9f10b131"), } diff --git a/tests/hermes_autotriage_harness.py b/tests/hermes_autotriage_harness.py index f7d55d7..bb14834 100644 --- a/tests/hermes_autotriage_harness.py +++ b/tests/hermes_autotriage_harness.py @@ -64,7 +64,6 @@ def _settings(**overrides) -> SimpleNamespace: # type: ignore[no-untyped-def] "hermes_run_timeout_seconds": 420.0, "hermes_demo_namespace": "hermes-triage-demo", "hermes_demo_fixture_configmap": "hermes-triage-demo-fixture", - "hermes_repair_image": "busybox:1.37", "hermes_code_enabled": False, "hermes_code_job": "hermes-code-demo", "hermes_code_owner": "bstein", @@ -217,7 +216,12 @@ def _prepare( # type: ignore[no-untyped-def] # noqa: PLR0913 calls["repairs"].append((repair_cfg, incident_id, build_number)) if repair is not None: return repair - return {"job_name": "hermes-demo-repair-12", "succeeded": True, "error": None} + return { + "action": "configmap_patch", + "target": "hermes-triage-demo/hermes-triage-demo-fixture", + "succeeded": True, + "error": None, + } def fake_trigger_rebuild(rebuild_cfg, job): # type: ignore[no-untyped-def] calls["rebuilds"].append(job) diff --git a/tests/test_hermes_autotriage.py b/tests/test_hermes_autotriage.py index 8281f05..f4f7060 100644 --- a/tests/test_hermes_autotriage.py +++ b/tests/test_hermes_autotriage.py @@ -56,7 +56,8 @@ def test_new_failure_full_happy_path(monkeypatch) -> None: summary = module.run_hermes_autotriage(env.storage) job_summary = summary["jobs"][JOB] assert job_summary["status"] == "awaiting_rebuild" - assert job_summary["repair_job"] == "hermes-demo-repair-12" + assert job_summary["repair"] == "configmap_patch" + assert job_summary["target"] == "hermes-triage-demo/hermes-triage-demo-fixture" assert _statuses(env.storage) == ["detected", "diagnosed", "repairing", "awaiting_rebuild"] actions = _events(env.storage, module.ACTION_EVENT_TYPE) assert [action["result"] for action in actions] == ["requested", "accepted", "executed"] @@ -78,12 +79,21 @@ def test_new_failure_full_happy_path(monkeypatch) -> None: { "namespace": "hermes-triage-demo", "fixture_configmap": "hermes-triage-demo-fixture", - "image": "busybox:1.37", }, INCIDENT_ID, 12, ) ] + assert actions[-1]["detail"] == { + "repair": "configmap_patch", + "target": "hermes-triage-demo/hermes-triage-demo-fixture", + } + awaiting = _events(env.storage, module.INCIDENT_EVENT_TYPE)[-1] + assert awaiting["phase"] == { + "action": "repair_demo_fixture", + "repair": "configmap_patch", + "target": "hermes-triage-demo/hermes-triage-demo-fixture", + } assert env.calls["rebuilds"] == [JOB] assert _counter("repair_demo_fixture", "success") == success_before + 1.0 assert _gauge("12", "awaiting_rebuild") == 1.0 @@ -224,13 +234,18 @@ def test_repair_failure_marks_failed_and_human(monkeypatch) -> None: failed_before = _counter("repair_demo_fixture", "failed") env = _prepare( monkeypatch, - repair={"job_name": "hermes-demo-repair-12", "succeeded": False, "error": "repair job failed"}, + repair={ + "action": "configmap_patch", + "target": "hermes-triage-demo/hermes-triage-demo-fixture", + "succeeded": False, + "error": "configmap patch http 403", + }, ) summary = module.run_hermes_autotriage(env.storage) assert summary["jobs"][JOB] == { "status": "failed", "incident_id": INCIDENT_ID, - "reason": "repair job failed", + "reason": "configmap patch http 403", } assert _statuses(env.storage) == ["detected", "diagnosed", "repairing", "failed"] actions = _events(env.storage, module.ACTION_EVENT_TYPE) diff --git a/tests/test_hermes_autotriage_repair.py b/tests/test_hermes_autotriage_repair.py index d2d0f14..db3726b 100644 --- a/tests/test_hermes_autotriage_repair.py +++ b/tests/test_hermes_autotriage_repair.py @@ -2,23 +2,13 @@ from __future__ import annotations from types import SimpleNamespace +from ariadne.k8s import client as k8s_client from ariadne.services import hermes_autotriage_repair as module INCIDENT_ID = "hermes-triage-demo/12" - - -class FakeClock: - def __init__(self) -> None: - self.now = 0.0 - self.sleeps: list[float] = [] - - def time(self) -> float: - return self.now - - def sleep(self, seconds: float) -> None: - self.sleeps.append(seconds) - self.now += seconds +CONFIGMAP_PATH = "/api/v1/namespaces/hermes-triage-demo/configmaps/hermes-triage-demo-fixture" +TARGET = "hermes-triage-demo/hermes-triage-demo-fixture" class FakeResponse: @@ -30,7 +20,6 @@ def _cfg(**overrides) -> dict: # type: ignore[no-untyped-def] base = { "namespace": "hermes-triage-demo", "fixture_configmap": "hermes-triage-demo-fixture", - "image": "bitnami/kubectl@sha256:554ab88b1858e8424c55de37ad417b16f2a0e65d1607aa0f3fe3ce9b9f10b131", } base.update(overrides) return base @@ -47,110 +36,108 @@ def _jenkins_settings(**overrides) -> SimpleNamespace: # type: ignore[no-untype return SimpleNamespace(**values) -def _install_k8s(monkeypatch, statuses, post_exc=None) -> dict: # type: ignore[no-untyped-def] - calls: dict = {"posts": [], "gets": []} - queue = list(statuses) +def _install_k8s(monkeypatch, patch_exc=None) -> dict: # type: ignore[no-untyped-def] + """Record every Kubernetes call the repair path makes.""" + + calls: dict = {"patches": [], "posts": [], "gets": []} + + def fake_patch(path, payload): # type: ignore[no-untyped-def] + calls["patches"].append((path, payload)) + if patch_exc is not None: + raise patch_exc + return {"metadata": {"name": "hermes-triage-demo-fixture"}} def fake_post(path, payload): # type: ignore[no-untyped-def] calls["posts"].append((path, payload)) - if post_exc is not None: - raise post_exc - return {"metadata": {"name": payload["metadata"]["name"]}} + return {} def fake_get(path): # type: ignore[no-untyped-def] calls["gets"].append(path) - item = queue.pop(0) if queue else {"status": {"active": 1}} - if isinstance(item, Exception): - raise item - return item + return {} - monkeypatch.setattr(module, "post_json", fake_post) - monkeypatch.setattr(module, "get_json", fake_get) - clock = FakeClock() - monkeypatch.setattr(module, "time", clock) - calls["clock"] = clock + monkeypatch.setattr(module, "patch_json", fake_patch) + monkeypatch.setattr(k8s_client, "post_json", fake_post) + monkeypatch.setattr(k8s_client, "get_json", fake_get) return calls -def test_repair_job_payload_contract(monkeypatch) -> None: - calls = _install_k8s(monkeypatch, [{"status": {"succeeded": 1}}]) +def _http_error(status_code: int) -> Exception: + error = RuntimeError(f"http {status_code}") + error.response = SimpleNamespace(status_code=status_code) # type: ignore[attr-defined] + return error + + +def test_execute_repair_patches_the_fixture_configmap(monkeypatch) -> None: + calls = _install_k8s(monkeypatch) result = module.execute_repair(_cfg(), INCIDENT_ID, 12) - assert result == {"job_name": "hermes-demo-repair-12", "succeeded": True, "error": None} - path, payload = calls["posts"][0] - assert path == "/apis/batch/v1/namespaces/hermes-triage-demo/jobs" - assert payload["apiVersion"] == "batch/v1" - assert payload["kind"] == "Job" - assert payload["metadata"]["name"] == "hermes-demo-repair-12" - assert payload["metadata"]["namespace"] == "hermes-triage-demo" - assert payload["metadata"]["labels"] == { - "atlas.bstein.dev/trigger": "ariadne", - "app.kubernetes.io/part-of": "hermes-triage-demo", + assert result == { + "action": "configmap_patch", + "target": TARGET, + "succeeded": True, + "error": None, } - spec = payload["spec"] - assert spec["backoffLimit"] == 0 - assert spec["ttlSecondsAfterFinished"] == 3600 - pod = spec["template"]["spec"] - assert pod["restartPolicy"] == "Never" - assert pod["nodeSelector"] == {"node-role.kubernetes.io/worker": "true"} - assert pod["serviceAccountName"] == "hermes-demo-repair" - container = pod["containers"][0] - assert container["image"].startswith("bitnami/kubectl@sha256:") - assert container["command"][:2] == ["sh", "-c"] - script = container["command"][2] - assert "kubectl -n hermes-triage-demo patch configmap hermes-triage-demo-fixture" in script - assert '{"data":{"state":"healthy"}}' in script - assert '"event":"hermes_demo_repair"' in script - assert f'"incident_id":"{INCIDENT_ID}"' in script - assert '"message":"fixture state reset to healthy"' in script - assert "volumeMounts" not in container - assert "volumes" not in pod + assert calls["patches"] == [(CONFIGMAP_PATH, {"data": {"state": "healthy"}})] -def test_execute_repair_waits_through_active_polls(monkeypatch) -> None: - calls = _install_k8s( - monkeypatch, - [{"status": {"active": 1}}, {"status": {"succeeded": 1}}], - ) +def test_execute_repair_creates_no_kubernetes_job(monkeypatch) -> None: + calls = _install_k8s(monkeypatch) + module.execute_repair(_cfg(), INCIDENT_ID, 12) + + assert calls["posts"] == [] + assert calls["gets"] == [] + assert not any("/jobs" in path for path, _ in calls["patches"]) + assert not hasattr(module, "_job_payload") + assert not hasattr(module, "_wait_for_completion") + + +def test_execute_repair_reports_non_2xx_status(monkeypatch) -> None: + calls = _install_k8s(monkeypatch, patch_exc=_http_error(403)) result = module.execute_repair(_cfg(), INCIDENT_ID, 12) - assert result["succeeded"] is True - assert len(calls["gets"]) == 2 - assert calls["clock"].sleeps == [2.0] - -def test_execute_repair_job_failure(monkeypatch) -> None: - _install_k8s(monkeypatch, [{"status": {"failed": 1}}]) - result = module.execute_repair(_cfg(), INCIDENT_ID, 12) - assert result == {"job_name": "hermes-demo-repair-12", "succeeded": False, "error": "repair job failed"} - - -def test_execute_repair_timeout(monkeypatch) -> None: - _install_k8s(monkeypatch, []) - result = module.execute_repair(_cfg(wait_timeout_seconds=6), INCIDENT_ID, 12) + assert result["action"] == "configmap_patch" + assert result["target"] == TARGET assert result["succeeded"] is False - assert result["error"] == "repair job timeout after 6.0s" + assert result["error"] == "configmap patch http 403" + assert len(calls["patches"]) == 1 -def test_execute_repair_duplicate_job(monkeypatch) -> None: - conflict = RuntimeError("conflict") - conflict.response = SimpleNamespace(status_code=409) # type: ignore[attr-defined] - _install_k8s(monkeypatch, [], post_exc=conflict) +def test_execute_repair_survives_a_transport_error(monkeypatch) -> None: + _install_k8s(monkeypatch, patch_exc=RuntimeError("connection reset by peer")) result = module.execute_repair(_cfg(), INCIDENT_ID, 12) - assert result == {"job_name": "hermes-demo-repair-12", "succeeded": False, "error": "duplicate job"} - -def test_execute_repair_create_error(monkeypatch) -> None: - _install_k8s(monkeypatch, [], post_exc=ValueError("nope")) - result = module.execute_repair(_cfg(), INCIDENT_ID, 12) assert result["succeeded"] is False - assert result["error"] == "job create failed: nope" + assert result["error"] == "configmap patch failed: connection reset by peer" -def test_execute_repair_status_read_error(monkeypatch) -> None: - _install_k8s(monkeypatch, [RuntimeError("api down")]) - result = module.execute_repair(_cfg(), INCIDENT_ID, 12) +def test_execute_repair_without_namespace_issues_no_request(monkeypatch) -> None: + calls = _install_k8s(monkeypatch) + result = module.execute_repair(_cfg(namespace=""), INCIDENT_ID, 12) + assert result["succeeded"] is False - assert result["error"] == "job status read failed: api down" + assert result["error"] == "fixture target incomplete" + assert calls["patches"] == [] + + +def test_execute_repair_without_configmap_name_issues_no_request(monkeypatch) -> None: + calls = _install_k8s(monkeypatch) + result = module.execute_repair(_cfg(fixture_configmap=None), INCIDENT_ID, 12) + + assert result["succeeded"] is False + assert result["error"] == "fixture target incomplete" + assert calls["patches"] == [] + + +def test_execute_repair_is_idempotent_across_repeats(monkeypatch) -> None: + calls = _install_k8s(monkeypatch) + first = module.execute_repair(_cfg(), INCIDENT_ID, 12) + second = module.execute_repair(_cfg(), INCIDENT_ID, 12) + + assert first == second + assert calls["patches"] == [ + (CONFIGMAP_PATH, {"data": {"state": "healthy"}}), + (CONFIGMAP_PATH, {"data": {"state": "healthy"}}), + ] def _install_http(monkeypatch, response=None, exc=None) -> dict: # type: ignore[no-untyped-def]