From 806fd7409c4597e869053802b92daa65580ba10d Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 3 Jul 2026 10:52:18 +0100 Subject: [PATCH] Show cluster labels in Proxmox node names Refs #1475 --- .../v6/internal/subsystems/monitoring.md | 5 ++++ .../monitoring/canonical_guardrails_test.go | 20 +++++++++++++ internal/monitoring/helpers_test.go | 18 ++++++++---- internal/monitoring/monitor.go | 19 ++++++++++--- .../monitoring/monitor_integration_test.go | 9 +++--- internal/unifiedresources/adapters_test.go | 28 +++++++++++++++++++ 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 88f1c48fa..00b454937 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -30,6 +30,11 @@ healthy source is between normal poll cycles. Removed host-agent reconnect blocks are identity-scoped: matching may use the canonical host ID or token-qualified machine/hostname continuity, but must never block a distinct live host by hostname alone. +Proxmox cluster node labels are monitoring-owned identity presentation: +configured cluster labels must project into node display names as +`cluster label (node name)` so duplicate node hostnames across clusters remain +distinguishable, while `models.Node.Name` and `ProxmoxData.NodeName` keep the +raw Proxmox node identity for linking, metrics, URLs, and actions. ## Canonical Files diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index a70b27227..06fbfe9bd 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -1107,6 +1107,26 @@ func TestStoragePollingKeepsSharedStorageClusterStatusCanonical(t *testing.T) { } } +func TestMonitoringProxmoxClusterNodeDisplayNamesStayClusterQualified(t *testing.T) { + data, err := os.ReadFile("monitor.go") + if err != nil { + t.Fatalf("failed to read monitor.go: %v", err) + } + source := string(data) + + for _, snippet := range []string{ + "func getNodeDisplayName(instance *config.PVEInstance, nodeName string) string {", + "clusterLabel := friendly", + "clusterLabel = strings.TrimSpace(instance.ClusterName)", + `return fmt.Sprintf("%s (%s)", clusterLabel, baseName)`, + "return baseName", + } { + if !strings.Contains(source, snippet) { + t.Fatalf("monitor.go must contain %q", snippet) + } + } +} + func TestMonitoringTemperatureFallbackPrefersRecentAgentTelemetry(t *testing.T) { requiredSnippets := map[string][]string{ "host_agent_temps.go": { diff --git a/internal/monitoring/helpers_test.go b/internal/monitoring/helpers_test.go index d92b44ca9..444e5497c 100644 --- a/internal/monitoring/helpers_test.go +++ b/internal/monitoring/helpers_test.go @@ -136,14 +136,20 @@ func TestGetNodeDisplayName(t *testing.T) { // non-cluster: returns unknown-node when host is IP address {"host IP fallback to unknown-node", &config.PVEInstance{Name: "", Host: "https://192.168.1.100:8006"}, "", "unknown-node"}, - // cluster: lookupClusterEndpointLabel result takes priority - {"cluster host label", clusterInstance, "node1", "node1.local"}, + // cluster: configured instance label disambiguates the Proxmox node name + {"cluster configured label", clusterInstance, "node1", "cluster (node1)"}, - // cluster: falls back to baseName when no endpoint label - {"cluster base fallback", clusterInstance, "node3", "node3"}, + // cluster: still uses configured label even when no endpoint label is present + {"cluster base fallback", clusterInstance, "node3", "cluster (node3)"}, - // cluster: falls back to nodeName (IP fallback via endpoint) - {"cluster ip fallback", clusterInstance, "node2", "node2"}, + // cluster: keeps the node identity instead of substituting endpoint IP labels + {"cluster ip fallback", clusterInstance, "node2", "cluster (node2)"}, + + // cluster: ClusterName backs the display label when the user-facing name is empty + {"cluster name fallback", &config.PVEInstance{IsCluster: true, ClusterName: "detected-cluster"}, "node4", "detected-cluster (node4)"}, + + // cluster: do not duplicate the label when it already equals the node name + {"cluster duplicate label", &config.PVEInstance{IsCluster: true, Name: "node5", ClusterName: "detected-cluster"}, "node5", "node5"}, // cluster: falls back to friendly name when baseName is "unknown-node" {"cluster friendly fallback", &config.PVEInstance{IsCluster: true, Name: "Cluster Name", ClusterEndpoints: []config.ClusterEndpoint{}}, "", "Cluster Name"}, diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 5a652aa7a..cd78325bd 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -186,16 +186,27 @@ func getNodeDisplayName(instance *config.PVEInstance, nodeName string) string { friendly := strings.TrimSpace(instance.Name) if instance.IsCluster { - if endpointLabel := lookupClusterEndpointLabel(instance, nodeName); endpointLabel != "" { - return endpointLabel + clusterLabel := friendly + if clusterLabel == "" { + clusterLabel = strings.TrimSpace(instance.ClusterName) } if baseName != "" && baseName != "unknown-node" { + if clusterLabel != "" && !strings.EqualFold(clusterLabel, baseName) { + return fmt.Sprintf("%s (%s)", clusterLabel, baseName) + } return baseName } - if friendly != "" { - return friendly + if endpointLabel := lookupClusterEndpointLabel(instance, nodeName); endpointLabel != "" { + if clusterLabel != "" && !strings.EqualFold(clusterLabel, endpointLabel) { + return fmt.Sprintf("%s (%s)", clusterLabel, endpointLabel) + } + return endpointLabel + } + + if clusterLabel != "" { + return clusterLabel } return baseName diff --git a/internal/monitoring/monitor_integration_test.go b/internal/monitoring/monitor_integration_test.go index 88446cb92..092452c89 100644 --- a/internal/monitoring/monitor_integration_test.go +++ b/internal/monitoring/monitor_integration_test.go @@ -354,8 +354,8 @@ func TestGetNodeDisplayName_FriendlyName(t *testing.T) { func TestGetNodeDisplayName_ClusterUsesNodeName(t *testing.T) { inst := &config.PVEInstance{Name: "cluster1", IsCluster: true} got := getNodeDisplayName(inst, "node1") - if got != "node1" { - t.Errorf("cluster mode should use node name, got %q", got) + if got != "cluster1 (node1)" { + t.Errorf("cluster mode should include configured cluster label, got %q", got) } } @@ -368,8 +368,7 @@ func TestGetNodeDisplayName_ClusterWithEndpointLabel(t *testing.T) { }, } got := getNodeDisplayName(inst, "node1") - // With matching endpoint, should still return node1 (or the endpoint label) - if got == "" || got == "unknown-node" { - t.Errorf("expected a valid display name for cluster node, got %q", got) + if got != "cluster1 (node1)" { + t.Errorf("expected cluster-qualified node display name, got %q", got) } } diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index ff7c61329..18713a767 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -39,6 +39,34 @@ func TestResourceFromProxmoxNodeIncludesTemperature(t *testing.T) { } } +func TestResourceFromProxmoxNodeUsesDisplayNameWithoutChangingRawNodeIdentity(t *testing.T) { + node := models.Node{ + ID: "pve-cluster-node01", + Name: "node01", + DisplayName: "PVE Cluster (node01)", + ClusterName: "PVE Cluster", + Status: "online", + } + + resource, identity := resourceFromProxmoxNode(node, nil) + if resource.Name != "PVE Cluster (node01)" { + t.Fatalf("resource.Name = %q, want cluster-qualified display label", resource.Name) + } + if resource.Proxmox == nil || resource.Proxmox.NodeName != "node01" { + t.Fatalf("Proxmox.NodeName = %+v, want raw node identity node01", resource.Proxmox) + } + foundClusterIdentity := false + for _, hostname := range identity.Hostnames { + if hostname == "PVE Cluster:node01" { + foundClusterIdentity = true + break + } + } + if !foundClusterIdentity { + t.Fatalf("identity hostnames = %v, want cluster-qualified raw hostname identity", identity.Hostnames) + } +} + func TestResourceFromDockerContainerAdvertisesLifecycleCapabilities(t *testing.T) { now := time.Now().UTC() host := models.DockerHost{