From 84f6302450cf39ee5d35ff49fcc77d8b002c1970 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 16 Jun 2026 14:43:18 +0100 Subject: [PATCH] Fix Proxmox cluster snapshot polling read state Refs #1437 --- ...ga-proxmox-cluster-snapshots-2026-06-16.md | 38 ++++++++++++ docs/release-control/v6/internal/status.json | 12 ++++ .../v6/internal/subsystems/monitoring.md | 11 ++++ internal/monitoring/monitor_backups.go | 62 +++++++++++++++++-- .../monitor_backups_readstate_test.go | 46 ++++++++++++++ 5 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md diff --git a/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md b/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md new file mode 100644 index 000000000..390465978 --- /dev/null +++ b/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md @@ -0,0 +1,38 @@ +# Known RC Issue Closure For GA Proxmox Cluster Snapshots Record + +- Date: `2026-06-16` +- Gate: `known-rc-issue-closure-for-ga` +- Issue: `#1437` +- Result: `fixed-local-proof` + +## Context + +Issue `#1437` remained open after the final v5 maintenance release because a +reporter still saw Proxmox guest snapshots on a standalone node but not on a +Proxmox cluster. The reporter plans to retest on the first stable v6 release +instead of exporting more v5 logs. + +That makes the v6 cluster snapshot path part of the known-issue GA floor even +though the original v5 polling starvation fix was already addressed. + +## Disposition + +The v6 PVE backup/snapshot poller now keeps backup inventory reads on the +canonical `unifiedresources.ReadState` contract while refreshing the canonical +resource store from current monitor state when the store-backed read-state has +not yet observed the PVE instance's freshly collected guests. + +This prevents the detached backup/snapshot poll from missing clustered guests +that were collected earlier in the same monitor cycle but had not yet reached +the store-backed read-state view. + +## Proof + +- `go test ./internal/monitoring -run 'TestMonitorPollGuestSnapshots|TestMonitor_PollGuestSnapshots|TestMonitor_PollPVEBackupsAndSnapshots|TestMonitorPollStorageBackupsWithNodes|TestMonitorCalculateBackupOperationTimeout'` + +## Outcome + +Clustered PVE guest snapshot polling no longer depends on a stale resource +store tick before it calls the Proxmox guest snapshot APIs. The reporter can +retest `#1437` against v6 stable without v6 knowingly carrying the cluster +snapshot discovery risk forward from v5. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index eb5c49e68..c4b580d57 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -4553,6 +4553,12 @@ "kind": "file", "evidence_tier": "managed-runtime-exercise" }, + { + "repo": "pulse", + "path": "docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md", + "kind": "file", + "evidence_tier": "test-proof" + }, { "repo": "pulse", "path": "docs/release-control/v6/internal/subsystems/monitoring.md", @@ -6314,6 +6320,12 @@ "kind": "file", "evidence_tier": "managed-runtime-exercise" }, + { + "repo": "pulse", + "path": "docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-cluster-snapshots-2026-06-16.md", + "kind": "file", + "evidence_tier": "test-proof" + }, { "repo": "pulse", "path": "docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-rc3-followup-2026-05-01.md", diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index e149999dd..957969a34 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -75,6 +75,7 @@ truth for live infrastructure data. 51. `internal/truenas/types.go` 52. `internal/monitoring/monitor_alert_sync.go` 53. `internal/monitoring/platform_poller_shared.go` +54. `internal/monitoring/monitor_backups.go` ## Shared Boundaries @@ -149,6 +150,16 @@ truth for live infrastructure data. `PBSBackupsSnapshot()` so PBS size, protection, verification, file, owner, datastore, and namespace facts remain the live PBS poller result carried on `models.PBSBackup`. + Proxmox PVE backup and guest snapshot polling in + `internal/monitoring/monitor_backups.go` must consume the canonical + `unifiedresources.ReadState` shape for guest, storage, and recovery mapping. + If a store-backed read-state has not yet been refreshed for the PVE instance + whose current monitor state already contains guests, backup/snapshot polling + refreshes the canonical resource store from the current state and continues + through the read-state interface. It must not fall back to direct legacy + guest slices as the primary source of truth. Clustered PVE snapshot polling + must therefore see guests collected earlier in the same cycle before calling + the Proxmox guest snapshot APIs. 12. Add or change agentless availability monitoring only through the poll-provider path. `internal/monitoring/availability_poller.go` owns ICMP, TCP, and HTTP probes, provider health, scheduler task construction, and diff --git a/internal/monitoring/monitor_backups.go b/internal/monitoring/monitor_backups.go index 2546edc11..e3c1da293 100644 --- a/internal/monitoring/monitor_backups.go +++ b/internal/monitoring/monitor_backups.go @@ -143,7 +143,7 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName hadPermissionError := false // Track if any permission errors occurred this cycle storagePreserveNeeded := map[string]struct{}{} storageSuccess := map[string]struct{}{} - readState := m.GetUnifiedReadStateOrSnapshot() + readState := m.backupReadStateForInstance(instanceName) // Build guest lookup map to find actual node for each VMID snapshot := m.state.GetSnapshot() @@ -425,6 +425,60 @@ func (m *Monitor) syncGuestBackupTimesAndResourceStore() { m.updateResourceStore(m.state.GetSnapshot()) } +func (m *Monitor) backupReadStateForInstance(instanceName string) unifiedresources.ReadState { + if m == nil { + return nil + } + readState := m.GetUnifiedReadStateOrSnapshot() + if backupReadStateHasGuestForInstance(readState, instanceName) || m.state == nil { + return readState + } + + snapshot := m.state.GetSnapshot() + if !backupSnapshotHasGuestForInstance(snapshot, instanceName) { + return readState + } + + m.updateResourceStore(snapshot) + readState = m.GetUnifiedReadStateOrSnapshot() + if backupReadStateHasGuestForInstance(readState, instanceName) { + return readState + } + + return monitorUnifiedStateViewFromSnapshot(snapshot).readState +} + +func backupReadStateHasGuestForInstance(readState unifiedresources.ReadState, instanceName string) bool { + if readState == nil { + return false + } + for _, vm := range readState.VMs() { + if vm != nil && vm.Instance() == instanceName { + return true + } + } + for _, ct := range readState.Containers() { + if ct != nil && ct.Instance() == instanceName { + return true + } + } + return false +} + +func backupSnapshotHasGuestForInstance(snapshot models.StateSnapshot, instanceName string) bool { + for _, vm := range snapshot.VMs { + if vm.Instance == instanceName { + return true + } + } + for _, ct := range snapshot.Containers { + if ct.Instance == instanceName { + return true + } + } + return false +} + func storageNamesForNode(readState unifiedresources.ReadState, instanceName, nodeName string) []string { if readState == nil || nodeName == "" { return nil @@ -670,7 +724,7 @@ func (m *Monitor) calculateBackupOperationTimeout(instanceName string) time.Dura ) timeout := minTimeout - readState := m.GetUnifiedReadStateOrSnapshot() + readState := m.backupReadStateForInstance(instanceName) guestCount := 0 for _, vm := range readState.VMs() { @@ -735,7 +789,7 @@ func (m *Monitor) pollPVEBackupsAndSnapshots(parentCtx context.Context, instance func (m *Monitor) pollGuestSnapshots(ctx context.Context, instanceName string, client PVEClientInterface) { log.Debug().Str("instance", instanceName).Msg("polling guest snapshots") - readState := m.GetUnifiedReadStateOrSnapshot() + readState := m.backupReadStateForInstance(instanceName) var vms []models.VM for _, vm := range readState.VMs() { if vm == nil || vm.Instance() != instanceName { @@ -1743,7 +1797,7 @@ func (m *Monitor) pollBackupTasks(ctx context.Context, instanceName string, clie m.state.UpdateBackupTasksForInstance(instanceName, backupTasks) // Best-effort ingestion into recovery store (for rollups / unified backups UX). - guestInfo := buildProxmoxGuestInfoIndex(m.GetUnifiedReadStateOrSnapshot()) + guestInfo := buildProxmoxGuestInfoIndex(m.backupReadStateForInstance(instanceName)) m.ingestRecoveryPointsAsync(proxmoxrecoverymapper.FromPVEBackupTasks(backupTasks, guestInfo)) } diff --git a/internal/monitoring/monitor_backups_readstate_test.go b/internal/monitoring/monitor_backups_readstate_test.go index 20fd4dc81..d71e6240f 100644 --- a/internal/monitoring/monitor_backups_readstate_test.go +++ b/internal/monitoring/monitor_backups_readstate_test.go @@ -198,6 +198,52 @@ func TestMonitorPollGuestSnapshots_UsesCanonicalReadState(t *testing.T) { } } +func TestMonitorPollGuestSnapshots_RefreshesStaleCanonicalStoreForClusterGuest(t *testing.T) { + now := time.Date(2026, 6, 16, 10, 0, 0, 0, time.UTC) + state := models.NewState() + state.UpdateVMsForInstance("homelab", []models.VM{{ + ID: "homelab-pve-a-100", + VMID: 100, + Name: "prod-vm", + Node: "pve-a", + Instance: "homelab", + Status: "running", + LastSeen: now, + }}) + + adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil)) + m := &Monitor{ + state: state, + resourceStore: adapter, + } + + client := &backupStorageTimeoutSnapshotClient{ + snapshots: []pveapi.Snapshot{{ + Name: "cluster-snap", + SnapTime: now.Unix(), + Description: "from fresh clustered guest state", + }}, + } + + m.pollGuestSnapshots(context.Background(), "homelab", client) + + if client.snapshotCalls == 0 { + t.Fatal("expected guest snapshot polling to use fresh clustered guest state") + } + + snapshot := state.GetSnapshot() + if len(snapshot.PVEBackups.GuestSnapshots) != 1 { + t.Fatalf("expected one guest snapshot from fresh clustered guest state, got %+v", snapshot.PVEBackups.GuestSnapshots) + } + if got := snapshot.PVEBackups.GuestSnapshots[0]; got.Name != "cluster-snap" || got.Node != "pve-a" || got.Instance != "homelab" { + t.Fatalf("unexpected guest snapshot: %+v", got) + } + + if vms := adapter.VMs(); len(vms) != 1 || vms[0].Instance() != "homelab" || vms[0].Node() != "pve-a" { + t.Fatalf("expected canonical store to refresh from fresh clustered guest state, got %+v", vms) + } +} + func TestMonitorPollStorageBackupsWithNodes_UsesCanonicalReadStateForGuestNodeLookup(t *testing.T) { m := &Monitor{ state: models.NewState(),