From bf67ba92017858c5035d28ba13efae6a60c35807 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 24 Jul 2026 00:19:09 +0100 Subject: [PATCH] Fix cache-aware Linux memory fallbacks --- .../v6/internal/subsystems/agent-lifecycle.md | 7 + .../v6/internal/subsystems/alerts.md | 6 + .../v6/internal/subsystems/api-contracts.md | 7 + .../subsystems/frontend-primitives.md | 7 + .../v6/internal/subsystems/monitoring.md | 43 ++- .../subsystems/performance-and-scalability.md | 7 +- .../internal/subsystems/storage-recovery.md | 8 + .../internal/subsystems/unified-resources.md | 11 + .../resourceDetailMappers.branchcov.test.ts | 54 ++++ .../Infrastructure/resourceDetailMappers.ts | 9 +- .../src/components/Workloads/GuestRow.tsx | 17 +- .../Workloads/NodeDrawerOverview.tsx | 13 +- .../Workloads/StackedMemoryBar.test.tsx | 9 + .../components/Workloads/StackedMemoryBar.tsx | 7 +- .../components/Workloads/WorkloadPanel.tsx | 5 +- .../guestDrawerModel.branchcov2.test.ts | 34 ++ .../__tests__/stackedMemoryBarModel.test.ts | 26 ++ .../components/Workloads/guestDrawerModel.ts | 9 +- .../Workloads/stackedMemoryBarModel.ts | 28 +- .../docker/DockerHostDrawerOverview.tsx | 15 +- .../src/features/docker/DockerHostsTable.tsx | 11 +- .../__tests__/DockerHostsTable.test.tsx | 39 ++- .../features/proxmox/ProxmoxNodesTable.tsx | 13 +- .../standalone/AgentsMachinesTable.tsx | 4 + .../agentMachineTableModel.coverage.test.ts | 25 ++ .../standalone/agentMachineTableModel.ts | 3 + frontend-modern/src/types/api.ts | 2 + frontend-modern/src/types/resource.ts | 3 + .../resourceStateAdapters.coverage2.test.ts | 48 +++ .../__tests__/resourceStateAdapters.test.ts | 43 +++ .../src/utils/resourceStateAdapters.ts | 10 +- internal/alerts/guest.go | 6 +- internal/alerts/guest_snapshot.go | 87 ++--- internal/alerts/host.go | 4 +- internal/alerts/node.go | 6 +- internal/alerts/operational_contract_test.go | 103 ++++++ internal/alerts/unified_eval.go | 7 + internal/api/diagnostics_memory_test.go | 49 +++ internal/models/memory_availability_test.go | 51 +++ internal/models/models.go | 43 ++- .../monitoring/canonical_guardrails_test.go | 15 +- internal/monitoring/diagnostic_snapshots.go | 18 ++ internal/monitoring/guest_memory_agent.go | 60 +++- .../monitoring/guest_memory_agent_test.go | 92 ++++++ internal/monitoring/guest_memory_sources.go | 183 +++++++---- internal/monitoring/guest_memory_stability.go | 4 +- .../monitoring/guest_memory_stability_test.go | 28 ++ internal/monitoring/memory_source_catalog.go | 2 + .../monitoring/memory_source_catalog_test.go | 10 +- .../memory_trust_characterization_test.go | 299 ++++++++++++++++-- internal/monitoring/monitor.go | 108 +++++-- internal/monitoring/monitor_agents.go | 146 +++++---- .../monitoring/monitor_host_agents_test.go | 90 ++++++ internal/monitoring/monitor_memory_test.go | 25 ++ .../monitoring/monitor_polling_containers.go | 27 +- internal/monitoring/monitor_polling_node.go | 82 +++-- internal/monitoring/monitor_polling_vm.go | 2 +- .../monitoring/monitor_pve_guest_builders.go | 32 +- .../monitoring/monitor_pve_guest_helpers.go | 19 +- internal/monitoring/monitor_pve_guest_lxc.go | 88 +++--- internal/monitoring/monitor_pve_storage.go | 10 +- internal/monitoring/node_memory_sources.go | 194 ++++++------ .../monitoring/node_memory_sources_test.go | 16 + internal/unifiedresources/adapters.go | 67 ++-- internal/unifiedresources/adapters_test.go | 43 +++ .../canonical_identity_test.go | 54 ++++ internal/unifiedresources/clone.go | 12 + internal/unifiedresources/clone_test.go | 28 ++ internal/unifiedresources/metrics.go | 6 +- internal/unifiedresources/metrics_test.go | 87 +++++ internal/unifiedresources/registry.go | 35 ++ .../registry_merge_policy_test.go | 71 +++++ internal/unifiedresources/types.go | 63 ++-- pkg/proxmox/client.go | 182 +++++++++-- pkg/proxmox/client_test.go | 135 +++++++- pkg/proxmox/cluster_client.go | 15 + pkg/proxmox/cluster_client_api_test.go | 32 ++ 77 files changed, 2684 insertions(+), 575 deletions(-) create mode 100644 internal/models/memory_availability_test.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index a08b94627..68faf5c14 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -3716,6 +3716,13 @@ for lifecycle-adjacent diagnostics surfaces, legacy aliases and empty fallback-reason fields must still normalize onto the governed canonical reason contract instead of depending on monitor-owned snapshot accessors to have run first. +Unified Linux agents carry memory evidence across this lifecycle boundary, but +they do not own memory-source inference. A report with only total and free +bytes must remain explicitly usage-unavailable because Linux `MemFree` excludes +reclaimable page cache; a complete used/cache/free split or an explicit +cache-aware usage remains trusted. Lifecycle transport, reconnect, and JSON +reload must preserve `UsageUnavailable`, including known total capacity, +without turning it into zero usage or an enrollment/freshness failure. That same shared diagnostics dependency now also assumes local commercial and onboarding analytics stay out of user diagnostics entirely: lifecycle-adjacent admin surfaces may consume operational diagnostics, but they must not restore diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index fbd56886f..b5597d4ae 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -37,6 +37,12 @@ Availability incident and alert identity belongs to the source-owned machine, but it must not copy the check incident onto that machine or create a second alert lifecycle. Failure, recovery, history, acknowledgement, and notification routing therefore remain stable under relinking and restart. +Linux memory thresholds consume only canonical cache-aware usage. An explicit +usage-unavailable sample, a non-finite percentage, or contradictory +used/free/total evidence must not open or clear a memory alert. If such a +sample follows an active alert, the alert remains active with its last trusted +value until a subsequent trusted sample proves recovery; missing evidence is +not evidence that pressure disappeared. ## Canonical Files diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index d1087533b..0bad864c8 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -5901,6 +5901,13 @@ That same diagnostics boundary must also backfill canonical fallback reasons when a raw snapshot reaches the API layer without one, so `buildMemorySourceDiagnostics` stays self-consistent even if a caller bypasses `GetDiagnosticSnapshots()` and hands diagnostics a legacy alias directly. +The canonical `unavailable` memory source is serialized with trust +`unavailable`, fallback reason `cache-aware-memory-unavailable`, and the raw +source evidence that was actually observed. Node diagnostics keep status and +RRD available/used/total fields distinct; guest diagnostics keep status +meminfo, RRD, guest-agent free/cache/reclaimable/shmem, and linked-agent fields +distinct. Diagnostics must not relabel RRD evidence as meminfo or turn an +unknown usage into zero merely because total capacity is known. PVE and PBS node diagnostics must also classify connection failures into machine-readable `errorKind` values with operator guidance in `troubleshooting`. Missing stored credentials, Proxmox auth rejection, TLS diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 462d3492a..5383bfb3c 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -3486,6 +3486,13 @@ Standalone, TrueNAS, and vSphere platform tables and their table-model helpers must compose those helpers instead of declaring local `metricFallback` / `finiteMetric` helpers or inlining centered muted dash fallback markup in metric cells. +Canonical Linux memory usage-unavailable is a first-class metric fallback, +not a numeric zero. Shared workload bars, platform tables, drawers, and live +history labels must render `N/A` (while retaining known capacity where useful) +and must not synthesize a zero-width used segment or a healthy-looking `0%`. +When unified-resource metadata carries an unavailable raw Proxmox, agent, or +Docker memory facet alongside a trusted metric from another source, the +trusted merged metric wins; the raw facet remains diagnostic evidence only. Platform load-failure states are registry-backed as well. `PlatformErrorState` owns the repeated table-card error shell, warning icon, and Refresh action for platform page and table load failures; platform diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index dd0364ba3..3ce044121 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1619,12 +1619,43 @@ when Proxmox reports a saturated or materially inconsistent used figure, so Windows and ballooned guests do not get pinned to false 100% usage samples. That same guest-memory boundary also owns fallback order and cache scoping for Proxmox VMs when `MemInfo` is absent. Monitoring must try instance-scoped RRD -`memavailable`, then guest-agent `/proc/meminfo` via the shared Proxmox -client, and only then linked host-agent memory as the final fallback. Both RRD -and guest-agent fallback caches must key on `(instance, node, vmid)` instead -of raw `node/vmid`, so separate Proxmox instances cannot leak stale or foreign -memory evidence into each other just because they reuse the same node name and -VMID. +`memavailable`, RRD `memused`, guest-agent `/proc/meminfo` via the shared +Proxmox client, and only then linked host-agent memory. Saturated Linux VM +status is the narrow exception: a queryable guest agent's cache-aware +`/proc/meminfo` evidence is tried before RRD because the saturated status is +the known Proxmox failure mode. Both RRD and guest-agent fallback caches must +key on `(instance, node, vmid)` instead of raw `node/vmid`, so separate +Proxmox instances cannot leak stale or foreign memory evidence into each other +just because they reuse the same node name and VMID. +Linux memory availability must never be inferred from `MemTotal-MemFree`. +Nodes and guests prefer a valid explicit `MemAvailable`/`available` field, +then a complete reclaimable-component estimate, then valid RRD availability or +used evidence. The conservative old-kernel guest-agent estimate is +`MemFree + Buffers + Cached + SReclaimable - Shmem`; it is valid without swap +but not from truncated or total/free-only meminfo. A material `total-used` +gap may remain a lower-trust estimate only when it supplies independent +evidence that the reported used value already excludes cache. Invalid, +overflowed, non-finite, over-total, or conflicting candidates must be rejected +before the next source is considered; an explicitly present zero RRD used or +zero available value remains a valid idle or full-pressure sample rather than +being mistaken for an absent field. +Node RRD fallback caches must key on `(instance, node)`, just as guest RRD and +guest-agent caches key on `(instance, node, vmid)`, so identically named nodes +in different Proxmox instances cannot exchange memory evidence. +Running LXC cluster-resource memory is cache-inclusive and therefore cannot be +used as live usage when both RRD `memavailable` and `memused` are absent. +Unified Linux and Docker agent ingest likewise must not repair a missing used +value from total minus free alone; it may use an explicit used/percentage or +complete free-plus-cache evidence. In every collector, known capacity with no +cache-aware usage is represented by `models.Memory.UsageUnavailable`, the +canonical memory source `unavailable`, and fallback reason +`cache-aware-memory-unavailable`. A recent trusted node or guest snapshot may +be carried across a transient reconnect under the existing bounded +`previous-snapshot` rule; otherwise the unknown state must remain honest. +Unknown memory samples must not append zeroes to in-memory or persistent +history, project a canonical unified-resource memory metric, start or clear a +threshold alert, or render as 0% in product surfaces. Existing active alerts +remain fail-safe until a later trusted sample crosses the clear threshold. That same guest-memory boundary also owns stabilization when Proxmox falls back to low-trust VM full-usage readings. The shared VM polling paths must use the previous guest diagnostic snapshot, not the resource model, to decide when diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 4d2faa0cf..d92922172 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -1574,7 +1574,12 @@ resize-observer plus tooltip lifecycle live in `frontend-modern/src/components/Workloads/useStackedMemoryBarState.ts`. Future memory-bar runtime changes must extend through those owners instead of reintroducing mixed resize state, balloon branching, and tooltip shaping into -the shell. +the shell. The model must also short-circuit explicit usage-unavailable input: +it may retain total capacity for explanatory copy, but it emits no numeric +memory segments, percentage, or fit calculation that could make unknown usage +look like zero. Platform tables and resource-detail mappers consume the same +canonical unavailable state instead of recomputing `total - free` in render +hot paths. The dashboard metric bar now follows that same pattern: the shell stays in `frontend-modern/src/components/Workloads/MetricBar.tsx`, while width, show-label, sublabel-fit, and threshold-color derivation live in diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 635621a43..8d5119c60 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -1547,6 +1547,14 @@ recovery scope, or a storage/recovery-owned secret source. contract. `internal/unifiedresources/types.go` carries the reclaimable page-cache split (`AgentMemoryMeta.cache`, holding used + cache + free within the reported total) as host RAM description for machine surfaces. + It also carries `AgentMemoryMeta.usageUnavailable`, and the parallel + Proxmox and Docker raw memory facets, so a collector can preserve known + capacity while declining to invent Linux usage from `total - MemFree`. + A newer unavailable report clears a stale canonical memory metric only + when both observations come from the same source; a trusted linked source + remains eligible to supply the merged metric. Storage and recovery + consumers must preserve that distinction and must not reinterpret unknown + host RAM usage as zero, full pressure, or a storage-health signal. Storage and recovery must not reinterpret that reclaimable RAM figure as disk cache, ZFS ARC sizing, storage-tier health, or capacity-planning evidence; disk and pool truth stays on the canonical storage and diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index ba2f88861..66a5d498e 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -2255,6 +2255,17 @@ parallel payload, mirroring the `proxmox.memoryCache` transport nodes and guests use. The field is additive and omitted when an agent does not report it; consumers must treat missing cache as zero rather than inferring it from free space. +`AgentMemoryMeta.usageUnavailable`, `DockerData.memory.usageUnavailable`, and +`ProxmoxData.memory.usageUnavailable` carry the separate case where capacity is +known but cache-aware live usage is not. Adapters must omit +`ResourceMetrics.memory` for that source rather than +publishing a synthetic 0% value. Registry and presentation merges must retain +a trusted memory metric from another source when the current source is +unavailable, clear an older metric from the same unavailable source, and keep +the raw source facet available for diagnostics and capacity display. Frontend +adapters apply the unavailable marker only when no trusted merged metric +exists, so cross-source recovery cannot be masked by an unknown Proxmox or +agent facet. Frontend resource identity presenters may append a runtime version to a displayed system badge only when that version is sourced from the same canonical platform or host-profile identity, such as PVE `ResourceProxmoxMeta.pveVersion` diff --git a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailMappers.branchcov.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailMappers.branchcov.test.ts index 76213a816..85c800292 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailMappers.branchcov.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailMappers.branchcov.test.ts @@ -60,6 +60,38 @@ describe('buildMemory', () => { }); }); + it('preserves explicit unavailable memory through agent detail mapping', () => { + const fallback: Partial = { + total: 200, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }; + + expect(buildMemory(undefined, fallback)).toEqual({ + total: 200, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }); + }); + + it('lets a canonical cross-source metric override an unavailable fallback', () => { + const fallback: Partial = { + total: 200, + usageUnavailable: true, + }; + + expect(buildMemory({ current: 0.25, total: 200, used: 50 }, fallback)).toEqual({ + total: 200, + used: 50, + free: 150, + usage: 0.25, + }); + }); + it('computes usage from total>0 and ignores fallback.usage', () => { const fallback: Partial = { usage: 0.99 }; @@ -273,6 +305,28 @@ describe('toNodeFromProxmox', () => { }); }); + it('preserves unavailable Proxmox capacity when no canonical metric exists', () => { + const node = toNodeFromProxmox( + baseProxmoxResource({ + memory: undefined, + platformData: { + proxmox: { + nodeName: 'pve-node-1', + memory: { total: 8192, usageUnavailable: true }, + }, + }, + }), + ); + + expect(node?.memory).toEqual({ + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }); + }); + it('formats lastSeen as an ISO string of the finite timestamp', () => { const node = toNodeFromProxmox(baseProxmoxResource({ lastSeen: 1_700_000_000_000 })); expect(node?.lastSeen).toBe(new Date(1_700_000_000_000).toISOString()); diff --git a/frontend-modern/src/components/Infrastructure/resourceDetailMappers.ts b/frontend-modern/src/components/Infrastructure/resourceDetailMappers.ts index b8d5a1364..7c2ed64dd 100644 --- a/frontend-modern/src/components/Infrastructure/resourceDetailMappers.ts +++ b/frontend-modern/src/components/Infrastructure/resourceDetailMappers.ts @@ -28,6 +28,7 @@ export type ProxmoxPlatformData = { nodeName?: string; clusterName?: string; vmid?: number; + memory?: Partial; pveVersion?: string; kernelVersion?: string; uptime?: number; @@ -189,15 +190,17 @@ const getPreferredHostLabel = (resource: Resource): string => resource.id; export const buildMemory = (metric?: ResourceMetric, fallback?: Partial): Memory => { + const usageUnavailable = metric == null && fallback?.usageUnavailable === true; const total = metric?.total ?? fallback?.total ?? 0; const used = metric?.used ?? fallback?.used ?? 0; - const free = metric?.free ?? fallback?.free ?? Math.max(total - used, 0); - const usage = total > 0 ? used / total : (fallback?.usage ?? 0); + const free = metric?.free ?? fallback?.free ?? (usageUnavailable ? 0 : Math.max(total - used, 0)); + const usage = usageUnavailable ? 0 : total > 0 ? used / total : (fallback?.usage ?? 0); return { total, used, free, usage, + ...(usageUnavailable ? { usageUnavailable: true } : {}), }; }; @@ -241,7 +244,7 @@ export const toNodeFromProxmox = (resource: Resource): Node | null => { const proxmox = platformData?.proxmox; if (!proxmox) return null; - const memory = buildMemory(resource.memory); + const memory = buildMemory(resource.memory, proxmox.memory); const disk = buildDisk(resource.disk); const lastSeen = Number.isFinite(resource.lastSeen) ? new Date(resource.lastSeen).toISOString() diff --git a/frontend-modern/src/components/Workloads/GuestRow.tsx b/frontend-modern/src/components/Workloads/GuestRow.tsx index d24092e95..68b40f82c 100644 --- a/frontend-modern/src/components/Workloads/GuestRow.tsx +++ b/frontend-modern/src/components/Workloads/GuestRow.tsx @@ -368,6 +368,7 @@ export function GuestRow(props: GuestRowProps) { {renderMetricSparkline( 'memory', - formatMetricPercent( - usagePercent( - props.guest.memory?.used, - props.guest.memory?.total, - props.guest.memory?.usage, - ), - ), + props.guest.memory?.usageUnavailable + ? 'N/A' + : formatMetricPercent( + usagePercent( + props.guest.memory?.used, + props.guest.memory?.total, + props.guest.memory?.usage, + ), + ), `${props.guest.name} memory history`, '%', 'inline', diff --git a/frontend-modern/src/components/Workloads/NodeDrawerOverview.tsx b/frontend-modern/src/components/Workloads/NodeDrawerOverview.tsx index dd0e6edda..b297bed74 100644 --- a/frontend-modern/src/components/Workloads/NodeDrawerOverview.tsx +++ b/frontend-modern/src/components/Workloads/NodeDrawerOverview.tsx @@ -250,9 +250,12 @@ export function NodeDrawerOverview(props: NodeDrawerOverviewProps) { const memoryRows = (): NodeOverviewRow[] => [ { label: 'Usage', - value: `${getUsedPercent(props.node.memory?.used, props.node.memory?.total)} · ${formatBytes( - props.node.memory?.used || 0, - )}`, + value: + props.node.memory?.usageUnavailable === true + ? 'Unavailable' + : `${getUsedPercent(props.node.memory?.used, props.node.memory?.total)} · ${formatBytes( + props.node.memory?.used || 0, + )}`, }, { label: 'Total', value: formatBytes(props.node.memory?.total || 0) }, ...(props.node.memory?.cache @@ -263,7 +266,9 @@ export function NodeDrawerOverview(props: NodeDrawerOverviewProps) { } satisfies NodeOverviewRow, ] : []), - { label: 'Free', value: formatBytes(props.node.memory?.free || 0) }, + ...(props.node.memory?.usageUnavailable === true + ? [] + : [{ label: 'Free', value: formatBytes(props.node.memory?.free || 0) }]), ...(props.node.memory?.swapTotal ? [ { diff --git a/frontend-modern/src/components/Workloads/StackedMemoryBar.test.tsx b/frontend-modern/src/components/Workloads/StackedMemoryBar.test.tsx index cd343702a..46b56ccb3 100644 --- a/frontend-modern/src/components/Workloads/StackedMemoryBar.test.tsx +++ b/frontend-modern/src/components/Workloads/StackedMemoryBar.test.tsx @@ -44,6 +44,15 @@ describe('StackedMemoryBar', () => { expect(screen.getByText('50%')).toBeInTheDocument(); }); + it('renders N/A with no utilization segment when usage is unavailable', () => { + const { container } = render(() => ( + + )); + expect(screen.getByText('N/A')).toBeInTheDocument(); + expect(screen.queryByText('0%')).not.toBeInTheDocument(); + expect(getSegments(container)).toHaveLength(0); + }); + it('renders 0% when both used and total are 0', () => { render(() => ); expect(screen.getByText('0%')).toBeInTheDocument(); diff --git a/frontend-modern/src/components/Workloads/StackedMemoryBar.tsx b/frontend-modern/src/components/Workloads/StackedMemoryBar.tsx index 0d72eeabb..97331c8d8 100644 --- a/frontend-modern/src/components/Workloads/StackedMemoryBar.tsx +++ b/frontend-modern/src/components/Workloads/StackedMemoryBar.tsx @@ -72,7 +72,12 @@ export function StackedMemoryBar(props: StackedMemoryBarProps) { - + N/A} + > + + diff --git a/frontend-modern/src/components/Workloads/WorkloadPanel.tsx b/frontend-modern/src/components/Workloads/WorkloadPanel.tsx index d7202b6e7..f8cccb2a2 100644 --- a/frontend-modern/src/components/Workloads/WorkloadPanel.tsx +++ b/frontend-modern/src/components/Workloads/WorkloadPanel.tsx @@ -237,7 +237,9 @@ export function WorkloadPanel(props: WorkloadPanelProps) { return renderGroupNodeSparkline( node, 'memory', - formatMetricPercent(getUsedPercent(node.memory?.used, node.memory?.total)), + node.memory?.usageUnavailable + ? 'N/A' + : formatMetricPercent(getUsedPercent(node.memory?.used, node.memory?.total)), `${node.name} memory history`, '%', 'inline', @@ -249,6 +251,7 @@ export function WorkloadPanel(props: WorkloadPanelProps) { { expect(result.disk).toBeUndefined(); }); + it('does not synthesize a history point from unavailable live memory', () => { + const result = getGuestDrawerHistoryFallbackMetrics( + makeGuest({ + memory: { + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }, + }), + ); + expect(result.memory).toBeUndefined(); + }); + it('drops a non-finite network value via the finite() guard', () => { const result = getGuestDrawerHistoryFallbackMetrics( makeGuest({ networkIn: Number.POSITIVE_INFINITY }), @@ -452,6 +467,25 @@ describe('guestDrawerModel (branch coverage)', () => { ).toEqual([]); }); + it('shows unavailable usage and known capacity without showing zero usage', () => { + expect( + getGuestDrawerMemoryRows( + makeGuest({ + memory: { + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }, + }), + ), + ).toEqual([ + { label: 'Usage', value: 'Unavailable' }, + { label: 'Total', value: '8.00 KB' }, + ]); + }); + it('emits only Usage and Total when total>0 and no cache/free/balloon/swap', () => { expect( getGuestDrawerMemoryRows( diff --git a/frontend-modern/src/components/Workloads/__tests__/stackedMemoryBarModel.test.ts b/frontend-modern/src/components/Workloads/__tests__/stackedMemoryBarModel.test.ts index cbe9e8aeb..a8d21450d 100644 --- a/frontend-modern/src/components/Workloads/__tests__/stackedMemoryBarModel.test.ts +++ b/frontend-modern/src/components/Workloads/__tests__/stackedMemoryBarModel.test.ts @@ -5,6 +5,32 @@ import { buildStackedMemoryBarPresentation } from '../stackedMemoryBarModel'; const GiB = 1024 ** 3; describe('buildStackedMemoryBarPresentation', () => { + it('renders unavailable usage without inventing a zero-percent segment', () => { + const presentation = buildStackedMemoryBarPresentation( + { used: 0, total: 8 * GiB, unavailable: true }, + 400, + ); + + expect(presentation.unavailable).toBe(true); + expect(presentation.segments).toEqual([]); + expect(presentation.displaySublabel).toBe(''); + expect(presentation.showSublabel).toBe(false); + expect(presentation.tooltipRows).toEqual([ + { + borderTop: false, + label: 'Usage', + labelClass: 'text-slate-400', + value: 'Unavailable', + }, + { + borderTop: true, + label: 'Total', + labelClass: 'text-slate-400', + value: '8.00 GB', + }, + ]); + }); + it('renders the used | reclaimable cache split with a source-neutral reconciliation row', () => { const presentation = buildStackedMemoryBarPresentation( { used: 4 * GiB, total: 16 * GiB, cache: 6 * GiB }, diff --git a/frontend-modern/src/components/Workloads/guestDrawerModel.ts b/frontend-modern/src/components/Workloads/guestDrawerModel.ts index 0d6b626a8..7ce8b5c1f 100644 --- a/frontend-modern/src/components/Workloads/guestDrawerModel.ts +++ b/frontend-modern/src/components/Workloads/guestDrawerModel.ts @@ -72,7 +72,7 @@ export const getGuestDrawerHistoryFallbackMetrics = ( guest: Guest, ): Record => { const cpuPercent = getWorkloadCPUPercent(guest.cpu); - const memUsage = guest.memory?.usage; + const memUsage = guest.memory?.usageUnavailable ? undefined : guest.memory?.usage; const diskUsage = guest.disk?.usage; const finite = (value: number | undefined): number | undefined => typeof value === 'number' && Number.isFinite(value) ? value : undefined; @@ -302,7 +302,12 @@ export const getGuestDrawerMemoryRows = (guest: Guest): GuestDrawerMemoryRow[] = const used = memory.used ?? 0; const cache = memory.cache ?? 0; - if (total > 0) { + if (memory.usageUnavailable) { + rows.push({ label: 'Usage', value: 'Unavailable' }); + if (total > 0) { + rows.push({ label: 'Total', value: formatBytes(total) }); + } + } else if (total > 0) { rows.push({ label: 'Usage', value: `${formatPercent((used / total) * 100)} · ${formatBytes(used)}`, diff --git a/frontend-modern/src/components/Workloads/stackedMemoryBarModel.ts b/frontend-modern/src/components/Workloads/stackedMemoryBarModel.ts index 06e197011..0d01326bc 100644 --- a/frontend-modern/src/components/Workloads/stackedMemoryBarModel.ts +++ b/frontend-modern/src/components/Workloads/stackedMemoryBarModel.ts @@ -12,6 +12,7 @@ import type { MetricDisplayThresholds, MetricSeverity } from '@/utils/metricThre export interface StackedMemoryBarProps { used: number; total: number; + unavailable?: boolean; percentOnly?: number; /** Reclaimable buff/cache (available - truly free); used + cache + free ≈ total. */ cache?: number; @@ -51,6 +52,7 @@ export interface StackedMemoryBarPresentation { swapBarPercent: number; tooltipRows: StackedMemoryTooltipRow[]; tooltipTitle: string; + unavailable: boolean; } // Tooltip legend for the used segment tracks the same severity that colors @@ -79,6 +81,7 @@ function getEffectiveCache(props: StackedMemoryBarProps): number { } function getUtilizationPercent(props: StackedMemoryBarProps): number { + if (props.unavailable) return 0; if (props.total > 0) { return (props.used / props.total) * 100; } @@ -92,6 +95,9 @@ function getSegments( props: StackedMemoryBarProps, utilizationPercent: number, ): StackedMemorySegment[] { + if (props.unavailable) { + return []; + } if (props.total <= 0) { if (utilizationPercent <= 0) { return []; @@ -161,7 +167,22 @@ function getTooltipRows( const hasActiveBallooning = props.total > 0 && balloon > 0 && balloon < props.total; const hasSwap = (props.swapTotal || 0) > 0; - if (props.total > 0) { + if (props.unavailable) { + rows.push({ + borderTop: false, + label: 'Usage', + labelClass: 'text-slate-400', + value: 'Unavailable', + }); + if (props.total > 0) { + rows.push({ + borderTop: true, + label: 'Total', + labelClass: 'text-slate-400', + value: formatBytes(props.total), + }); + } + } else if (props.total > 0) { const usedPercent = (props.used / props.total) * 100; rows.push({ borderTop: false, @@ -236,7 +257,9 @@ export function buildStackedMemoryBarPresentation( const utilizationPercent = getUtilizationPercent(props); const displayLabel = formatPercent(utilizationPercent); const displaySublabel = - props.total > 0 ? `${formatBytes(props.used)}/${formatBytes(props.total)}` : ''; + !props.unavailable && props.total > 0 + ? `${formatBytes(props.used)}/${formatBytes(props.total)}` + : ''; const showSublabel = displaySublabel.length > 0 && containerWidth >= estimateTextWidth(`${displayLabel} (${displaySublabel})`); @@ -259,5 +282,6 @@ export function buildStackedMemoryBarPresentation( : 0, tooltipRows: getTooltipRows(props, displayLabel), tooltipTitle: 'Memory Composition', + unavailable: props.unavailable === true, }; } diff --git a/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx b/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx index a0ee6d657..7417da08f 100644 --- a/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx +++ b/frontend-modern/src/features/docker/DockerHostDrawerOverview.tsx @@ -65,6 +65,12 @@ const getNumericField = (value: unknown, field: string): number | undefined => { return typeof fieldValue === 'number' ? fieldValue : undefined; }; +const getBooleanField = (value: unknown, field: string): boolean | undefined => { + if (!value || typeof value !== 'object') return undefined; + const fieldValue = (value as Record)[field]; + return typeof fieldValue === 'boolean' ? fieldValue : undefined; +}; + const formatLastSeenRow = (value: string | number | null | undefined): DockerOverviewRow | null => { if (value == null) return null; const parsed = typeof value === 'number' ? new Date(value) : new Date(String(value)); @@ -142,7 +148,7 @@ export function DockerHostDrawerOverview(props: DockerHostDrawerOverviewProps) { return 0; }; - const memorySource = () => props.host.memory ?? agent()?.memory; + const memorySource = () => props.host.memory ?? agent()?.memory ?? docker()?.memory; const diskSource = () => props.host.disk; const systemRows = (): DockerOverviewRow[] => [ @@ -261,7 +267,12 @@ export function DockerHostDrawerOverview(props: DockerHostDrawerOverviewProps) { const memory = memorySource(); if (!memory) return []; const rows: DockerOverviewRow[] = []; - if (typeof memory.total === 'number' && memory.total > 0) { + if (getBooleanField(memory, 'usageUnavailable') === true) { + rows.push({ label: 'Usage', value: 'Unavailable' }); + if (typeof memory.total === 'number' && memory.total > 0) { + rows.push({ label: 'Total', value: formatBytes(memory.total) }); + } + } else if (typeof memory.total === 'number' && memory.total > 0) { rows.push({ label: 'Usage', value: `${getUsedPercent(memory.used, memory.total)} · ${formatBytes(memory.used || 0)}`, diff --git a/frontend-modern/src/features/docker/DockerHostsTable.tsx b/frontend-modern/src/features/docker/DockerHostsTable.tsx index 5d3f3bed7..320c1dcda 100644 --- a/frontend-modern/src/features/docker/DockerHostsTable.tsx +++ b/frontend-modern/src/features/docker/DockerHostsTable.tsx @@ -53,21 +53,28 @@ const percentFromMetric = (metric: Resource['cpu'] | undefined): number | undefi const memoryTotalFor = (host: Resource): number => getPlatformTableFiniteMetric(host.memory?.total) ?? getPlatformTableFiniteMetric(host.agent?.memory?.total) ?? + getPlatformTableFiniteMetric(host.docker?.memory?.total) ?? 0; const memoryUsedFor = (host: Resource): number => getPlatformTableFiniteMetric(host.memory?.used) ?? getPlatformTableFiniteMetric(host.agent?.memory?.used) ?? + getPlatformTableFiniteMetric(host.docker?.memory?.used) ?? 0; const memoryPercentOnlyFor = (host: Resource): number | undefined => { if (memoryTotalFor(host) > 0) return undefined; return ( getPlatformTableFiniteMetric(host.memory?.current) ?? - getPlatformTableFiniteMetric(host.agent?.memory?.usage) + getPlatformTableFiniteMetric(host.agent?.memory?.usage) ?? + getPlatformTableFiniteMetric(host.docker?.memory?.usage) ); }; +const memoryUnavailableFor = (host: Resource): boolean => + !host.memory && + (host.agent?.memory?.usageUnavailable === true || host.docker?.memory?.usageUnavailable === true); + // Host telemetry the Docker agent reports beyond the typed Resource docker // block. One cast site shared by the row renderer and the sort accessor. type DockerHostDockerMeta = NonNullable & { @@ -124,6 +131,7 @@ const getDockerHostSortValue = (host: Resource, key: DockerHostSortKey): Platfor case 'cpu': return percentFromMetric(host.cpu) ?? null; case 'memory': { + if (memoryUnavailableFor(host)) return null; const total = memoryTotalFor(host); if (total > 0) return (memoryUsedFor(host) / total) * 100; return memoryPercentOnlyFor(host) ?? null; @@ -452,6 +460,7 @@ export const DockerHostsTable: Component<{ diff --git a/frontend-modern/src/features/docker/__tests__/DockerHostsTable.test.tsx b/frontend-modern/src/features/docker/__tests__/DockerHostsTable.test.tsx index 179c98a91..4875f34bb 100644 --- a/frontend-modern/src/features/docker/__tests__/DockerHostsTable.test.tsx +++ b/frontend-modern/src/features/docker/__tests__/DockerHostsTable.test.tsx @@ -26,11 +26,17 @@ vi.mock('@/stores/alertsActivation', () => ({ })); vi.mock('@/components/Workloads/StackedMemoryBar', () => ({ - StackedMemoryBar: (props: { used: number; total: number; percentOnly?: number }) => ( + StackedMemoryBar: (props: { + used: number; + total: number; + unavailable?: boolean; + percentOnly?: number; + }) => (
), @@ -356,4 +362,35 @@ describe('DockerHostsTable', () => { expect(screen.getByTestId('stacked-disk-bar')).toHaveAttribute('data-disks', '0'); expect(screen.getByTestId('stacked-disk-bar')).toHaveAttribute('data-aggregate-usage', '71'); }); + + it('renders unavailable Docker memory honestly while retaining known capacity', () => { + render(() => ( + } + emptyTitle="No Docker hosts" + emptyDescription="No hosts" + showToolbar={false} + /> + )); + + expect(screen.getByTestId('stacked-memory-bar')).toHaveAttribute('data-total', '8000'); + expect(screen.getByTestId('stacked-memory-bar')).toHaveAttribute('data-unavailable', 'true'); + + fireEvent.click(screen.getByText('docker-01').closest('tr')!); + const drawer = screen.getByTestId('docker-host-drawer'); + expect(within(drawer).getByText('Unavailable')).toBeInTheDocument(); + expect(within(drawer).getByText('7.81 KB')).toBeInTheDocument(); + }); }); diff --git a/frontend-modern/src/features/proxmox/ProxmoxNodesTable.tsx b/frontend-modern/src/features/proxmox/ProxmoxNodesTable.tsx index 273cce142..6f5f2f95b 100644 --- a/frontend-modern/src/features/proxmox/ProxmoxNodesTable.tsx +++ b/frontend-modern/src/features/proxmox/ProxmoxNodesTable.tsx @@ -437,6 +437,7 @@ export const ProxmoxNodesTable: Component<{ diff --git a/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx b/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx index cabdf27f1..de5cd224f 100644 --- a/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx +++ b/frontend-modern/src/features/standalone/AgentsMachinesTable.tsx @@ -868,6 +868,9 @@ const memoryPercentOnlyFor = (machine: Resource): number | undefined => { ); }; +const memoryUnavailableFor = (machine: Resource): boolean => + !machine.memory && machine.agent?.memory?.usageUnavailable === true; + const memoryBalloonFor = (machine: Resource): number | undefined => getPlatformTableFiniteMetric(machine.agent?.memory?.balloon); @@ -1656,6 +1659,7 @@ export const AgentsMachinesTable: Component<{ { it('returns undefined when no memory source is available', () => { expect(getAgentMachineMemoryPercent(resource({}))).toBeUndefined(); }); + + it('does not turn an unavailable raw agent capacity into zero percent', () => { + expect( + getAgentMachineMemoryPercent( + resource({ + agent: { + memory: { total: 16_000, used: 0, usageUnavailable: true }, + }, + }), + ), + ).toBeUndefined(); + }); + + it('keeps a trusted merged metric when the agent facet is unavailable', () => { + expect( + getAgentMachineMemoryPercent( + resource({ + memory: { current: 25, total: 16_000, used: 4_000 }, + agent: { + memory: { total: 16_000, used: 0, usageUnavailable: true }, + }, + }), + ), + ).toBe(25); + }); }); describe('timestampMillisFrom', () => { diff --git a/frontend-modern/src/features/standalone/agentMachineTableModel.ts b/frontend-modern/src/features/standalone/agentMachineTableModel.ts index f686c64b4..8e42d60cf 100644 --- a/frontend-modern/src/features/standalone/agentMachineTableModel.ts +++ b/frontend-modern/src/features/standalone/agentMachineTableModel.ts @@ -309,6 +309,9 @@ export const getAgentMachineCpuPercent = (machine: Resource): number | undefined getMetricPercent(machine.cpu); export const getAgentMachineMemoryPercent = (machine: Resource): number | undefined => { + if (!machine.memory && machine.agent?.memory?.usageUnavailable === true) { + return undefined; + } const total = getPlatformTableFiniteMetric(machine.memory?.total); const used = getPlatformTableFiniteMetric(machine.memory?.used); if (total && total > 0 && typeof used === 'number') { diff --git a/frontend-modern/src/types/api.ts b/frontend-modern/src/types/api.ts index 8e97a03f6..7971bc481 100644 --- a/frontend-modern/src/types/api.ts +++ b/frontend-modern/src/types/api.ts @@ -1087,6 +1087,8 @@ export interface Memory { free: number; /** Reclaimable buff/cache (available - truly free); used + cache + free ≈ total. */ cache?: number; + /** Total is known, but no cache-aware source can establish current usage. */ + usageUnavailable?: boolean; usage: number; balloon?: number; swapUsed?: number; diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index 4893cc7b7..ff9488be6 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -665,6 +665,8 @@ export interface ResourceProxmoxMeta { balloon?: number; /** Reclaimable buff/cache split out of the memory metric's free bytes. */ memoryCache?: number; + /** Raw capacity and explicit availability state when no canonical metric is projected. */ + memory?: Partial; pendingUpdates?: number; isOci?: boolean; osTemplate?: string; @@ -698,6 +700,7 @@ export interface ResourceDockerMeta { runtime?: string; runtimeVersion?: string; dockerVersion?: string; + memory?: Partial; os?: string; kernelVersion?: string; architecture?: string; diff --git a/frontend-modern/src/utils/__tests__/resourceStateAdapters.coverage2.test.ts b/frontend-modern/src/utils/__tests__/resourceStateAdapters.coverage2.test.ts index 5d38b1a3d..679c95f6a 100644 --- a/frontend-modern/src/utils/__tests__/resourceStateAdapters.coverage2.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceStateAdapters.coverage2.test.ts @@ -679,6 +679,54 @@ describe('buildMemory (via nodeFromResource)', () => { expect(node?.memory.balloon).toBe(30); }); + it('preserves explicit unavailable usage from the fallback memory contract', () => { + const node = nodeFromResource( + agentNode({ + memory: undefined, + proxmox: { + nodeName: 'n1', + memory: { + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }, + } as unknown as Resource['proxmox'], + }), + ); + expect(node?.memory).toMatchObject({ + total: 8192, + used: 0, + usage: 0, + usageUnavailable: true, + }); + }); + + it('lets a trusted merged metric override an unavailable Proxmox fallback', () => { + const node = nodeFromResource( + agentNode({ + memory: { current: 50, total: 8192, used: 4096, free: 4096 }, + proxmox: { + nodeName: 'n1', + memory: { + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }, + } as unknown as Resource['proxmox'], + }), + ); + expect(node?.memory).toMatchObject({ + total: 8192, + used: 4096, + usage: 50, + usageUnavailable: false, + }); + }); + it('prefers proxmoxMeta.memoryCache over fallback.cache', () => { const node = nodeFromResource( agentNode({ diff --git a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts index 5259f67da..3f017c4e5 100644 --- a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts @@ -1009,3 +1009,46 @@ describe('resourceStateAdapters nodeFromResource', () => { }); }); }); + +describe('resourceStateAdapters unavailable memory contract', () => { + const unavailableProxmoxMemory = { + nodeName: 'n1', + memory: { + total: 8192, + used: 0, + free: 0, + usage: 0, + usageUnavailable: true, + }, + } as unknown as Resource['proxmox']; + + it('preserves explicit unavailable usage when no trusted metric exists', () => { + const node = nodeFromResource({ + ...createNodeResource({}), + memory: undefined, + proxmox: unavailableProxmoxMemory, + }); + + expect(node?.memory).toMatchObject({ + total: 8192, + used: 0, + usage: 0, + usageUnavailable: true, + }); + }); + + it('lets a trusted merged metric override unavailable raw evidence', () => { + const node = nodeFromResource({ + ...createNodeResource({}), + memory: { current: 50, total: 8192, used: 4096, free: 4096 }, + proxmox: unavailableProxmoxMemory, + }); + + expect(node?.memory).toMatchObject({ + total: 8192, + used: 4096, + usage: 50, + usageUnavailable: false, + }); + }); +}); diff --git a/frontend-modern/src/utils/resourceStateAdapters.ts b/frontend-modern/src/utils/resourceStateAdapters.ts index 6f4ccc6ca..4e98adc2d 100644 --- a/frontend-modern/src/utils/resourceStateAdapters.ts +++ b/frontend-modern/src/utils/resourceStateAdapters.ts @@ -836,18 +836,24 @@ const buildMemory = ( proxmoxMeta?: Record, ): Memory => { const total = metric?.total ?? asNumber(fallback?.total) ?? 0; + const usageUnavailable = metric == null && asBoolean(fallback?.usageUnavailable) === true; const used = metric?.used ?? asNumber(fallback?.used) ?? 0; const cache = asNumber(proxmoxMeta?.memoryCache) ?? asNumber(fallback?.cache) ?? 0; // The metric ships no free bytes for PVE payloads; total-used is the // reclaimable-inclusive available, so carve the cache back out when known. - const free = metric?.free ?? asNumber(fallback?.free) ?? Math.max(total - used - cache, 0); + const free = + metric?.free ?? + asNumber(fallback?.free) ?? + (usageUnavailable ? 0 : Math.max(total - used - cache, 0)); const usage = - metric?.current ?? (total > 0 ? (used / total) * 100 : (asNumber(fallback?.usage) ?? 0)); + metric?.current ?? + (usageUnavailable ? 0 : total > 0 ? (used / total) * 100 : (asNumber(fallback?.usage) ?? 0)); return { total, used, free, usage, + usageUnavailable, cache: cache > 0 ? cache : undefined, swapUsed: asNumber(proxmoxMeta?.swapUsed) ?? asNumber(fallback?.swapUsed), swapTotal: asNumber(proxmoxMeta?.swapTotal) ?? asNumber(fallback?.swapTotal), diff --git a/internal/alerts/guest.go b/internal/alerts/guest.go index 66c056170..d8a1bcef2 100644 --- a/internal/alerts/guest.go +++ b/internal/alerts/guest.go @@ -261,6 +261,10 @@ func (m *Manager) CheckGuest(guest any, instanceName string) { if monitorOnly { evalOpts = &metricOptions{MonitorOnly: true} } + var memoryMetric *UnifiedResourceMetric + if !snapshot.MemoryUnavailable { + memoryMetric = &UnifiedResourceMetric{Percent: memUsage} + } m.evaluateUnifiedMetrics(&UnifiedResourceInput{ ID: guestID, Type: snapshot.resourceType(), @@ -268,7 +272,7 @@ func (m *Manager) CheckGuest(guest any, instanceName string) { Node: node, Instance: instanceName, CPU: &UnifiedResourceMetric{Percent: cpu}, - Memory: &UnifiedResourceMetric{Percent: memUsage}, + Memory: memoryMetric, Disk: &UnifiedResourceMetric{Percent: diskUsage}, DiskRead: &UnifiedResourceMetric{Value: float64(diskRead) / 1024 / 1024}, DiskWrite: &UnifiedResourceMetric{Value: float64(diskWrite) / 1024 / 1024}, diff --git a/internal/alerts/guest_snapshot.go b/internal/alerts/guest_snapshot.go index 1d3ea8378..5113c5f34 100644 --- a/internal/alerts/guest_snapshot.go +++ b/internal/alerts/guest_snapshot.go @@ -28,11 +28,14 @@ type guestSnapshot struct { CPUPercent float64 MemUsage float64 - DiskUsage float64 - DiskRead int64 - DiskWrite int64 - NetworkIn int64 - NetworkOut int64 + // MemoryUnavailable prevents a missing cache-aware sample from clearing a + // real alert or starting a false one. + MemoryUnavailable bool + DiskUsage float64 + DiskRead int64 + DiskWrite int64 + NetworkIn int64 + NetworkOut int64 Disks []models.Disk Tags []string @@ -93,47 +96,49 @@ func (g guestSnapshot) metrics() guestMetrics { func guestSnapshotFromVM(vm models.VM) guestSnapshot { return guestSnapshot{ - Kind: guestKindVM, - ID: vm.ID, - VMID: vm.VMID, - Name: vm.Name, - Node: vm.Node, - Instance: vm.Instance, - Status: vm.Status, - Lock: vm.Lock, - CPUPercent: unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), - MemUsage: vm.Memory.Usage, - DiskUsage: vm.Disk.Usage, - DiskRead: vm.DiskRead, - DiskWrite: vm.DiskWrite, - NetworkIn: vm.NetworkIn, - NetworkOut: vm.NetworkOut, - Disks: append([]models.Disk(nil), vm.Disks...), - Tags: append([]string(nil), vm.Tags...), - OnBoot: vm.OnBoot, + Kind: guestKindVM, + ID: vm.ID, + VMID: vm.VMID, + Name: vm.Name, + Node: vm.Node, + Instance: vm.Instance, + Status: vm.Status, + Lock: vm.Lock, + CPUPercent: unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), + MemUsage: vm.Memory.Usage, + MemoryUnavailable: vm.Memory.UsageUnavailable, + DiskUsage: vm.Disk.Usage, + DiskRead: vm.DiskRead, + DiskWrite: vm.DiskWrite, + NetworkIn: vm.NetworkIn, + NetworkOut: vm.NetworkOut, + Disks: append([]models.Disk(nil), vm.Disks...), + Tags: append([]string(nil), vm.Tags...), + OnBoot: vm.OnBoot, }.normalizeCollections() } func guestSnapshotFromContainer(container models.Container) guestSnapshot { return guestSnapshot{ - Kind: guestKindContainer, - ID: container.ID, - VMID: container.VMID, - Name: container.Name, - Node: container.Node, - Instance: container.Instance, - Status: container.Status, - Lock: container.Lock, - CPUPercent: unifiedresources.ProxmoxGuestCPUPercent(container.CPU), - MemUsage: container.Memory.Usage, - DiskUsage: container.Disk.Usage, - DiskRead: container.DiskRead, - DiskWrite: container.DiskWrite, - NetworkIn: container.NetworkIn, - NetworkOut: container.NetworkOut, - Disks: append([]models.Disk(nil), container.Disks...), - Tags: append([]string(nil), container.Tags...), - OnBoot: container.OnBoot, + Kind: guestKindContainer, + ID: container.ID, + VMID: container.VMID, + Name: container.Name, + Node: container.Node, + Instance: container.Instance, + Status: container.Status, + Lock: container.Lock, + CPUPercent: unifiedresources.ProxmoxGuestCPUPercent(container.CPU), + MemUsage: container.Memory.Usage, + MemoryUnavailable: container.Memory.UsageUnavailable, + DiskUsage: container.Disk.Usage, + DiskRead: container.DiskRead, + DiskWrite: container.DiskWrite, + NetworkIn: container.NetworkIn, + NetworkOut: container.NetworkOut, + Disks: append([]models.Disk(nil), container.Disks...), + Tags: append([]string(nil), container.Tags...), + OnBoot: container.OnBoot, }.normalizeCollections() } diff --git a/internal/alerts/host.go b/internal/alerts/host.go index e8b8d7a60..0541b6abc 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -307,7 +307,7 @@ func (m *Manager) CheckHost(host models.Host) { m.clearHostMetricAlerts(host.ID, "cpu") } - if thresholds.Memory != nil { + if thresholds.Memory != nil && host.Memory.HasKnownUsage() { memMetadata := cloneMetadata(baseMetadata) memMetadata["metric"] = "memory" memMetadata["memoryUsagePercent"] = host.Memory.Usage @@ -326,7 +326,7 @@ func (m *Manager) CheckHost(host models.Host) { } else { m.checkMetricWithCanonicalSpec(spec, resourceName, nodeName, instanceName, "agent", host.Memory.Usage, thresholds.Memory, &metricOptions{Metadata: memMetadata}) } - } else { + } else if thresholds.Memory == nil { m.clearHostMetricAlerts(host.ID, "memory") } diff --git a/internal/alerts/node.go b/internal/alerts/node.go index a5f4fcf44..4ee993441 100644 --- a/internal/alerts/node.go +++ b/internal/alerts/node.go @@ -100,6 +100,10 @@ func (m *Manager) CheckNode(node models.Node) { Str("node", node.Name). Msg("Skipping node metric alerts - host agent is monitoring this machine") } else { + var memoryMetric *UnifiedResourceMetric + if node.Memory.HasKnownUsage() { + memoryMetric = &UnifiedResourceMetric{Percent: node.Memory.Usage} + } m.evaluateUnifiedMetrics(&UnifiedResourceInput{ ID: node.ID, Type: "node", @@ -107,7 +111,7 @@ func (m *Manager) CheckNode(node models.Node) { Node: node.Name, Instance: node.Instance, CPU: &UnifiedResourceMetric{Percent: node.CPU * 100}, - Memory: &UnifiedResourceMetric{Percent: node.Memory.Usage}, + Memory: memoryMetric, Disk: &UnifiedResourceMetric{Percent: node.Disk.Usage}, }, thresholds, nil) diff --git a/internal/alerts/operational_contract_test.go b/internal/alerts/operational_contract_test.go index 229f15b1f..bed29d188 100644 --- a/internal/alerts/operational_contract_test.go +++ b/internal/alerts/operational_contract_test.go @@ -6,6 +6,7 @@ import ( "time" alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" + "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/operationaltrust" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) @@ -566,3 +567,105 @@ func TestEnsureOperationalContractKeepsExplicitUncertainStateAheadOfLegacyAcknow t.Fatalf("LastObservedAt = %s, want %s", got, observedAt) } } + +func TestMemoryUnavailablePreservesActiveAlertsUntilTrustedRecovery(t *testing.T) { + tests := []struct { + name string + setup func(*Manager) + check func(*Manager, models.Memory) + }{ + { + name: "guest", + setup: func(manager *Manager) { + manager.UpdateConfig(AlertConfig{ + Enabled: true, + GuestDefaults: ThresholdConfig{ + Memory: &HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + }) + }, + check: func(manager *Manager, memory models.Memory) { + manager.CheckGuest(models.VM{ + ID: "qemu/1501", + Name: "linux-guest", + Node: "pve-1", + Instance: "test", + Status: "running", + Memory: memory, + }, "test") + }, + }, + { + name: "node", + setup: func(manager *Manager) { + manager.UpdateConfig(AlertConfig{ + Enabled: true, + NodeDefaults: ThresholdConfig{ + Memory: &HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + }) + }, + check: func(manager *Manager, memory models.Memory) { + manager.CheckNode(models.Node{ + ID: "pve-1", + Name: "pve-1", + Instance: "test", + Status: "online", + Memory: memory, + }) + }, + }, + { + name: "agent", + setup: func(manager *Manager) { + manager.UpdateConfig(AlertConfig{ + Enabled: true, + AgentDefaults: ThresholdConfig{ + Memory: &HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + }) + }, + check: func(manager *Manager, memory models.Memory) { + manager.CheckHost(models.Host{ + ID: "agent-1501", + Hostname: "linux-host", + Status: "online", + Memory: memory, + }) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := newUnifiedEvalParityManager(t) + tt.setup(manager) + disableTestTimeThresholds(manager) + + tt.check(manager, models.Memory{ + Total: 8 << 30, + Used: 7 << 30, + Usage: 90, + }) + active := manager.GetActiveAlerts() + if len(active) != 1 || active[0].Type != "memory" { + t.Fatalf("active alerts after trusted high sample = %#v, want one memory alert", active) + } + + tt.check(manager, models.UnavailableMemory(8<<30)) + active = manager.GetActiveAlerts() + if len(active) != 1 || active[0].Type != "memory" || active[0].Value != 90 { + t.Fatalf("active alerts after unavailable sample = %#v, want prior memory alert", active) + } + + tt.check(manager, models.Memory{ + Total: 8 << 30, + Used: 5 << 30, + Usage: 70, + }) + if active = manager.GetActiveAlerts(); len(active) != 0 { + t.Fatalf("active alerts after trusted recovery = %#v, want cleared", active) + } + }) + } +} diff --git a/internal/alerts/unified_eval.go b/internal/alerts/unified_eval.go index ace503741..5fa07689f 100644 --- a/internal/alerts/unified_eval.go +++ b/internal/alerts/unified_eval.go @@ -1,6 +1,7 @@ package alerts import ( + "math" "strings" alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" @@ -255,6 +256,12 @@ func buildUnifiedMetricCandidates(input *UnifiedResourceInput, thresholds Thresh if metric == nil { return } + if math.IsNaN(value) || math.IsInf(value, 0) { + return + } + if isPercentageMetric(metricType) && (value < 0 || value > 100) { + return + } spec, err := buildCanonicalMetricSpec(input.ID, input.Name, resourceType, metricType, threshold) if err != nil { log.Warn(). diff --git a/internal/api/diagnostics_memory_test.go b/internal/api/diagnostics_memory_test.go index ad08b9c35..c7eb78a3e 100644 --- a/internal/api/diagnostics_memory_test.go +++ b/internal/api/diagnostics_memory_test.go @@ -33,6 +33,7 @@ func TestClassifyMemorySourceTrust(t *testing.T) { {source: "listing", want: "fallback"}, {source: "powered-off", want: "fallback"}, {source: "previous-snapshot", want: "fallback"}, + {source: "unavailable", want: "unavailable"}, {source: "", want: "fallback"}, } @@ -45,6 +46,54 @@ func TestClassifyMemorySourceTrust(t *testing.T) { } } +func TestBuildMemorySourceDiagnosticsReportsUnavailableCacheAwareEvidence(t *testing.T) { + base := time.Date(2026, 7, 23, 12, 0, 0, 0, time.UTC) + snapshots := monitoring.DiagnosticSnapshotSet{ + Nodes: []monitoring.NodeMemorySnapshot{{ + Instance: "pve-a", + Node: "node-1", + RetrievedAt: base, + MemorySource: "unavailable", + FallbackReason: "cache-aware-memory-unavailable", + Raw: monitoring.NodeMemoryRaw{ + Total: 8 << 30, + Used: 15 << 29, + Free: 1 << 29, + ProxmoxMemorySource: "cache-aware-unavailable", + }, + }}, + Guests: []monitoring.GuestMemorySnapshot{{ + Instance: "pve-a", + GuestType: "qemu", + Node: "node-1", + VMID: 1501, + RetrievedAt: base, + MemorySource: "unavailable", + FallbackReason: "cache-aware-memory-unavailable", + Raw: monitoring.VMMemoryRaw{ + MemInfoTotal: 8 << 30, + MemInfoFree: 1 << 29, + }, + }}, + } + + nodeStats, breakdown := buildMemorySourceDiagnostics(snapshots) + if len(nodeStats) != 1 || nodeStats[0].Source != "unavailable" || nodeStats[0].Trust != "unavailable" { + t.Fatalf("node memory source stats = %#v, want explicit unavailable source", nodeStats) + } + if len(breakdown) != 2 { + t.Fatalf("memory source breakdown = %#v, want node and guest entries", breakdown) + } + for _, entry := range breakdown { + if entry.Source != "unavailable" || entry.Trust != "unavailable" || !entry.Fallback { + t.Fatalf("unavailable breakdown entry = %#v", entry) + } + if len(entry.FallbackReasons) != 1 || entry.FallbackReasons[0] != "cache-aware-memory-unavailable" { + t.Fatalf("fallback reasons = %#v, want cache-aware-memory-unavailable", entry.FallbackReasons) + } + } +} + func TestBuildMemorySourceDiagnostics(t *testing.T) { base := time.Date(2026, 3, 14, 16, 30, 0, 0, time.UTC) snapshots := monitoring.DiagnosticSnapshotSet{ diff --git a/internal/models/memory_availability_test.go b/internal/models/memory_availability_test.go new file mode 100644 index 000000000..610a4b9d9 --- /dev/null +++ b/internal/models/memory_availability_test.go @@ -0,0 +1,51 @@ +package models + +import ( + "encoding/json" + "math" + "testing" +) + +func TestUnavailableMemorySurvivesJSONReload(t *testing.T) { + before := UnavailableMemory(8 << 30) + + payload, err := json.Marshal(before) + if err != nil { + t.Fatalf("json.Marshal() error = %v", err) + } + + var after Memory + if err := json.Unmarshal(payload, &after); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if !after.UsageUnavailable { + t.Fatalf("UsageUnavailable = false after reload; payload=%s", payload) + } + if after.Total != before.Total { + t.Fatalf("Total = %d, want %d", after.Total, before.Total) + } + if after.HasKnownUsage() { + t.Fatalf("HasKnownUsage() = true after unavailable reload: %+v", after) + } +} + +func TestMemoryHasKnownUsageAcceptsTrustedZeroUsage(t *testing.T) { + memory := Memory{Total: 8 << 30, Free: 8 << 30} + if !memory.HasKnownUsage() { + t.Fatalf("HasKnownUsage() = false for a valid zero-usage sample: %+v", memory) + } +} + +func TestMemoryHasKnownUsageRejectsNonFiniteAndContradictoryValues(t *testing.T) { + for name, memory := range map[string]Memory{ + "nan": {Total: 8 << 30, Used: 4 << 30, Free: 4 << 30, Usage: math.NaN()}, + "infinity": {Total: 8 << 30, Used: 4 << 30, Free: 4 << 30, Usage: math.Inf(1)}, + "free overflow": {Total: 8 << 30, Free: 9 << 30}, + } { + t.Run(name, func(t *testing.T) { + if memory.HasKnownUsage() { + t.Fatalf("HasKnownUsage() = true for invalid memory: %+v", memory) + } + }) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index a48be6c0d..c14bcd4d9 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -3,6 +3,7 @@ package models import ( "errors" "fmt" + "math" "net" "net/url" "sort" @@ -2803,11 +2804,43 @@ type Memory struct { Used int64 `json:"used"` Free int64 `json:"free"` // Reclaimable buff/cache (available - truly free); used + cache + free ≈ total. - Cache int64 `json:"cache,omitempty"` - Usage float64 `json:"usage"` - Balloon int64 `json:"balloon,omitempty"` - SwapUsed int64 `json:"swapUsed,omitempty"` - SwapTotal int64 `json:"swapTotal,omitempty"` + Cache int64 `json:"cache,omitempty"` + // UsageUnavailable is true when total capacity is known but no cache-aware + // source can establish current usage. Consumers must not interpret the + // zero-valued Used, Free, or Usage fields as a measurement in that state. + UsageUnavailable bool `json:"usageUnavailable,omitempty"` + Usage float64 `json:"usage"` + Balloon int64 `json:"balloon,omitempty"` + SwapUsed int64 `json:"swapUsed,omitempty"` + SwapTotal int64 `json:"swapTotal,omitempty"` +} + +// HasKnownUsage reports whether this memory value is safe for live +// projections, history, and threshold evaluation. +func (m Memory) HasKnownUsage() bool { + if m.UsageUnavailable || math.IsNaN(m.Usage) || math.IsInf(m.Usage, 0) || m.Usage < 0 || m.Usage > 100 { + return false + } + if m.Total == 0 { + return m.Usage > 0 + } + return m.Total > 0 && + m.Used >= 0 && + m.Used <= m.Total && + m.Free >= 0 && + m.Free <= m.Total +} + +// UnavailableMemory preserves a known capacity without inventing usage from +// total-free when cache-aware evidence is missing. +func UnavailableMemory(total int64) Memory { + if total < 0 { + total = 0 + } + return Memory{ + Total: total, + UsageUnavailable: true, + } } type GuestNetworkInterface struct { diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index a30317ab3..16c96ff12 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -866,14 +866,17 @@ func TestProxmoxGuestMemoryFallbackUsesInstanceScopedCachesAndAgentMeminfo(t *te "guest_memory_sources.go": { "func shouldPreferGuestAgentMemAvailable(status *proxmox.VMStatus, memTotal uint64) bool {", "func (m *Monitor) tryGuestAgentMemAvailable(", - "if memAvailable == 0 && shouldPreferGuestAgentMemAvailable(status, memTotal) {", - "if rrdAvailable, rrdErr := m.getVMRRDMetrics(ctx, client, instanceName, node, vmid); rrdErr == nil && rrdAvailable > 0 {", - "if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok {", - `memorySource = "guest-agent-meminfo"`, + "if !hasMemAvailable && shouldPreferGuestAgentMemAvailable(status, memTotal) {", + "if rrdMemory, rrdErr := m.getVMRRDMemory(ctx, client, instanceName, node, vmid); rrdErr == nil {", + "case rrdMemory.hasUsed && rrdMemory.used <= memTotal:", + "if agentAvailable, agentSource, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok {", + "memorySource = agentSource", "guestRaw.GuestAgentMemAvailable = agentAvailable", + `memorySource = "unavailable"`, }, "monitor.go": { "func (m *Monitor) getVMRRDMetrics(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (uint64, error) {", + "func (m *Monitor) getVMRRDMemory(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (rrdMemCacheEntry, error) {", "cacheKey := guestMemoryCacheKey(instanceName, node, vmid)", "vmAgentMemCache map[string]agentMemCacheEntry", }, @@ -1999,14 +2002,14 @@ func TestMockNativePollersDeferToCanonicalMockSampler(t *testing.T) { file: "monitor_polling_vm.go", snippets: []string{ "if !shouldSkipNativeMockStateMetricWrites() {", - `m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), vm.Memory.Usage, vm.Disk.Usage, -1, -1, -1, -1, now)`, + `m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), historyMemoryUsage(vm.Memory), vm.Disk.Usage, -1, -1, -1, -1, now)`, }, }, { file: "monitor_polling_containers.go", snippets: []string{ "if !shouldSkipNativeMockStateMetricWrites() {", - `m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), ct.Memory.Usage, ct.Disk.Usage, -1, -1, -1, -1, now)`, + `m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), historyMemoryUsage(ct.Memory), ct.Disk.Usage, -1, -1, -1, -1, now)`, }, }, { diff --git a/internal/monitoring/diagnostic_snapshots.go b/internal/monitoring/diagnostic_snapshots.go index 6524e5eca..0edc2588c 100644 --- a/internal/monitoring/diagnostic_snapshots.go +++ b/internal/monitoring/diagnostic_snapshots.go @@ -64,6 +64,15 @@ type VMMemoryRaw struct { MemInfoShared uint64 `json:"meminfoShared,omitempty"` MemInfoTotalMinusUsed uint64 `json:"meminfoTotalMinusUsed,omitempty"` GuestAgentMemAvailable uint64 `json:"guestAgentMemAvailable,omitempty"` + GuestAgentMemFree uint64 `json:"guestAgentMemFree,omitempty"` + GuestAgentMemBuffers uint64 `json:"guestAgentMemBuffers,omitempty"` + GuestAgentMemCached uint64 `json:"guestAgentMemCached,omitempty"` + GuestAgentSReclaimable uint64 `json:"guestAgentSReclaimable,omitempty"` + GuestAgentShmem uint64 `json:"guestAgentShmem,omitempty"` + GuestAgentDerived bool `json:"guestAgentDerived,omitempty"` + RRDMemAvailable uint64 `json:"rrdMemAvailable,omitempty"` + RRDMemUsed uint64 `json:"rrdMemUsed,omitempty"` + RRDMaxMem uint64 `json:"rrdMaxMem,omitempty"` HostAgentTotal uint64 `json:"hostAgentTotal,omitempty"` HostAgentUsed uint64 `json:"hostAgentUsed,omitempty"` Agent int `json:"agent,omitempty"` @@ -256,6 +265,15 @@ func (m *Monitor) logGuestMemorySource(instance, guestType, node string, vmid in if snapshot.Raw.GuestAgentMemAvailable > 0 { evt = evt.Uint64("guestAgentMemAvailable", snapshot.Raw.GuestAgentMemAvailable) } + if snapshot.Raw.GuestAgentDerived { + evt = evt.Bool("guestAgentDerived", true) + } + if snapshot.Raw.RRDMemAvailable > 0 { + evt = evt.Uint64("rrdMemAvailable", snapshot.Raw.RRDMemAvailable) + } + if snapshot.Raw.RRDMemUsed > 0 { + evt = evt.Uint64("rrdMemUsed", snapshot.Raw.RRDMemUsed) + } if snapshot.Raw.MemInfoBuffers > 0 { evt = evt.Uint64("memInfoBuffers", snapshot.Raw.MemInfoBuffers) } diff --git a/internal/monitoring/guest_memory_agent.go b/internal/monitoring/guest_memory_agent.go index c09217f39..8b29b2327 100644 --- a/internal/monitoring/guest_memory_agent.go +++ b/internal/monitoring/guest_memory_agent.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" "time" + + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) const ( @@ -17,6 +19,7 @@ const ( type agentMemCacheEntry struct { available uint64 + info proxmox.LinuxMemoryAvailability negative bool fetchedAt time.Time } @@ -25,17 +28,22 @@ type guestAgentMemAvailableClient interface { GetVMMemAvailableFromAgent(ctx context.Context, node string, vmid int) (uint64, error) } +type guestAgentMemoryAvailabilityClient interface { + GetVMMemoryAvailabilityFromAgent(ctx context.Context, node string, vmid int) (proxmox.LinuxMemoryAvailability, error) +} + func guestMemoryCacheKey(instanceName, node string, vmid int) string { return fmt.Sprintf("%s/%s/%d", instanceName, node, vmid) } func (m *Monitor) getVMAgentMemAvailable(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (uint64, error) { - memClient, ok := client.(guestAgentMemAvailableClient) - if !ok { - return 0, fmt.Errorf("guest agent meminfo fallback unsupported") - } + info, err := m.getVMAgentMemoryAvailability(ctx, client, instanceName, node, vmid) + return info.EffectiveAvailable, err +} + +func (m *Monitor) getVMAgentMemoryAvailability(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (proxmox.LinuxMemoryAvailability, error) { if node == "" || vmid <= 0 { - return 0, fmt.Errorf("invalid arguments for guest agent meminfo lookup") + return proxmox.LinuxMemoryAvailability{}, fmt.Errorf("invalid arguments for guest agent meminfo lookup") } cacheKey := guestMemoryCacheKey(instanceName, node, vmid) @@ -50,9 +58,16 @@ func (m *Monitor) getVMAgentMemAvailable(ctx context.Context, client PVEClientIn if now.Sub(entry.fetchedAt) < ttl { m.rrdCacheMu.RUnlock() if entry.negative { - return 0, fmt.Errorf("guest agent meminfo fallback unavailable") + return proxmox.LinuxMemoryAvailability{}, fmt.Errorf("guest agent meminfo fallback unavailable") } - return entry.available, nil + if entry.info.Source != "" { + return entry.info, nil + } + return proxmox.LinuxMemoryAvailability{ + Available: entry.available, + EffectiveAvailable: entry.available, + Source: "meminfo-available", + }, nil } } m.rrdCacheMu.RUnlock() @@ -60,22 +75,43 @@ func (m *Monitor) getVMAgentMemAvailable(ctx context.Context, client PVEClientIn requestCtx, cancel := context.WithTimeout(ctx, vmAgentMemRequestTTL) defer cancel() - available, err := memClient.GetVMMemAvailableFromAgent(requestCtx, node, vmid) + var info proxmox.LinuxMemoryAvailability + var err error + if memClient, ok := client.(guestAgentMemoryAvailabilityClient); ok { + info, err = memClient.GetVMMemoryAvailabilityFromAgent(requestCtx, node, vmid) + } else if memClient, ok := client.(guestAgentMemAvailableClient); ok { + var available uint64 + available, err = memClient.GetVMMemAvailableFromAgent(requestCtx, node, vmid) + if available > 0 { + info = proxmox.LinuxMemoryAvailability{ + Available: available, + EffectiveAvailable: available, + Source: "meminfo-available", + } + } + } else { + return proxmox.LinuxMemoryAvailability{}, fmt.Errorf("guest agent meminfo fallback unsupported") + } + m.rrdCacheMu.Lock() defer m.rrdCacheMu.Unlock() if m.vmAgentMemCache == nil { m.vmAgentMemCache = make(map[string]agentMemCacheEntry) } - if err != nil || available == 0 { + if err != nil || info.Source == "" { m.vmAgentMemCache[cacheKey] = agentMemCacheEntry{negative: true, fetchedAt: now} if err == nil { err = fmt.Errorf("guest agent meminfo fallback unavailable") } - return 0, err + return proxmox.LinuxMemoryAvailability{}, err } - m.vmAgentMemCache[cacheKey] = agentMemCacheEntry{available: available, fetchedAt: now} - return available, nil + m.vmAgentMemCache[cacheKey] = agentMemCacheEntry{ + available: info.EffectiveAvailable, + info: info, + fetchedAt: now, + } + return info, nil } func (m *Monitor) vmAgentMemNegativeCacheTTL(instanceName, node string, vmid int) time.Duration { diff --git a/internal/monitoring/guest_memory_agent_test.go b/internal/monitoring/guest_memory_agent_test.go index dda34de9f..53c3f2236 100644 --- a/internal/monitoring/guest_memory_agent_test.go +++ b/internal/monitoring/guest_memory_agent_test.go @@ -13,6 +13,7 @@ type guestMemoryAgentTestClient struct { *stubPVEClient vmRRDPoints []proxmox.GuestRRDPoint memAvailable uint64 + memInfo *proxmox.LinuxMemoryAvailability memErr error rrdCalls int memCalls int @@ -31,6 +32,24 @@ func (c *guestMemoryAgentTestClient) GetVMMemAvailableFromAgent(ctx context.Cont return c.memAvailable, nil } +func (c *guestMemoryAgentTestClient) GetVMMemoryAvailabilityFromAgent(ctx context.Context, node string, vmid int) (proxmox.LinuxMemoryAvailability, error) { + c.memCalls++ + if c.memErr != nil { + return proxmox.LinuxMemoryAvailability{}, c.memErr + } + if c.memInfo != nil { + return *c.memInfo, nil + } + if c.memAvailable == 0 { + return proxmox.LinuxMemoryAvailability{}, nil + } + return proxmox.LinuxMemoryAvailability{ + Available: c.memAvailable, + EffectiveAvailable: c.memAvailable, + Source: "meminfo-available", + }, nil +} + func TestGetVMRRDMetricsCacheKeyIncludesInstance(t *testing.T) { t.Parallel() @@ -65,6 +84,79 @@ func TestGetVMRRDMetricsCacheKeyIncludesInstance(t *testing.T) { } } +func TestResolveGuestStatusMemoryAcceptsExplicitZeroRRDUsed(t *testing.T) { + t.Parallel() + + const gib = uint64(1024 * 1024 * 1024) + mon := &Monitor{vmRRDMemCache: make(map[string]rrdMemCacheEntry)} + client := &guestMemoryAgentTestClient{ + stubPVEClient: &stubPVEClient{}, + vmRRDPoints: []proxmox.GuestRRDPoint{{ + MaxMem: floatPtr(float64(8 * gib)), + MemUsed: floatPtr(0), + }}, + } + + total, used, source := mon.resolveGuestStatusMemory( + context.Background(), + client, + "pve-a", + "idle-vm", + "node1", + 100, + "pve-a:node1:100", + &proxmox.VMStatus{MaxMem: 8 * gib, Mem: 8 * gib}, + nil, + 8*gib, + "", + &VMMemoryRaw{}, + ) + if total != 8*gib || used != 0 || source != "rrd-memused" { + t.Fatalf("resolved memory = total %d used %d source %q, want idle RRD sample", total, used, source) + } +} + +func TestResolveGuestStatusMemoryAcceptsExplicitZeroGuestAgentAvailable(t *testing.T) { + t.Parallel() + + const gib = uint64(1024 * 1024 * 1024) + mon := &Monitor{ + vmRRDMemCache: make(map[string]rrdMemCacheEntry), + vmAgentMemCache: make(map[string]agentMemCacheEntry), + } + client := &guestMemoryAgentTestClient{ + stubPVEClient: &stubPVEClient{}, + memInfo: &proxmox.LinuxMemoryAvailability{ + Total: 8 * gib, + Available: 0, + EffectiveAvailable: 0, + Source: "meminfo-available", + }, + } + + total, used, source := mon.resolveGuestStatusMemory( + context.Background(), + client, + "pve-a", + "pressured-vm", + "node1", + 101, + "pve-a:node1:101", + &proxmox.VMStatus{ + MaxMem: 8 * gib, + Mem: 8 * gib, + Agent: proxmox.VMAgentField{Value: 1}, + }, + nil, + 8*gib, + "", + &VMMemoryRaw{}, + ) + if total != 8*gib || used != 8*gib || source != "guest-agent-meminfo" { + t.Fatalf("resolved memory = total %d used %d source %q, want full-pressure guest-agent sample", total, used, source) + } +} + func TestGetVMAgentMemAvailableCachesResults(t *testing.T) { t.Parallel() diff --git a/internal/monitoring/guest_memory_sources.go b/internal/monitoring/guest_memory_sources.go index a89f31a8c..e5330f27c 100644 --- a/internal/monitoring/guest_memory_sources.go +++ b/internal/monitoring/guest_memory_sources.go @@ -44,9 +44,7 @@ func deriveGuestMemInfoAvailable(memInfo *proxmox.VMMemInfo, guestRaw *VMMemoryR componentAvailable += memInfo.Cached } } - if memInfo.Total > 0 && componentAvailable > memInfo.Total { - componentAvailable = memInfo.Total - } + componentsConflict := memInfo.Total > 0 && componentAvailable > memInfo.Total availableFromUsed := uint64(0) if memInfo.Total > 0 && memInfo.Used > 0 && memInfo.Total >= memInfo.Used { @@ -61,7 +59,7 @@ func deriveGuestMemInfoAvailable(memInfo *proxmox.VMMemInfo, guestRaw *VMMemoryR memInfo.Cached == 0 switch { - case memInfo.Available > 0: + case memInfo.Available > 0 && (memInfo.Total == 0 || memInfo.Available <= memInfo.Total): return memInfo.Available, "available-field" case memInfo.Free > 0 || memInfo.Buffers > 0 || memInfo.Cached > 0: if availableFromUsed > 0 && missingCacheMetrics { @@ -76,6 +74,9 @@ func deriveGuestMemInfoAvailable(memInfo *proxmox.VMMemInfo, guestRaw *VMMemoryR if missingCacheMetrics { return 0, "" } + if componentsConflict { + return 0, "" + } return componentAvailable, "derived-free-buffers-cached" default: if availableFromUsed > 0 && missingCacheMetrics { @@ -206,13 +207,27 @@ func (m *Monitor) tryGuestAgentMemAvailable( vmid int, memTotal uint64, guestRaw *VMMemoryRaw, -) (uint64, bool) { - agentAvailable, agentErr := m.getVMAgentMemAvailable(ctx, client, instanceName, node, vmid) - if agentErr != nil || agentAvailable == 0 { - return 0, false +) (uint64, string, bool) { + availability, agentErr := m.getVMAgentMemoryAvailability(ctx, client, instanceName, node, vmid) + if agentErr != nil || availability.Source == "" { + return 0, "", false } + agentAvailable := availability.EffectiveAvailable if guestRaw != nil { guestRaw.GuestAgentMemAvailable = agentAvailable + guestRaw.GuestAgentMemFree = availability.Free + guestRaw.GuestAgentMemBuffers = availability.Buffers + guestRaw.GuestAgentMemCached = availability.Cached + guestRaw.GuestAgentSReclaimable = availability.SReclaimable + guestRaw.GuestAgentShmem = availability.Shmem + guestRaw.GuestAgentDerived = availability.Source == "meminfo-derived" + } + if memTotal == 0 || agentAvailable > memTotal { + return 0, "", false + } + source := "guest-agent-meminfo" + if availability.Source == "meminfo-derived" { + source = "guest-agent-meminfo-derived" } log.Debug(). Str("vm", guestName). @@ -220,8 +235,9 @@ func (m *Monitor) tryGuestAgentMemAvailable( Int("vmid", vmid). Uint64("total", memTotal). Uint64("available", agentAvailable). + Str("source", source). Msg("QEMU memory: using guest agent /proc/meminfo fallback (excludes reclaimable cache)") - return agentAvailable, true + return agentAvailable, source, true } func (m *Monitor) resolveGuestStatusMemory( @@ -260,43 +276,67 @@ func (m *Monitor) resolveGuestStatusMemory( } memAvailable := uint64(0) + hasMemAvailable := false + derivedTotalMinusUsedAvailable := uint64(0) + selectedUsed := uint64(0) + hasSelectedUsed := false if status.MemInfo != nil { memAvailable, memorySource = deriveGuestMemInfoAvailable(status.MemInfo, guestRaw) - if memAvailable > 0 && memorySource == "derived-total-minus-used" { - log.Debug(). - Str("vm", guestName). - Str("node", node). - Int("vmid", vmid). - Uint64("total", memTotal). - Uint64("available", memAvailable). - Uint64("availableFromUsed", guestRaw.MemInfoTotalMinusUsed). - Msg("QEMU memory: deriving guest available from total-used gap when cache fields are missing") + hasMemAvailable = memorySource != "" + if memAvailable > memTotal { + memAvailable = 0 + hasMemAvailable = false + memorySource = "" + } + if memorySource == "derived-total-minus-used" { + derivedTotalMinusUsedAvailable = memAvailable + memAvailable = 0 + hasMemAvailable = false + memorySource = "" } } triedGuestAgentMemAvailable := false - if memAvailable == 0 && shouldPreferGuestAgentMemAvailable(status, memTotal) { + if !hasMemAvailable && shouldPreferGuestAgentMemAvailable(status, memTotal) { triedGuestAgentMemAvailable = true - if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { + if agentAvailable, agentSource, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { memAvailable = agentAvailable - memorySource = "guest-agent-meminfo" + hasMemAvailable = true + memorySource = agentSource } } - if memAvailable == 0 { - if rrdAvailable, rrdErr := m.getVMRRDMetrics(ctx, client, instanceName, node, vmid); rrdErr == nil && rrdAvailable > 0 { - memAvailable = rrdAvailable - memorySource = "rrd-memavailable" + if !hasMemAvailable { + if rrdMemory, rrdErr := m.getVMRRDMemory(ctx, client, instanceName, node, vmid); rrdErr == nil { if guestRaw != nil { - guestRaw.MemInfoAvailable = memAvailable + guestRaw.RRDMemAvailable = rrdMemory.available + guestRaw.RRDMemUsed = rrdMemory.used + guestRaw.RRDMaxMem = rrdMemory.total + } + switch { + case rrdMemory.hasAvail && rrdMemory.available <= memTotal: + memAvailable = rrdMemory.available + hasMemAvailable = true + memorySource = "rrd-memavailable" + log.Debug(). + Str("vm", guestName). + Str("node", node). + Int("vmid", vmid). + Uint64("total", memTotal). + Uint64("available", memAvailable). + Msg("QEMU memory: using RRD memavailable fallback") + case rrdMemory.hasUsed && rrdMemory.used <= memTotal: + selectedUsed = rrdMemory.used + hasSelectedUsed = true + memorySource = "rrd-memused" + log.Debug(). + Str("vm", guestName). + Str("node", node). + Int("vmid", vmid). + Uint64("total", memTotal). + Uint64("used", rrdMemory.used). + Msg("QEMU memory: using RRD memused fallback") } - log.Debug(). - Str("vm", guestName). - Str("node", node). - Int("vmid", vmid). - Uint64("total", memTotal). - Uint64("available", memAvailable). - Msg("QEMU memory: using RRD memavailable fallback (excludes reclaimable cache)") } else if rrdErr != nil { log.Debug(). Err(rrdErr). @@ -307,50 +347,69 @@ func (m *Monitor) resolveGuestStatusMemory( } } - if memAvailable == 0 && status.Agent.IsAvailable() && !triedGuestAgentMemAvailable { - if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { + if !hasMemAvailable && !hasSelectedUsed && status.Agent.IsAvailable() && !triedGuestAgentMemAvailable { + if agentAvailable, agentSource, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { memAvailable = agentAvailable - memorySource = "guest-agent-meminfo" + hasMemAvailable = true + memorySource = agentSource } } - if memAvailable == 0 { + if !hasMemAvailable && !hasSelectedUsed && derivedTotalMinusUsedAvailable > 0 { + memAvailable = derivedTotalMinusUsedAvailable + hasMemAvailable = true + memorySource = "derived-total-minus-used" + log.Debug(). + Str("vm", guestName). + Str("node", node). + Int("vmid", vmid). + Uint64("total", memTotal). + Uint64("available", memAvailable). + Uint64("availableFromUsed", derivedTotalMinusUsedAvailable). + Msg("QEMU memory: deriving guest available from total-used gap after preferred fallbacks") + } + + if !hasMemAvailable && !hasSelectedUsed { if agentHost, ok := vmIDToHostAgent[guestID]; ok && - agentHost.Memory.Total > 0 && - agentHost.Memory.Used >= 0 && + agentHost.Memory.HasKnownUsage() && + agentHost.Memory.Used <= int64(memTotal) && agentHost.Memory.Total >= agentHost.Memory.Used { agentAvailable := uint64(agentHost.Memory.Total - agentHost.Memory.Used) - if agentAvailable > 0 { - memAvailable = agentAvailable - memorySource = "agent" - if guestRaw != nil { - guestRaw.HostAgentTotal = uint64(agentHost.Memory.Total) - guestRaw.HostAgentUsed = uint64(agentHost.Memory.Used) - } - log.Debug(). - Str("vm", guestName). - Str("node", node). - Int("vmid", vmid). - Uint64("total", memTotal). - Uint64("available", memAvailable). - Int64("agentTotal", agentHost.Memory.Total). - Int64("agentUsed", agentHost.Memory.Used). - Msg("QEMU memory: using linked Pulse host agent memory (excludes page cache)") + memAvailable = agentAvailable + hasMemAvailable = true + selectedUsed = uint64(agentHost.Memory.Used) + hasSelectedUsed = true + memorySource = "agent" + if guestRaw != nil { + guestRaw.HostAgentTotal = uint64(agentHost.Memory.Total) + guestRaw.HostAgentUsed = uint64(agentHost.Memory.Used) } + log.Debug(). + Str("vm", guestName). + Str("node", node). + Int("vmid", vmid). + Uint64("total", memTotal). + Uint64("available", memAvailable). + Int64("agentTotal", agentHost.Memory.Total). + Int64("agentUsed", agentHost.Memory.Used). + Msg("QEMU memory: using linked Pulse host agent memory (excludes page cache)") } } memUsed := uint64(0) switch { - case memAvailable > 0: - if memAvailable > memTotal { - memAvailable = memTotal - } + case hasSelectedUsed: + memUsed = selectedUsed + case hasMemAvailable: memUsed = memTotal - memAvailable default: - memUsed, memorySource = selectGuestLowTrustUsedMemory(memTotal, status) - if memorySource == "" { - memorySource = "status-unavailable" + if status.MemInfo != nil { + memorySource = "unavailable" + } else { + memUsed, memorySource = selectGuestLowTrustUsedMemory(memTotal, status) + if memorySource == "" { + memorySource = "unavailable" + } } } if memUsed > memTotal { diff --git a/internal/monitoring/guest_memory_stability.go b/internal/monitoring/guest_memory_stability.go index 52b3586f9..a4e546690 100644 --- a/internal/monitoring/guest_memory_stability.go +++ b/internal/monitoring/guest_memory_stability.go @@ -20,11 +20,11 @@ const ( func guestMemorySourceReliability(source string) int { switch CanonicalMemorySource(source) { case "available-field", "derived-free-buffers-cached", - "guest-agent-meminfo", "rrd-memavailable", "rrd-memused", "agent": + "guest-agent-meminfo", "guest-agent-meminfo-derived", "rrd-memavailable", "rrd-memused", "agent": return guestMemoryReliabilityTrusted case "derived-total-minus-used", "previous-snapshot": return guestMemoryReliabilityFallback - case "unknown", "cluster-resources", "status-mem", "status-freemem", "status-unavailable": + case "unknown", "unavailable", "cluster-resources", "status-mem", "status-freemem", "status-unavailable": return guestMemoryReliabilityLow default: return guestMemoryReliabilityFallback diff --git a/internal/monitoring/guest_memory_stability_test.go b/internal/monitoring/guest_memory_stability_test.go index 6c5e9e129..dd91b29b6 100644 --- a/internal/monitoring/guest_memory_stability_test.go +++ b/internal/monitoring/guest_memory_stability_test.go @@ -36,6 +36,34 @@ func TestStabilizeGuestLowTrustMemoryCarriesForwardTrustedSnapshot(t *testing.T) } } +func TestStabilizeGuestLowTrustMemoryCarriesForwardAcrossUnavailableReconnect(t *testing.T) { + const gib = uint64(1024 * 1024 * 1024) + + now := time.Now() + prev := &GuestMemorySnapshot{ + Status: "running", + RetrievedAt: now.Add(-30 * time.Second), + MemorySource: "guest-agent-meminfo", + Memory: models.Memory{ + Total: 8 * int64(gib), + Used: 3 * int64(gib), + Free: 5 * int64(gib), + Usage: 37.5, + }, + } + + used, source, notes := stabilizeGuestLowTrustMemory(prev, "running", "unavailable", 8*gib, 0, now, false) + if used != 3*gib { + t.Fatalf("used = %d, want %d", used, 3*gib) + } + if source != "previous-snapshot" { + t.Fatalf("source = %q, want previous-snapshot", source) + } + if len(notes) != 1 { + t.Fatalf("notes = %#v, want carry-forward diagnostic", notes) + } +} + func TestStabilizeGuestLowTrustMemoryUsesHealthyGuestAgentEvidence(t *testing.T) { const gib = uint64(1024 * 1024 * 1024) diff --git a/internal/monitoring/memory_source_catalog.go b/internal/monitoring/memory_source_catalog.go index 6ea1d473f..5f440c831 100644 --- a/internal/monitoring/memory_source_catalog.go +++ b/internal/monitoring/memory_source_catalog.go @@ -12,6 +12,7 @@ type MemorySourceDescriptor struct { var memorySourceCatalog = map[string]MemorySourceDescriptor{ "": {Canonical: "unknown", Trust: "fallback", Fallback: true}, "unknown": {Canonical: "unknown", Trust: "fallback", Fallback: true}, + "unavailable": {Canonical: "unavailable", Trust: "unavailable", Fallback: true, DefaultFallbackReason: "cache-aware-memory-unavailable"}, "nodes-endpoint": {Canonical: "nodes-endpoint", Trust: "fallback", Fallback: true}, "node-status-used": {Canonical: "node-status-used", Trust: "fallback", Fallback: true}, "previous-snapshot": {Canonical: "previous-snapshot", Trust: "fallback", Fallback: true, DefaultFallbackReason: "preserved-previous-snapshot"}, @@ -25,6 +26,7 @@ var memorySourceCatalog = map[string]MemorySourceDescriptor{ "derived-total-minus-used": {Canonical: "derived-total-minus-used", Trust: "derived", Fallback: true, DefaultFallbackReason: "derived-total-minus-used"}, "meminfo-total-minus-used": {Canonical: "derived-total-minus-used", Trust: "derived", Fallback: true, DefaultFallbackReason: "derived-total-minus-used"}, "guest-agent-meminfo": {Canonical: "guest-agent-meminfo", Trust: "fallback", Fallback: true, DefaultFallbackReason: "guest-agent-meminfo"}, + "guest-agent-meminfo-derived": {Canonical: "guest-agent-meminfo-derived", Trust: "derived", Fallback: true, DefaultFallbackReason: "guest-agent-meminfo-derived"}, "rrd-memavailable": {Canonical: "rrd-memavailable", Trust: "fallback", Fallback: true, DefaultFallbackReason: "rrd-memavailable"}, "rrd-available": {Canonical: "rrd-memavailable", Trust: "fallback", Fallback: true, DefaultFallbackReason: "rrd-memavailable"}, "rrd-memused": {Canonical: "rrd-memused", Trust: "fallback", Fallback: true, DefaultFallbackReason: "rrd-memused"}, diff --git a/internal/monitoring/memory_source_catalog_test.go b/internal/monitoring/memory_source_catalog_test.go index 56d49985d..8b2874589 100644 --- a/internal/monitoring/memory_source_catalog_test.go +++ b/internal/monitoring/memory_source_catalog_test.go @@ -97,6 +97,14 @@ func TestDescribeMemorySourceCanonicalizesAliases(t *testing.T) { wantCanonical: "powered-off", wantTrust: "fallback", }, + { + name: "unavailable is explicit and diagnosable", + source: "unavailable", + wantCanonical: "unavailable", + wantTrust: "unavailable", + wantFallback: true, + wantDefaultFallback: "cache-aware-memory-unavailable", + }, { name: "unknown source falls back canonically", source: "custom-source", @@ -138,7 +146,7 @@ func TestMockVMPollingDefersMemoryHistoryToCanonicalSampler(t *testing.T) { requiredSnippets := []string{ "if !shouldSkipNativeMockStateMetricWrites() {", - `m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), vm.Memory.Usage, vm.Disk.Usage, -1, -1, -1, -1, now)`, + `m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), historyMemoryUsage(vm.Memory), vm.Disk.Usage, -1, -1, -1, -1, now)`, } for _, snippet := range requiredSnippets { if !strings.Contains(source, snippet) { diff --git a/internal/monitoring/memory_trust_characterization_test.go b/internal/monitoring/memory_trust_characterization_test.go index b58bf6a1a..b48ef5d6b 100644 --- a/internal/monitoring/memory_trust_characterization_test.go +++ b/internal/monitoring/memory_trust_characterization_test.go @@ -61,6 +61,8 @@ func TestPollPVENodeMemoryTrustCharacterization(t *testing.T) { wantFallback string wantUsed uint64 wantRawSource string + wantRRDUsed uint64 + wantUnknown bool }{ { name: "missing MemAvailable derives from free+buffers+cached", @@ -92,6 +94,67 @@ func TestPollPVENodeMemoryTrustCharacterization(t *testing.T) { wantUsed: 107351023616, wantRawSource: "node-status-total-minus-used", }, + { + name: "RRD memused wins over MemFree and cache-inclusive status used", + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * gib, + Used: 15 * gib / 2, + Free: gib / 2, + }, + }, + rrdPoints: []proxmox.NodeRRDPoint{{MemUsed: floatPtr(float64(6 * gib))}}, + wantSource: "rrd-memused", + wantFallback: "rrd-memused", + wantUsed: 6 * gib, + wantRawSource: "rrd-memused", + wantRRDUsed: 6 * gib, + }, + { + name: "explicit available wins over conflicting RRD", + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * gib, + Used: 15 * gib / 2, + Free: gib / 2, + Available: 2 * gib, + }, + }, + rrdPoints: []proxmox.NodeRRDPoint{{MemUsed: floatPtr(float64(7 * gib))}}, + wantSource: "available-field", + wantUsed: 6 * gib, + wantRawSource: "node-status", + }, + { + name: "MemFree without cache evidence stays unknown", + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * gib, + Used: 15 * gib / 2, + Free: gib / 2, + }, + }, + wantSource: "unavailable", + wantFallback: "cache-aware-memory-unavailable", + wantRawSource: "cache-aware-unavailable", + wantUnknown: true, + }, + { + name: "conflicting cache components above total stay unknown", + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * gib, + Used: 4 * gib, + Free: 4 * gib, + Buffers: 3 * gib, + Cached: 3 * gib, + }, + }, + wantSource: "unavailable", + wantFallback: "cache-aware-memory-unavailable", + wantRawSource: "cache-aware-unavailable", + wantUnknown: true, + }, } for _, tt := range tests { @@ -116,6 +179,9 @@ func TestPollPVENodeMemoryTrustCharacterization(t *testing.T) { if got := uint64(modelNode.Memory.Used); got != tt.wantUsed { t.Fatalf("modelNode.Memory.Used = %d, want %d", got, tt.wantUsed) } + if modelNode.Memory.UsageUnavailable != tt.wantUnknown { + t.Fatalf("modelNode.Memory.UsageUnavailable = %t, want %t", modelNode.Memory.UsageUnavailable, tt.wantUnknown) + } snap := mon.nodeSnapshots[makeNodeSnapshotKey("test", "node1")] if snap.MemorySource != tt.wantSource { @@ -127,6 +193,9 @@ func TestPollPVENodeMemoryTrustCharacterization(t *testing.T) { if snap.Raw.ProxmoxMemorySource != tt.wantRawSource { t.Fatalf("snapshot.Raw.ProxmoxMemorySource = %q, want %q", snap.Raw.ProxmoxMemorySource, tt.wantRawSource) } + if snap.Raw.RRDUsed != tt.wantRRDUsed { + t.Fatalf("snapshot.Raw.RRDUsed = %d, want %d", snap.Raw.RRDUsed, tt.wantRRDUsed) + } }) } } @@ -194,6 +263,23 @@ func TestPollPVENodePreservesPreviousSnapshotDuringTransientFallback(t *testing. if snap.Memory.Used != first.Memory.Used { t.Fatalf("snapshot.Memory.Used = %d, want preserved %d", snap.Memory.Used, first.Memory.Used) } + + third, _, _, err := mon.pollPVENode( + context.Background(), + "test", + &mon.config.PVEInstances[0], + client, + node, + "healthy", + map[string]models.Memory{second.ID: second.Memory}, + nil, + ) + if err != nil { + t.Fatalf("third pollPVENode() error = %v", err) + } + if !third.Memory.UsageUnavailable { + t.Fatalf("third.Memory = %+v, want honest unavailable state after bounded carry-forward", third.Memory) + } } func TestGuestDiskTrustCharacterizationCarriesForwardRecentSnapshot(t *testing.T) { @@ -244,13 +330,16 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { const gib = uint64(1024 * 1024 * 1024) tests := []struct { - name string - status *proxmox.VMStatus - rrdAvailable uint64 - wantSource string - wantUsed uint64 - wantAvailable uint64 - wantGap uint64 + name string + status *proxmox.VMStatus + rrdAvailable uint64 + rrdUsed uint64 + agentAvailable uint64 + wantSource string + wantUsed uint64 + wantAvailable uint64 + wantGap uint64 + wantUnknown bool }{ { name: "cache inflated Linux VM usage prefers RRD memavailable fallback", @@ -302,6 +391,102 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { wantUsed: 5 * gib, wantGap: 3 * gib, }, + { + name: "partial Linux meminfo uses RRD memused", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 7 * gib, + MemInfo: &proxmox.VMMemInfo{ + Total: 8 * gib, + Free: gib / 2, + }, + Agent: proxmox.VMAgentField{Value: 1}, + }, + rrdUsed: 5 * gib, + wantSource: "rrd-memused", + wantUsed: 5 * gib, + }, + { + name: "partial Linux meminfo without cache aware fallback stays unknown", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 15 * gib / 2, + MemInfo: &proxmox.VMMemInfo{ + Total: 8 * gib, + Free: gib / 2, + }, + Agent: proxmox.VMAgentField{Value: 1}, + }, + wantSource: "unavailable", + wantUnknown: true, + }, + { + name: "MemAvailable conflicting with the selected capacity stays unknown", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 7 * gib, + MemInfo: &proxmox.VMMemInfo{ + Total: 16 * gib, + Available: 12 * gib, + }, + }, + wantSource: "unavailable", + wantUnknown: true, + }, + { + name: "reclaimable components above the selected capacity stay unknown", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 7 * gib, + MemInfo: &proxmox.VMMemInfo{ + Total: 8 * gib, + Free: 2 * gib, + Buffers: 2 * gib, + Cached: 5 * gib, + }, + }, + wantSource: "unavailable", + wantUnknown: true, + }, + { + name: "guest agent availability above the selected capacity stays unknown", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 7 * gib, + MemInfo: &proxmox.VMMemInfo{ + Total: 8 * gib, + Free: gib / 2, + }, + Agent: proxmox.VMAgentField{Value: 1}, + }, + agentAvailable: 9 * gib, + wantSource: "unavailable", + wantUnknown: true, + }, + { + name: "RRD available wins over delayed total minus used estimate", + status: &proxmox.VMStatus{ + Status: "running", + MaxMem: 8 * gib, + Mem: 7 * gib, + MemInfo: &proxmox.VMMemInfo{ + Total: 8 * gib, + Used: 5 * gib, + Free: gib / 2, + }, + Agent: proxmox.VMAgentField{Value: 1}, + }, + rrdAvailable: 4 * gib, + wantSource: "rrd-memavailable", + wantUsed: 4 * gib, + wantAvailable: 4 * gib, + wantGap: 3 * gib, + }, { name: "materially inconsistent status memory prefers freemem fallback", status: &proxmox.VMStatus{ @@ -348,11 +533,14 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { defer mon.notificationMgr.Stop() client := &vmMemoryTrustStubClient{ - stubPVEClient: &stubPVEClient{}, - vmStatus: tt.status, + stubPVEClient: &stubPVEClient{}, + vmStatus: tt.status, + vmAgentMemAvailable: tt.agentAvailable, } if tt.rrdAvailable > 0 { client.vmRRDPoints = []proxmox.GuestRRDPoint{{MemAvailable: floatPtr(float64(tt.rrdAvailable))}} + } else if tt.rrdUsed > 0 { + client.vmRRDPoints = []proxmox.GuestRRDPoint{{MemUsed: floatPtr(float64(tt.rrdUsed))}} } res := proxmox.ClusterResource{ @@ -374,6 +562,9 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { if got := uint64(vm.Memory.Used); got != tt.wantUsed { t.Fatalf("vm.Memory.Used = %d, want %d", got, tt.wantUsed) } + if vm.Memory.UsageUnavailable != tt.wantUnknown { + t.Fatalf("vm.Memory.UsageUnavailable = %t, want %t", vm.Memory.UsageUnavailable, tt.wantUnknown) + } snap := mon.guestSnapshots[makeGuestSnapshotKey("test", "qemu", "node1", 101)] if snap.MemorySource != tt.wantSource { @@ -382,8 +573,11 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { if tt.wantSource == "derived-total-minus-used" && snap.FallbackReason != "derived-total-minus-used" { t.Fatalf("snapshot.FallbackReason = %q, want derived-total-minus-used", snap.FallbackReason) } - if tt.wantAvailable > 0 && snap.Raw.MemInfoAvailable != tt.wantAvailable { - t.Fatalf("snapshot.Raw.MemInfoAvailable = %d, want %d", snap.Raw.MemInfoAvailable, tt.wantAvailable) + if tt.wantAvailable > 0 && snap.Raw.RRDMemAvailable != tt.wantAvailable { + t.Fatalf("snapshot.Raw.RRDMemAvailable = %d, want %d", snap.Raw.RRDMemAvailable, tt.wantAvailable) + } + if tt.rrdUsed > 0 && snap.Raw.RRDMemUsed != tt.rrdUsed { + t.Fatalf("snapshot.Raw.RRDMemUsed = %d, want %d", snap.Raw.RRDMemUsed, tt.rrdUsed) } if tt.wantGap > 0 && snap.Raw.MemInfoTotalMinusUsed != tt.wantGap { t.Fatalf("snapshot.Raw.MemInfoTotalMinusUsed = %d, want %d", snap.Raw.MemInfoTotalMinusUsed, tt.wantGap) @@ -645,8 +839,8 @@ func TestPollVMsWithNodesMemoryTrustCharacterization(t *testing.T) { if got := uint64(snap.Memory.Used); got != tt.wantUsed { t.Fatalf("snapshot.Memory.Used = %d, want %d", got, tt.wantUsed) } - if tt.wantAvailable > 0 && snap.Raw.MemInfoAvailable != tt.wantAvailable { - t.Fatalf("snapshot.Raw.MemInfoAvailable = %d, want %d", snap.Raw.MemInfoAvailable, tt.wantAvailable) + if tt.wantAvailable > 0 && snap.Raw.RRDMemAvailable != tt.wantAvailable { + t.Fatalf("snapshot.Raw.RRDMemAvailable = %d, want %d", snap.Raw.RRDMemAvailable, tt.wantAvailable) } if tt.wantSource == "derived-total-minus-used" && snap.FallbackReason != "derived-total-minus-used" { t.Fatalf("snapshot.FallbackReason = %q, want derived-total-minus-used", snap.FallbackReason) @@ -736,6 +930,26 @@ func TestRecordGuestMetric_SkipsNativeWritesInMockMode(t *testing.T) { } } +func TestRecordGuestMetricPreservesHistoryWhenLiveMemoryIsUnavailable(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + mon := newTestPVEMonitor("test") + defer mon.alertManager.Stop() + defer mon.notificationMgr.Stop() + + now := time.Now().UTC() + mon.recordGuestMetric("vm", "test:node1:101", 20, 76, 40, -1, -1, -1, -1, now.Add(-time.Minute)) + mon.recordGuestMetric("vm", "test:node1:101", 25, -1, 42, -1, -1, -1, -1, now) + + points := mon.metricsHistory.GetGuestMetrics("test:node1:101", "memory", time.Hour) + if len(points) != 1 { + t.Fatalf("memory history points = %d, want 1 trusted point", len(points)) + } + if points[0].Value != 76 { + t.Fatalf("memory history value = %.2f, want 76", points[0].Value) + } +} + func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) @@ -749,6 +963,7 @@ func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) wantUsed uint64 wantAvail uint64 wantStatus string + wantUnknown bool }{ { name: "cache inflated LXC usage prefers RRD memavailable fallback", @@ -788,7 +1003,42 @@ func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) wantStatus: "running", }, { - name: "stopped LXC keeps cluster resources source", + name: "explicit zero RRD memavailable is valid full usage", + res: proxmox.ClusterResource{ + ID: "lxc/205", + Type: "lxc", + Node: "node1", + Name: "ct-205", + Status: "running", + VMID: 205, + MaxMem: 8 * gib, + Mem: 7 * gib, + MaxCPU: 4, + }, + lxcRRDPoints: []proxmox.GuestRRDPoint{{MemAvailable: floatPtr(0)}}, + wantSource: "rrd-memavailable", + wantUsed: 8 * gib, + wantStatus: "running", + }, + { + name: "running LXC without RRD memory stays unknown", + res: proxmox.ClusterResource{ + ID: "lxc/204", + Type: "lxc", + Node: "node1", + Name: "ct-204", + Status: "running", + VMID: 204, + MaxMem: 8 * gib, + Mem: 7 * gib, + MaxCPU: 4, + }, + wantSource: "unavailable", + wantStatus: "running", + wantUnknown: true, + }, + { + name: "stopped LXC reports powered off memory", res: proxmox.ClusterResource{ ID: "lxc/203", Type: "lxc", @@ -800,8 +1050,8 @@ func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) Mem: 2 * gib, MaxCPU: 4, }, - wantSource: "cluster-resources", - wantUsed: 2 * gib, + wantSource: "powered-off", + wantUsed: 0, wantStatus: "stopped", }, } @@ -834,6 +1084,9 @@ func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) if got := uint64(container.Memory.Used); got != tt.wantUsed { t.Fatalf("container.Memory.Used = %d, want %d", got, tt.wantUsed) } + if container.Memory.UsageUnavailable != tt.wantUnknown { + t.Fatalf("container.Memory.UsageUnavailable = %t, want %t", container.Memory.UsageUnavailable, tt.wantUnknown) + } key := makeGuestSnapshotKey("test", container.Type, "node1", tt.res.VMID) snap, ok := mon.guestSnapshots[key] @@ -846,8 +1099,8 @@ func TestHandleClusterContainerResourceMemoryTrustCharacterization(t *testing.T) if got := uint64(snap.Memory.Used); got != tt.wantUsed { t.Fatalf("snapshot.Memory.Used = %d, want %d", got, tt.wantUsed) } - if tt.wantAvail > 0 && snap.Raw.MemInfoAvailable != tt.wantAvail { - t.Fatalf("snapshot.Raw.MemInfoAvailable = %d, want %d", snap.Raw.MemInfoAvailable, tt.wantAvail) + if tt.wantAvail > 0 && snap.Raw.RRDMemAvailable != tt.wantAvail { + t.Fatalf("snapshot.Raw.RRDMemAvailable = %d, want %d", snap.Raw.RRDMemAvailable, tt.wantAvail) } }) } @@ -917,7 +1170,7 @@ func TestPollContainersWithNodesMemoryTrustCharacterization(t *testing.T) { wantAvail: 3 * gib, }, { - name: "stopped LXC keeps cluster resources source in node polling", + name: "stopped LXC reports powered off memory in node polling", container: proxmox.Container{ VMID: 302, Name: "ct-302", @@ -927,8 +1180,8 @@ func TestPollContainersWithNodesMemoryTrustCharacterization(t *testing.T) { Mem: 2 * gib, CPUs: 2, }, - wantSource: "cluster-resources", - wantUsed: 2 * gib, + wantSource: "powered-off", + wantUsed: 0, }, } @@ -959,8 +1212,8 @@ func TestPollContainersWithNodesMemoryTrustCharacterization(t *testing.T) { if got := uint64(snap.Memory.Used); got != tt.wantUsed { t.Fatalf("snapshot.Memory.Used = %d, want %d", got, tt.wantUsed) } - if tt.wantAvail > 0 && snap.Raw.MemInfoAvailable != tt.wantAvail { - t.Fatalf("snapshot.Raw.MemInfoAvailable = %d, want %d", snap.Raw.MemInfoAvailable, tt.wantAvail) + if tt.wantAvail > 0 && snap.Raw.RRDMemAvailable != tt.wantAvail { + t.Fatalf("snapshot.Raw.RRDMemAvailable = %d, want %d", snap.Raw.RRDMemAvailable, tt.wantAvail) } }) } diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 2e28517f8..eb5f6b603 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -1217,6 +1217,9 @@ type rrdMemCacheEntry struct { available uint64 used uint64 total uint64 + hasAvail bool + hasUsed bool + hasTotal bool netIn float64 netOut float64 hasNetIn bool @@ -1301,15 +1304,20 @@ type taskOutcome struct { recordedAt time.Time } -func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterface, nodeName string) (rrdMemCacheEntry, error) { +func nodeRRDCacheKey(instanceName, nodeName string) string { + return instanceName + "/" + nodeName +} + +func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterface, instanceName, nodeName string) (rrdMemCacheEntry, error) { if client == nil || nodeName == "" { return rrdMemCacheEntry{}, fmt.Errorf("invalid arguments for RRD lookup") } now := time.Now() + cacheKey := nodeRRDCacheKey(instanceName, nodeName) m.rrdCacheMu.RLock() - if entry, ok := m.nodeRRDMemCache[nodeName]; ok && now.Sub(entry.fetchedAt) < nodeRRDCacheTTL { + if entry, ok := m.nodeRRDMemCache[cacheKey]; ok && now.Sub(entry.fetchedAt) < nodeRRDCacheTTL { m.rrdCacheMu.RUnlock() return entry, nil } @@ -1326,6 +1334,9 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa var memAvailable uint64 var memUsed uint64 var memTotal uint64 + var hasAvail bool + var hasUsed bool + var hasTotal bool var netIn float64 var netOut float64 var hasNetIn bool @@ -1334,23 +1345,26 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa for i := len(points) - 1; i >= 0; i-- { point := points[i] - if memTotal == 0 && point.MemTotal != nil && !math.IsNaN(*point.MemTotal) && *point.MemTotal > 0 { + if !hasTotal && point.MemTotal != nil && !math.IsNaN(*point.MemTotal) && !math.IsInf(*point.MemTotal, 0) && *point.MemTotal > 0 && *point.MemTotal <= math.MaxUint64 { memTotal = uint64(math.Round(*point.MemTotal)) + hasTotal = true } - if memAvailable == 0 && point.MemAvailable != nil && !math.IsNaN(*point.MemAvailable) && *point.MemAvailable > 0 { + if !hasAvail && point.MemAvailable != nil && !math.IsNaN(*point.MemAvailable) && !math.IsInf(*point.MemAvailable, 0) && *point.MemAvailable >= 0 && *point.MemAvailable <= math.MaxUint64 { memAvailable = uint64(math.Round(*point.MemAvailable)) + hasAvail = true } - if memUsed == 0 && point.MemUsed != nil && !math.IsNaN(*point.MemUsed) && *point.MemUsed > 0 { + if !hasUsed && point.MemUsed != nil && !math.IsNaN(*point.MemUsed) && !math.IsInf(*point.MemUsed, 0) && *point.MemUsed >= 0 && *point.MemUsed <= math.MaxUint64 { memUsed = uint64(math.Round(*point.MemUsed)) + hasUsed = true } - if !hasNetIn && point.NetIn != nil && !math.IsNaN(*point.NetIn) { + if !hasNetIn && point.NetIn != nil && !math.IsNaN(*point.NetIn) && !math.IsInf(*point.NetIn, 0) { netIn = *point.NetIn hasNetIn = true } - if !hasNetOut && point.NetOut != nil && !math.IsNaN(*point.NetOut) { + if !hasNetOut && point.NetOut != nil && !math.IsNaN(*point.NetOut) && !math.IsInf(*point.NetOut, 0) { netOut = *point.NetOut hasNetOut = true } @@ -1358,14 +1372,16 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa if memTotal > 0 { if memAvailable > memTotal { - memAvailable = memTotal + memAvailable = 0 + hasAvail = false } if memUsed > memTotal { - memUsed = memTotal + memUsed = 0 + hasUsed = false } } - if memAvailable == 0 && memUsed == 0 && !hasNetIn && !hasNetOut { + if !hasAvail && !hasUsed && !hasNetIn && !hasNetOut { return rrdMemCacheEntry{}, fmt.Errorf("rrd node metrics not present") } @@ -1373,6 +1389,9 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa available: memAvailable, used: memUsed, total: memTotal, + hasAvail: hasAvail, + hasUsed: hasUsed, + hasTotal: hasTotal, netIn: netIn, netOut: netOut, hasNetIn: hasNetIn, @@ -1381,7 +1400,10 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa } m.rrdCacheMu.Lock() - m.nodeRRDMemCache[nodeName] = entry + if m.nodeRRDMemCache == nil { + m.nodeRRDMemCache = make(map[string]rrdMemCacheEntry) + } + m.nodeRRDMemCache[cacheKey] = entry m.rrdCacheMu.Unlock() return entry, nil @@ -1391,8 +1413,19 @@ func (m *Monitor) getNodeRRDMetrics(ctx context.Context, client PVEClientInterfa // short-lived cache to avoid a live API call on every poll for VMs that // consistently lack guest-agent memory data (e.g. Windows VMs). func (m *Monitor) getVMRRDMetrics(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (uint64, error) { + entry, err := m.getVMRRDMemory(ctx, client, instanceName, node, vmid) + if err != nil { + return 0, err + } + if !entry.hasAvail { + return 0, fmt.Errorf("rrd memavailable not present for VM %s/%d", node, vmid) + } + return entry.available, nil +} + +func (m *Monitor) getVMRRDMemory(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (rrdMemCacheEntry, error) { if client == nil || node == "" || vmid <= 0 { - return 0, fmt.Errorf("invalid arguments for VM RRD lookup") + return rrdMemCacheEntry{}, fmt.Errorf("invalid arguments for VM RRD lookup") } cacheKey := guestMemoryCacheKey(instanceName, node, vmid) @@ -1401,34 +1434,65 @@ func (m *Monitor) getVMRRDMetrics(ctx context.Context, client PVEClientInterface m.rrdCacheMu.RLock() if entry, ok := m.vmRRDMemCache[cacheKey]; ok && now.Sub(entry.fetchedAt) < nodeRRDCacheTTL { m.rrdCacheMu.RUnlock() - return entry.available, nil + return entry, nil } m.rrdCacheMu.RUnlock() requestCtx, cancel := context.WithTimeout(ctx, nodeRRDRequestTimeout) defer cancel() - points, err := client.GetVMRRDData(requestCtx, node, vmid, "hour", "AVERAGE", []string{"memavailable"}) + points, err := client.GetVMRRDData(requestCtx, node, vmid, "hour", "AVERAGE", []string{"memavailable", "memused", "maxmem"}) if err != nil { - return 0, err + return rrdMemCacheEntry{}, err } if len(points) == 0 { - return 0, fmt.Errorf("no RRD points for VM %s/%d", node, vmid) + return rrdMemCacheEntry{}, fmt.Errorf("no RRD points for VM %s/%d", node, vmid) } var memAvailable uint64 + var memUsed uint64 + var memTotal uint64 + var hasAvail bool + var hasUsed bool + var hasTotal bool for i := len(points) - 1; i >= 0; i-- { p := points[i] - if p.MemAvailable != nil && !math.IsNaN(*p.MemAvailable) && *p.MemAvailable > 0 { + if !hasAvail && p.MemAvailable != nil && !math.IsNaN(*p.MemAvailable) && !math.IsInf(*p.MemAvailable, 0) && *p.MemAvailable >= 0 && *p.MemAvailable <= math.MaxUint64 { memAvailable = uint64(math.Round(*p.MemAvailable)) - break + hasAvail = true + } + if !hasUsed && p.MemUsed != nil && !math.IsNaN(*p.MemUsed) && !math.IsInf(*p.MemUsed, 0) && *p.MemUsed >= 0 && *p.MemUsed <= math.MaxUint64 { + memUsed = uint64(math.Round(*p.MemUsed)) + hasUsed = true + } + if !hasTotal && p.MaxMem != nil && !math.IsNaN(*p.MaxMem) && !math.IsInf(*p.MaxMem, 0) && *p.MaxMem > 0 && *p.MaxMem <= math.MaxUint64 { + memTotal = uint64(math.Round(*p.MaxMem)) + hasTotal = true } } - if memAvailable == 0 { - return 0, fmt.Errorf("rrd memavailable not present for VM %s/%d", node, vmid) + if hasTotal { + if hasAvail && memAvailable > memTotal { + hasAvail = false + memAvailable = 0 + } + if hasUsed && memUsed > memTotal { + hasUsed = false + memUsed = 0 + } + } + if !hasAvail && !hasUsed { + return rrdMemCacheEntry{}, fmt.Errorf("rrd memory fields not present for VM %s/%d", node, vmid) } - entry := rrdMemCacheEntry{available: memAvailable, fetchedAt: now} + entry := rrdMemCacheEntry{ + available: memAvailable, + used: memUsed, + total: memTotal, + hasAvail: hasAvail, + hasUsed: hasUsed, + hasTotal: hasTotal, + fetchedAt: now, + } m.rrdCacheMu.Lock() if m.vmRRDMemCache == nil { m.vmRRDMemCache = make(map[string]rrdMemCacheEntry) @@ -1436,7 +1500,7 @@ func (m *Monitor) getVMRRDMetrics(ctx context.Context, client PVEClientInterface m.vmRRDMemCache[cacheKey] = entry m.rrdCacheMu.Unlock() - return memAvailable, nil + return entry, nil } // RemoveDockerHost removes a docker host from the shared state and clears related alerts. diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 4ca95fc78..ce34081d2 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -28,6 +28,62 @@ const hostContinuityRetention = 72 * time.Hour const maxRetiredHostReportStreams = 8 +func normalizeAgentMemory(total, used, free, cache int64, usage float64, swapTotal, swapUsed int64) models.Memory { + if total <= 0 { + return models.Memory{} + } + hasReportedUsed := used > 0 + + unavailable := func() models.Memory { + memory := models.UnavailableMemory(total) + memory.SwapTotal = max(0, swapTotal) + memory.SwapUsed = max(0, swapUsed) + return memory + } + + usage = safeFloat(usage) + if used < 0 || free < 0 || cache < 0 || used > total || usage < 0 || usage > 100 { + return unavailable() + } + + if used == 0 { + switch { + case usage > 0: + used = int64(float64(total) * usage / 100) + case cache > 0 && cache <= total && free <= total-cache: + used = total - free - cache + case free == total: + // A completely idle host is a valid zero-usage measurement. + default: + return unavailable() + } + } + + if cache > total-used { + cache = total - used + } + if free > total-used-cache { + free = total - used - cache + } + // Byte counters are canonical when the agent reported them. Recompute the + // percentage so a stale percentage cannot diverge display, history, and + // threshold evaluation. Preserve a percentage-only report to avoid losing + // precision when deriving its byte approximation. + if hasReportedUsed || usage == 0 { + usage = safePercentage(float64(used), float64(total)) + } + + return models.Memory{ + Total: total, + Used: used, + Free: free, + Cache: cache, + Usage: usage, + SwapTotal: max(0, swapTotal), + SwapUsed: max(0, swapUsed), + } +} + type hostReportOrder struct { ObservedAt time.Time LastReceivedAt time.Time @@ -1847,37 +1903,21 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con loadAverage = append(loadAverage, report.Host.LoadAverage...) } - var memory models.Memory - if report.Host.Memory.TotalBytes > 0 || report.Host.Memory.UsedBytes > 0 { - memory = models.Memory{ - Total: report.Host.Memory.TotalBytes, - Used: report.Host.Memory.UsedBytes, - Free: report.Host.Memory.FreeBytes, - Usage: safeFloat(report.Host.Memory.Usage), - SwapTotal: report.Host.Memory.SwapTotal, - SwapUsed: report.Host.Memory.SwapUsed, - } - } + memory := normalizeAgentMemory( + report.Host.Memory.TotalBytes, + report.Host.Memory.UsedBytes, + report.Host.Memory.FreeBytes, + report.Host.Memory.CacheBytes, + report.Host.Memory.Usage, + report.Host.Memory.SwapTotal, + report.Host.Memory.SwapUsed, + ) // Fallback: if gopsutil's memory reading failed but Docker's TotalMemoryBytes // is valid (possibly already a fallback from the agent), use that for Total. // This handles Docker-in-LXC scenarios where both Docker and gopsutil may // fail to read memory stats, but the agent fix provides a valid fallback. if memory.Total <= 0 && report.Host.TotalMemoryBytes > 0 { - memory.Total = report.Host.TotalMemoryBytes - } - - // Additional fallback for Docker-in-LXC: gopsutil may read Total and Free - // correctly from cgroup limits but return 0 for Used. Calculate Used from - // Total - Free when this happens. This fixes the "0B / 7GB" display issue. - if memory.Used <= 0 && memory.Total > 0 && memory.Free > 0 { - memory.Used = memory.Total - memory.Free - if memory.Used < 0 { - memory.Used = 0 - } - // Recalculate usage percentage - if memory.Total > 0 { - memory.Usage = safePercentage(float64(memory.Used), float64(memory.Total)) - } + memory = models.UnavailableMemory(report.Host.TotalMemoryBytes) } disks := make([]models.Disk, 0, len(report.Host.Disks)) @@ -2041,13 +2081,17 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con if m.metricsHistory != nil { m.metricsHistory.AddGuestMetric(hostMetricKey, "cpu", host.CPUUsage, now) - m.metricsHistory.AddGuestMetric(hostMetricKey, "memory", host.Memory.Usage, now) + if host.Memory.HasKnownUsage() { + m.metricsHistory.AddGuestMetric(hostMetricKey, "memory", host.Memory.Usage, now) + } m.metricsHistory.AddGuestMetric(hostMetricKey, "disk", hostDiskPercent, now) } if m.metricsStore != nil { m.metricsStore.Write("dockerHost", host.ID, "cpu", host.CPUUsage, now) - m.metricsStore.Write("dockerHost", host.ID, "memory", host.Memory.Usage, now) + if host.Memory.HasKnownUsage() { + m.metricsStore.Write("dockerHost", host.ID, "memory", host.Memory.Usage, now) + } m.metricsStore.Write("dockerHost", host.ID, "disk", hostDiskPercent, now) } @@ -2379,35 +2423,15 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. displayName = hostname } - memory := models.Memory{ - Total: report.Metrics.Memory.TotalBytes, - Used: report.Metrics.Memory.UsedBytes, - Free: report.Metrics.Memory.FreeBytes, - Cache: report.Metrics.Memory.CacheBytes, - Usage: safeFloat(report.Metrics.Memory.Usage), - SwapTotal: report.Metrics.Memory.SwapTotal, - SwapUsed: report.Metrics.Memory.SwapUsed, - } - // Older agents don't report cache; clamp so used + cache never exceeds total. - if memory.Cache < 0 { - memory.Cache = 0 - } - if memory.Total > 0 && memory.Used+memory.Cache > memory.Total { - memory.Cache = max(0, memory.Total-memory.Used) - } - - // Fallback for LXC environments: gopsutil may read Total and Free correctly - // from cgroup limits but return 0 for Used. Calculate Used from Total - Free. - if memory.Used <= 0 && memory.Total > 0 && memory.Free > 0 { - memory.Used = memory.Total - memory.Free - if memory.Used < 0 { - memory.Used = 0 - } - } - - if memory.Usage <= 0 && memory.Total > 0 { - memory.Usage = safePercentage(float64(memory.Used), float64(memory.Total)) - } + memory := normalizeAgentMemory( + report.Metrics.Memory.TotalBytes, + report.Metrics.Memory.UsedBytes, + report.Metrics.Memory.FreeBytes, + report.Metrics.Memory.CacheBytes, + report.Metrics.Memory.Usage, + report.Metrics.Memory.SwapTotal, + report.Metrics.Memory.SwapUsed, + ) disks := make([]models.Disk, 0, len(report.Disks)) for _, disk := range report.Disks { @@ -2766,7 +2790,9 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. if !shouldSkipNativeMockStateMetricWrites() { if m.metricsHistory != nil { m.metricsHistory.AddGuestMetric(hostMetricKey, "cpu", host.CPUUsage, now) - m.metricsHistory.AddGuestMetric(hostMetricKey, "memory", host.Memory.Usage, now) + if host.Memory.HasKnownUsage() { + m.metricsHistory.AddGuestMetric(hostMetricKey, "memory", host.Memory.Usage, now) + } m.metricsHistory.AddGuestMetric(hostMetricKey, "disk", hostDiskPercent, now) if hostTemperature != nil { m.metricsHistory.AddGuestMetric(hostMetricKey, "temperature", *hostTemperature, now) @@ -2790,7 +2816,9 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. if m.metricsStore != nil { m.metricsStore.Write("agent", host.ID, "cpu", host.CPUUsage, now) - m.metricsStore.Write("agent", host.ID, "memory", host.Memory.Usage, now) + if host.Memory.HasKnownUsage() { + m.metricsStore.Write("agent", host.ID, "memory", host.Memory.Usage, now) + } m.metricsStore.Write("agent", host.ID, "disk", hostDiskPercent, now) if hostTemperature != nil { m.metricsStore.Write("agent", host.ID, "temperature", *hostTemperature, now) diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index e50141f37..7d6f3a185 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -47,6 +47,96 @@ func TestMonitoringBroadcastCarriesEveryAvailabilityProjection(t *testing.T) { } } +func TestNormalizeAgentMemoryCacheAwareFallbacks(t *testing.T) { + const gib = int64(1024 * 1024 * 1024) + + tests := []struct { + name string + total int64 + used int64 + free int64 + cache int64 + usage float64 + swapTotal int64 + swapUsed int64 + wantUsed int64 + wantUsage float64 + wantUnavailable bool + }{ + { + name: "explicit agent usage remains authoritative", + total: 8 * gib, + used: 6 * gib, + free: gib / 2, + cache: 2 * gib, + wantUsed: 6 * gib, + wantUsage: 75, + }, + { + name: "explicit bytes correct a conflicting reported percentage", + total: 8 * gib, + used: 4 * gib, + free: gib, + usage: 94, + wantUsed: 4 * gib, + wantUsage: 50, + }, + { + name: "complete Linux cache evidence derives used without swap", + total: 8 * gib, + free: gib, + cache: 2 * gib, + wantUsed: 5 * gib, + wantUsage: 62.5, + }, + { + name: "total and free alone stay unknown", + total: 8 * gib, + free: gib, + wantUnavailable: true, + }, + { + name: "reported percentage can establish usage", + total: 8 * gib, + free: gib, + usage: 76, + wantUsed: 8 * gib * 76 / 100, + wantUsage: 76, + }, + { + name: "zero swap does not invalidate cache aware memory", + total: 8 * gib, + free: 2 * gib, + cache: gib, + swapTotal: 0, + swapUsed: 0, + wantUsed: 5 * gib, + wantUsage: 62.5, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := normalizeAgentMemory(tt.total, tt.used, tt.free, tt.cache, tt.usage, tt.swapTotal, tt.swapUsed) + if got.UsageUnavailable != tt.wantUnavailable { + t.Fatalf("UsageUnavailable = %t, want %t", got.UsageUnavailable, tt.wantUnavailable) + } + if tt.wantUnavailable { + if got.HasKnownUsage() { + t.Fatalf("HasKnownUsage() = true for unavailable memory: %+v", got) + } + return + } + if got.Used != tt.wantUsed { + t.Fatalf("Used = %d, want %d", got.Used, tt.wantUsed) + } + if got.Usage != tt.wantUsage { + t.Fatalf("Usage = %.2f, want %.2f", got.Usage, tt.wantUsage) + } + }) + } +} + func TestApplyHostReportOperationReceiptProtocolReplacesCapabilityAuthority(t *testing.T) { now := time.Now().UTC() baseReport := func(version int) agentshost.Report { diff --git a/internal/monitoring/monitor_memory_test.go b/internal/monitoring/monitor_memory_test.go index dc08b4534..8f31687b6 100644 --- a/internal/monitoring/monitor_memory_test.go +++ b/internal/monitoring/monitor_memory_test.go @@ -250,6 +250,31 @@ func TestPollPVEInstanceUsesRRDMemUsedFallback(t *testing.T) { } } +func TestNodeRRDMemoryCacheIsScopedByInstance(t *testing.T) { + mon := &Monitor{nodeRRDMemCache: make(map[string]rrdMemCacheEntry)} + const gib = uint64(1024 * 1024 * 1024) + + first, err := mon.getNodeRRDMetrics(context.Background(), &stubPVEClient{ + rrdPoints: []proxmox.NodeRRDPoint{{MemUsed: floatPtr(float64(2 * gib))}}, + }, "pve-a", "node1") + if err != nil { + t.Fatalf("first getNodeRRDMetrics() error = %v", err) + } + second, err := mon.getNodeRRDMetrics(context.Background(), &stubPVEClient{ + rrdPoints: []proxmox.NodeRRDPoint{{MemUsed: floatPtr(float64(6 * gib))}}, + }, "pve-b", "node1") + if err != nil { + t.Fatalf("second getNodeRRDMetrics() error = %v", err) + } + + if first.used != 2*gib || second.used != 6*gib { + t.Fatalf("cross-instance node RRD values leaked: first=%d second=%d", first.used, second.used) + } + if len(mon.nodeRRDMemCache) != 2 { + t.Fatalf("node RRD cache entries = %d, want two instance-scoped entries", len(mon.nodeRRDMemCache)) + } +} + func TestPollPVEInstancePreservesRecentNodesWhenGetNodesReturnsEmpty(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) diff --git a/internal/monitoring/monitor_polling_containers.go b/internal/monitoring/monitor_polling_containers.go index d2b8bb58f..9cbad2e16 100644 --- a/internal/monitoring/monitor_polling_containers.go +++ b/internal/monitoring/monitor_polling_containers.go @@ -141,6 +141,15 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri MaxMem: container.MaxMem, Mem: container.Mem, }, client) + memUsed, memorySource, _ = stabilizeGuestLowTrustMemory( + m.previousGuestSnapshot(instanceName, "lxc", n.Node, int(container.VMID)), + container.Status, + memorySource, + memTotal, + memUsed, + sampleTime, + false, + ) memTotalBytes := clampToInt64(memTotal) memUsedBytes := clampToInt64(memUsed) @@ -152,6 +161,15 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri memFreeBytes = 0 } memUsagePercent := safePercentage(float64(memUsedBytes), float64(memTotalBytes)) + memory := models.UnavailableMemory(memTotalBytes) + if CanonicalMemorySource(memorySource) != "unavailable" { + memory = models.Memory{ + Total: memTotalBytes, + Used: memUsedBytes, + Free: memFreeBytes, + Usage: memUsagePercent, + } + } diskTotalBytes := clampToInt64(container.MaxDisk) diskUsedBytes := clampToInt64(container.Disk) @@ -176,12 +194,7 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri Type: "lxc", CPU: cpuUsage, CPUs: int(container.CPUs), - Memory: models.Memory{ - Total: memTotalBytes, - Used: memUsedBytes, - Free: memFreeBytes, - Usage: memUsagePercent, - }, + Memory: memory, Disk: models.Disk{ Total: diskTotalBytes, Used: diskUsedBytes, @@ -337,7 +350,7 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri } // IO/network series are not recorded on the traditional polling // path (parity with the historical inline writes). - m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), ct.Memory.Usage, ct.Disk.Usage, -1, -1, -1, -1, now) + m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), historyMemoryUsage(ct.Memory), ct.Disk.Usage, -1, -1, -1, -1, now) } } diff --git a/internal/monitoring/monitor_polling_node.go b/internal/monitoring/monitor_polling_node.go index 28a094537..98b80eacd 100644 --- a/internal/monitoring/monitor_polling_node.go +++ b/internal/monitoring/monitor_polling_node.go @@ -11,6 +11,31 @@ import ( "github.com/rs/zerolog/log" ) +const nodeMemoryCarryForwardMaxAge = 2 * time.Minute + +func (m *Monitor) canCarryForwardNodeMemory(instance, node string, now time.Time) bool { + if m == nil { + return false + } + m.diagMu.RLock() + snapshot, ok := m.nodeSnapshots[makeNodeSnapshotKey(instance, node)] + m.diagMu.RUnlock() + if !ok || !snapshot.Memory.HasKnownUsage() || snapshot.RetrievedAt.IsZero() { + return false + } + age := now.Sub(snapshot.RetrievedAt) + if age < 0 || age > nodeMemoryCarryForwardMaxAge { + return false + } + switch CanonicalMemorySource(snapshot.MemorySource) { + case "available-field", "derived-free-buffers-cached", "derived-total-minus-used", + "rrd-memavailable", "rrd-memused": + return true + default: + return false + } +} + func (m *Monitor) pollPVENode( ctx context.Context, instanceName string, @@ -25,23 +50,22 @@ func (m *Monitor) pollPVENode( displayName := getNodeDisplayName(instanceCfg, node.Node) connectionHost, guestURL := resolveNodeConnectionInfo(instanceCfg, monitorDiscoveryConfig(m), node.Node) nodeID, effectiveStatus := m.determineNodeIDAndStatus(instanceName, instanceCfg, node) + nodeFallbackFree := uint64(0) + if node.MaxMem >= node.Mem { + nodeFallbackFree = node.MaxMem - node.Mem + } modelNode := models.Node{ - ID: nodeID, - Name: node.Node, - DisplayName: displayName, - Instance: instanceName, - Host: connectionHost, - GuestURL: guestURL, - Status: effectiveStatus, - Type: "node", - CPU: safeFloat(node.CPU), // Proxmox returns 0-1 ratio (e.g., 0.15 = 15%) - Memory: models.Memory{ - Total: int64(node.MaxMem), - Used: int64(node.Mem), - Free: int64(node.MaxMem - node.Mem), - Usage: safePercentage(float64(node.Mem), float64(node.MaxMem)), - }, + ID: nodeID, + Name: node.Node, + DisplayName: displayName, + Instance: instanceName, + Host: connectionHost, + GuestURL: guestURL, + Status: effectiveStatus, + Type: "node", + CPU: safeFloat(node.CPU), // Proxmox returns 0-1 ratio (e.g., 0.15 = 15%) + Memory: models.UnavailableMemory(clampToInt64(node.MaxMem)), Uptime: int64(node.Uptime), LoadAverage: []float64{}, LastSeen: time.Now(), @@ -56,15 +80,15 @@ func (m *Monitor) pollPVENode( nodeSnapshotRaw := NodeMemoryRaw{ Total: node.MaxMem, Used: node.Mem, - Free: node.MaxMem - node.Mem, + Free: nodeFallbackFree, FallbackTotal: node.MaxMem, FallbackUsed: node.Mem, - FallbackFree: node.MaxMem - node.Mem, + FallbackFree: nodeFallbackFree, FallbackCalculated: true, ProxmoxMemorySource: "nodes-endpoint", } - nodeMemorySource := "nodes-endpoint" - var nodeFallbackReason string + nodeMemorySource := "unavailable" + nodeFallbackReason := "cache-aware-memory-unavailable" // Debug logging for disk metrics - note that these values can fluctuate // due to thin provisioning and dynamic allocation @@ -93,7 +117,7 @@ func (m *Monitor) pollPVENode( Err(nodeErr). Uint64("usingDisk", node.Disk). Uint64("usingMaxDisk", node.MaxDisk). - Msg("Could not get node status - using fallback metrics (memory will include cache/buffers)") + Msg("Could not get node status - disk fallback retained; memory usage unavailable") } else { log.Warn(). Str("instance", instanceName). @@ -101,7 +125,7 @@ func (m *Monitor) pollPVENode( Err(nodeErr). Uint64("disk", node.Disk). Uint64("maxDisk", node.MaxDisk). - Msg("Could not get node status - no fallback metrics available (memory will include cache/buffers)") + Msg("Could not get node status - memory usage unavailable") } } else if nodeInfo != nil { if nodeInfo.Memory != nil { @@ -159,11 +183,9 @@ func (m *Monitor) pollPVENode( if ok { modelNode.Memory = resolvedMemory nodeMemorySource = resolvedSource - if resolvedFallback != "" { - nodeFallbackReason = resolvedFallback - } + nodeFallbackReason = resolvedFallback nodeSnapshotRaw = resolvedRaw - memoryUpdated = true + memoryUpdated = resolvedMemory.HasKnownUsage() } } @@ -195,8 +217,10 @@ func (m *Monitor) pollPVENode( } // If we couldn't update memory metrics using detailed status, preserve previous accurate values if available - if !memoryUpdated && effectiveStatus == "online" { - if prevMem, exists := prevNodeMemory[modelNode.ID]; exists && prevMem.Total > 0 { + if !memoryUpdated && + effectiveStatus == "online" && + m.canCarryForwardNodeMemory(instanceName, node.Node, time.Now()) { + if prevMem, exists := prevNodeMemory[modelNode.ID]; exists && prevMem.HasKnownUsage() { total := int64(node.MaxMem) if total == 0 { total = prevMem.Total @@ -222,9 +246,7 @@ func (m *Monitor) pollPVENode( Str("node", node.Node). Msg("Preserving previous memory metrics - node status unavailable this cycle") - if nodeFallbackReason == "" { - nodeFallbackReason = "preserved-previous-snapshot" - } + nodeFallbackReason = "preserved-previous-snapshot" nodeMemorySource = "previous-snapshot" if nodeSnapshotRaw.ProxmoxMemorySource == "node-status" && nodeSnapshotRaw.Total == 0 { nodeSnapshotRaw.ProxmoxMemorySource = "previous-snapshot" diff --git a/internal/monitoring/monitor_polling_vm.go b/internal/monitoring/monitor_polling_vm.go index fb8e77b3a..6fbc8ace3 100644 --- a/internal/monitoring/monitor_polling_vm.go +++ b/internal/monitoring/monitor_polling_vm.go @@ -124,7 +124,7 @@ func (m *Monitor) pollVMsWithNodes(ctx context.Context, instanceName string, clu } // IO/network series are not recorded on the traditional polling // path (parity with the historical inline writes). - m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), vm.Memory.Usage, vm.Disk.Usage, -1, -1, -1, -1, now) + m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), historyMemoryUsage(vm.Memory), vm.Disk.Usage, -1, -1, -1, -1, now) } } diff --git a/internal/monitoring/monitor_pve_guest_builders.go b/internal/monitoring/monitor_pve_guest_builders.go index 8b2d052c5..b84f9e2d2 100644 --- a/internal/monitoring/monitor_pve_guest_builders.go +++ b/internal/monitoring/monitor_pve_guest_builders.go @@ -316,11 +316,6 @@ func (m *Monitor) buildVMFromClusterResource( state.onBoot = m.fetchVMOnBoot(ctx, client, res.Node, res.VMID) } - memFree := uint64(0) - if state.memTotal >= state.memUsed { - memFree = state.memTotal - state.memUsed - } - var snapshotNotes []string state.memUsed, state.memorySource, snapshotNotes = stabilizeGuestLowTrustMemory( prevSnapshot, @@ -339,6 +334,10 @@ func (m *Monitor) buildVMFromClusterResource( state.agentVersion, ), ) + memFree := uint64(0) + if state.memTotal >= state.memUsed { + memFree = state.memTotal - state.memUsed + } currentMetrics := IOMetrics{ DiskRead: state.diskReadBytes, DiskWrite: state.diskWriteBytes, @@ -348,12 +347,14 @@ func (m *Monitor) buildVMFromClusterResource( } diskReadRate, diskWriteRate, netInRate, netOutRate := m.rateTracker.CalculateRates(guestID, currentMetrics) - memoryUsage := safePercentage(float64(state.memUsed), float64(state.memTotal)) - memory := models.Memory{ - Total: int64(state.memTotal), - Used: int64(state.memUsed), - Free: int64(memFree), - Usage: memoryUsage, + memory := models.UnavailableMemory(clampToInt64(state.memTotal)) + if CanonicalMemorySource(state.memorySource) != "unavailable" { + memory = models.Memory{ + Total: int64(state.memTotal), + Used: int64(state.memUsed), + Free: int64(memFree), + Usage: safePercentage(float64(state.memUsed), float64(state.memTotal)), + } } if memory.Free < 0 { memory.Free = 0 @@ -363,7 +364,14 @@ func (m *Monitor) buildVMFromClusterResource( } // Free above is total-used, i.e. available. When the guest reported its // truly-free pages (meminfo), split the reclaimable cache back out. - splitReclaimableMemory(&memory, state.guestRaw.MemInfoFree) + var trulyFree uint64 + switch CanonicalMemorySource(state.memorySource) { + case "available-field", "derived-free-buffers-cached", "derived-total-minus-used": + trulyFree = state.guestRaw.MemInfoFree + case "guest-agent-meminfo", "guest-agent-meminfo-derived": + trulyFree = state.guestRaw.GuestAgentMemFree + } + splitReclaimableMemory(&memory, trulyFree) if state.detailedStatus != nil && state.detailedStatus.Balloon > 0 { memory.Balloon = int64(state.detailedStatus.Balloon) } diff --git a/internal/monitoring/monitor_pve_guest_helpers.go b/internal/monitoring/monitor_pve_guest_helpers.go index fbdbb6f4a..f156349d1 100644 --- a/internal/monitoring/monitor_pve_guest_helpers.go +++ b/internal/monitoring/monitor_pve_guest_helpers.go @@ -110,16 +110,23 @@ func (m *Monitor) recordGuestMetrics(allVMs []models.VM, allContainers []models. now := time.Now() for _, vm := range allVMs { if vm.Status == "running" { - m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), vm.Memory.Usage, vm.Disk.Usage, vm.DiskRead, vm.DiskWrite, vm.NetworkIn, vm.NetworkOut, now) + m.recordGuestMetric("vm", vm.ID, unifiedresources.ProxmoxGuestCPUPercent(vm.CPU), historyMemoryUsage(vm.Memory), vm.Disk.Usage, vm.DiskRead, vm.DiskWrite, vm.NetworkIn, vm.NetworkOut, now) } } for _, ct := range allContainers { if ct.Status == "running" { - m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), ct.Memory.Usage, ct.Disk.Usage, ct.DiskRead, ct.DiskWrite, ct.NetworkIn, ct.NetworkOut, now) + m.recordGuestMetric("container", ct.ID, unifiedresources.ProxmoxGuestCPUPercent(ct.CPU), historyMemoryUsage(ct.Memory), ct.Disk.Usage, ct.DiskRead, ct.DiskWrite, ct.NetworkIn, ct.NetworkOut, now) } } } +func historyMemoryUsage(memory models.Memory) float64 { + if !memory.HasKnownUsage() { + return -1 + } + return memory.Usage +} + // recordGuestMetric records metrics for a single guest (VM or container) to both // the in-memory metrics history and the persistent metrics store. func (m *Monitor) recordGuestMetric( @@ -134,7 +141,9 @@ func (m *Monitor) recordGuestMetric( if m.metricsHistory != nil { m.metricsHistory.AddGuestMetric(resourceID, "cpu", cpu, now) - m.metricsHistory.AddGuestMetric(resourceID, "memory", memory, now) + if memory >= 0 { + m.metricsHistory.AddGuestMetric(resourceID, "memory", memory, now) + } if diskUsage >= 0 { m.metricsHistory.AddGuestMetric(resourceID, "disk", diskUsage, now) } @@ -154,7 +163,9 @@ func (m *Monitor) recordGuestMetric( if m.metricsStore != nil { m.metricsStore.Write(resourceType, resourceID, "cpu", cpu, now) - m.metricsStore.Write(resourceType, resourceID, "memory", memory, now) + if memory >= 0 { + m.metricsStore.Write(resourceType, resourceID, "memory", memory, now) + } if diskUsage >= 0 { m.metricsStore.Write(resourceType, resourceID, "disk", diskUsage, now) } diff --git a/internal/monitoring/monitor_pve_guest_lxc.go b/internal/monitoring/monitor_pve_guest_lxc.go index cef67dd02..f2e96029a 100644 --- a/internal/monitoring/monitor_pve_guest_lxc.go +++ b/internal/monitoring/monitor_pve_guest_lxc.go @@ -2,6 +2,7 @@ package monitoring import ( "context" + "math" "strings" "time" @@ -20,8 +21,8 @@ func (m *Monitor) calculateLXCMemory( // The cluster resources API returns mem from cgroup which includes cache/buffers (inflated). // Try to get more accurate memory metrics from RRD data. memTotal := res.MaxMem - memUsed := res.Mem - memorySource := "cluster-resources" + memUsed := uint64(0) + memorySource := "powered-off" guestRaw := VMMemoryRaw{ ListingMem: res.Mem, ListingMaxMem: res.MaxMem, @@ -29,6 +30,7 @@ func (m *Monitor) calculateLXCMemory( // For running containers, try to get RRD data for cache-aware memory calculation if res.Status == "running" { + memorySource = "unavailable" rrdCtx, rrdCancel := context.WithTimeout(ctx, 5*time.Second) rrdPoints, err := client.GetLXCRRDData(rrdCtx, res.Node, res.VMID, "hour", "AVERAGE", []string{"memavailable", "memused", "maxmem"}) rrdCancel() @@ -37,40 +39,39 @@ func (m *Monitor) calculateLXCMemory( // Use the most recent RRD point point := rrdPoints[len(rrdPoints)-1] - if point.MaxMem != nil && *point.MaxMem > 0 { - guestRaw.StatusMaxMem = uint64(*point.MaxMem) + if point.MaxMem != nil && !math.IsNaN(*point.MaxMem) && !math.IsInf(*point.MaxMem, 0) && *point.MaxMem > 0 && *point.MaxMem <= math.MaxUint64 { + guestRaw.RRDMaxMem = uint64(*point.MaxMem) + if memTotal == 0 { + memTotal = uint64(*point.MaxMem) + } } // Prefer memavailable-based calculation (excludes cache/buffers) - if point.MemAvailable != nil && *point.MemAvailable > 0 { + if point.MemAvailable != nil && !math.IsNaN(*point.MemAvailable) && !math.IsInf(*point.MemAvailable, 0) && *point.MemAvailable >= 0 && *point.MemAvailable <= math.MaxUint64 && memTotal > 0 && *point.MemAvailable <= float64(memTotal) { memAvailable := uint64(*point.MemAvailable) - if memAvailable <= memTotal { - memUsed = memTotal - memAvailable - memorySource = "rrd-memavailable" - guestRaw.MemInfoAvailable = memAvailable - log.Debug(). - Str("container", res.Name). - Str("node", res.Node). - Uint64("total", memTotal). - Uint64("available", memAvailable). - Uint64("used", memUsed). - Float64("usage", safePercentage(float64(memUsed), float64(memTotal))). - Msg("LXC memory: using RRD memavailable (excludes reclaimable cache)") - } - } else if point.MemUsed != nil && *point.MemUsed > 0 { + memUsed = memTotal - memAvailable + memorySource = "rrd-memavailable" + guestRaw.RRDMemAvailable = memAvailable + log.Debug(). + Str("container", res.Name). + Str("node", res.Node). + Uint64("total", memTotal). + Uint64("available", memAvailable). + Uint64("used", memUsed). + Float64("usage", safePercentage(float64(memUsed), float64(memTotal))). + Msg("LXC memory: using RRD memavailable (excludes reclaimable cache)") + } else if point.MemUsed != nil && !math.IsNaN(*point.MemUsed) && !math.IsInf(*point.MemUsed, 0) && *point.MemUsed >= 0 && *point.MemUsed <= math.MaxUint64 && memTotal > 0 && *point.MemUsed <= float64(memTotal) { // Fall back to memused from RRD if available memUsed = uint64(*point.MemUsed) - if memUsed <= memTotal { - memorySource = "rrd-memused" - guestRaw.MemInfoUsed = memUsed - log.Debug(). - Str("container", res.Name). - Str("node", res.Node). - Uint64("total", memTotal). - Uint64("used", memUsed). - Float64("usage", safePercentage(float64(memUsed), float64(memTotal))). - Msg("LXC memory: using RRD memused (excludes reclaimable cache)") - } + memorySource = "rrd-memused" + guestRaw.RRDMemUsed = memUsed + log.Debug(). + Str("container", res.Name). + Str("node", res.Node). + Uint64("total", memTotal). + Uint64("used", memUsed). + Float64("usage", safePercentage(float64(memUsed), float64(memTotal))). + Msg("LXC memory: using RRD memused (excludes reclaimable cache)") } } else if err != nil { log.Debug(). @@ -78,7 +79,7 @@ func (m *Monitor) calculateLXCMemory( Str("instance", instanceName). Str("container", res.Name). Int("vmid", res.VMID). - Msg("RRD memory data unavailable for LXC, using cluster resources value") + Msg("RRD memory data unavailable for LXC; memory usage remains unavailable") } } @@ -122,6 +123,15 @@ func (m *Monitor) buildContainerFromClusterResource( diskReadRate, diskWriteRate, netInRate, netOutRate := m.rateTracker.CalculateRates(guestID, currentMetrics) memTotal, memUsed, memorySource, guestRaw := m.calculateLXCMemory(ctx, instanceName, res, client) + memUsed, memorySource, _ = stabilizeGuestLowTrustMemory( + m.previousGuestSnapshot(instanceName, "lxc", res.Node, res.VMID), + res.Status, + memorySource, + memTotal, + memUsed, + sampleTime, + false, + ) // Clamp memory and disk values to prevent >100% usage // (Proxmox can report used > total for LXC due to cgroup accounting, @@ -134,6 +144,15 @@ func (m *Monitor) buildContainerFromClusterResource( if memFree < 0 { memFree = 0 } + memory := models.UnavailableMemory(clampToInt64(memTotal)) + if CanonicalMemorySource(memorySource) != "unavailable" { + memory = models.Memory{ + Total: int64(memTotal), + Used: int64(clampedMemUsed), + Free: memFree, + Usage: safePercentage(float64(clampedMemUsed), float64(memTotal)), + } + } diskUsed := res.Disk if diskUsed > res.MaxDisk && res.MaxDisk > 0 { diskUsed = res.MaxDisk @@ -155,12 +174,7 @@ func (m *Monitor) buildContainerFromClusterResource( Type: "lxc", CPU: safeFloat(res.CPU), CPUs: res.MaxCPU, - Memory: models.Memory{ - Total: int64(memTotal), - Used: int64(clampedMemUsed), - Free: memFree, - Usage: safePercentage(float64(clampedMemUsed), float64(memTotal)), - }, + Memory: memory, Disk: models.Disk{ Total: int64(res.MaxDisk), Used: int64(diskUsed), diff --git a/internal/monitoring/monitor_pve_storage.go b/internal/monitoring/monitor_pve_storage.go index c2c0e146c..0b49ad486 100644 --- a/internal/monitoring/monitor_pve_storage.go +++ b/internal/monitoring/monitor_pve_storage.go @@ -196,13 +196,15 @@ func (m *Monitor) applyStorageFallbackAndRecordNodeMetrics( now := time.Now() var nodeNetMetrics *rrdMemCacheEntry if client != nil { - if rrdMetrics, err := m.getNodeRRDMetrics(context.Background(), client, modelNodes[i].Name); err == nil { + if rrdMetrics, err := m.getNodeRRDMetrics(context.Background(), client, instanceName, modelNodes[i].Name); err == nil { nodeNetMetrics = &rrdMetrics } } m.metricsHistory.AddNodeMetric(modelNodes[i].ID, "cpu", modelNodes[i].CPU*100, now) - m.metricsHistory.AddNodeMetric(modelNodes[i].ID, "memory", modelNodes[i].Memory.Usage, now) + if modelNodes[i].Memory.HasKnownUsage() { + m.metricsHistory.AddNodeMetric(modelNodes[i].ID, "memory", modelNodes[i].Memory.Usage, now) + } m.metricsHistory.AddNodeMetric(modelNodes[i].ID, "disk", modelNodes[i].Disk.Usage, now) if nodeNetMetrics != nil { if nodeNetMetrics.hasNetIn { @@ -216,7 +218,9 @@ func (m *Monitor) applyStorageFallbackAndRecordNodeMetrics( // Also write to persistent store if m.metricsStore != nil { m.metricsStore.Write("node", modelNodes[i].ID, "cpu", modelNodes[i].CPU*100, now) - m.metricsStore.Write("node", modelNodes[i].ID, "memory", modelNodes[i].Memory.Usage, now) + if modelNodes[i].Memory.HasKnownUsage() { + m.metricsStore.Write("node", modelNodes[i].ID, "memory", modelNodes[i].Memory.Usage, now) + } m.metricsStore.Write("node", modelNodes[i].ID, "disk", modelNodes[i].Disk.Usage, now) if nodeNetMetrics != nil { if nodeNetMetrics.hasNetIn { diff --git a/internal/monitoring/node_memory_sources.go b/internal/monitoring/node_memory_sources.go index a6f5c1a18..43eac74ae 100644 --- a/internal/monitoring/node_memory_sources.go +++ b/internal/monitoring/node_memory_sources.go @@ -29,11 +29,10 @@ func (m *Monitor) resolveNodeMemory( raw.Buffers = memory.Buffers raw.Cached = memory.Cached raw.Shared = memory.Shared - raw.EffectiveAvailable = memory.EffectiveAvailable() + raw.EffectiveAvailable = 0 raw.ProxmoxMemorySource = "node-status" raw.FallbackCalculated = false - effectiveAvailable := memory.EffectiveAvailable() componentAvailable := memory.Free if memory.Buffers > 0 { if math.MaxUint64-componentAvailable < memory.Buffers { @@ -49,9 +48,7 @@ func (m *Monitor) resolveNodeMemory( componentAvailable += memory.Cached } } - if memory.Total > 0 && componentAvailable > memory.Total { - componentAvailable = memory.Total - } + hasCacheComponents := memory.Buffers > 0 || memory.Cached > 0 availableFromUsed := uint64(0) if memory.Total > 0 && memory.Used > 0 && memory.Total >= memory.Used { @@ -59,27 +56,12 @@ func (m *Monitor) resolveNodeMemory( } raw.TotalMinusUsed = availableFromUsed - missingCacheMetrics := memory.Available == 0 && - memory.Avail == 0 && - memory.Buffers == 0 && - memory.Cached == 0 - var rrdMetrics rrdMemCacheEntry haveRRDMetrics := false - usedRRDAvailableFallback := false - rrdMemUsedFallback := false - - if missingCacheMetrics { - if metrics, err := m.getNodeRRDMetrics(ctx, client, nodeName); err == nil { + if memory.Available == 0 && memory.Avail == 0 && !hasCacheComponents { + if metrics, err := m.getNodeRRDMetrics(ctx, client, instanceName, nodeName); err == nil { haveRRDMetrics = true rrdMetrics = metrics - if metrics.available > 0 { - effectiveAvailable = metrics.available - usedRRDAvailableFallback = true - } - if metrics.used > 0 { - rrdMemUsedFallback = true - } } else if err != nil { log.Debug(). Err(err). @@ -90,98 +72,98 @@ func (m *Monitor) resolveNodeMemory( } const totalMinusUsedGapTolerance uint64 = 16 * 1024 * 1024 - gapGreaterThanComponents := false - if availableFromUsed > componentAvailable { - gap := availableFromUsed - componentAvailable - if componentAvailable == 0 || gap >= totalMinusUsedGapTolerance { - gapGreaterThanComponents = true - } - } - - derivedFromTotalMinusUsed := !usedRRDAvailableFallback && - missingCacheMetrics && - availableFromUsed > 0 && - gapGreaterThanComponents && - effectiveAvailable == availableFromUsed - var actualUsed uint64 + var effectiveAvailable uint64 source := "" fallbackReason := "" switch { - case effectiveAvailable > 0 && effectiveAvailable <= memory.Total: + case memory.Available > 0 && memory.Available <= memory.Total: + effectiveAvailable = memory.Available actualUsed = memory.Total - effectiveAvailable - if actualUsed > memory.Total { - actualUsed = memory.Total - } - - logCtx := log.Debug(). + source = "available-field" + log.Debug(). Str("node", nodeName). Uint64("total", memory.Total). - Uint64("effectiveAvailable", effectiveAvailable). - Uint64("actualUsed", actualUsed). - Float64("usage", safePercentage(float64(actualUsed), float64(memory.Total))) + Uint64("available", effectiveAvailable). + Msg("Node memory: using available field (excludes reclaimable cache)") + + case memory.Avail > 0 && memory.Avail <= memory.Total: + effectiveAvailable = memory.Avail + actualUsed = memory.Total - effectiveAvailable + source = "available-field" + log.Debug(). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("available", effectiveAvailable). + Msg("Node memory: using avail field (excludes reclaimable cache)") + + case hasCacheComponents && componentAvailable <= memory.Total: + effectiveAvailable = componentAvailable + actualUsed = memory.Total - effectiveAvailable + source = "derived-free-buffers-cached" + log.Debug(). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("free", memory.Free). + Uint64("buffers", memory.Buffers). + Uint64("cached", memory.Cached). + Uint64("effectiveAvailable", effectiveAvailable). + Msg("Node memory: deriving availability from free+buffers+cached") + + case haveRRDMetrics && rrdMetrics.hasAvail && rrdMetrics.available <= memory.Total: + effectiveAvailable = rrdMetrics.available + actualUsed = memory.Total - effectiveAvailable + source = "rrd-memavailable" + fallbackReason = "rrd-memavailable" + raw.FallbackCalculated = true + raw.ProxmoxMemorySource = "rrd-memavailable" + log.Debug(). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("rrdAvailable", rrdMetrics.available). + Msg("Node memory: using RRD memavailable fallback") + + case haveRRDMetrics && rrdMetrics.hasUsed && rrdMetrics.used <= memory.Total: + actualUsed = rrdMetrics.used + effectiveAvailable = memory.Total - actualUsed + source = "rrd-memused" + fallbackReason = "rrd-memused" + raw.FallbackCalculated = true + raw.ProxmoxMemorySource = "rrd-memused" + log.Debug(). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("rrdUsed", rrdMetrics.used). + Msg("Node memory: using RRD memused fallback") + + case availableFromUsed > memory.Free && + availableFromUsed-memory.Free >= totalMinusUsedGapTolerance: + effectiveAvailable = availableFromUsed + actualUsed = memory.Used + source = "derived-total-minus-used" + fallbackReason = "node-status-total-minus-used" + raw.FallbackCalculated = true + raw.ProxmoxMemorySource = "node-status-total-minus-used" + log.Debug(). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("availableFromUsed", availableFromUsed). + Uint64("reportedFree", memory.Free). + Msg("Node memory: deriving availability from total-used cache gap") - if usedRRDAvailableFallback { - if haveRRDMetrics && rrdMetrics.available > 0 { - logCtx = logCtx.Uint64("rrdAvailable", rrdMetrics.available) - } - logCtx.Msg("node memory: using RRD memavailable fallback (excludes reclaimable cache)") - source = "rrd-memavailable" - fallbackReason = "rrd-memavailable" - raw.FallbackCalculated = true - raw.ProxmoxMemorySource = "rrd-memavailable" - } else if memory.Available > 0 { - logCtx.Msg("node memory: using available field (excludes reclaimable cache)") - source = "available-field" - } else if memory.Avail > 0 { - logCtx.Msg("node memory: using avail field (excludes reclaimable cache)") - source = "available-field" - } else if derivedFromTotalMinusUsed { - logCtx. - Uint64("availableFromUsed", availableFromUsed). - Uint64("reportedFree", memory.Free). - Msg("Node memory: derived available from total-used gap (cache fields missing)") - source = "derived-total-minus-used" - fallbackReason = "node-status-total-minus-used" - raw.FallbackCalculated = true - raw.ProxmoxMemorySource = "node-status-total-minus-used" - } else { - logCtx. - Uint64("free", memory.Free). - Uint64("buffers", memory.Buffers). - Uint64("cached", memory.Cached). - Msg("Node memory: derived available from free+buffers+cached (excludes reclaimable cache)") - source = "derived-free-buffers-cached" - } default: - switch { - case rrdMemUsedFallback && haveRRDMetrics && rrdMetrics.used > 0: - actualUsed = rrdMetrics.used - if actualUsed > memory.Total { - actualUsed = memory.Total - } - log.Debug(). - Str("node", nodeName). - Uint64("total", memory.Total). - Uint64("rrdUsed", rrdMetrics.used). - Msg("Node memory: using RRD memused fallback (excludes reclaimable cache)") - source = "rrd-memused" - fallbackReason = "rrd-memused" - raw.FallbackCalculated = true - raw.ProxmoxMemorySource = "rrd-memused" - default: - actualUsed = memory.Used - if actualUsed > memory.Total { - actualUsed = memory.Total - } - log.Debug(). - Str("node", nodeName). - Uint64("total", memory.Total). - Uint64("used", actualUsed). - Msg("Node memory: no cache-aware metrics - using traditional calculation (includes cache)") - source = "node-status-used" - } + source = "unavailable" + fallbackReason = "cache-aware-memory-unavailable" + raw.FallbackCalculated = true + raw.ProxmoxMemorySource = "cache-aware-unavailable" + log.Warn(). + Str("instance", instanceName). + Str("node", nodeName). + Uint64("total", memory.Total). + Uint64("used", memory.Used). + Uint64("free", memory.Free). + Msg("Node memory unavailable: no cache-aware source") } raw.EffectiveAvailable = effectiveAvailable @@ -191,6 +173,10 @@ func (m *Monitor) resolveNodeMemory( raw.RRDTotal = rrdMetrics.total } + if source == "unavailable" { + return models.UnavailableMemory(clampToInt64(memory.Total)), source, fallbackReason, raw, true + } + free := int64(memory.Total - actualUsed) if free < 0 { free = 0 diff --git a/internal/monitoring/node_memory_sources_test.go b/internal/monitoring/node_memory_sources_test.go index 05e5c9236..e19ab03ff 100644 --- a/internal/monitoring/node_memory_sources_test.go +++ b/internal/monitoring/node_memory_sources_test.go @@ -80,6 +80,22 @@ func TestResolveNodeMemoryCharacterization(t *testing.T) { wantUsed: 4 * gib, wantRawSource: "rrd-memavailable", }, + { + name: "explicit zero RRD memused is a valid idle sample", + memory: &proxmox.MemoryStatus{ + Total: 8 * gib, + Used: 7 * gib, + Free: 1 * gib, + }, + rrdPoints: []proxmox.NodeRRDPoint{{ + MemTotal: floatPtr(float64(8 * gib)), + MemUsed: floatPtr(0), + }}, + wantSource: "rrd-memused", + wantFallback: "rrd-memused", + wantUsed: 0, + wantRawSource: "rrd-memused", + }, } for _, tt := range tests { diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index 21f43ff3b..3eb62a824 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -66,6 +66,7 @@ func resourceFromProxmoxNode(node models.Node, linkedHost *models.Host) (Resourc TemperatureMonitoringEnabled: cloneBoolPtr(node.TemperatureMonitoringEnabled), PendingUpdatesCheckedAt: zeroTimeToPtr(node.PendingUpdatesCheckedAt), MemoryCache: node.Memory.Cache, + Memory: &node.Memory, SwapUsed: node.Memory.SwapUsed, SwapTotal: node.Memory.SwapTotal, LinkedAgentID: linkedAgentID, @@ -129,35 +130,28 @@ func resourceFromHost(host models.Host) (Resource, ResourceIdentity) { platform := agentRuntimePlatformForHost(host, hostProfile) agent := &AgentData{ - AgentID: host.ID, - AgentVersion: host.AgentVersion, - Hostname: host.Hostname, - MachineID: host.MachineID, - TokenID: host.TokenID, - TokenName: host.TokenName, - TokenHint: host.TokenHint, - TokenLastUsedAt: host.TokenLastUsedAt, - Platform: platform, - HostProfile: hostProfile, - OSName: host.OSName, - OSVersion: host.OSVersion, - KernelVersion: host.KernelVersion, - Architecture: host.Architecture, - CPUCount: host.CPUCount, - LoadAverage: append([]float64(nil), host.LoadAverage...), - UptimeSeconds: host.UptimeSeconds, - IntervalSeconds: host.IntervalSeconds, - Temperature: maxCPUTemp(host.Sensors), - NetworkInterfaces: convertInterfaces(host.NetworkInterfaces), - Disks: convertDisks(host.Disks), - Memory: &AgentMemoryMeta{ - Total: host.Memory.Total, - Used: host.Memory.Used, - Free: host.Memory.Free, - Cache: host.Memory.Cache, - SwapUsed: host.Memory.SwapUsed, - SwapTotal: host.Memory.SwapTotal, - }, + AgentID: host.ID, + AgentVersion: host.AgentVersion, + Hostname: host.Hostname, + MachineID: host.MachineID, + TokenID: host.TokenID, + TokenName: host.TokenName, + TokenHint: host.TokenHint, + TokenLastUsedAt: host.TokenLastUsedAt, + Platform: platform, + HostProfile: hostProfile, + OSName: host.OSName, + OSVersion: host.OSVersion, + KernelVersion: host.KernelVersion, + Architecture: host.Architecture, + CPUCount: host.CPUCount, + LoadAverage: append([]float64(nil), host.LoadAverage...), + UptimeSeconds: host.UptimeSeconds, + IntervalSeconds: host.IntervalSeconds, + Temperature: maxCPUTemp(host.Sensors), + NetworkInterfaces: convertInterfaces(host.NetworkInterfaces), + Disks: convertDisks(host.Disks), + Memory: agentMemoryMetaFromModel(host.Memory), CommandsEnabled: host.CommandsEnabled, OperationReceiptVersion: host.OperationReceiptVersion, ReportIP: host.ReportIP, @@ -1324,6 +1318,7 @@ func resourceFromDockerHost(host models.DockerHost) (Resource, ResourceIdentity) AgentVersion: host.AgentVersion, CPUs: host.CPUs, TotalMemoryBytes: host.TotalMemoryBytes, + Memory: agentMemoryMetaFromModel(host.Memory), UptimeSeconds: host.UptimeSeconds, LoadAverage: append([]float64(nil), host.LoadAverage...), IntervalSeconds: host.IntervalSeconds, @@ -1389,6 +1384,18 @@ func resourceFromDockerHost(host models.DockerHost) (Resource, ResourceIdentity) return resource, identity } +func agentMemoryMetaFromModel(memory models.Memory) *AgentMemoryMeta { + return &AgentMemoryMeta{ + Total: memory.Total, + Used: memory.Used, + Free: memory.Free, + Cache: memory.Cache, + UsageUnavailable: memory.UsageUnavailable, + SwapUsed: memory.SwapUsed, + SwapTotal: memory.SwapTotal, + } +} + func resourceFromPBSInstance(instance models.PBSInstance) (Resource, ResourceIdentity) { name := instance.Name if strings.TrimSpace(name) == "" { @@ -1689,6 +1696,7 @@ func resourceFromVM(vm models.VM) (Resource, ResourceIdentity) { SwapTotal: vm.Memory.SwapTotal, Balloon: vm.Memory.Balloon, MemoryCache: vm.Memory.Cache, + Memory: &vm.Memory, Lock: vm.Lock, } resource := Resource{ @@ -1754,6 +1762,7 @@ func resourceFromContainer(ct models.Container) (Resource, ResourceIdentity) { SwapTotal: ct.Memory.SwapTotal, Balloon: ct.Memory.Balloon, MemoryCache: ct.Memory.Cache, + Memory: &ct.Memory, Lock: ct.Lock, } resource := Resource{ diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index 64f2a2139..828b5c8f4 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -1788,3 +1788,46 @@ func TestPhysicalDiskWearoutFromSMARTAttributesClampsExhaustedMedia(t *testing.T t.Fatalf("wearout remaining = %d, want 0 for percentage-used above 100", got) } } + +func TestResourceAdaptersPreserveUnavailableMemoryAsRawEvidence(t *testing.T) { + unavailable := models.UnavailableMemory(8 << 30) + + hostResource, _ := resourceFromHost(models.Host{ + ID: "agent-1501", + Hostname: "linux-host", + Memory: unavailable, + }) + if hostResource.Metrics == nil || hostResource.Metrics.Memory != nil { + t.Fatalf("agent metrics = %+v, want unavailable memory omitted", hostResource.Metrics) + } + if hostResource.Agent == nil || hostResource.Agent.Memory == nil || + !hostResource.Agent.Memory.UsageUnavailable { + t.Fatalf("agent raw memory = %+v, want explicit unavailable evidence", hostResource.Agent) + } + + vmResource, _ := resourceFromVM(models.VM{ + VMID: 1501, + Name: "linux-guest", + Memory: unavailable, + }) + if vmResource.Metrics == nil || vmResource.Metrics.Memory != nil { + t.Fatalf("Proxmox metrics = %+v, want unavailable memory omitted", vmResource.Metrics) + } + if vmResource.Proxmox == nil || vmResource.Proxmox.Memory == nil || + !vmResource.Proxmox.Memory.UsageUnavailable { + t.Fatalf("Proxmox raw memory = %+v, want explicit unavailable evidence", vmResource.Proxmox) + } + + dockerResource, _ := resourceFromDockerHost(models.DockerHost{ + ID: "docker-1501", + Hostname: "docker-host", + Memory: unavailable, + }) + if dockerResource.Metrics == nil || dockerResource.Metrics.Memory != nil { + t.Fatalf("Docker metrics = %+v, want unavailable memory omitted", dockerResource.Metrics) + } + if dockerResource.Docker == nil || dockerResource.Docker.Memory == nil || + !dockerResource.Docker.Memory.UsageUnavailable { + t.Fatalf("Docker raw memory = %+v, want explicit unavailable evidence", dockerResource.Docker) + } +} diff --git a/internal/unifiedresources/canonical_identity_test.go b/internal/unifiedresources/canonical_identity_test.go index ba1f61b90..90e002cf7 100644 --- a/internal/unifiedresources/canonical_identity_test.go +++ b/internal/unifiedresources/canonical_identity_test.go @@ -75,6 +75,60 @@ func TestRefreshCanonicalIdentityPrefersTargetsAndCanonicalHostData(t *testing.T } } +func TestUnavailableMemoryFacetsDoNotChangeCanonicalIdentity(t *testing.T) { + total := int64(8 << 30) + resource := Resource{ + ID: "vm:proxmox-a:1501", + Type: ResourceTypeVM, + Name: "linux-guest", + Identity: ResourceIdentity{ + Hostnames: []string{"linux-guest.example"}, + }, + Proxmox: &ProxmoxData{ + Instance: "proxmox-a", + VMID: 1501, + }, + Agent: &AgentData{ + AgentID: "agent-1501", + }, + Docker: &DockerData{ + HostSourceID: "docker-1501", + }, + } + + RefreshCanonicalIdentity(&resource) + if resource.Canonical == nil { + t.Fatal("canonical identity is nil") + } + wantPrimaryID := resource.Canonical.PrimaryID + wantHostname := resource.Canonical.Hostname + + resource.Proxmox.Memory = &models.Memory{ + Total: total, + UsageUnavailable: true, + } + resource.Agent.Memory = &AgentMemoryMeta{ + Total: total, + UsageUnavailable: true, + } + resource.Docker.Memory = &AgentMemoryMeta{ + Total: total, + UsageUnavailable: true, + } + RefreshCanonicalIdentity(&resource) + + if resource.Canonical == nil || + resource.Canonical.PrimaryID != wantPrimaryID || + resource.Canonical.Hostname != wantHostname { + t.Fatalf( + "canonical identity = %+v, want primary ID %q and hostname %q unchanged", + resource.Canonical, + wantPrimaryID, + wantHostname, + ) + } +} + func TestVMGuestAgentIncidentUsesCanonicalSourceID(t *testing.T) { vm := models.VM{ ID: "cluster-a:pve-a:101", diff --git a/internal/unifiedresources/clone.go b/internal/unifiedresources/clone.go index 8ffd35968..fe17d5f6f 100644 --- a/internal/unifiedresources/clone.go +++ b/internal/unifiedresources/clone.go @@ -144,6 +144,10 @@ func cloneProxmoxData(in *ProxmoxData) *ProxmoxData { return nil } out := *in + if in.Memory != nil { + memory := *in.Memory + out.Memory = &memory + } out.Temperature = cloneFloat64Ptr(in.Temperature) out.TemperatureDetails = cloneTemperature(in.TemperatureDetails) out.CPUInfo = cloneCPUInfo(in.CPUInfo) @@ -191,6 +195,10 @@ func cloneAgentData(in *AgentData) *AgentData { return nil } out := *in + if in.Memory != nil { + memory := *in.Memory + out.Memory = &memory + } out.Temperature = cloneFloat64Ptr(in.Temperature) out.LoadAverage = cloneFloat64Slice(in.LoadAverage) out.NetworkInterfaces = cloneNetworkInterfaces(in.NetworkInterfaces) @@ -229,6 +237,10 @@ func cloneDockerData(in *DockerData) *DockerData { return nil } out := *in + if in.Memory != nil { + memory := *in.Memory + out.Memory = &memory + } out.OOMKilled = cloneBoolPtr(in.OOMKilled) out.Temperature = cloneFloat64Ptr(in.Temperature) out.LoadAverage = cloneFloat64Slice(in.LoadAverage) diff --git a/internal/unifiedresources/clone_test.go b/internal/unifiedresources/clone_test.go index ce577bdcb..ded86776c 100644 --- a/internal/unifiedresources/clone_test.go +++ b/internal/unifiedresources/clone_test.go @@ -377,6 +377,34 @@ func TestCloneProxmoxData_NetworkInterfaceIsolation(t *testing.T) { } } +func TestCloneProxmoxDataMemoryIsolation(t *testing.T) { + original := &ProxmoxData{ + Memory: &models.Memory{Total: 8 << 30, Used: 4 << 30, Usage: 50}, + } + cloned := cloneProxmoxData(original) + + cloned.Memory.Used = 7 << 30 + if original.Memory.Used != 4<<30 { + t.Error("mutating cloned memory should not affect original") + } +} + +func TestCloneAgentAndDockerMemoryIsolation(t *testing.T) { + agent := &AgentData{Memory: &AgentMemoryMeta{Total: 8 << 30}} + clonedAgent := cloneAgentData(agent) + clonedAgent.Memory.Total = 4 << 30 + if agent.Memory.Total != 8<<30 { + t.Error("mutating cloned agent memory should not affect original") + } + + docker := &DockerData{Memory: &AgentMemoryMeta{Total: 8 << 30}} + clonedDocker := cloneDockerData(docker) + clonedDocker.Memory.Total = 4 << 30 + if docker.Memory.Total != 8<<30 { + t.Error("mutating cloned Docker memory should not affect original") + } +} + // --- cloneStorageMeta --- func TestCloneStorageMeta_Nil(t *testing.T) { diff --git a/internal/unifiedresources/metrics.go b/internal/unifiedresources/metrics.go index c8ae33dc7..f3c401c79 100644 --- a/internal/unifiedresources/metrics.go +++ b/internal/unifiedresources/metrics.go @@ -15,7 +15,7 @@ func metricsFromProxmoxNode(node models.Node) *ResourceMetrics { cpuPercent := percentFromUsage(node.CPU) metrics.CPU = &MetricValue{Value: cpuPercent, Percent: cpuPercent, Unit: "percent", Source: SourceProxmox} - if node.Memory.Total > 0 { + if node.Memory.Total > 0 && node.Memory.HasKnownUsage() { percent := percentFromUsage(node.Memory.Usage) metrics.Memory = &MetricValue{Used: &node.Memory.Used, Total: &node.Memory.Total, Percent: percent, Unit: "bytes", Source: SourceProxmox} } @@ -89,7 +89,7 @@ func buildHostMetricPayload( metrics := &ResourceMetrics{} cpuPercent := percentFromReportedPercent(cpuUsage) metrics.CPU = &MetricValue{Value: cpuPercent, Percent: cpuPercent, Unit: "percent", Source: source} - if memory.Total > 0 { + if memory.Total > 0 && memory.HasKnownUsage() { percent := percentFromReportedPercent(memory.Usage) metrics.Memory = &MetricValue{Used: &memory.Used, Total: &memory.Total, Percent: percent, Unit: "bytes", Source: source} } @@ -186,7 +186,7 @@ func buildVMMetricPayload( metrics := &ResourceMetrics{} cpuPercent := ProxmoxGuestCPUPercent(cpu) metrics.CPU = &MetricValue{Value: cpuPercent, Percent: cpuPercent, Unit: "percent", Source: source} - if memory.Total > 0 { + if memory.Total > 0 && memory.HasKnownUsage() { percent := percentFromUsage(memory.Usage) metrics.Memory = &MetricValue{Used: &memory.Used, Total: &memory.Total, Percent: percent, Unit: "bytes", Source: source} } diff --git a/internal/unifiedresources/metrics_test.go b/internal/unifiedresources/metrics_test.go index d50d33e14..b42486031 100644 --- a/internal/unifiedresources/metrics_test.go +++ b/internal/unifiedresources/metrics_test.go @@ -2,6 +2,7 @@ package unifiedresources import ( "testing" + "time" "github.com/rcourtman/pulse-go-rewrite/internal/models" ) @@ -32,6 +33,92 @@ func TestMetricsFromDockerHostIncludesIORates(t *testing.T) { } } +func TestUnavailableMemoryDoesNotProjectOrOverwriteTrustedCrossSourceMetric(t *testing.T) { + unavailable := models.UnavailableMemory(8 * 1024 * 1024 * 1024) + + for name, metrics := range map[string]*ResourceMetrics{ + "proxmox-node": metricsFromProxmoxNode(models.Node{Memory: unavailable}), + "proxmox-vm": metricsFromVM(models.VM{Memory: unavailable}), + "proxmox-lxc": metricsFromContainer(models.Container{Memory: unavailable}), + "agent-host": metricsFromHost(models.Host{Memory: unavailable}), + "docker-host": metricsFromDockerHost(models.DockerHost{Memory: unavailable}), + } { + t.Run(name, func(t *testing.T) { + if metrics.Memory != nil { + t.Fatalf("Memory = %+v, want nil for unavailable usage", metrics.Memory) + } + }) + } + + trustedTotal := int64(8 * 1024 * 1024 * 1024) + trustedUsed := int64(4 * 1024 * 1024 * 1024) + trusted := &ResourceMetrics{ + Memory: &MetricValue{ + Used: &trustedUsed, + Total: &trustedTotal, + Percent: 50, + Source: SourceAgent, + }, + } + merged := mergeMetrics( + &Resource{Type: ResourceTypeVM, Proxmox: &ProxmoxData{VMID: 1501}}, + trusted, + metricsFromVM(models.VM{Memory: unavailable}), + SourceProxmox, + time.Now().UTC(), + nil, + nil, + ) + if merged.Memory == nil || merged.Memory.Percent != 50 || merged.Memory.Source != SourceAgent { + t.Fatalf("merged memory = %+v, want trusted agent metric preserved", merged.Memory) + } + + resource, _ := resourceFromVM(models.VM{VMID: 1501, Memory: unavailable}) + if resource.Metrics == nil || resource.Metrics.Memory != nil { + t.Fatalf("canonical metrics = %+v, want absent unavailable memory metric", resource.Metrics) + } + if resource.Proxmox == nil || resource.Proxmox.Memory == nil || !resource.Proxmox.Memory.UsageUnavailable { + t.Fatalf("Proxmox memory fallback = %+v, want explicit unavailable capacity", resource.Proxmox) + } + + for name, tc := range map[string]struct { + source DataSource + resource *Resource + }{ + "proxmox": { + source: SourceProxmox, + resource: &Resource{Proxmox: &ProxmoxData{Memory: &unavailable}}, + }, + "agent": { + source: SourceAgent, + resource: &Resource{Agent: &AgentData{Memory: &AgentMemoryMeta{ + Total: unavailable.Total, + UsageUnavailable: true, + }}}, + }, + "docker": { + source: SourceDocker, + resource: &Resource{Docker: &DockerData{Memory: &AgentMemoryMeta{ + Total: unavailable.Total, + UsageUnavailable: true, + }}}, + }, + } { + t.Run("same-source-"+name, func(t *testing.T) { + sameSource := &ResourceMetrics{Memory: &MetricValue{Percent: 50, Source: tc.source}} + cleared := clearUnavailableSourceMemoryMetric(sameSource, tc.resource, tc.source) + if cleared.Memory != nil { + t.Fatalf("Memory = %+v, want same-source stale value cleared", cleared.Memory) + } + }) + } + + dockerResource, _ := resourceFromDockerHost(models.DockerHost{Memory: unavailable}) + if dockerResource.Docker == nil || dockerResource.Docker.Memory == nil || !dockerResource.Docker.Memory.UsageUnavailable { + t.Fatalf("Docker memory fallback = %+v, want explicit unavailable capacity", dockerResource.Docker) + } +} + func TestMetricsFromHostKeepsReportedPercentValues(t *testing.T) { host := models.Host{ CPUUsage: 1, diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index f5a737304..8369819a0 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -3306,6 +3306,7 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou existing.Status = chooseStatus(existing.Status, incoming.Status, source) existing.Metrics = mergeMetrics(existing, existing.Metrics, incoming.Metrics, source, now, existing.SourceStatus, nil) + existing.Metrics = clearUnavailableSourceMemoryMetric(existing.Metrics, &incoming, source) // Prefer agent naming when available if incoming.Name != "" { @@ -3315,6 +3316,35 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou } } +func clearUnavailableSourceMemoryMetric(metrics *ResourceMetrics, incoming *Resource, source DataSource) *ResourceMetrics { + if metrics == nil || metrics.Memory == nil || metrics.Memory.Source != source || incoming == nil { + return metrics + } + + unavailable := false + switch source { + case SourceProxmox: + unavailable = incoming.Proxmox != nil && + incoming.Proxmox.Memory != nil && + incoming.Proxmox.Memory.UsageUnavailable + case SourceAgent: + unavailable = incoming.Agent != nil && + incoming.Agent.Memory != nil && + incoming.Agent.Memory.UsageUnavailable + case SourceDocker: + unavailable = incoming.Docker != nil && + incoming.Docker.Memory != nil && + incoming.Docker.Memory.UsageUnavailable + } + if !unavailable { + return metrics + } + + cleared := *metrics + cleared.Memory = nil + return &cleared +} + func mergeProxmoxData(existing *ProxmoxData, incoming *ProxmoxData) *ProxmoxData { if existing == nil { return incoming @@ -3389,6 +3419,11 @@ func mergeProxmoxData(existing *ProxmoxData, incoming *ProxmoxData) *ProxmoxData if incoming.Balloon != 0 { merged.Balloon = incoming.Balloon } + if incoming.Memory != nil { + memory := *incoming.Memory + merged.Memory = &memory + merged.MemoryCache = incoming.MemoryCache + } if incoming.Lock != "" { merged.Lock = incoming.Lock } diff --git a/internal/unifiedresources/registry_merge_policy_test.go b/internal/unifiedresources/registry_merge_policy_test.go index f5fa23f7f..ba1729d5f 100644 --- a/internal/unifiedresources/registry_merge_policy_test.go +++ b/internal/unifiedresources/registry_merge_policy_test.go @@ -4,6 +4,8 @@ import ( "strings" "testing" "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" ) func TestLinkedMergeAllowsOneSidedNodeHostLinkWhenHostnameCorroborates(t *testing.T) { @@ -417,3 +419,72 @@ func containsDataSource(sources []DataSource, want DataSource) bool { } return false } + +func TestRegistryMemoryUnavailableClearsOnlySameSourceMetric(t *testing.T) { + total := int64(8 << 30) + used := total / 2 + unavailable := &AgentMemoryMeta{ + Total: total, + UsageUnavailable: true, + } + lastSeen := time.Now().UTC() + + registry := NewRegistry(nil) + registry.IngestRecords(SourceAgent, []IngestRecord{{ + SourceID: "agent-1501", + Resource: Resource{ + Type: ResourceTypeAgent, + Name: "linux-host", + Status: StatusOnline, + LastSeen: lastSeen, + Metrics: &ResourceMetrics{Memory: &MetricValue{ + Used: &used, + Total: &total, + Percent: 50, + Source: SourceAgent, + }}, + Agent: &AgentData{Memory: &AgentMemoryMeta{Total: total, Used: used}}, + }, + }}) + registry.IngestRecords(SourceAgent, []IngestRecord{{ + SourceID: "agent-1501", + Resource: Resource{ + Type: ResourceTypeAgent, + Name: "linux-host", + Status: StatusOnline, + LastSeen: lastSeen.Add(time.Second), + Metrics: &ResourceMetrics{}, + Agent: &AgentData{Memory: unavailable}, + }, + }}) + + resources := registry.ListByType(ResourceTypeAgent) + if len(resources) != 1 || resources[0].Metrics == nil || resources[0].Metrics.Memory != nil { + t.Fatalf("same-source resources = %+v, want stale memory metric cleared", resources) + } + + proxmoxUnavailable := &Resource{ + Type: ResourceTypeVM, + Proxmox: &ProxmoxData{Memory: &models.Memory{Total: total, UsageUnavailable: true}}, + Metrics: &ResourceMetrics{}, + LastSeen: lastSeen.Add(2 * time.Second), + } + trusted := &ResourceMetrics{Memory: &MetricValue{ + Used: &used, + Total: &total, + Percent: 50, + Source: SourceAgent, + }} + merged := mergeMetrics( + proxmoxUnavailable, + trusted, + proxmoxUnavailable.Metrics, + SourceProxmox, + proxmoxUnavailable.LastSeen, + nil, + nil, + ) + if merged.Memory == nil || merged.Memory.Percent != 50 || merged.Memory.Source != SourceAgent { + t.Fatalf("cross-source memory = %+v, want trusted agent metric preserved", merged.Memory) + } +} diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index ce92d662f..4c50d3e25 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -371,8 +371,9 @@ type ProxmoxData struct { SwapTotal int64 `json:"swapTotal,omitempty"` Balloon int64 `json:"balloon,omitempty"` // Reclaimable buff/cache split out of the memory metric's free bytes. - MemoryCache int64 `json:"memoryCache,omitempty"` - Lock string `json:"lock,omitempty"` // Proxmox lock state (e.g. "backup", "migrate", "snapshot") + MemoryCache int64 `json:"memoryCache,omitempty"` + Memory *models.Memory `json:"memory,omitempty"` + Lock string `json:"lock,omitempty"` // Proxmox lock state (e.g. "backup", "migrate", "snapshot") // Internal link hint to a host agent resource. LinkedAgentID string `json:"-"` } @@ -802,9 +803,10 @@ type AgentMemoryMeta struct { Used int64 `json:"used,omitempty"` Free int64 `json:"free,omitempty"` // Cache is the reclaimable page cache; used + cache + free ≈ total. - Cache int64 `json:"cache,omitempty"` - SwapUsed int64 `json:"swapUsed,omitempty"` - SwapTotal int64 `json:"swapTotal,omitempty"` + Cache int64 `json:"cache,omitempty"` + UsageUnavailable bool `json:"usageUnavailable,omitempty"` + SwapUsed int64 `json:"swapUsed,omitempty"` + SwapTotal int64 `json:"swapTotal,omitempty"` } const HostPackageUpdateFreshness = 45 * time.Minute @@ -989,31 +991,32 @@ type DockerPodmanContainerMeta struct { // DockerData contains Docker host- and container-specific data. type DockerData struct { - HostSourceID string `json:"hostSourceId,omitempty"` // raw model ID for the docker host - AgentID string `json:"agentId,omitempty"` - ContainerID string `json:"containerId,omitempty"` - Hostname string `json:"hostname,omitempty"` - DisplayName string `json:"displayName,omitempty"` - CustomDisplayName string `json:"customDisplayName,omitempty"` - MachineID string `json:"machineId,omitempty"` - Image string `json:"image,omitempty"` - Temperature *float64 `json:"temperature,omitempty"` - Runtime string `json:"runtime,omitempty"` - RuntimeVersion string `json:"runtimeVersion,omitempty"` - DockerVersion string `json:"dockerVersion,omitempty"` - OS string `json:"os,omitempty"` - KernelVersion string `json:"kernelVersion,omitempty"` - Architecture string `json:"architecture,omitempty"` - AgentVersion string `json:"agentVersion,omitempty"` - CPUs int `json:"cpus,omitempty"` - TotalMemoryBytes int64 `json:"totalMemoryBytes,omitempty"` - UptimeSeconds int64 `json:"uptimeSeconds,omitempty"` - LoadAverage []float64 `json:"loadAverage,omitempty"` - IntervalSeconds int `json:"intervalSeconds,omitempty"` - NetInRate float64 `json:"netInRate,omitempty"` - NetOutRate float64 `json:"netOutRate,omitempty"` - DiskReadRate float64 `json:"diskReadRate,omitempty"` - DiskWriteRate float64 `json:"diskWriteRate,omitempty"` + HostSourceID string `json:"hostSourceId,omitempty"` // raw model ID for the docker host + AgentID string `json:"agentId,omitempty"` + ContainerID string `json:"containerId,omitempty"` + Hostname string `json:"hostname,omitempty"` + DisplayName string `json:"displayName,omitempty"` + CustomDisplayName string `json:"customDisplayName,omitempty"` + MachineID string `json:"machineId,omitempty"` + Image string `json:"image,omitempty"` + Temperature *float64 `json:"temperature,omitempty"` + Runtime string `json:"runtime,omitempty"` + RuntimeVersion string `json:"runtimeVersion,omitempty"` + DockerVersion string `json:"dockerVersion,omitempty"` + OS string `json:"os,omitempty"` + KernelVersion string `json:"kernelVersion,omitempty"` + Architecture string `json:"architecture,omitempty"` + AgentVersion string `json:"agentVersion,omitempty"` + CPUs int `json:"cpus,omitempty"` + TotalMemoryBytes int64 `json:"totalMemoryBytes,omitempty"` + Memory *AgentMemoryMeta `json:"memory,omitempty"` + UptimeSeconds int64 `json:"uptimeSeconds,omitempty"` + LoadAverage []float64 `json:"loadAverage,omitempty"` + IntervalSeconds int `json:"intervalSeconds,omitempty"` + NetInRate float64 `json:"netInRate,omitempty"` + NetOutRate float64 `json:"netOutRate,omitempty"` + DiskReadRate float64 `json:"diskReadRate,omitempty"` + DiskWriteRate float64 `json:"diskWriteRate,omitempty"` // Host-level summary fields (populated when Resource.Type == ResourceTypeAgent and Docker != nil) ContainerCount int `json:"containerCount,omitempty"` diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 1f47d567a..57a709a27 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -734,34 +734,45 @@ func (m *MemoryStatus) UnmarshalJSON(data []byte) error { // EffectiveAvailable returns the best-effort estimate of reclaimable memory. // Prefer the dedicated "available"/"avail" fields when present, otherwise derive -// from free + buffers + cached which mirrors Linux's MemAvailable calculation. +// from free + buffers + cached when reclaimable-cache evidence is present. +// MemFree by itself is not an availability estimate. func (m *MemoryStatus) EffectiveAvailable() uint64 { if m == nil { return 0 } if m.Available > 0 { + if m.Total > 0 && m.Available > m.Total { + return 0 + } return m.Available } if m.Avail > 0 { + if m.Total > 0 && m.Avail > m.Total { + return 0 + } return m.Avail } - derived := m.Free + m.Buffers + m.Cached - if m.Total > 0 && m.Used > 0 && m.Total >= m.Used { - availableFromUsed := m.Total - m.Used - if availableFromUsed > derived { - derived = availableFromUsed - } + if m.Buffers == 0 && m.Cached == 0 { + return 0 } + derived := m.Free + for _, value := range []uint64{m.Buffers, m.Cached} { + if math.MaxUint64-derived < value { + derived = math.MaxUint64 + break + } + derived += value + } if derived == 0 { return 0 } - // Cap at total to guard against over-reporting when buffers/cached exceed total. + // Conflicting components are not a usable availability estimate. if m.Total > 0 && derived > m.Total { - return m.Total + return 0 } return derived @@ -2112,13 +2123,124 @@ func vmNetworkInterfaceHasUsefulData(iface VMNetworkInterface) bool { iface.HasIp6Gateway } -// GetVMMemAvailableFromAgent reads /proc/meminfo via the QEMU guest agent's -// file-read endpoint and returns MemAvailable in bytes. -func (c *Client) GetVMMemAvailableFromAgent(ctx context.Context, node string, vmid int) (uint64, error) { +// LinuxMemoryAvailability records the cache-aware fields used to establish a +// Linux guest's available memory. Source is either meminfo-available or +// meminfo-derived. +type LinuxMemoryAvailability struct { + Total uint64 + Free uint64 + Available uint64 + Buffers uint64 + Cached uint64 + SReclaimable uint64 + Shmem uint64 + SwapTotal uint64 + SwapFree uint64 + EffectiveAvailable uint64 + Source string +} + +// ParseLinuxMemoryAvailability parses /proc/meminfo. New kernels provide +// MemAvailable directly. Older kernels use the conservative cache-aware +// estimate Free + Buffers + Cached + SReclaimable - Shmem. A partial payload +// containing only MemTotal/MemFree is deliberately rejected. +func ParseLinuxMemoryAvailability(content string, truncated bool) (LinuxMemoryAvailability, error) { + var result LinuxMemoryAvailability + seen := make(map[string]struct{}) + fieldsByName := map[string]*uint64{ + "MemTotal": &result.Total, + "MemFree": &result.Free, + "MemAvailable": &result.Available, + "Buffers": &result.Buffers, + "Cached": &result.Cached, + "SReclaimable": &result.SReclaimable, + "Shmem": &result.Shmem, + "SwapTotal": &result.SwapTotal, + "SwapFree": &result.SwapFree, + } + + for _, line := range strings.Split(content, "\n") { + nameValue := strings.SplitN(strings.TrimSpace(line), ":", 2) + if len(nameValue) != 2 { + continue + } + target, ok := fieldsByName[nameValue[0]] + if !ok { + continue + } + if _, duplicate := seen[nameValue[0]]; duplicate { + return LinuxMemoryAvailability{}, fmt.Errorf("duplicate %s field in /proc/meminfo", nameValue[0]) + } + seen[nameValue[0]] = struct{}{} + fields := strings.Fields(nameValue[1]) + if len(fields) < 2 || fields[1] != "kB" { + return LinuxMemoryAvailability{}, fmt.Errorf("invalid %s field in /proc/meminfo", nameValue[0]) + } + kB, err := strconv.ParseUint(fields[0], 10, 64) + if err != nil { + return LinuxMemoryAvailability{}, fmt.Errorf("parse %s value %q: %w", nameValue[0], fields[0], err) + } + if kB > math.MaxUint64/1024 { + return LinuxMemoryAvailability{}, fmt.Errorf("%s value %d kB overflows uint64", nameValue[0], kB) + } + *target = kB * 1024 + } + + if _, present := seen["MemAvailable"]; present { + if _, ok := seen["MemTotal"]; !ok || result.Total == 0 { + return LinuxMemoryAvailability{}, fmt.Errorf("MemAvailable has no valid MemTotal") + } + if result.Available > result.Total { + return LinuxMemoryAvailability{}, fmt.Errorf("MemAvailable exceeds MemTotal") + } + result.EffectiveAvailable = result.Available + result.Source = "meminfo-available" + return result, nil + } + + if truncated { + return LinuxMemoryAvailability{}, fmt.Errorf("truncated /proc/meminfo has no MemAvailable") + } + for _, required := range []string{"MemTotal", "MemFree", "Buffers", "Cached"} { + if _, ok := seen[required]; !ok { + return LinuxMemoryAvailability{}, fmt.Errorf("old-kernel /proc/meminfo missing %s", required) + } + } + if result.Total == 0 || result.Free == 0 && result.Buffers == 0 && result.Cached == 0 && result.SReclaimable == 0 { + return LinuxMemoryAvailability{}, fmt.Errorf("insufficient cache-aware fields in /proc/meminfo") + } + if result.Buffers == 0 && result.Cached == 0 && result.SReclaimable == 0 { + return LinuxMemoryAvailability{}, fmt.Errorf("MemAvailable and reclaimable cache fields not found in /proc/meminfo") + } + + available := result.Free + for _, value := range []uint64{result.Buffers, result.Cached, result.SReclaimable} { + if math.MaxUint64-available < value { + available = math.MaxUint64 + break + } + available += value + } + if result.Shmem >= available { + available = 0 + } else { + available -= result.Shmem + } + if available > result.Total { + available = result.Total + } + result.EffectiveAvailable = available + result.Source = "meminfo-derived" + return result, nil +} + +// GetVMMemoryAvailabilityFromAgent reads /proc/meminfo via the QEMU guest +// agent and returns the raw evidence plus the selected cache-aware value. +func (c *Client) GetVMMemoryAvailabilityFromAgent(ctx context.Context, node string, vmid int) (LinuxMemoryAvailability, error) { fileParam := url.QueryEscape("/proc/meminfo") resp, err := c.get(ctx, fmt.Sprintf("/nodes/%s/qemu/%d/agent/file-read?file=%s", node, vmid, fileParam)) if err != nil { - return 0, fmt.Errorf("guest agent file-read /proc/meminfo: %w", err) + return LinuxMemoryAvailability{}, fmt.Errorf("guest agent file-read /proc/meminfo: %w", err) } defer resp.Body.Close() @@ -2129,29 +2251,25 @@ func (c *Client) GetVMMemAvailableFromAgent(ctx context.Context, node string, vm } `json:"data"` } if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return 0, fmt.Errorf("decode file-read response: %w", err) + return LinuxMemoryAvailability{}, fmt.Errorf("decode file-read response: %w", err) } - for _, line := range strings.Split(result.Data.Content, "\n") { - line = strings.TrimSpace(line) - if !strings.HasPrefix(line, "MemAvailable:") { - continue - } - fields := strings.Fields(line) - if len(fields) < 2 { - continue - } - kB, err := strconv.ParseUint(fields[1], 10, 64) - if err != nil { - return 0, fmt.Errorf("parse MemAvailable value %q: %w", fields[1], err) - } - if kB > math.MaxUint64/1024 { - return 0, fmt.Errorf("MemAvailable value %d kB overflows uint64", kB) - } - return kB * 1024, nil + truncated := result.Data.Truncated != nil && *result.Data.Truncated + availability, err := ParseLinuxMemoryAvailability(result.Data.Content, truncated) + if err != nil { + return LinuxMemoryAvailability{}, err } + return availability, nil +} - return 0, fmt.Errorf("MemAvailable not found in /proc/meminfo") +// GetVMMemAvailableFromAgent is retained for callers that only need the +// selected availability value. +func (c *Client) GetVMMemAvailableFromAgent(ctx context.Context, node string, vmid int) (uint64, error) { + availability, err := c.GetVMMemoryAvailabilityFromAgent(ctx, node, vmid) + if err != nil { + return 0, err + } + return availability.EffectiveAvailable, nil } // GetVMStatus returns detailed VM status including balloon info diff --git a/pkg/proxmox/client_test.go b/pkg/proxmox/client_test.go index 31da86d85..28c2abc97 100644 --- a/pkg/proxmox/client_test.go +++ b/pkg/proxmox/client_test.go @@ -435,9 +435,14 @@ func TestMemoryStatusEffectiveAvailable(t *testing.T) { want: 24 * 1024, }, { - name: "caps derived value at total", + name: "rejects conflicting components above total", status: MemoryStatus{Total: 8 * 1024, Free: 4 * 1024, Buffers: 4 * 1024, Cached: 4 * 1024}, - want: 8 * 1024, + want: 0, + }, + { + name: "rejects explicit available above total", + status: MemoryStatus{Total: 8 * 1024, Available: 9 * 1024}, + want: 0, }, { name: "returns zero when no data", @@ -455,6 +460,118 @@ func TestMemoryStatusEffectiveAvailable(t *testing.T) { } } +func TestParseLinuxMemoryAvailability(t *testing.T) { + tests := []struct { + name string + content string + truncated bool + wantSource string + wantAvailable uint64 + wantErr bool + }{ + { + name: "issue 1501 uses explicit MemAvailable without swap", + content: `MemTotal: 7796964 kB +MemFree: 444824 kB +MemAvailable: 1872820 kB +Buffers: 51360 kB +Cached: 1580464 kB +SReclaimable: 191812 kB +Shmem: 31200 kB`, + wantSource: "meminfo-available", + wantAvailable: 1872820 * 1024, + }, + { + name: "explicit zero MemAvailable is valid full pressure", + content: `MemTotal: 8388608 kB +MemFree: 0 kB +MemAvailable: 0 kB`, + wantSource: "meminfo-available", + wantAvailable: 0, + }, + { + name: "old kernel derives cache aware availability", + content: `MemTotal: 8388608 kB +MemFree: 524288 kB +Buffers: 262144 kB +Cached: 1572864 kB +SReclaimable: 262144 kB +Shmem: 131072 kB`, + wantSource: "meminfo-derived", + wantAvailable: (524288 + 262144 + 1572864 + 262144 - 131072) * 1024, + }, + { + name: "partial total and free is unknown", + content: `MemTotal: 8388608 kB +MemFree: 524288 kB`, + wantErr: true, + }, + { + name: "partial old kernel components are unknown even when not marked truncated", + content: `MemTotal: 8388608 kB +MemFree: 524288 kB +Cached: 1572864 kB`, + wantErr: true, + }, + { + name: "truncated old kernel evidence is unknown", + content: `MemTotal: 8388608 kB +MemFree: 524288 kB +Cached: 1572864 kB`, + truncated: true, + wantErr: true, + }, + { + name: "conflicting available above total is rejected", + content: `MemTotal: 1024 kB +MemFree: 512 kB +MemAvailable: 2048 kB`, + wantErr: true, + }, + { + name: "malformed cache field is rejected", + content: `MemTotal: 1024 kB +MemFree: 512 kB +Cached: nope kB`, + wantErr: true, + }, + { + name: "duplicate source evidence is rejected", + content: `MemTotal: 1024 kB +MemAvailable: 512 kB +MemAvailable: 256 kB`, + wantErr: true, + }, + { + name: "non kB units are rejected", + content: `MemTotal: 1024 bytes +MemAvailable: 512 bytes`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseLinuxMemoryAvailability(tt.content, tt.truncated) + if tt.wantErr { + if err == nil { + t.Fatal("ParseLinuxMemoryAvailability() error = nil, want error") + } + return + } + if err != nil { + t.Fatalf("ParseLinuxMemoryAvailability() error = %v", err) + } + if got.Source != tt.wantSource { + t.Fatalf("Source = %q, want %q", got.Source, tt.wantSource) + } + if got.EffectiveAvailable != tt.wantAvailable { + t.Fatalf("EffectiveAvailable = %d, want %d", got.EffectiveAvailable, tt.wantAvailable) + } + }) + } +} + func TestMemoryStatusUnmarshalFlexibleValues(t *testing.T) { tests := []struct { name string @@ -1248,7 +1365,7 @@ func TestMemoryStatusEffectiveAvailable_RegressionIssue435(t *testing.T) { description: "When available/avail missing, derive from free+buffers+cached", }, { - name: "proxmox 8.4 hides cache fields - derive from total-minus-used gap", + name: "proxmox 8.4 hides cache fields - availability remains unknown", status: MemoryStatus{ Total: 134794743808, // ~125.6GB Used: 107351023616, // ~100GB actual usage @@ -1256,9 +1373,9 @@ func TestMemoryStatusEffectiveAvailable_RegressionIssue435(t *testing.T) { Buffers: 0, Cached: 0, }, - wantAvailable: 27443720192, // total - used => ~25.6GB reclaimable (free + cache) - wantUsedPct: 79.6, // Matches Proxmox node dashboard - description: "Proxmox 8.4 stops reporting buffers/cached; use total-used gap to recover cache-aware metric", + wantAvailable: 0, + wantUsedPct: 0, + description: "MemoryStatus alone does not treat total-used or MemFree as MemAvailable", }, { name: "issue #435 specific case - 86% vs 42% real usage", @@ -1292,9 +1409,9 @@ func TestMemoryStatusEffectiveAvailable_RegressionIssue435(t *testing.T) { Used: 6871947674, Free: 0, // All fields missing }, - wantAvailable: 1717986918, // Derived from total - used - wantUsedPct: 80.0, // Still aligns with cache-inclusive calculation when nothing else reported - description: "When all cache fields missing, fall back to total-used gap instead of zero", + wantAvailable: 0, + wantUsedPct: 0, + description: "When all cache-aware fields are missing, preserve an honest unknown value", }, } diff --git a/pkg/proxmox/cluster_client.go b/pkg/proxmox/cluster_client.go index 08585569f..59950f825 100644 --- a/pkg/proxmox/cluster_client.go +++ b/pkg/proxmox/cluster_client.go @@ -1377,6 +1377,21 @@ func (cc *ClusterClient) GetVMMemAvailableFromAgent(ctx context.Context, node st return result, err } +// GetVMMemoryAvailabilityFromAgent reads cache-aware /proc/meminfo evidence +// through the first healthy cluster client. +func (cc *ClusterClient) GetVMMemoryAvailabilityFromAgent(ctx context.Context, node string, vmid int) (LinuxMemoryAvailability, error) { + var result LinuxMemoryAvailability + err := cc.executeWithFailover(ctx, func(client *Client) error { + availability, err := client.GetVMMemoryAvailabilityFromAgent(ctx, node, vmid) + if err != nil { + return err + } + result = availability + return nil + }) + return result, err +} + // GetClusterResources returns all resources (VMs, containers) across the cluster in a single call func (cc *ClusterClient) GetClusterResources(ctx context.Context, resourceType string) ([]ClusterResource, error) { var result []ClusterResource diff --git a/pkg/proxmox/cluster_client_api_test.go b/pkg/proxmox/cluster_client_api_test.go index 8c124fedb..7bfe14f28 100644 --- a/pkg/proxmox/cluster_client_api_test.go +++ b/pkg/proxmox/cluster_client_api_test.go @@ -34,6 +34,38 @@ func TestClusterClient_GetCephStatus(t *testing.T) { } } +func TestClusterClient_GetVMMemoryAvailabilityFromAgent(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api2/json/nodes": + fmt.Fprint(w, `{"data":[{"node":"node1","status":"online"}]}`) + case "/api2/json/nodes/node1/qemu/1501/agent/file-read": + fmt.Fprint(w, `{"data":{"content":"MemTotal: 7796964 kB\nMemFree: 444824 kB\nMemAvailable: 1872820 kB\nCached: 1580464 kB\n","truncated":false}}`) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + cc := NewClusterClient( + "test", + ClientConfig{Host: server.URL, TokenName: "u@p!t", TokenValue: "v"}, + []string{server.URL}, + nil, + ) + availability, err := cc.GetVMMemoryAvailabilityFromAgent(context.Background(), "node1", 1501) + if err != nil { + t.Fatalf("GetVMMemoryAvailabilityFromAgent() error = %v", err) + } + if availability.Source != "meminfo-available" { + t.Fatalf("source = %q, want meminfo-available", availability.Source) + } + if availability.EffectiveAvailable != 1872820*1024 { + t.Fatalf("available = %d, want %d", availability.EffectiveAvailable, 1872820*1024) + } +} + func TestClusterClient_GetVMSnapshots(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json")