Stop attaching node-local ZFS pools to shared storages

matchZFSPoolForStorage's sole-pool fallback attached the node's only ZFS
pool to every storage the per-node endpoint returned, including NFS, CIFS,
PBS, and RBD entries that can never be backed by a local pool. One failing
device then raised a duplicate ZFS device alert per shared storage. Gate
the matcher on isInherentlySharedStorageType so remote-backed storages are
never matched, while dir-type storages keep the single-pool fallback.

Refs #1731
This commit is contained in:
rcourtman
2026-08-16 06:22:55 +01:00
parent 991b3cad35
commit 1ff0680c75
3 changed files with 84 additions and 0 deletions
@@ -930,6 +930,12 @@ cleanup so readers cannot retain orphaned runtime or alert projections.
and the attached ZFS health model must carry the provider-reported `pool`
field through to runtime storage snapshots and use it before name/path
heuristics when matching ZFS pool health on multi-storage hosts.
Inherently shared or remote-backed storage types (NFS, CIFS, PBS, RBD, and
peers classified by `isInherentlySharedStorageType`) must never be matched
to a node-local ZFS pool, including by the single-pool fallback: a
node-local pool backs only local-capable storage types, and attaching it
more broadly raises one duplicate ZFS device alert per shared storage when
a device degrades (#1731).
That same Proxmox compatibility boundary also owns top-level ZFS vdev-role
normalization. Provider payload buckets such as `special`, `log`, `cache`,
and `spares` may omit a concrete health state; `pkg/proxmox/zfs.go` must
@@ -609,3 +609,73 @@ func TestIssue1645ClusterStorageRestrictedToOtherNodes(t *testing.T) {
})
}
}
// A node-local ZFS pool must never be attached to inherently shared/remote
// storage types. On a single-pool node the sole-pool fallback previously
// attached the pool to every storage in the datacenter config, so one failing
// device raised a duplicate ZFS alert per NFS/PBS storage (#1731).
func TestMatchZFSPoolForStorageSkipsInherentlySharedTypes(t *testing.T) {
rpool := &models.ZFSPool{Name: "rpool"}
singlePool := map[string]*models.ZFSPool{"rpool": rpool}
sharedStorages := []models.Storage{
{Name: "NFS_Qnap_Proxmox_Backup", Type: "nfs", Path: "/mnt/pve/NFS_Qnap_Proxmox_Backup"},
{Name: "PBS_01_QNAP", Type: "pbs"},
{Name: "smb_share", Type: "cifs", Path: "/mnt/pve/smb_share"},
{Name: "ceph_pool", Type: "rbd", Pool: "rpool"},
// A name collision with the pool must not override the type gate.
{Name: "rpool", Type: "nfs", Path: "/mnt/pve/rpool"},
}
for _, storage := range sharedStorages {
if got := matchZFSPoolForStorage(storage, singlePool); got != nil {
t.Fatalf("expected no pool for shared storage %q (type %s), got %q", storage.Name, storage.Type, got.Name)
}
}
}
func TestMatchZFSPoolForStorageKeepsLocalMatches(t *testing.T) {
rpool := &models.ZFSPool{Name: "rpool"}
tank := &models.ZFSPool{Name: "tank"}
singlePool := map[string]*models.ZFSPool{"rpool": rpool}
multiPool := map[string]*models.ZFSPool{"rpool": rpool, "tank": tank}
cases := []struct {
name string
storage models.Storage
pools map[string]*models.ZFSPool
want *models.ZFSPool
}{
{
name: "zfspool storage matches by pool dataset prefix",
storage: models.Storage{Name: "vm_storage0_01", Type: "zfspool", Pool: "tank/data"},
pools: multiPool,
want: tank,
},
{
name: "zfspool storage matches by pool name",
storage: models.Storage{Name: "local-zfs", Type: "zfspool", Pool: "rpool"},
pools: multiPool,
want: rpool,
},
{
name: "dir storage on single-pool node keeps sole-pool fallback",
storage: models.Storage{Name: "local", Type: "dir", Path: "/var/lib/vz"},
pools: singlePool,
want: rpool,
},
{
name: "dir storage on multi-pool node stays unmatched",
storage: models.Storage{Name: "backup_dir", Type: "dir", Path: "/mnt/backup"},
pools: multiPool,
want: nil,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := matchZFSPoolForStorage(tc.storage, tc.pools); got != tc.want {
t.Fatalf("matchZFSPoolForStorage(%q) = %v, want %v", tc.storage.Name, got, tc.want)
}
})
}
}
@@ -84,6 +84,14 @@ func matchZFSPoolForStorage(storage models.Storage, zfsPoolMap map[string]*model
return nil
}
// An inherently shared/remote storage (NFS, CIFS, PBS, RBD, ...) is never
// backed by a node-local ZFS pool. Without this gate the sole-pool fallback
// below attaches the pool to every storage on a single-pool node, so one
// degraded device raises a duplicate alert per NFS/PBS storage (#1731).
if isInherentlySharedStorageType(storage.Type) {
return nil
}
normalizedPools := make(map[string]*models.ZFSPool, len(zfsPoolMap))
var solePool *models.ZFSPool
for name, pool := range zfsPoolMap {