diff --git a/internal/monitoring/monitor_additional_test.go b/internal/monitoring/monitor_additional_test.go index 0bb583f593..235acd9250 100644 --- a/internal/monitoring/monitor_additional_test.go +++ b/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. diff --git a/internal/monitoring/monitor_cluster_helpers.go b/internal/monitoring/monitor_cluster_helpers.go index 9ad5164fc6..3759395a72 100644 --- a/internal/monitoring/monitor_cluster_helpers.go +++ b/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) diff --git a/pkg/proxmox/cluster_client.go b/pkg/proxmox/cluster_client.go index 24dc17d5f9..cb7db906b0 100644 --- a/pkg/proxmox/cluster_client.go +++ b/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") } } diff --git a/pkg/proxmox/cluster_client_test.go b/pkg/proxmox/cluster_client_test.go index 694a9acfb4..e5eaf068fe 100644 --- a/pkg/proxmox/cluster_client_test.go +++ b/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) + } +}