phase: 22_disks_hide_unconfigured
All green. Final report: **Verified (phase 22 final pass — both task files already in `complete/`)** - `api._disk_visible` (`is not False`) is the single gate, applied to the assembled host list in `/api/disks/snapshot` **and** `/api/summary`'s disks block before the fleet sum and the rows; `sources/disks.py`, `page-disks.js`, `poller.py`, `store.py` and `alerts.py` are byte-unchanged (`git diff --stat`: only `api.py`, `test_api.py`, docs). - Re-ran the live browser pass `data/logs/22-task02-disks-live.cjs` against the running dev server: **34/34 PASS** — no gateway card/why-line, `2/2 hosts answering`, fleet header reads the API's `2 of 2` / `17-17-0-1`, deskwork `nvme0n1` OK 47 °C/19 202 h/11 %, driveripper 16 live cards + `13 uncorrectable` chip, both aria-labelled plots, `/` still 3 fleet rows. Screenshots + `22-task02-api.json` refreshed. - `./scripts/dev-server.sh status`: all **nine sources `ok`**, `disks:gateway` included; live `/api/events?source=disks` still carries gateway transitions (10 of 45), proving the README/AGENTS claim the gate hides a row and not the job's history. **Completion criteria** - `false` host absent from both endpoints, `None` + `unreachable` stub present, fleet ≡ row count: **PASS** (6 pins in `tests/test_api.py`, incl. the single-hidden-host = 200-with-empty-list leg). - Live `/disks` = deskwork + driveripper, header 2 hosts, cards unchanged, screenshot in `data/logs/`: **PASS** (re-run above). - `uv run pytest` → **1477 passed, 15 skipped**; `--cov --cov-fail-under=94` → **98.94 %** (`api.py` 100 %); `ruff check src tests agent` → all passed; `pyright` → 0 errors; `bash .agents/validate.sh` → **EXIT 0**. `uv run pytest -m live --live` → **11 passed**. - No behaviour change in completed work: **PASS** (unchanged files above; live numbers match the pre-phase fleet record). **Docs** — README: `/disks` page-table row + the agent SMART paragraph now distinguish "cannot read drives" (dimmed cards kept) from "no smartctl" (host absent from the page/API lists). AGENTS.md: hermetic tally 1471→**1477**, and the `collectors.py`/`api.py` passages now note the `false` arm never reaches the `/disks` why-line while an overview alert from a hidden host's `disks` job still names it. Everything else (routes vs served table, env table vs `config.py`, commands, scripts/fixtures/tests references, masthead/POLL_MS/threshold/retention claims) **verified consistent**. **Deviations** — `NOTES.md` left empty by the task runs (both notes are tracked: mTLS → phase 24 `todo/`, the admin-links bug → fixed in phase 20 task 04); `/disks`' `#hosts-empty` copy left as pinned markup (it already names smartmontools + the disk group, so an all-hidden fleet is still told the fix); validate's advisory `fastapi 0.153.0 → 0.156.4` untouched (no dependency changed this phase). No live fleet writes of any kind. **Next pending phase:** `23_llm_rate_responsiveness`.
This commit is contained in:
+184
-2
@@ -28,7 +28,7 @@ from auth_client import sign_in
|
||||
from auth_contract import ADMIN_PASSWORD, FAST_PBKDF2
|
||||
from card_contract import HOST_SHAPES, assert_card_contract
|
||||
from dashboards import alerts, auth, certs
|
||||
from dashboards.api import MIN_PASSWORD_LENGTH, RANGE_SECONDS, parse_range
|
||||
from dashboards.api import MIN_PASSWORD_LENGTH, RANGE_SECONDS, _disk_visible, parse_range
|
||||
from dashboards.app import create_app
|
||||
from dashboards.collectors import UpstreamError
|
||||
from dashboards.config import Settings, Target
|
||||
@@ -3960,6 +3960,45 @@ def disks_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch,
|
||||
store.close()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mixed_disks_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch,
|
||||
disk_snapshot: Any) -> Iterator[TestClient]:
|
||||
"""Four configured hosts: one per answer an agent can give about `smartctl`, plus a stub.
|
||||
|
||||
Phase 22's visibility gate needs all three arms of `smartctl_available` in **one** fleet,
|
||||
because "hidden" only means something against what stays:
|
||||
|
||||
* `driveripper` — every drive read: `smartctl_available: true`
|
||||
* `deskwork` — every drive `null` with no error line: `null` (`--no-smart`, or an agent
|
||||
predating the SMART pass — it answered, it just cannot say)
|
||||
* `gateway` — every drive `null` plus the agent's own "not found" line: `false`, which is
|
||||
the one arm that hides a host (`api._disk_visible`)
|
||||
* `deadhost` — never answered: the `unreachable` stub, whose flag is `null` by shape
|
||||
|
||||
Each payload is `parse_disks_snapshot`'s own output over the fixture captures, and every
|
||||
target points at a dead port, so nothing dials and no poll can overwrite what is set here.
|
||||
"""
|
||||
db = str(tmp_path / "mixed-disks.sqlite3")
|
||||
store = SampleStore(db)
|
||||
app = create_app(Settings(db_path=db, admin_password=ADMIN_PASSWORD,
|
||||
pbkdf2_iterations=FAST_PBKDF2,
|
||||
host_targets=(Target("driveripper", DEAD_URL),
|
||||
Target("deskwork", DEAD_URL),
|
||||
Target("gateway", DEAD_URL),
|
||||
Target("deadhost", DEAD_URL))), store)
|
||||
with TestClient(app) as test_client:
|
||||
sign_in(test_client)
|
||||
now = time.time()
|
||||
for name, mode in (("driveripper", "healthy"), ("deskwork", "silent"),
|
||||
("gateway", "no_binary")):
|
||||
snapshot = disk_snapshot(name, mode=mode)
|
||||
snapshot["ts"] = now # "the agent answered just now"
|
||||
result = parse_disks_snapshot(name, snapshot, DisksCollector(), now=now)
|
||||
app.state.ctx.hub.jobs[f"disks:{name}"].state.payload = result.payload
|
||||
yield test_client
|
||||
store.close()
|
||||
|
||||
|
||||
#: (`smart` field, severity metric) for the two drive numbers the table rates — the same
|
||||
#: pair `sources/disks.py:SEVERITY_FIELDS` declares, restated here so a rename that only
|
||||
#: touches one side fails rather than colouring nothing.
|
||||
@@ -3995,7 +4034,12 @@ def series_source(body: dict[str, Any]) -> set[str]:
|
||||
|
||||
def test_disks_snapshot_answers_one_entry_per_host_with_its_physical_disks(
|
||||
disks_client) -> None:
|
||||
"""`GET /api/disks/snapshot` is per host: rows + `smart` + `smartctl_available`."""
|
||||
"""`GET /api/disks/snapshot` is per host: rows + `smart` + `smartctl_available`.
|
||||
|
||||
"Per host" means per **visible** host since phase 22 — this fleet has no `false` host in
|
||||
it, so both stub and answering host are listed; the hidden case is pinned on a fleet that
|
||||
has one (`mixed_disks_client`).
|
||||
"""
|
||||
body = disks_client.get("/api/disks/snapshot")
|
||||
assert response_ok(body)
|
||||
payload = body.json()
|
||||
@@ -4069,6 +4113,144 @@ def test_the_disks_fleet_header_is_summed_by_the_api_not_the_page(disks_client)
|
||||
assert "severity" not in fleet, "a count of drives is not a graded number"
|
||||
|
||||
|
||||
# ---- the `/disks` visibility gate (phase 22 task 01) ----------------------- #
|
||||
#
|
||||
# `TODO.md:13`: "VMs that don't have smartctl enabled disks should not show up. Gateway
|
||||
# shouldn't even be a host on the disks page." One predicate (`api._disk_visible`) over the
|
||||
# three arms of `smartctl_available`, applied to the assembled host list in both endpoints, so
|
||||
# the page's rows, that block's rows and the fleet sums cannot describe three fleets. The
|
||||
# source, the store, the `disks:<host>` job and its health record still see every host.
|
||||
|
||||
def test_the_disk_visibility_gate_reads_only_the_explicit_false() -> None:
|
||||
"""Three arms, one hide condition — and the test is `is False`, not falsy.
|
||||
|
||||
`None` means "nobody knows" (an agent predating the SMART pass, `--no-smart`, or a payload
|
||||
with no such key at all) and an agent that cannot say is not an agent that said *no*; a
|
||||
dead agent's stub stays too, because a machine that stopped answering must never read as
|
||||
"never configured" (invariant 3).
|
||||
"""
|
||||
assert _disk_visible({"smartctl_available": True}) is True
|
||||
assert _disk_visible({"smartctl_available": None}) is True
|
||||
assert _disk_visible({}) is True, "a payload that never said anything is not a 'no'"
|
||||
assert _disk_visible({"unreachable": True, "smartctl_available": None}) is True, (
|
||||
"a dead agent is not a host without smartmontools")
|
||||
assert _disk_visible({"smartctl_available": 0}) is True, (
|
||||
"falsy is not the predicate — only the value the agent answered with")
|
||||
assert _disk_visible({"smartctl_available": False}) is False
|
||||
|
||||
|
||||
def test_a_host_whose_agent_has_no_smartctl_is_not_a_host_on_the_disks_page(
|
||||
mixed_disks_client) -> None:
|
||||
"""The absent host and the two `unknown` hosts, pinned in the same response.
|
||||
|
||||
gateway's VM answers "`smartctl` not found", so it has no card and no row; deskwork's
|
||||
agent cannot say either way and deadhost's never answered, and both keep their dimmed
|
||||
`n/a` rows exactly as before this gate existed.
|
||||
"""
|
||||
payload = mixed_disks_client.get("/api/disks/snapshot").json()
|
||||
visible = {host["name"]: host for host in payload["hosts"]}
|
||||
assert list(visible) == ["deadhost", "deskwork", "driveripper"], (
|
||||
"only the host whose agent said 'no smartctl' is gone")
|
||||
assert visible["driveripper"]["smartctl_available"] is True
|
||||
assert visible["driveripper"]["disks"], "the host that stayed kept every drive row"
|
||||
assert visible["deskwork"]["smartctl_available"] is None, (
|
||||
"an agent that cannot say is still a card, with its own why-line")
|
||||
assert "unreachable" not in visible["deskwork"], "it answered — silently, but it answered"
|
||||
assert visible["deadhost"]["unreachable"] is True
|
||||
assert visible["deadhost"]["smartctl_available"] is None
|
||||
|
||||
|
||||
def test_the_disks_fleet_header_counts_only_the_hosts_the_page_lists(
|
||||
mixed_disks_client) -> None:
|
||||
"""`hosts_total` is the page's row count, and a hidden host lends it no drives."""
|
||||
payload = mixed_disks_client.get("/api/disks/snapshot").json()
|
||||
fleet = payload["fleet"]
|
||||
rows_by_host = {host["name"]: host["disks"] for host in payload["hosts"]}
|
||||
assert fleet["hosts_total"] == len(payload["hosts"]) == 3
|
||||
assert fleet["hosts_live"] == 2, "the stub host is counted and never summed"
|
||||
assert fleet["disks"] == len(rows_by_host["driveripper"]) + len(rows_by_host["deskwork"]), (
|
||||
"the hidden host's `vda` is in no fleet count")
|
||||
assert fleet["with_smart"] == len(rows_by_host["driveripper"])
|
||||
assert "severity" not in fleet
|
||||
|
||||
|
||||
def test_hiding_a_diskless_host_leaves_its_health_record_and_the_fleet_alone(
|
||||
mixed_disks_client) -> None:
|
||||
"""The gate reads a payload; it does not unwatch the machine.
|
||||
|
||||
`states` still lists `disks:gateway` (the job polls, and healthy is healthy), `/system`
|
||||
still lists gateway as a host, and `/api/health` still answers for the job — a box with no
|
||||
smartmontools is missing from the **drive** page and from nothing else (invariant 3).
|
||||
"""
|
||||
snapshot = mixed_disks_client.get("/api/disks/snapshot").json()
|
||||
summary = mixed_disks_client.get("/api/summary").json()
|
||||
assert {state["key"] for state in snapshot["states"]} == {
|
||||
"disks:deadhost", "disks:deskwork", "disks:driveripper", "disks:gateway"}
|
||||
assert "gateway" in {row["name"] for row in summary["system"]["hosts"]}, (
|
||||
"the system page lost a machine over a missing binary")
|
||||
health = {row["key"]: row for row in
|
||||
mixed_disks_client.get("/api/health").json()["sources"]}
|
||||
assert "disks:gateway" in health
|
||||
|
||||
|
||||
def test_summary_and_the_disks_page_hide_the_same_hosts(mixed_disks_client) -> None:
|
||||
"""One predicate, two endpoints: `/` and `/disks` may not list different machines."""
|
||||
body = mixed_disks_client.get("/api/summary").json()
|
||||
snapshot = mixed_disks_client.get("/api/disks/snapshot").json()
|
||||
disks = body["disks"]
|
||||
# the **set**, not the list: `/api/disks/snapshot` sorts by name, `/api/summary` answers
|
||||
# rows in hub order and always has
|
||||
assert {row["name"] for row in disks["hosts"]} == {
|
||||
host["name"] for host in snapshot["hosts"]} == {
|
||||
"deadhost", "deskwork", "driveripper"}
|
||||
assert disks["fleet"] == snapshot["fleet"], "the header would disagree with the page"
|
||||
assert disks["fleet"]["hosts_total"] == 3
|
||||
assert disks["enabled"] is True
|
||||
assert disks["fleet"]["hosts_total"] != len(body["system"]["hosts"]), (
|
||||
"four machines configured, three drive rows — the two numbers are different facts")
|
||||
# the rows that stayed carry the shape they always did, `unknown` and all (rule 2)
|
||||
rows = {row["name"]: row for row in disks["hosts"]}
|
||||
assert rows["deskwork"]["smartctl_available"] is None
|
||||
assert rows["deskwork"]["unreachable"] is False
|
||||
assert rows["deadhost"]["unreachable"] is True and rows["deadhost"]["disks"] is None
|
||||
|
||||
|
||||
def test_the_disk_visibility_gate_applies_with_only_one_host_configured(
|
||||
tmp_path: Path, disk_snapshot: Any) -> None:
|
||||
"""A fleet of nothing but a `false` host is an empty drive page, not a 404.
|
||||
|
||||
Unconfigured still means "no `HOST_TARGETS`" (a 404 that dials nothing, invariant 3);
|
||||
configured-and-hidden answers with an empty list and a zero fleet, which is what the page
|
||||
renders as its empty state.
|
||||
"""
|
||||
db = str(tmp_path / "one-hidden-host.sqlite3")
|
||||
store = SampleStore(db)
|
||||
app = create_app(Settings(db_path=db, admin_password=ADMIN_PASSWORD,
|
||||
pbkdf2_iterations=FAST_PBKDF2,
|
||||
host_targets=(Target("gateway", DEAD_URL),)), store)
|
||||
with TestClient(app) as test_client:
|
||||
sign_in(test_client)
|
||||
now = time.time()
|
||||
snapshot = disk_snapshot("gateway", mode="no_binary")
|
||||
snapshot["ts"] = now
|
||||
result = parse_disks_snapshot("gateway", snapshot, DisksCollector(), now=now)
|
||||
assert result.payload["smartctl_available"] is False, "the arm this test is about"
|
||||
app.state.ctx.hub.jobs["disks:gateway"].state.payload = result.payload
|
||||
body = test_client.get("/api/disks/snapshot")
|
||||
assert body.status_code == 200, "configured-and-hidden is not unconfigured"
|
||||
payload = body.json()
|
||||
assert payload["hosts"] == []
|
||||
assert payload["fleet"]["hosts_total"] == 0
|
||||
assert payload["fleet"]["disks"] == 0
|
||||
assert [state["key"] for state in payload["states"]] == ["disks:gateway"]
|
||||
summary = test_client.get("/api/summary").json()["disks"]
|
||||
assert summary["enabled"] is True
|
||||
assert summary["hosts"] == []
|
||||
assert summary["fleet"] == payload["fleet"], (
|
||||
"an all-hidden fleet must look the same from both endpoints")
|
||||
store.close()
|
||||
|
||||
|
||||
def test_the_disks_surface_is_gets_only_and_the_write_stays_the_ups_one(
|
||||
disks_client) -> None:
|
||||
"""Invariant 1: the app writes to an **upstream** from exactly one route."""
|
||||
|
||||
Reference in New Issue
Block a user