diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 01b410ac4e..a0d0c16dcf 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -322,6 +322,20 @@ successful zero objects remain observations. `TestPMGCollectionMissingObjectData and `TestPMGCollectionObservedZeroData` in `pkg/pmg/client_test.go` exercise the real request/decoder boundary using an in-memory HTTP transport, with no listener. +### PMG 9 cluster ids and the backup node fallback — issue #2960 + +PMG 9 returns the cluster status `cid` as a string. `pmg.ClusterStatusEntry` +no longer decodes the field, which Pulse never read, so no `cid` format can +fail the cluster read and hide the node list. The PMG poller falls back to +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. +`TestClientClusterStatusIgnoresClusterIDFormat` and the +`TestPollPMGInstance*` issue tests in `monitor_pmg_test.go` pin all three +cases. + ### 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 be87ec3fb5..cadb9bbf13 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -806,11 +806,11 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie } } - cluster, err := client.GetClusterStatus(ctx, true) - recordReadError("pmg_cluster_status", err) - if err != nil { + cluster, clusterErr := client.GetClusterStatus(ctx, true) + recordReadError("pmg_cluster_status", clusterErr) + if clusterErr != nil { if debugEnabled { - log.Debug().Err(err).Str("instance", instanceName).Msg("failed to retrieve PMG cluster status") + log.Debug().Err(clusterErr).Str("instance", instanceName).Msg("failed to retrieve PMG cluster status") } } @@ -860,7 +860,11 @@ func (m *Monitor) pollPMGInstance(ctx context.Context, instanceName string, clie pmgInst.Nodes = nodes } - if len(backupNodes) == 0 { + // Fall back to the connection name only when PMG answered with no cluster + // entries. After a failed cluster read the name is a display label, not a + // node: requesting backups with it fails with a misleading hostname-lookup + // error on PMG (#2960) and cannot return backups. + if len(backupNodes) == 0 && clusterErr == nil { trimmed := strings.TrimSpace(instanceName) if trimmed != "" { backupNodes[trimmed] = struct{}{} diff --git a/internal/monitoring/monitor_pmg_test.go b/internal/monitoring/monitor_pmg_test.go index 6be3796c0a..c7680d5240 100644 --- a/internal/monitoring/monitor_pmg_test.go +++ b/internal/monitoring/monitor_pmg_test.go @@ -536,3 +536,101 @@ func TestPMGCollectionCancelledAndUnknown(t *testing.T) { t.Fatalf("cancelled/unknown requests=%v", f.calls) } } + +// pollPMGIssue2960 polls one PMG instance named like the reporter's +// connection ("PMG-01") against a server whose cluster status is served by +// clusterStatus, and records every backup path requested. +func pollPMGIssue2960(t *testing.T, clusterStatus func(w http.ResponseWriter)) (*Monitor, []string) { + t.Helper() + var backupPaths []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/api2/json/version": + fmt.Fprint(w, `{"data":{"version":"9.1.4","release":"1"}}`) + case r.URL.Path == "/api2/json/config/cluster/status": + clusterStatus(w) + case strings.HasSuffix(r.URL.Path, "/backup"): + backupPaths = append(backupPaths, r.URL.Path) + if r.URL.Path != "/api2/json/nodes/pmg-01/backup" { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"message":"hostname lookup failed","data":null}`) + return + } + timestamp := time.Now().Add(-24 * time.Hour).Unix() + fmt.Fprintf(w, `{"data":[{"filename":"pmg-backup.tgz","size":2048,"timestamp":%d}]}`, timestamp) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + t.Cleanup(server.Close) + + client, err := pmg.NewClient(pmg.ClientConfig{ + Host: server.URL, + TokenName: "pulse@pmg!monitor", + TokenValue: "secret", + VerifySSL: false, + }) + if err != nil { + t.Fatalf("unexpected client error: %v", err) + } + mon, err := New(&config.Config{PMGInstances: []config.PMGInstance{{ + Name: "PMG-01", + Host: server.URL, + TokenName: "pulse@pmg!monitor", + TokenValue: "secret", + // Explicit scope with every collector off: only cluster status and + // backups are read. + MonitoringConfigured: true, + }}}) + if err != nil { + t.Fatalf("failed to create monitor: %v", err) + } + mon.pollPMGInstance(context.Background(), "PMG-01", client) + return mon, backupPaths +} + +// Issue #2960: PMG 9 sends the cluster id as a string. The cluster read must +// still succeed, so backups come from the node PMG names, not the connection. +func TestPollPMGInstanceAcceptsStringClusterID(t *testing.T) { + mon, backupPaths := pollPMGIssue2960(t, func(w http.ResponseWriter) { + fmt.Fprint(w, `{"data":[{"cid":"1","name":"pmg-01","type":"master","ip":"10.0.0.5"}]}`) + }) + + snapshot := mon.state.GetSnapshot() + if len(snapshot.PMGInstances) != 1 || len(snapshot.PMGInstances[0].Nodes) != 1 || + snapshot.PMGInstances[0].Nodes[0].Name != "pmg-01" { + t.Fatalf("expected the PMG node from cluster status, got %+v", snapshot.PMGInstances) + } + if !reflect.DeepEqual(backupPaths, []string{"/api2/json/nodes/pmg-01/backup"}) { + t.Fatalf("expected backups from node pmg-01 only, got %v", backupPaths) + } + if len(snapshot.PMGBackups) != 1 || snapshot.PMGBackups[0].Node != "pmg-01" { + t.Fatalf("expected one backup on pmg-01, got %+v", snapshot.PMGBackups) + } +} + +// Issue #2960: after a failed cluster read the connection's display name is +// not a node, so Pulse must not request backups with it. +func TestPollPMGInstanceDoesNotGuessNodeAfterFailedClusterRead(t *testing.T) { + _, backupPaths := pollPMGIssue2960(t, func(w http.ResponseWriter) { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"message":"cluster status unavailable","data":null}`) + }) + + if len(backupPaths) != 0 { + t.Fatalf("expected no backup request after a failed cluster read, got %v", backupPaths) + } +} + +// A PMG that answers with no cluster entries keeps the connection-name +// fallback some standalone installs rely on. +func TestPollPMGInstanceKeepsNameFallbackForEmptyCluster(t *testing.T) { + _, backupPaths := pollPMGIssue2960(t, func(w http.ResponseWriter) { + fmt.Fprint(w, `{"data":[]}`) + }) + + if !reflect.DeepEqual(backupPaths, []string{"/api2/json/nodes/PMG-01/backup"}) { + t.Fatalf("expected the connection-name fallback for an empty cluster, got %v", backupPaths) + } +} diff --git a/pkg/pmg/client.go b/pkg/pmg/client.go index 3df1edd827..d2d5f4d7b8 100644 --- a/pkg/pmg/client.go +++ b/pkg/pmg/client.go @@ -127,8 +127,9 @@ type SpamScore struct { Ratio flexibleFloat `json:"ratio"` } +// ClusterStatusEntry omits PMG's cluster id (cid): Pulse never reads it, and +// PMG 9 sends it as a string, which failed the whole cluster read (#2960). type ClusterStatusEntry struct { - CID int `json:"cid"` Name string `json:"name"` Type string `json:"type"` IP string `json:"ip"` diff --git a/pkg/pmg/client_test.go b/pkg/pmg/client_test.go index 391cb18e00..778ff30c3d 100644 --- a/pkg/pmg/client_test.go +++ b/pkg/pmg/client_test.go @@ -677,6 +677,39 @@ func TestClientClusterStatusListSingle(t *testing.T) { } } +// PMG 9 returns the cluster id as a string (#2960). Pulse does not use it, so +// no cid format may fail the cluster read. +func TestClientClusterStatusIgnoresClusterIDFormat(t *testing.T) { + t.Parallel() + + for _, cid := range []string{`1`, `"1"`, `"node-a"`, `null`} { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"data":[{"cid":%s,"name":"pmg-01","type":"master","ip":"10.0.0.5"}]}`, cid) + })) + + client, err := NewClient(ClientConfig{ + Host: server.URL, + TokenName: "apitoken", + TokenValue: "secret", + VerifySSL: false, + }) + if err != nil { + server.Close() + t.Fatalf("unexpected error creating client: %v", err) + } + + entries, err := client.GetClusterStatus(context.Background(), true) + server.Close() + if err != nil { + t.Fatalf("cid %s: GetClusterStatus failed: %v", cid, err) + } + if len(entries) != 1 || entries[0].Name != "pmg-01" || entries[0].Type != "master" { + t.Fatalf("cid %s: unexpected entries %+v", cid, entries) + } + } +} + func TestClientListBackupsEscapesNode(t *testing.T) { t.Parallel()