Skip to content

fix(collector): pass explicitly configured smartctl device types verbatim - #678

Merged
Starosdev merged 2 commits into
developfrom
fix/664-explicit-device-types
Jul 28, 2026
Merged

Starosdev merged 2 commits into
developfrom
fix/664-explicit-device-types

Conversation

@Starosdev

Copy link
Copy Markdown
Owner

Summary

Some Hitachi and Toshiba drives answer an auto-detected smartctl query with no identity, SMART support is: Unavailable and no attributes. Scrutiny renders that as a failed disk with zero metrics. The documented remedy is to pin the device type in collector.yaml; for most of these drives that type is sat, which already reached smartctl. A configured type of scsi or ata did not.

All three smartctl call sites (detection --info, collection --xall, FARM log) carried a hand-copied guard:

if len(deviceType) > 0 && deviceType != "scsi" && deviceType != "ata" {
    args = append(args, "--device", deviceType)
}

The guard is correct for a type that came from smartctl --scan: container scans routinely mislabel ATA disks as SCSI, and forcing -d scsi on one loses its ATA attributes. It is wrong for a type the user wrote down, but models.Device carries no provenance, so the two were indistinguishable. The comparison was also exact-case, so type: 'ATA' was passed through while type: 'ata' was dropped.

Changes

  • New detect.AppendDeviceTypeArgs is now the only place a device type becomes smartctl arguments. All three call sites route through it, so they cannot drift apart.
  • A type configured for that device path in collector.yaml is passed verbatim, including scsi and ata. A type that is not configured keeps the existing suppression, now trimmed and case-insensitive.
  • Provenance is resolved per call from Config.GetDeviceOverrides() (already on the interface, memoized). It cannot be a field on models.Device: the device type round-trips through /api/devices/register before Collect reads it back, so a collector-only field would not survive.
  • nvme is unchanged and still passed through.

Behavior change on upgrade

A type: 'scsi' or type: 'ata' entry that was a silent no-op in earlier releases now takes effect. For a drive that was working through auto-detection, that can lose attributes. The troubleshooting doc calls this out and tells users to delete the entry if they want auto-detection. Worth repeating in the release notes.

Tests

go test ./collector/... passes. New coverage:

  • detect_test.go: configured sat reaches the --info argv and populates model, serial, WWN and smart_support.available; the auto-detected response for the same drive yields the empty identity that produces the false failure; configured scsi is now passed; a configured device that inherits a scanned scsi still suppresses it; an uppercase scanned SCSI is suppressed.
  • TestDetect_AppendDeviceTypeArgs: table over the resolution rules, including a type configured for a different device path.
  • metrics_collect_test.go: configured sat reaches both the --xall and FARM calls; configured scsi is passed; a scanned scsi with nothing configured produces neither --device.
  • Two new fixtures under collector/pkg/detect/testdata/.

Docs

  • docs/TROUBLESHOOTING_DEVICE_COLLECTOR.md: new section on drives reported as failed with zero SMART values, with the smartctl -a vs smartctl -a -d sat diagnosis and the upgrade note.
  • example.collector.yaml: states that a configured type is passed verbatim.

Fixes #664

…atim

Some Hitachi and Toshiba drives answer an auto-detected smartctl query with no
identity, "SMART support is: Unavailable" and no attributes, which Scrutiny
renders as a failed disk with zero metrics. The documented remedy is to pin the
device type in collector.yaml, and for most of these drives that type is `sat`,
which already reached smartctl. A type of `scsi` or `ata` did not.

All three smartctl call sites (detection --info, collection --xall, and the FARM
log) carried a hand-copied guard that dropped `scsi` and `ata` before appending
--device. The guard is right for a type that came from `smartctl --scan`, since
container scans routinely mislabel ATA disks as SCSI and forcing -d scsi loses
the ATA attributes. It is wrong for a type the user wrote down, but
models.Device carries no provenance, so the two were indistinguishable. The
comparison was also exact-case, so `type: 'ATA'` was passed while `type: 'ata'`
was dropped.

Replace all three copies with detect.AppendDeviceTypeArgs, which resolves
provenance from Config.GetDeviceOverrides() at the call site. A type configured
for that device path is passed verbatim, including `scsi` and `ata`; a type that
is not configured keeps the existing suppression, now case-insensitive. The
lookup cannot use a field on models.Device because the device type round-trips
through /api/devices/register before Collect reads it back.

Behavior change on upgrade: a `type: 'scsi'` or `type: 'ata'` entry that was a
no-op in earlier releases now takes effect. The troubleshooting doc says so and
tells users to remove the entry if they want auto-detection.

Fixes #664
The auto-detected fixture has no wwn block, so SmartCtlInfo falls through to the
platform-specific wwnFallback. On Linux that reads the real disk at /dev/sda, so
the WWN is empty only on hosts without one. Assert the fixture-derived fields
instead.
@Starosdev
Starosdev merged commit b2d03c7 into develop Jul 28, 2026
14 checks passed
@Starosdev
Starosdev deleted the fix/664-explicit-device-types branch July 28, 2026 03:55
@Starosdev

Copy link
Copy Markdown
Owner Author

Merged (2026-07-28)

Changes Made

  1. Single builder for the smartctl --device argument

  2. A configured device type is now passed verbatim

    • A type written in the devices section of collector.yaml reaches smartctl exactly as written, including scsi and ata
    • Previously those two were dropped, so a Hitachi or Toshiba drive pinned to type: 'scsi' silently fell back to auto-detection: the false-failure, zero-metric case in the issue
  3. Scan-derived types keep the old behavior

    • A type that only came from smartctl --scan is still suppressed when it is scsi or ata, because container scans routinely mislabel ATA disks as SCSI and forcing -d scsi loses the ATA attributes
    • The comparison is now trimmed and case-insensitive, so type: 'ATA' no longer behaves differently from type: 'ata'
    • Provenance is resolved per call from Config.GetDeviceOverrides(), not from a field on models.Device: the device type round-trips through /api/devices/register before Collect reads it back, so a struct field would not survive
  4. Docs

    • New troubleshooting section for drives reported as failed with zero SMART values, with the smartctl -a vs smartctl -a -d sat diagnosis
    • example.collector.yaml states the passthrough rule

Upgrade note

A type: 'scsi' or type: 'ata' entry that was a silent no-op in earlier releases now takes effect. For a drive that was working through auto-detection, that can lose attributes. Remove the entry to restore auto-detection. This belongs in the release notes.

PR #678
Issue #664
Branch develop
Docker ghcr.io/starosdev/scrutiny:develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant