fix(disk-health): the alert that never sent — severity, a real Hiba level, and a memory that survives a restart
Three defects made the disk-health feature silent in exactly the case it
exists for. Evidence: felhom.eu documentation/audits/DIAG-smart-passed-trap-2026-08-14.md
1. SEVERITY (the one that changes whether anything arrives at all).
NotifyDiskHealthDegraded emitted severity "warn", which is NOT in the
hub's accepted set {info,warning,error,critical}. The hub coerced it to
"info" (hub/internal/api/handler.go) and severityNotifies dropped it
(hub/internal/notify/dispatcher.go), so every Figyelmeztetes-level disk
alert was filed as an informational notice and emailed to NOBODY, on the
customer and the operator leg alike. Now "warning". DiskAlertKind.Severity()
is exported so the contract is checkable from any package.
2. NO LEVEL ABOVE "worth an eye". smart_status.passed CANNOT fail on
unreadable sectors (attrs 187/197/198 all carry thresh 0 and a normalized
value floors at 1), so Hiba was unreachable for this whole fault class.
DiskVerdictFor now takes a DiskPrior and implements a 14-row top-down
ladder: sustained unreadable sectors, a count too large to be a blip (64),
unreadable+remapping together, overheating, NVMe critical flag or spent
endurance all reach Hiba. No fourth label — predicted failure is "Hiba".
3. IT SPOKE ONCE, AND FORGOT ON RESTART. The baseline was in-memory, so a box
that rebooted while a disk was failing never alerted again; and between 8
and 352 sectors nothing was emitted at all. State is now persisted
(disk-health-state.json, atomic tmp+rename), the decision compares against
the last ALERTED verdict (collapsing flaps to one alert while letting a
genuine escalation fire immediately), and a disk already at Hiba re-alerts
once it has BOTH doubled its count and waited out a 24h cooldown.
The card replays the same prior the check used (diskRecord.PriorSawUncorrectable)
so the chip and the email cannot disagree — the property the shared verdict
function exists to guarantee, now pinned rather than asserted.
Tests: 12 scenario groups A-L. Group L builds the Server through web.NewServer,
the same call main.go makes, over a real file.
This commit is contained in:
@@ -0,0 +1,251 @@
|
||||
package web
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/agentapi"
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/notify"
|
||||
)
|
||||
|
||||
// Persisted disk-health reporting state (v0.215.0).
|
||||
//
|
||||
// WHY IT IS PERSISTED: until v0.215.0 the baseline was in-memory only, so a controller restart while
|
||||
// a disk was already failing re-baselined that disk silently — and it never alerted again. A box that
|
||||
// reboots during exactly the fault this feature exists for was the one case it could not report.
|
||||
//
|
||||
// WHAT IT IS NOT: one small record per disk about what has been OBSERVED and REPORTED. It is NOT a
|
||||
// sample history and must not grow into one — SMART sample series belong in metrics.MetricsStore
|
||||
// under Phase 2/3, which is a schema migration and a wire change. Keeping this a flat file keeps a
|
||||
// one-word severity fix off that critical path.
|
||||
//
|
||||
// Crash-safety: atomic tmp+rename is sufficient. The state describes reporting history, not a
|
||||
// mutation in flight, so a lost write costs at most one re-baseline — and the failure direction is a
|
||||
// DUPLICATE alert rather than a missing one.
|
||||
|
||||
const diskStateFileName = "disk-health-state.json"
|
||||
|
||||
// diskStateVersion guards the on-disk shape. An unrecognised version is treated as "no prior"
|
||||
// (fail-safe) rather than mis-parsed into a verdict.
|
||||
const diskStateVersion = 1
|
||||
|
||||
// diskRealertCooldown is the minimum gap between two alerts for a disk already at Hiba. It is ANDed
|
||||
// with the doubling bar — clearing only one of them emits nothing.
|
||||
const diskRealertCooldown = 24 * time.Hour
|
||||
|
||||
// diskRecord is everything the checker remembers about ONE disk between runs.
|
||||
type diskRecord struct {
|
||||
// Verdict is the last non-UNKNOWN verdict observed (agentapi.DiskVerdict as an int).
|
||||
Verdict int `json:"verdict"`
|
||||
// SawUncorrectable is THIS observation: it becomes the NEXT check's agentapi.DiskPrior, and is
|
||||
// what turns a one-off excursion into a sustained fault.
|
||||
SawUncorrectable bool `json:"saw_uncorrectable"`
|
||||
// PriorSawUncorrectable is the prior that PRODUCED Verdict — i.e. the previous observation.
|
||||
//
|
||||
// It exists so the CARD can reach the same verdict as the check. The two run at different times
|
||||
// over the same SMART: the check consumes the prior and then overwrites it with the current
|
||||
// observation, so a card rendering afterwards would consume its own check's write and read one
|
||||
// level too high — a disk at its first sighting of 8 sectors would show "Hiba" on the dashboard
|
||||
// while the alert (correctly) said "Figyelmeztetés". Replaying the SAME prior keeps the shared
|
||||
// verdict function's guarantee intact instead of merely asserting it.
|
||||
PriorSawUncorrectable bool `json:"prior_saw_uncorrectable,omitempty"`
|
||||
// Sectors is max(pending, offline_uncorrectable) at the last observation.
|
||||
Sectors int `json:"sectors,omitempty"`
|
||||
// ChangedAt is when Verdict last changed value.
|
||||
ChangedAt time.Time `json:"changed_at,omitempty"`
|
||||
|
||||
// --- alert history: survives an intervening improvement, which is what makes flap damping work ---
|
||||
|
||||
// Alerted is false until this disk has ever produced an event. Distinguishes "never alerted"
|
||||
// from "alerted at Nincs adat", which cannot happen but would otherwise share the zero value.
|
||||
Alerted bool `json:"alerted,omitempty"`
|
||||
AlertedVerdict int `json:"alerted_verdict,omitempty"`
|
||||
AlertedSectors int `json:"alerted_sectors,omitempty"`
|
||||
AlertedAt time.Time `json:"alerted_at,omitempty"`
|
||||
}
|
||||
|
||||
// diskStateFile is the on-disk envelope.
|
||||
type diskStateFile struct {
|
||||
Version int `json:"version"`
|
||||
Disks map[string]*diskRecord `json:"disks"`
|
||||
}
|
||||
|
||||
// diskStatePath is the state file's location, or "" when no data dir is configured (unit tests and
|
||||
// an unprovisioned guest). An empty path degrades to in-memory-only: correct behaviour within the
|
||||
// process, simply not durable.
|
||||
func (s *Server) diskStatePath() string {
|
||||
if s.cfg == nil || s.cfg.Paths.DataDir == "" {
|
||||
return ""
|
||||
}
|
||||
return filepath.Join(s.cfg.Paths.DataDir, diskStateFileName)
|
||||
}
|
||||
|
||||
// loadDiskStateLocked populates the in-memory records from disk exactly once per process. The caller
|
||||
// holds s.diskHealth.mu.
|
||||
//
|
||||
// NEVER FATAL. A missing file is the normal first-boot case. A corrupt or unreadable one is LOGGED
|
||||
// and treated as "no prior" — the worst that costs is one silent re-baseline, whereas crashing the
|
||||
// check would take away disk monitoring entirely.
|
||||
func (s *Server) loadDiskStateLocked() {
|
||||
if s.diskHealth.loaded {
|
||||
return
|
||||
}
|
||||
s.diskHealth.loaded = true
|
||||
if s.diskHealth.records == nil {
|
||||
s.diskHealth.records = map[string]*diskRecord{}
|
||||
}
|
||||
path := s.diskStatePath()
|
||||
if path == "" {
|
||||
return
|
||||
}
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
if !os.IsNotExist(err) && s.logger != nil {
|
||||
s.logger.Printf("[WARN] [web] disk-health state unreadable, continuing with no prior: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
var f diskStateFile
|
||||
if err := json.Unmarshal(data, &f); err != nil {
|
||||
if s.logger != nil {
|
||||
s.logger.Printf("[WARN] [web] disk-health state corrupt, continuing with no prior: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if f.Version != diskStateVersion {
|
||||
if s.logger != nil {
|
||||
s.logger.Printf("[WARN] [web] disk-health state version %d unrecognised (want %d), continuing with no prior",
|
||||
f.Version, diskStateVersion)
|
||||
}
|
||||
return
|
||||
}
|
||||
for k, r := range f.Disks {
|
||||
if r != nil {
|
||||
s.diskHealth.records[k] = r
|
||||
}
|
||||
}
|
||||
if s.logger != nil {
|
||||
s.logger.Printf("[DEBUG] [web] disk-health state loaded: %d disk(s)", len(s.diskHealth.records))
|
||||
}
|
||||
}
|
||||
|
||||
// saveDiskStateLocked writes the records atomically (.tmp then rename), following the shape of
|
||||
// selfupdate.SaveState. Called ONCE per check run, after every disk has been evaluated — not per
|
||||
// disk. Caller holds s.diskHealth.mu. Errors are logged, never returned: failing to remember is not
|
||||
// a reason to stop monitoring.
|
||||
func (s *Server) saveDiskStateLocked() {
|
||||
path := s.diskStatePath()
|
||||
if path == "" {
|
||||
return
|
||||
}
|
||||
if err := writeDiskState(path, s.diskHealth.records); err != nil && s.logger != nil {
|
||||
s.logger.Printf("[WARN] [web] disk-health state not saved (a restart will re-baseline): %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func writeDiskState(path string, records map[string]*diskRecord) error {
|
||||
data, err := json.MarshalIndent(diskStateFile{Version: diskStateVersion, Disks: records}, "", " ")
|
||||
if err != nil {
|
||||
return fmt.Errorf("marshaling disk-health state: %w", err)
|
||||
}
|
||||
tmp := path + ".tmp"
|
||||
if err := os.WriteFile(tmp, data, 0644); err != nil {
|
||||
return fmt.Errorf("writing temp disk-health state: %w", err)
|
||||
}
|
||||
if err := os.Rename(tmp, path); err != nil {
|
||||
os.Remove(tmp)
|
||||
return fmt.Errorf("renaming disk-health state: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// priorFor is the agentapi.DiskPrior the CHECK consumes: what the previous check observed. A zero
|
||||
// DiskPrior ("nothing known") is the fail-safe — a first sighting can then only reach
|
||||
// Figyelmeztetés, never Hiba. Caller holds the mutex.
|
||||
func (s *Server) priorFor(key string) agentapi.DiskPrior {
|
||||
if r := s.diskHealth.records[key]; r != nil {
|
||||
return agentapi.DiskPrior{SawUncorrectable: r.SawUncorrectable}
|
||||
}
|
||||
return agentapi.DiskPrior{}
|
||||
}
|
||||
|
||||
// cardPriorFor is the agentapi.DiskPrior the CARD replays: the prior that produced the stored
|
||||
// verdict, NOT the observation that check went on to record. See diskRecord.PriorSawUncorrectable —
|
||||
// using priorFor here would make the dashboard chip read one level more severe than the alert.
|
||||
// Caller holds the mutex.
|
||||
func (s *Server) cardPriorFor(key string) agentapi.DiskPrior {
|
||||
if r := s.diskHealth.records[key]; r != nil {
|
||||
return agentapi.DiskPrior{SawUncorrectable: r.PriorSawUncorrectable}
|
||||
}
|
||||
return agentapi.DiskPrior{}
|
||||
}
|
||||
|
||||
// diskAlertDecision decides whether THIS observation produces an event, per the v0.215.0 rules.
|
||||
// Pure: everything, including the clock reading, arrives as an argument.
|
||||
//
|
||||
// prev == nil -> no event (first verdict ever for this disk: baseline silently)
|
||||
// v is Rendben (or better) -> no event (recovery is silent — unchanged behaviour)
|
||||
// v worse than the LAST ALERTED -> EVENT. Escalation is never damped.
|
||||
// v == last alerted, and v is Hiba,
|
||||
// and sectors >= 2x the count at
|
||||
// the last alert, and >= 24h since -> EVENT ("still getting worse")
|
||||
// anything else -> no event
|
||||
//
|
||||
// The second return value reports whether this is the "still getting worse" re-alert, which selects
|
||||
// a different message shape.
|
||||
//
|
||||
// WHY "worse than the last ALERTED verdict" rather than the last OBSERVED one: a disk that goes
|
||||
// Figyelmeztetés -> Rendben -> Figyelmeztetés has worsened against its last observation twice, and
|
||||
// today's transition-only logic emits two identical mild alerts on that zero-crossing. That is how a
|
||||
// customer learns to ignore the message. Comparing against what was last REPORTED collapses the flap
|
||||
// to one alert while leaving a genuine escalation (Figyelmeztetés -> Hiba) free to fire immediately.
|
||||
//
|
||||
// WHY nothing else emits at v == last alerted: the steady-state case and the post-improvement flap
|
||||
// case are indistinguishable without tracking intervening improvements, and both must stay silent.
|
||||
// Only the explicit Hiba re-alert above breaks that silence, and only when BOTH its bars are cleared.
|
||||
func diskAlertDecision(prev *diskRecord, v agentapi.DiskVerdict, sectors int, now time.Time) (emit bool, worsened bool) {
|
||||
if v <= agentapi.DiskVerdictOK {
|
||||
return false, false // Nincs adat never reaches here; Rendben is silent
|
||||
}
|
||||
if prev == nil {
|
||||
return false, false // first verdict ever for this disk
|
||||
}
|
||||
alerted := agentapi.DiskVerdictUnknown
|
||||
if prev.Alerted {
|
||||
alerted = agentapi.DiskVerdict(prev.AlertedVerdict)
|
||||
}
|
||||
if v > alerted {
|
||||
return true, false // escalation — damping never applies
|
||||
}
|
||||
if v == alerted && v == agentapi.DiskVerdictFail &&
|
||||
prev.AlertedSectors > 0 && sectors >= 2*prev.AlertedSectors &&
|
||||
now.Sub(prev.AlertedAt) >= diskRealertCooldown {
|
||||
return true, true // still getting worse
|
||||
}
|
||||
return false, false
|
||||
}
|
||||
|
||||
// diskAlertKindFor picks the customer-facing message shape for an alert that has already been
|
||||
// decided. It mirrors the truth-table order so the reason quoted to the customer is the reason the
|
||||
// verdict actually fired on.
|
||||
func diskAlertKindFor(sm *agentapi.SmartSummary, v agentapi.DiskVerdict, worsened bool) notify.DiskAlertKind {
|
||||
if v != agentapi.DiskVerdictFail {
|
||||
return notify.DiskAlertWarn
|
||||
}
|
||||
switch {
|
||||
case sm != nil && sm.Health == agentapi.SmartFailing:
|
||||
return notify.DiskAlertFailSelfReported
|
||||
case sm != nil && sm.TemperatureC != nil && *sm.TemperatureC >= agentapi.TemperatureFailC:
|
||||
return notify.DiskAlertFailTemperature
|
||||
case agentapi.UncorrectableSectors(sm) > 0 && worsened:
|
||||
return notify.DiskAlertFailWorsened
|
||||
case agentapi.UncorrectableSectors(sm) > 0:
|
||||
return notify.DiskAlertFailSectors
|
||||
}
|
||||
// Hiba with no sector count and no heat: NVMe's own critical flag, or spent rated endurance.
|
||||
// Both are declarations BY THE DEVICE, so the self-reported wording is the honest one.
|
||||
return notify.DiskAlertFailSelfReported
|
||||
}
|
||||
Reference in New Issue
Block a user