diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 13b97e1cec..743401a8ff 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/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, diff --git a/internal/monitoring/truenas_poller_test.go b/internal/monitoring/truenas_poller_test.go index 505ddcc393..e22cbc3d44 100644 --- a/internal/monitoring/truenas_poller_test.go +++ b/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") + } +} diff --git a/internal/truenas/client.go b/internal/truenas/client.go index b057e40d18..75069af77c 100644 --- a/internal/truenas/client.go +++ b/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 { diff --git a/internal/truenas/client_api_shapes_test.go b/internal/truenas/client_api_shapes_test.go index 659abe6d10..af98ff81cb 100644 --- a/internal/truenas/client_api_shapes_test.go +++ b/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) + } + }) + } +}