fix(disk-health): one physical disk must be evaluated once per run (R-335)
gates / gates (push) Successful in 9s
gates / gates (push) Successful in 9s
Found on live hardware two hours after the v0.215.0 deploy, by noticing the release's own positive observable disagreed with its own persisted artefact: the check logged '3 disk(s) evaluated' while disk-health-state.json held two records. demo-hp's c11-scratch and felhom-backup are the same NVMe and share a durable id, so one disk was walked twice per run. Not cosmetic. The loop writes a disk's record before the next entry reads it, so the second copy of an aliased disk consumed the FIRST copy's write as its prior: the disk sustained against ITSELF and reached Hiba on a first sighting, defeating truth-table row 6 — the rule that separates a one-hour benign excursion from a false critical. It would also have emitted two identical events for one drive. Latent on demo-hp only because all counters are zero. Each diskKey is now evaluated once per run. Both entries stay marked seen so neither looks like a disappeared disk, and the card still renders both rows — the dedup is about state and alerts, not display. Red-proof run and reverted: deleting the guard makes the first sighting emit Kind:2 (Hiba-from-sectors) at 8 sectors.
This commit is contained in:
@@ -1,3 +1,34 @@
|
||||
## v0.216.0 — one physical disk, one verdict (2026-08-14, R-335)
|
||||
**MinAgent: 0.129.0**
|
||||
|
||||
**Found on live hardware within two hours of the v0.215.0 deploy, by reading the artefact the release
|
||||
itself introduced.** The first two hourly checks on demo-hp logged *"3 disk(s) evaluated"* while the
|
||||
persisted `disk-health-state.json` held only **two** records. The discrepancy was the bug: demo-hp's
|
||||
`c11-scratch` and `felhom-backup` are the same physical NVMe (`/dev/nvme0n1`) and resolve to the same
|
||||
`diskKey`, so one disk was walked twice in a single run.
|
||||
|
||||
**Why that was not cosmetic.** The loop writes a disk's new record before the next entry reads it, so
|
||||
the SECOND copy of an aliased disk consumed the FIRST copy's write as its prior. The disk therefore
|
||||
**sustained against itself** and reached **Hiba on a FIRST sighting** — defeating truth-table row 6,
|
||||
the rule the whole v0.215.0 ladder is built on and the one thing standing between a one-hour benign
|
||||
excursion and a false critical alert. It would also have emitted **two identical events** for one
|
||||
drive. Nothing fired on demo-hp because all three entries are healthy with zero counters, so the fault
|
||||
was latent, not active — but any aliased disk developing a single pending sector would have gone
|
||||
straight to Hiba.
|
||||
|
||||
**Fix:** `RunDiskHealthCheck` evaluates each `diskKey` **once per run** (`internal/web/disk_health.go`).
|
||||
Both entries are still marked `seen`, so neither is mistaken for a disappeared disk, and the card still
|
||||
renders **both** storage rows — the dedup is about state and alerts, not display.
|
||||
|
||||
**Pinned by `TestDiskCheck_SameDiskTwiceIsEvaluatedOnce`**, which asserts the first sighting stays
|
||||
Figyelmeztetés and silent, that the stored prior is not this run's own write, that the second run emits
|
||||
exactly ONE event, and that the card still shows two rows. Companion red-proof run and reverted:
|
||||
deleting the guard makes the first sighting emit `Kind:2` (Hiba-from-sectors) at 8 sectors — the exact
|
||||
false critical described above.
|
||||
|
||||
This is the shape §3 of the workspace rules warns about: the release's own positive observable
|
||||
("N disks evaluated") disagreed with its own persisted artefact, and only reading BOTH exposed it.
|
||||
|
||||
## v0.215.0 — the disk alert that never sent (2026-08-14, R-328..R-333)
|
||||
**MinAgent: 0.129.0** (unchanged — every field this reads has been on the wire since agent v0.94.0/0.95.0)
|
||||
|
||||
|
||||
@@ -201,6 +201,21 @@ func (s *Server) RunDiskHealthCheck(ctx context.Context) error {
|
||||
continue
|
||||
}
|
||||
key := diskKey(d)
|
||||
// ONE physical disk can appear as SEVERAL storage entries — on demo-hp the same NVMe is both
|
||||
// `c11-scratch` and `felhom-backup`, and both resolve to the same durable id. They must be
|
||||
// evaluated ONCE per run, for two reasons:
|
||||
//
|
||||
// 1. Correctness. The loop writes this run's record before the next entry reads it, so the
|
||||
// second copy of the same disk would consume the FIRST copy's write as its prior — i.e.
|
||||
// the disk would sustain against itself and reach Hiba on a FIRST sighting, defeating the
|
||||
// entire sustain rule (truth-table row 6).
|
||||
// 2. One disk, one alert. Two entries would otherwise emit two identical events.
|
||||
//
|
||||
// Found on live hardware after the v0.215.0 deploy: the check logged "3 disk(s) evaluated"
|
||||
// while the persisted state held two records. Pinned by TestDiskCheck_SameDiskTwiceIsEvaluatedOnce.
|
||||
if seen[key] {
|
||||
continue
|
||||
}
|
||||
seen[key] = true
|
||||
prev := s.diskHealth.records[key]
|
||||
prior := s.priorFor(key)
|
||||
|
||||
@@ -554,6 +554,62 @@ func TestDiskCheck_DisappearedDiskIsForgotten(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ── One physical disk, several storage entries ──────────────────────────────────────────────────
|
||||
|
||||
// FOUND ON LIVE HARDWARE (demo-hp, 2026-08-14): the check reported "3 disk(s) evaluated" while the
|
||||
// persisted state held only TWO records, because `c11-scratch` and `felhom-backup` are the same NVMe
|
||||
// and resolve to the same durable id.
|
||||
//
|
||||
// That aliasing was a real defect, not a cosmetic one. The loop writes this run's record before the
|
||||
// next entry reads it, so the second copy of a disk consumed the FIRST copy's write as its prior —
|
||||
// the disk sustained against ITSELF and reached Hiba on a first sighting, defeating truth-table row 6
|
||||
// entirely, and emitted two identical events while doing it.
|
||||
//
|
||||
// Red-proof: delete the `if seen[key] { continue }` guard in RunDiskHealthCheck → the first run
|
||||
// reaches Fail and emits, and both assertions below fail.
|
||||
func TestDiskCheck_SameDiskTwiceIsEvaluatedOnce(t *testing.T) {
|
||||
h := newDiskHarness(t)
|
||||
// Two storage entries, ONE physical device — identical durable id, as the agent really reports it.
|
||||
twice := func() []agentapi.DiskInfo {
|
||||
sm := &agentapi.SmartSummary{Health: agentapi.SmartPassed,
|
||||
PendingSectors: smartPtr(8), OfflineUncorrectable: smartPtr(8), ReallocatedSectors: smartPtr(0)}
|
||||
return []agentapi.DiskInfo{
|
||||
{Name: "c11-scratch", Type: "local-dir", BackingDevice: "/dev/nvme0n1", DurableID: "uuid:nvme-1", Smart: sm},
|
||||
{Name: "felhom-backup", Type: "local-dir", BackingDevice: "/dev/nvme0n1", DurableID: "uuid:nvme-1", Smart: sm},
|
||||
}
|
||||
}
|
||||
h.set(twice()...)
|
||||
h.run(t)
|
||||
|
||||
// FIRST sighting: silent, and Figyelmeztetés — a disk must not sustain against its own alias.
|
||||
if n := len(h.events()); n != 0 {
|
||||
t.Fatalf("first sighting of an aliased disk must be silent, got %d: %+v", n, h.events())
|
||||
}
|
||||
rec := h.record(t, "uuid:nvme-1")
|
||||
if rec == nil {
|
||||
t.Fatal("no record for the aliased disk")
|
||||
}
|
||||
if agentapi.DiskVerdict(rec.Verdict) != agentapi.DiskVerdictWarn {
|
||||
t.Errorf("first sighting verdict = %d, want Warn — the disk sustained against its own alias",
|
||||
rec.Verdict)
|
||||
}
|
||||
if rec.PriorSawUncorrectable {
|
||||
t.Error("the prior used on a first sighting must be empty, not this run's own write")
|
||||
}
|
||||
|
||||
// SECOND run: now genuinely sustained → exactly ONE event, not one per storage entry.
|
||||
h.set(twice()...)
|
||||
h.run(t)
|
||||
if n := len(h.events()); n != 1 {
|
||||
t.Errorf("one physical disk must produce ONE event, got %d: %+v", n, h.events())
|
||||
}
|
||||
|
||||
// Both entries still render on the card — dedup is about state and alerts, not display.
|
||||
if rows := h.s.diskHealthRows(context.Background()); len(rows) != 2 {
|
||||
t.Errorf("card must still show both storage entries, got %d rows", len(rows))
|
||||
}
|
||||
}
|
||||
|
||||
// An unreachable agent changes nothing at all — no state churn, no alarm, no error.
|
||||
func TestDiskCheck_UnreachableAgentIsInert(t *testing.T) {
|
||||
h := newDiskHarness(t)
|
||||
|
||||
Reference in New Issue
Block a user