From 4d6c8a60563b16ba58d79ca43852eda6b695d7c7 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 29 Jul 2026 08:30:36 +0200 Subject: [PATCH] =?UTF-8?q?v0.184.1=20=E2=80=94=20E-2b=20keying=20fix:=20t?= =?UTF-8?q?he=20backup-target=20branch=20was=20unreachable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught before deploy by tracing a.Path back to its source, not by a failure. The 0.184.0 image is superseded and must not be shipped. ReconcileDriveGates resolves the target as isTarget[a.Path], and a.Path is the REGISTERED StoragePath -- for an external drive that is the GUEST path /mnt/felhom-drives/, not the agent's host MountPath (/mnt/) that /disks reports. driveTargetByPath keyed on MountPath alone, so the lookup never matched: every absent drive, the target included, fell through to the generic storage_disconnected. The alarm would have looked wired, passed its own unit tests, shipped, and been silently wrong on exactly the drive it exists for -- the same defect class E-2b was opened to fix, one level down. Now keyed under BOTH paths, mirroring planDriveGates which already registers present[] under GuestPath and MountPath for the same reason. Red-proof: MountPath-only keying fails with "the backup target is not resolvable by its GUEST path -- the gate passes a.Path (the registered StoragePath), so the backup-target branch would never fire". Green gate: build + vet + test rc=0, run separately from this commit. --- CHANGELOG.md | 21 ++++++++ .../internal/web/backup_target_keying_test.go | 51 +++++++++++++++++++ controller/internal/web/intermediary.go | 11 +++- 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 controller/internal/web/backup_target_keying_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index db62354..6e58e8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ ## Changelog +### v0.184.1 — E-2b keying fix: the backup-target branch was unreachable (2026-07-29) + +**Caught before deploy by tracing, not by a failure — and the 0.184.0 image is therefore superseded +and must not be shipped.** + +`ReconcileDriveGates` resolves the target as `isTarget[a.Path]`, and `a.Path` is the **registered** +`StoragePath` — for an external drive that is the GUEST path `/mnt/felhom-drives/`, not the +agent's host `MountPath` (`/mnt/`) that `/disks` reports. `driveTargetByPath` keyed the map on +`MountPath` alone, so the lookup never matched: **every absent drive, the target included, fell +through to the generic `storage_disconnected`.** + +The alarm would have looked wired, passed its own unit tests, shipped, and been silently wrong on +exactly the drive it exists for — the same defect class E-2b was opened to fix, one level down. + +Now keyed under BOTH paths, mirroring `planDriveGates`, which already registers `present[]` under +`GuestPath` and `MountPath` for the same reason. + +Red-proofed: reverting to MountPath-only keying fails with *"the backup target is not resolvable by +its GUEST path — the gate passes a.Path (the registered StoragePath), so the backup-target branch +would never fire"*. + ### v0.184.0 — E-2b + Part 5: the drive-absent alarm that was never wired (2026-07-29) — MinAgent 0.112.0 **`NotifyStorageDisconnected` and `NotifyStorageReconnected` were defined and called from NOWHERE.** diff --git a/controller/internal/web/backup_target_keying_test.go b/controller/internal/web/backup_target_keying_test.go new file mode 100644 index 0000000..bacc677 --- /dev/null +++ b/controller/internal/web/backup_target_keying_test.go @@ -0,0 +1,51 @@ +package web + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +// THE BUG THIS PINS, found by tracing rather than by a failure: +// +// ReconcileDriveGates looks the target up as isTarget[a.Path], and a.Path is the REGISTERED +// StoragePath. For an external drive that is the GUEST path (/mnt/felhom-drives/) — NOT the +// agent's host mount path (/mnt/), which is what /disks reports as MountPath. +// +// Keying driveTargetByPath on MountPath alone therefore made the whole backup-target branch +// UNREACHABLE: every absent drive, the target included, fell through to the generic +// storage_disconnected. The alarm would have looked wired, shipped, and been silently wrong on +// exactly the drive it exists for — the same class of defect E-2b was opened to fix. +// +// planDriveGates already registers present[] under BOTH keys; this mirrors that convention. +func TestDriveTargetIsResolvableByTheGuestPathTheGateActuallyUses(t *testing.T) { + disks := []agentapi.DiskInfo{ + { + Name: "felhom-backup", MountPath: "/mnt/nvme-1tb", + GuestPath: "/mnt/felhom-drives/nvme-1tb", BackupTarget: true, + }, + { + Name: "spare", MountPath: "/mnt/spare", + GuestPath: "/mnt/felhom-drives/spare", BackupTarget: false, + }, + } + got := driveTargetByPath(disks) + + // The guest path is the one the gate hands in — this is the assertion that would have caught it. + if !got["/mnt/felhom-drives/nvme-1tb"] { + t.Error("the backup target is not resolvable by its GUEST path — the gate passes a.Path " + + "(the registered StoragePath), so the backup-target branch would never fire") + } + // The host path must keep working too: a legacy raw /mnt/ registration is gated on it. + if !got["/mnt/nvme-1tb"] { + t.Error("the backup target is not resolvable by its HOST path — legacy raw registrations break") + } + // A non-target must be false under both keys, or the map would flag everything. + if got["/mnt/felhom-drives/spare"] || got["/mnt/spare"] { + t.Error("a non-target drive is flagged as the backup target under one of its keys") + } + // An unknown path must be absent, not a zero-value surprise. + if got["/mnt/felhom-drives/never-seen"] { + t.Error("an unregistered path resolved as the backup target") + } +} diff --git a/controller/internal/web/intermediary.go b/controller/internal/web/intermediary.go index eca0bc0..f62fb10 100644 --- a/controller/internal/web/intermediary.go +++ b/controller/internal/web/intermediary.go @@ -600,8 +600,17 @@ func (s *Server) gateWhere(w http.ResponseWriter, r *http.Request) (string, bool // intent was never recorded while the drive really is the target. An older agent omits the field, so // every entry is false and we degrade to the generic disconnect alarm — never a wrong one. func driveTargetByPath(disks []agentapi.DiskInfo) map[string]bool { - out := make(map[string]bool, len(disks)) + out := make(map[string]bool, 2*len(disks)) for _, d := range disks { + // BOTH keyings, mirroring planDriveGates (which registers present[] under GuestPath AND + // MountPath). The gate's a.Path is the REGISTERED StoragePath, and for an external drive that + // is the GUEST path /mnt/felhom-drives/ — not the agent's host /mnt/. Keying this + // map on MountPath alone made the backup-target branch unreachable: every absent drive, + // including the target, fell through to the generic storage_disconnected. Caught before + // deploy by tracing a.Path back to its source rather than assuming it matched. + if d.GuestPath != "" { + out[d.GuestPath] = d.BackupTarget + } if d.MountPath != "" { out[d.MountPath] = d.BackupTarget }