From 23a930e849896a5d0d70d8d1d94a9ac520f1566a Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 7 Jul 2026 22:55:04 +0100 Subject: [PATCH] fix(discovery): bring hosts into the fingerprint model so they auto-discover (#1479) Hosts (PVE nodes and host agents) were entirely outside the discovery fingerprint model. Two consequences, one root: 1. collectFingerprints only covered docker/lxc/vm/k8s, so a host never appeared in GetChangedResources and automatic discovery never ran for it. Only the per-resource manual trigger worked, which is exactly the v6.0.5-rc.3 behaviour reported in #1479. 2. cleanupOrphanedData built its keep-set from the same four types, so CleanupOrphanedDiscoveries swept every agent:* discovery as an orphan on the next fingerprint cycle (default 5 minutes). Manually discovered hosts silently reverted to undiscovered. Fix: add GenerateHostFingerprint (identity, OS, kernel, arch, tags; status excluded so online/offline flapping cannot trigger rediscovery), fingerprint snap.Hosts under the canonical agent:: key, and keep host/node discovery keys (canonical, hostname-alias, and legacy host: prefix forms) out of orphan cleanup. Also purge the store's in-memory cache when an orphaned discovery file is removed, so deletions are not masked for the cache TTL. --- internal/servicediscovery/fingerprint.go | 42 +++++++++ internal/servicediscovery/service.go | 84 ++++++++++++++++- internal/servicediscovery/service_test.go | 105 ++++++++++++++++++++++ internal/servicediscovery/store.go | 4 + 4 files changed, 234 insertions(+), 1 deletion(-) diff --git a/internal/servicediscovery/fingerprint.go b/internal/servicediscovery/fingerprint.go index d3bddc3e5..9169f0355 100644 --- a/internal/servicediscovery/fingerprint.go +++ b/internal/servicediscovery/fingerprint.go @@ -186,6 +186,48 @@ func GenerateVMFingerprint(nodeID string, vm *VM) *ContainerFingerprint { return fp } +// GenerateHostFingerprint creates a fingerprint from host-agent metadata. +// Tracks: agent ID, hostname, platform, OS identity, kernel, architecture, +// and tags. Status is intentionally excluded — online/offline flapping must +// not trigger rediscovery. +func GenerateHostFingerprint(host *Host) *ContainerFingerprint { + fp := &ContainerFingerprint{ + ResourceID: host.ID, + TargetID: host.ID, + SchemaVersion: FingerprintSchemaVersion, + GeneratedAt: time.Now(), + ImageName: host.OSName, + } + + var components []string + + // Core identity + components = append(components, host.ID) + components = append(components, host.Hostname) + + // OS identity (kernel/OS upgrades should trigger rediscovery) + components = append(components, host.Platform) + components = append(components, host.OSName) + components = append(components, host.OSVersion) + components = append(components, host.KernelVersion) + components = append(components, host.Architecture) + components = append(components, strconv.Itoa(host.CPUCount)) + + // Tags + if len(host.Tags) > 0 { + sortedTags := make([]string, len(host.Tags)) + copy(sortedTags, host.Tags) + sort.Strings(sortedTags) + components = append(components, sortedTags...) + } + + h := sha256.New() + h.Write([]byte(strings.Join(components, "|"))) + fp.Hash = hex.EncodeToString(h.Sum(nil))[:16] + + return fp +} + // GenerateK8sPodFingerprint creates a fingerprint from Kubernetes pod metadata. // Tracks: UID, name, namespace, labels, owner (deployment/statefulset/etc), and container images. func GenerateK8sPodFingerprint(clusterID string, pod *KubernetesPod) *ContainerFingerprint { diff --git a/internal/servicediscovery/service.go b/internal/servicediscovery/service.go index 025fda07e..908d95817 100644 --- a/internal/servicediscovery/service.go +++ b/internal/servicediscovery/service.go @@ -1101,6 +1101,65 @@ func (s *Service) collectFingerprints(ctx context.Context) { newCount += vmNew changedCount += vmChanged + // Process agent hosts (PVE nodes and standalone hosts with a Pulse agent). + // Without a fingerprint a host never enters GetChangedResources, so a new + // host would only ever be discovered by a manual per-resource trigger. + for i := range snap.Hosts { + host := &snap.Hosts[i] + select { + case <-ctx.Done(): + return + default: + } + if strings.TrimSpace(host.ID) == "" { + continue + } + + newFP := GenerateHostFingerprint(host) + // Match the canonical host discovery key: agent::. + fpKey := string(ResourceTypeAgent) + ":" + host.ID + ":" + host.ID + + oldFP, err := s.store.GetFingerprint(fpKey) + if err != nil { + log.Warn(). + Err(err). + Str("resource_id", fpKey). + Str("host", host.Hostname). + Msg("Failed to load previous host fingerprint") + } + + newFP.ResourceID = fpKey + + if err := s.store.SaveFingerprint(newFP); err != nil { + log.Warn().Err(err).Str("host", host.Hostname).Msg("failed to save host fingerprint") + continue + } + + if oldFP == nil { + newCount++ + log.Debug(). + Str("type", "agent"). + Str("host", host.Hostname). + Str("hash", newFP.Hash). + Msg("New fingerprint captured") + } else if newFP.HasSchemaChanged(oldFP) { + log.Debug(). + Str("type", "agent"). + Str("host", host.Hostname). + Int("old_schema", oldFP.SchemaVersion). + Int("new_schema", newFP.SchemaVersion). + Msg("Fingerprint schema updated") + } else if oldFP.Hash != newFP.Hash { + changedCount++ + log.Info(). + Str("type", "agent"). + Str("host", host.Hostname). + Str("old_hash", oldFP.Hash). + Str("new_hash", newFP.Hash). + Msg("Fingerprint changed - discovery will run on next request") + } + } + // Process Kubernetes pods for _, cluster := range snap.KubernetesClusters { for _, pod := range cluster.Pods { @@ -1281,7 +1340,7 @@ func (s *Service) processFingerprint( func (s *Service) cleanupOrphanedData(snap StateSnapshot) { // Safety check: Don't cleanup if state appears empty // This prevents catastrophic deletion if state provider has an error - totalResources := len(snap.Containers) + len(snap.VMs) + len(snap.KubernetesClusters) + totalResources := len(snap.Containers) + len(snap.VMs) + len(snap.KubernetesClusters) + len(snap.Hosts) for _, host := range snap.DockerHosts { totalResources += len(host.Containers) } @@ -1321,6 +1380,29 @@ func (s *Service) cleanupOrphanedData(snap StateSnapshot) { } } + // Agent hosts and PVE nodes. Host discoveries are keyed canonically by + // agent UUID (agent::) but legacy/alias records may be keyed by + // hostname or node name — keep every form so live hosts are never swept + // as orphans. + addAgentKey := func(id string) { + id = strings.TrimSpace(id) + if id == "" { + return + } + currentIDs[string(ResourceTypeAgent)+":"+id+":"+id] = true + // Pre-v6 records were stored under the legacy "host" alias and IDs + // are persisted verbatim, so keep that form too. + currentIDs[string(legacyHostAlias)+":"+id+":"+id] = true + } + for _, host := range snap.Hosts { + addAgentKey(host.ID) + addAgentKey(host.Hostname) + } + for _, node := range snap.Nodes { + addAgentKey(node.ID) + addAgentKey(node.Name) + } + // Run cleanup fpRemoved := s.store.CleanupOrphanedFingerprints(currentIDs) discRemoved := s.store.CleanupOrphanedDiscoveries(currentIDs) diff --git a/internal/servicediscovery/service_test.go b/internal/servicediscovery/service_test.go index 7f1221119..1e2677322 100644 --- a/internal/servicediscovery/service_test.go +++ b/internal/servicediscovery/service_test.go @@ -1083,6 +1083,111 @@ func TestService_CollectFingerprints_LXCAndVM(t *testing.T) { } } +// Regression for #1479: hosts never entered the fingerprint model, so a new +// PVE host / host agent never appeared in GetChangedResources and automatic +// discovery never ran for it — only per-resource manual triggers worked. +func TestService_CollectFingerprints_HostsBecomeAutoDiscoveryCandidates(t *testing.T) { + store, err := NewStore(t.TempDir()) + if err != nil { + t.Fatalf("NewStore error: %v", err) + } + store.crypto = nil + + state := StateSnapshot{ + Hosts: []Host{ + {ID: "agent-uuid-1", Hostname: "pve-one", Platform: "linux", OSName: "Debian", OSVersion: "13", KernelVersion: "6.8.0", Architecture: "amd64", Status: "online"}, + }, + Containers: []Container{ + {VMID: 100, Name: "lxc-one", Node: "pve-one", Status: "running", OSTemplate: "debian-12", CPUs: 2, MaxMemory: 2 << 30}, + }, + } + + service := NewService(store, nil, DefaultConfig()) + service.SetReadState(readStateFromSnapshot(state)) + + service.collectFingerprints(context.Background()) + + hostKey := "agent:agent-uuid-1:agent-uuid-1" + fp, err := store.GetFingerprint(hostKey) + if err != nil { + t.Fatalf("GetFingerprint(%q) error: %v", hostKey, err) + } + if fp == nil || fp.Hash == "" { + t.Fatalf("expected host fingerprint at %q, got %#v", hostKey, fp) + } + + changed, err := store.GetChangedResources() + if err != nil { + t.Fatalf("GetChangedResources error: %v", err) + } + found := false + for _, id := range changed { + if id == hostKey { + found = true + break + } + } + if !found { + t.Fatalf("expected undiscovered host %q in changed resources, got %v", hostKey, changed) + } +} + +// Regression for #1479: cleanupOrphanedData built its keep-set only from +// docker/system-container/vm/k8s IDs, so every host discovery (agent:*) was +// deleted as an orphan on the next fingerprint cycle — a manually discovered +// host reverted to undiscovered within minutes. +func TestService_CleanupPreservesLiveHostDiscoveries(t *testing.T) { + store, err := NewStore(t.TempDir()) + if err != nil { + t.Fatalf("NewStore error: %v", err) + } + store.crypto = nil + + live := &ResourceDiscovery{ + ID: "agent:agent-uuid-1:agent-uuid-1", + ResourceType: ResourceTypeAgent, + TargetID: "agent-uuid-1", + ResourceID: "agent-uuid-1", + Hostname: "pve-one", + ServiceType: "proxmox", + } + orphan := &ResourceDiscovery{ + ID: "agent:gone-uuid:gone-uuid", + ResourceType: ResourceTypeAgent, + TargetID: "gone-uuid", + ResourceID: "gone-uuid", + Hostname: "decommissioned", + ServiceType: "debian", + } + for _, d := range []*ResourceDiscovery{live, orphan} { + if err := store.Save(d); err != nil { + t.Fatalf("Save(%s) error: %v", d.ID, err) + } + } + + state := StateSnapshot{ + Hosts: []Host{ + {ID: "agent-uuid-1", Hostname: "pve-one", Platform: "linux", Status: "online"}, + }, + Containers: []Container{ + {VMID: 100, Name: "lxc-one", Node: "pve-one", Status: "running"}, + }, + } + + service := NewService(store, nil, DefaultConfig()) + service.SetReadState(readStateFromSnapshot(state)) + service.cleanupOrphanedData(state) + + if got, err := store.Get(live.ID); err != nil || got == nil { + t.Fatalf("live host discovery was swept as an orphan: got %#v err=%v", got, err) + } + if got, err := store.Get(orphan.ID); err != nil { + t.Fatalf("Get(orphan) error: %v", err) + } else if got != nil { + t.Fatalf("expected decommissioned host discovery to be removed, still present: %#v", got) + } +} + func TestService_PromptsAndDiscoveryLoop(t *testing.T) { service := NewService(nil, nil, DefaultConfig()) diff --git a/internal/servicediscovery/store.go b/internal/servicediscovery/store.go index 9b7eaacb4..1bd402e1c 100644 --- a/internal/servicediscovery/store.go +++ b/internal/servicediscovery/store.go @@ -1012,6 +1012,10 @@ func (s *Store) CleanupOrphanedDiscoveries(currentResourceIDs map[string]bool) i if err := os.Remove(filePath); err != nil { log.Warn().Err(err).Str("file", entry.Name()).Msg("failed to remove orphaned discovery file") } else { + s.mu.Lock() + delete(s.cache, resourceID) + delete(s.cacheTime, resourceID) + s.mu.Unlock() log.Debug().Str("id", resourceID).Msg("removed orphaned discovery") removed++ }