Fix SAT SMART temperature collection

Refs #1471
This commit is contained in:
rcourtman 2026-07-06 23:42:00 +01:00
parent 41a99c1a03
commit 4b669ad9b5
13 changed files with 233 additions and 27 deletions

View file

@ -919,6 +919,9 @@ surface and no new `internal/api/` lifecycle handler.
`internal/hostagent/smartctl.go`. Linux SMART discovery must prefer
`smartctl --scan-open` typed targets before generic block-device fallback so
controller-backed disks keep their canonical SMART and wearout coverage.
Direct Linux SATA/SAT-style block devices that return health but no
temperature through smartctl auto-detection must retry explicit `-d sat` and
`-d scsi` probes before settling on a no-temperature result.
FreeBSD SMART probing must retry through the canonical typed and untyped
device modes and the SCT temperature status path before settling on standby
or no-data results, and partial or plain-text smartctl output must still
@ -3459,7 +3462,9 @@ shape for temperature-monitoring SSH keys: new keys must execute the
Pulse-owned `/usr/local/sbin/pulse-sensors` wrapper, not raw `sensors -j`, so
lifecycle setup can collect CPU sensor data and SMART disk temperatures through
one bounded JSON payload while preserving old forced-key compatibility in the
runtime collector.
runtime collector. That wrapper must mirror the host-agent Linux SMART probe
fallback for direct SATA/SAT-style disks by retrying explicit `-d sat` and
`-d scsi` before reporting an active disk with no temperature.
That same generated setup-script boundary must also use exact token-name
matching when it decides whether to rotate an existing Pulse-managed token, so
reruns do not treat partial-name collisions as the canonical managed token.

View file

@ -3127,6 +3127,12 @@ replay timing, recent rate-limit counts, and flapping suppression evidence. The
route requires `monitoring:read`, returns `404` for unknown active alerts, and
must not send notifications or mutate delivery tracking state.
The generated PVE setup-script temperature wrapper is part of the API contract
for legacy SSH sensor collection. Direct Linux SATA/SAT-style disks that return
health but no temperature through smartctl auto-detection must retry explicit
`-d sat` and `-d scsi` probes before the rendered wrapper reports an active
disk with no temperature.
Manifest-backed Patrol finding lifecycle schemas are the API source of truth
for Assistant provider-tool optionality as well as MCP/API discovery. Legacy
Assistant runtime paths may project those schemas into provider-tool JSON, but
@ -5920,7 +5926,10 @@ raw `sensors -j`. The wrapper is the setup-script API contract for legacy SSH
temperature collection: it must emit a bounded JSON object with `sensors` and
`smart` members, install or verify `smartmontools` for SATA/SAS/HDD disk
temperatures, and keep `sensors -j` only as a compatibility fallback inside
the wrapper/runtime collector path.
the wrapper/runtime collector path. Direct Linux SATA/SAT-style disks that
return health but no temperature through smartctl auto-detection must retry
explicit `-d sat` and `-d scsi` probes before the wrapper reports an active
disk with no temperature.
That same generated-script payload must also preserve the canonical encoded
rerun URL contract: embedded `SETUP_SCRIPT_URL` values must carry the exact
selected `host`, `pulse_url`, and `backup_perms` query state instead of

View file

@ -633,7 +633,11 @@ TLS floor in the dynamic config.
`hotfix_exception=true`, a non-empty `hotfix_reason`, and
`rollback_version` set to the previous stable tag. Non-hotfix stable
promotions, and any first-GA or minor-line stable promotion, still require
explicit promoted prerelease lineage and soak proof.
explicit promoted prerelease lineage and soak proof. Stable patch release
packets must also enumerate every customer-visible support fix included in
the cut, and the release-asset proof must pin the current packet to those
runtime fixes so a patch that includes support work cannot ship as a
metadata-only release note.
7. Preserve release-matched installer and Helm operator documentation links through `scripts/install.sh`, `.github/workflows/helm-pages.yml`, `.github/workflows/publish-helm-chart.yml`, and the chart metadata itself so deployment guidance and packaged chart metadata do not drift back to branch-tip `main` docs when a release line or promoted tag already exists.
The same governed Helm boundary also owns `deploy/helm/pulse/` itself:
chart metadata, default values, templates, and generated chart docs must

View file

