From 2ad094c927756b84726438fce2aba8b308481d60 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 1 Sep 2026 06:15:46 +0100 Subject: [PATCH 1/3] Deflake typed action containment tests The group-writable executable check wrote its fixture through os.WriteFile, which filters the mode through the process umask, so under the runner's 022 umask the group-write bit never reached disk and validateTrustedExecutable correctly trusted the file. Chmod the fixture to the asserted mode. The Proxmox guest executor tests stamped their after observations one second past the test-start clock, but actionStartedAt is stamped inside ExecuteAction after handler setup, so a loaded runner overran the margin and the independent observation was discarded as pre-action. Widen the observation offset to a minute. Both failures broke build-and-test on main (run 33454838531). Reproduced the hostagent failure on Linux under umask 022 and verified both packages green after the fix. --- internal/api/proxmox_guest_action_executor_test.go | 9 ++++++--- internal/hostagent/typed_action_command_linux_test.go | 5 +++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/api/proxmox_guest_action_executor_test.go b/internal/api/proxmox_guest_action_executor_test.go index 9c8a3e88c..93feccb51 100644 --- a/internal/api/proxmox_guest_action_executor_test.go +++ b/internal/api/proxmox_guest_action_executor_test.go @@ -186,7 +186,10 @@ func TestProxmoxGuestActionExecutorUsesIndependentControlPlaneVerification(t *te }} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 3600, "proxmox-control-plane:default:homelab"), - proxmoxGuestActionObservation(now.Add(time.Second), "stopped", 0, "proxmox-control-plane:default:homelab"), + // The after observation must postdate actionStartedAt, which is stamped + // inside ExecuteAction after handler setup; a one-second offset loses + // that race on a loaded runner, so use a generous margin. + proxmoxGuestActionObservation(now.Add(time.Minute), "stopped", 0, "proxmox-control-plane:default:homelab"), }} executor := newProxmoxGuestActionExecutor(h, agents, observer) @@ -215,7 +218,7 @@ func TestProxmoxGuestActionExecutorRequiresUptimeResetToVerifyReboot(t *testing. agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{{RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "reboot requested"}}} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 7200, "proxmox-control-plane:default:homelab"), - proxmoxGuestActionObservation(now.Add(time.Second), "running", 4, "proxmox-control-plane:default:homelab"), + proxmoxGuestActionObservation(now.Add(time.Minute), "running", 4, "proxmox-control-plane:default:homelab"), }} executor := newProxmoxGuestActionExecutor(h, agents, observer) @@ -239,7 +242,7 @@ func TestProxmoxGuestActionExecutorKeepsIndependentContradictionSeparateFromExec agents := &fakeDockerActionAgentCommander{results: []*agentexec.CommandResultPayload{{RequestID: "act_vm", Success: true, ExitCode: 0, Stdout: "reboot requested"}}} observer := &fakeProxmoxGuestPostconditionObserver{observations: []proxmoxGuestPostconditionObservation{ proxmoxGuestActionObservation(now.Add(-time.Second), "running", 7200, "proxmox-control-plane:default:homelab"), - proxmoxGuestActionObservation(now.Add(time.Second), "running", 7201, "proxmox-control-plane:default:homelab"), + proxmoxGuestActionObservation(now.Add(time.Minute), "running", 7201, "proxmox-control-plane:default:homelab"), }} executor := newProxmoxGuestActionExecutor(h, agents, observer) ctx, cancel := context.WithTimeout(actionDispatchTestContext(t, "act_vm"), 100*time.Millisecond) diff --git a/internal/hostagent/typed_action_command_linux_test.go b/internal/hostagent/typed_action_command_linux_test.go index a7b5bfa48..7eab05b26 100644 --- a/internal/hostagent/typed_action_command_linux_test.go +++ b/internal/hostagent/typed_action_command_linux_test.go @@ -201,6 +201,11 @@ func TestValidateTrustedExecutableRequiresControlledExecutableFile(t *testing.T) if err := os.WriteFile(writable, []byte("binary"), 0o720); err != nil { t.Fatal(err) } + // WriteFile filters mode through the process umask, which strips the + // group-write bit under the common 022 umask; force the mode we assert on. + if err := os.Chmod(writable, 0o720); err != nil { + t.Fatal(err) + } if err := validateTrustedExecutable(writable, uint32(os.Geteuid())); err == nil { t.Fatal("group-writable executable was trusted") } From f4886c2dfbb519cc291bc318568190915848726e Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 1 Sep 2026 06:36:02 +0100 Subject: [PATCH 2/3] Document per-node agent installs for PVE clusters The generated install command is host-generic, but nothing said so, and cluster operators kept asking how to cover every member (#1816, #1618). State it in the agent Quick Start next to the command generator pointer. Refs #1816 --- docs/UNIFIED_AGENT.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/UNIFIED_AGENT.md b/docs/UNIFIED_AGENT.md index 079acafba..a7c3b7f13 100644 --- a/docs/UNIFIED_AGENT.md +++ b/docs/UNIFIED_AGENT.md @@ -26,6 +26,12 @@ Generate an installation command in the UI: Choose a target profile in that screen when you want explicit install flags for Docker, Kubernetes, Proxmox VE, or Proxmox Backup Server. +The generated command is not tied to a single machine. For a Proxmox VE +cluster, one API connection already provides cluster-wide inventory; the agent +is per host, so run the same generated command on each cluster node where you +want agent-provided telemetry (temperatures, SMART, host identity). Each agent +registers itself and attaches to its own cluster member. + The same generated command is also the supported v5-to-v6 agent upgrade path. Run it on the host that already has the v5 `pulse-agent` service to replace the binary and service configuration in place; do not uninstall the old service From 3f4ea6f64070c2172dd169bf169543a2dd4c43aa Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 1 Sep 2026 06:38:34 +0100 Subject: [PATCH 3/3] Stop same-hostname standalone sites collapsing at presentation Two hand-added standalone Proxmox connections whose machines are both natively named pve survived the state-layer identity fixes but still collapsed in the presentation host coalesce: a provider node row carries no machine identity of its own, so once the site agents connected, one site's node row absorbed an agent row on the shared short hostname and the other site's node row folded into it. Input ordering varies per snapshot, so the surviving row alternated between sites on every refresh, which is exactly the reported NODES 1 view with the VM count flipping 11/18 and the Agent badge hopping rows on v6.4.1. Add a provider-scope veto mirroring the state rule: node facets from different connection instances only merge with same-machine proof (same node identity, same non-empty cluster, or same endpoint host). In a bucket holding two distinct provider scopes, an agent row now attaches only to the node whose state-layer agent link names it, instead of the first compatible row. The merged row also keeps the Proxmox node row's display-name-aware name instead of the agent's bare reported hostname, which is the original 'Node Name field not observed' complaint. Validated: internal/unifiedresources, internal/monitoring, and internal/api green on linux/amd64 with the change. Refs #1753 --- .../unifiedresources/presentation_coalesce.go | 100 ++++++++++++++++++ .../presentation_coalesce_test.go | 97 +++++++++++++++++ 2 files changed, 197 insertions(+) diff --git a/internal/unifiedresources/presentation_coalesce.go b/internal/unifiedresources/presentation_coalesce.go index ce7b77581..ec1741410 100644 --- a/internal/unifiedresources/presentation_coalesce.go +++ b/internal/unifiedresources/presentation_coalesce.go @@ -43,6 +43,7 @@ func coalescePresentationHostResourcesOnce( coalesced := make([]Resource, 0, len(resources)) indexesByHostKey := make(map[string][]int, len(resources)) parentRedirects := make(map[string]string) + guardedHostKeys := presentationAmbiguousProxmoxHostKeys(resources) for _, resource := range resources { resource.Type = CanonicalResourceType(resource.Type) hostKey := presentationHostMergeKey(resource) @@ -64,6 +65,9 @@ func coalescePresentationHostResourcesOnce( if presentationHostIdentitiesDistinct(existing, resource) { continue } + if guardedHostKeys[hostKey] && !presentationGuardedMergeAllowed(existing, resource) { + continue + } if !presentationHostnamesCompatible(existing, resource) { continue } @@ -215,9 +219,98 @@ func presentationHostIdentitiesDistinct(left, right Resource) bool { presentationIdentityValuesConflict(left.Proxmox.ClusterName, right.Proxmox.ClusterName) { return true } + if presentationProxmoxNodeScopesDistinct(left.Proxmox, right.Proxmox) { + return true + } return false } +// presentationProxmoxNodeScopesDistinct reports whether two Proxmox node +// facets describe different provider connections without any same-machine +// proof. Standalone connections are provider scopes too, and both machines in +// two hand-added sites are commonly just "pve" (#1753), so a shared short +// hostname must not fold one site's node row into the other's. The proof +// mirrors the state-layer rule: the same connection instance, the same node +// identity, the same non-empty cluster, or the same endpoint host still +// merge; anything less keeps the rows apart. +func presentationProxmoxNodeScopesDistinct(left, right *ProxmoxData) bool { + if left == nil || right == nil { + return false + } + if strings.TrimSpace(left.NodeName) == "" || strings.TrimSpace(right.NodeName) == "" { + return false + } + if !presentationIdentityValuesConflict(left.Instance, right.Instance) { + return false + } + if presentationIdentityValuesEqual(left.NodeIdentity, right.NodeIdentity) { + return false + } + if presentationIdentityValuesEqual(left.ClusterName, right.ClusterName) { + return false + } + if presentationIdentityValuesEqual(extractHostname(left.HostURL), extractHostname(right.HostURL)) { + return false + } + return true +} + +func presentationIdentityValuesEqual(left, right string) bool { + left = strings.TrimSpace(left) + right = strings.TrimSpace(right) + return left != "" && right != "" && strings.EqualFold(left, right) +} + +// presentationAmbiguousProxmoxHostKeys marks merge buckets that contain node +// facets from two or more distinct Proxmox provider scopes. Inside such a +// bucket a bare shared hostname no longer identifies a machine, so +// hostname-only merges of an agent row into a node row must fail closed and +// only the state-layer agent link may attach one (#1753: the Agent badge and +// per-node stats jumped between two sites named "pve" on every refresh). +func presentationAmbiguousProxmoxHostKeys(resources []Resource) map[string]bool { + facetsByKey := make(map[string][]*ProxmoxData) + guarded := make(map[string]bool) + for i := range resources { + facet := resources[i].Proxmox + if facet == nil || strings.TrimSpace(facet.NodeName) == "" { + continue + } + hostKey := presentationHostMergeKey(resources[i]) + if hostKey == "" || guarded[hostKey] { + continue + } + for _, existing := range facetsByKey[hostKey] { + if presentationProxmoxNodeScopesDistinct(existing, facet) { + guarded[hostKey] = true + break + } + } + facetsByKey[hostKey] = append(facetsByKey[hostKey], facet) + } + return guarded +} + +// presentationGuardedMergeAllowed gates merges inside an ambiguous bucket. +// A pair of node facets is already governed by the scope veto, and agent-only +// pairs never satisfy the runtime-platform source requirement, so the case +// that matters is an agent row meeting a node row: it may only attach to the +// node whose state-layer agent link names it. +func presentationGuardedMergeAllowed(left, right Resource) bool { + leftFacet := left.Proxmox != nil && strings.TrimSpace(left.Proxmox.NodeName) != "" + rightFacet := right.Proxmox != nil && strings.TrimSpace(right.Proxmox.NodeName) != "" + if leftFacet == rightFacet { + return true + } + node, agent := left, right + if rightFacet { + node, agent = right, left + } + if agent.Agent == nil { + return false + } + return presentationIdentityValuesEqual(node.Proxmox.LinkedAgentID, agent.Agent.AgentID) +} + func presentationIdentityValuesConflict(left, right string) bool { left = strings.TrimSpace(left) right = strings.TrimSpace(right) @@ -338,6 +431,13 @@ func mergePresentationHostResources(left, right Resource) Resource { } if merged.Proxmox == nil { merged.Proxmox = secondary.Proxmox + // The Proxmox node row's name is display-name aware (the configured + // Node Name), while an agent-backed primary is named after the bare + // reported hostname. Keep the configured name on the merged row + // (#1753: "Node Name field not observed"). + if merged.Proxmox != nil && strings.TrimSpace(secondary.Name) != "" { + merged.Name = secondary.Name + } } if merged.Docker == nil { merged.Docker = secondary.Docker diff --git a/internal/unifiedresources/presentation_coalesce_test.go b/internal/unifiedresources/presentation_coalesce_test.go index d151b04f4..9afdb9bdd 100644 --- a/internal/unifiedresources/presentation_coalesce_test.go +++ b/internal/unifiedresources/presentation_coalesce_test.go @@ -536,3 +536,100 @@ func TestCoalescePresentationHostResourcesKeepsSameShortNameEstatesApart(t *test } }) } + +// Two hand-added standalone Proxmox connections whose native node name is the +// same short hostname (both machines are called "pve") must keep separate +// host rows once their agents connect. The provider node rows carry no +// machine identity of their own, so before the provider-scope veto existed +// the first node row absorbed an agent row and then the other site's node row +// folded in on bare short-hostname compatibility, collapsing the estate into +// one flip-flopping row (#1753, v6.4.1 retest). +func TestCoalescePresentationHostResourcesKeepsStandaloneProviderScopesApart(t *testing.T) { + nodeRow := func(id, instance, host, displayName, linkedAgentID string) Resource { + return Resource{ + ID: id, + Type: ResourceTypeAgent, + Name: displayName, + Status: StatusOnline, + Sources: []DataSource{SourceProxmox}, + Identity: ResourceIdentity{Hostnames: []string{"pve"}}, + Proxmox: &ProxmoxData{ + SourceID: id, + NodeIdentity: id, + NodeName: "pve", + Instance: instance, + HostURL: host, + LinkedAgentID: linkedAgentID, + }, + } + } + agentRow := func(id, machineID string) Resource { + return Resource{ + ID: "agent-" + id, + Type: ResourceTypeAgent, + Name: "pve", + Status: StatusOnline, + Sources: []DataSource{SourceAgent}, + Identity: ResourceIdentity{Hostnames: []string{"pve"}, MachineID: machineID}, + Agent: &AgentData{AgentID: id, Hostname: "pve", MachineID: machineID, OSName: "Proxmox VE"}, + } + } + + t.Run("linked agents keep both sites separate and attached", func(t *testing.T) { + resources := []Resource{ + nodeRow("staging-pve", "hema-staging", "https://pve.hemastaging.hot:8006", "hema-staging", "host-staging"), + agentRow("host-staging", "machine-staging"), + nodeRow("production-pve", "hema-production", "https://pve.hemaproduction.hot:8006", "hema-production", "host-production"), + agentRow("host-production", "machine-production"), + } + got := CoalescePresentationHostResources(resources) + if len(got) != 2 { + t.Fatalf("expected two merged site rows, got %d rows: %#v", len(got), got) + } + for _, resource := range got { + if resource.Proxmox == nil || resource.Agent == nil { + t.Fatalf("expected each site row to keep its node and agent facets, got %#v", resource) + } + want := "host-staging" + if resource.Proxmox.Instance == "hema-production" { + want = "host-production" + } + if resource.Agent.AgentID != want { + t.Fatalf("agent %q attached to instance %q", resource.Agent.AgentID, resource.Proxmox.Instance) + } + } + }) + + t.Run("unlinked agents fail closed instead of collapsing the sites", func(t *testing.T) { + resources := []Resource{ + nodeRow("staging-pve", "hema-staging", "https://192.168.1.10:8006", "hema-staging", ""), + agentRow("host-staging", "machine-staging"), + nodeRow("production-pve", "hema-production", "https://192.168.2.10:8006", "hema-production", ""), + agentRow("host-production", "machine-production"), + } + got := CoalescePresentationHostResources(resources) + nodeRows := 0 + for _, resource := range got { + if resource.Proxmox != nil { + nodeRows++ + } + } + if nodeRows != 2 { + t.Fatalf("expected both provider node rows to survive, got %d in %d rows: %#v", nodeRows, len(got), got) + } + }) + + t.Run("single site still merges its node and agent rows", func(t *testing.T) { + resources := []Resource{ + nodeRow("home-pve", "home", "https://pve.home.lan:8006", "home", ""), + agentRow("host-home", "machine-home"), + } + got := CoalescePresentationHostResources(resources) + if len(got) != 1 || got[0].Proxmox == nil || got[0].Agent == nil { + t.Fatalf("expected single-site node and agent to merge, got %#v", got) + } + if got[0].Name != "home" { + t.Fatalf("expected merged row to keep the configured node name, got %q", got[0].Name) + } + }) +}