diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 4cce41269e..7b5731b673 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -336,11 +336,29 @@ the connection name as the backup node only when PMG answered with no cluster entries; after a failed cluster read that name is a display label, and requesting backups with it produced a misleading hostname-lookup error without ever returning backups. The poll still records the cluster read -failure, and the published backup list is empty exactly as before. +failure. It requests no backups, and the connection's last-known backups +are kept rather than replaced by an empty list (see the next section). `TestClientClusterStatusIgnoresClusterIDFormat` and the `TestPollPMGInstance*` issue tests in `monitor_pmg_test.go` pin all three cases. +### PMG backups keep last-known entries for unread nodes + +`UpdatePMGBackups` replaces a connection's whole backup list, so only a node +whose backup list was read in this poll is authoritative for it. A node that +was not read keeps its previous backups: every node after a failed cluster +read, a node whose backup read failed, and a node a cancelled poll never +reached. This is the same per-scope carry that `preserveFailedStorageBackups` +applies to failed PVE storages. A node that answered with no backups, or that +a successful cluster read no longer lists, still clears its entries. The +failed read is still recorded as a poll failure, and the connection-name +fallback above is unchanged. `TestPMGBackupsSurviveFailedClusterRead`, +`TestPMGBackupsSurviveEveryNodeBackupReadFailing`, +`TestPMGBackupsKeepFailedNodeOnPartialRead` and +`TestPMGBackupsClearAfterCompleteEnumeration` in `monitor_pmg_test.go` cover +the carry and the complete-enumeration clear. These are synthetic poller +controls, not native PMG acceptance. + ### Guest configuration reads preserve installation and node identity Resolving omitted placement for the existing guest-config read honours every diff --git a/internal/monitoring/monitor_pbs_pmg.go b/internal/monitoring/monitor_pbs_pmg.go index cadb9bbf13..1508a9217e 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -5,6 +5,7 @@ import ( stderrors "errors" "fmt" "math" + "sort" "strings" "time" @@ -873,6 +874,7 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie pmgBackups := make([]models.PMGBackup, 0) seenBackupIDs := make(map[string]struct{}) + readBackupNodes := make(map[string]struct{}, len(backupNodes)) for nodeName := range backupNodes { if ctx.Err() != nil { @@ -890,6 +892,7 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie } continue } + readBackupNodes[nodeName] = struct{}{} for _, b := range backups { timestamp := b.Timestamp.Int64() @@ -910,6 +913,24 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie } } + // Only a node whose backup list was read is authoritative this cycle. A + // failed cluster read, a failed per-node read or a cancelled poll observed + // nothing, so those nodes keep their last-known backups instead of being + // published as having none; the failed read is already in pollErr. + if clusterErr != nil || len(readBackupNodes) < len(backupNodes) { + var preservedBackupNodes []string + pmgBackups, preservedBackupNodes = preserveUnreadPMGNodeBackups( + instanceName, m.state.GetSnapshot().PMGBackups, pmgBackups, + clusterErr != nil, backupNodes, readBackupNodes) + if len(preservedBackupNodes) > 0 { + log.Warn(). + Str("instance", instanceName). + Strs("nodes", preservedBackupNodes). + Bool("clusterReadFailed", clusterErr != nil). + Msg("PMG backup list unavailable for some nodes; keeping their previous backups") + } + } + if debugEnabled { log.Debug(). Str("instance", instanceName). @@ -1065,4 +1086,45 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie } } +// preserveUnreadPMGNodeBackups carries forward previous backups for nodes +// whose backup list was not read this cycle, mirroring +// preserveFailedStorageBackups for PVE storages. When the cluster read failed +// no node was enumerated, so every previous node is unread. When the cluster +// read succeeded, a node it no longer lists is gone and its backups drop. +func preserveUnreadPMGNodeBackups(instanceName string, previous, current []models.PMGBackup, clusterUnread bool, enumeratedNodes, readNodes map[string]struct{}) ([]models.PMGBackup, []string) { + existing := make(map[string]struct{}, len(current)) + for _, backup := range current { + existing[backup.ID] = struct{}{} + } + + preserved := make(map[string]struct{}) + for _, backup := range previous { + if backup.Instance != instanceName { + continue + } + if _, read := readNodes[backup.Node]; read { + continue + } + if _, enumerated := enumeratedNodes[backup.Node]; !enumerated && !clusterUnread { + continue + } + if _, duplicate := existing[backup.ID]; duplicate { + continue + } + current = append(current, backup) + existing[backup.ID] = struct{}{} + preserved[backup.Node] = struct{}{} + } + + if len(preserved) == 0 { + return current, nil + } + nodes := make([]string, 0, len(preserved)) + for node := range preserved { + nodes = append(nodes, node) + } + sort.Strings(nodes) + return current, nodes +} + // GetState returns the current state diff --git a/internal/monitoring/monitor_pmg_test.go b/internal/monitoring/monitor_pmg_test.go index c7680d5240..46a9944207 100644 --- a/internal/monitoring/monitor_pmg_test.go +++ b/internal/monitoring/monitor_pmg_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "sort" "strings" "testing" "time" @@ -634,3 +635,148 @@ func TestPollPMGInstanceKeepsNameFallbackForEmptyCluster(t *testing.T) { t.Fatalf("expected the connection-name fallback for an empty cluster, got %v", backupPaths) } } + +// pmgBackupRetentionFixture drives the real PMG poll consumer with a cluster +// listing and per-node backup answers chosen by each test. +type pmgBackupRetentionFixture struct { + *pmgScopeFixture + clusterErr error + nodes []string + backupErr map[string]error + backups map[string][]pmg.BackupEntry +} + +func newPMGBackupRetentionFixture() *pmgBackupRetentionFixture { + return &pmgBackupRetentionFixture{ + pmgScopeFixture: &pmgScopeFixture{calls: map[string]int{}}, + nodes: []string{"one", "two"}, + backupErr: map[string]error{}, + backups: map[string][]pmg.BackupEntry{ + "one": {{Filename: "one.tgz", Timestamp: 1700000000, Size: 10}}, + "two": {{Filename: "two.tgz", Timestamp: 1700000100, Size: 20}}, + }, + } +} + +func (f *pmgBackupRetentionFixture) GetClusterStatus(context.Context, bool) ([]pmg.ClusterStatusEntry, error) { + f.calls["cluster"]++ + if f.clusterErr != nil { + return nil, f.clusterErr + } + entries := make([]pmg.ClusterStatusEntry, 0, len(f.nodes)) + for _, node := range f.nodes { + entries = append(entries, pmg.ClusterStatusEntry{Name: node, Type: "node"}) + } + return entries, nil +} + +func (f *pmgBackupRetentionFixture) ListBackups(_ context.Context, node string) ([]pmg.BackupEntry, error) { + f.calls["backup/"+node]++ + if err := f.backupErr[node]; err != nil { + return nil, err + } + return f.backups[node], nil +} + +func pmgBackupFilesByNode(t *testing.T, m *Monitor) map[string][]string { + t.Helper() + out := map[string][]string{} + for _, backup := range m.state.GetSnapshot().PMGBackups { + if backup.Instance != "gateway" { + t.Fatalf("unexpected instance on PMG backup: %+v", backup) + } + out[backup.Node] = append(out[backup.Node], backup.Filename) + } + for node := range out { + sort.Strings(out[node]) + } + return out +} + +func pollPMGBackupRetention(t *testing.T, m *Monitor, f *pmgBackupRetentionFixture) { + t.Helper() + m.pollPMGInstance(context.Background(), "gateway", f) +} + +func assertPMGPollFailed(t *testing.T, m *Monitor) { + t.Helper() + status := m.pollStatusMap["pmg::gateway"] + if status == nil || status.ConsecutiveFailures == 0 || status.LastErrorMessage == "" { + t.Fatalf("failed backup enumeration not recorded as a poll failure: %+v", status) + } +} + +// A failed cluster read cannot enumerate backups, so it must not publish an +// empty list over the backups the last good poll found. +func TestPMGBackupsSurviveFailedClusterRead(t *testing.T) { + m := pmgScopeMonitor(config.PMGInstance{MonitoringConfigured: true}) + f := newPMGBackupRetentionFixture() + pollPMGBackupRetention(t, m, f) + want := map[string][]string{"one": {"one.tgz"}, "two": {"two.tgz"}} + if got := pmgBackupFilesByNode(t, m); !reflect.DeepEqual(got, want) { + t.Fatalf("first poll backups=%v want=%v", got, want) + } + + f.clusterErr = errors.New("cluster status unavailable") + f.calls = map[string]int{} + pollPMGBackupRetention(t, m, f) + if f.calls["backup/one"] != 0 || f.calls["backup/two"] != 0 { + t.Fatalf("backups requested without a node list: %v", f.calls) + } + if got := pmgBackupFilesByNode(t, m); !reflect.DeepEqual(got, want) { + t.Fatalf("failed cluster read replaced backups: got=%v want=%v", got, want) + } + assertPMGPollFailed(t, m) +} + +// When every per-node backup read fails, nothing was observed this poll and +// the last-known list stays. +func TestPMGBackupsSurviveEveryNodeBackupReadFailing(t *testing.T) { + m := pmgScopeMonitor(config.PMGInstance{MonitoringConfigured: true}) + f := newPMGBackupRetentionFixture() + pollPMGBackupRetention(t, m, f) + + f.backupErr["one"] = errors.New("backup list timed out") + f.backupErr["two"] = errors.New("backup list timed out") + pollPMGBackupRetention(t, m, f) + want := map[string][]string{"one": {"one.tgz"}, "two": {"two.tgz"}} + if got := pmgBackupFilesByNode(t, m); !reflect.DeepEqual(got, want) { + t.Fatalf("failed backup reads replaced backups: got=%v want=%v", got, want) + } + assertPMGPollFailed(t, m) +} + +// A failed read on one node keeps that node's last-known backups while the +// nodes that answered publish what they returned. +func TestPMGBackupsKeepFailedNodeOnPartialRead(t *testing.T) { + m := pmgScopeMonitor(config.PMGInstance{MonitoringConfigured: true}) + f := newPMGBackupRetentionFixture() + pollPMGBackupRetention(t, m, f) + + f.backupErr["one"] = errors.New("backup list timed out") + f.backups["two"] = []pmg.BackupEntry{{Filename: "two-new.tgz", Timestamp: 1700000200, Size: 30}} + pollPMGBackupRetention(t, m, f) + want := map[string][]string{"one": {"one.tgz"}, "two": {"two-new.tgz"}} + if got := pmgBackupFilesByNode(t, m); !reflect.DeepEqual(got, want) { + t.Fatalf("partial read backups=%v want=%v", got, want) + } + assertPMGPollFailed(t, m) +} + +// A complete enumeration is authoritative: nodes that answered with no +// backups, and nodes no longer in the cluster, clear their entries. +func TestPMGBackupsClearAfterCompleteEnumeration(t *testing.T) { + m := pmgScopeMonitor(config.PMGInstance{MonitoringConfigured: true}) + f := newPMGBackupRetentionFixture() + pollPMGBackupRetention(t, m, f) + + f.nodes = []string{"one"} + f.backups["one"] = nil + pollPMGBackupRetention(t, m, f) + if got := pmgBackupFilesByNode(t, m); len(got) != 0 { + t.Fatalf("complete enumeration with no backups left entries: %v", got) + } + if status := m.pollStatusMap["pmg::gateway"]; status == nil || status.ConsecutiveFailures != 0 { + t.Fatalf("complete enumeration recorded as failed: %+v", status) + } +}