Keep CORE inventory when disk rotation metadata is unknown

Treat optional REST rotation rates as the RPC reader does, without weakening required inventory decoding or fabricating disk health. Cover complete CORE snapshots, mixed alert arguments, topology and saved-config poll failure/recovery/disable.

Refs: #2466
Change-source: pulse-maintainer

docs/release-control/v6/internal/subsystems/monitoring.md

@@ -4411,6 +4411,25 @@ and the live-generation guard.
 
 ### Native pool-health collection and appliance isolation
 
+CORE's legacy REST disk inventory treats `rotationrate` as optional provider
+metadata, just like the RPC reader: integers and numeric strings retain their
+classification, while null, absent, unknown and unrecognised values provide no
+rotation evidence. The existing explicit HDD/SSD/NVMe type fallback remains;
+an unknown QEMU/VTSCSI disk gains no SMART health, temperature, serial or SSD
+type assertion. Required disk-field decoding, complete JSON inventory shape
+and HTTP errors still fail collection. Both inventory and disk-temperature
+identifier discovery use the tolerant shape.
+
+`TestRESTDiskRotationRateSnapshot` and
+`TestRESTDiskRotationRatePreservesErrorsAndTopology` in
+`internal/truenas/client_api_shapes_test.go` bind complete snapshots, mixed
+alert arguments, RPC parity, healthy neighbours and actual topology evidence.
+`TestTrueNASPollerCOREUnknownRotationRate` in
+`internal/monitoring/truenas_poller_test.go` binds saved-config ordinary polling,
+canonical unknowns, failure without renewed evidence, recovery and disable.
+These synthetic controls reproduce and repair #2466's disk failure; they do
+not establish native appliance acceptance or a containing release.
+
 TrueNAS monitoring preserves the complete native `pool.query` observation
 needed by the shared storage-health contract: pool GUID and status detail,
 structured scrub or resilver state, pool and vdev read/write/checksum counters,

internal/monitoring/truenas_poller_test.go

@@ -2947,3 +2947,105 @@ func TestTrueNASPollerStorageDiscoveryDoesNotReuseUnreadableConnectionConfig(t *
 		t.Fatal("unreadable authoritative config reused old credentials or records")
 	}
 }
