diff --git a/modules/sensors/README.md b/modules/sensors/README.md index 132a91ec..0cf08be2 100644 --- a/modules/sensors/README.md +++ b/modules/sensors/README.md @@ -28,13 +28,29 @@ seat's verbs. The full reasoning is in ADR 0256. |---|---|---| | `smartmontools` | the package: `smartctl` and `smartd` | | | `/etc/smartd.conf` | every drive (`DEVICESCAN`), every check, never waking a drive in standby, no mail | written whole | -| `smartd.service` drop-in | runs on a virtual machine too, and stays up with no drive to watch (`-q never`) | written whole | +| `smartd.service` drop-in | runs on a virtual machine too, stays up with no drive to watch (`-q never`), and checks each drive hourly (`-i 3600`) | written whole | | `smartd` | watches every drive between looks, and logs what it finds to the journal | root, the package's unit | -| `watch` (`sensors watch`) | every 10 min each drive's health with `smartctl --json -a -n standby`; every 30 s which fans turn; one file, `/run/mesh-sensors/readings.json` (0644) | root, the node-engine's process | -| the tools | read `/sys`, `/proc` and the watch's file | the operator account, in the tool runner | +| `watch` (`sensors watch`) | each drive's health with `/usr/bin/smartctl --json -a -n standby,2,3`: a spinning drive hourly, any other every 10 min; every 30 s which fans turn; one file, `/run/mesh-sensors/readings.json` (0644, in a 0755 directory) | root, the node-engine's process | +| the tools | read `/sys`, `/proc` and the watch's file, once per call | the operator account, in the tool runner | -The tools need no root, because the watch has already read what only root may read. A drive in standby -is never woken. The watch keeps its last reading of that drive and says that it was asleep. +The tools need no root, because the watch has already read what only root may read. + +**The drives never hold up the fans.** The watch reads the drives beside its looks at the fans, one pass +at a time. Each smartctl is bounded by 20 s, plus 2 s for a child that keeps its output open. The file is +written every 30 s whatever a drive does, so a file older than five minutes means the watch is hung or +gone, and the health check says so. + +**A drive is never woken, and is read seldom enough to sleep.** +- smartctl's `-n standby,2,3` stops with status 2 and the drive's power mode when the drive is in standby + or asleep. The watch then keeps the reading before and says the drive was asleep. A drive is asleep only + on status 2 with a parsed power mode (or smartctl's own "STANDBY (OS)"). +- It stops with status 3 when the drive cannot say (a USB bridge that does not pass CHECK POWER MODE on). + Such a drive is not read, so that reading it never wakes it, and it is said as not read. +- Reading SMART from an awake drive can restart its standby timer, so a drive read more often than its + timer is set for never spins down. No mesh tool reads a drive's standby timer, so whether the servers' + drives spin down is not known. A spinning drive is therefore read hourly, by the watch and by smartd. + A drive whose timer is set longer than an hour may still be kept awake. +- smartd has no per-drive equivalent of status 3. A drive behind such a bridge may be woken by smartd. ## Tools @@ -55,20 +71,26 @@ All six only read. |---|---| | temperature HIGH | at or above its own maximum, or a zone's `hot` trip (without one, its lowest `passive` trip) | | temperature CRITICAL | at or above its own critical or emergency value, or a zone's `critical` trip | -| drive FAILING | SMART's overall verdict failed, an attribute is failing now, or an NVMe critical warning bit is set | -| drive HIGH | reallocated, pending or unreadable sectors; an attribute that failed in the past; NVMe media errors or 90 % of rated endurance used; errors in the error log; a last self-test that did not pass | -| fan STOPPED | its chip's alarm, or under its own minimum, once it has been seen turning; or it ran since boot and reads 0 while a temperature is at or above its own maximum | +| drive FAILING | SMART's overall verdict failed (SATA), a pre-failure attribute is failing now, or an NVMe critical warning bit other than temperature is set | +| drive HIGH | an old-age attribute failing now; an attribute that failed in the past; the NVMe temperature bit alone; reallocated, pending or unreadable sectors; NVMe media errors or 90 % of rated endurance used; errors in the error log; a last self-test that did not pass | +| fan STOPPED | it ran since boot, reads 0 now, and the CPU, the board or a sensor on its own chip is at or above its own maximum | +| fan note | under its chip's minimum, or its chip's alarm bit set: said, never flagged | | memory HIGH / CRITICAL | corrected / uncorrectable errors since boot | | PCIe HIGH | fatal or non-fatal errors since boot | -**What is no threshold.** A threshold given as the format's largest value is no threshold. The laptop's -NVMe drive gives 65261.85 °C as the maximum of two of its sensors. A threshold of 0 is no threshold -either, and neither is anything outside −40 to 200 °C. The reading is then said without a flag. A CPU -sensor that gives no threshold, such as AMD's `Tctl`, is never flagged. +**What is no threshold, and no reading.** +- A threshold given as the format's largest value is no threshold. The laptop's NVMe drive gives + 65261.85 °C as the maximum of two of its sensors. +- A threshold of 0 is no threshold either, and neither is anything outside −40 to 200 °C. The reading + is then said without a flag. +- A CPU sensor that gives no threshold, such as AMD's `Tctl`, is never flagged. +- 0 °C is a reading. 127 and 127.5 °C are not: they are what a board chip's input with nothing on it + answers (a phantom AUXTIN). Such a sensor is said as not read. -**Why a fan at 0 is not enough.** Laptop fans and GPU fans in zero-fan mode stop on purpose when the -machine is cool. A board header with nothing on it reads 0, and may carry an alarm or a minimum that the -firmware set. So a fan is judged only once it has been seen turning. +**Why a fan at 0, a minimum or an alarm is not enough.** Laptop fans and GPU fans in zero-fan mode stop on +purpose when the machine is cool, under the minimum their chip gives. A board chip latches an alarm bit, +and a header with nothing on it reads 0. Only a fan that ran and stopped while what it cools is hot is a +fault. A hot drive or network adapter says nothing of a fan. ## Health @@ -80,15 +102,18 @@ The module declares its health under ADR 0240: `sensors_health` is unhealthy when: -- a temperature is at or above its own critical value; -- a drive says it is failing; -- a fan that ran has stopped while the machine is hot; -- memory reports an uncorrectable error. +- a CPU, GPU, drive or zone temperature is at or above its own critical value. A board chip's (Super I/O) + critical value is shown CRITICAL by the tools and never makes the module unhealthy: its limits are + whatever the firmware left; +- a drive says it is failing (the FAILING row above); +- a fan is STOPPED (the row above); +- memory reports an uncorrectable error; +- the watch has written no readings since the machine started, or none for five minutes. Its `why` is one plain sentence per finding, for the operator. For example: "The drive sdb (WDC -WUH722222ALE6L4) says it is failing: its overall SMART health check failed." HIGH readings raise nothing. -A drive whose health was not read is said in `why` and does not make the check unhealthy. The watch not -running is caught by the watch's own liveness. +WUH722222ALE6L4) says it is failing: its overall SMART health check failed." HIGH readings and fan notes +raise nothing. A drive whose health was not read (asleep since boot, or behind a bridge) is said in `why` +and does not make the check unhealthy. The condition this raises is `module.sensors..unhealthy`. ADR 0256 records what this costs: @@ -100,7 +125,7 @@ The machine judging its own hardware, as it judges its network, is the follow-up ## Tests `go test ./...`. The tests read a tree standing in for `/sys`, `/proc` and `/run`, and smartctl answers -recorded as smartmontools 7 writes them. There are three machines: +as smartmontools 7.5 writes them (its standby answer as its source sets it). There are three machines: - a laptop, as the laptop showed itself on 2026-10-08; - a server with ECC memory, three SATA drives read by drivetemp and a board chip with an empty fan header; @@ -110,14 +135,20 @@ They cover: - thresholds taken from the hardware only, and a zone's hwmon copy left out; - HIGH at the maximum and CRITICAL at the critical value; a zone's passive trip as its high; -- a sensor that gives no reading said as not read; +- 0 °C as a reading, a phantom 127 °C as not read, and a board chip's critical value raising nothing; - every class not present on the virtual machine, which is healthy; -- a failing SATA drive, an NVMe critical warning, and early signs that are HIGH and raise nothing; -- a missing, stale or earlier-boot reading said as such; -- a fan at rest, a fan that stopped while hot, an empty header, and a fan under its minimum; +- a failing SATA drive; an NVMe critical warning; the NVMe temperature bit alone as HIGH; NVMe without + `smart_status`; an old-age attribute failing now and a past failure as HIGH; +- a missing, stale or earlier-boot reading, and the watch's staleness making the module unhealthy; +- a fan at rest, a fan that stopped while the board is hot, a hot drive saying nothing of a fan, a hot + sensor on the fan's own chip, an empty header, a zero-fan GPU under its minimum, and a latched alarm; - memory and PCIe errors, and CPU throttling; -- the watch reading as root without waking a drive, keeping a reading for a drive asleep, reading drives - only every 10 min, and keeping what it saw across a restart; +- the watch reading as root without waking a drive: smartctl's real standby answer, an open failure + that is not sleep, and a bridge without a power mode; +- a spinning drive read hourly and a solid-state one every 10 min; +- a smartctl that hangs with a child holding its output, bounded, and the fans looked at and written + while a drive hangs; +- a restart keeping what the last watch read; the readings directory given its mode; - a watch that is not root asking through `sudo -n`, and saying what stops it; - the health answer in the shape the node-engine reads; - the manifest: the tools listed are the tools served, all read-only, no seat claimed, no machine named, diff --git a/modules/sensors/cmd/sensors/drives.go b/modules/sensors/cmd/sensors/drives.go index 4fc01a85..ed4de019 100644 --- a/modules/sensors/cmd/sensors/drives.go +++ b/modules/sensors/cmd/sensors/drives.go @@ -18,9 +18,13 @@ import ( // ReadingsPath is the watch's one file: what it last read of each drive, and which fans it saw turning. const ReadingsPath = "/run/mesh-sensors/readings.json" -// StaleAfter is how old the watch's file may be before the tools say the watch may not be running: three -// of its drive readings missed. -const StaleAfter = 30 * time.Minute +// StaleAfter is how old the watch's file may be before the module is unhealthy: the watch writes it every +// FanEvery, so ten looks missed is a watch that is hung or gone, never a slow drive (drives are read +// beside the fans, not before them). +const StaleAfter = 5 * time.Minute + +// Smartctl is the program, by its path: the watch runs as root and runs nothing found on a PATH. +const Smartctl = "/usr/bin/smartctl" // FlagFailing is said beside a drive that says it is failing. const FlagFailing = "FAILING" @@ -43,6 +47,8 @@ type DriveReading struct { // one before. Asleep bool `json:"asleep,omitempty"` NotRead string `json:"not_read,omitempty"` + // LookedAt is the newest look at the drive, asleep or not: what the next look is timed from. + LookedAt time.Time `json:"looked_at"` } // Drive is one drive, as the tools say it. @@ -144,10 +150,10 @@ func (m *Machine) ReadReadings() (*Readings, string) { return &r, "" } -// Drives is every drive with its health as the watch last read it, judged. -func (m *Machine) Drives() ([]Drive, string) { +// Drives is every drive with its health as the watch's readings say it, judged. r is nil, with why, when +// there are none to use. +func (m *Machine) Drives(r *Readings, why string) ([]Drive, string) { drives := m.BlockDrives() - r, why := m.ReadReadings() note := why if r != nil && m.Now().Sub(r.WrittenAt) > StaleAfter { note = fmt.Sprintf("the module's watch last wrote %s ago; it may not be running", ago(m.Now().Sub(r.WrittenAt))) @@ -163,7 +169,7 @@ func (m *Machine) Drives() ([]Drive, string) { d.Health, d.NotRead = "not read", "the module's watch has not read this drive yet" continue } - judgeDrive(d, reading, m.Now()) + judgeDrive(d, reading) } return drives, note } @@ -196,6 +202,9 @@ type smartJSON struct { SmartStatus *struct { Passed bool `json:"passed"` } `json:"smart_status"` + PowerMode *struct { + Name string `json:"name"` + } `json:"power_mode"` NVMe *struct { CriticalWarning int64 `json:"critical_warning"` AvailableSpare int64 `json:"available_spare"` @@ -211,7 +220,10 @@ type smartJSON struct { Value int64 `json:"value"` Thresh int64 `json:"thresh"` WhenFailed string `json:"when_failed"` - Raw struct { + Flags struct { + Prefailure bool `json:"prefailure"` + } `json:"flags"` + Raw struct { Value int64 `json:"value"` } `json:"raw"` } `json:"table"` @@ -259,7 +271,7 @@ func num(v int64) *int64 { return &v } // judgeDrive reads one drive's smartctl answer into the drive, with its findings: FAILING when the drive // says so itself, HIGH for the early signs a drive gives before it fails. -func judgeDrive(d *Drive, reading DriveReading, now time.Time) { +func judgeDrive(d *Drive, reading DriveReading) { if !reading.ReadAt.IsZero() { d.ReadAt = reading.ReadAt.UTC().Format(time.RFC3339) } @@ -276,7 +288,7 @@ func judgeDrive(d *Drive, reading DriveReading, now time.Time) { d.Model = s.ModelName } d.Serial, d.Firmware = s.SerialNumber, s.FirmwareVersion - if s.SmartStatus == nil { + if s.SmartStatus == nil && s.NVMe == nil { d.Health = "not read" switch { case s.SmartSupport != nil && !s.SmartSupport.Available: @@ -290,7 +302,9 @@ func judgeDrive(d *Drive, reading DriveReading, now time.Time) { } d.Health = "passed" var failing, high []string - if !s.SmartStatus.Passed { + // An NVMe drive's overall verdict is its critical warning byte, so it is judged from the bits below; + // the temperature bit alone is a drive running hot, not one failing. + if s.SmartStatus != nil && !s.SmartStatus.Passed && s.NVMe == nil { failing = append(failing, "its overall SMART health check failed") } if s.Temperature != nil && s.Temperature.Current != 0 { @@ -306,7 +320,11 @@ func judgeDrive(d *Drive, reading DriveReading, now time.Time) { d.PowerOnHours = num(n.PowerOnHours) } for _, w := range nvmeWarnings { - if n.CriticalWarning&w.bit != 0 { + switch { + case n.CriticalWarning&w.bit == 0: + case w.bit == 0x02: + high = append(high, w.said) + default: failing = append(failing, w.said) } } @@ -329,7 +347,14 @@ func judgeDrive(d *Drive, reading DriveReading, now time.Time) { } switch at.WhenFailed { case "now": - failing = append(failing, fmt.Sprintf("its attribute %s is at %d, at or below its own threshold of %d", at.Name, at.Value, at.Thresh)) + // A pre-failure attribute at its threshold is the drive saying it is about to fail; an + // old-age one (a temperature, a count of hours) is wear or a hot day. + said := fmt.Sprintf("its attribute %s is at %d, at or below its own threshold of %d", at.Name, at.Value, at.Thresh) + if at.Flags.Prefailure { + failing = append(failing, said) + } else { + high = append(high, said+" (an old-age attribute)") + } case "past": high = append(high, fmt.Sprintf("its attribute %s was at or below its own threshold in the past", at.Name)) } @@ -380,18 +405,21 @@ func firstError(s smartJSON, fallback string) string { } // ReadDrive asks smartctl for one drive's health, as root (directly when this process is root, else -// through sudo -n), without waking a drive in standby. A drive asleep keeps the reading before. +// through sudo -n), without waking a drive in standby. `-n standby,2,3` makes smartctl stop with status 2 +// and the drive's power mode when it is asleep, and with status 3 when the drive cannot say (a USB bridge +// that does not pass the question on): such a drive is not read, because reading it could wake it. A +// drive asleep keeps the reading before. func (m *Machine) ReadDrive(device string, before *DriveReading) DriveReading { - args := []string{"--json=c", "-a", "-n", "standby", "/dev/" + device} - program := "smartctl" + args := []string{"--json=c", "-a", "-n", "standby,2,3", "/dev/" + device} + program := Smartctl if m.UID != 0 { - program, args = "sudo", append([]string{"-n", "smartctl"}, args...) + program, args = "sudo", append([]string{"-n", Smartctl}, args...) } r := m.Run(context.Background(), program, args...) now := m.Now() - out := DriveReading{ReadAt: now, Status: r.Status} + out := DriveReading{ReadAt: now, LookedAt: now, Status: r.Status} switch { - case r.Err == "ENOENT" && program == "smartctl": + case r.Err == "ENOENT" && program == Smartctl: out.NotRead = "smartctl is not installed; the module declares smartmontools, so the package has not been applied yet" return out case r.Err == "ENOENT": @@ -407,17 +435,21 @@ func (m *Machine) ReadDrive(device string, before *DriveReading) DriveReading { out.NotRead = "the watch is not root and may not run smartctl through sudo without a prompt" return out } - said := strings.ToUpper(r.Stdout + r.Stderr) - if r.Status&2 != 0 && (strings.Contains(said, "STANDBY") || strings.Contains(said, "SLEEP")) { + var s smartJSON + parsed := json.Unmarshal([]byte(r.Stdout), &s) == nil + switch { + case parsed && r.Status == 2 && asleep(s): if before != nil { kept := *before - kept.Asleep = true + kept.Asleep, kept.LookedAt = true, now return kept } out.Asleep, out.NotRead = true, "the drive was asleep at every look, and is not woken to be read" return out - } - if !json.Valid([]byte(r.Stdout)) { + case parsed && r.Status == 3 && saysNoPowerMode(s): + out.NotRead = "the drive cannot say whether it is asleep (a USB bridge, usually), so it is not read, never to wake it" + return out + case !parsed: out.NotRead = "smartctl answered something other than JSON (status " + strconv.Itoa(r.Status) + ")" return out } @@ -425,6 +457,32 @@ func (m *Machine) ReadDrive(device string, before *DriveReading) DriveReading { return out } +// asleep says smartctl stopped because the drive is in standby or asleep: by the power mode it parsed, +// or by its own word that the operating system has the drive powered down. +func asleep(s smartJSON) bool { + if s.PowerMode != nil { + switch s.PowerMode.Name { + case "STANDBY", "STANDBY_Y", "SLEEP": + return true + } + } + for _, msg := range s.Smartctl.Messages { + if strings.HasPrefix(msg.String, "Device is in STANDBY (OS) mode") { + return true + } + } + return false +} + +func saysNoPowerMode(s smartJSON) bool { + for _, msg := range s.Smartctl.Messages { + if strings.HasPrefix(msg.String, "CHECK POWER MODE not implemented") || strings.HasPrefix(msg.String, "CHECK POWER MODE returned unknown value") { + return true + } + } + return false +} + // WriteReadings writes the watch's file whole, through a rename, readable by every account: it holds no // secret, and the tools read it as the operator account. func (m *Machine) WriteReadings(r Readings) error { @@ -432,6 +490,10 @@ func (m *Machine) WriteReadings(r Readings) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } + // MkdirAll leaves a directory that was already there as it was: the tools must be able to enter it. + if err := os.Chmod(filepath.Dir(path), 0o755); err != nil { + return err + } raw, err := json.Marshal(r) if err != nil { return err diff --git a/modules/sensors/cmd/sensors/fans.go b/modules/sensors/cmd/sensors/fans.go index d65344f5..be63d415 100644 --- a/modules/sensors/cmd/sensors/fans.go +++ b/modules/sensors/cmd/sensors/fans.go @@ -2,13 +2,16 @@ package main import ( "fmt" + "path" "strings" ) // Fans, from the hwmon chips. Many fans stop on purpose when the machine is cool (a laptop at rest, a GPU -// in its zero-fan mode), so 0 RPM alone is not a fault. A fan is said STOPPED only when the hardware says -// so itself (its alarm bit, or a reading under its own minimum) or when it ran since the machine started -// and reads 0 while a temperature is at or above the hardware's own maximum (novox/hq ADR 0256 rule 4). +// in its zero-fan mode), so 0 RPM alone is not a fault; nor is a chip's minimum or alarm bit, which a +// zero-fan GPU is under by design and a Super I/O chip latches. Those are said in a note and raise nothing. +// A fan is STOPPED only when it ran since the machine started, reads 0 now, and a temperature that fan +// cools is at or above the hardware's own maximum: the CPU's, the board's, or one on the fan's own chip +// (novox/hq ADR 0256 rule 4). // FlagStopped is said beside a fan that should be turning and is not. const FlagStopped = "STOPPED" @@ -28,8 +31,10 @@ type Fan struct { RanSinceBoot *bool `json:"ran_since_boot,omitempty"` Flag string `json:"flag,omitempty"` Why string `json:"why,omitempty"` - NotRead string `json:"not_read,omitempty"` - Where string `json:"where"` + // Note is what the chip says that raises nothing: under its own minimum, its alarm bit. + Note string `json:"note,omitempty"` + NotRead string `json:"not_read,omitempty"` + Where string `json:"where"` } // Name is how a person reads which fan this is. @@ -70,10 +75,9 @@ func (m *Machine) Fans() []Fan { return out } -// judgeFans flags each fan by what the hardware says of it and, given which fans ran since the machine -// started (nil when the watch has not said) and the hottest temperature at or above its own maximum, a fan -// that stopped while the machine is hot. -func judgeFans(fans []Fan, ran map[string]bool, hot *Temperature) []Fan { +// judgeFans flags each fan, given which fans the watch saw turning since the machine started (nil when it +// has not said) and the temperatures read now. +func judgeFans(fans []Fan, ran map[string]bool, temps []Temperature) []Fan { for i := range fans { f := &fans[i] if ran != nil { @@ -83,31 +87,40 @@ func judgeFans(fans []Fan, ran map[string]bool, hot *Temperature) []Fan { if f.RPM == nil { continue } - // A header with nothing on it reads 0 and may carry an alarm or a minimum the firmware set: a fan - // is judged only once it has been seen turning, now or since the machine started. - ranBefore := f.RanSinceBoot != nil && *f.RanSinceBoot - seen := *f.RPM > 0 || ranBefore - switch { - case f.Alarm && seen: - f.Flag, f.Why = FlagStopped, fmt.Sprintf("its chip raises its alarm at %d RPM", *f.RPM) - case f.Min != nil && *f.RPM < *f.Min && seen: - f.Flag, f.Why = FlagStopped, fmt.Sprintf("it turns at %d RPM, under its own minimum of %d RPM", *f.RPM, *f.Min) - case *f.RPM == 0 && ranBefore && hot != nil: - f.Flag, f.Why = FlagStopped, fmt.Sprintf("it ran earlier and reads 0 RPM while %s is at %.0f °C, at or above its own maximum of %.0f °C", - hot.Name(), *hot.C, *hot.High) + var notes []string + if f.Alarm { + notes = append(notes, fmt.Sprintf("its chip's alarm bit is set at %d RPM", *f.RPM)) + } + if f.Min != nil && *f.RPM < *f.Min { + notes = append(notes, fmt.Sprintf("it turns at %d RPM, under its chip's minimum of %d RPM", *f.RPM, *f.Min)) + } + f.Note = strings.Join(notes, "; ") + if *f.RPM != 0 || f.RanSinceBoot == nil || !*f.RanSinceBoot { + continue + } + if hot := hottestFor(*f, temps); hot != nil { + f.Flag, f.Why = FlagStopped, fmt.Sprintf("it ran earlier and reads 0 RPM while %s is at %.1f °C, at or above %s of %.1f °C", + hot.Name(), *hot.C, hot.HighIs, *hot.High) } } return fans } -// hottest is the temperature furthest at or above its own maximum, or nil when none is. -func hottest(ts []Temperature) *Temperature { +// hottestFor is the temperature furthest at or above its own high value among those this fan may be +// cooling: the CPU's, the board's, or one on the fan's own chip. A drive or a network adapter running hot +// says nothing of a fan. +func hottestFor(f Fan, ts []Temperature) *Temperature { + chip := path.Dir(f.Where) var best *Temperature for i := range ts { t := &ts[i] if t.C == nil || t.High == nil || *t.C < *t.High { continue } + related := t.Kind == "cpu" || t.Kind == "board" || (t.Source == "hwmon" && path.Dir(t.Where) == chip) + if !related { + continue + } if best == nil || *t.C-*t.High > *best.C-*best.High { best = t } diff --git a/modules/sensors/cmd/sensors/findings.go b/modules/sensors/cmd/sensors/findings.go index 37d646b3..6c68e524 100644 --- a/modules/sensors/cmd/sensors/findings.go +++ b/modules/sensors/cmd/sensors/findings.go @@ -15,21 +15,32 @@ type Look struct { Drives []Drive DrivesNote string Faults Faults + // Watch is why the watch's readings are not to be trusted now: none, from an earlier boot, or older + // than StaleAfter. Empty when they are fresh. + Watch string } -// LookNow reads every class, and judges the fans against what the watch saw turning since the machine -// started. +// LookNow reads every class, the watch's file once, and judges the fans against what the watch saw +// turning since the machine started. func (m *Machine) LookNow() Look { l := Look{Temperatures: m.Temperatures(), Fans: m.Fans(), Faults: m.Faults()} - l.Drives, l.DrivesNote = m.Drives() + r, why := m.ReadReadings() + l.Drives, l.DrivesNote = m.Drives(r, why) var ran map[string]bool - if r, why := m.ReadReadings(); why == "" && r != nil { + switch { + case r == nil: + l.Watch = "The module's watch has left no readings since the machine started, so drive health and fan history are not being read." + case m.Now().Sub(r.WrittenAt) > StaleAfter: + l.Watch = fmt.Sprintf("The module's watch has not written its readings for %s, so drive health and fan history are not being read.", + ago(m.Now().Sub(r.WrittenAt))) + } + if r != nil { ran = map[string]bool{} for k := range r.FansRan { ran[k] = true } } - l.Fans = judgeFans(l.Fans, ran, hottest(l.Temperatures)) + l.Fans = judgeFans(l.Fans, ran, l.Temperatures) return l } @@ -37,8 +48,8 @@ func (m *Machine) LookNow() Look { func (l Look) Problems() []string { var out []string for _, t := range l.Temperatures { - if t.Flag == FlagCritical { - out = append(out, fmt.Sprintf("%s is at %.0f °C, at or above %s of %.0f °C.", capital(t.Name()), *t.C, t.CriticalIs, *t.Critical)) + if t.Flag == FlagCritical && t.DrivesHealth() { + out = append(out, fmt.Sprintf("%s is at %.1f °C, at or above %s of %.1f °C.", capital(t.Name()), *t.C, t.CriticalIs, *t.Critical)) } } for _, d := range l.Drives { @@ -57,6 +68,9 @@ func (l Look) Problems() []string { c.Uncorrectable, c.Controller)) } } + if l.Watch != "" { + out = append(out, l.Watch) + } return out } @@ -79,7 +93,7 @@ func (l Look) Health() Health { unread++ } } - // Said, never passed as read. A watch that is not running is the watch's liveness to judge. + // Said, never passed as read: a drive asleep since boot, or one the watch cannot read. if unread > 0 { why += fmt.Sprintf(" The health of %d of %d drives has not been read.", unread, len(l.Drives)) } @@ -128,6 +142,9 @@ func (l Look) Said() []string { out = append(out, fmt.Sprintf("HIGH: the PCIe device %s (%s) counted %d fatal and %d non-fatal errors since the machine started", e.Device, orUnknown(e.Driver), e.Fatal, e.NonFatal)) } } + if l.Watch != "" { + out = append(out, "STALE: "+l.Watch) + } return append(out, l.overview()...) } diff --git a/modules/sensors/cmd/sensors/helpers_test.go b/modules/sensors/cmd/sensors/helpers_test.go index c43945d9..b3ceb255 100644 --- a/modules/sensors/cmd/sensors/helpers_test.go +++ b/modules/sensors/cmd/sensors/helpers_test.go @@ -181,9 +181,32 @@ const ( `"ata_smart_error_log":{"summary":{"count":0}},"ata_smart_self_test_log":{"standard":{"table":[{"type":{"string":"Short offline"},"status":{"string":"Completed without error","passed":true}}]}},` + `"temperature":{"current":36},"power_on_time":{"hours":17000}}` ataFailing = `{"smartctl":{"exit_status":24},"model_name":"WDC WUH722222ALE6L4","serial_number":"A2","smart_support":{"available":true,"enabled":true},"smart_status":{"passed":false},` + - `"ata_smart_attributes":{"table":[{"id":5,"name":"Reallocated_Sector_Ct","value":1,"thresh":10,"when_failed":"now","raw":{"value":4012}},` + + `"ata_smart_attributes":{"table":[{"id":5,"name":"Reallocated_Sector_Ct","value":1,"thresh":10,"when_failed":"now","flags":{"prefailure":true},"raw":{"value":4012}},` + `{"id":197,"name":"Current_Pending_Sector","value":100,"thresh":0,"when_failed":"","raw":{"value":16}}]},"ata_smart_error_log":{"summary":{"count":12}}}` ataPending = `{"smartctl":{"exit_status":64},"model_name":"WDC WUH722222ALE6L4","serial_number":"A3","smart_support":{"available":true,"enabled":true},"smart_status":{"passed":true},` + `"ata_smart_attributes":{"table":[{"id":197,"name":"Current_Pending_Sector","value":100,"thresh":0,"when_failed":"","raw":{"value":8}}]},"ata_smart_error_log":{"summary":{"count":0}}}` noSMART = `{"smartctl":{"exit_status":4,"messages":[{"string":"Read Device Identity failed: Unknown","severity":"error"}]},"smart_support":{"available":false}}` ) + +type ctxT = context.Context + +// smartctlLine is the command the watch runs for one drive, as root. +func smartctlLine(device string) string { + return "/usr/bin/smartctl --json=c -a -n standby,2,3 /dev/" + device +} + +func chmod(path string, mode os.FileMode) error { return os.Chmod(path, mode) } + +func modeOf(t *testing.T, path string) os.FileMode { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + return info.Mode().Perm() +} + +// fresh writes readings the watch has just written, with every drive of the server passed. +func (f *fake) fresh() { + f.readings(f.now, bootID, map[string]string{"sda": ataPassed, "sdb": ataPassed, "sdc": ataPassed}) +} diff --git a/modules/sensors/cmd/sensors/machine.go b/modules/sensors/cmd/sensors/machine.go index 925b3669..9c63e7d8 100644 --- a/modules/sensors/cmd/sensors/machine.go +++ b/modules/sensors/cmd/sensors/machine.go @@ -22,8 +22,13 @@ import ( // node-engine runs as root, reads it with smartctl and leaves its readings in one file the tools read // (novox/hq ADR 0256). -// CommandTimeout bounds one smartctl: a drive that does not answer costs one line, never the watch. -const CommandTimeout = 20 * time.Second +// CommandTimeout bounds one smartctl: a drive that does not answer costs one line, never the watch. A +// variable only so a test can shorten it. +var CommandTimeout = 20 * time.Second + +// WaitDelay is how long a command ended for its timeout may keep its output open (a child it left behind) +// before the pipes are closed under it: without it, the wait for the output outlives the timeout. +const WaitDelay = 2 * time.Second // Ran is what one command did: its output and exit status, and why it never ran to an answer. type Ran struct { @@ -43,12 +48,13 @@ func ExecRunner(ctx context.Context, name string, args ...string) Ran { defer cancel() cmd := exec.CommandContext(ctx, name, args...) cmd.Env = append(os.Environ(), "LC_ALL=C") + cmd.WaitDelay = WaitDelay var out, errb bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &errb err := cmd.Run() r := Ran{Stdout: out.String(), Stderr: errb.String()} - if ctx.Err() == context.DeadlineExceeded { - r.Status, r.Err = 124, fmt.Sprintf("no answer within %d s", int(CommandTimeout.Seconds())) + if ctx.Err() == context.DeadlineExceeded || errors.Is(err, exec.ErrWaitDelay) { + r.Status, r.Err = 124, fmt.Sprintf("no answer within %s", CommandTimeout) return r } var exit *exec.ExitError diff --git a/modules/sensors/cmd/sensors/manifest_test.go b/modules/sensors/cmd/sensors/manifest_test.go index 9fcf9829..11b2d93d 100644 --- a/modules/sensors/cmd/sensors/manifest_test.go +++ b/modules/sensors/cmd/sensors/manifest_test.go @@ -79,7 +79,7 @@ func TestTheHealthIsTheDriveDaemonsUnitAndTheWatchsToolCheck(t *testing.T) { t.Errorf("smartd never wakes a drive and sends no mail: %q", conf) } dropIn, _ := byID["smartd-drop-in"]["content"].(string) - for _, want := range []string{"ConditionVirtualization=\n", "ExecStart=\nExecStart=/usr/bin/smartd -n -q never\n"} { + for _, want := range []string{"ConditionVirtualization=\n", "ExecStart=\nExecStart=/usr/bin/smartd -n -q never -i 3600\n"} { if !strings.Contains(dropIn, want) { t.Errorf("the drop-in misses %q", want) } diff --git a/modules/sensors/cmd/sensors/review_test.go b/modules/sensors/cmd/sensors/review_test.go new file mode 100644 index 00000000..f5460042 --- /dev/null +++ b/modules/sensors/cmd/sensors/review_test.go @@ -0,0 +1,294 @@ +package main + +// The reproductions of the review of 2026-10-08 (mesh-catalog #129): each fails on the first build of the +// module and passes on the fix. + +import ( + "context" + "strings" + "testing" + "time" +) + +const ( + // smartmontools 7.5's own answer for a drive in standby with -n standby,2,3: the power mode is set + // before the information message, and the exit status is the one -n asked for (ataprint.cpp). + standbyJSON = `{"json_format_version":[1,0],"smartctl":{"version":[7,5],"messages":[{"string":"Device is in STANDBY mode, exit(2)",` + + `"severity":"information"}],"exit_status":2},"device":{"name":"/dev/sda","info_name":"/dev/sda [SAT]","type":"sat","protocol":"ATA"},` + + `"power_mode":{"ata_value":0,"name":"STANDBY"}}` + // A bridge that cannot answer CHECK POWER MODE, with -n standby,2,3: exit 3 and no power mode. + noPowerModeJSON = `{"smartctl":{"messages":[{"string":"CHECK POWER MODE not implemented, exit(3)","severity":"information"}],` + + `"exit_status":3},"device":{"name":"/dev/sdd","type":"sat","protocol":"ATA"}}` + // A drive that could not be opened, whose model happens to hold the word SLEEP: exit 2, no power mode. + openFailedJSON = `{"smartctl":{"messages":[{"string":"Smartctl open device: /dev/sda failed: No such device","severity":"error"}],` + + `"exit_status":2},"model_name":"SLEEPY DRIVE"}` + nvmeHot = `{"smartctl":{"exit_status":8},"model_name":"Samsung SSD 990 PRO 2TB","smart_status":{"passed":false,"nvme":{"value":2}},` + + `"nvme_smart_health_information_log":{"critical_warning":2,"available_spare":100,"available_spare_threshold":10,"percentage_used":1,"media_errors":0}}` + nvmeNoStatusDegraded = `{"smartctl":{"exit_status":0},"model_name":"Old NVMe","nvme_smart_health_information_log":{"critical_warning":4,"available_spare":100,` + + `"available_spare_threshold":10,"percentage_used":20,"media_errors":0}}` + nvmeNoStatusWell = `{"smartctl":{"exit_status":0},"model_name":"Old NVMe","nvme_smart_health_information_log":{"critical_warning":0,"available_spare":100,` + + `"available_spare_threshold":10,"percentage_used":20,"media_errors":0}}` + // An old-age attribute failing now, and another that failed in the past; the overall verdict passed. + ataOldAge = `{"smartctl":{"exit_status":32},"model_name":"ST16000NM001G","smart_support":{"available":true,"enabled":true},"smart_status":{"passed":true},` + + `"ata_smart_attributes":{"table":[{"id":5,"name":"Reallocated_Sector_Ct","value":100,"thresh":10,"when_failed":"","flags":{"prefailure":true},"raw":{"value":0}},` + + `{"id":194,"name":"Temperature_Celsius","value":40,"thresh":45,"when_failed":"now","flags":{"prefailure":false},"raw":{"value":60}},` + + `{"id":190,"name":"Airflow_Temperature_Cel","value":40,"thresh":45,"when_failed":"past","flags":{"prefailure":false},"raw":{"value":60}}]}}` +) + +func TestTheNVMeTemperatureBitAloneIsHighNotFailing(t *testing.T) { + f := newFake(t) + f.laptop() + f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmeHot}) + l := f.machine().LookNow() + d := l.Drives[0] + if d.Flag != FlagHigh || d.Health != "passed" { + t.Errorf("the temperature bit alone is HIGH, and the verdict it implies is not a failure: %+v", d) + } + if !l.Health().Healthy { + t.Errorf("health = %+v", l.Health()) + } +} + +func TestNVMeWithoutSmartStatusIsJudgedFromItsHealthLog(t *testing.T) { + f := newFake(t) + f.laptop() + f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmeNoStatusDegraded}) + if d := f.machine().LookNow().Drives[0]; d.Flag != FlagFailing { + t.Errorf("a degraded log is failing without smart_status: %+v", d) + } + f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmeNoStatusWell}) + if d := f.machine().LookNow().Drives[0]; d.Health != "passed" || d.Flag != "" { + t.Errorf("a clean log has passed: %+v", d) + } +} + +func TestAnOldAgeAttributeFailingNowAndAPastFailureAreHigh(t *testing.T) { + f := newFake(t) + f.server() + f.readings(f.now, bootID, map[string]string{"sda": ataOldAge}) + l := f.machine().LookNow() + d := l.Drives[0] + if d.Flag != FlagHigh || d.Health != "passed" { + t.Fatalf("an old-age attribute and a past failure are HIGH: %+v", d) + } + joined := strings.Join(d.Findings, "; ") + for _, want := range []string{"Temperature_Celsius", "Airflow_Temperature_Cel was at or below its own threshold in the past"} { + if !strings.Contains(joined, want) { + t.Errorf("findings miss %q: %s", want, joined) + } + } + if !l.Health().Healthy { + t.Errorf("health = %+v", l.Health()) + } +} + +func TestAZeroFanGPUUnderItsMinimumIsNotStopped(t *testing.T) { + f := newFake(t) + f.laptop() + f.files("/sys/class/hwmon/hwmon12", map[string]string{"name": "amdgpu", "fan1_input": "0", "fan1_min": "500", "temp1_input": "45000", "temp1_label": "edge"}) + f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmePassed}, "amdgpu/fan1") + l := f.machine().LookNow() + var gpu Fan + for _, x := range l.Fans { + if x.Key == "amdgpu/fan1" { + gpu = x + } + } + if gpu.Flag != "" || gpu.Note == "" { + t.Errorf("a zero-fan GPU at rest is said, not flagged: %+v", gpu) + } + if !l.Health().Healthy { + t.Errorf("health = %+v", l.Health()) + } +} + +func TestALatchedAlarmOnATurningFanRaisesNothing(t *testing.T) { + f := newFake(t) + f.server() + f.file("/sys/class/hwmon/hwmon1/fan1_alarm", "1") + f.readings(f.now, bootID, map[string]string{"sda": ataPassed, "sdb": ataPassed, "sdc": ataPassed}, "nct6775/fan1") + l := f.machine().LookNow() + if l.Fans[0].Flag != "" || !strings.Contains(l.Fans[0].Note, "alarm") { + t.Errorf("a latched alarm while turning is said, not flagged: %+v", l.Fans[0]) + } + if !l.Health().Healthy { + t.Errorf("health = %+v", l.Health()) + } +} + +func TestAnUnrelatedHotSensorDoesNotMakeAFanStopped(t *testing.T) { + f := newFake(t) + f.laptop() + f.file("/sys/class/hwmon/hwmon7/temp1_input", "84000") // the NVMe drive past its own maximum + f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmePassed}, "asus/fan1", "asus/fan2") + l := f.machine().LookNow() + if l.Fans[2].Flag != "" { + t.Fatalf("a hot drive says nothing of the GPU fan: %+v", l.Fans[2]) + } + // The board's zone at its hot trip does. + f.file("/sys/class/thermal/thermal_zone0/temp", "124000") + l = f.machine().LookNow() + if l.Fans[2].Flag != FlagStopped { + t.Fatalf("the board at its hot trip with a fan that ran reading 0: %+v", l.Fans[2]) + } +} + +func TestAFanIsStoppedByAHotSensorOnItsOwnChip(t *testing.T) { + f := newFake(t) + f.server() + f.files("/sys/class/hwmon/hwmon9", map[string]string{"name": "pwmchip", "fan1_input": "0", "temp1_input": "71000", "temp1_max": "70000"}) + f.readings(f.now, bootID, map[string]string{"sda": ataPassed, "sdb": ataPassed, "sdc": ataPassed}, "pwmchip/fan1") + h := f.machine().LookNow().Health() + if h.Healthy || !strings.Contains(h.Why, "at 71.0 °C, at or above its own maximum of 70.0 °C") { + t.Errorf("health = %+v", h) + } +} + +func TestAPhantom127SensorIsNotReadAndABoardChipsCriticalRaisesNothing(t *testing.T) { + f := newFake(t) + f.server() + f.files("/sys/class/hwmon/hwmon1", map[string]string{"temp1_input": "105000", "temp1_label": "SYSTIN", "temp1_crit": "100000", + "temp2_input": "127000", "temp2_label": "AUXTIN0", "temp2_crit": "100000", "temp3_input": "127500", "temp3_label": "AUXTIN1"}) + f.readings(f.now, bootID, map[string]string{"sda": ataPassed, "sdb": ataPassed, "sdc": ataPassed}) + l := f.machine().LookNow() + for _, x := range l.Temperatures { + if x.Chip != "nct6775" { + continue + } + switch x.Label { + case "SYSTIN": + if x.Flag != FlagCritical { + t.Errorf("SYSTIN is shown CRITICAL: %+v", x) + } + default: + if x.C != nil || x.NotRead == "" { + t.Errorf("%s at 127 °C is a phantom: %+v", x.Label, x) + } + } + } + if !l.Health().Healthy { + t.Errorf("a board chip's critical value drives no health: %+v", l.Health()) + } +} + +func TestZeroDegreesIsAReading(t *testing.T) { + f := newFake(t) + f.files("/sys/class/hwmon/hwmon0", map[string]string{"name": "outdoor", "temp1_input": "0"}) + ts := f.machine().Temperatures() + if len(ts) != 1 || ts[0].C == nil || *ts[0].C != 0 { + t.Errorf("0 °C is a reading: %+v", ts) + } +} + +func TestAMissingOrStaleWatchMakesTheModuleUnhealthy(t *testing.T) { + f := newFake(t) + f.laptop() + if h := f.machine().LookNow().Health(); h.Healthy || !strings.Contains(h.Why, "watch") { + t.Errorf("no readings at all: %+v", h) + } + f.readings(f.now.Add(-StaleAfter-time.Minute), bootID, map[string]string{"nvme0n1": nvmePassed}) + if h := f.machine().LookNow().Health(); h.Healthy || !strings.Contains(h.Why, "has not written its readings for") { + t.Errorf("stale readings: %+v", h) + } + f.readings(f.now.Add(-time.Minute), bootID, map[string]string{"nvme0n1": nvmePassed}) + if h := f.machine().LookNow().Health(); !h.Healthy { + t.Errorf("fresh readings: %+v", h) + } +} + +func TestSmartctlsStandbyAnswerIsAsleepOnlyWithItsPowerMode(t *testing.T) { + f := newFake(t) + f.drive("sda", "X", "ATA ", 100, true) + f.answers[smartctlLine("sda")] = Ran{Stdout: standbyJSON, Status: 2} + before := DriveReading{ReadAt: f.now.Add(-time.Hour), Smartctl: []byte(ataPassed)} + got := f.machine().ReadDrive("sda", &before) + if !got.Asleep || string(got.Smartctl) != ataPassed { + t.Errorf("standby keeps the reading before: %+v", got) + } + f.answers[smartctlLine("sda")] = Ran{Stdout: openFailedJSON, Status: 2} + got = f.machine().ReadDrive("sda", &before) + if got.Asleep { + t.Errorf("an open failure is not sleep, whatever its words: %+v", got) + } + f.answers[smartctlLine("sda")] = Ran{Stdout: noPowerModeJSON, Status: 3} + got = f.machine().ReadDrive("sda", nil) + if got.Asleep || !strings.Contains(got.NotRead, "cannot say whether it is asleep") { + t.Errorf("a bridge without a power mode is skipped and said: %+v", got) + } +} + +func TestASmartctlThatHangsCostsOneLineAndTheFansGoOn(t *testing.T) { + old := CommandTimeout + CommandTimeout = 300 * time.Millisecond + defer func() { CommandTimeout = old }() + start := time.Now() + // A child that keeps the output open after its parent is ended. + r := ExecRunner(context.Background(), "sh", "-c", "sleep 30 & sleep 30") + if took := time.Since(start); took > 5*time.Second { + t.Fatalf("a hung command held the watch for %s", took) + } + if r.Err == "" { + t.Errorf("the hang is said: %+v", r) + } + + f := newFake(t) + f.server() + release := make(chan struct{}) + m := f.machine() + m.Run = func(_ ctxT, name string, args ...string) Ran { <-release; return Ran{Stdout: ataPassed} } + w := NewWatch(m) + w.Async = true + if err := w.Look(); err != nil { + t.Fatal(err) + } + f.now = f.now.Add(FanEvery) + if err := w.Look(); err != nil { + t.Fatal(err) + } + r2, why := m.ReadReadings() + if why != "" || !r2.WrittenAt.Equal(f.now) || len(r2.FansRan) != 1 { + t.Errorf("the fans are looked at and written while a drive hangs: %+v %q", r2, why) + } + close(release) + w.WaitDrives() +} + +func TestASpinningDriveIsReadAtMostHourlyAndASolidStateOneEveryTenMinutes(t *testing.T) { + f := newFake(t) + f.server() + f.drive("nvme0n1", "N", "", 100, false) + for _, d := range []string{"sda", "sdb", "sdc"} { + f.answers[smartctlLine(d)] = Ran{Stdout: ataPassed} + } + f.answers[smartctlLine("nvme0n1")] = Ran{Stdout: nvmePassed} + w := NewWatch(f.machine()) + if err := w.Look(); err != nil || len(f.calls) != 4 { + t.Fatalf("first look reads every drive: %q %v", f.calls, err) + } + f.calls = nil + f.now = f.now.Add(SolidStateEvery) + _ = w.Look() + if len(f.calls) != 1 || !strings.HasSuffix(f.calls[0], "/dev/nvme0n1") { + t.Fatalf("after ten minutes only the solid-state drive: %q", f.calls) + } + f.calls = nil + f.now = f.now.Add(SpinningEvery) + _ = w.Look() + if len(f.calls) != 4 { + t.Fatalf("after an hour every drive: %q", f.calls) + } +} + +func TestAnExistingReadingsDirectoryGetsItsMode(t *testing.T) { + f := newFake(t) + f.dir("/run/mesh-sensors") + if err := chmod(f.root+"/run/mesh-sensors", 0o700); err != nil { + t.Fatal(err) + } + if err := f.machine().WriteReadings(Readings{}); err != nil { + t.Fatal(err) + } + if mode := modeOf(t, f.root+"/run/mesh-sensors"); mode != 0o755 { + t.Errorf("mode %o", mode) + } +} diff --git a/modules/sensors/cmd/sensors/sensors_test.go b/modules/sensors/cmd/sensors/sensors_test.go index 931a4b41..387ac156 100644 --- a/modules/sensors/cmd/sensors/sensors_test.go +++ b/modules/sensors/cmd/sensors/sensors_test.go @@ -54,6 +54,7 @@ func TestATemperatureIsHighAtItsMaximumAndCriticalAtItsCriticalValue(t *testing. f.server() f.file("/sys/class/hwmon/hwmon0/temp2_input", "80000") f.file("/sys/class/hwmon/hwmon0/temp1_input", "100000") + f.fresh() ts := f.machine().Temperatures() if core := find(ts, "coretemp", "Core 0"); core.Flag != FlagHigh { t.Errorf("at its own maximum it is HIGH: %+v", core) @@ -63,7 +64,7 @@ func TestATemperatureIsHighAtItsMaximumAndCriticalAtItsCriticalValue(t *testing. t.Fatalf("at its own critical value it is CRITICAL: %+v", pkg) } h := f.machine().LookNow().Health() - want := "The CPU (coretemp Package id 0) is at 100 °C, at or above its own critical value of 100 °C." + want := "The CPU (coretemp Package id 0) is at 100.0 °C, at or above its own critical value of 100.0 °C." if h.Healthy || h.Why != want { t.Errorf("health = %+v, want unhealthy with %q", h, want) } @@ -121,7 +122,7 @@ func TestADriveThatSaysItIsFailingMakesTheModuleUnhealthy(t *testing.T) { f := newFake(t) f.server() f.readings(f.now, bootID, map[string]string{"sda": ataPassed, "sdb": ataFailing, "sdc": ataPending}) - ds, note := f.machine().Drives() + ds, note := f.machine().Drives(f.machine().ReadReadings()) if note != "" || len(ds) != 3 { t.Fatalf("three drives, the RAID volume left out, no note: %d, %q", len(ds), note) } @@ -146,7 +147,7 @@ func TestAnNVMeCriticalWarningBitIsFailing(t *testing.T) { f := newFake(t) f.laptop() f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmeSpareLow}) - ds, _ := f.machine().Drives() + ds, _ := f.machine().Drives(f.machine().ReadReadings()) d := ds[0] if d.Flag != FlagFailing || *d.CriticalWarning != 1 || *d.PercentageUsed != 97 || d.Model != "Samsung SSD 990 PRO 2TB" { t.Fatalf("spare below threshold is failing: %+v", d) @@ -183,11 +184,11 @@ func TestAHealthyLaptopReadsHealthyWithItsDriveRead(t *testing.T) { func TestAMissingStaleOrEarlierBootReadingIsSaid(t *testing.T) { f := newFake(t) f.laptop() - if ds, note := f.machine().Drives(); ds[0].Health != "not read" || !strings.Contains(note, "has left no reading") { + if ds, note := f.machine().Drives(f.machine().ReadReadings()); ds[0].Health != "not read" || !strings.Contains(note, "has left no reading") { t.Errorf("no file: %+v %q", ds[0], note) } f.readings(f.now.Add(-2*time.Hour), bootID, map[string]string{"nvme0n1": nvmePassed}) - if ds, note := f.machine().Drives(); ds[0].Health != "passed" || note != "the module's watch last wrote 2.0 h ago; it may not be running" { + if ds, note := f.machine().Drives(f.machine().ReadReadings()); ds[0].Health != "passed" || note != "the module's watch last wrote 2.0 h ago; it may not be running" { t.Errorf("stale: %+v %q", ds[0], note) } f.readings(f.now, "an-earlier-boot", map[string]string{"nvme0n1": nvmeSpareLow}, "asus/fan2") @@ -208,24 +209,25 @@ func TestAFanAtRestOnACoolMachineIsNotStopped(t *testing.T) { } } -func TestAFanThatRanAndReadsZeroWhileTheMachineIsHotHasStopped(t *testing.T) { +func TestAFanThatRanAndReadsZeroWhileTheBoardIsHotHasStopped(t *testing.T) { f := newFake(t) f.laptop() - f.file("/sys/class/hwmon/hwmon7/temp1_input", "84000") + f.file("/sys/class/thermal/thermal_zone0/temp", "124500") f.readings(f.now, bootID, map[string]string{"nvme0n1": nvmePassed}, "asus/fan1", "asus/fan2") l := f.machine().LookNow() if l.Fans[2].Flag != FlagStopped { - t.Fatalf("gpu_fan ran and reads 0 while the drive is past its maximum: %+v", l.Fans[2]) + t.Fatalf("gpu_fan ran and reads 0 while the board is past its hot trip: %+v", l.Fans[2]) } h := l.Health() - want := "The fan gpu_fan (asus) has stopped: it ran earlier and reads 0 RPM while the drive nvme0n1 (WD PC SN560 SDDPNQE-1T00-1102), " + - "sensor Composite is at 84 °C, at or above its own maximum of 84 °C." + want := "The board (acpitz thermal_zone0) is at 124.5 °C, at or above the zone's critical trip of 124.0 °C. " + + "The fan gpu_fan (asus) has stopped: it ran earlier and reads 0 RPM while the board (acpitz thermal_zone0) " + + "is at 124.5 °C, at or above the zone's hot trip of 124.0 °C." if h.Healthy || h.Why != want { t.Errorf("health = %+v\nwant %q", h, want) } } -func TestAnEmptyHeaderIsNotAStoppedFanAndAFanUnderItsMinimumIs(t *testing.T) { +func TestAnEmptyHeaderAndAFanUnderItsMinimumAreSaidAndNotFlagged(t *testing.T) { f := newFake(t) f.server() l := f.machine().LookNow() @@ -234,14 +236,15 @@ func TestAnEmptyHeaderIsNotAStoppedFanAndAFanUnderItsMinimumIs(t *testing.T) { } f.file("/sys/class/hwmon/hwmon1/fan1_input", "200") l = f.machine().LookNow() - if l.Fans[0].Flag != FlagStopped || l.Fans[0].Why != "it turns at 200 RPM, under its own minimum of 300 RPM" { - t.Errorf("under its own minimum: %+v", l.Fans[0]) + if l.Fans[0].Flag != "" || l.Fans[0].Note != "it turns at 200 RPM, under its chip's minimum of 300 RPM" { + t.Errorf("under its own minimum is a note: %+v", l.Fans[0]) } } func TestMemoryErrorsAreCountedAndAnUncorrectableOneIsUnhealthy(t *testing.T) { f := newFake(t) f.server() + f.fresh() f.file("/sys/devices/system/edac/mc/mc0/ce_count", "3") f.file("/sys/devices/system/edac/mc/mc0/dimm0/dimm_ce_count", "3") l := f.machine().LookNow() @@ -277,9 +280,9 @@ func TestPCIeErrorsAndThrottlingAreCountedWhereTheMachineHasThem(t *testing.T) { func TestTheWatchReadsEachDriveAsRootWithoutWakingItAndNotesTheFans(t *testing.T) { f := newFake(t) f.server() - f.answers["smartctl --json=c -a -n standby /dev/sda"] = Ran{Stdout: ataPassed} - f.answers["smartctl --json=c -a -n standby /dev/sdb"] = Ran{Stdout: ataFailing, Status: 24} - f.answers["smartctl --json=c -a -n standby /dev/sdc"] = Ran{Stdout: "Device is in STANDBY mode, exit(2)\n", Status: 2} + f.answers[smartctlLine("sda")] = Ran{Stdout: ataPassed} + f.answers[smartctlLine("sdb")] = Ran{Stdout: ataFailing, Status: 24} + f.answers[smartctlLine("sdc")] = Ran{Stdout: strings.Replace(standbyJSON, "/dev/sda", "/dev/sdc", 1), Status: 2} w := NewWatch(f.machine()) if err := w.Look(); err != nil { t.Fatal(err) @@ -298,15 +301,15 @@ func TestTheWatchReadsEachDriveAsRootWithoutWakingItAndNotesTheFans(t *testing.T if _, ok := r.FansRan["nct6775/fan1"]; !ok || len(r.FansRan) != 1 { t.Errorf("the turning fan is noted, the empty header not: %v", r.FansRan) } - // Ten minutes have not passed: the fans are looked at, the drives are not read again. + // An hour has not passed: the fans are looked at, the spinning drives are not read again. f.now = f.now.Add(FanEvery) f.calls = nil if err := w.Look(); err != nil || len(f.calls) != 0 { - t.Fatalf("drives read again within ten minutes: %q %v", f.calls, err) + t.Fatalf("drives read again within the hour: %q %v", f.calls, err) } // A new watch (a restart) keeps what the last one read, and a drive asleep keeps its reading before. - f.now = f.now.Add(DriveEvery) - f.answers["smartctl --json=c -a -n standby /dev/sda"] = Ran{Stdout: "Device is in SLEEP mode, exit(2)\n", Status: 2} + f.now = f.now.Add(SpinningEvery) + f.answers[smartctlLine("sda")] = Ran{Stdout: strings.Replace(standbyJSON, `"STANDBY"}`, `"SLEEP"}`, 1), Status: 2} w2 := NewWatch(f.machine()) if err := w2.Look(); err != nil { t.Fatal(err) @@ -315,7 +318,7 @@ func TestTheWatchReadsEachDriveAsRootWithoutWakingItAndNotesTheFans(t *testing.T if !r.Drives["sda"].Asleep || len(r.Drives["sda"].Smartctl) == 0 { t.Errorf("sda asleep keeps its reading: %+v", r.Drives["sda"]) } - ds, _ := f.machine().Drives() + ds, _ := f.machine().Drives(f.machine().ReadReadings()) if ds[0].Health != "passed" || !ds[0].Asleep || ds[1].Health != "failing" { t.Errorf("drives = %+v", ds) } @@ -325,7 +328,7 @@ func TestTheWatchNotRootAsksThroughSudoAndSaysWhatStopsIt(t *testing.T) { f := newFake(t) f.uid = 1000 f.drive("sda", "X", "ATA ", 100, true) - f.answers["sudo -n smartctl --json=c -a -n standby /dev/sda"] = Ran{Stderr: "sudo: a password is required\n", Status: 1} + f.answers["sudo -n "+smartctlLine("sda")] = Ran{Stderr: "sudo: a password is required\n", Status: 1} got := f.machine().ReadDrive("sda", nil) if got.NotRead != "the watch is not root and may not run smartctl through sudo without a prompt" { t.Errorf("not read = %q", got.NotRead) @@ -342,6 +345,7 @@ func TestTheHealthAnswerIsWhatTheNodeEngineReads(t *testing.T) { f.server() f.file("/sys/class/hwmon/hwmon0/temp1_input", "101000") f.file("/sys/devices/system/edac/mc/mc0/ue_count", "2") + f.fresh() var tool func(map[string]any) (any, error) for _, tl := range Tools(f.machine()) { if tl.Name == "sensors_health" { diff --git a/modules/sensors/cmd/sensors/temperatures.go b/modules/sensors/cmd/sensors/temperatures.go index a89ceca0..a59909ac 100644 --- a/modules/sensors/cmd/sensors/temperatures.go +++ b/modules/sensors/cmd/sensors/temperatures.go @@ -43,6 +43,11 @@ type Temperature struct { Where string `json:"where"` } +// DrivesHealth says whether this sensor's critical value makes the module unhealthy. A board chip's +// (a Super I/O's) is not to be trusted: its inputs and their limits are whatever the firmware left, and a +// wrongly wired input reads far past them on a healthy machine. It is shown, and raises nothing. +func (t Temperature) DrivesHealth() bool { return !(t.Source == "hwmon" && t.Kind == "board") } + // Name is how a person reads which sensor this is. func (t Temperature) Name() string { what := t.Chip @@ -67,7 +72,12 @@ func (t Temperature) Name() string { // sane says whether a value in °C is a temperature or a threshold the hardware means as one. A // threshold given as the format's largest value (an NVMe sensor's 65261.85 °C) or as 0 means none, and // so does anything a machine cannot be at and still answer. -func sane(c float64) bool { return c > -40 && c < 200 && c != 0 } +func sane(c float64) bool { return reading(c) && c != 0 } + +// reading says whether a value in °C is a temperature a machine can be at. 0 °C is one. 127 and 127.5 °C +// are not: they are what a board chip's input with nothing on it answers (a phantom AUXTIN, the largest +// value of its eight-bit register), never a real sensor's reading. +func reading(c float64) bool { return c > -40 && c < 200 && c != 127 && c != 127.5 } func milli(v int64) *float64 { c := round1(float64(v) / 1000) @@ -157,7 +167,7 @@ func (m *Machine) hwmonTemperature(dir, chip, stem, drive string) Temperature { t.Kind = "drive" } v, ok := m.readInt(dir + "/" + stem + "_input") - if !ok || !sane(float64(v)/1000) { + if !ok || !reading(float64(v)/1000) { t.NotRead = "the sensor gave no temperature" if ok { t.NotRead = fmt.Sprintf("the sensor answered %.1f °C, which is no temperature", float64(v)/1000) @@ -190,7 +200,7 @@ func (m *Machine) zone(dir string) Temperature { t := Temperature{Source: "thermal zone", Chip: typ, Kind: kindOf(typ), Label: strings.TrimPrefix(dir, "/sys/class/thermal/"), Where: dir} v, ok := m.readInt(dir + "/temp") - if !ok || !sane(float64(v)/1000) { + if !ok || !reading(float64(v)/1000) { t.NotRead = "the zone gave no temperature" if ok { t.NotRead = fmt.Sprintf("the zone answered %.1f °C, which is no temperature", float64(v)/1000) diff --git a/modules/sensors/cmd/sensors/tools.go b/modules/sensors/cmd/sensors/tools.go index 1bf8e6d0..2e28bd98 100644 --- a/modules/sensors/cmd/sensors/tools.go +++ b/modules/sensors/cmd/sensors/tools.go @@ -33,9 +33,10 @@ func Tools(m *Machine) []stdio.Tool { }, { Name: "sensors_fans", - Description: "Every fan the machine reports: RPM, the chip's own minimum and alarm where it has them, and whether the " + - "module's watch saw it turning since the machine started. STOPPED when its chip says so, or when it ran and reads 0 " + - "while a temperature is at or above its own maximum; a fan resting at 0 on a cool machine is not stopped. " + + Description: "Every fan the machine reports: RPM, the chip's own minimum and alarm where it has them (said in a note, never " + + "flagged: a zero-fan GPU is under its minimum by design and a board chip latches its alarm), and whether the " + + "module's watch saw it turning since the machine started. STOPPED only when it ran and reads 0 while the CPU, the " + + "board or a sensor on its own chip is at or above its own maximum. " + "Replaces sensors. (r)", Run: func(map[string]any) (any, error) { l := m.LookNow() @@ -50,7 +51,7 @@ func Tools(m *Machine) []stdio.Tool { "error log and the last self-test. FAILING when the drive says so, HIGH for the early signs. Replaces smartctl -a " + "and nvme smart-log. (r)", Run: func(map[string]any) (any, error) { - ds, note := m.Drives() + ds, note := m.Drives(m.ReadReadings()) out := map[string]any{"drives": ds, "count": len(ds)} if note != "" { out["note"] = note @@ -71,9 +72,11 @@ func Tools(m *Machine) []stdio.Tool { { Name: "sensors_health", Description: "Whether the hardware is failing, as the module's health check (novox/hq ADR 0256): unhealthy when a " + - "temperature is at or above its own critical value, a drive says it is failing, a fan that ran has stopped while " + - "the machine is hot, or memory reports an uncorrectable error; with why, one sentence per finding. HIGH readings " + - "do not make it unhealthy. (r)", + "CPU, GPU, drive or zone temperature is at or above its own critical value (a board chip's is shown, never judged), " + + "a drive says it is failing, a fan that ran reads 0 while the CPU, the board or its own chip is at its own maximum, " + + "memory reports an uncorrectable error, or the module's watch has not written its readings for five minutes; with " + + "why, one sentence per finding. HIGH readings, a fan's alarm bit and a fan under its minimum do not make it " + + "unhealthy. (r)", Run: func(map[string]any) (any, error) { return m.LookNow().Health(), nil }, diff --git a/modules/sensors/cmd/sensors/watch.go b/modules/sensors/cmd/sensors/watch.go index 0d32a1fa..a7eefb6e 100644 --- a/modules/sensors/cmd/sensors/watch.go +++ b/modules/sensors/cmd/sensors/watch.go @@ -4,27 +4,44 @@ import ( "context" "fmt" "os" + "sync" "time" ) // The watch: the module's long-running process, run by the node-engine as root (novox/hq ADR 0256). It -// reads each drive's health every DriveEvery, so the tools, which run as the operator account, need no -// root; it never wakes a drive in standby; and it notes every FanEvery which fans are turning, so a fan -// that ran and stopped can be told from one that never runs. It writes ReadingsPath and nothing else. +// reads each drive's health, so the tools, which run as the operator account, need no root; it never wakes +// a drive in standby; and it notes every FanEvery which fans are turning, so a fan that ran and stopped can +// be told from one that never runs. It writes ReadingsPath and nothing else. // -// The module's health is judged on it: by its unit (alive), and by asking sensors_health what the -// hardware says. +// **The drives never hold up the fans.** In Run, the drives are read beside the looks at the fans, one +// pass at a time, each smartctl bounded by CommandTimeout; the file is written every FanEvery whatever a +// drive does, so a watch whose file grows old is a watch that is hung or gone, and the health check says +// so (StaleAfter). +// +// **A spinning drive is read at most hourly.** Reading SMART from an awake drive can restart its standby +// timer, so a drive read more often than its timer is set for never spins down. Whether the machines' +// drives are set to spin down at all is not known to the mesh (no tool reads a drive's standby timer), so +// the watch reads a rotational drive every SpinningEvery and smartd checks every drive at the same +// interval (its -i, in the module's drop-in). A drive in standby is never woken: smartctl is asked with -n. const ( - FanEvery = 30 * time.Second - DriveEvery = 10 * time.Minute + FanEvery = 30 * time.Second + // SolidStateEvery is how often a drive that does not spin is read. + SolidStateEvery = 10 * time.Minute + // SpinningEvery is how often a rotational drive is read: longer than a spin-down timer is commonly set. + SpinningEvery = time.Hour ) // Watch keeps the readings between looks. type Watch struct { - M *Machine + M *Machine + // Async reads the drives beside the fans instead of before them; Run sets it. + Async bool + + mu sync.Mutex readings Readings - lastRead time.Time + reading bool + done sync.WaitGroup } // NewWatch starts from what an earlier watch of this boot left, so a restart neither forgets the fans @@ -39,48 +56,100 @@ func NewWatch(m *Machine) *Watch { if w.readings.FansRan == nil { w.readings.FansRan = map[string]time.Time{} } - for _, d := range r.Drives { - if d.ReadAt.After(w.lastRead) { - w.lastRead = d.ReadAt - } - } } w.readings.BootID = m.bootID() return w } -// Look is one look: the fans always, the drives when DriveEvery has passed. It writes the file. +// due lists the drives whose next look has come: never looked at, or looked at longer ago than their +// cadence. +func (w *Watch) due(now time.Time) (due []Drive, present map[string]bool) { + present = map[string]bool{} + for _, d := range w.M.BlockDrives() { + present[d.Device] = true + every := SolidStateEvery + if d.Rotational { + every = SpinningEvery + } + last, ok := w.readings.Drives[d.Device] + looked := last.LookedAt + if looked.IsZero() { + looked = last.ReadAt + } + if !ok || now.Sub(looked) >= every { + due = append(due, d) + } + } + return due, present +} + +// Look is one look: the fans always, the drives that are due. It writes the file. func (w *Watch) Look() error { now := w.M.Now() - for _, f := range w.M.Fans() { + fans := w.M.Fans() + w.mu.Lock() + for _, f := range fans { if f.RPM != nil && *f.RPM > 0 { w.readings.FansRan[f.Key] = now } } - if w.lastRead.IsZero() || now.Sub(w.lastRead) >= DriveEvery { - present := map[string]bool{} - for _, d := range w.M.BlockDrives() { - present[d.Device] = true - var before *DriveReading - if b, ok := w.readings.Drives[d.Device]; ok && len(b.Smartctl) > 0 { - before = &b - } - w.readings.Drives[d.Device] = w.M.ReadDrive(d.Device, before) + due, present := w.due(now) + for name := range w.readings.Drives { + if !present[name] { + delete(w.readings.Drives, name) } - for name := range w.readings.Drives { - if !present[name] { - delete(w.readings.Drives, name) - } - } - w.lastRead = now } + start := len(due) > 0 && !w.reading + if start { + w.reading = true + } + w.mu.Unlock() + if start { + w.done.Add(1) + if w.Async { + go w.readDrives(due) + } else { + w.readDrives(due) + } + } + return w.write() +} + +// readDrives reads each due drive, one after another, and keeps each reading as it comes. +func (w *Watch) readDrives(due []Drive) { + defer w.done.Done() + for _, d := range due { + w.mu.Lock() + var before *DriveReading + if b, ok := w.readings.Drives[d.Device]; ok && len(b.Smartctl) > 0 { + before = &b + } + w.mu.Unlock() + got := w.M.ReadDrive(d.Device, before) + w.mu.Lock() + w.readings.Drives[d.Device] = got + w.mu.Unlock() + } + w.mu.Lock() + w.reading = false + w.mu.Unlock() +} + +// WaitDrives waits for a pass over the drives that is under way. +func (w *Watch) WaitDrives() { w.done.Wait() } + +func (w *Watch) write() error { + w.mu.Lock() + defer w.mu.Unlock() w.readings.WrittenAt = w.M.Now() return w.M.WriteReadings(w.readings) } -// Run looks at once and every FanEvery until ctx ends. A look that cannot write is said on standard -// error, which the node-engine keeps in the unit's journal, and the watch goes on. +// Run looks at once and every FanEvery until ctx ends, the drives beside the fans. A look that cannot +// write is said on standard error, which the node-engine keeps in the unit's journal, and the watch goes +// on. func (w *Watch) Run(ctx context.Context) { + w.Async = true t := time.NewTicker(FanEvery) defer t.Stop() for { diff --git a/modules/sensors/module.json b/modules/sensors/module.json index 49f141dc..7abe7a4f 100644 --- a/modules/sensors/module.json +++ b/modules/sensors/module.json @@ -24,7 +24,7 @@ "type": "file", "path": "/etc/smartd.conf", "mode": "0644", - "content": "# The mesh's (module sensors, novox/hq ADR 0256): what smartd watches. Written whole at every push;\n# an edit here is overwritten.\n#\n# Every drive smartd can find, with every check (-a). A drive in standby is skipped, never woken to be\n# checked, and the skip is not logged (-n standby,q): a server's spinning drives may sleep. No mail is\n# sent (no -m): what smartd finds goes to the journal, and the module's watch reads each drive's health\n# for the mesh's tools and its health check.\nDEVICESCAN -a -n standby,q\n" + "content": "# The mesh's (module sensors, novox/hq ADR 0256): what smartd watches. Written whole at every push;\n# an edit here is overwritten.\n#\n# Every drive smartd can find, with every check (-a). A drive in standby is skipped, never woken to be\n# checked, and the skip is not logged (-n standby,q): a server's spinning drives may sleep. A drive\n# behind a USB bridge that cannot say whether it is asleep is checked anyway by smartd, and may be woken\n# (novox/hq ADR 0256); the watch does not read such a drive. No mail is\n# sent (no -m): what smartd finds goes to the journal, and the module's watch reads each drive's health\n# for the mesh's tools and its health check.\nDEVICESCAN -a -n standby,q\n" }, { "id": "smartd-drop-ins", @@ -37,7 +37,7 @@ "type": "file", "path": "/etc/systemd/system/smartd.service.d/10-mesh.conf", "mode": "0644", - "content": "# The mesh's (module sensors, novox/hq ADR 0256). Written whole at every push; edit nothing here.\n#\n# smartd runs on every machine the module is assigned to, a virtual machine included, where the package's\n# unit would not start; and it stays up on a machine with no drive it can watch (-q never), where it\n# would otherwise exit, because the mesh judges this service as one that stays up.\n[Unit]\nConditionVirtualization=\n\n[Service]\nExecStart=\nExecStart=/usr/bin/smartd -n -q never\n" + "content": "# The mesh's (module sensors, novox/hq ADR 0256). Written whole at every push; edit nothing here.\n#\n# smartd runs on every machine the module is assigned to, a virtual machine included, where the package's\n# unit would not start; and it stays up on a machine with no drive it can watch (-q never), where it\n# would otherwise exit, because the mesh judges this service as one that stays up. It checks each drive\n# hourly (-i 3600), not every half hour: a drive read more often than its standby timer never spins down,\n# and the module's watch reads a spinning drive hourly too.\n[Unit]\nConditionVirtualization=\n\n[Service]\nExecStart=\nExecStart=/usr/bin/smartd -n -q never -i 3600\n" }, { "id": "smartd",