From e2a036ce2ed26356148fe51a6fa57942a1dfd489 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 11 Jun 2026 10:57:22 +0100 Subject: [PATCH] fix(unifiedresources): carry real poll timestamps for storage and docker container sightings resourceFromStorage and resourceFromDockerContainer stamped LastSeen with time.Now() at conversion because their source models carried no poll timestamp. The registry rebuilds from the retained state snapshot every cycle, so those resources re-reported a fresh sighting each rebuild even after their upstream source (PVE instance, docker host agent) stopped delivering, and their per-source SourceStatus could never go stale via markStaleLocked. 53faa4e46 fixed this fabrication at the ingest layer but left these two adapter-level stamps. - models.Storage gains LastSeen (omitzero), stamped where entries are built: the PVE storage poll (poll start time, including synthesized cluster-shared entries; preserved entries for unpolled nodes keep their old stamp), the PBS datastore conversion (PBS instance sighting), Ceph pool projection (cluster LastUpdated), and the mock generator (offline mock nodes get a backdated stamp so the stale path renders). - resourceFromStorage passes storage.LastSeen through; zero stays zero ("never seen") instead of becoming conversion time. - resourceFromDockerContainer uses host.LastSeen: containers are delivered wholesale with each host report, so the host report timestamp is the container sighting. This matches every other docker sub-resource adapter (services, tasks, volumes, networks, images already use host.LastSeen). - ingestStorage routes PBS-poller datastore entries (instance "pbs-", type pbs) to SourcePBS, parented to the PBS instance. Keying them SourceProxmox would judge their freshness against the 60s Proxmox stale threshold while PBS polls every 60s by default, flapping healthy datastores stale between polls; SourcePBS carries the cadence-matched 120s threshold. PVE-reported pbs-typed storage.cfg backends stay SourceProxmox. Side effect: syncUnifiedStorageMetrics no longer skips PBS datastore storage, so those entries gain usage history. - storageFromReadStateView round-trips LastSeen so the legacy storage API reports the honest sighting; mock refresh re-stamps available storage on each simulated poll. The parent host/node staleness was already honest, so platform pages reflected outages at the parent level; this makes the per-resource source freshness honest too. --- frontend-modern/src/types/api.ts | 2 + internal/mock/generator.go | 15 ++++ internal/models/models.go | 44 +++++----- internal/monitoring/monitor.go | 1 + internal/monitoring/monitor_pbs_pmg.go | 1 + .../monitoring/monitor_polling_storage.go | 2 + internal/unifiedresources/adapters.go | 4 +- internal/unifiedresources/adapters_test.go | 58 ++++++++++++++ internal/unifiedresources/registry.go | 23 ++++++ internal/unifiedresources/registry_test.go | 80 +++++++++++++++++++ 10 files changed, 207 insertions(+), 23 deletions(-) diff --git a/frontend-modern/src/types/api.ts b/frontend-modern/src/types/api.ts index d18b8578c..effc42d19 100644 --- a/frontend-modern/src/types/api.ts +++ b/frontend-modern/src/types/api.ts @@ -685,6 +685,8 @@ export interface Storage { shared: boolean; enabled: boolean; active: boolean; + // RFC3339 timestamp of the last poll that delivered this entry; absent = never seen + lastSeen?: string; // Added for deduplication in storage view nodes?: string[]; nodeIds?: string[]; diff --git a/internal/mock/generator.go b/internal/mock/generator.go index b3e00d5fb..35d588bd2 100644 --- a/internal/mock/generator.go +++ b/internal/mock/generator.go @@ -4391,6 +4391,12 @@ func generateStorage(nodes []models.Node) []models.Storage { for _, node := range nodes { isOffline := node.Status != "online" || node.ConnectionHealth == "offline" || node.Uptime <= 0 + // Storage on an offline node hasn't been freshly observed; backdate + // its sighting so the stale path renders instead of fake freshness. + nodeStorageSeen := now + if isOffline { + nodeStorageSeen = now.Add(-10 * time.Minute) + } // Local storage (always present) localTotal := int64(500 * 1024 * 1024 * 1024) // 500GB localID := fmt.Sprintf("%s-%s-local", node.Instance, node.Name) @@ -4427,6 +4433,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: false, Enabled: localEnabled, Active: localActive, + LastSeen: nodeStorageSeen, }) // Local-zfs (common) @@ -4492,6 +4499,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: false, Enabled: zfsEnabled, Active: zfsActive, + LastSeen: nodeStorageSeen, ZFSPool: zfsPool, }) @@ -4533,6 +4541,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: true, // PBS storage is shared cluster-wide (all nodes can access it) Enabled: true, Active: true, + LastSeen: now, }) } } @@ -4557,6 +4566,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: true, Enabled: true, Active: true, + LastSeen: now, }) } @@ -4590,6 +4600,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: true, Enabled: true, Active: true, + LastSeen: now, }) cephFsTotal := int64(24 * 1024 * 1024 * 1024 * 1024) // 24TB @@ -4610,6 +4621,7 @@ func generateStorage(nodes []models.Node) []models.Storage { Shared: true, Enabled: true, Active: true, + LastSeen: now, }) } @@ -5554,6 +5566,9 @@ func updateFixtureStateMetricsAt(data *models.StateSnapshot, config MockConfig, continue } ApplyStorageUsage(storage, SampleMetric("storage", storage.ID, "usage", refreshNow)) + // Refreshing usage simulates a fresh poll; stamp the sighting so + // available mock storage doesn't age past the stale thresholds. + storage.LastSeen = refreshNow } // Update disk metrics. diff --git a/internal/models/models.go b/internal/models/models.go index 723828d7e..d656e52b0 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -2046,27 +2046,28 @@ type DockerHostCommandStatus struct { // Storage represents a storage resource type Storage struct { - ID string `json:"id"` - Name string `json:"name"` - Node string `json:"node"` - Instance string `json:"instance"` - Nodes []string `json:"nodes,omitempty"` - NodeIDs []string `json:"nodeIds,omitempty"` - AliasIDs []string `json:"aliasIds,omitempty"` - NodeCount int `json:"nodeCount,omitempty"` - Type string `json:"type"` - Status string `json:"status"` - Pool string `json:"pool,omitempty"` - Path string `json:"path,omitempty"` - Total int64 `json:"total"` - Used int64 `json:"used"` - Free int64 `json:"free"` - Usage float64 `json:"usage"` - Content string `json:"content"` - Shared bool `json:"shared"` - Enabled bool `json:"enabled"` - Active bool `json:"active"` - ZFSPool *ZFSPool `json:"zfsPool,omitempty"` // ZFS pool details if this is ZFS storage + ID string `json:"id"` + Name string `json:"name"` + Node string `json:"node"` + Instance string `json:"instance"` + Nodes []string `json:"nodes,omitempty"` + NodeIDs []string `json:"nodeIds,omitempty"` + AliasIDs []string `json:"aliasIds,omitempty"` + NodeCount int `json:"nodeCount,omitempty"` + Type string `json:"type"` + Status string `json:"status"` + Pool string `json:"pool,omitempty"` + Path string `json:"path,omitempty"` + Total int64 `json:"total"` + Used int64 `json:"used"` + Free int64 `json:"free"` + Usage float64 `json:"usage"` + Content string `json:"content"` + Shared bool `json:"shared"` + Enabled bool `json:"enabled"` + Active bool `json:"active"` + LastSeen time.Time `json:"lastSeen,omitzero"` // when the owning poller last delivered this entry; zero = never seen + ZFSPool *ZFSPool `json:"zfsPool,omitempty"` // ZFS pool details if this is ZFS storage } func CephPoolStorageID(instanceName, poolName string) string { @@ -2146,6 +2147,7 @@ func StorageFromCephPool(cluster CephCluster, pool CephPool) Storage { Shared: true, Enabled: true, Active: status == "available", + LastSeen: cluster.LastUpdated, } } diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index f09d72aa8..73f73d38d 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -3014,6 +3014,7 @@ func storageFromReadStateView(view *unifiedresources.StoragePoolView) models.Sto Shared: view.Shared(), Enabled: view.Enabled(), Active: view.Active(), + LastSeen: view.LastSeen(), ZFSPool: storageZFSPoolFromReadStateView(view), } } diff --git a/internal/monitoring/monitor_pbs_pmg.go b/internal/monitoring/monitor_pbs_pmg.go index 403de894f..fb64768bd 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -471,6 +471,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie Shared: true, // PBS datastores are typically shared/network storage Enabled: true, Active: pbsInst.Status == "online", + LastSeen: pbsInst.LastSeen, } pbsStorages = append(pbsStorages, pbsStorage) } diff --git a/internal/monitoring/monitor_polling_storage.go b/internal/monitoring/monitor_polling_storage.go index 45a4edcf7..22b8bcd2d 100644 --- a/internal/monitoring/monitor_polling_storage.go +++ b/internal/monitoring/monitor_polling_storage.go @@ -375,6 +375,7 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string, Shared: shared, Enabled: storage.Enabled == 1, Active: storage.Active == 1, + LastSeen: startTime.UTC(), } if hasClusterConfig { @@ -605,6 +606,7 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string, Shared: true, Enabled: true, Active: true, + LastSeen: startTime.UTC(), } allStorage = append(allStorage, synthetic) diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index 80e398654..7ad3449e0 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -1621,7 +1621,7 @@ func resourceFromStorage(storage models.Storage) (Resource, ResourceIdentity) { Type: ResourceTypeStorage, Name: name, Status: statusFromStorage(storage), - LastSeen: now, + LastSeen: storage.LastSeen, UpdatedAt: now, Metrics: metricsFromStorage(storage), Proxmox: &ProxmoxData{ @@ -1973,7 +1973,7 @@ func resourceFromDockerContainer(ct models.DockerContainer, host models.DockerHo Technology: runtime, Name: ct.Name, Status: statusFromDockerState(ct.State), - LastSeen: time.Now().UTC(), + LastSeen: host.LastSeen, UpdatedAt: time.Now().UTC(), Metrics: metrics, } diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index 7844f041c..c4da3aa12 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -853,3 +853,61 @@ func TestNamespacedKubernetesResourceScaffold(t *testing.T) { t.Fatalf("identity clusterName = %q, want prod", identity.ClusterName) } } + +// Storage models carry the timestamp of the poll that delivered them. The +// adapter must pass that sighting through — and preserve zero for never-seen +// entries — instead of fabricating a fresh one at conversion time, because +// the registry is rebuilt from the retained snapshot every cycle and a +// conversion-time stamp would report a dead source as freshly delivering. +func TestResourceFromStorageUsesModelSighting(t *testing.T) { + seen := time.Now().UTC().Add(-45 * time.Second).Truncate(time.Millisecond) + storage := models.Storage{ + ID: "cluster-a-pve-1-local", + Name: "local", + Node: "pve-1", + Instance: "cluster-a", + Type: "dir", + Status: "available", + LastSeen: seen, + } + + resource, _ := resourceFromStorage(storage) + if !resource.LastSeen.Equal(seen) { + t.Fatalf("resource.LastSeen = %s, want the model sighting %s", resource.LastSeen, seen) + } + + storage.LastSeen = time.Time{} + resource, _ = resourceFromStorage(storage) + if !resource.LastSeen.IsZero() { + t.Fatalf("resource.LastSeen = %s, want zero for a never-seen storage entry", resource.LastSeen) + } +} + +// Docker containers are delivered wholesale with each host report, so the +// host's report timestamp is the container's sighting. Stamping conversion +// time instead would keep a container "fresh" forever after its host agent +// stopped reporting. +func TestResourceFromDockerContainerUsesHostSighting(t *testing.T) { + seen := time.Now().UTC().Add(-45 * time.Second).Truncate(time.Millisecond) + container := models.DockerContainer{ + ID: "abcdef123456", + Name: "web", + State: "running", + } + host := models.DockerHost{ + ID: "docker-1", + Hostname: "docker-1", + LastSeen: seen, + } + + resource, _ := resourceFromDockerContainer(container, host) + if !resource.LastSeen.Equal(seen) { + t.Fatalf("resource.LastSeen = %s, want the host report sighting %s", resource.LastSeen, seen) + } + + host.LastSeen = time.Time{} + resource, _ = resourceFromDockerContainer(container, host) + if !resource.LastSeen.IsZero() { + t.Fatalf("resource.LastSeen = %s, want zero when the host has never reported", resource.LastSeen) + } +} diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 2580b6759..9e9d1abb7 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -1513,6 +1513,20 @@ func attachLinkedAgentID(proxmox *ProxmoxData, linkedAgentID string) { func (rr *ResourceRegistry) ingestStorage(storage models.Storage) { resource, identity := resourceFromStorage(storage) + if pbsStorageInstance(storage) { + // Datastore-derived entries from the PBS poller (monitor_pbs_pmg) + // are delivered by the PBS source on the PBS cadence, not by a PVE + // poll. Keying them SourceProxmox would judge their freshness + // against the (faster) Proxmox stale threshold and flap healthy + // datastores stale between PBS polls. storage.Instance carries the + // PBS instance source ID ("pbs-"), so parent them to the PBS + // instance, which IngestSnapshot ingests before storage. + if parentID, ok := rr.bySource[SourcePBS][storage.Instance]; ok { + resource.ParentID = &parentID + } + rr.ingest(SourcePBS, storage.ID, resource, identity) + return + } parentSourceID := proxmoxNodeSourceID(storage.Instance, storage.Node) if parentID, ok := rr.bySource[SourceProxmox][parentSourceID]; ok { resource.ParentID = &parentID @@ -1520,6 +1534,15 @@ func (rr *ResourceRegistry) ingestStorage(storage models.Storage) { rr.ingest(SourceProxmox, storage.ID, resource, identity) } +// pbsStorageInstance reports whether a storage entry was produced by the PBS +// poller's datastore conversion rather than a PVE storage poll. PVE itself +// reports pbs-typed storage.cfg backends too, but those carry the PVE +// instance name; only the PBS poller writes the "pbs-" instance prefix. +func pbsStorageInstance(storage models.Storage) bool { + return strings.EqualFold(strings.TrimSpace(storage.Type), "pbs") && + strings.HasPrefix(strings.TrimSpace(storage.Instance), "pbs-") +} + func (rr *ResourceRegistry) ingestPhysicalDisk(disk models.PhysicalDisk) { resource, identity := resourceFromPhysicalDisk(disk) parentSourceID := proxmoxNodeSourceID(disk.Instance, disk.Node) diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index 892f2ea6a..75c3319c1 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -4521,3 +4521,83 @@ func TestIngestStampsOnlineSourceStatusForRealSighting(t *testing.T) { t.Fatalf("SourceStatus[proxmox].LastSeen = %s, want %s", status.LastSeen, seen) } } + +// Datastore-derived storage entries written by the PBS poller (instance +// "pbs-", type "pbs") are delivered on the PBS cadence, so they must be +// keyed to SourcePBS — judging them against the faster Proxmox stale +// threshold would flap healthy datastores stale between PBS polls. PVE also +// reports pbs-typed storage.cfg backends, and those stay SourceProxmox: they +// really are delivered by the PVE storage poll. +func TestIngestStorageRoutesPBSPollerEntriesToPBSSource(t *testing.T) { + seen := time.Now().UTC().Add(-30 * time.Second).Truncate(time.Millisecond) + snapshot := models.StateSnapshot{ + PBSInstances: []models.PBSInstance{{ + ID: "pbs-backup", + Name: "backup", + Host: "https://pbs.local:8007", + Status: "online", + LastSeen: seen, + }}, + Storage: []models.Storage{ + { + ID: "pbs-backup-main", + Name: "main", + Node: "backup", + Instance: "pbs-backup", + Type: "pbs", + Status: "available", + LastSeen: seen, + }, + { + ID: "home-pve1-pbs-remote", + Name: "pbs-remote", + Node: "pve1", + Instance: "home", + Type: "pbs", + Status: "available", + LastSeen: seen, + }, + }, + } + + rr := NewRegistry(nil) + rr.IngestSnapshot(snapshot) + + var pbsInstanceID string + var pollerStorage, pveStorage *Resource + for _, resource := range rr.List() { + resource := resource + switch { + case resource.Type == ResourceTypePBS: + pbsInstanceID = resource.ID + case resource.Type == ResourceTypeStorage && resource.Proxmox != nil && resource.Proxmox.SourceID == "pbs-backup-main": + pollerStorage = &resource + case resource.Type == ResourceTypeStorage && resource.Proxmox != nil && resource.Proxmox.SourceID == "home-pve1-pbs-remote": + pveStorage = &resource + } + } + if pbsInstanceID == "" { + t.Fatal("expected a PBS instance resource") + } + if pollerStorage == nil || pveStorage == nil { + t.Fatalf("expected both storage resources (pollerStorage=%v, pveStorage=%v)", pollerStorage != nil, pveStorage != nil) + } + + if len(pollerStorage.Sources) != 1 || pollerStorage.Sources[0] != SourcePBS { + t.Fatalf("PBS poller storage Sources = %v, want [%s]", pollerStorage.Sources, SourcePBS) + } + status, ok := pollerStorage.SourceStatus[SourcePBS] + if !ok { + t.Fatal("expected a pbs source status entry on the PBS poller storage") + } + if !status.LastSeen.Equal(seen) { + t.Fatalf("SourceStatus[pbs].LastSeen = %s, want %s", status.LastSeen, seen) + } + if pollerStorage.ParentID == nil || *pollerStorage.ParentID != pbsInstanceID { + t.Fatalf("PBS poller storage ParentID = %v, want the PBS instance %q", pollerStorage.ParentID, pbsInstanceID) + } + + if len(pveStorage.Sources) != 1 || pveStorage.Sources[0] != SourceProxmox { + t.Fatalf("PVE-reported pbs-typed storage Sources = %v, want [%s]", pveStorage.Sources, SourceProxmox) + } +}