@ -1588,6 +1588,12 @@ Default-org token scoping and notification-settings fan-out on shared
api-contract/security owned and create no storage, recovery-point, or
backup-surface semantics.
The shared PVE setup-script SMART wrapper remains a storage/recovery dependency
only for disk-temperature evidence. Storage surfaces may depend on its explicit
`-d sat` and `-d scsi` retries for active direct Linux SATA/SAT-style disks, but
they must not fork a storage-local disk-temperature collector or replace the
API-owned setup-script contract.
Notification webhook management changes on shared `internal/api/` handlers are
likewise adjacent only: the webhook `signingSecret` payload field and its
masking semantics are notifications/API-contract owned and create no storage,
@ -3271,10 +3277,12 @@ script renderer, but they must not replace the symlink path with a local file
when filtering Pulse-managed `# pulse-` SSH key entries.
That same dependency also assumes the shared PVE setup script binds
temperature-monitoring SSH keys to `/usr/local/sbin/pulse-sensors` and emits
SMART disk temperatures in the wrapper payload. Storage and recovery disk
temperature surfaces may depend on that monitoring-owned SMART merge path, but
they must not reintroduce raw `sensors -j` as the setup contract or build a
storage-local disk-temperature collector.
SMART disk temperatures in the wrapper payload, including explicit `-d sat`
and `-d scsi` retries for direct Linux SATA/SAT-style disks whose smartctl
auto-detection returns no temperature. Storage and recovery disk temperature
surfaces may depend on that monitoring-owned SMART merge path, but they must
not reintroduce raw `sensors -j` as the setup contract or build a storage-local
disk-temperature collector.
Pressure-only host-agent telemetry remains outside that storage collector
contract: storage surfaces may read the shared host context, but may not add a
parallel macOS thermal collector or fold `thermalState` into disk SMART state.

View file

@ -2,7 +2,8 @@
`v6.0.5` is a stable patch release for the Pulse v6 line. It follows
`v6.0.4` and bundles support fixes for Patrol Gemini model readiness and
remembered-login submit persistence.
remembered-login submit persistence, plus a Proxmox SMART temperature fallback
for direct SATA/SAT disks.
## Fixes
@ -10,6 +11,9 @@ remembered-login submit persistence.
detected from Gemini candidate parts as well as top-level tool-call lists.
- Fixed the login form so enabling "remember me" during submit persists the
remembered username immediately.
- Fixed Proxmox SMART temperature collection for direct SATA/SAT disks where
smartctl auto-detection returned disk health but no temperature until retried
with an explicit SAT probe.
- Refreshed Docker, Helm, installer, and release-helper metadata for the stable
patch line.

View file

@ -9,6 +9,9 @@ _This changelog describes the stable `v6.0.5` patch release compared with
candidate parts as well as top-level tool-call lists.
- Remembered-login state now persists the saved username when the checkbox is
enabled during submit.
- Proxmox SMART temperature collection now retries direct SATA/SAT disks with an
explicit SAT probe when smartctl auto-detection returns health but no
temperature.
- Docker, Helm, installer, and release-helper metadata now track the active
stable patch version.

View file

@ -180,6 +180,10 @@ func TestPVESetupScriptRestrictsTemperatureMonitoringToPulseSensorWrapper(t *tes
`["lsblk", "-J", "-d", "-o", "NAME,TYPE,TRAN,MODEL,VENDOR,SUBSYSTEMS"]`,
`def union_smart_targets(scan_targets, block_devices):`,
`def smart_probe_attempts(device, device_type):`,
`def inferred_smart_device_types(device):`,
`for dtype in inferred_smart_device_types(device):`,
`for attempt_index, (attempt_device, attempt_type) in enumerate(attempts):`,
`attempt_index == len(attempts) - 1`,
`"smart": collect_smart(),`,
`SSH_SENSORS_KEY_ENTRY="command=\"$PULSE_SENSORS_WRAPPER\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty $SSH_SENSORS_PUBLIC_KEY # pulse-sensors"`,
}

View file

