diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 7f6c03e780..9c5dc08450 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -6976,6 +6976,77 @@ section). `TestRegistrySASPathJoinRefusesContradictingWWN` covers the stale row and both same-disk shapes; monitoring's SMART merge applies the matching guard first. +### Linked disk join honours operator splits + +`resolveLinkedPhysicalDisk` joins an agent SMART disk and the Proxmox disk +under one linked host by hardware identity or, for SAS and identity-less +disks, by device path, ahead of `findMatch`. It read no exclusions, so +`POST /api/resources/{id}/report-merge` on such a disk (the drawer's Split +merged resource) recorded its exclusions and answered 200 while the disks +merged again on every rebuild. `resolveLinkedResource` now refuses the join +when an exclusion separates the observation's source-specific candidate ID +from the one disk it matched; a refusal never makes another candidate unique, +so the path join's ambiguity rule stands. `physicalDiskSplitLocked` decides +this for the three joins that match a disk observation by identity or path +(this one, `findMatch` and `physicalDiskIDForMachineLocked`): the exclusion +may name the existing disk's current ID, or the unscoped hardware-keyed or +machine-scoped ID either disk's identity mints. So a split survives the +re-key a same-serial disk on another machine causes, in either direction +(`findMatch` used to read only the current ID), and an agent disk that +smartctl finds in standby, reported without its serial and so keyed by its +device, stays apart from a Proxmox observation still carrying that serial. +Agent disks reach the registry only through the snapshot, which ingests them +before Proxmox disks, so the agent's row holds the merged ID and keeps it, +and the Proxmox observation is the one report-merge's pair against that ID +names. Split, the Proxmox row takes the ID it holds alone or, when both carry +one hardware identity (which the agent's row holds), its source-specific ID +through `findMatch`'s excluded branch; that row takes the identity's ID +while the agent's disk is absent or reports no serial, as for any identity +split. An exclusion naming only the agent's candidate splits nothing, since +the agent's row holds the merged ID, so a report-merge filtered to the agent +source leaves the disks merged (it answers 200, or 400 "No exclusions +created" where the merged ID is the agent's own candidate); the drawer sends +every merged source. The monitor rebuild and a resources API registry built from the +snapshot apply the split on their next build; the registry seeded from the +monitor's listing follows once the monitor has rebuilt. + +What the split does not reach. The PVE disk poller's +`mergeHostAgentSMARTIntoDisks` pairs the linked agent's SMART rows with +Proxmox disks by WWN, serial or path before the registry sees them and reads +no exclusions, so the split Proxmox row still carries the agent's SMART +attributes and I/O, its health where it reports a failure or Proxmox none, +and any serial, WWN, type or temperature it filled, and the agent keeps the +disk's temperature alert while it reports (`AgentSMARTReported`). Two split +rows that share a serial read one serial-keyed history +(`PhysicalDiskMetaMetricID`), and a PVE disk alert lifecycle row whose +recorded identity matches both, with no WWN only one of them shares, stays +under its PVE reference (`proxmoxDiskAlertOwner` never breaks a tie by path), +so neither row's canonical history lists it. Proxmox keys a disk by its slot +(`ProxmoxPhysicalDiskSourceID`), so a split disk that Proxmox reports under +another device path is a new observation the exclusions do not name and +joins again until split again. A split names IDs: one recorded while the +agent's disk reported no serial, or against an agent disk whose identity +Proxmox does not carry (a SAS disk whose Proxmox serial is its transport +address), stops applying while the agent's disk reports otherwise, as every +exclusion keyed by a canonical ID does. In a registry seeded from a listing +where another disk was split onto a slot's source-specific ID, a refused +observation at that slot (next section) whose linked join a split refuses +lands on that same ID and merges into the refused disk, as the slot guard's +own replacement ID would; no production path replays Proxmox disks over a +seeded registry. +`TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost` pins the exclusions +report-merge records (all candidates, or the Proxmox candidate alone), one +naming only the agent's candidate and an unrelated one, for four shapes (a +SAS disk whose Proxmox serial is its transport address, matching NVMe +serials, an agent disk without a serial behind a USB bridge, and Proxmox's +literal `unknown` serial), through three rebuilds of a SQLite-backed monitor +adapter in the monitor listing and the resources API's registry seeded from +the rebuilt listing or built from the snapshot, plain and for presentation; +then a relink of the split rows, the exclusions report-merge records on the +relinked disk (the link's own pair and both candidates), the agent's disk in +standby, a same-serial disk appearing on another machine, and a split +recorded while such a disk was present that holds once it leaves. + ### Seeded slot mapping refuses a replaced disk `ingestRecord` honours a source key's existing mapping, such as one diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 5ced144b64..ce631cbcc6 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -3351,7 +3351,8 @@ func (rr *ResourceRegistry) findMatch(incoming Resource, candidateID string) (*M !physicalDiskMatchScopeCompatible(existing, &incoming) { continue } - if rr.isExcluded(candidate.ID, candidateID) { + if rr.isExcluded(candidate.ID, candidateID) || + (incoming.Type == ResourceTypePhysicalDisk && rr.physicalDiskSplitLocked(existing, &incoming, candidateID)) { excludedMatch = true continue } @@ -3437,7 +3438,10 @@ func physicalDiskTypeIs(disk *Resource, diskType string) bool { func (rr *ResourceRegistry) resolveLinkedResource(source DataSource, sourceID string, resource Resource) string { if resource.Type == ResourceTypePhysicalDisk && (source == SourceAgent || source == SourceProxmox) { - if linked := rr.resolveLinkedPhysicalDisk(source, resource); linked != "" { + // An operator split of the one match refuses the join; it never + // makes another candidate unique. + if linked := rr.resolveLinkedPhysicalDisk(source, resource); linked != "" && + !rr.physicalDiskSplitLocked(rr.resources[linked], &resource, rr.sourceSpecificID(resource.Type, source, sourceID)) { return linked } } @@ -3543,6 +3547,39 @@ func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incomin return matchID } +// physicalDiskSplitLocked reports whether an operator split (exclusion) +// separates a disk observation, named by its source-specific candidate ID, +// from an existing disk. The exclusion may name any ID the joined disk could +// hold: the existing disk's current ID, or the unscoped hardware-keyed ID or +// machine-scoped ID either disk's identity mints. So a split survives a +// re-key, and an existing disk that reports no hardware identity this time +// (an agent disk in standby) stays apart from an observation still carrying +// the identity the split named. Report-merge records the merged disk's ID +// against each source's candidate, and the registry ingests agent disks +// before Proxmox disks, so the agent's row holds the merged ID and the +// Proxmox observation is the one the split names. +func (rr *ResourceRegistry) physicalDiskSplitLocked(existing, incoming *Resource, candidateID string) bool { + if existing == nil || candidateID == "" || len(rr.exclusions) == 0 { + return false + } + if rr.isExcluded(existing.ID, candidateID) { + return true + } + for _, disk := range []*Resource{existing, incoming} { + if disk == nil { + continue + } + if key := physicalDiskHardwareKey(disk.Identity); key != "" && + rr.isExcluded(buildHashID(ResourceTypePhysicalDisk, key), candidateID) { + return true + } + if rr.isExcluded(rr.physicalDiskScopedIDLocked(disk), candidateID) { + return true + } + } + return false +} + // A kernel path identifies a direct device only within its already-correlated // host. Missing identity is not conflicting identity. Never use this fallback // for controller members: the block path can represent several physical disks. @@ -5677,8 +5714,7 @@ func (rr *ResourceRegistry) physicalDiskIDForMachineLocked(id, candidateID strin if rr.physicalDiskMachinesConflictLocked(sibling, incoming, source) { continue } - if rr.isExcluded(sibling.ID, candidateID) || rr.isExcluded(id, candidateID) || - rr.isExcluded(rr.physicalDiskScopedIDLocked(sibling), candidateID) { + if rr.physicalDiskSplitLocked(sibling, incoming, candidateID) { excluded = true continue } diff --git a/internal/unifiedresources/registry_merge_policy_test.go b/internal/unifiedresources/registry_merge_policy_test.go index 8d2cba07cc..cb4d9a4f08 100644 --- a/internal/unifiedresources/registry_merge_policy_test.go +++ b/internal/unifiedresources/registry_merge_policy_test.go @@ -2,6 +2,7 @@ package unifiedresources import ( "encoding/json" + "fmt" "reflect" "slices" "strconv" @@ -3618,3 +3619,303 @@ func TestPhysicalDiskExclusionFallbackStaysPerMachine(t *testing.T) { t.Fatalf("agent disks = %d, want one per host rather than both hosts on the shared fallback ID", agentDisks) } } + +// An operator split separates the disks the registry joins inside one linked +// host. resolveLinkedPhysicalDisk joins an agent SMART disk and the Proxmox +// disk under the same parent by hardware identity or, for SAS and +// identity-less disks, by device path, ahead of findMatch, and it used to +// ignore exclusions: report-merge answered 200 while the disks merged again +// on every rebuild. The registry ingests agent disks first, so the agent's +// row holds the merged ID and keeps it; the Proxmox row takes the ID it holds +// alone, or its source-specific ID where both share a hardware identity (the +// agent holds that ID), as findMatch's excluded branch does. +func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) { + now := time.Now().UTC() + for _, shape := range []struct { + name string + agent models.HostDiskSMART + pve models.PhysicalDisk + }{ + // Proxmox reports the SAS transport address as the serial (#1595), + // so only the device path joins the two. + {"sas-path", + models.HostDiskSMART{Device: "sda", Model: "ST4000NM0023", Serial: "Z1Z0ABCD", Type: "sas", Temperature: 34, Health: "PASSED"}, + models.PhysicalDisk{ID: "lab-pve1--dev-sda", Node: "pve1", Instance: "lab", DevPath: "/dev/sda", Model: "ST4000NM0023", Serial: "5000c500a1b2c3d4", Type: "hdd", Health: "PASSED", LastChecked: now}}, + // One serial on both sides: the linked join matches before findMatch. + {"matching-serial", + models.HostDiskSMART{Device: "nvme0n1", Model: "Samsung 990 PRO", Serial: "S6B0NL0W123456", Type: "nvme", Temperature: 40, Health: "PASSED"}, + models.PhysicalDisk{ID: "lab-pve1--dev-nvme0n1", Node: "pve1", Instance: "lab", DevPath: "/dev/nvme0n1", Model: "Samsung 990 PRO", Serial: "S6B0NL0W123456", Type: "nvme", Health: "PASSED", LastChecked: now}}, + // smartctl reads no serial through this USB bridge. + {"agent-without-identity", + models.HostDiskSMART{Device: "sdb", Model: "USB Disk", Type: "sat", Temperature: 30, Health: "PASSED"}, + models.PhysicalDisk{ID: "lab-pve1--dev-sdb", Node: "pve1", Instance: "lab", DevPath: "/dev/sdb", Model: "USB Disk", Serial: "WD-ABC123", Type: "usb", Health: "PASSED", LastChecked: now}}, + // PVE's literal "unknown" serial and WWN carry no identity. + {"proxmox-unknown-serial", + models.HostDiskSMART{Device: "sdc", Model: "CT1000MX500SSD1", Serial: "2117E59AB1C2", Type: "sata", Temperature: 31, Health: "PASSED"}, + models.PhysicalDisk{ID: "lab-pve1--dev-sdc", Node: "pve1", Instance: "lab", DevPath: "/dev/sdc", Model: "CT1000MX500SSD1", Serial: "unknown", WWN: "unknown", Type: "ssd", Health: "PASSED", LastChecked: now}}, + } { + node := models.Node{ID: "lab-pve1", Name: "pve1", Instance: "lab", Host: "https://10.0.0.5:8006", Status: "online", LastSeen: now, LinkedAgentID: "host-pve1"} + host := models.Host{ID: "host-pve1", Hostname: "pve1", MachineID: "0123456789abcdef", Status: "online", LastSeen: now, LinkedNodeID: node.ID} + host.Sensors.SMART = []models.HostDiskSMART{shape.agent} + snapshot := models.StateSnapshot{LastUpdate: now, Nodes: []models.Node{node}, Hosts: []models.Host{host}, PhysicalDisks: []models.PhysicalDisk{shape.pve}} + // A host on another machine reporting the agent's serial, as a + // cloned VM does. Its hosts sort first, so its disk holds the + // unscoped hardware ID until the host's own disk re-keys both. + clone := models.Host{ID: "host-clone", Hostname: "clone1", MachineID: "fedcba9876543210", Status: "online", LastSeen: now} + clone.Sensors.SMART = []models.HostDiskSMART{shape.agent} + withClone := snapshot + withClone.Hosts = []models.Host{clone, host} + + // The IDs each disk holds alone under the joined host. + alone := func(snapshot models.StateSnapshot) linkedDiskRows { + registry := NewRegistry(nil) + registry.IngestSnapshot(snapshot) + return linkedDiskRowsOn(t, registry.List()) + } + agentOnly := snapshot + agentOnly.PhysicalDisks = nil + agentAlone := alone(agentOnly).agent + pveOnly := snapshot + pveOnly.Hosts = []models.Host{host} + pveOnly.Hosts[0].Sensors.SMART = nil + pveAlone := alone(pveOnly).proxmox + + for _, request := range []struct { + name string + exclusions func(ids diskSplitIDs) [][2]string + split bool + }{ + {"report-merge", func(ids diskSplitIDs) [][2]string { + return [][2]string{{ids.merged, ids.agentCandidate}, {ids.merged, ids.proxmoxCandidate}} + }, true}, + // Report-merge filtered to the Proxmox source, or an unlink + // naming the merged disk and the Proxmox candidate. + {"proxmox-candidate-only", func(ids diskSplitIDs) [][2]string { + return [][2]string{{ids.merged, ids.proxmoxCandidate}} + }, true}, + // Report-merge records this pair whenever it splits any source + // off a disk the agent reports. It names the agent, whose row + // holds the merged ID, not the observation that joins it. + {"agent-candidate-only", func(ids diskSplitIDs) [][2]string { + return [][2]string{{ids.merged, ids.agentCandidate}} + }, false}, + {"unrelated-exclusion", func(ids diskSplitIDs) [][2]string { + return [][2]string{{ids.merged, "physical_disk-00000000deadbeef"}} + }, false}, + } { + t.Run(shape.name+"/"+request.name, func(t *testing.T) { + store, err := NewSQLiteResourceStore(t.TempDir(), "default") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + adapter := NewMonitorAdapter(NewRegistry(store)) + adapter.PopulateFromSnapshot(snapshot) + merged := linkedDiskRowsOn(t, adapter.GetAll()).joined + if merged == "" || merged != agentAlone || pveAlone == "" { + t.Fatalf("fixture did not join the disks under the agent's ID: merged %q, alone: agent %q, proxmox %q", merged, agentAlone, pveAlone) + } + ids := diskSplitCandidates(t, adapter.currentRegistry(), merged) + addExclusions(t, store, request.exclusions(ids)) + + want := linkedDiskRows{joined: merged} + if request.split { + want = linkedDiskRows{agent: agentAlone, proxmox: pveAlone} + if pveAlone == agentAlone { + want.proxmox = ids.proxmoxCandidate + } + } + assertViews := func(step string, snapshot models.StateSnapshot, want func(linkedDiskRows) bool, describe string) { + t.Helper() + for rebuild := 1; rebuild <= 3; rebuild++ { + adapter.PopulateFromSnapshot(snapshot) + listed := adapter.GetAll() + rest := NewRegistry(store) + rest.IngestResources(listed) + restFromSnapshot := NewRegistry(store) + restFromSnapshot.IngestSnapshot(snapshot) + for view, resources := range map[string][]Resource{ + "monitor": listed, + "rest": rest.List(), + "rest presentation": rest.ListForPresentation(), + "rest from snapshot": restFromSnapshot.List(), + "rest presentation snapshot": restFromSnapshot.ListForPresentation(), + } { + if got := linkedDiskRowsOn(t, resources); !want(got) { + t.Fatalf("%s rebuild %d %s: got %+v, want %s", step, rebuild, view, got, describe) + } + } + } + } + exactly := func(want linkedDiskRows) (func(linkedDiskRows) bool, string) { + return func(got linkedDiskRows) bool { return got == want }, fmt.Sprintf("%+v", want) + } + apart := func(got linkedDiskRows) bool { + return got.joined == "" && got.agent != "" && got.proxmox != "" && got.agent != got.proxmox + } + + check, describe := exactly(want) + assertViews("after the request", snapshot, check, describe) + if !request.split { + return + } + + // A link between the rows the split left joins them again, + // and report-merge on the relinked disk splits them again: + // it excludes the link's own pair, which replaces the link. + if err := store.AddLink(ResourceLink{ResourceA: want.agent, ResourceB: want.proxmox, PrimaryID: want.agent, CreatedAt: time.Now().UTC()}); err != nil { + t.Fatal(err) + } + check, describe = exactly(linkedDiskRows{joined: merged}) + assertViews("after relink", snapshot, check, describe) + relinked := adapter.currentRegistry() + var again [][2]string + for _, fold := range relinked.ManualLinkFolds(merged) { + again = append(again, [2]string{fold.HolderID, fold.FoldedID}) + } + ids = diskSplitCandidates(t, relinked, merged) + again = append(again, [2]string{merged, ids.agentCandidate}, [2]string{merged, ids.proxmoxCandidate}) + addExclusions(t, store, again) + check, describe = exactly(want) + assertViews("after splitting again", snapshot, check, describe) + + // smartctl reads no serial from a disk in standby, so the + // agent's row takes its device-keyed ID. A Proxmox + // observation still carrying the serial the split named + // stays apart from it, on the serial's ID meanwhile. + if shape.pve.Serial == shape.agent.Serial { + asleep := snapshot + asleep.Hosts = []models.Host{host} + asleep.Hosts[0].Sensors.SMART = []models.HostDiskSMART{{Device: shape.agent.Device, Model: shape.agent.Model, Type: shape.agent.Type, Standby: true}} + assertViews("while the agent's disk is in standby", asleep, func(got linkedDiskRows) bool { + return apart(got) && got.proxmox == merged + }, "the disks apart, the Proxmox row on "+merged) + check, describe = exactly(want) + assertViews("once the agent's disk wakes", snapshot, check, describe) + } + + // A same-serial disk on another machine re-keys the agent's + // disk to its machine-scoped ID; the split, recorded against + // the unscoped ID, still holds. + if shape.agent.Serial != "" { + assertViews("after a same-serial disk appears elsewhere", withClone, apart, "the disks apart") + } + }) + } + + // A split recorded while a same-serial disk sits on another machine + // names the agent disk's machine-scoped ID, and still holds once that + // disk goes away and the agent's disk returns to the unscoped ID. + if shape.agent.Serial == "" { + continue + } + t.Run(shape.name+"/split-while-scoped", func(t *testing.T) { + store, err := NewSQLiteResourceStore(t.TempDir(), "default") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + adapter := NewMonitorAdapter(NewRegistry(store)) + adapter.PopulateFromSnapshot(withClone) + merged := linkedDiskRowsOn(t, adapter.GetAll()).joined + if merged == "" || merged == agentAlone { + t.Fatalf("fixture did not join the disks under a machine-scoped ID: merged %q, agent alone %q", merged, agentAlone) + } + ids := diskSplitCandidates(t, adapter.currentRegistry(), merged) + addExclusions(t, store, [][2]string{{ids.merged, ids.agentCandidate}, {ids.merged, ids.proxmoxCandidate}}) + for _, step := range []struct { + name string + snapshot models.StateSnapshot + }{{"with the other machine's disk", withClone}, {"after it goes away", snapshot}} { + for rebuild := 1; rebuild <= 2; rebuild++ { + adapter.PopulateFromSnapshot(step.snapshot) + restFromSnapshot := NewRegistry(store) + restFromSnapshot.IngestSnapshot(step.snapshot) + for view, resources := range map[string][]Resource{"monitor": adapter.GetAll(), "rest from snapshot": restFromSnapshot.List()} { + if got := linkedDiskRowsOn(t, resources); got.joined != "" || got.agent == "" || got.proxmox == "" { + t.Fatalf("%s rebuild %d %s: got %+v, want the disks apart", step.name, rebuild, view, got) + } + } + } + } + }) + } +} + +// diskSplitIDs are the IDs a report-merge of a joined agent and Proxmox disk +// names: the merged disk and its SourceTargets candidates. +type diskSplitIDs struct { + merged, agentCandidate, proxmoxCandidate string +} + +func diskSplitCandidates(t *testing.T, registry *ResourceRegistry, mergedID string) diskSplitIDs { + t.Helper() + ids := diskSplitIDs{merged: mergedID} + for _, target := range registry.SourceTargets(mergedID) { + switch target.Source { + case SourceAgent: + ids.agentCandidate = target.CandidateID + case SourceProxmox: + ids.proxmoxCandidate = target.CandidateID + } + } + if ids.agentCandidate == "" || ids.proxmoxCandidate == "" || ids.proxmoxCandidate == mergedID { + t.Fatalf("merged disk %s lists candidates agent=%q proxmox=%q", mergedID, ids.agentCandidate, ids.proxmoxCandidate) + } + return ids +} + +// addExclusions records each pair as report-merge does, skipping a pair that +// names one ID twice (an identity-less agent disk's merged ID is its +// candidate). +func addExclusions(t *testing.T, store ResourceStore, pairs [][2]string) { + t.Helper() + for _, pair := range pairs { + if pair[0] == pair[1] { + continue + } + if err := store.AddExclusion(ResourceExclusion{ResourceA: pair[0], ResourceB: pair[1], CreatedAt: time.Now().UTC()}); err != nil { + t.Fatal(err) + } + } +} + +type linkedDiskRows struct { + agent string // a disk only the agent reports + proxmox string // a disk only Proxmox reports + joined string // a disk both report +} + +// linkedDiskRowsOn sorts the disks under the joined pve1 host by the sources +// that report them, ignoring disks on other machines. +func linkedDiskRowsOn(t *testing.T, resources []Resource) linkedDiskRows { + t.Helper() + hostID := "" + for _, resource := range resources { + if resource.Type == ResourceTypeAgent && resource.Agent != nil && resource.Agent.AgentID == "host-pve1" { + hostID = resource.ID + } + } + var rows linkedDiskRows + set := func(slot *string, id string) { + if *slot != "" { + t.Fatalf("two disk rows of one kind: %s and %s", *slot, id) + } + *slot = id + } + for _, resource := range resources { + if resource.Type != ResourceTypePhysicalDisk || resource.ParentID == nil || *resource.ParentID != hostID { + continue + } + agent, proxmox := hasDataSource(resource.Sources, SourceAgent), hasDataSource(resource.Sources, SourceProxmox) + switch { + case agent && proxmox: + set(&rows.joined, resource.ID) + case agent: + set(&rows.agent, resource.ID) + case proxmox: + set(&rows.proxmox, resource.ID) + } + } + return rows +}