Repository navigation
fix(notify): notify on manufacturer SMART failures at every notify level - #1126
Merged
Merged
Conversation
smartctl JSON emits "now" and "past" for ata_smart_attributes when_failed, but the constants were FAILING_NOW and IN_THE_PAST, so no ATA attribute was ever flagged FailedSmart or given the past-failure warning. Switch the constants to the real values (comparison stays case-insensitive). The legacy strings are never produced by smartctl and nothing stored or tested relies on them, so they are not kept. Drives with a "past" attribute now show WarningScrutiny and drives with a "now" attribute show FailedSmart at attribute level. Stored data is not changed; status is recomputed on the next ingest from the persisted raw when_failed field. (cherry picked from commit e2e6ddd)
…evel ShouldNotify only looked at the device status through the shortcut for Filter=All, Repeat=Always and a level other than Warn. Every other combination decided on failing attributes alone, so a drive whose smartctl overall health check failed (smart_status.passed=false) sent nothing when no single attribute was flagged: a vendor attribute with no Scrutiny thresholds such as Helium_Level, or an attribute an override ignores. A manufacturer SMART failure in this submission now qualifies at every notify level and attribute filter. The status threshold still applies, so Threshold=Scrutiny ignores it. With repeat notifications off it notifies only when the stored device status before this submission did not already carry FailedSmart; the handler passes that status from ResolveDevice, read before reconcileDeviceStatus updates it. Adds the reporter's smart-fail2-helium-only.json fixture and checks that ingest flags attribute 22 from "when_failed": "now". Fixes #1108
Starosdev
pushed a commit
that referenced
this pull request
Nov 5, 2026
## [1.80.0](v1.79.3...v1.80.0) (2026-11-05) ### Features * **btrfs:** notify on degraded filesystems, device errors and scrub errors ([#1127](#1127)) ([36e1e23](36e1e23)), closes [#1110](#1110) * manual release trigger ([44f3e44](44f3e44)) * **notify:** alert when Btrfs or ZFS usage reaches a configurable threshold ([#1133](#1133)) ([6e79118](6e79118)), closes [#1127](#1127) [#1132](#1132) [#1127](#1127) * **zfs:** notify on unhealthy pools, error totals and scrub errors ([#1131](#1131)) ([9c6544c](9c6544c)), closes [#1127](#1127) [#1130](#1130) [#1127](#1127) ### Bug Fixes * **database:** inline validated ids in Btrfs and ZFS history queries for InfluxDB OSS ([#1111](#1111)) ([#1124](#1124)) ([b0a1c69](b0a1c69)), closes [#157](#157) [#155](#155) [#157](#157) [#157](#157) * **mdadm:** honor repeat notifications and retry failed degraded alerts ([#1128](#1128)) ([62f9cef](62f9cef)), closes [#1109](#1109) * **notify:** notify on manufacturer SMART failures at every notify level ([#1126](#1126)) ([5cb3622](5cb3622))
9 of 15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Manufacturer SMART failures now send a notification at notify level Warn, and in every other settings combination. Two defects caused the bug. Both are fixed here for 1.x.
when_failedconstants did not match smartctl's output. smartctl writes"now"and"past", but 1.x compared the value against"FAILING_NOW"and"IN_THE_PAST". An ATA attribute was therefore never flagged from the drive's own threshold. This PR cherry-picks the fix already onbeta(e2e6ddd8, part of fix(smart): detect manufacturer SMART failures for ATA and NVMe and keep them through recalculation #970).ShouldNotifyignored a failed overall health check (smart_status.passed=false) unless one attribute was also flagged. Only the shortcut for Filter=All, Repeat=Always and a level other than Warn checked the device status. So Warn, Filter=Critical, and Repeat=Only-on-change all dropped the failure. Thebetacode has the same defect.Type of Change
Related Issues
Fixes #1108
Changes Made
constants.go,smart_ata_attribute.go: matchwhen_failedagainst"now"/"past"withstrings.EqualFold. This is a cherry-pick ofe2e6ddd8frombeta. The only conflict was in a test file, where both branches had added tests at the end.notify.go: if this submission hasDeviceStatusFailedSmart, and the status threshold covers it,ShouldNotifynow returns true at every notify level and attribute filter. The check uses this submission's status (smartAttrs.Status), not the stored device status. The stored status keeps failure bits until a fully passing submission resets it. With Threshold=Scrutiny, the failure is still ignored.FailedSmart.ShouldNotifygets a newpreviousDeviceStatusargument. The upload handler passesdevice.DeviceStatusfromResolveDevice, which is read beforereconcileDeviceStatusupdates it. No new database read and no schema change.smart-fail2-helium-only.json, provided by the reporter: only Helium_Level (22) fails, with"when_failed": "now". It has no Scrutiny thresholds.Testing
I have tested this locally
I have added/updated tests that prove my fix/feature works
All existing tests pass (CI green; one test fails only locally, see below)
go build ./...andgo vetonpkg/notify,pkg/models/measurementsandpkg/web/handler: clean.go test ./pkg/models/measurements/ ./pkg/web/handler/: pass.go test ./pkg/notify/: everything passes exceptTestScriptStreamsOutputAndPreservesEnvironment, which fails locally on macOS (Condition never satisfied). That test exercisessendScriptNotification, which this PR does not change. It passes in CI (Test Backendgreen at6b56b400), so the failure is specific to the local macOS environment.I confirmed that the new tests detect the bugs. With the
ShouldNotifycheck disabled and the old"FAILING_NOW"constant restored,TestShouldNotify_OverallHealthFailed_QualifiesAtEveryLevelAndFilter,..._NoRepeatNotifiesOnlyOnTransition,TestPopulateAttributeStatus_WhenFailedandTestFromCollectorSmartInfo_Fail_ManufacturerThresholdOnlyAttributeall fail.Checklist
Additional Notes
Review: not required (the change lands on
developand reaches users only through a promotion PR and a manual release. It does not change a security boundary or a shared contract).Verdict change for existing drives: fixing
when_failedchanges the verdicts for drives that are already monitored."past"becomesWarningScrutiny. At notify level Warn with repeat notifications on, that drive now sends a notification on every submission."now"becomesFailedSmartand shows as failed in the UI.Both results are correct, but users can see new warnings and notifications after they upgrade.
Known limit with repeat notifications off:
UpdateDeviceStatusORs new bits into the stored device status. SoFailedSmartstays set until a submission passes fully. Example: the overall health check fails, then passes while a Scrutiny failure is still active, then fails again. The second failure does not send a notification, because the stored status before that submission still hasFailedSmart. This case is rare. Fixing it means storing the status per submission, which is out of scope here.Behavior change: with Filter=Critical, a failed overall health check now notifies even when no critical attribute is flagged. This is intended. The overall health result is the drive's own verdict, not an attribute.
Follow-up for the next master-into-beta sync:
constants.go, wherebetahas the same fix in a different form.beta, fix(smart): detect manufacturer SMART failures for ATA and NVMe and keep them through recalculation #970 also setsFailedSmarton the device from an ATA attribute that is not ignored (setFailedSmartFromAttribute). With this change, a non-critical attribute that fails its manufacturer threshold will therefore also notify under Filter=Critical. Decide during the sync whether to keep that.