Honor explicit cluster member address overrides and surface recovery failures

An explicit connection address override on a cluster member was silently
discarded when VerifySSL was enabled and the member had no per-endpoint
fingerprint: the hostname-for-TLS preference displaced the operator's
address, so overriding an undialable discovered hostname changed nothing.
The override now wins in every TLS mode, and the hostname preference
applies only to auto-discovered addresses.

Failed endpoint recovery attempts also logged their cause at debug level
only, leaving the recurring 'No endpoints recovered' warning without a
reason. The warning now carries per-endpoint failure reasons and the
sanitized error is stored so the UI health status shows it too.

Refs #1665

Contract-Neutral: behavioral bug fix refs #1665: cluster member IPOverride honored in all TLS modes, recovery failure reasons surfaced; no public contract delta

internal/monitoring/monitor_additional_test.go

@@ -273,6 +273,19 @@ func TestClusterEndpointEffectiveURL(t *testing.T) {
 		t.Fatalf("override IP preference = %q, want %q", got, "https://192.168.1.10:8006")
 	}
 
+	// #1665: an explicit override must win under verifySSL too, even without a
+	// per-endpoint fingerprint. The hostname-for-TLS preference only applies to
+	// auto-discovered addresses.
+	if got := clusterEndpointEffectiveURL(endpoint, true, false); got != "https://192.168.1.10:8006" {
+		t.Fatalf("override under verifySSL = %q, want %q", got, "https://192.168.1.10:8006")
+	}
+
+	// An override may be a hostname with a port, not just an IP.
+	endpoint.IPOverride = "pve2.internal:9006"
+	if got := clusterEndpointEffectiveURL(endpoint, true, false); got != "https://pve2.internal:9006" {
+		t.Fatalf("hostname override = %q, want %q", got, "https://pve2.internal:9006")
+	}
+
 	// #1199: a cluster-level fingerprint (passed as hasFingerprint=true) must NOT
 	// force IP routing for a member endpoint that has no per-endpoint fingerprint;
 	// hostname routing must be preserved so TLS validation still works.

internal/monitoring/monitor_cluster_helpers.go

