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 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") } 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) + } + }) +}