+
+func TestTrueNASPollerCOREUnknownRotationRate(t *testing.T) {
+	previous := truenas.IsFeatureEnabled()
+	truenas.SetFeatureEnabled(true)
+	t.Cleanup(func() { truenas.SetFeatureEnabled(previous) })
+	var diskFailure atomic.Bool
+	var diskRequests atomic.Int64
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.URL.Path {
+		case "/api/v2.0/system/info":
+			_, _ = w.Write([]byte(`{"hostname":"core-poll","version":"TrueNAS-13.0-U6.8"}`))
+		case "/api/v2.0/pool":
+			_, _ = w.Write([]byte(`[{"id":1,"name":"tank","status":"ONLINE","size":1000,"allocated":400,"free":600}]`))
+		case "/api/v2.0/pool/dataset":
+			_, _ = w.Write([]byte(`[{"id":"tank/data","name":"tank/data","pool":"tank","used":{"parsed":123},"available":{"parsed":456},"mountpoint":"/mnt/tank/data"}]`))
+		case "/api/v2.0/disk":
+			diskRequests.Add(1)
+			if diskFailure.Load() {
+				_, _ = w.Write([]byte(`[{"rotationrate":`))
+			} else {
+				_, _ = w.Write([]byte(`[{"identifier":"{uuid}synthetic-da0","name":"da0","serial":"","size":34359738368,"model":"QEMU QEMU HARDDISK","rotationrate":"unknown","type":"UNKNOWN","bus":"VTSCSI"},{"identifier":"{uuid}synthetic-da1","name":"da1","serial":"","size":34359738368,"model":"QEMU QEMU HARDDISK","rotationrate":null,"type":"UNKNOWN","bus":"VTSCSI"}]`))
+			}
+		case "/api/v2.0/alert/list":
+			_, _ = w.Write([]byte(`[{"id":"string","level":"WARNING","formatted":"Pool warning","args":"boot-pool","datetime":{"$date":1707400000000}},{"id":"object","level":"INFO","formatted":"Notice","args":{},"datetime":{"$date":1707400000000}}]`))
+		case "/api/v2.0/vm":
+			_, _ = w.Write([]byte(`[{"id":42,"name":"core-vm","memory":1024,"vcpus":2,"status":{"state":"RUNNING"}}]`))
+		case "/api/v2.0/zfs/snapshot":
+			_, _ = w.Write([]byte(`[{"id":"tank/data@daily","dataset":"tank/data","snapshot_name":"daily","created_at":1707400000}]`))
+		case "/api/v2.0/replication", "/api/v2.0/service", "/api/v2.0/app", "/api/v2.0/sharing/smb", "/api/v2.0/sharing/nfs":
+			_, _ = w.Write([]byte(`[]`))
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	t.Cleanup(server.Close)
+	mtp, persistence := newTestTenantPersistence(t)
+	connection := trueNASInstanceForServer(t, "core-rotation", server.URL, true)
+	if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{connection}); err != nil {
+		t.Fatal(err)
+	}
+	poller := NewTrueNASPoller(mtp, 0, nil)
+	t.Cleanup(func() {
+		for _, byConnection := range poller.providersByOrg {
+			for _, provider := range byConnection {
+				provider.Close()
+			}
+		}
+	})
+	poller.syncConnections()
+	// Explicit cycles avoid wall-clock sleeps while exercising the ordinary
+	// saved-config provider, client, projection and runtime summary boundaries.
+	for cycle := 0; cycle < 4; cycle++ {
+		diskFailure.Store(cycle == 2)
+		if cycle > 0 {
+			poller.mu.Lock()
+			poller.statusByOrg["default"][connection.ID].nextPollAt = time.Time{}
+			poller.mu.Unlock()
+		}
+		prior := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID]
+		poller.pollAll(context.Background())
+		summary := poller.ConnectionSummaries("default", []config.TrueNASInstance{connection})[connection.ID]
+		if summary.Poll == nil || summary.Poll.LastAttemptAt == nil {
+			t.Fatalf("no ordinary poll attempt in cycle %d: %+v", cycle, summary)
+		}
+		if cycle == 2 {
+			if summary.Poll.LastError == nil || summary.Poll.ConsecutiveFailures != 1 || summary.Poll.LastSuccessAt == nil || !summary.Poll.LastSuccessAt.Equal(*prior.Poll.LastSuccessAt) || summary.Observed == nil || !summary.Observed.CollectedAt.Equal(*prior.Observed.CollectedAt) {
+				t.Fatalf("malformed transport must fail without renewing last-known evidence: %+v", summary)
+			}
+		} else if summary.Poll.LastError != nil || summary.Poll.ConsecutiveFailures != 0 || summary.Poll.LastSuccessAt == nil || summary.Observed == nil || summary.Observed.Systems != 1 || summary.Observed.StoragePools != 1 || summary.Observed.Datasets != 1 || summary.Observed.Disks != 2 || summary.Observed.VMs != 1 || summary.Observed.RecoveryArtifacts != 1 || summary.Transport == nil || !summary.Transport.Connected {
+			t.Fatalf("CORE unknown rate prevented complete ordinary poll, cycle %d: %+v", cycle, summary)
+		}
+		if !hasTrueNASHostForOrg(poller, "default", "core-poll") {
+			t.Fatal("ordinary CORE poll lost the appliance")
+		}
+		disks := 0
+		for _, record := range poller.GetCurrentRecordsForOrg("default") {
+			if meta := record.Resource.PhysicalDisk; meta != nil {
+				disks++
+				if meta.Health != "UNKNOWN" || meta.DiskType != "vtscsi" || meta.RPM != 0 || meta.Temperature != 0 || meta.Serial != "" {
+					t.Fatalf("poll projection invented virtual-disk observations: %+v", meta)
+				}
+			}
+		}
+		if disks != 2 {
+			t.Fatalf("ordinary poll contributed %d disks, want both virtual disks", disks)
+		}
+		if s := poller.providersByOrg["default"][connection.ID].Snapshot(); s == nil || len(s.Alerts) != 2 || len(s.ZFSSnapshots) != 1 || len(s.VMs) != 1 {
+			t.Fatalf("mixed alert args or recovery inventory still aborted snapshot: %+v", s)
+		}
+	}
+	before := diskRequests.Load()
+	connection.Enabled = false
+	if err := persistence.SaveTrueNASConfig([]config.TrueNASInstance{connection}); err != nil {
+		t.Fatal(err)
+	}
+	poller.syncConnections()
+	poller.pollAll(context.Background())
+	if diskRequests.Load() != before || len(poller.GetCurrentRecordsForOrg("default")) != 0 {
+		t.Fatal("disabled CORE connection kept polling or retained current inventory")
+	}
+}

