diff --git a/controller/lib/smart.py b/controller/lib/smart.py index de69df9b..e7d5a440 100644 --- a/controller/lib/smart.py +++ b/controller/lib/smart.py @@ -50,21 +50,25 @@ def get_all_smart_status() -> list[dict]: for device in DeviceList().devices: results.append(_get_device_smart(device)) covered.add(f"/dev/{device.name}") + except Exception: + return [] - # Drives behind USB enclosures that reject ATA pass-through (e.g. - # Seagate Expansion, WD easystore) are invisible to pySMART/DeviceList. - # smartctl --scan still lists them; add a coarse SCSI health entry so - # the drive is at least represented instead of silently unmonitored. + # Drives behind USB enclosures that reject ATA pass-through (e.g. Seagate + # Expansion, WD easystore) are invisible to pySMART/DeviceList. smartctl + # --scan still lists them; add a coarse SCSI health entry so the drive is + # at least represented instead of silently unmonitored. Isolated from the + # pySMART read above: a failure here must not discard those results. + try: for path, _interface in _scan_devices(): if path in covered: continue limited = _limited_device_smart(path) if limited is not None: results.append(limited) - - return results except Exception: - return [] + pass + + return results def _get_device_smart(device) -> dict: @@ -229,10 +233,15 @@ def _scsi_health(path: str) -> Optional[str]: for line in result.stdout.splitlines(): key, sep, val = line.partition(":") if sep and key.strip().lower() == "smart health status": - val = val.strip() - if not val: - return None - return "PASS" if val.upper() == "OK" else "FAIL" + val = val.strip().upper() + # Standard SCSI health is "OK" or "FAILED!". Some bridges emit + # non-standard strings (e.g. "UNKNOWN", "N/A"); treat anything we + # do not recognise as unreadable (None) rather than a false FAIL. + if val == "OK": + return "PASS" + if val.startswith("FAIL"): + return "FAIL" + return None return None diff --git a/controller/test/test_smart.py b/controller/test/test_smart.py index 4f25cfa2..a4b97b4b 100644 --- a/controller/test/test_smart.py +++ b/controller/test/test_smart.py @@ -133,6 +133,29 @@ class TestGetAllSmartStatus: devices = {d["device"] for d in result} assert devices == {"sdc", "sda"} + def test_supplement_failure_preserves_pysmart_results(self): + """An error in the SCSI supplement must not discard the drives already + collected from pySMART.""" + ata = mock.Mock() + ata.name = "sdb" + ata.model = "Seagate" + ata.serial = "S1" + ata.capacity = "8.0 TB" + ata.assessment = "PASS" + ata.smart_enabled = True + ata.attributes = [] + + device_list = mock.Mock() + device_list.devices = [ata] + + with mock.patch("controller.lib.smart.is_smart_available", return_value=True), \ + mock.patch("controller.lib.smart.DeviceList", return_value=device_list), \ + mock.patch("controller.lib.smart._scan_devices", + side_effect=RuntimeError("scan blew up")): + result = get_all_smart_status() + + assert [d["device"] for d in result] == ["sdb"] + class TestGetDeviceSmart: """Tests for _get_device_smart function.""" @@ -466,6 +489,17 @@ class TestReadScsiLimited: # Only the health read happened; identity was skipped. run_fn.assert_called_once() + def test_unrecognised_status_is_treated_as_unreadable(self): + """A non-standard bridge status (e.g. UNKNOWN) must NOT show as a red + FAIL; treat it as unreadable so the drive is omitted, not falsely + failed.""" + run = mock.Mock(stdout="SMART Health Status: UNKNOWN\n") + with mock.patch("controller.lib.smart.subprocess.run", + return_value=run) as run_fn: + result = _read_scsi_limited("/dev/sda") + assert result == (None, None, None, None) + run_fn.assert_called_once() + def test_smartctl_error_returns_all_none(self): with mock.patch("controller.lib.smart.subprocess.run", side_effect=OSError("boom")):