mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
fix(alerts): resolve PVE disk alerts to physical-disk policy
Keep persisted PVE disk alert identities stable while indexing them as exact physical-disk aliases. A muted disk now clears and suppresses its wearout alert without suppressing neighbouring disks. Update both subsystem contracts and add alert-identity, canonical-alias and registry-backed policy proofs. Change-source: pulse-maintainer
This commit is contained in:
@@ -1664,6 +1664,18 @@ Proxmox disk health alert evaluation now lives in
|
||||
identity, disk health assessment alerts, known-firmware health suppression, and
|
||||
SSD wearout alerts; future Proxmox disk-health behavior should extend that
|
||||
checker owner rather than expanding the central Manager file.
|
||||
PVE disk-health and wearout occurrences retain the existing
|
||||
`instance:node:disk:sanitized-device-path` resource reference. The shared
|
||||
constructor in unified resources must produce the same identity as the alert
|
||||
checker; changing occurrence IDs to match registry source IDs would reopen
|
||||
historical incidents and can re-notify a worn disk. The registry resolves this
|
||||
reference as an alias of the exact physical disk for operator-state lookups.
|
||||
Muting one physical disk therefore resolves its active wearout alert and blocks
|
||||
later writes and notifications without muting another disk or changing the
|
||||
wearout recovery rules. `TestProxmoxDiskCanonicalResourceIDTrimsIdentity` pins
|
||||
the persisted identity shape; monitoring's registry-backed
|
||||
`TestProxmoxPhysicalDiskMuteResolvesAndSuppressesWearoutAlert` pins the policy
|
||||
path. These are source invariants, not evidence of installed field relief.
|
||||
Shared metric threshold runtime now lives in
|
||||
`internal/alerts/metric_runtime.go`. That file owns metric threshold lookup,
|
||||
per-metric delay and intent resolution, reducer input composition, active-alert
|
||||
|
||||
@@ -4483,6 +4483,15 @@ Canonical physical-disk views now expose the full disk identity and SMART
|
||||
metadata needed by monitoring refresh paths, so physical-disk temperature and
|
||||
SMART merges can run from unified `ReadState` instead of from snapshot-owned
|
||||
disk arrays.
|
||||
For a Proxmox physical disk with a complete instance, node and device path,
|
||||
canonical aliases include the persisted PVE disk-alert reference
|
||||
`instance:node:disk:sanitized-device-path`. The alert checker and registry use
|
||||
one constructor for that legacy reference. It is a lookup alias, never the
|
||||
disk's primary canonical ID or source ID, and must not leak across nodes,
|
||||
instances or non-disk resources. Ambiguous alias claims still fail closed under
|
||||
normal reference resolution. `TestProxmoxDiskAlertAliasIsScopedAndNotPrimaryIdentity`
|
||||
pins the identity boundary, and the registry-backed monitoring mute test pins
|
||||
its operator-state consumer.
|
||||
When host-agent SMART and Proxmox physical-disk rows merge, the unified
|
||||
resource must preserve both the enriched `PhysicalDiskMeta` and the Proxmox
|
||||
source payload (`ProxmoxData.NodeName`, `Instance`, and source id). A
|
||||
|
||||
@@ -423,9 +423,18 @@ func TestBuildBackupPVETemplateSubjectKeyTrimsParts(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestProxmoxDiskCanonicalResourceIDTrimsIdentity(t *testing.T) {
|
||||
got := proxmoxDiskCanonicalResourceID(" inst ", " node ", "/dev/sda")
|
||||
if got != "inst:node:disk:dev-sda" {
|
||||
t.Fatalf("proxmoxDiskCanonicalResourceID() = %q, want %q", got, "inst:node:disk:dev-sda")
|
||||
for _, tt := range []struct {
|
||||
name, instance, node, path, want string
|
||||
}{
|
||||
{"trimmed source identity", " inst ", " node ", "/dev/sda", "inst:node:disk:dev-sda"},
|
||||
{"nested device path", "inst", "node", "/dev/disk/by-id/ATA_DISK", "inst:node:disk:dev-disk-by-id-ata-disk"},
|
||||
{"root path", "inst", "node", "/", "inst:node:disk:root"},
|
||||
} {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := proxmoxDiskCanonicalResourceID(tt.instance, tt.node, tt.path); got != tt.want {
|
||||
t.Fatalf("proxmoxDiskCanonicalResourceID() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import (
|
||||
)
|
||||
|
||||
func proxmoxDiskCanonicalResourceID(instance, node, devPath string) string {
|
||||
return fmt.Sprintf("%s:%s:disk:%s", strings.TrimSpace(instance), strings.TrimSpace(node), sanitizeAlertKey(devPath))
|
||||
return unifiedresources.ProxmoxPhysicalDiskAlertResourceID(instance, node, devPath)
|
||||
}
|
||||
|
||||
func proxmoxDiskAlertMetadata(disk proxmox.Disk) map[string]interface{} {
|
||||
|
||||
@@ -39,6 +39,59 @@ func TestInstallOperatorIntentResolverProjectsCanonicalResourcePolicy(t *testing
|
||||
}
|
||||
}
|
||||
|
||||
func TestProxmoxPhysicalDiskMuteResolvesAndSuppressesWearoutAlert(t *testing.T) {
|
||||
store := unifiedresources.NewMemoryStore()
|
||||
registry := unifiedresources.NewRegistry(store)
|
||||
instance, node := "pve", "rocket"
|
||||
makeDisk := func(path string) models.PhysicalDisk {
|
||||
return models.PhysicalDisk{
|
||||
ID: unifiedresources.ProxmoxPhysicalDiskSourceID(instance, node, path, "", ""),
|
||||
Instance: instance, Node: node, DevPath: path,
|
||||
Model: "KINGSTON SA400", Type: "ssd", Health: "PASSED", Wearout: 0,
|
||||
LastChecked: time.Now().UTC(),
|
||||
}
|
||||
}
|
||||
registry.IngestSnapshot(models.StateSnapshot{
|
||||
PhysicalDisks: []models.PhysicalDisk{makeDisk("/dev/sda"), makeDisk("/dev/sdb")},
|
||||
})
|
||||
diskID := unifiedresources.ProxmoxPhysicalDiskAlertResourceID(instance, node, "/dev/sda")
|
||||
canonicalID, found := registry.ResolveReferenceID(diskID)
|
||||
if !found {
|
||||
t.Fatalf("PVE wearout alert resource %q did not resolve to a physical disk", diskID)
|
||||
}
|
||||
otherID := unifiedresources.ProxmoxPhysicalDiskAlertResourceID(instance, node, "/dev/sdb")
|
||||
if other, ok := registry.ResolveReferenceID(otherID); !ok || other == canonicalID {
|
||||
t.Fatalf("other PVE disk identity = %q, %v; want a different disk", other, ok)
|
||||
}
|
||||
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
t.Cleanup(manager.Stop)
|
||||
worn := proxmox.Disk{DevPath: "/dev/sda", Model: "KINGSTON SA400", Type: "ssd", Health: "PASSED", Wearout: 0}
|
||||
manager.CheckDiskHealth(instance, node, worn)
|
||||
if got := len(manager.GetActiveAlerts()); got != 1 {
|
||||
t.Fatalf("unmuted worn disk raised %d alerts, want one", got)
|
||||
}
|
||||
if err := store.SetResourceOperatorState(unifiedresources.ResourceOperatorState{
|
||||
CanonicalID: canonicalID, MonitoringMode: unifiedresources.MonitoringModeMuted,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
monitor := &Monitor{alertManager: manager}
|
||||
monitor.installOperatorIntentResolver(unifiedresources.NewMonitorAdapter(registry))
|
||||
if got := len(manager.GetActiveAlerts()); got != 0 {
|
||||
t.Fatalf("mute left %d existing PVE wearout alerts active", got)
|
||||
}
|
||||
manager.CheckDiskHealth(instance, node, worn)
|
||||
if got := len(manager.GetActiveAlerts()); got != 0 {
|
||||
t.Fatalf("muted PVE disk raised %d new wearout alerts", got)
|
||||
}
|
||||
worn.DevPath = "/dev/sdb"
|
||||
manager.CheckDiskHealth(instance, node, worn)
|
||||
if got := len(manager.GetActiveAlerts()); got != 1 {
|
||||
t.Fatalf("other PVE disk raised %d alerts, want one", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallOperatorIntentResolverInheritsScopedMaintenanceFromParent(t *testing.T) {
|
||||
store := unifiedresources.NewMemoryStore()
|
||||
registry := unifiedresources.NewRegistry(store)
|
||||
|
||||
@@ -117,6 +117,16 @@ func canonicalAliases(resource Resource, primaryID, platformID, hostname string)
|
||||
strings.TrimSpace(resource.Proxmox.NodeName),
|
||||
)
|
||||
values = append(values, resource.Proxmox.NodeAliases...)
|
||||
// PVE disk alerts predate the unified resource registry and retain a
|
||||
// path-shaped resource reference. Resolve it to this exact physical
|
||||
// disk so a UI mute reaches both new and restored occurrences (#2112).
|
||||
if CanonicalResourceType(resource.Type) == ResourceTypePhysicalDisk && resource.PhysicalDisk != nil &&
|
||||
strings.TrimSpace(resource.Proxmox.Instance) != "" && strings.TrimSpace(resource.Proxmox.NodeName) != "" &&
|
||||
strings.TrimSpace(resource.PhysicalDisk.DevPath) != "" {
|
||||
values = append(values, ProxmoxPhysicalDiskAlertResourceID(
|
||||
resource.Proxmox.Instance, resource.Proxmox.NodeName, resource.PhysicalDisk.DevPath,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
return uniqueTrimmed(values...)
|
||||
|
||||
@@ -143,6 +143,36 @@ func TestRefreshCanonicalIdentityKeepsProxmoxPresentationSeparateFromNativeAlias
|
||||
}
|
||||
}
|
||||
|
||||
func TestProxmoxDiskAlertAliasIsScopedAndNotPrimaryIdentity(t *testing.T) {
|
||||
resource := Resource{
|
||||
ID: "physical-disk-1", Type: ResourceTypePhysicalDisk,
|
||||
Proxmox: &ProxmoxData{Instance: "pve-a", NodeName: "node-1"},
|
||||
PhysicalDisk: &PhysicalDiskMeta{DevPath: "/dev/sda"},
|
||||
}
|
||||
assertAlias := func(want string, present bool) {
|
||||
t.Helper()
|
||||
RefreshCanonicalIdentity(&resource)
|
||||
if resource.Canonical == nil || resource.Canonical.PrimaryID != resource.ID {
|
||||
t.Fatalf("disk alert alias changed primary identity: %+v", resource.Canonical)
|
||||
}
|
||||
found := false
|
||||
for _, alias := range resource.Canonical.Aliases {
|
||||
found = found || alias == want
|
||||
}
|
||||
if found != present {
|
||||
t.Fatalf("alias %q present = %v, want %v; aliases = %v", want, found, present, resource.Canonical.Aliases)
|
||||
}
|
||||
}
|
||||
|
||||
alertID := ProxmoxPhysicalDiskAlertResourceID("pve-a", "node-1", "/dev/sda")
|
||||
assertAlias(alertID, true)
|
||||
resource.Proxmox.NodeName = "node-2"
|
||||
assertAlias(alertID, false)
|
||||
assertAlias(ProxmoxPhysicalDiskAlertResourceID("pve-a", "node-2", "/dev/sda"), true)
|
||||
resource.Type = ResourceTypeStorage
|
||||
assertAlias(ProxmoxPhysicalDiskAlertResourceID("pve-a", "node-2", "/dev/sda"), false)
|
||||
}
|
||||
|
||||
func TestUnavailableMemoryFacetsDoNotChangeCanonicalIdentity(t *testing.T) {
|
||||
total := int64(8 << 30)
|
||||
resource := Resource{
|
||||
|
||||
@@ -64,6 +64,42 @@ func ProxmoxPhysicalDiskSourceID(instance, node, device, controller, target stri
|
||||
return legacy
|
||||
}
|
||||
|
||||
// ProxmoxPhysicalDiskAlertResourceID is the persisted resource reference used
|
||||
// by PVE disk-health and wearout alerts. Keep this distinct from the disk's
|
||||
// source ID: existing alert occurrences use this shape, while the registry
|
||||
// needs it as an alias to apply per-disk operator intent.
|
||||
func ProxmoxPhysicalDiskAlertResourceID(instance, node, device string) string {
|
||||
return fmt.Sprintf("%s:%s:disk:%s", strings.TrimSpace(instance), strings.TrimSpace(node), physicalDiskAlertKey(device))
|
||||
}
|
||||
|
||||
func physicalDiskAlertKey(device string) string {
|
||||
trimmed := strings.TrimSpace(device)
|
||||
if trimmed == "/" {
|
||||
return "root"
|
||||
}
|
||||
trimmed = strings.Trim(trimmed, "/\\ ")
|
||||
if trimmed == "" {
|
||||
trimmed = "root"
|
||||
}
|
||||
var builder strings.Builder
|
||||
previousDash := false
|
||||
for _, r := range strings.ToLower(trimmed) {
|
||||
switch {
|
||||
case r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '.':
|
||||
builder.WriteRune(r)
|
||||
previousDash = false
|
||||
case !previousDash:
|
||||
builder.WriteByte('-')
|
||||
previousDash = true
|
||||
}
|
||||
}
|
||||
key := strings.Trim(builder.String(), "-.")
|
||||
if key == "" {
|
||||
return "disk"
|
||||
}
|
||||
return key
|
||||
}
|
||||
|
||||
func PhysicalDiskMetricID(disk models.PhysicalDisk) string {
|
||||
if diskinventory.IsUsableHardwareID(disk.Serial) || diskinventory.IsUsableHardwareID(disk.WWN) {
|
||||
return PreferredPhysicalDiskMetricID(disk.Serial, disk.WWN, "")
|
||||
|
||||
Reference in New Issue
Block a user