fix(monitoring): preserve physical disk source identity on refresh

Skipped disk polls must write the Proxmox source key back into inventory rather than hashing the canonical resource ID again. Add repeated-cycle runtime and JSON projection coverage for serial-less disks, distinct node/controller scopes and removal.

Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
2026-09-13 18:25:42 +01:00
parent a081769c05
commit 01742e2279
7 changed files with 192 additions and 1 deletions
@@ -8039,3 +8039,20 @@ an isolated copy of the same test binary. Joining that probe's handler does not
join upgraded handlers from preceding tests; process isolation keeps their final
log writes from racing with the hook. The child retains race instrumentation and
an explicit timeout, and the original server still fails the ordering assertion.
### Physical-disk provider identity round trip
The monitor's disk readback preserves the Proxmox source ID separately from
its canonical resource ID. Skipped physical-disk polls may refresh SMART data
but must not feed canonical hashes into source inventory, changing identity at
each subsequent projection. Views without Proxmox source metadata retain the
legacy ID fallback. This does not relax cross-host/device correlation, migrate
old duplicates, change agent authority, or redefine serial-less disk matching.
`TestPhysicalDiskSkippedPollPreservesSourceIdentity` in
`internal/monitoring/physical_disk_roundtrip_test.go` drives eight real skipped
polls and adapter rebuilds for both path-shaped and bare device names, checking
provider keys, canonical IDs, JSON projection, temperature/size/cadence, separate
nodes/controller members, and confirmed inventory removal.
`TestPhysicalDiskReadbackSourceIDFallback` covers missing source metadata.
This is synthetic runtime evidence, not USB hardware or reporter acceptance.
@@ -4027,3 +4027,20 @@ and acknowledgement, requiring no queued notification. Existing routing,
quiet-hours and cooldown tests now use real active manager occurrences rather
than fabricated inactive alerts. This is callback-boundary proof, not a claim
that already admitted or provider-accepted deliveries can be recalled.
### Physical-disk provider identity round trip
The monitor's disk readback preserves the Proxmox source ID separately from
its canonical resource ID. Skipped physical-disk polls may refresh SMART data
but must not feed canonical hashes into source inventory, changing identity at
each subsequent projection. Views without Proxmox source metadata retain the
legacy ID fallback. This does not relax cross-host/device correlation, migrate
old duplicates, change agent authority, or redefine serial-less disk matching.
`TestPhysicalDiskSkippedPollPreservesSourceIdentity` in
`internal/monitoring/physical_disk_roundtrip_test.go` drives eight real skipped
polls and adapter rebuilds for both path-shaped and bare device names, checking
provider keys, canonical IDs, JSON projection, temperature/size/cadence, separate
nodes/controller members, and confirmed inventory removal.
`TestPhysicalDiskReadbackSourceIDFallback` covers missing source metadata.
This is synthetic runtime evidence, not USB hardware or reporter acceptance.
@@ -1623,6 +1623,7 @@
"internal/monitoring/docker_metric_presence_test.go",
"internal/monitoring/monitor_host_agent_removal_lifecycle_test.go",
"internal/monitoring/monitor_host_agents_test.go",
"internal/monitoring/physical_disk_roundtrip_test.go",
"scripts/installtests/agent_state_dir_lifecycle_test.go",
"scripts/installtests/install_ps1_test.go",
"scripts/installtests/install_sh_test.go"
@@ -8218,6 +8219,7 @@
"internal/monitoring/docker_filesystem_evidence_test.go",
"internal/monitoring/issue1595_collection_trust_test.go",
"internal/monitoring/issue1913_host_continuity_test.go",
"internal/monitoring/physical_disk_roundtrip_test.go",
"internal/unifiedresources/adapter_coverage_test.go",
"internal/unifiedresources/adapters_test.go",
"internal/unifiedresources/ceph_pool_health_contract_test.go",
@@ -5214,3 +5214,20 @@ inference and presentation path, including preservation of the reciprocal link;
`TestProxmoxOneWayLinkRejectsHostLocalEndpointCorroboration` protect management
network and explicit report-IP compatibility. These synthetic cases do not
establish the cause of a particular installation's association.
### Physical-disk provider identity round trip
The monitor's disk readback preserves the Proxmox source ID separately from
its canonical resource ID. Skipped physical-disk polls may refresh SMART data
but must not feed canonical hashes into source inventory, changing identity at
each subsequent projection. Views without Proxmox source metadata retain the
legacy ID fallback. This does not relax cross-host/device correlation, migrate
old duplicates, change agent authority, or redefine serial-less disk matching.
`TestPhysicalDiskSkippedPollPreservesSourceIdentity` in
`internal/monitoring/physical_disk_roundtrip_test.go` drives eight real skipped
polls and adapter rebuilds for both path-shaped and bare device names, checking
provider keys, canonical IDs, JSON projection, temperature/size/cadence, separate
nodes/controller members, and confirmed inventory removal.
`TestPhysicalDiskReadbackSourceIDFallback` covers missing source metadata.
This is synthetic runtime evidence, not USB hardware or reporter acceptance.
+7 -1
View File
@@ -735,8 +735,14 @@ func physicalDiskFromReadStateView(view *unifiedresources.PhysicalDiskView) mode
return models.PhysicalDisk{Wearout: unifiedresources.WearoutUnreported}
}
// Preserve the provider key across the canonical read-state round trip.
// Older/synthetic views without source metadata retain their existing fallback.
sourceID := view.SourceID()
if sourceID == "" {
sourceID = view.ID()
}
disk := models.PhysicalDisk{
ID: view.ID(),
ID: sourceID,
Node: view.Node(),
Instance: view.Instance(),
DevPath: view.DevPath(),
@@ -0,0 +1,122 @@
package monitoring
import (
"context"
"encoding/json"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
)
// Exercise the real skipped-poll path, not just the ID helper: canonical views
// are converted back into source state and then re-ingested by the adapter.
func TestPhysicalDiskSkippedPollPreservesSourceIdentity(t *testing.T) {
for _, device := range []string{"/dev/sdx", "sdx"} {
t.Run(device, func(t *testing.T) {
state := models.NewState()
nodes := []models.Node{{ID: "pve-node1", Name: "node1", Instance: "pve"}, {ID: "pve-node2", Name: "node2", Instance: "pve"}}
state.UpdateNodesForInstance("pve", nodes)
var disks []models.PhysicalDisk
for _, fixture := range []struct{ node, target string }{{"node1", ""}, {"node2", ""}, {"node1", "megaraid,0"}, {"node1", "megaraid,1"}} {
disks = append(disks, models.PhysicalDisk{
ID: unifiedresources.ProxmoxPhysicalDiskSourceID("pve", fixture.node, device, "", fixture.target),
Instance: "pve", Node: fixture.node, DevPath: device, Target: fixture.target,
Model: "USB fixture", Size: 1024, Temperature: 37, Wearout: -1, LastChecked: time.Now(),
ExpectedUpdateInterval: 5 * time.Minute,
})
}
state.UpdatePhysicalDisks("pve", disks)
adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil))
adapter.PopulateFromSnapshot(state.GetSnapshot())
m := &Monitor{state: state, resourceStore: adapter, lastPhysicalDiskPoll: map[string]time.Time{"pve": time.Now()}}
canonical := map[string]bool{}
for _, view := range adapter.PhysicalDisks() {
canonical[view.ID()] = true
}
if len(canonical) != len(disks) {
t.Fatalf("initial disks = %d, want %d", len(canonical), len(disks))
}
for cycle := 0; cycle < 8; cycle++ {
m.maybePollPhysicalDisksAsync(context.Background(), "pve", &config.PVEInstance{}, nil, nil, nil, nil)
got := state.GetSnapshot().PhysicalDisks
if len(got) != len(disks) {
t.Fatalf("cycle %d: source count %d", cycle, len(got))
}
wantIDs := map[string]bool{}
for _, disk := range disks {
wantIDs[disk.ID] = true
}
for _, disk := range got {
if !wantIDs[disk.ID] {
t.Fatalf("cycle %d: canonical ID leaked into provider state: %q", cycle, disk.ID)
}
if disk.DevPath != device || disk.Temperature != 37 || disk.Size != 1024 || disk.ExpectedUpdateInterval != 5*time.Minute {
t.Fatalf("cycle %d: metadata lost: %+v", cycle, disk)
}
}
adapter.PopulateFromSnapshot(state.GetSnapshot())
views := adapter.PhysicalDisks()
if len(views) != len(disks) {
t.Fatalf("cycle %d: view count = %d", cycle, len(views))
}
for _, view := range views {
if !canonical[view.ID()] {
t.Fatalf("cycle %d: canonical identity churn: %s", cycle, view.ID())
}
}
// Retain the JSON-visible projection as well as the typed read-state checks.
payload, err := json.Marshal(adapter.GetAll())
if err != nil {
t.Fatal(err)
}
var resources []unifiedresources.Resource
if err := json.Unmarshal(payload, &resources); err != nil {
t.Fatal(err)
}
count := 0
for _, resource := range resources {
if resource.Type != unifiedresources.ResourceTypePhysicalDisk {
continue
}
count++
if !canonical[resource.ID] || resource.Proxmox == nil || !wantIDs[resource.Proxmox.SourceID] || resource.PhysicalDisk == nil || resource.PhysicalDisk.Temperature != 37 {
t.Fatalf("cycle %d: JSON disk identity/metadata lost: %+v", cycle, resource)
}
}
if count != len(disks) {
t.Fatalf("cycle %d: JSON disk count %d", cycle, count)
}
}
// A confirmed full inventory removal must still remove source-owned disks.
state.UpdatePhysicalDisks("pve", nil)
adapter.PopulateFromSnapshot(state.GetSnapshot())
if got := len(adapter.PhysicalDisks()); got != 0 {
t.Fatalf("removed inventory retained %d disks", got)
}
})
}
}
func TestPhysicalDiskReadbackSourceIDFallback(t *testing.T) {
for _, resource := range []unifiedresources.Resource{
{ID: "canonical"},
{ID: "canonical", Proxmox: &unifiedresources.ProxmoxData{}},
{ID: "canonical", Proxmox: &unifiedresources.ProxmoxData{SourceID: " native "}},
} {
view := unifiedresources.NewPhysicalDiskView(&resource)
want := "canonical"
if resource.Proxmox != nil && resource.Proxmox.SourceID != "" {
want = "native"
}
if got := physicalDiskFromReadStateView(&view).ID; got != want {
t.Fatalf("ID = %q, want %q", got, want)
}
}
var view unifiedresources.PhysicalDiskView
if view.SourceID() != "" {
t.Fatal("nil resource has a source ID")
}
}
+10
View File
@@ -2120,6 +2120,16 @@ func (v PhysicalDiskView) ID() string {
return v.r.ID
}
// SourceID returns the Proxmox-native inventory key, not the canonical
// resource ID. Polling adapters must preserve this key when writing views back
// into provider state; hashing a canonical ID again creates a new resource.
func (v PhysicalDiskView) SourceID() string {
if v.r == nil || v.r.Proxmox == nil {
return ""
}
return strings.TrimSpace(v.r.Proxmox.SourceID)
}
func (v PhysicalDiskView) Name() string {
if v.r == nil {
return ""