diff --git a/internal/monitoring/connected_infrastructure.go b/internal/monitoring/connected_infrastructure.go index 38486df05..453eb9a12 100644 --- a/internal/monitoring/connected_infrastructure.go +++ b/internal/monitoring/connected_infrastructure.go @@ -266,7 +266,7 @@ func addConnectedInfrastructureSurface( hostname: connectedInfrastructureHostname(resource), status: "active", healthStatus: strings.TrimSpace(string(resource.Status)), - lastSeen: resource.LastSeen.UnixMilli(), + lastSeen: connectedInfrastructureLastSeenMillis(resource), version: connectedInfrastructureVersion(resource), isOutdatedBinary: connectedInfrastructureIsOutdated(resource), linkedNodeID: connectedInfrastructureLinkedNodeID(resource), @@ -282,7 +282,7 @@ func addConnectedInfrastructureSurface( groups[key] = group } - if resourceLastSeen := resource.LastSeen.UnixMilli(); resourceLastSeen > group.lastSeen { + if resourceLastSeen := connectedInfrastructureLastSeenMillis(resource); resourceLastSeen > group.lastSeen { group.lastSeen = resourceLastSeen } if group.version == "" { @@ -437,6 +437,16 @@ func connectedInfrastructureHostname(resource unifiedresources.Resource) string return "" } +// connectedInfrastructureLastSeenMillis converts a resource's last sighting +// to frontend milliseconds. A never-seen resource (zero LastSeen) reports 0, +// not the huge negative UnixMilli of the zero time. +func connectedInfrastructureLastSeenMillis(resource unifiedresources.Resource) int64 { + if resource.LastSeen.IsZero() { + return 0 + } + return resource.LastSeen.UnixMilli() +} + func connectedInfrastructureName(resource unifiedresources.Resource) string { for _, candidate := range []string{ unifiedresources.ResourceDisplayName(resource), diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 167345627..f09d72aa8 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -6149,8 +6149,11 @@ func monitorStringValue(value *string) string { } func monitorLastSeenUnix(value time.Time) int64 { + // Zero means the resource has never actually been sighted (synthesized + // offline placeholders). Report 0 so the frontend renders "never" instead + // of fabricating a fresh sighting. if value.IsZero() { - return time.Now().UTC().UnixMilli() + return 0 } return value.UnixMilli() } diff --git a/internal/monitoring/monitor_conversion_helpers_additional_test.go b/internal/monitoring/monitor_conversion_helpers_additional_test.go index b02b280ed..a8ec7e61d 100644 --- a/internal/monitoring/monitor_conversion_helpers_additional_test.go +++ b/internal/monitoring/monitor_conversion_helpers_additional_test.go @@ -196,11 +196,10 @@ func TestMonitorIdentityLabelsSourceTypeAndLastSeenAdditional(t *testing.T) { t.Fatalf("monitorLastSeenUnix(non-zero) = %d, want %d", got, now.UnixMilli()) } - before := time.Now().UTC().UnixMilli() - zero := monitorLastSeenUnix(time.Time{}) - after := time.Now().UTC().UnixMilli() - if zero < before || zero > after { - t.Fatalf("monitorLastSeenUnix(zero) = %d, want between %d and %d", zero, before, after) + // A zero LastSeen means the resource has never been sighted; the + // payload must not fabricate a fresh timestamp for it. + if got := monitorLastSeenUnix(time.Time{}); got != 0 { + t.Fatalf("monitorLastSeenUnix(zero) = %d, want 0", got) } }) diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index fd731f750..5d2146abe 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -711,11 +711,11 @@ func (m *Monitor) preserveOrExpireNodes(prevInstanceNodes []models.Node) []model // Grace requires evidence of a real online sighting: either this // process saw the node online (nodeLastOnline), or the previous // snapshot itself reports it online with a recent poll timestamp. - // prevNode.LastSeen alone is NOT enough for an offline node — prev - // state round-trips through the unified registry, which stamps - // zero LastSeen values with the ingest time, so a synthesized - // offline placeholder would otherwise come back "recently seen" - // every cycle and be resurrected to online forever. + // prevNode.LastSeen alone is NOT enough for an offline node: a + // synthesized offline placeholder has never been sighted, and even + // though the unified registry now preserves its zero LastSeen + // instead of stamping ingest time, requiring an online sighting + // keeps this policy correct independent of registry stamping. m.mu.Lock() lastOnline, sawOnline := m.nodeLastOnline[prevNode.ID] m.mu.Unlock() diff --git a/internal/unifiedresources/monitor_adapter.go b/internal/unifiedresources/monitor_adapter.go index 1037bfda0..1b47d4d97 100644 --- a/internal/unifiedresources/monitor_adapter.go +++ b/internal/unifiedresources/monitor_adapter.go @@ -103,6 +103,11 @@ func (a *MonitorAdapter) replaceRegistry(snapshot models.StateSnapshot, recordsB } rebuilt.IngestRecords(source, records) } + // IngestSnapshot runs its stale pass before the record sources above are + // ingested, so record-sourced resources would otherwise keep their + // ingest-time "online" stamp regardless of how old their data is. + // Re-evaluate staleness over the fully assembled registry. + rebuilt.MarkStale(time.Now().UTC(), nil) rebuiltAt := time.Now().UTC() if !snapshot.LastUpdate.IsZero() && snapshot.LastUpdate.After(rebuiltAt) { rebuiltAt = snapshot.LastUpdate diff --git a/internal/unifiedresources/presentation_coalesce.go b/internal/unifiedresources/presentation_coalesce.go index 4a83be64c..69e98f8a3 100644 --- a/internal/unifiedresources/presentation_coalesce.go +++ b/internal/unifiedresources/presentation_coalesce.go @@ -333,7 +333,7 @@ func mergePresentationSourceStatus( if rightLastSeen.After(lastSeen) { lastSeen = rightLastSeen } - merged[source] = SourceStatus{Status: "online", LastSeen: lastSeen} + merged[source] = SourceStatus{Status: sourceSightingStatus(lastSeen), LastSeen: lastSeen} } return merged } diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 058c05091..2580b6759 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -504,7 +504,7 @@ func (rr *ResourceRegistry) IngestResources(resources []Resource) { resource.SourceStatus = make(map[DataSource]SourceStatus, len(resource.Sources)) for _, source := range resource.Sources { resource.SourceStatus[source] = SourceStatus{ - Status: "online", + Status: sourceSightingStatus(resource.LastSeen), LastSeen: resource.LastSeen, } } @@ -1271,6 +1271,19 @@ func (rr *ResourceRegistry) MarkStale(now time.Time, thresholds map[DataSource]t rr.markStaleLocked(now, thresholds) } +// sourceSightingStatus derives the per-source delivery status from the last +// time that source actually delivered the resource. A zero sighting means the +// source has never delivered it (synthesized offline placeholders, instances +// that have not completed a poll since startup); its delivery state is +// unknown, not online. Stamping it "online" would exempt it from +// stale-marking forever, because markStaleLocked skips zero LastSeen entries. +func sourceSightingStatus(lastSeen time.Time) string { + if lastSeen.IsZero() { + return "unknown" + } + return "online" +} + func (rr *ResourceRegistry) markStaleLocked(now time.Time, thresholds map[DataSource]time.Duration) { if thresholds == nil { thresholds = defaultStaleThresholds @@ -2198,15 +2211,11 @@ func (rr *ResourceRegistry) ingest(source DataSource, sourceID string, resource resource.Identity = identity resource.Sources = []DataSource{source} resource.SourceStatus = map[DataSource]SourceStatus{ - source: {Status: "online", LastSeen: resource.LastSeen}, + source: {Status: sourceSightingStatus(resource.LastSeen), LastSeen: resource.LastSeen}, } resource.parentBySource = make(map[DataSource]string) rr.setSourceParent(&resource, source, resource.ParentID) - if resource.LastSeen.IsZero() { - resource.LastSeen = time.Now().UTC() - } - // Linked resources must be mutually linked to avoid one-sided/ambiguous auto-merges. if linked := rr.resolveLinkedResource(source, sourceID, resource); linked != "" { existing := rr.resources[linked] @@ -2276,9 +2285,6 @@ func (rr *ResourceRegistry) mergeLinkedKubernetesNode( resource.Identity = identity resource.Type = CanonicalResourceType(resource.Type) - if resource.LastSeen.IsZero() { - resource.LastSeen = time.Now().UTC() - } rr.mergeInto(existing, resource, SourceK8s) rr.bySource[SourceK8s][sourceID] = existing.ID @@ -2486,7 +2492,7 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou if existing.SourceStatus == nil { existing.SourceStatus = make(map[DataSource]SourceStatus) } - existing.SourceStatus[source] = SourceStatus{Status: "online", LastSeen: incoming.LastSeen} + existing.SourceStatus[source] = SourceStatus{Status: sourceSightingStatus(incoming.LastSeen), LastSeen: incoming.LastSeen} if incoming.LastSeen.After(existing.LastSeen) { existing.LastSeen = incoming.LastSeen diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index ed4634584..892f2ea6a 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -4451,3 +4451,73 @@ func TestMergedHostCanonicalIDStableAcrossRestartsAndIngestOrders(t *testing.T) t.Fatalf("expected one merged resource after reversed-order ingest, got %d", got) } } + +// A synthesized placeholder (e.g. an offline PVE node for an instance that +// has never completed a poll) carries a zero LastSeen. Ingest must preserve +// that zero instead of fabricating a fresh sighting, and the per-source +// delivery status must be "unknown", not "online" — an online stamp with a +// zero LastSeen is permanently exempt from stale-marking and masks the +// source's real delivery state (the loop behind commit 8372a22c5). +func TestIngestPreservesNeverSeenSightingAndStampsUnknownSourceStatus(t *testing.T) { + rr := NewRegistry(nil) + + placeholder := topLevelTestProxmoxNode("proxmox-node", "tower", "proxmox-1", "https://tower.local:8006") + placeholder.Status = StatusOffline + placeholder.LastSeen = time.Time{} + + rr.IngestRecords(SourceProxmox, []IngestRecord{ + {SourceID: "proxmox-node", Resource: placeholder}, + }) + + resources := rr.List() + if len(resources) != 1 { + t.Fatalf("List() returned %d resources, want 1", len(resources)) + } + resource := resources[0] + + if !resource.LastSeen.IsZero() { + t.Fatalf("resource.LastSeen = %s, want zero: never-seen resources must not get a fabricated sighting", resource.LastSeen) + } + status, ok := resource.SourceStatus[SourceProxmox] + if !ok { + t.Fatal("expected a proxmox source status entry") + } + if status.Status != "unknown" { + t.Fatalf("SourceStatus[proxmox].Status = %q, want %q: a source that never delivered the resource must not be stamped online", status.Status, "unknown") + } + if !status.LastSeen.IsZero() { + t.Fatalf("SourceStatus[proxmox].LastSeen = %s, want zero", status.LastSeen) + } + + // Stale marking must leave a never-seen source alone in both directions. + rr.MarkStale(time.Now().UTC(), nil) + if got := rr.List()[0].SourceStatus[SourceProxmox].Status; got != "unknown" { + t.Fatalf("SourceStatus[proxmox].Status after MarkStale = %q, want %q", got, "unknown") + } +} + +// A real sighting keeps the online stamp and the genuine timestamp on both +// the resource and the per-source delivery status. +func TestIngestStampsOnlineSourceStatusForRealSighting(t *testing.T) { + rr := NewRegistry(nil) + seen := time.Now().UTC().Add(-5 * time.Second).Truncate(time.Millisecond) + + node := topLevelTestProxmoxNode("proxmox-node", "tower", "proxmox-1", "https://tower.local:8006") + node.LastSeen = seen + + rr.IngestRecords(SourceProxmox, []IngestRecord{ + {SourceID: "proxmox-node", Resource: node}, + }) + + resource := rr.List()[0] + if !resource.LastSeen.Equal(seen) { + t.Fatalf("resource.LastSeen = %s, want %s", resource.LastSeen, seen) + } + status := resource.SourceStatus[SourceProxmox] + if status.Status != "online" { + t.Fatalf("SourceStatus[proxmox].Status = %q, want online", status.Status) + } + if !status.LastSeen.Equal(seen) { + t.Fatalf("SourceStatus[proxmox].LastSeen = %s, want %s", status.LastSeen, seen) + } +}