v0.115.0 — R-116: the backup-target flag reaches the row the controller keys on
Session C measured it live: a drive whose device vanished raised the GENERIC storage_disconnected while its return raised the SPECIFIC backup_target_restored -- an alarm and an all-clear an operator cannot pair. backup_target_absent never fired at all. The mechanism is not what the Session-C audit first said, and the difference decides the fix. RoleForStorage returns RoleSystem whenever backingDevice == "" (internal/storage/role.go:180-181). When the device goes, exactMountDevice fails, BackingDevice becomes "", the target row's role flips to system and it loses its guest path -- but keeps its MountPath. The union loop skips any drive whose MountPath is already seen, so the registry row is DEDUPED AWAY ENTIRELY. /disks carries no row with that guest path, so isTarget[guestPath] is a MISSING KEY, not a false. Setting BackupTarget on the union row -- the obvious fix -- could not have worked, because that row is not emitted when the alarm is needed. The audit is corrected in the same push. Fix: on the Observe row only, carry the guest path when the row IS the backup target and its role flipped because the device vanished. Three gates, verified not assumed: - t.BackingDevice == "" restricts it to the vanished-device flip; a genuinely system-BACKED storage has a real device and is excluded, so a dir storage at /mnt/<name> on the root disk cannot acquire a guest path. - Case B, the common fresh-box shape, is safe twice over: its target is the builtin local on /var/lib/vz and StablePathForRaw returns "" for anything not exactly /mnt/<name>, so nothing is set even before the gates apply. - It cannot make the gate read an absent drive as PRESENT. BoundUnderParent is assigned at exactly two sites, both inside guest-path blocks a system-role row never enters, so it stays false and planDriveGates computes false || false. Pinned by TestAbsentTargetRowDoesNotRegisterPresence -- getting this backwards would have silenced the alarm the fix exists to raise. The :213-214 boundary stands: no system or backup mount gains a guest path. Tests +5, asserting the emitted /disks JSON through a faithful copy of the controller's driveTargetByPath, because the failure class is "the value is on the wrong row". Red-proof: removing the block fails with "isTarget[...] is a MISSING KEY"; reverted byte-identical. Filed not closed: the two-row shape that produced this survives.
This commit is contained in:
@@ -1,3 +1,53 @@
|
||||
## v0.115.0 — R-116: the backup-target flag reaches the row the controller keys on (2026-07-29)
|
||||
|
||||
**The defect, measured live in Session C.** A drive whose device vanished raised the **generic**
|
||||
`storage_disconnected`, while its return raised the **specific** `backup_target_restored` — an alarm
|
||||
and an all-clear an operator cannot pair. `backup_target_absent` never fired at all.
|
||||
|
||||
**The mechanism, and it is not what the Session-C audit first said.** `RoleForStorage` returns
|
||||
`RoleSystem` whenever `backingDevice == ""` (`internal/storage/role.go:180-181`). When the device goes,
|
||||
Observe's `exactMountDevice` fails, `t.BackingDevice` becomes `""`, the target row's role flips to
|
||||
system and it **loses its guest path** — but keeps its `MountPath`. The union loop skips any drive whose
|
||||
`MountPath` is already `seen`, so the registry row is **deduped away entirely**. `/disks` ends up with
|
||||
**no row carrying that guest path**, so the controller's `isTarget[guestPath]` is a **missing key**, not
|
||||
a `false`. The obvious fix — setting `BackupTarget` on the union row — **could not have worked**: that
|
||||
row is not emitted in the state where the alarm is needed. The audit has been corrected.
|
||||
|
||||
**The fix.** On the Observe row only, carry the guest path when the row **is** the backup target **and**
|
||||
its role flipped because the device vanished:
|
||||
|
||||
```go
|
||||
if di.GuestPath == "" && di.BackupTarget && t.BackingDevice == "" {
|
||||
di.GuestPath = StablePathForRaw(t.MountPath)
|
||||
}
|
||||
```
|
||||
|
||||
**Three gates, each verified rather than assumed:**
|
||||
|
||||
- `t.BackingDevice == ""` restricts this to the vanished-device flip. A storage that is `RoleSystem`
|
||||
because it is genuinely system-**backed** has a real device and is excluded — otherwise a dir storage
|
||||
at `/mnt/<name>` on the root disk would acquire a guest path.
|
||||
- **Case B, the common fresh-box shape, is safe twice over.** Its target is the builtin `local` on
|
||||
`/var/lib/vz`, and `StablePathForRaw` returns `""` for anything that is not exactly `/mnt/<name>`
|
||||
(`DriveNameFromRaw`, `intermediary.go:79-88`) — nothing is set even before the gates apply.
|
||||
- **It cannot make the gate read an absent drive as PRESENT.** `BoundUnderParent` is assigned at exactly
|
||||
two sites (`disks.go:222`, `:280`), both inside guest-path blocks a system-role row never enters, so
|
||||
it stays `false` and `planDriveGates` computes `present[gp] = false || false`. Inert by construction —
|
||||
pinned by `TestAbsentTargetRowDoesNotRegisterPresence`. Getting this wrong would have **silenced the
|
||||
alarm this fix exists to raise**.
|
||||
|
||||
**The `:213-214` boundary stands.** No system or backup mount gains a guest path; only the drive the
|
||||
alarm is *about* keeps its identity while it is missing, and only while its device is gone.
|
||||
|
||||
**Tests** +5 in `internal/localapi`, asserting the emitted `/disks` JSON through a faithful copy of the
|
||||
controller's `driveTargetByPath`, because the failure class is "the value is on the wrong row" and a
|
||||
hand-built fixture proves nothing about which row the handler emits. Red-proof: removing the block
|
||||
fails with *"isTarget[…] is a MISSING KEY"*; reverted, byte-identical.
|
||||
|
||||
**Known limitation, filed not closed:** the two-row shape that produced this survives. The flag and the
|
||||
guest path still live on different rows in the healthy state, and nothing prevents a future consumer
|
||||
keying on the wrong one.
|
||||
|
||||
## v0.114.0 — R-113: drive presence means the DEVICE, not the bind (2026-07-29)
|
||||
|
||||
**The bug, measured live in E-2d.** `BoundUnderParent` — the one field the controller's drive-absent
|
||||
|
||||
@@ -223,6 +223,41 @@ func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) {
|
||||
s.devicePresent(t.MountPath)
|
||||
}
|
||||
}
|
||||
// R-116: carry the GUEST PATH on the backup-target row even when its role has flipped to
|
||||
// system — but ONLY when that flip was caused by the device vanishing.
|
||||
//
|
||||
// WHY. The controller keys the drive-absent alarm on the registered StoragePath, which for an
|
||||
// external drive is the GUEST path. When the device goes, Observe's exactMountDevice fails, so
|
||||
// t.BackingDevice becomes "" and RoleForStorage returns RoleSystem (role.go:180-181) — the block
|
||||
// above is skipped and this row loses its guest path. It keeps its MountPath, so the union loop
|
||||
// below DEDUPES the registry row away (`seen[d.MountPath]`), and /disks ends up carrying NO row
|
||||
// with that guest path at all. driveTargetByPath then has no entry, isTarget[guestPath] is a
|
||||
// missing key, and the specific backup_target_absent alarm cannot fire — the generic one goes
|
||||
// out instead, while the RETURN (rows rejoined) fires the specific recovery. An unmatchable
|
||||
// pair. Measured live: felhom.eu audits/SESSION-C-2026-07-29.md §5.
|
||||
//
|
||||
// THE GATES, each load-bearing:
|
||||
// di.GuestPath == "" — never touch the user-data path above; this is a fallback, not a rule.
|
||||
// di.BackupTarget — only the target row. No other system/backup mount gains a guest path,
|
||||
// so the boundary at :213-214 stands: this is not "system mounts now
|
||||
// cross into the guest", it is "the drive the alarm is about keeps its
|
||||
// identity while it is missing".
|
||||
// t.BackingDevice == "" — ONLY the vanished-device flip. A storage that is RoleSystem because
|
||||
// it is genuinely system-BACKED has a non-empty BackingDevice and is
|
||||
// excluded. Without this gate a dir storage at /mnt/<name> living on the
|
||||
// root disk would acquire a guest path.
|
||||
//
|
||||
// Case B (the COMMON fresh-box shape) is safe twice over: the target is the builtin `local` on
|
||||
// /var/lib/vz, and StablePathForRaw returns "" for anything that is not exactly /mnt/<name>
|
||||
// (DriveNameFromRaw, intermediary.go:79-88), so nothing is set even before the gates apply.
|
||||
//
|
||||
// This cannot make the gate read an absent drive as PRESENT: BoundUnderParent is assigned only
|
||||
// inside the two guest-path blocks a system-role row never enters, so it stays false, and
|
||||
// planDriveGates computes present[gp] = present[gp] || d.BoundUnderParent. Inert by construction
|
||||
// — pinned by TestAbsentTargetRowDoesNotRegisterPresence.
|
||||
if di.GuestPath == "" && di.BackupTarget && t.BackingDevice == "" {
|
||||
di.GuestPath = StablePathForRaw(t.MountPath)
|
||||
}
|
||||
// Inspect the backing device for the UI's data-bearing hint (the authoritative check
|
||||
// is re-run at format time on the actual device).
|
||||
if t.BackingDevice != "" {
|
||||
|
||||
@@ -0,0 +1,194 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"log/slog"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/storage"
|
||||
)
|
||||
|
||||
// R-116 — the backup-target flag must be reachable from the row the CONTROLLER keys on.
|
||||
//
|
||||
// THE DEFECT. The controller resolves the drive-absent alarm by the registered StoragePath, which for
|
||||
// an external drive is the GUEST path. When the device vanishes, Observe's exactMountDevice fails, so
|
||||
// BackingDevice becomes "" and RoleForStorage returns RoleSystem (role.go:180-181) — the guest-path
|
||||
// block is skipped and the flag-bearing row loses its guest path. It keeps its MountPath, so the union
|
||||
// loop DEDUPES the registry row away, and /disks carries NO row with that guest path at all.
|
||||
// driveTargetByPath then has no entry, isTarget[guestPath] is a MISSING KEY, and the specific
|
||||
// backup_target_absent alarm cannot fire — the generic storage_disconnected goes out instead, while
|
||||
// the RETURN (rows rejoined) fires the specific recovery. An operator gets a pair they cannot match.
|
||||
// Measured live: felhom.eu audits/SESSION-C-2026-07-29.md §5.
|
||||
//
|
||||
// These tests exercise the REAL GET /disks response and assert the emitted JSON, because the failure
|
||||
// class is "the value is on the wrong row" — a test that hand-builds rows proves nothing about which
|
||||
// row the handler actually emits.
|
||||
|
||||
// targetRowServer builds a /disks server whose primary backup tier is `primaryTarget`, over the given
|
||||
// Observe targets. boundCheck/deviceCheck are pinned so the R-113 conjunction is not the variable
|
||||
// under test here.
|
||||
func targetRowServer(t *testing.T, primaryTarget string, targets []hub.StorageTarget) *Server {
|
||||
t.Helper()
|
||||
srv, err := NewServer(Options{
|
||||
ListenAddr: "127.0.0.1:0",
|
||||
Guests: &fakeGuestsCfg{}, Backups: &fakeBackups{}, Store: &fakeStore{},
|
||||
Storage: fakeStorage{targets: targets},
|
||||
// Service is REQUIRED: normalizeBackupTiers (backup_tiers.go:21-22) drops any tier with a nil
|
||||
// Service, and the legacy fallback then yields TargetID "" — which silently makes every
|
||||
// BackupTarget false and would make these tests pass for the wrong reason.
|
||||
BackupTiers: []BackupTier{{TargetID: primaryTarget, Primary: true, Service: &fakeBackups{}}},
|
||||
Tokens: staticTokens{"A": 8200},
|
||||
Disks: &fakeDiskOps{probe: storage.DeviceProbe{Probed: true, HasFilesystem: true, FSType: "ext4"}},
|
||||
DiskGate: &fakeGate{}, HostReader: sysOnSDA(),
|
||||
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
srv.baseCtx = context.Background()
|
||||
srv.boundCheck = func(string) bool { return true }
|
||||
srv.deviceCheck = func(string) bool { return true }
|
||||
return srv
|
||||
}
|
||||
|
||||
// wireDisks returns the decoded /disks rows exactly as the controller receives them.
|
||||
func wireDisks(t *testing.T, srv *Server) []map[string]any {
|
||||
t.Helper()
|
||||
body := do(t, srv.Handler(), "GET", "/disks", "A", "").Body.Bytes()
|
||||
var w struct {
|
||||
Data struct {
|
||||
Disks []map[string]any `json:"disks"`
|
||||
} `json:"data"`
|
||||
}
|
||||
if err := json.Unmarshal(body, &w); err != nil {
|
||||
t.Fatalf("decode /disks: %v (%s)", err, body)
|
||||
}
|
||||
return w.Data.Disks
|
||||
}
|
||||
|
||||
// isTargetByPath reproduces the controller's driveTargetByPath EXACTLY (intermediary.go:602-616):
|
||||
// both keyings, value = backup_target. This is the map whose missing key is the whole defect, so the
|
||||
// assertion is made against a faithful copy of it rather than against a field in isolation.
|
||||
func isTargetByPath(disks []map[string]any) map[string]bool {
|
||||
out := map[string]bool{}
|
||||
for _, d := range disks {
|
||||
bt, _ := d["backup_target"].(bool)
|
||||
if gp, ok := d["guest_path"].(string); ok && gp != "" {
|
||||
out[gp] = bt
|
||||
}
|
||||
if mp, ok := d["mount_path"].(string); ok && mp != "" {
|
||||
out[mp] = bt
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// theAbsentTarget is the Session-C shape: the felhom-backup storage whose device has gone, so Observe
|
||||
// reports no backing device — which is what flips its role to system and drops its guest path.
|
||||
var theAbsentTarget = hub.StorageTarget{
|
||||
Name: "felhom-backup", Type: hub.StorageTypeLocalDir,
|
||||
MountPath: "/mnt/mentes", BackingDevice: "", State: hub.StorageStateDisconnected,
|
||||
}
|
||||
|
||||
// ── the observable that must move ───────────────────────────────────────────────────────────────
|
||||
|
||||
// RED-PROOF: delete the `di.GuestPath == "" && di.BackupTarget && t.BackingDevice == ""` block and
|
||||
// this fails with "the guest path the controller keys on is MISSING from /disks entirely".
|
||||
func TestAbsentBackupTargetIsResolvableByGuestPath(t *testing.T) {
|
||||
disks := wireDisks(t, targetRowServer(t, "felhom-backup", []hub.StorageTarget{theAbsentTarget}))
|
||||
isTarget := isTargetByPath(disks)
|
||||
|
||||
const guestPath = "/mnt/felhom-drives/mentes"
|
||||
got, present := isTarget[guestPath]
|
||||
if !present {
|
||||
t.Fatalf("isTarget[%q] is a MISSING KEY — the guest path the controller keys on is missing from "+
|
||||
"/disks entirely, so notifyDriveAbsent takes the generic branch and backup_target_absent "+
|
||||
"can never fire (R-116)", guestPath)
|
||||
}
|
||||
if !got {
|
||||
t.Errorf("isTarget[%q] = false; the row carrying the guest path does not carry the flag", guestPath)
|
||||
}
|
||||
// The host-path key was never the broken one — it must stay true.
|
||||
if !isTarget["/mnt/mentes"] {
|
||||
t.Error("isTarget by host path regressed to false")
|
||||
}
|
||||
}
|
||||
|
||||
// ── V2: the new guest path must NOT make the gate read the drive as PRESENT ─────────────────────
|
||||
|
||||
// This is the over-correction guard, in the exact component under test. planDriveGates computes
|
||||
// present[gp] = present[gp] || d.BoundUnderParent. If the row we now emit carried a true
|
||||
// BoundUnderParent, this fix would SILENCE the alarm it exists to raise.
|
||||
func TestAbsentTargetRowDoesNotRegisterPresence(t *testing.T) {
|
||||
srv := targetRowServer(t, "felhom-backup", []hub.StorageTarget{theAbsentTarget})
|
||||
// deviceCheck/boundCheck are pinned TRUE — the strongest possible case for a false positive.
|
||||
// The row must still report bound_under_parent=false, because that field is only ever assigned
|
||||
// inside the guest-path blocks a system-role row does not enter.
|
||||
for _, d := range wireDisks(t, srv) {
|
||||
if d["guest_path"] != "/mnt/felhom-drives/mentes" {
|
||||
continue
|
||||
}
|
||||
if bup, _ := d["bound_under_parent"].(bool); bup {
|
||||
t.Fatal("the absent backup-target row reports bound_under_parent=true — planDriveGates " +
|
||||
"would compute present=true, the Stop branch would never run, and this fix would " +
|
||||
"SUPPRESS the very alarm it exists to raise")
|
||||
}
|
||||
return
|
||||
}
|
||||
t.Fatal("the absent target row never reached the wire")
|
||||
}
|
||||
|
||||
// ── V1: the gates, each on its own ──────────────────────────────────────────────────────────────
|
||||
|
||||
// Case B is the COMMON fresh-box shape, not an edge: the tier target is the builtin `local` on the
|
||||
// root fs. It must never acquire a guest path.
|
||||
func TestCaseBLocalTargetGetsNoGuestPath(t *testing.T) {
|
||||
disks := wireDisks(t, targetRowServer(t, "local", []hub.StorageTarget{
|
||||
{Name: "local", Type: "local", MountPath: "/var/lib/vz", BackingDevice: "", State: hub.StorageStateAttached},
|
||||
}))
|
||||
for _, d := range disks {
|
||||
if gp, _ := d["guest_path"].(string); gp != "" {
|
||||
t.Errorf("the Case B target on %v acquired guest path %q — a system-drive backup target "+
|
||||
"must not cross into the guest", d["mount_path"], gp)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A storage that is RoleSystem because it is genuinely system-BACKED (non-empty BackingDevice on the
|
||||
// system disk) must be excluded — this is the case StablePathForRaw would NOT have filtered, since
|
||||
// /mnt/<name> maps to a real stable path. The BackingDevice gate is what stops it.
|
||||
func TestSystemBackedTargetUnderMntGetsNoGuestPath(t *testing.T) {
|
||||
disks := wireDisks(t, targetRowServer(t, "sysbackup", []hub.StorageTarget{
|
||||
// sysOnSDA() makes /dev/sda the system disk, so this classifies RoleSystem with a REAL device.
|
||||
{Name: "sysbackup", Type: hub.StorageTypeLocalDir, MountPath: "/mnt/sysbackup",
|
||||
BackingDevice: "/dev/sda1", State: hub.StorageStateAttached},
|
||||
}))
|
||||
for _, d := range disks {
|
||||
if gp, _ := d["guest_path"].(string); gp != "" {
|
||||
t.Errorf("a system-BACKED backup target acquired guest path %q — the BackingDevice gate "+
|
||||
"failed and the :213-214 boundary was widened", gp)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── the negative ────────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// A drive that is NOT the target must not acquire the flag on any row, present or absent.
|
||||
func TestNonTargetDriveNeverCarriesTheFlag(t *testing.T) {
|
||||
disks := wireDisks(t, targetRowServer(t, "felhom-backup", []hub.StorageTarget{
|
||||
{Name: "adat", Type: hub.StorageTypeLocalDir, MountPath: "/mnt/adat",
|
||||
BackingDevice: "", State: hub.StorageStateDisconnected},
|
||||
}))
|
||||
for _, d := range disks {
|
||||
if bt, _ := d["backup_target"].(bool); bt {
|
||||
t.Errorf("non-target drive %v reports backup_target=true", d["name"])
|
||||
}
|
||||
if gp, _ := d["guest_path"].(string); gp != "" {
|
||||
t.Errorf("an absent NON-target drive acquired guest path %q via the R-116 fallback — the "+
|
||||
"BackupTarget gate failed", gp)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user