Files
felhom-controller/controller/internal/web/disk_health.go
T
admin 90f2545679
gates / gates (push) Successful in 9s
fix(disk-health): one physical disk must be evaluated once per run (R-335)
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.
2026-08-14 10:30:22 +02:00

294 lines
11 KiB
Go

package web
import (
"context"
"strconv"
"sync"
"time"
"gitea.dooplex.hu/admin/felhom-controller/internal/agentapi"
"gitea.dooplex.hu/admin/felhom-controller/internal/notify"
)
// Disk-health card + periodic degradation check (v0.169.0; ladder + persistence v0.215.0). The card
// and the check share ONE data path (the 60s-TTL-cached /disks call) and ONE verdict function
// (agentapi.DiskVerdictFor), so the chip a customer sees and the alert they receive can never
// disagree. That shared-function property is load-bearing: BOTH callers must pass the SAME prior, or
// the chip could read Hiba while the email says Figyelmeztetés. No new smartctl load — the agent
// (v0.94.0) serializes its already-computed SMART; this consumes it and feature-detects (nil Smart →
// "Nincs adat", never alarms). No global banner (CONTEXT ruling) — card + email only.
const diskCacheTTL = 60 * time.Second
type diskHealthState struct {
mu sync.Mutex
cacheAt time.Time
cacheResp agentapi.DisksResponse
cacheErr error
cacheSet bool
// records is the PERSISTED per-disk observation + alert history (v0.215.0), keyed by diskKey.
// It replaced the in-memory-only baseline, whose loss on restart meant a box that rebooted while
// a disk was already failing never alerted again. UNKNOWN is never recorded and never deletes an
// existing record. See disk_health_state.go.
records map[string]*diskRecord
loaded bool
// clock is the injectable time source (nil → time.Now). The 24h re-alert cooldown is computed
// from it, so it must be injectable or the cooldown cannot be tested without sleeping.
clock func() time.Time
}
// now reads the injectable clock.
func (st *diskHealthState) now() time.Time {
if st.clock != nil {
return st.clock()
}
return time.Now()
}
// DiskHealthRow is one rendered card row.
type DiskHealthRow struct {
Label string
ChipLabel string // "Rendben" | "Figyelmeztetés" | "Hiba" | "Nincs adat"
ChipClass string // design-system state-text-* color
Temp string // e.g. "34" — empty when the device reports no temperature
}
// cachedDisks fetches /disks through a 60s in-process TTL cache so dashboard refresh-spam cannot
// smartctl-storm the host (the agent recomputes SMART per /disks call). disksFn is the test seam
// (nil → the real agent client). Never blocks the page: the caller treats an error as "Nincs adat".
func (s *Server) cachedDisks(ctx context.Context) (agentapi.DisksResponse, error) {
s.diskHealth.mu.Lock()
defer s.diskHealth.mu.Unlock()
if s.diskHealth.cacheSet && time.Since(s.diskHealth.cacheAt) < diskCacheTTL {
return s.diskHealth.cacheResp, s.diskHealth.cacheErr
}
resp, err := s.fetchDisks(ctx)
s.diskHealth.cacheResp, s.diskHealth.cacheErr, s.diskHealth.cacheAt, s.diskHealth.cacheSet = resp, err, time.Now(), true
return resp, err
}
func (s *Server) fetchDisks(ctx context.Context) (agentapi.DisksResponse, error) {
if s.disksFn != nil {
return s.disksFn(ctx)
}
client, err := s.agentClient()
if err != nil {
return agentapi.DisksResponse{}, err
}
return client.Disks(ctx)
}
// isPhysicalDisk reports whether a target is a physical disk SMART applies to. Logical/network
// storage (PBS, LVM-thin, NFS, CIFS) is excluded even though the agent may default its SMART to
// UNKNOWN — those are storage abstractions, not disks, and must not clutter the disk-health card.
func isPhysicalDisk(d agentapi.DiskInfo) bool {
switch d.Type {
case "pbs", "lvmthin", "nfs", "cifs":
return false
}
return d.BackingDevice != "" || d.Smart != nil
}
// diskDisplayLabel is the customer-facing disk label (PVE name + a Hungarian speed hint when known).
func diskDisplayLabel(d agentapi.DiskInfo) string {
// Prefer the device model (agent v0.95.0) over the raw storage name/UUID — a customer reads
// "TOSHIBA MQ04ABF100", not "47a3361a-…". Falls back to Name (+ speed hint) on an older agent.
if d.Smart != nil && d.Smart.ModelName != nil && *d.Smart.ModelName != "" {
return *d.Smart.ModelName
}
switch d.Class {
case "fast":
return d.Name + " (gyors)"
case "slow":
return d.Name + " (lassú)"
default:
return d.Name
}
}
func diskChipClass(v agentapi.DiskVerdict) string {
switch v {
case agentapi.DiskVerdictOK:
return "state-text-run"
case agentapi.DiskVerdictWarn:
return "state-text-warn"
case agentapi.DiskVerdictFail:
return "state-text-crit"
default:
return "state-text-neutral"
}
}
// diskHealthRows builds the "Lemezek állapota" card rows for the physical disks. Never errors: an
// unreachable agent yields nil and the card renders its empty state.
func (s *Server) diskHealthRows(ctx context.Context) []DiskHealthRow {
resp, err := s.cachedDisks(ctx)
if err != nil {
return nil
}
// The card must consult the SAME persisted prior the check uses, or the chip and the email can
// disagree — which is the exact property the shared verdict function exists to guarantee. Read
// only: rendering a page never writes reporting history.
s.diskHealth.mu.Lock()
s.loadDiskStateLocked()
priors := make(map[string]agentapi.DiskPrior, len(resp.Disks))
for _, d := range resp.Disks {
priors[diskKey(d)] = s.cardPriorFor(diskKey(d))
}
s.diskHealth.mu.Unlock()
var rows []DiskHealthRow
for _, d := range resp.Disks {
if !isPhysicalDisk(d) {
continue
}
v := agentapi.DiskVerdictFor(d.Smart, priors[diskKey(d)])
row := DiskHealthRow{Label: diskDisplayLabel(d), ChipLabel: v.Label(), ChipClass: diskChipClass(v)}
if d.Smart != nil && d.Smart.TemperatureC != nil {
row.Temp = strconv.Itoa(*d.Smart.TemperatureC)
}
rows = append(rows, row)
}
return rows
}
// diskKey is a disk's stable identity across checks (durable id preferred; falls back to name).
func diskKey(d agentapi.DiskInfo) string {
if d.DurableID != "" {
return d.DurableID
}
if d.WipeDurableID != "" {
return d.WipeDurableID
}
return "name:" + d.Name
}
// RunDiskHealthCheck is the periodic job. It emits disk_health_degraded per the v0.215.0 decision
// rules in diskAlertDecision: escalation against the last ALERTED verdict fires immediately, a disk
// already at Hiba re-alerts when it has both doubled its unreadable-sector count AND waited out the
// 24h cooldown, and everything else is silent.
//
// Behaviours preserved verbatim from v0.169.0:
// - the first verdict ever for a disk baselines silently;
// - UNKNOWN is excluded both directions — never recorded, never a transition endpoint, and it does
// NOT delete an existing record (a transient unreadable SMART must not erase a real disk's
// history and hand it a clean slate);
// - a disk that disappears from the report is dropped, so a reappearance re-baselines silently;
// - recovery (improvement) notifies nothing;
// - an unreachable agent returns nil and changes nothing at all.
func (s *Server) RunDiskHealthCheck(ctx context.Context) error {
// Fetch FRESH (not the 60s card cache): the check must see current SMART, and this keeps its
// decision logic independent of dashboard render timing.
resp, err := s.fetchDisks(ctx)
if err != nil {
// POSITIVE OBSERVABLE: say the check ran and why it produced nothing. An absent log line is
// equally consistent with "healthy" and "never ran".
if s.logger != nil {
s.logger.Printf("[DEBUG] [web] disk-health check skipped: agent unreachable: %v", err)
}
return nil
}
var fired []notify.DiskAlert
evaluated := 0
s.diskHealth.mu.Lock()
s.loadDiskStateLocked()
now := s.diskHealth.now()
seen := map[string]bool{}
for _, d := range resp.Disks {
if !isPhysicalDisk(d) {
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)
v := agentapi.DiskVerdictFor(d.Smart, prior)
if v == agentapi.DiskVerdictUnknown {
// Excluded both directions: don't record, don't transition, don't drop the existing record.
continue
}
evaluated++
sectors := agentapi.UncorrectableSectors(d.Smart)
emit, worsened := diskAlertDecision(prev, v, sectors, now)
// Build the successor record. The alert history is CARRIED FORWARD across an improvement —
// that is what makes flap damping work: a disk that recovers and degrades again still knows
// what it last told the customer.
rec := &diskRecord{Verdict: int(v), SawUncorrectable: sectors > 0, Sectors: sectors, ChangedAt: now,
PriorSawUncorrectable: prior.SawUncorrectable}
if prev != nil {
rec.Alerted, rec.AlertedVerdict = prev.Alerted, prev.AlertedVerdict
rec.AlertedSectors, rec.AlertedAt = prev.AlertedSectors, prev.AlertedAt
if prev.Verdict == int(v) && !prev.ChangedAt.IsZero() {
rec.ChangedAt = prev.ChangedAt // unchanged verdict keeps its original change time
}
}
if emit {
rec.Alerted, rec.AlertedVerdict, rec.AlertedSectors, rec.AlertedAt = true, int(v), sectors, now
fired = append(fired, notify.DiskAlert{
Label: diskDisplayLabel(d),
Attributes: agentapi.DegradedAttributes(d.Smart),
Kind: diskAlertKindFor(d.Smart, v, worsened),
Sectors: sectors,
TemperatureC: temperatureOf(d.Smart),
})
}
s.diskHealth.records[key] = rec
}
// Forget disks no longer reported so a reappearance re-baselines silently.
for k := range s.diskHealth.records {
if !seen[k] {
delete(s.diskHealth.records, k)
}
}
// ONE write per run, after every disk has been evaluated.
s.saveDiskStateLocked()
s.diskHealth.mu.Unlock()
// POSITIVE OBSERVABLE, every cycle: "0 alert(s)" from a check that evaluated N disks is evidence
// of health; silence is evidence of nothing.
if s.logger != nil {
s.logger.Printf("[INFO] [web] disk-health check complete: %d disk(s) evaluated, %d alert(s)", evaluated, len(fired))
}
for _, a := range fired {
s.emitDiskDegraded(a)
}
return nil
}
// temperatureOf is the disk's reported temperature, or 0 when the device reports none.
func temperatureOf(sm *agentapi.SmartSummary) int {
if sm != nil && sm.TemperatureC != nil {
return *sm.TemperatureC
}
return 0
}
// emitDiskDegraded routes an alert to the notifier (or the test seam). nil notifier → no-op.
func (s *Server) emitDiskDegraded(a notify.DiskAlert) {
if s.diskNotifyFn != nil {
s.diskNotifyFn(a)
return
}
if s.notifier != nil {
s.notifier.NotifyDiskHealthDegraded(a)
}
}