Accept PMG 9 cluster ids and stop guessing backup nodes

Proxmox Mail Gateway 9 returns the cluster status cid as a string, so
decoding it into an int failed the whole cluster read and the poller
lost the node list (issue 2960). Pulse never reads cid, so the field is
no longer decoded and any format works.

With no nodes, the poller then requested backups from a node named
after the connection's display name ("PMG-01"), which PMG rejected with
a hostname-lookup error that pointed users at DNS. That fallback now
runs only when PMG answered with no cluster entries; after a failed
cluster read the request could never return backups, and the poll
already records the cluster failure.

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

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{}{}

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)
+	}
+}

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"`

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()