Accept PMG 9 cluster ids and stop guessing backup nodes (#2991)

Refs #2960.

## What was wrong

Proxmox Mail Gateway 9.1 users on 6.6.0 betas and rc.1 saw their PMG
connection go degraded, with two errors:

```
pmg_cluster_status failed: json: cannot unmarshal string into Go struct field ClusterStatusEntry.data.cid of type int
pmg_backups failed: API error 500: hostname lookup 'PMG-01' failed
```

1. **Cluster ID format.** PMG 9 sends the cluster status `cid` as a
string. Pulse decoded it into an `int`, so the whole cluster read failed
and the node list (queues, backups) was lost. Pulse never reads `cid`,
so `ClusterStatusEntry` no longer decodes it.
2. **Guessed backup node.** With no node list, the poller asked for
backups from a node named after the connection's display name
(`PMG-01`). PMG rejected that with a hostname-lookup error, which read
like a DNS problem. That fallback now runs only when PMG answers
successfully with an empty cluster list, which is the case some
standalone installs rely on. After a failed cluster read it is skipped.

Skipping the guessed request changes no data. That request could never
return backups, so the published backup list was empty either way. The
poll still records the cluster-read failure.

## Verification

- **New tests**, which fail against `main`:
- `TestClientClusterStatusIgnoresClusterIDFormat`: `cid` as `1`, `"1"`,
`"node-a"` and `null` all decode.
- `TestPollPMGInstanceAcceptsStringClusterID`: the reporter's payload
populates node `pmg-01` and reads backups from `/nodes/pmg-01/backup`
only.
- `TestPollPMGInstanceDoesNotGuessNodeAfterFailedClusterRead`: after a
failed cluster read, no backup request is sent.
- **Unchanged behaviour:**
`TestPollPMGInstanceKeepsNameFallbackForEmptyCluster` passes on `main`
and with this change. An empty cluster still falls back to the
connection name.
- **Suites:** `go test ./pkg/pmg/` passes, and so does all of
`./internal/monitoring/` (433s). `go vet` and `gofmt` are clean.
- **Review:** an independent review found no other reader of `cid`, and
no other PMG integer field that isn't already tolerant of strings.
- **Not tested here:** there is no PMG 9 in the lab, so this hasn't run
against a real PMG 9.

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