From ea1a07a721fd25b3b7513da2f82b03a5fc39b992 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:11:51 +0100 Subject: [PATCH] Keep a source's own offline verdict through the stale pass The registry's stale pass recomputed a resource's status from its source sightings, and read a sighting that had delivered recently as "online". A sighting only says the source delivered, not what it reported, so: - A Proxmox node the cluster reports offline on a live poll came back online once its linked host agent went silent, which is what a powered-off machine's agent does. The Proxmox nodes table showed an amber dot beside its own "Offline" label while the drawer led with a critical "Node is offline" alert. - A node the poller expired after failed polls read as a warning ("Degraded"), and the stopped guests on it counted as needing attention. Each sighting now records the status its source reported, and the aggregation reads that verdict. Current sources decide in the same priority order the merge uses, so a live agent still keeps a node online when the Proxmox API path to it fails. A source that went quiet drops out while another is current. Once every source is quiet, an offline verdict survives and anything else reads as warning, as before. This generalises the lease marker that kept a silent agent offline, so a stopped guest keeps its offline verdict too when its only source goes quiet. A running guest whose only source goes quiet still reads as warning. Sightings still report delivery, so health keeps its telemetry_stale reason. An in-memory copy keeps its verdicts; only a copy that lost them all, a serialized one, takes its stored status as each source's verdict. Availability checks projected onto a target still count by delivery, as before. Giving them a verdict needs each check's own freshness, so it is left to a follow-up. --- .../v6/internal/subsystems/monitoring.md | 13 + .../internal/subsystems/storage-recovery.md | 20 +- .../internal/subsystems/unified-resources.md | 83 +++-- internal/unifiedresources/clone_test.go | 8 +- internal/unifiedresources/registry.go | 111 ++++--- .../registry_merge_policy_test.go | 308 ++++++++++++++++++ internal/unifiedresources/types.go | 14 +- 7 files changed, 460 insertions(+), 97 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 612334dea6..1806e08bbc 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -3981,6 +3981,19 @@ offline. Node memory carry-over after a failed status read uses the poller's own validated `NodeMemorySnapshot`, not the read state, which holds the agent's reading on a node merged with a host agent, and keeps used, cache and free summing to the total. +The poller's node verdicts reach the read state as its own. A cluster member +`pollPVENode` reports offline on a live poll stays `offline` in +`NodesSnapshot`, `/api/resources` and every read-state consumer when its linked +host agent falls silent. A node `preserveOrExpireNodes` expires stays `offline` +once its Proxmox sighting goes stale, unless another source says otherwise: a +reporting agent or a current availability check keeps it `online`, and a quiet +agent whose last verdict was not offline makes it `warning`. The registry's +stale pass used to turn the first into `online` and the second into `warning`, +because it read a sighting that had delivered recently as online +(unified-resources, "Status follows each source's own verdict"). A node inside +the offline grace keeps its `online` verdict and reads `warning` once its +sighting goes stale. Node alerts were never affected, because `CheckNode` reads +the poller's own `models.Node`. Legacy SSH temperature collection must also use the Pulse sensor-wrapper contract before falling back to raw lm-sensors output. `internal/monitoring/temperature.go` must request `/usr/local/sbin/pulse-sensors` when it exists, parse the wrapper diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index ce49337a20..446b898071 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -3178,15 +3178,19 @@ pointer isolation are pinned by `TestResourceFromHostPreservesCustomSensorMeta` and `TestCloneResourceIsolatesCustomSensorValues`. -### Expired reporting leases change status only +### Source status verdicts change status only -`SourceStatus` now carries an unexported marker that a push reporter (host -agent, Docker, Kubernetes) delivered its machine, Docker host or cluster as -offline at lease expiry. Status aggregation reads it, so such a row is -`offline` instead of `warning`. The marker is not serialized, persisted or -exposed on the wire, and the public `status`, `lastSeen` and cadence fields -are unchanged. No storage path, recovery read or protection judgement depends -on it. `TestCloneResource_MutateSourceStatusMap` pins that clones keep it. +`SourceStatus` carries an unexported verdict: the status each source reported +for the resource. It replaced the narrower marker for push reporters past their +lease. Status aggregation reads it, so a node a live poll reports offline, a +node the poller expired, an agent past its lease and a stopped guest on a quiet +source are `offline` instead of `online` or `warning`, unless another source +reports otherwise. The verdict is not serialized, persisted or exposed on the +wire, and the public `status`, `lastSeen` and cadence fields are unchanged. A +storage or datastore row that reported itself unavailable likewise stays +`offline` when its only source's poll goes quiet. No storage path, recovery +read or protection judgement depends on that status. +`TestCloneResource_MutateSourceStatusMap` pins that clones keep the verdict. ### Shared system-settings boundary dropped dead auto-update schedule fields diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 15b99bb727..d810dc84e0 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -2284,7 +2284,7 @@ and temperature facts must also compose the frontend-primitives desktop rows use the shared fixed label track so labels and values remain visually adjacent instead of spanning the full drawer width. A standalone host agent past its reporting lease reaches the Machines table as -`offline`, as described in "A push reporter past its lease is offline". The +`offline`, as described in "Status follows each source's own verdict". The offline metric fallback blanks its CPU, memory, disk, network, disk I/O, uptime and temperature cells. The machine drawer drops a silent agent's uptime and marks its non-disk Thermals rows "(last known)" with the reason as title, @@ -6116,33 +6116,64 @@ to that boolean. Identity succession continues to rekey the same state row. round-trip persistence, attention suppression, and the retired remediation lock. -### A push reporter past its lease is offline +### Status follows each source's own verdict -The host agent, Docker and Kubernetes collectors push their own reports, and -the monitor marks the machine, Docker host or cluster offline only once its -reporting lease runs out. Ingest records that verdict on the source's -sighting, and `aggregateStatus` counts such a sighting as `offline` whatever -its age. The stale pass used to rank the stale sighting above `offline`, so a -silent standalone agent reached every consumer as `warning`. The Machines -table then showed it amber with its last report rendered as current readings, -while its own drawer led with a critical "Host is offline" alert. A Docker host -was offline only until its sighting crossed the 120-second stale threshold, -then flipped to `warning`. With this rule, such rows take the existing -offline treatment: frontend danger gates, the health verdict's `offline` -reason, offline filters and counts. +A sighting's `SourceStatus.Status` describes delivery, not the resource: +`online` means the source delivered recently and `stale` that it has gone +quiet. Ingest records on each sighting the status that source reported for the +resource, as an unexported verdict, and `aggregateStatus` reads it. The stale +pass and manual links both apply that function: -`SourceStatus.Status` keeps describing delivery freshness, so the sighting -still reads `stale`. Health keeps its `telemetry_stale` reason, and -monitored-system reasons are unchanged. A live source still carries a merged -row: a Proxmox node whose linked agent stopped reporting stays `online` through -the PVE poll. Pull sources (Proxmox, PBS, PMG, TrueNAS, vSphere) and the -resources push collectors report about (guests, containers, pods, disks) keep -the stale-to-warning rule. A JSON copy re-derives the unexported marker from -its stored status in `IngestResources`, and a manual link that joins two -resources reported by one source keeps the fresher sighting. The tests in -`registry_merge_policy_test.go` pin the agent, Docker, Kubernetes, -mixed-source, round-trip and manual-link cases, plus the boundaries that stay -unchanged. +- While any source with a verdict is current, only the current sources decide, + in the priority order `chooseStatus` uses at merge time. The highest-priority + verdict wins (agent, then Proxmox or vSphere, then Docker, then the rest), and + the best of equal ones. A quiet source drops out of that decision. +- Once every source is quiet, an `offline` verdict survives and any other + verdict reads `warning`. The best of those wins. +- A current facet sighting carries no verdict (the PBS host-agent association, + an availability check projected onto its target). It counts as `online` only + when no current source has a verdict, and reads `warning` once quiet. + +The stale pass used to read delivery as the verdict. A Proxmox node the cluster +reported offline on a live poll came back `online` once its linked agent fell +silent, which is what a powered-off machine's agent does. The Proxmox nodes +table then put an amber dot beside its own "Offline" label, while the drawer +led with a critical "Node is offline" alert. A node `preserveOrExpireNodes` +expired after failed polls read `warning` ("Degraded"), and the stopped guests +on it counted as needing attention. A silent standalone agent past its +reporting lease reached every consumer as `warning`, with its last report +rendered as current readings in the Machines table. + +The push reporters keep their lease meaning. The host agent, Docker and +Kubernetes collectors mark the machine, Docker host or cluster offline only +once its reporting lease runs out. An agent past its lease is `offline` and +takes the existing offline treatment: frontend danger gates, the health +verdict's `offline` reason, offline filters and counts. One that is late but +inside its lease stays `warning`. A Docker host past its shorter lease stays +offline after its sighting crosses the 120-second stale threshold. A live +source still carries a merged row. A Proxmox node whose live poll reports it +online stays `online` when its linked agent stops reporting. A live agent keeps +a node `online` when the Proxmox poll reports it offline, which the Proxmox +nodes table reads with `connectionHealth: error` as a stale provider. When a +guest's or container's only source goes quiet, a running one stays `warning` +and a stopped one stays `offline`. + +The sighting still reads `stale`, so health keeps its `telemetry_stale` reason +and monitored-system reasons are unchanged. In-memory clones keep the verdicts, +including a facet's missing one. A serialized copy carries none, so +`IngestResources` gives each sighting of a copy that lost them all the stored +resource status; a merged row's separate source verdicts do not survive that +round trip. A manual link that joins two resources reported by one source keeps +the fresher sighting. `registry_merge_policy_test.go` pins the node, +poller-expired, guest, round-trip, copy, agent, Docker, Kubernetes, +mixed-source and manual-link cases, and the aggregation table. + +Availability checks still count by delivery. With every source that has a +verdict quiet, a current check keeps its target `online` even when the check +fails, as it did before. Several checks share one sighting whose freshness +comes from whichever check was projected last, so an availability verdict +needs each check's own freshness, including local checks that miss their +cadence, and has to survive manual links. That is left to a follow-up. ### Canonical object drawer hierarchy diff --git a/internal/unifiedresources/clone_test.go b/internal/unifiedresources/clone_test.go index 151f411769..e46dc22840 100644 --- a/internal/unifiedresources/clone_test.go +++ b/internal/unifiedresources/clone_test.go @@ -221,17 +221,17 @@ func TestCloneResource_MutateSourceStatusMap(t *testing.T) { ID: "r-1", SourceStatus: map[DataSource]SourceStatus{ SourceProxmox: {Status: "online", ExpectedUpdateIntervalSeconds: 300}, - SourceAgent: {Status: "stale", leaseExpired: true}, + SourceAgent: {Status: "stale", reported: StatusOffline}, }, } cloned := cloneResource(original) if cloned.SourceStatus[SourceProxmox].ExpectedUpdateIntervalSeconds != 300 { t.Fatal("clone lost collector cadence") } - // A clone that dropped the lease verdict would aggregate a silent agent + // A clone that dropped the source verdict would aggregate a silent agent // back to warning. - if !cloned.SourceStatus[SourceAgent].leaseExpired { - t.Fatal("clone lost the agent's expired reporting lease") + if cloned.SourceStatus[SourceAgent].reported != StatusOffline { + t.Fatal("clone lost the agent's offline verdict") } cloned.SourceStatus[SourceDocker] = SourceStatus{Status: "online"} diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 75eaedea1a..48bc5b03cd 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -809,11 +809,13 @@ func (rr *ResourceRegistry) ingestResources(resources []Resource, thresholds map } } } - // The lease marker is unexported, so a persisted or serialized copy - // arrives without it. Its stored status still carries the verdict. - for source, status := range resource.SourceStatus { - if !status.leaseExpired && reportingLeaseExpired(source, *resource) { - status.leaseExpired = true + // Source verdicts are unexported, so a serialized copy arrives with + // none; an in-memory clone keeps them, and a facet sighting may lack + // one on purpose. Only a copy that lost them all takes its stored + // status as each source's verdict. + if !sourceVerdictsRecorded(resource.SourceStatus) && resource.Status != "" && resource.Status != StatusUnknown { + for source, status := range resource.SourceStatus { + status.reported = resource.Status resource.SourceStatus[source] = status } } @@ -1764,27 +1766,13 @@ func sourceSightingStatus(lastSeen time.Time) string { return "online" } -// reportingLeaseExpired reports whether a source delivered the machine, -// Docker host or Kubernetes cluster it reports for as offline. Those -// collectors push their own reports, and Pulse marks the reporter offline -// only once its reporting lease runs out (evaluateHostAgents, -// evaluateDockerAgents, evaluateKubernetesAgents). The registry's stale -// threshold is a different clock: it can mark the sighting stale before -// the lease ends, or leave it fresh after, so it must not decide the status. -// Resources those sources describe (guests, containers, pods, disks) are -// not lease holders and keep the stale-to-warning rule. -func reportingLeaseExpired(source DataSource, resource Resource) bool { - if resource.Status != StatusOffline { - return false - } - switch source { - case SourceAgent, SourceDocker: - return resource.Type == ResourceTypeAgent - case SourceK8s: - return resource.Type == ResourceTypeK8sCluster - default: - return false +func sourceVerdictsRecorded(sightings map[DataSource]SourceStatus) bool { + for _, sighting := range sightings { + if sighting.reported != "" { + return true + } } + return false } func (rr *ResourceRegistry) markStaleLocked(now time.Time, thresholds map[DataSource]time.Duration) { @@ -2879,7 +2867,7 @@ func (rr *ResourceRegistry) ingestRecord(source DataSource, sourceID string, res sighting := resource.SourceStatus[source] sighting.Status = sourceSightingStatus(resource.LastSeen) sighting.LastSeen = resource.LastSeen - sighting.leaseExpired = reportingLeaseExpired(source, resource) + sighting.reported = resource.Status resource.SourceStatus = map[DataSource]SourceStatus{source: sighting} resource.parentBySource = make(map[DataSource]string) rr.setSourceParent(&resource, source, resource.ParentID) @@ -3820,7 +3808,7 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou sighting := incoming.SourceStatus[source] sighting.Status = sourceSightingStatus(incoming.LastSeen) sighting.LastSeen = incoming.LastSeen - sighting.leaseExpired = reportingLeaseExpired(source, incoming) + sighting.reported = incoming.Status existing.SourceStatus[source] = sighting if incoming.LastSeen.After(existing.LastSeen) { @@ -5857,39 +5845,54 @@ func chooseStatus(existing ResourceStatus, incoming ResourceStatus, source DataS return incoming } +// aggregateStatus derives a resource's status from its source sightings. A +// sighting's Status only says whether the source delivered recently; the +// status the source reported is its verdict. While any source with a verdict +// is current, only the current sources decide, in chooseStatus's priority +// order: the highest-priority verdict, the best of equal ones. A source that +// went quiet drops out of that decision, so a node the cluster reports +// offline stays offline when its linked agent falls silent. A current facet +// sighting without a verdict (the PBS association, an availability check) +// counts as online only when no current source has one. Once every source is +// quiet, an offline verdict survives (a node the poller expired, an agent past +// its lease) and any other reads as warning; the best of those wins. func aggregateStatus(resource *Resource) ResourceStatus { - statusPriority := map[string]int{ - "online": 3, - "stale": 2, - "offline": 1, - } - best := StatusUnknown - bestScore := 0 - for _, status := range resource.SourceStatus { - state := strings.ToLower(strings.TrimSpace(status.Status)) - // A reporter past its lease is offline, not merely late. Ranking its - // stale sighting above offline would show a silent machine as a - // warning while its retained readings render as current. - if status.leaseExpired { - state = "offline" - } - score := statusPriority[state] - if score > bestScore { - bestScore = score - switch state { - case "online": - best = StatusOnline - case "stale": - best = StatusWarning - case "offline": - best = StatusOffline + var current, quiet ResourceStatus + currentPriority := -1 + deliveredWithoutVerdict := false + for source, sighting := range resource.SourceStatus { + switch strings.ToLower(strings.TrimSpace(sighting.Status)) { + case "online": + switch sighting.reported { + case "": + deliveredWithoutVerdict = true + continue + case StatusUnknown: + continue } + if priority := sourcePriority(source); priority > currentPriority { + current, currentPriority = sighting.reported, priority + } else if priority == currentPriority { + current = betterPresentationStatus(current, sighting.reported) + } + case "stale", "offline": + verdict := StatusWarning + if sighting.reported == StatusOffline || strings.EqualFold(strings.TrimSpace(sighting.Status), "offline") { + verdict = StatusOffline + } + quiet = betterPresentationStatus(quiet, verdict) } } - if best == "" { + switch { + case currentPriority >= 0: + return current + case deliveredWithoutVerdict: + return StatusOnline + case quiet != "": + return quiet + default: return StatusUnknown } - return best } func (rr *ResourceRegistry) isExcluded(a, b string) bool { diff --git a/internal/unifiedresources/registry_merge_policy_test.go b/internal/unifiedresources/registry_merge_policy_test.go index 14d407aa2e..ecf99d30e4 100644 --- a/internal/unifiedresources/registry_merge_policy_test.go +++ b/internal/unifiedresources/registry_merge_policy_test.go @@ -479,6 +479,314 @@ func TestStaleProxmoxNodeStaysWarning(t *testing.T) { } } +// A Proxmox node the cluster reports offline on a live poll (pollPVENode +// stamps LastSeen now) stays offline when its linked agent falls silent, as a +// powered-off machine's agent does. The stale pass used to read the live PVE +// sighting as online, because it only says the poll delivered, and lifted the +// node back to online. +func TestProxmoxNodeReportedOfflineStaysOfflineWhenItsAgentFallsSilent(t *testing.T) { + now := time.Now().UTC() + for _, tc := range []struct { + name string + agentStatus string + agentSeen time.Time + }{ + {name: "agent late inside its lease", agentStatus: "online", agentSeen: now.Add(-90 * time.Second)}, + {name: "agent past its lease", agentStatus: "offline", agentSeen: now.Add(-5 * time.Minute)}, + } { + t.Run(tc.name, func(t *testing.T) { + const machineID = "machine-pve1" + rr := NewRegistry(nil) + rr.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{{ + ID: "homelab-pve1", + Name: "pve1", + Instance: "homelab", + ClusterName: "homelab", + Status: "offline", + ConnectionHealth: "error", + LastSeen: now, + }}, + Hosts: []models.Host{{ + ID: machineID, + MachineID: machineID, + Hostname: "pve1", + LinkedNodeID: "homelab-pve1", + Status: tc.agentStatus, + LastSeen: tc.agentSeen, + IntervalSeconds: 30, + }}, + }) + + resource := onlyResourceOfType(t, rr, ResourceTypeAgent) + if got := resource.SourceStatus[SourceProxmox].Status; got != "online" { + t.Fatalf("proxmox sighting = %q, want the live poll's delivery", got) + } + if got := resource.SourceStatus[SourceAgent].Status; got != "stale" { + t.Fatalf("agent sighting = %q, want stale", got) + } + if resource.Status != StatusOffline { + t.Fatalf("status = %q, want offline as the live poll reported", resource.Status) + } + nodes := rr.Nodes() + if len(nodes) != 1 || nodes[0].Status() != StatusOffline { + t.Fatalf("node view status = %v, want offline", nodes) + } + + rr.MarkStale(time.Now().UTC(), nil) + if got, _ := rr.Get(resource.ID); got.Status != StatusOffline { + t.Fatalf("second stale pass changed status to %q, want offline", got.Status) + } + }) + } +} + +// preserveOrExpireNodes marks a node offline once its poll has failed past +// the grace period and keeps its old LastSeen. That verdict is the poller's +// own and survives the sighting going stale; it used to read as warning. +func TestPollerExpiredProxmoxNodeStaysOffline(t *testing.T) { + expiredAt := time.Now().UTC().Add(-10 * time.Minute) + node := models.Node{ + ID: "homelab-pve9", + Name: "pve9", + Instance: "homelab", + ClusterName: "homelab", + Status: "offline", + ConnectionHealth: "error", + LastSeen: expiredAt, + } + + t.Run("node alone", func(t *testing.T) { + rr := NewRegistry(nil) + rr.IngestSnapshot(models.StateSnapshot{Nodes: []models.Node{node}}) + + resource := onlyResourceOfType(t, rr, ResourceTypeAgent) + if got := resource.SourceStatus[SourceProxmox].Status; got != "stale" { + t.Fatalf("proxmox sighting = %q, want stale", got) + } + if resource.Status != StatusOffline { + t.Fatalf("status = %q, want offline for a node the poller expired", resource.Status) + } + // The sighting still reports the lost poller to health. + health := EvaluateResourceHealth(resource, nil, time.Now().UTC()) + if health.Verdict != HealthCritical || len(health.Reasons) < 2 || + health.Reasons[0].Code != "offline" || health.Reasons[1].Code != "telemetry_stale" { + t.Fatalf("health = %+v, want critical offline with the stale reason after it", health) + } + }) + + t.Run("node and its silent agent", func(t *testing.T) { + const machineID = "machine-pve9" + rr := NewRegistry(nil) + rr.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{node}, + Hosts: []models.Host{{ + ID: machineID, + MachineID: machineID, + Hostname: "pve9", + LinkedNodeID: node.ID, + Status: "offline", + LastSeen: expiredAt, + IntervalSeconds: 30, + }}, + }) + + resource := onlyResourceOfType(t, rr, ResourceTypeAgent) + if _, ok := resource.SourceStatus[SourceAgent]; !ok { + t.Fatalf("expected the node and its agent to share one resource, got sources %+v", resource.SourceStatus) + } + if resource.Status != StatusOffline { + t.Fatalf("status = %q, want offline when both sources last reported it offline", resource.Status) + } + }) +} + +// Guests take the same rule: a guest Proxmox last saw stopped stays offline +// when the poll goes quiet, while a running one is only known to be stale. +func TestQuietProxmoxPollKeepsEachGuestsLastVerdict(t *testing.T) { + lastPoll := time.Now().UTC().Add(-10 * time.Minute) + rr := NewRegistry(nil) + rr.IngestSnapshot(models.StateSnapshot{ + VMs: []models.VM{ + {ID: "homelab-pve9-101", VMID: 101, Name: "stopped-vm", Node: "pve9", Instance: "homelab", Type: "qemu", Status: "stopped", LastSeen: lastPoll}, + {ID: "homelab-pve9-102", VMID: 102, Name: "running-vm", Node: "pve9", Instance: "homelab", Type: "qemu", Status: "running", LastSeen: lastPoll}, + }, + }) + + want := map[string]ResourceStatus{"stopped-vm": StatusOffline, "running-vm": StatusWarning} + vms := rr.ListByType(ResourceTypeVM) + if len(vms) != len(want) { + t.Fatalf("expected %d VMs, got %d", len(want), len(vms)) + } + for _, vm := range vms { + if got := vm.SourceStatus[SourceProxmox].Status; got != "stale" { + t.Fatalf("%s sighting = %q, want stale", vm.Name, got) + } + if vm.Status != want[vm.Name] { + t.Fatalf("%s status = %q, want %q", vm.Name, vm.Status, want[vm.Name]) + } + } +} + +// A copy that went through JSON loses the unexported verdicts and recovers +// them from its stored status, so a node reported offline with a silent +// agent does not come back online. +func TestProxmoxNodeReportedOfflineStaysOfflineAfterJSONRoundTrip(t *testing.T) { + now := time.Now().UTC() + const machineID = "machine-pve1" + source := NewRegistry(nil) + source.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{{ + ID: "homelab-pve1", + Name: "pve1", + Instance: "homelab", + ClusterName: "homelab", + Status: "offline", + ConnectionHealth: "error", + LastSeen: now, + }}, + Hosts: []models.Host{{ + ID: machineID, + MachineID: machineID, + Hostname: "pve1", + LinkedNodeID: "homelab-pve1", + Status: "offline", + LastSeen: now.Add(-5 * time.Minute), + IntervalSeconds: 30, + }}, + }) + payload, err := json.Marshal(source.List()) + if err != nil { + t.Fatalf("marshal resources: %v", err) + } + var copied []Resource + if err := json.Unmarshal(payload, &copied); err != nil { + t.Fatalf("unmarshal resources: %v", err) + } + + rr := NewRegistry(nil) + rr.IngestResources(copied) + + resource := onlyResourceOfType(t, rr, ResourceTypeAgent) + if resource.Status != StatusOffline { + t.Fatalf("status = %q, want offline after a JSON round trip", resource.Status) + } +} + +// An in-memory copy keeps its verdicts, and a facet sighting that has none +// on purpose (the PBS host-agent association) must not take the stored +// status as one: that would let the facet decide once the agent goes quiet. +func TestIngestResourcesKeepsAFacetSightingWithoutAVerdict(t *testing.T) { + now := time.Now().UTC() + rr := NewRegistry(nil) + rr.IngestResources([]Resource{{ + ID: "agent-pbs-host", + Type: ResourceTypeAgent, + Name: "pbs-host", + Status: StatusWarning, + LastSeen: now, + Sources: []DataSource{SourceAgent, SourcePBS}, + SourceStatus: map[DataSource]SourceStatus{ + SourceAgent: {Status: "online", LastSeen: now, reported: StatusWarning}, + SourcePBS: {Status: "online", LastSeen: now}, + }, + Agent: &AgentData{AgentID: "host-pbs", Hostname: "pbs-host"}, + }}) + + resource, ok := rr.Get("agent-pbs-host") + if !ok { + t.Fatal("resource missing") + } + if got := resource.SourceStatus[SourceAgent].reported; got != StatusWarning { + t.Fatalf("agent verdict = %q, want the copied warning", got) + } + if got := resource.SourceStatus[SourcePBS].reported; got != "" { + t.Fatalf("PBS facet verdict = %q, want none", got) + } + if resource.Status != StatusWarning { + t.Fatalf("status = %q, want the agent's warning", resource.Status) + } +} + +// aggregateStatus is the one rule the stale pass and manual links apply. +// Current sources decide by their own verdicts in chooseStatus's priority +// order; quiet sources only decide when no source is current. +func TestAggregateStatusReadsSourceVerdictsNotDelivery(t *testing.T) { + current := func(reported ResourceStatus) SourceStatus { + return SourceStatus{Status: "online", reported: reported} + } + quiet := func(reported ResourceStatus) SourceStatus { + return SourceStatus{Status: "stale", reported: reported} + } + for _, tc := range []struct { + name string + sightings map[DataSource]SourceStatus + want ResourceStatus + }{ + { + name: "live poll reports offline, agent quiet", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusOffline), SourceAgent: quiet(StatusOnline)}, + want: StatusOffline, + }, + { + name: "live poll reports online, agent past its lease", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusOnline), SourceAgent: quiet(StatusOffline)}, + want: StatusOnline, + }, + { + name: "live agent outranks a poll that reports offline", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusOffline), SourceAgent: current(StatusOnline)}, + want: StatusOnline, + }, + { + name: "live agent warning holds as at merge time", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusOnline), SourceAgent: current(StatusWarning), SourceDocker: quiet(StatusOnline)}, + want: StatusWarning, + }, + { + name: "quiet source that reported offline keeps it", + sightings: map[DataSource]SourceStatus{SourceProxmox: quiet(StatusOffline)}, + want: StatusOffline, + }, + { + name: "quiet source that reported online is a warning", + sightings: map[DataSource]SourceStatus{SourceProxmox: quiet(StatusOnline)}, + want: StatusWarning, + }, + { + name: "best of quiet sources", + sightings: map[DataSource]SourceStatus{SourceProxmox: quiet(StatusOnline), SourceAgent: quiet(StatusOffline)}, + want: StatusWarning, + }, + { + name: "current facet without a verdict counts as online", + sightings: map[DataSource]SourceStatus{SourceProxmox: quiet(StatusOffline), SourcePBS: current("")}, + want: StatusOnline, + }, + { + name: "a facet without a verdict never outranks a current verdict", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusOffline), SourceAvailability: current("")}, + want: StatusOffline, + }, + { + name: "current unknown verdict leaves the quiet sources to decide", + sightings: map[DataSource]SourceStatus{SourceProxmox: current(StatusUnknown), SourceAgent: quiet(StatusOnline)}, + want: StatusWarning, + }, + { + name: "no delivered sightings", + sightings: map[DataSource]SourceStatus{SourceProxmox: {Status: "unknown"}}, + want: StatusUnknown, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := aggregateStatus(&Resource{SourceStatus: tc.sightings}); got != tc.want { + t.Fatalf("aggregateStatus = %q, want %q", got, tc.want) + } + }) + } +} + func TestCloneDockerDataPreservesContainerRuntimeMetadata(t *testing.T) { startedAt := time.Date(2026, 6, 11, 13, 15, 30, 0, time.UTC) finishedAt := startedAt.Add(45 * time.Minute) diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index d64e288bef..dd55de384d 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -291,16 +291,20 @@ const ( // SourceStatus describes the freshness of data from a source. type SourceStatus struct { + // Status describes delivery, not the resource: "online" means the source + // delivered recently, "stale" that it has gone quiet. Status string `json:"status"` // online, stale, offline LastSeen time.Time `json:"lastSeen"` Error string `json:"error,omitempty"` ExpectedUpdateIntervalSeconds int64 `json:"expectedUpdateIntervalSeconds,omitempty"` // Collector-authored cadence, zero uses source default - // leaseExpired records that this source delivered the machine, Docker - // host or cluster it reports for as offline because its reporting lease - // ran out. Status aggregation counts that as offline whatever the - // sighting's age, while Status keeps describing delivery freshness. - leaseExpired bool + // reported is the status this source itself delivered for the resource: + // a node the cluster reports offline, a host agent past its reporting + // lease, a stopped guest. Status aggregation reads this verdict, so a + // source that delivered recently does not make the resource online. A + // facet sighting (the PBS association, an availability check) carries + // none; see aggregateStatus. + reported ResourceStatus } // ResourceIdentity holds identifiers used for matching.