@@ -127,6 +127,15 @@ func ensureClusterEndpointURL(raw string) string {
 }
 
 func clusterEndpointEffectiveURL(endpoint config.ClusterEndpoint, verifySSL bool, _ bool) string {
+	// An explicit connection address override is the operator telling Pulse
+	// which address reaches this member, so it wins in every TLS mode. The
+	// hostname preference below is a heuristic for auto-discovered addresses
+	// only: letting it displace an explicit override leaves the member
+	// undialable with no sign the override was ignored. Refs: #1665
+	if override := strings.TrimSpace(endpoint.IPOverride); override != "" {
+		return ensureClusterEndpointURL(override)
+	}
+
 	// A fingerprint only applies to the specific endpoint it was captured from.
 	// The primary node's (cluster-level) fingerprint must not be treated as valid
 	// for every cluster member, otherwise we route to per-node IPs while pinning
@@ -136,21 +145,18 @@ func clusterEndpointEffectiveURL(endpoint config.ClusterEndpoint, verifySSL bool
 	hasEndpointFingerprint := strings.TrimSpace(endpoint.Fingerprint) != ""
 	requiresHostnameForTLS := verifySSL && !hasEndpointFingerprint
 
-	// Use EffectiveIP() which prefers user-specified IPOverride over auto-discovered IP
-	effectiveIP := endpoint.EffectiveIP()
-
 	if requiresHostnameForTLS {
 		// Prefer hostname for proper TLS certificate validation
 		if endpoint.Host != "" {
 			return ensureClusterEndpointURL(endpoint.Host)
 		}
-		if effectiveIP != "" {
-			return ensureClusterEndpointURL(effectiveIP)
+		if endpoint.IP != "" {
+			return ensureClusterEndpointURL(endpoint.IP)
 		}
 	} else {
 		// Prefer IP address to avoid excessive DNS lookups
-		if effectiveIP != "" {
-			return ensureClusterEndpointURL(effectiveIP)
+		if endpoint.IP != "" {
+			return ensureClusterEndpointURL(endpoint.IP)
 		}
 		if endpoint.Host != "" {
 			return ensureClusterEndpointURL(endpoint.Host)

pkg/proxmox/cluster_client.go

@@ -711,6 +711,18 @@ func (cc *ClusterClient) recoverUnhealthyNodes(ctx context.Context) {
 	var wg sync.WaitGroup
 	recoveredEndpoints := make(chan string, len(unhealthyEndpoints))
 
+	var failureMu sync.Mutex
+	failureReasons := make(map[string]string, len(unhealthyEndpoints))
+	recordFailure := func(ep string, err error) {
+		sanitized := sanitizeEndpointError(err.Error())
+		failureMu.Lock()
+		failureReasons[ep] = sanitized
+		failureMu.Unlock()
+		cc.mu.Lock()
+		cc.lastError[ep] = sanitized
+		cc.mu.Unlock()
+	}
+
 	for _, endpoint := range unhealthyEndpoints {
 		wg.Add(1)
 		go func(ep string) {
@@ -731,6 +743,7 @@ func (cc *ClusterClient) recoverUnhealthyNodes(ctx context.Context) {
 
 			testClient, err := NewClient(cfg)
 			if err != nil {
+				recordFailure(ep, err)
 				log.Debug().
 					Str("cluster", cc.name).
 					Str("endpoint", ep).
@@ -779,6 +792,7 @@ func (cc *ClusterClient) recoverUnhealthyNodes(ctx context.Context) {
 						Msg("Recovered unhealthy cluster node")
 				}
 			} else {
+				recordFailure(ep, err)
 				log.Debug().
 					Str("cluster", cc.name).
 					Str("endpoint", ep).
@@ -808,10 +822,22 @@ func (cc *ClusterClient) recoverUnhealthyNodes(ctx context.Context) {
 			Int("attempted", len(unhealthyEndpoints)).
 			Msg("Cluster endpoint recovery completed")
 	} else if len(unhealthyEndpoints) > 0 {
+		// Include the per-endpoint failure reasons: without them this summary
+		// is the only non-debug signal and leaves operators guessing whether
+		// the cause is DNS, TLS, auth, or routing. Refs: #1665
+		failureMu.Lock()
+		reasons := make([]string, 0, len(failureReasons))
+		for _, ep := range unhealthyEndpoints {
+			if reason, ok := failureReasons[ep]; ok {
+				reasons = append(reasons, ep+": "+reason)
+			}
+		}
+		failureMu.Unlock()
 		log.Warn().
 			Str("cluster", cc.name).
 			Int("attempted", len(unhealthyEndpoints)).
 			Strs("failedEndpoints", unhealthyEndpoints).
+			Strs("failureReasons", reasons).
 			Msg("No endpoints recovered - cluster may be unreachable from Pulse server")
 	}
 }

pkg/proxmox/cluster_client_test.go

@@ -483,3 +483,48 @@ func TestClusterClient_GetNodes_PermanentFailure(t *testing.T) {
 		t.Errorf("expected nil nodes on error, got %v", nodes)
 	}
 }
+
+func TestRecoverUnhealthyNodesRecordsFailureReasons(t *testing.T) {
+	// Server that always rejects with an auth error so recovery cannot succeed.
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusUnauthorized)
+		fmt.Fprint(w, `{"data":null}`)
+	}))
+	defer server.Close()
+
+	cfg := ClientConfig{
+		Host:       server.URL,
+		TokenName:  "pulse@pve!token",
+		TokenValue: "sometokenvalue",
+		VerifySSL:  false,
+		Timeout:    2 * time.Second,
+	}
+
+	// Single endpoint skips the initial health check, keeping the test deterministic.
+	cc := NewClusterClient("test-cluster", cfg, []string{server.URL}, nil)
+
+	cc.mu.Lock()
+	cc.nodeHealth[server.URL] = false
+	delete(cc.lastHealthCheck, server.URL)
+	cc.mu.Unlock()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	cc.recoverUnhealthyNodes(ctx)
+
+	health := cc.GetHealthStatusWithErrors()
+	status, ok := health[server.URL]
+	if !ok {
+		t.Fatalf("expected health entry for %s, got %+v", server.URL, health)
+	}
+	if status.Healthy {
+		t.Fatal("expected endpoint to remain unhealthy after failed recovery")
+	}
+	if status.LastError == "" {
+		t.Fatal("expected failed recovery to record a failure reason in LastError")
+	}
+	if status.LastError != "Authentication failed - check API token or credentials" {
+		t.Fatalf("expected sanitized auth failure reason, got %q", status.LastError)
+	}
+}