diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index f89902f89..2cd8be455 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -7536,17 +7536,7 @@ func (m *Monitor) pollPVEInstance(ctx context.Context, instanceName string, clie parentCtx = context.Background() } - backupCtx, cancel := context.WithTimeout(parentCtx, timeout) - defer cancel() - - // Poll backup tasks - m.pollBackupTasks(backupCtx, inst, pveClient) - - // Poll storage backups - pass nodes to avoid duplicate API calls - m.pollStorageBackupsWithNodes(backupCtx, inst, pveClient, nodes, nodeEffectiveStatus) - - // Poll guest snapshots - m.pollGuestSnapshots(backupCtx, inst, pveClient) + m.pollPVEBackupsAndSnapshots(parentCtx, inst, pveClient, nodes, nodeEffectiveStatus, timeout) duration := time.Since(startTime) log.Info(). @@ -11084,6 +11074,34 @@ func (m *Monitor) calculateBackupOperationTimeout(instanceName string) time.Dura return timeout } +func (m *Monitor) pollPVEBackupsAndSnapshots(parentCtx context.Context, instanceName string, client PVEClientInterface, nodes []proxmox.Node, nodeEffectiveStatus map[string]string, timeout time.Duration) { + if parentCtx == nil { + parentCtx = context.Background() + } + + backupCtx, cancel := context.WithTimeout(parentCtx, timeout) + + // Poll backup tasks + m.pollBackupTasks(backupCtx, instanceName, client) + + // Poll storage backups - pass nodes to avoid duplicate API calls + m.pollStorageBackupsWithNodes(backupCtx, instanceName, client, nodes, nodeEffectiveStatus) + + backupErr := backupCtx.Err() + cancel() + + if backupErr != nil && parentCtx.Err() == nil { + log.Warn(). + Str("instance", instanceName). + Err(backupErr). + Msg("Backup storage polling budget was exhausted before guest snapshot polling; continuing snapshots with their own bounded poll budget") + } + + // Snapshots are independent backup inventory. Let pollGuestSnapshots establish + // its own bounded budget so a slow storage scan cannot starve snapshot discovery. + m.pollGuestSnapshots(parentCtx, instanceName, client) +} + // pollGuestSnapshots polls snapshots for all VMs and containers func (m *Monitor) pollGuestSnapshots(ctx context.Context, instanceName string, client PVEClientInterface) { log.Debug().Str("instance", instanceName).Msg("Polling guest snapshots") diff --git a/internal/monitoring/monitor_snapshot_test.go b/internal/monitoring/monitor_snapshot_test.go index e1cc4d0d9..05cecadd6 100644 --- a/internal/monitoring/monitor_snapshot_test.go +++ b/internal/monitoring/monitor_snapshot_test.go @@ -31,6 +31,42 @@ func (m *mockPVEClientSnapshots) GetVMMemAvailableFromAgent(ctx context.Context, return 0, fmt.Errorf("not implemented") } +type backupStorageTimeoutSnapshotClient struct { + mockPVEClientExtra + snapshots []proxmox.Snapshot + snapshotCalls int + storageCalls int +} + +func (m *backupStorageTimeoutSnapshotClient) GetBackupTasks(ctx context.Context) ([]proxmox.Task, error) { + return nil, nil +} + +func (m *backupStorageTimeoutSnapshotClient) GetStorage(ctx context.Context, node string) ([]proxmox.Storage, error) { + m.storageCalls++ + if m.storageCalls > 1 { + return nil, nil + } + + <-ctx.Done() + return nil, fmt.Errorf("storage scan exceeded backup inventory budget") +} + +func (m *backupStorageTimeoutSnapshotClient) GetVMSnapshots(ctx context.Context, node string, vmid int) ([]proxmox.Snapshot, error) { + m.snapshotCalls++ + if err := ctx.Err(); err != nil { + return nil, err + } + return m.snapshots, nil +} + +func (m *backupStorageTimeoutSnapshotClient) GetContainerSnapshots(ctx context.Context, node string, vmid int) ([]proxmox.Snapshot, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + return nil, nil +} + func TestMonitor_PollGuestSnapshots_Coverage(t *testing.T) { m := &Monitor{ state: models.NewState(), @@ -144,6 +180,48 @@ func TestMonitor_PollGuestSnapshots_PreservesPreviousOnPerVMError(t *testing.T) } } +func TestMonitor_PollPVEBackupsAndSnapshots_DoesNotStarveSnapshotsAfterStorageTimeout(t *testing.T) { + m := &Monitor{state: models.NewState()} + + m.state.UpdateVMsForInstance("pve1", []models.VM{{ + ID: "qemu/100", + VMID: 100, + Node: "node1", + Instance: "pve1", + Name: "vm100", + Template: false, + }}) + + client := &backupStorageTimeoutSnapshotClient{ + snapshots: []proxmox.Snapshot{{ + Name: "snap_after_storage_timeout", + SnapTime: 4000, + Description: "created while storage scan was slow", + }}, + } + + m.pollPVEBackupsAndSnapshots( + context.Background(), + "pve1", + client, + []proxmox.Node{{Node: "node1", Status: "online"}}, + map[string]string{"node1": "online"}, + time.Millisecond, + ) + + if client.snapshotCalls == 0 { + t.Fatal("expected guest snapshot polling to run even after storage backup polling exhausted its budget") + } + + got := m.state.GetSnapshot().PVEBackups.GuestSnapshots + if len(got) != 1 { + t.Fatalf("expected one guest snapshot after storage timeout, got %#v", got) + } + if got[0].Name != "snap_after_storage_timeout" { + t.Fatalf("expected fresh snapshot after storage timeout, got %#v", got[0]) + } +} + func keys[K comparable, V any](m map[K]V) []K { out := make([]K, 0, len(m)) for k := range m {