@ -6131,7 +6131,11 @@ func TestContract_SetupScriptEmbedsFailFastGuidance(t *testing.T) {
if !strings.Contains(script, `SSH_SENSORS_KEY_ENTRY="command=\"$PULSE_SENSORS_WRAPPER\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty $SSH_SENSORS_PUBLIC_KEY # pulse-sensors"`) {
t.Fatalf("setup script must force temperature SSH keys to the Pulse sensor wrapper: %s", script)
}
if !strings.Contains(script, `"smart": collect_smart(),`) || !strings.Contains(script, `apt-get install -y smartmontools`) {
if !strings.Contains(script, `"smart": collect_smart(),`) ||
!strings.Contains(script, `apt-get install -y smartmontools`) ||
!strings.Contains(script, `def inferred_smart_device_types(device):`) ||
!strings.Contains(script, `for dtype in inferred_smart_device_types(device):`) ||
!strings.Contains(script, `attempt_index == len(attempts) - 1`) {
t.Fatalf("setup script missing SMART temperature wrapper contract: %s", script)
}
if strings.Contains(script, `SSH_SENSORS_KEY_ENTRY="command=\"sensors -j\"`) {

View file

@ -909,6 +909,13 @@ def canonical_block_for_device(device):
return name
def inferred_smart_device_types(device):
name = os.path.basename(str(device or "").strip()).lower()
if re.fullmatch(r"(sd|hd)[a-z]+", name or ""):
return ["sat", "scsi"]
return []
def is_physical_block_device(device):
name = str(device.get("name") or "").strip()
dtype = str(device.get("type") or "").strip().lower()
@ -1024,9 +1031,26 @@ def smart_targets(path):
def smart_probe_attempts(device, device_type):
attempts = [(device, device_type)] if device_type else [(device, "")]
attempts = []
seen = set()
def add(dtype):
key = (device, dtype)
if key in seen:
return
seen.add(key)
attempts.append(key)
if device_type:
add(device_type)
else:
add("")
if device_type and not is_multiplexed_device_type(device_type):
attempts.append((device, ""))
add("")
if not is_multiplexed_device_type(device_type):
for dtype in inferred_smart_device_types(device):
add(dtype)
return attempts
@ -1129,7 +1153,8 @@ def collect_smart():
observed_at = datetime.datetime.now(datetime.timezone.utc).replace(microsecond=0).isoformat().replace("+00:00", "Z")
for device, device_type in smart_targets(path):
best_entry = None
for attempt_device, attempt_type in smart_probe_attempts(device, device_type):
attempts = smart_probe_attempts(device, device_type)
for attempt_index, (attempt_device, attempt_type) in enumerate(attempts):
args = [path]
if attempt_type:
args.extend(["-d", attempt_type])
@ -1164,7 +1189,7 @@ def collect_smart():
"lastUpdated": observed_at,
}
best_entry = entry
if entry["temperature"] > 0 or entry["standbySkipped"] or not attempt_type:
if entry["temperature"] > 0 or entry["standbySkipped"] or attempt_index == len(attempts) - 1:
break
if best_entry:

View file

@ -12,6 +12,47 @@ import (
gohost "github.com/shirou/gopsutil/v4/host"
)
func TestAgentMetricsSMARTProbeAttemptsRetryExplicitSAT(t *testing.T) {
stubLinuxSysfs(t, []string{"sda"}, nil)
origRun := smartRunCommandOutput
origLook := execLookPath
t.Cleanup(func() {
smartRunCommandOutput = origRun
execLookPath = origLook
})
execLookPath = func(string) (string, error) { return "smartctl", nil }
var attempts [][]string
smartRunCommandOutput = func(ctx context.Context, name string, args ...string) ([]byte, error) {
attempts = append(attempts, append([]string(nil), args...))
if len(args) >= 2 && args[0] == "-d" && args[1] == "sat" {
return []byte(smartctlSATTemperatureAttributeJSON), nil
}
if len(args) > 0 && args[0] == "-d" {
return []byte(smartctlNoDataJSON), nil
}
return []byte(smartctlUntypedHealthOnlyJSON), nil
}
result, err := collectSMARTTarget(context.Background(), smartctlTarget{Path: "/dev/sda"})
if err != nil {
t.Fatalf("collectSMARTTarget error: %v", err)
}
if len(attempts) != 2 {
t.Fatalf("expected untyped attempt then explicit SAT retry, got %v", attempts)
}
if attempts[0][0] == "-d" {
t.Fatalf("first attempt should use smartctl auto-detection, got %v", attempts[0])
}
if len(attempts[1]) < 2 || attempts[1][0] != "-d" || attempts[1][1] != "sat" {
t.Fatalf("second attempt should force -d sat, got %v", attempts[1])
}
if result == nil || result.Temperature != 32 || result.Type != "sata" {
t.Fatalf("expected SAT temperature result, got %#v", result)
}
}
func TestBuildReport(t *testing.T) {
// Setup mocks
fixedTime := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)

View file

@ -203,6 +203,7 @@ var (
smartTextTempAttributeRE = regexp.MustCompile(`^\s*(190|194)\s+\S+.*-\s+(\d{1,3})\b`)
smartTextCurrentTempRE = regexp.MustCompile(`(?i)^current(?: drive)? temperature:\s*(\d{1,3})\b`)
smartTextTemperatureRE = regexp.MustCompile(`(?i)^temperature:\s*(\d{1,3})\b`)
linuxDirectSATDeviceRE = regexp.MustCompile(`^(sd|hd)[a-z]+$`)
)
type smartctlTarget struct {
@ -1106,7 +1107,7 @@ func collectSMARTTarget(ctx context.Context, target smartctlTarget) (*DiskSMART,
if firstParsed == nil {
firstParsed = result
}
if !shouldRetryFreeBSDSMART(target.Path, result, i, len(attempts)) {
if !shouldRetrySMARTTarget(target.Path, result, i, len(attempts)) {
log.Debug().
Str("component", smartctlComponent).
Str("action", "collect_device_smart_success").
@ -1149,9 +1150,7 @@ func collectSMARTTarget(ctx context.Context, target smartctlTarget) (*DiskSMART,
func smartctlProbeAttempts(target smartctlTarget) [][]string {
device := target.Path
if target.DeviceType != "" {
attempts := [][]string{
smartctlArgs(device, target.DeviceType),
}
deviceTypes := []string{target.DeviceType}
// A scan-open device type is a hint, not ground truth: smartctl can
// suggest a type whose full query (-i -A -H) fails or returns no usable
// data even though untyped auto-detection works (#1483: a SATA SSD
@ -1159,19 +1158,21 @@ func smartctlProbeAttempts(target smartctlTarget) [][]string {
// giving up. Multiplexed controller members are exempt because dropping
// the -d would re-probe the shared array device, not the member.
if runtimeGOOS == "linux" && !isMultiplexedDeviceType(target.DeviceType) {
attempts = append(attempts, smartctlArgs(device, ""))
deviceTypes = append(deviceTypes, "")
deviceTypes = append(deviceTypes, linuxInferredSmartctlDeviceTypes(device)...)
}
return attempts
return smartctlArgsForDeviceTypes(device, deviceTypes)
}
if runtimeGOOS == "linux" {
deviceTypes := append([]string{""}, linuxInferredSmartctlDeviceTypes(device)...)
return smartctlArgsForDeviceTypes(device, deviceTypes)
}
if runtimeGOOS == "freebsd" {
deviceTypes := freeBSDSmartctlDeviceTypes(filepath.Base(device))
if len(deviceTypes) > 0 {
attempts := make([][]string, 0, len(deviceTypes)+1)
for _, deviceType := range deviceTypes {
attempts = append(attempts, smartctlArgs(device, deviceType))
}
return append(attempts, smartctlArgs(device, ""))
return smartctlArgsForDeviceTypes(device, append(deviceTypes, ""))
}
}
@ -1180,6 +1181,33 @@ func smartctlProbeAttempts(target smartctlTarget) [][]string {
}
}
func smartctlArgsForDeviceTypes(device string, deviceTypes []string) [][]string {
attempts := make([][]string, 0, len(deviceTypes))
seen := make(map[string]struct{}, len(deviceTypes))
for _, deviceType := range deviceTypes {
deviceType = strings.TrimSpace(deviceType)
if _, ok := seen[deviceType]; ok {
continue
}
seen[deviceType] = struct{}{}
attempts = append(attempts, smartctlArgs(device, deviceType))
}
return attempts
}
func linuxInferredSmartctlDeviceTypes(device string) []string {
if runtimeGOOS != "linux" {
return nil
}
name := strings.ToLower(filepath.Base(strings.TrimSpace(device)))
switch {
case linuxDirectSATDeviceRE.MatchString(name):
return []string{"sat", "scsi"}
default:
return nil
}
}
func smartctlArgs(device, deviceType string) []string {
args := []string{}
if deviceType != "" {
@ -1224,8 +1252,8 @@ func freeBSDSmartctlDeviceTypes(device string) []string {
}
}
func shouldRetryFreeBSDSMART(device string, result *DiskSMART, attemptIndex, attemptCount int) bool {
if runtimeGOOS != "freebsd" || attemptIndex >= attemptCount-1 || result == nil {
func shouldRetrySMARTTarget(device string, result *DiskSMART, attemptIndex, attemptCount int) bool {
if attemptIndex >= attemptCount-1 || result == nil {
return false
}
if result.Temperature > 0 {
@ -1234,7 +1262,14 @@ func shouldRetryFreeBSDSMART(device string, result *DiskSMART, attemptIndex, att
if result.Standby {
return true
}
return len(freeBSDSmartctlDeviceTypes(filepath.Base(device))) > 0
switch runtimeGOOS {
case "freebsd":
return len(freeBSDSmartctlDeviceTypes(filepath.Base(device))) > 0
case "linux":
return true
default:
return false
}
}
func enrichFreeBSDSCTTemperature(ctx context.Context, smartctlPath string, args []string, target smartctlTarget, current *DiskSMART) *DiskSMART {

View file

@ -59,6 +59,25 @@ const smartctlNoDataJSON = `{
"device": {"name": "/dev/sda", "type": "sat", "protocol": "ATA"}
}`
const smartctlUntypedHealthOnlyJSON = `{
"device": {"name": "/dev/sda", "type": "scsi", "protocol": "SCSI"},
"model_name": "WDC_WD80EFPX-68C4ZN0",
"serial_number": "WD-SAT-TEMP-1",
"smart_status": {"passed": true}
}`
const smartctlSATTemperatureAttributeJSON = `{
"device": {"name": "/dev/sda", "type": "sat", "protocol": "ATA"},
"model_name": "WDC_WD80EFPX-68C4ZN0",
"serial_number": "WD-SAT-TEMP-1",
"smart_status": {"passed": true},
"ata_smart_attributes": {
"table": [
{"id": 194, "name": "Temperature_Celsius", "raw": {"value": 0, "string": "32"}}
]
}
}`
func stubLinuxSysfs(t *testing.T, entries []string, files map[string]string) {
t.Helper()
@ -260,6 +279,47 @@ func TestCollectSMARTTargetRetriesUntypedAfterTypedProbeFailure(t *testing.T) {
}
}
func TestCollectSMARTTargetRetriesExplicitSATWhenUntypedHasNoTemperature(t *testing.T) {
stubLinuxSysfs(t, []string{"sda"}, nil)
origRun := smartRunCommandOutput
origLook := execLookPath
t.Cleanup(func() {
smartRunCommandOutput = origRun
execLookPath = origLook
})
execLookPath = func(string) (string, error) { return "smartctl", nil }
var attempts [][]string
smartRunCommandOutput = func(ctx context.Context, name string, args ...string) ([]byte, error) {
attempts = append(attempts, append([]string(nil), args...))
if len(args) >= 2 && args[0] == "-d" && args[1] == "sat" {
return []byte(smartctlSATTemperatureAttributeJSON), nil
}
if len(args) > 0 && args[0] == "-d" {
return []byte(smartctlNoDataJSON), nil
}
return []byte(smartctlUntypedHealthOnlyJSON), nil
}
result, err := collectSMARTTarget(context.Background(), smartctlTarget{Path: "/dev/sda"})
if err != nil {
t.Fatalf("collectSMARTTarget error: %v", err)
}
if len(attempts) != 2 {
t.Fatalf("expected untyped attempt then explicit SAT retry, got %v", attempts)
}
if attempts[0][0] == "-d" {
t.Fatalf("first attempt should use smartctl auto-detection, got %v", attempts[0])
}
if len(attempts[1]) < 2 || attempts[1][0] != "-d" || attempts[1][1] != "sat" {
t.Fatalf("second attempt should force -d sat, got %v", attempts[1])
}
if result == nil || result.Temperature != 32 || result.Type != "sata" {
t.Fatalf("expected SAT temperature result, got %#v", result)
}
}
func TestCollectSMARTTargetKeepsSingleTypedAttemptForMultiplexed(t *testing.T) {
stubLinuxSysfs(t, []string{"sda"}, nil)

View file

@ -263,6 +263,8 @@ func TestCurrentStablePatchReleasePacketTracksInstallMetadata(t *testing.T) {
"`v"+previous+"`",
"Patrol Gemini model readiness",
"remembered-login submit persistence",
"Proxmox SMART temperature fallback",
"direct SATA/SAT disks",
"rollback target for this patch release is `v"+previous+"`",
)
assertFileContainsAll(t, changelogPath,
@ -271,6 +273,8 @@ func TestCurrentStablePatchReleasePacketTracksInstallMetadata(t *testing.T) {
"Promotion path: stable patch hotfix from `"+releaseBranch+"`",
"Gemini tool-call capability",
"Remembered-login state now persists",
"Proxmox SMART temperature collection",
"explicit SAT probe",
)
assertFileContainsAll(t, repoFile("docs", "RELEASE_NOTES.md"),
"docs/releases/RELEASE_NOTES_v"+version+".md",