fix(host_agent): disks behind a RAID controller each keep their own name (#5806)

smartctl reports every disk behind a PERC/MegaRAID controller as /dev/bus/0
with type megaraid,N; the helper named a drive by its path alone, so all of
VDNT-Docker02's and Docker03's disks read as one drive "0", each overwriting
the last. The slot is now part of the name (bus0-megaraid3), and Steward gives
a repeated name an ordinal so two drives never share a series. Agent 1.20.1.

fix(ansible): Steward stopping mid-run logs no traceback (#5749)

The run's done-callback called exception() on a cancelled task, which raises;
it now logs a run cut short by shutdown at INFO. A finished run's in-memory
state is dropped by a timer rather than a sleeping task, which shutdown found
still pending.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/plugins/host-agent-design.md

@@ -266,7 +266,7 @@ The figures are recorded at most once every 30 s, whichever manager sent them, s
 
 A maintenance job on a swarm node opens a second window over `docker` / `swarm` beside its host's (`maintenance._swarm_window`). An alert the drain or reboot causes then waits for the job, and one still firing when the window ends is sent then.
 
-### Drive health (1.20.0, #5789)
+### Drive health (1.20.0, #5789; 1.20.1, #5806)
 
 Reading SMART needs root, so the root helper does it, as the one action on its fixed list that only reads: `drives`. Hourly, and at start, the agent leaves a `drives` request. The helper runs `smartctl --scan-open` to find the drives itself (the request names none), reads each with `smartctl --json -a -d <type> <dev>`, and writes a summary to `/var/lib/steward-maint/drives.json` (0644). It writes no job result, because a drive read is not a maintenance job. The agent's newest copy rides every sample, as updates do.
 
@@ -277,7 +277,7 @@ smartctl ≥ 7.0 answers in JSON, the same on every distribution and for SATA, S
 - reallocated / pending / offline-uncorrectable sectors for ATA, and life used where the drive reports it;
 - grown defects for SCSI.
 
-A drive with no SMART (a virtual disk, a bridge that hides it) reads `available: false`, and one smartctl couldn't open carries `error`.
+A drive with no SMART (a virtual disk, a bridge that hides it) reads `available: false`, and one smartctl couldn't open carries `error`. Disks behind a RAID controller share one path (`/dev/bus/0`) and differ by the slot in their type (`megaraid,3`), so the slot is part of the name: `bus0-megaraid3` (1.20.1, #5806). Steward also gives a repeated name an ordinal (`0#2`), so two drives never share a series.
 
 Steward judges the figures in `drives.py`, so the judgment can change without an agent update:
 

plugins/host_agent/agent.py

@@ -22,7 +22,7 @@ from collections import deque
 from concurrent.futures import ThreadPoolExecutor, wait as wait_futures
 from datetime import datetime, timezone
 
-AGENT_VERSION = "1.20.0"
+AGENT_VERSION = "1.20.1"
 
 # Default path to the local Docker Engine socket. Overridable via the
 # `docker_socket` config key; collection is silently skipped if it's absent or

plugins/host_agent/drives.py

@@ -69,9 +69,20 @@ def judge(d: dict) -> dict:
 
 
 def drives_of(report) -> list[dict]:
-    """The report's drives, judged, what needs attention first."""
+    """The report's drives, judged, what needs attention first. A name seen
+    twice gets an ordinal, so two drives never share one series: agent
+    1.20.0 named every disk behind a RAID controller "0" (#5806)."""
     drives = report.get("drives") if isinstance(report, dict) else None
-    judged = [judge(d) for d in drives or [] if isinstance(d, dict) and d.get("device")]
+    judged, seen = [], {}
+    for d in drives or []:
+        if not isinstance(d, dict) or not d.get("device"):
+            continue
+        d = judge(d)
+        name = str(d["device"])
+        seen[name] = seen.get(name, 0) + 1
+        if seen[name] > 1:
+            d["device"] = f"{name}#{seen[name]}"
+        judged.append(d)
     return sorted(judged, key=lambda d: (_ORDER[d["state"]], str(d["device"])))
 
 

plugins/host_agent/files/maint.py

@@ -199,9 +199,21 @@ def _int(v):
     return v if isinstance(v, int) and not isinstance(v, bool) else None
 
 
-def drive_summary(name: str, status, data) -> dict:
+def drive_name(name: str, kind) -> str:
+    """A drive's name, unique on its host. Disks behind a RAID controller
+    share one path (/dev/bus/0) and differ only by the slot in their type
+    (megaraid,3; cciss,0), so the slot is part of the name (#5806)."""
+    base = os.path.basename(name)
+    if name.startswith("/dev/bus/"):
+        base = "bus" + base
+    if isinstance(kind, str) and "," in kind:
+        base += "-" + re.sub(r"[^A-Za-z0-9]", "", kind.split("+")[-1])
+    return base
+
+
+def drive_summary(name: str, status, data, kind=None) -> dict:
     """The few figures Steward reads from one drive's `smartctl -a` JSON."""
-    out = {"device": os.path.basename(name)}
+    out = {"device": drive_name(name, kind)}
     if not isinstance(data, dict) or status is None or status & _SMARTCTL_UNREAD:
         messages = ((data.get("smartctl") or {}).get("messages") or []) if isinstance(data, dict) else []
         text = "; ".join(str(m.get("string", "")) for m in messages if isinstance(m, dict))
@@ -267,7 +279,7 @@ def do_drives() -> dict:
             continue
         args = ["-a", name] if not isinstance(kind, str) else ["-a", "-d", kind, name]
         status, data = smartctl_json(args)
-        drives.append(drive_summary(name, status, data))
+        drives.append(drive_summary(name, status, data, kind))
     return {**base, "drives": drives}
 
 

steward/ansible/executor.py

@@ -182,14 +182,17 @@ def deregister_listener(run_id: str, q: asyncio.Queue) -> None:
         listeners.remove(q)
 
 
+def _drop_run_state(run_id: str) -> None:
+    _run_lines.pop(run_id, None)
+    _run_done.pop(run_id, None)
+
+
 def _schedule_cleanup(run_id: str) -> None:
-    """Drop a finished run's in-memory state after a grace window."""
+    """Drop a finished run's in-memory state after a grace window. A timer,
-    async def _cleanup() -> None:
+    not a task (#5749): a run cut short by shutdown schedules this as the
-        await asyncio.sleep(_MEM_TTL_SECS)
+    loop closes, and a sleeping task would be destroyed while pending."""
-        _run_lines.pop(run_id, None)
-        _run_done.pop(run_id, None)
     try:
-        asyncio.create_task(_cleanup())
+        asyncio.get_running_loop().call_later(_MEM_TTL_SECS, _drop_run_state, run_id)
     except RuntimeError:
         pass  # no running loop (e.g. tests) — fine to skip
 

steward/ansible/runner.py

@@ -109,9 +109,18 @@ async def trigger_run(
             connection=connection, secret_vars=secret_vars, redact=redact,
         )
     )
-    task.add_done_callback(
+    task.add_done_callback(run_ended(app, run_id))
-        lambda t: t.exception() and app.logger.error(
-            "Ansible run %s raised: %s", run_id, t.exception()
-        )
-    )
     return run, source, None
+
+
+def run_ended(app, run_id: str):
+    """A run task's done-callback: an error is logged as one. A run cut short
+    by Steward stopping is not an error (#5749): `exception()` would raise
+    CancelledError on it, and the next start marks the run interrupted."""
+    def callback(t: asyncio.Task) -> None:
+        if t.cancelled():
+            app.logger.info("Ansible run %s interrupted: Steward is stopping; "
+                            "it is marked interrupted at the next start", run_id)
+        elif t.exception() is not None:
+            app.logger.error("Ansible run %s raised: %s", run_id, t.exception())
+    return callback

tests/plugins/host_agent/test_drive_health.py

@@ -80,6 +80,38 @@ def test_a_drive_with_no_smart_and_one_that_can_t_be_opened_say_so(maint):
     assert maint.drive_summary("/dev/sdd", None, None)["error"] == "smartctl couldn't read it"
 
 
+@pytest.mark.parametrize("name, kind, device", [
+    ("/dev/sda", "sat", "sda"),
+    ("/dev/nvme0", "nvme", "nvme0"),
+    ("/dev/bus/0", "megaraid,3", "bus0-megaraid3"),     # VDNT-Docker02's PERC (#5806)
+    ("/dev/bus/0", "sat+megaraid,12", "bus0-megaraid12"),
+    ("/dev/sg1", "cciss,0", "sg1-cciss0"),
+    ("/dev/sdc", None, "sdc"),
+])
+def test_each_drive_has_a_name_of_its_own(maint, name, kind, device):
+    assert maint.drive_name(name, kind) == device
+
+
+def test_disks_behind_one_controller_stay_apart(maint, monkeypatch):
+    def fake(args):
+        if args == ["--scan-open"]:
+            return 0, {"devices": [{"name": "/dev/bus/0", "type": "megaraid,0"},
+                                   {"name": "/dev/bus/0", "type": "megaraid,1"}]}
+        return 0, ATA
+    monkeypatch.setattr(maint, "smartctl_version", lambda: (7, 4))
+    monkeypatch.setattr(maint, "smartctl_json", fake)
+    assert [d["device"] for d in maint.do_drives()["drives"]] == ["bus0-megaraid0", "bus0-megaraid1"]
+
+
+def test_a_name_seen_twice_is_still_two_drives():
+    """What agent 1.20.0 sent from a PERC: every disk named "0"."""
+    report = {"drives": [{"device": "0", "passed": True}, {"device": "0", "passed": False},
+                         {"device": "0", "passed": True}]}
+    assert sorted(d["device"] for d in dh.drives_of(report)) == ["0", "0#2", "0#3"]
+    rows = {(m, r) for m, r, _ in dh.metric_rows(report, "box") if m == "drive_failing"}
+    assert len(rows) == 3
+
+
 def test_a_failing_disk_s_figures_still_count(maint):
     """Exit bit 3 says the disk is failing; its JSON is the point, not an error."""
     failing = {**ATA, "smart_status": {"passed": False}}

tests/test_ansible_run_end.py

@@ -0,0 +1,52 @@
+"""#5749: a redeploy during an Ansible run logged two CancelledError tracebacks
+from the run's done-callback and two "Task was destroyed but it is pending!"
+lines for its in-memory cleanup. Steward stopping mid-run is not an error: the
+next start marks the run interrupted."""
+import asyncio
+import logging
+
+from steward.ansible import executor
+from steward.ansible.runner import run_ended
+
+
+class _App:
+    logger = logging.getLogger("test.ansible.run_end")
+
+
+async def test_a_run_cut_short_by_shutdown_logs_no_error(caplog):
+    async def run():
+        await asyncio.sleep(60)
+
+    task = asyncio.create_task(run())
+    task.add_done_callback(run_ended(_App, "r1"))
+    await asyncio.sleep(0)
+    task.cancel()
+    with caplog.at_level(logging.INFO, logger="test.ansible.run_end"):
+        await asyncio.gather(task, return_exceptions=True)
+        await asyncio.sleep(0)                      # done-callbacks run on the next turn
+    assert [r.levelname for r in caplog.records] == ["INFO"]
+    assert "Ansible run r1 interrupted" in caplog.records[0].getMessage()
+
+
+async def test_a_run_that_raised_is_still_an_error(caplog):
+    async def run():
+        raise RuntimeError("ansible-playbook vanished")
+
+    task = asyncio.create_task(run())
+    task.add_done_callback(run_ended(_App, "r2"))
+    with caplog.at_level(logging.INFO, logger="test.ansible.run_end"):
+        await asyncio.gather(task, return_exceptions=True)
+        await asyncio.sleep(0)
+    assert [(r.levelname, r.getMessage()) for r in caplog.records] == [
+        ("ERROR", "Ansible run r2 raised: ansible-playbook vanished")]
+
+
+async def test_a_finished_run_s_state_is_dropped_by_a_timer_not_a_task(monkeypatch):
+    """A task left sleeping is what shutdown found pending; a timer isn't one."""
+    monkeypatch.setattr(executor, "_MEM_TTL_SECS", 0.01)
+    executor._run_lines["r3"] = ["ok"]
+    before = asyncio.all_tasks()
+    executor._schedule_cleanup("r3")
+    assert asyncio.all_tasks() == before
+    await asyncio.sleep(0.05)
+    assert "r3" not in executor._run_lines