From 8177ee1788a710f06f5b445b8fae42201b7915db Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 29 May 2026 11:44:29 +0100 Subject: [PATCH] Fix Proxmox guest memory fallback Prefer QEMU guest-agent MemAvailable when Proxmox reports saturated VM memory without guest free fields. Add regression coverage for the issue #1319 Windows fsinfo volume payload so usable C/E/F volumes remain counted while System Reserved partitions are skipped. Refs #1319 --- .../v6/internal/subsystems/monitoring.md | 7 ++ .../monitoring/canonical_guardrails_test.go | 7 +- .../monitoring/guest_memory_agent_test.go | 55 +++++++++++++ internal/monitoring/guest_memory_sources.go | 79 +++++++++++++++---- .../memory_trust_characterization_test.go | 74 +++++++++++++++-- .../monitoring/monitor_extra_coverage_test.go | 53 +++++++++++++ pkg/proxmox/client_api_more3_test.go | 36 +++++++++ 7 files changed, 288 insertions(+), 23 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 5311ab065..6774a286b 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1254,6 +1254,13 @@ shared guest metadata cache must keep VM network and identity metadata alive long enough to survive short Proxmox status failures, while incomplete guest-agent metadata stays on a short retry cadence instead of freezing partial VM summary data for minutes. +When Proxmox reports saturated VM memory without `meminfo` or `freemem` but +the QEMU guest agent is queryable, the monitoring memory selector must prefer +the guest's own `/proc/meminfo` `MemAvailable` signal before lower-trust +Proxmox RRD or status fallbacks. Guest-agent filesystem payloads from Windows +volume GUID mounts remain part of the same canonical VM disk metric path and +must not be dropped just because system-reserved partitions share a physical +disk with usable volumes. That same monitoring boundary now also owns physical-disk I/O history as a first-class canonical metric stream. `internal/monitoring/monitor_agents.go` must project host per-device I/O counters onto the same SMART-resolved disk diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 5e47de82f..7978b8404 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -659,10 +659,13 @@ func TestProxmoxGuestMemoryFallbackUsesInstanceScopedCachesAndAgentMeminfo(t *te "ttl = vmAgentMemNegativeTTL", }, "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, agentErr := m.getVMAgentMemAvailable(ctx, client, instanceName, node, vmid); agentErr == nil && agentAvailable > 0 {", + "if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok {", `memorySource = "guest-agent-meminfo"`, - "guestRaw.GuestAgentMemAvailable = memAvailable", + "guestRaw.GuestAgentMemAvailable = agentAvailable", }, "monitor.go": { "func (m *Monitor) getVMRRDMetrics(ctx context.Context, client PVEClientInterface, instanceName, node string, vmid int) (uint64, error) {", diff --git a/internal/monitoring/guest_memory_agent_test.go b/internal/monitoring/guest_memory_agent_test.go index 72047e6d0..78155d0dd 100644 --- a/internal/monitoring/guest_memory_agent_test.go +++ b/internal/monitoring/guest_memory_agent_test.go @@ -159,3 +159,58 @@ func TestResolveGuestStatusMemoryUsesGuestAgentMeminfoFallback(t *testing.T) { t.Fatalf("expected guest agent meminfo fallback to be queried once, got %d calls", client.memCalls) } } + +func TestResolveGuestStatusMemoryPrefersGuestAgentMeminfoForSaturatedStatus(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{}, + vmRRDPoints: []proxmox.GuestRRDPoint{{MemAvailable: floatPtr(float64(512 * 1024 * 1024))}}, + memAvailable: 4 * giB, + } + raw := &VMMemoryRaw{} + + memTotal, memUsed, source := mon.resolveGuestStatusMemory( + context.Background(), + client, + "cluster-a", + "linux-vm", + "pve2", + 164, + "cluster-a:pve2:164", + &proxmox.VMStatus{ + Agent: proxmox.VMAgentField{Value: 1}, + MaxMem: 16 * giB, + Mem: 16*giB + 512*1024*1024, + }, + nil, + 16*giB, + "cluster-resources", + raw, + ) + + if memTotal != 16*giB { + t.Fatalf("memTotal = %d, want %d", memTotal, uint64(16*giB)) + } + if memUsed != 12*giB { + t.Fatalf("memUsed = %d, want %d", memUsed, uint64(12*giB)) + } + if source != "guest-agent-meminfo" { + t.Fatalf("source = %q, want guest-agent-meminfo", source) + } + if raw.GuestAgentMemAvailable != 4*giB { + t.Fatalf("raw.GuestAgentMemAvailable = %d, want %d", raw.GuestAgentMemAvailable, uint64(4*giB)) + } + if client.memCalls != 1 { + t.Fatalf("expected guest agent meminfo to be queried once, got %d calls", client.memCalls) + } + if client.rrdCalls != 0 { + t.Fatalf("expected saturated guest-agent memory path to skip RRD, got %d RRD calls", client.rrdCalls) + } +} diff --git a/internal/monitoring/guest_memory_sources.go b/internal/monitoring/guest_memory_sources.go index 109d84d88..2a6175386 100644 --- a/internal/monitoring/guest_memory_sources.go +++ b/internal/monitoring/guest_memory_sources.go @@ -168,10 +168,59 @@ func selectGuestLowTrustUsedMemory(memTotal uint64, status *proxmox.VMStatus) (u return 0, "" } +func shouldPreferGuestAgentMemAvailable(status *proxmox.VMStatus, memTotal uint64) bool { + if status == nil || !status.Agent.IsAvailable() || status.Mem == 0 { + return false + } + if guestStatusFreeMem(status) > 0 { + return false + } + + effectiveTotal := memTotal + if status.MaxMem > 0 { + effectiveTotal = status.MaxMem + } + if effectiveTotal == 0 { + return false + } + if status.Mem >= effectiveTotal { + return true + } + + return effectiveTotal-status.Mem <= guestStatusMemoryMismatchTolerance +} + func guestMemoryFallbackReason(source string) string { return MemorySourceFallbackReason(source) } +func (m *Monitor) tryGuestAgentMemAvailable( + ctx context.Context, + client PVEClientInterface, + instanceName string, + guestName string, + node string, + 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 + } + if guestRaw != nil { + guestRaw.GuestAgentMemAvailable = agentAvailable + } + log.Debug(). + Str("vm", guestName). + Str("node", node). + Int("vmid", vmid). + Uint64("total", memTotal). + Uint64("available", agentAvailable). + Msg("QEMU memory: using guest agent /proc/meminfo fallback (excludes reclaimable cache)") + return agentAvailable, true +} + func (m *Monitor) resolveGuestStatusMemory( ctx context.Context, client PVEClientInterface, @@ -203,6 +252,9 @@ func (m *Monitor) resolveGuestStatusMemory( guestRaw.BalloonInfoActual = status.BalloonInfo.Actual } } + if status.MaxMem > 0 { + memTotal = status.MaxMem + } memAvailable := uint64(0) if status.MemInfo != nil { @@ -219,6 +271,15 @@ func (m *Monitor) resolveGuestStatusMemory( } } + triedGuestAgentMemAvailable := false + if memAvailable == 0 && shouldPreferGuestAgentMemAvailable(status, memTotal) { + triedGuestAgentMemAvailable = true + if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { + memAvailable = agentAvailable + memorySource = "guest-agent-meminfo" + } + } + if memAvailable == 0 { if rrdAvailable, rrdErr := m.getVMRRDMetrics(ctx, client, instanceName, node, vmid); rrdErr == nil && rrdAvailable > 0 { memAvailable = rrdAvailable @@ -243,20 +304,10 @@ func (m *Monitor) resolveGuestStatusMemory( } } - if memAvailable == 0 && status.Agent.IsAvailable() { - if agentAvailable, agentErr := m.getVMAgentMemAvailable(ctx, client, instanceName, node, vmid); agentErr == nil && agentAvailable > 0 { + if memAvailable == 0 && status.Agent.IsAvailable() && !triedGuestAgentMemAvailable { + if agentAvailable, ok := m.tryGuestAgentMemAvailable(ctx, client, instanceName, guestName, node, vmid, memTotal, guestRaw); ok { memAvailable = agentAvailable memorySource = "guest-agent-meminfo" - if guestRaw != nil { - guestRaw.GuestAgentMemAvailable = memAvailable - } - log.Debug(). - Str("vm", guestName). - Str("node", node). - Int("vmid", vmid). - Uint64("total", memTotal). - Uint64("available", memAvailable). - Msg("QEMU memory: using guest agent /proc/meminfo fallback (excludes reclaimable cache)") } } @@ -286,10 +337,6 @@ func (m *Monitor) resolveGuestStatusMemory( } } - if status.MaxMem > 0 { - memTotal = status.MaxMem - } - memUsed := uint64(0) switch { case memAvailable > 0: diff --git a/internal/monitoring/memory_trust_characterization_test.go b/internal/monitoring/memory_trust_characterization_test.go index a6fb7ebb7..c379ff703 100644 --- a/internal/monitoring/memory_trust_characterization_test.go +++ b/internal/monitoring/memory_trust_characterization_test.go @@ -12,11 +12,14 @@ import ( type vmMemoryTrustStubClient struct { *stubPVEClient - vms []proxmox.VM - containers []proxmox.Container - vmStatus *proxmox.VMStatus - vmRRDPoints []proxmox.GuestRRDPoint - lxcRRDPoints []proxmox.GuestRRDPoint + vms []proxmox.VM + containers []proxmox.Container + vmStatus *proxmox.VMStatus + vmRRDPoints []proxmox.GuestRRDPoint + lxcRRDPoints []proxmox.GuestRRDPoint + vmAgentMemAvailable uint64 + vmRRDCalls int + vmAgentMemCalls int } func (s *vmMemoryTrustStubClient) GetVMs(ctx context.Context, node string) ([]proxmox.VM, error) { @@ -32,6 +35,7 @@ func (s *vmMemoryTrustStubClient) GetVMStatus(ctx context.Context, node string, } func (s *vmMemoryTrustStubClient) GetVMRRDData(ctx context.Context, node string, vmid int, timeframe, cf string, ds []string) ([]proxmox.GuestRRDPoint, error) { + s.vmRRDCalls++ return s.vmRRDPoints, nil } @@ -39,6 +43,11 @@ func (s *vmMemoryTrustStubClient) GetLXCRRDData(ctx context.Context, node string return s.lxcRRDPoints, nil } +func (s *vmMemoryTrustStubClient) GetVMMemAvailableFromAgent(ctx context.Context, node string, vmid int) (uint64, error) { + s.vmAgentMemCalls++ + return s.vmAgentMemAvailable, nil +} + func TestPollPVENodeMemoryTrustCharacterization(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) @@ -383,6 +392,61 @@ func TestHandleClusterVMResourceMemoryTrustCharacterization(t *testing.T) { } } +func TestHandleClusterVMResourcePrefersGuestAgentMemAvailableForSaturatedIssue1319Status(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + const gib = uint64(1024 * 1024 * 1024) + + mon := newTestPVEMonitor("test") + defer mon.alertManager.Stop() + defer mon.notificationMgr.Stop() + + client := &vmMemoryTrustStubClient{ + stubPVEClient: &stubPVEClient{}, + vmStatus: &proxmox.VMStatus{ + Status: "running", + MaxMem: 16 * gib, + Mem: 16*gib + 512*1024*1024, + Agent: proxmox.VMAgentField{Value: 1}, + }, + vmRRDPoints: []proxmox.GuestRRDPoint{{MemAvailable: floatPtr(float64(512 * 1024 * 1024))}}, + vmAgentMemAvailable: 4 * gib, + } + res := proxmox.ClusterResource{ + ID: "qemu/164", + Type: "qemu", + Node: "pve2", + Name: "linux-vm", + Status: "running", + VMID: 164, + MaxMem: 16 * gib, + Mem: 16*gib + 512*1024*1024, + MaxCPU: 4, + } + + vm, ok := mon.handleClusterVMResource(context.Background(), "cluster-a", res, makeGuestID("cluster-a", "pve2", 164), client, nil, nil) + if !ok { + t.Fatal("handleClusterVMResource() returned ok=false") + } + if got := uint64(vm.Memory.Used); got != 12*gib { + t.Fatalf("vm.Memory.Used = %d, want %d", got, uint64(12*gib)) + } + + snap := mon.guestSnapshots[makeGuestSnapshotKey("cluster-a", "qemu", "pve2", 164)] + if snap.MemorySource != "guest-agent-meminfo" { + t.Fatalf("snapshot.MemorySource = %q, want guest-agent-meminfo", snap.MemorySource) + } + if snap.Raw.GuestAgentMemAvailable != 4*gib { + t.Fatalf("snapshot.Raw.GuestAgentMemAvailable = %d, want %d", snap.Raw.GuestAgentMemAvailable, uint64(4*gib)) + } + if client.vmAgentMemCalls != 1 { + t.Fatalf("expected guest agent meminfo to be queried once, got %d calls", client.vmAgentMemCalls) + } + if client.vmRRDCalls != 0 { + t.Fatalf("expected saturated guest-agent memory path to skip RRD, got %d RRD calls", client.vmRRDCalls) + } +} + func TestPollVMsWithNodesPreservesProxmoxPool(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) diff --git a/internal/monitoring/monitor_extra_coverage_test.go b/internal/monitoring/monitor_extra_coverage_test.go index 4ff4c3129..c964ff970 100644 --- a/internal/monitoring/monitor_extra_coverage_test.go +++ b/internal/monitoring/monitor_extra_coverage_test.go @@ -1580,6 +1580,59 @@ func TestBuildVMFromClusterResource_PrefersLinkedHostAgentDiskInventoryOverGuest } } +func TestSummarizeVMFSInfoCountsIssue1319WindowsVolumes(t *testing.T) { + monitor := &Monitor{} + fsInfo := []proxmox.VMFileSystem{ + { + Mountpoint: "System Reserved", + Type: "FAT32", + Disk: `\\.\PhysicalDrive0`, + }, + { + Mountpoint: "F:\\", + Type: "NTFS", + TotalBytes: 3298516004864, + UsedBytes: 2671768784896, + Disk: `\\.\PhysicalDrive2`, + }, + { + Mountpoint: "E:\\", + Type: "NTFS", + TotalBytes: 9565733122048, + UsedBytes: 8126376873984, + Disk: `\\.\PhysicalDrive1`, + }, + { + Mountpoint: "C:\\", + Type: "NTFS", + TotalBytes: 267789529088, + UsedBytes: 195096502272, + Disk: `\\.\PhysicalDrive0`, + }, + { + Mountpoint: "System Reserved", + Type: "NTFS", + Disk: `\\.\PhysicalDrive0`, + }, + } + + summary := monitor.summarizeVMFSInfo("cluster-a", proxmox.ClusterResource{Name: "win-vm", VMID: 116}, fsInfo) + + const wantTotal = uint64(3298516004864 + 9565733122048 + 267789529088) + const wantUsed = uint64(2671768784896 + 8126376873984 + 195096502272) + if summary.totalBytes != wantTotal || summary.usedBytes != wantUsed { + t.Fatalf("summary bytes = total %d used %d, want total %d used %d", summary.totalBytes, summary.usedBytes, wantTotal, wantUsed) + } + if len(summary.individualDisks) != 3 { + t.Fatalf("expected 3 usable Windows volumes, got %+v", summary.individualDisks) + } + for _, disk := range summary.individualDisks { + if strings.Contains(disk.Mountpoint, "System Reserved") { + t.Fatalf("expected System Reserved volumes to be skipped, got %+v", summary.individualDisks) + } + } +} + func TestBuildVMFromClusterResource_MarksDiskUnknownWhenGuestFilesystemDataIsUnavailable(t *testing.T) { monitor := &Monitor{ rateTracker: NewRateTracker(), diff --git a/pkg/proxmox/client_api_more3_test.go b/pkg/proxmox/client_api_more3_test.go index dd9cd678b..0628fa6d1 100644 --- a/pkg/proxmox/client_api_more3_test.go +++ b/pkg/proxmox/client_api_more3_test.go @@ -131,6 +131,42 @@ func TestClientVMFSInfoParsingWindowsVolumeGUIDMountpointFallback(t *testing.T) } } +func TestClientVMFSInfoParsingIssue1319WindowsVolumePayload(t *testing.T) { + client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api2/json/nodes/pve7/qemu/116/agent/get-fsinfo": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":{"result":[ + {"disk":[{"dev":"\\\\.\\PhysicalDrive0","serial":"QM00005"}],"mountpoint":"System Reserved","name":"\\\\?\\Volume{cd7c4fae-8d0e-4f80-846b-f7121e12c38d}\\","type":"FAT32"}, + {"disk":[{"dev":"\\\\.\\PhysicalDrive2","serial":"QM00009"}],"mountpoint":"F:\\","name":"\\\\?\\Volume{80ab7d1d-af7b-48fa-86d7-d37aef6ed2ad}\\","total-bytes":3298516004864,"type":"NTFS","used-bytes":2671768784896}, + {"disk":[{"dev":"\\\\.\\PhysicalDrive1","serial":"QM00007"}],"mountpoint":"E:\\","name":"\\\\?\\Volume{5743c199-8613-4953-94a2-574d75a27bfc}\\","total-bytes":9565733122048,"type":"NTFS","used-bytes":8126376873984}, + {"disk":[{"dev":"\\\\.\\PhysicalDrive0","serial":"QM00005"}],"mountpoint":"C:\\","name":"\\\\?\\Volume{c65410ae-abf1-4829-8d8e-81d4b7581949}\\","total-bytes":267789529088,"type":"NTFS","used-bytes":195096502272}, + {"disk":[{"dev":"\\\\.\\PhysicalDrive0","serial":"QM00005"}],"mountpoint":"System Reserved","name":"\\\\?\\Volume{f1b9529f-ff6e-434b-a53a-81687141c733}\\","type":"NTFS"} + ]}}`)) + default: + http.NotFound(w, r) + } + }) + + ctx := context.Background() + filesystems, err := client.GetVMFSInfo(ctx, "pve7", 116) + if err != nil { + t.Fatalf("GetVMFSInfo error: %v", err) + } + if len(filesystems) != 5 { + t.Fatalf("expected 5 filesystems, got %d", len(filesystems)) + } + if filesystems[1].Mountpoint != "F:\\" || filesystems[1].Disk != "\\\\.\\PhysicalDrive2" { + t.Fatalf("expected F: volume with physical disk metadata, got %+v", filesystems[1]) + } + if filesystems[2].Mountpoint != "E:\\" || filesystems[2].Disk != "\\\\.\\PhysicalDrive1" { + t.Fatalf("expected E: volume with physical disk metadata, got %+v", filesystems[2]) + } + if filesystems[3].Mountpoint != "C:\\" || filesystems[3].Disk != "\\\\.\\PhysicalDrive0" { + t.Fatalf("expected C: volume with physical disk metadata, got %+v", filesystems[3]) + } +} + func TestClientVMFSInfoSkipsMalformedEntries(t *testing.T) { client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path {