internal/truenas/client.go

@@ -1147,7 +1147,13 @@ func (c *Client) getDisksREST(ctx context.Context) ([]Disk, error) {
 
 	disks := make([]Disk, 0, len(response))
 	for _, item := range response {
-		rotationRate := item.RotationRate
+		// CORE reports "unknown" (and sometimes numeric strings) for this
+		// optional classifier. Match the RPC reader without weakening the
+		// required disk fields or treating missing SMART data as healthy.
+		rotationRate, _ := parseInt64Any(item.RotationRate)
+		if rotationRate < math.MinInt32 || rotationRate > math.MaxInt32 {
+			rotationRate = 0
+		}
 		rotational := rotationRate > 0
 		if rotationRate == 0 {
 			switch strings.ToLower(strings.TrimSpace(item.Type)) {
@@ -4862,7 +4868,7 @@ type diskResponse struct {
 	Bus          string          `json:"bus"`
 	Status       string          `json:"status"`
 	SmartStatus  json.RawMessage `json:"smart_status"`
-	RotationRate int             `json:"rotationrate"`
+	RotationRate any             `json:"rotationrate"`
 }
 
 type alertResponse struct {

internal/truenas/client_api_shapes_test.go

@@ -2,8 +2,13 @@ package truenas
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"net/http"
+	"reflect"
+	"strings"
 	"testing"
+	"time"
 )
 
 // The real pool.dataset.query response has never carried a "mounted" field on
@@ -411,3 +416,135 @@ func TestParsePoolStatePreservesScanTopologyErrorsAndNativeMissingEvidence(t *te
 		t.Fatalf("synthetic unavailable disk = %+v", disks[1])
 	}
 }
+
+// The CORE 13 virtual-disk shape from #2466, with synthetic identity. Exercise
+// the complete real client path, not just the optional field's decoder.
+func TestRESTDiskRotationRateSnapshot(t *testing.T) {
+	for _, tc := range []struct {
+		name, field, diskType string
+		rotational            bool
+	}{
+		{"unknown", `,"rotationrate":"unknown"`, "UNKNOWN", false},
+		{"ssd-sentinel", `,"rotationrate":"ssd"`, "UNKNOWN", false},
+		{"integer", `,"rotationrate":7200`, "UNKNOWN", true},
+		{"numeric-string", `,"rotationrate":" 7200 "`, "UNKNOWN", true},
+		{"zero", `,"rotationrate":0`, "UNKNOWN", false},
+		{"null", `,"rotationrate":null`, "UNKNOWN", false},
+		{"absent", "", "UNKNOWN", false},
+		{"hdd-fallback", `,"rotationrate":"unknown"`, "HDD", true},
+		{"ssd-fallback", `,"rotationrate":null`, "SSD", false},
+		{"out-of-range", `,"rotationrate":4294967296`, "UNKNOWN", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			routes := alertArgsTransport(defaultAPIResponses())
+			routes["/api/v2.0/system/info"] = apiResponse{body: `{"hostname":"core-nas","version":"TrueNAS-13.0-U6.8"}`}
+			// A bad optional classifier on one disk must not remove its healthy
+			// neighbours, their actual SMART evidence or their temperatures.
+			body := fmt.Sprintf(`[{"identifier":"{uuid}synthetic-da0","name":"da0","serial":"","size":34359738368,"model":"QEMU QEMU HARDDISK","transfermode":"Auto","hddstandby":"ALWAYS ON","togglesmart":true,"type":%q,"bus":"VTSCSI"%s},{"identifier":"{disk-1}","name":"sda","serial":"SER-A","size":1000000,"model":"Seagate","type":"HDD","pool":"tank","bus":"SATA","rotationrate":7200,"status":"ONLINE","smart_status":"FAILED"},{"identifier":"{disk-2}","name":"nvme0n1","serial":"SER-B","size":2000000,"model":"Samsung","type":"SSD","pool":"tank","bus":"NVMe","rotationrate":0,"status":"ONLINE"}]`, tc.diskType, tc.field)
+			routes["/api/v2.0/disk"] = apiResponse{body: body}
+			routes["/api/v2.0/alert/list"] = apiResponse{body: `[{"id":"smart","level":"CRITICAL","formatted":"Disk errors","klass":"SMARTUncorrectedErrorsAlert","args":{"ue":53,"name":"/dev/sda","serial":"SER-A"},"datetime":{"$date":1707400000000}},{"id":"string","level":"WARNING","formatted":"Pool warning","args":"boot-pool","datetime":{"$date":1707400000000}},{"id":"null","level":"INFO","formatted":"Notice","args":null,"datetime":{"$date":1707400000000}}]`}
+			routes["/api/v2.0/zfs/snapshot"] = apiResponse{body: `[{"id":"tank/apps@daily","dataset":"tank/apps","snapshot_name":"daily","created_at":1707400000}]`}
+			routes["/api/v2.0/replication"] = apiResponse{body: `[{"id":1,"name":"daily-copy","source_datasets":["tank/apps"],"target_dataset":"remote/apps","direction":"PUSH","transport":"SSH","state":"FINISHED"}]`}
+			client, err := NewClient(ClientConfig{Host: "http://truenas.invalid", APIKey: "synthetic"})
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer client.Close()
+			client.httpClient.Transport = routes
+			client.mode = TransportLegacyREST
+			ctx := context.Background()
+			if err := client.TestConnection(ctx); err != nil {
+				t.Fatal(err)
+			}
+			provider := NewLiveProviderForConnection(&APIFetcher{Client: client}, "core-connection")
+			for cycle := 0; cycle < 2; cycle++ {
+				if err := provider.Refresh(ctx); err != nil {
+					t.Fatalf("successful Test Connection must permit complete inventory, cycle %d: %v", cycle, err)
+				}
+				s := provider.Snapshot()
+				if s == nil || s.System.Hostname != "core-nas" || s.CollectedAt.IsZero() || len(s.Pools) != 1 || len(s.Datasets) != 1 || len(s.Disks) != 3 || len(s.Alerts) != 3 || len(s.Services) != 2 || len(s.Apps) != 1 || len(s.VMs) != 1 || len(s.Shares) != 2 || len(s.ZFSSnapshots) != 1 || len(s.ReplicationTasks) != 1 {
+					t.Fatalf("incomplete CORE snapshot: %+v", s)
+				}
+				d := s.Disks[0]
+				if d.ID != "{uuid}synthetic-da0" || d.Name != "da0" || d.SizeBytes != 34359738368 || d.Model != "QEMU QEMU HARDDISK" || d.Transport != "vtscsi" || d.Rotational != tc.rotational || d.Serial != "" || d.Pool != "" || d.Status != "" || d.HealthStatusPresent || d.Health != "" || d.Temperature != 0 {
+					t.Fatalf("lost identity or fabricated virtual-disk observations: %+v", d)
+				}
+				if h := s.Disks[1]; h.Serial != "SER-A" || h.Pool != "tank" || h.Transport != "sata" || !h.Rotational || !h.HealthStatusPresent || h.Health != "FAILED" || h.Temperature != 34 {
+					t.Fatalf("lost independent HDD evidence: %+v", h)
+				}
+				if h := s.Disks[2]; h.Serial != "SER-B" || h.Transport != "nvme" || h.Rotational || h.Temperature != 49 {
+					t.Fatalf("lost independent SSD evidence: %+v", h)
+				}
+				if a := s.Alerts[0]; !a.SMARTUncorrectedReported || a.SMARTUncorrectedErrors != 53 || a.DiskName != "/dev/sda" || a.DiskSerial != "SER-A" {
+					t.Fatalf("lost object alert evidence: %+v", a)
+				}
+				for _, a := range s.Alerts[1:] {
+					if a.SMARTUncorrectedReported || a.SMARTAvailableSpareReported || a.DiskName != "" || a.DiskSerial != "" || !a.Datetime.Equal(time.UnixMilli(1707400000000).UTC()) {
+						t.Fatalf("invented metadata from mixed alert args: %+v", a)
+					}
+				}
+				for _, record := range provider.RecordsFromSnapshot(s) {
+					if record.Resource.Name == "da0" {
+						meta := record.Resource.PhysicalDisk
+						if meta == nil || meta.Health != "UNKNOWN" || meta.DiskType != "vtscsi" || meta.Temperature != 0 || (!tc.rotational && meta.RPM != 0) {
+							t.Fatalf("unknown disk must remain unknown in canonical projection: %+v", meta)
+						}
+					}
+				}
+			}
+			var rpcShape []map[string]any
+			if err := json.Unmarshal([]byte(body), &rpcShape); err != nil {
+				t.Fatal(err)
+			}
+			rest, err := client.GetDisks(ctx)
+			if err != nil {
+				t.Fatal(err)
+			}
+			rpc, err := client.disksFromMaps(ctx, rpcShape)
+			if err != nil || !reflect.DeepEqual(rest, rpc) {
+				t.Fatalf("REST/RPC disk evidence differs: REST=%+v RPC=%+v err=%v", rest, rpc, err)
+			}
+			ids, err := client.listDiskReportingIdentifiers(ctx)
+			if err != nil || !reflect.DeepEqual(ids, []string{"da0", "sda", "nvme0n1"}) {
+				t.Fatalf("temperature discovery must tolerate the same field: %v, %v", ids, err)
+			}
+		})
+	}
+}
+
+func TestRESTDiskRotationRatePreservesErrorsAndTopology(t *testing.T) {
+	routes := alertArgsTransport(defaultAPIResponses())
+	routes["/api/v2.0/pool"] = apiResponse{body: `[{"id":1,"name":"tank","status":"DEGRADED","topology":{"data":[{"type":"DISK","disk":"da0","status":"FAULTED","path":"/dev/da0p2","stats":{"read_errors":2,"write_errors":3,"checksum_errors":4}}]}}]`}
+	routes["/api/v2.0/disk"] = apiResponse{body: `[{"identifier":"{uuid}synthetic","name":"da0","model":"QEMU QEMU HARDDISK","size":34359738368,"type":"UNKNOWN","bus":"VTSCSI","rotationrate":"unknown"}]`}
+	client, err := NewClient(ClientConfig{Host: "http://truenas.invalid", APIKey: "synthetic"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+	client.httpClient.Transport = routes
+	client.mode = TransportLegacyREST
+	s, err := client.FetchSnapshot(context.Background())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(s.Disks) != 1 || s.Disks[0].Pool != "tank" || s.Disks[0].Status != "FAULTED" || healthFromDisk(s.Disks[0]) != "FAILED" || len(s.Pools[0].DiskMembers) != 1 || s.Pools[0].DiskMembers[0].ReadErrors != 2 {
+		t.Fatalf("tolerant rotation rate lost real topology/health: %+v", s)
+	}
+	for _, tc := range []struct {
+		name     string
+		response apiResponse
+	}{
+		{"malformed-json", apiResponse{body: `[{"rotationrate":`}},
+		{"wrong-inventory-shape", apiResponse{body: `{"rotationrate":"unknown"}`}},
+		{"invalid-required-field", apiResponse{body: `[{"name":"da0","rotationrate":"unknown","size":"not-a-size"}]`}},
+		{"authentication", apiResponse{status: 401, body: `{}`}},
+		{"server", apiResponse{status: 503, body: `{}`}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			routes["/api/v2.0/disk"] = tc.response
+			if result, err := client.FetchSnapshot(context.Background()); err == nil || result != nil || !strings.Contains(err.Error(), "fetch truenas disks:") {
+				t.Fatalf("disk response failure must remain a failed snapshot: %+v, %v", result, err)
+			}
+		})
+	}
+}