966d8f41ff
BoundUnderParent reported a namespace that returned EIO on every read and write
as healthy, and the gate restarted the customer's apps onto it. Both existing
terms parse a mountinfo line and then test only fields[4], the mount POINT.
Field 3 — major:minor — sat in the same parsed slice and was discarded.
Measured on hardware: raw 8:32 /dev/sdc, bind 8:16 /dev/sdb with `shutdown`,
bound_under_parent true, EIO both directions, and the controller taking its
Return branch and emailing backup_target_restored with no alarm on any channel.
BoundUnderParent gains a third term at both /disks construction sites. The new
bindLiveness reads /proc only and asks two questions: the bind must name the
same device as the raw mount, and the filesystem must not have aborted (ext4
`shutdown` or `emergency_ro`).
The second check is not optional. A device that fails WITHOUT disappearing gives
the identical all-signals-healthy state with the devnos EQUAL and the drive never
Disconnected, so the gate produces neither a Stop nor a Return and nothing is
emitted on any channel, indefinitely (R-117a). A devno-only fix would have passed
every payload test.
Three states, never a bool: {Unknown, Live, StaleDevice, Aborted}, read through
Usable(), where Unknown counts as PRESENT — reporting absent stops a working
customer's apps.
No new recovery path; the existing one was unblocked. AttachDrive's normalize leg
already did the repair and three call sites already invoked it, including the
controller's Return branch before it restarts apps. All three died on
`if n == 1 && GuestSeesMount(...)` returning early. Now: StaleDevice ⇒ re-bind
(repairs live, guest never restarts); Aborted ⇒ quiet no-op, because a re-bind
lands on the same dead superblock and this runs every 20s — an infinite silent
retry that masks the state; it surfaces via BoundUnderParent=false instead.
Ordering trap caught by a test: reading the abort flag before comparing devices
classifies the real return state as aborted (its stale bind carries `shutdown`
too) and refuses the repair while still reporting correctly. The abort flag is
read off the RAW mount in the stale case.
Tests 849 → 863, 29/29 packages green. 6 red-proofs, each verified to have
landed. A hollow test was caught during them: the aborted fixture first used a
/dev/mapper device, for which RoleForStorage derives role=system — a system row
has no GuestPath, never runs the conjunction, and reports false by default, so
the assertion passed vacuously and no mutation could fail it. Found because RP1
failed to fail.
188 lines
8.2 KiB
Go
188 lines
8.2 KiB
Go
package localapi
|
|
|
|
import (
|
|
"context"
|
|
"io"
|
|
"log/slog"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// R-117 §2.2 — WHAT AttachDrive DOES with each verdict, which is a RULING and not a detail.
|
|
//
|
|
// The two dead states have DIFFERENT repairs, and the existing self-heal only fits one:
|
|
//
|
|
// - STALE DEVICE (the detach/return case). The raw mount has healed onto the returning device via its
|
|
// fs-UUID-keyed unit, so umount + re-bind lands the namespace on a HEALTHY superblock. Repair is
|
|
// correct, and it happens live with no guest restart (proven on hardware, spike §8.1). It MUST run:
|
|
// three call sites already invoke it — the 20 s reconcile ticker, agent startup, and the controller's
|
|
// Return branch BEFORE it restarts the apps (spike §8.2) — and before v0.117.0 all three were
|
|
// short-circuited by `if n == 1 && GuestSeesMount(...)` declaring the dead namespace "fully live".
|
|
//
|
|
// - ABORTED FILESYSTEM (the steady-state case, R-117a). The raw mount is the SAME aborted superblock,
|
|
// so a re-bind produces a fresh bind to a still-dead filesystem. It MUST NOT run: AttachDrive is
|
|
// called every 20 s, so re-binding here is an infinite silent retry — the exact silence R-117a
|
|
// found, with more CPU — and it would mask the state instead of surfacing it. The truth travels in
|
|
// BoundUnderParent (now false), so the drive gate stops the apps and alarms. Clearing an aborted
|
|
// filesystem needs a remount or a fsck; that is an operator decision, never an automatic one.
|
|
//
|
|
// These tests assert the CONSEQUENCE — which privileged commands were issued — not that a verdict was
|
|
// computed. RED-PROOF: make the BindAborted arm fall through to the re-bind instead of returning, and
|
|
// TestAttachDrive_AbortedFilesystem_DoesNotRebind fails on the recorded umount/mount calls.
|
|
|
|
// attachRecorder is a proxmox.Runner that records every privileged call and answers `lxc-info -p` with a
|
|
// fixed PID, so the REAL GuestSeesMount runs against a captured guest mount table.
|
|
type attachRecorder struct {
|
|
calls [][]string
|
|
pid string
|
|
}
|
|
|
|
func (r *attachRecorder) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
|
|
r.calls = append(r.calls, append([]string{name}, args...))
|
|
if name == "lxc-info" {
|
|
return []byte(r.pid + "\n"), nil, nil
|
|
}
|
|
return nil, nil, nil
|
|
}
|
|
|
|
func (r *attachRecorder) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
|
|
return r.Run(ctx, name, args...)
|
|
}
|
|
|
|
// mountOps returns just the mount-table-mutating calls — the ones that constitute "a repair ran".
|
|
func (r *attachRecorder) mountOps() []string {
|
|
var out []string
|
|
for _, c := range r.calls {
|
|
switch c[0] {
|
|
case "umount", "mount":
|
|
out = append(out, strings.Join(c, " "))
|
|
}
|
|
}
|
|
return out
|
|
}
|
|
|
|
// attachFixture points BOTH mount tables at fixtures: the host one (procSelfMountinfo, which
|
|
// countHostMounts and bindLiveness read) and the guest one (procGuestMountinfo, which the REAL
|
|
// GuestSeesMount reads). Only the data is injected — every predicate runs for real.
|
|
func attachFixture(t *testing.T, hostBody, guestBody string) *attachRecorder {
|
|
t.Helper()
|
|
dir := t.TempDir()
|
|
hp := filepath.Join(dir, "host-mountinfo")
|
|
gp := filepath.Join(dir, "guest-mountinfo")
|
|
if err := os.WriteFile(hp, []byte(hostBody), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.WriteFile(gp, []byte(guestBody), 0o600); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
prevHost, prevGuest := procSelfMountinfo, procGuestMountinfo
|
|
procSelfMountinfo = hp
|
|
procGuestMountinfo = func(string) string { return gp }
|
|
t.Cleanup(func() { procSelfMountinfo, procGuestMountinfo = prevHost, prevGuest })
|
|
return &attachRecorder{pid: "9301"}
|
|
}
|
|
|
|
// guestSeesStale / guestSeesHealthy / guestSeesAborted are the GUEST-side captures — note `master:450`,
|
|
// the slave-of-the-shared-parent tag that proves propagation was wired (spike §3.1). The guest carries the
|
|
// same devno and super options as the host bind, because it IS the same mount.
|
|
const guestSeesStale = `759 1176 8:16 /felhom-data /mnt/felhom-drives/r117sd rw,relatime shared:459 master:450 - ext4 /dev/sdb rw,stripe=512,shutdown
|
|
`
|
|
const guestSeesHealthy = `759 1176 8:16 /felhom-data /mnt/felhom-drives/r117sd rw,relatime shared:459 master:450 - ext4 /dev/sdb rw,stripe=512
|
|
`
|
|
const guestSeesAborted = `759 1176 8:16 /felhom-data /mnt/felhom-drives/r117sd rw,relatime shared:459 master:450 - ext4 /dev/sdb rw,stripe=512,emergency_ro
|
|
`
|
|
|
|
func attachBinder(rec *attachRecorder) *GuestBinder {
|
|
return NewGuestBinder(rec, slog.New(slog.NewTextHandler(io.Discard, nil)))
|
|
}
|
|
|
|
// TestAttachDrive_StaleBind_Rebinds is Q6's consequence: the repair that was short-circuited for three
|
|
// releases now runs. Exactly the state measured on hardware — bind on 8:16, raw healed onto 8:32.
|
|
func TestAttachDrive_StaleBind_Rebinds(t *testing.T) {
|
|
rec := attachFixture(t, mountinfoStaleBind, guestSeesStale)
|
|
b := attachBinder(rec)
|
|
|
|
got, err := b.AttachDrive(context.Background(), 9301, "/mnt/r117sd")
|
|
if err != nil {
|
|
t.Fatalf("AttachDrive: %v", err)
|
|
}
|
|
if got != "/mnt/felhom-drives/r117sd" {
|
|
t.Errorf("stable path = %q", got)
|
|
}
|
|
ops := rec.mountOps()
|
|
if len(ops) == 0 {
|
|
t.Fatal("NO repair ran over a stale bind — this is the R-117 short-circuit: `n == 1 && " +
|
|
"GuestSeesMount` declared an EIO namespace \"fully live\", so the 20 s ticker, agent startup " +
|
|
"and the controller's pre-restart re-attach all did nothing")
|
|
}
|
|
var sawUmount, sawBind bool
|
|
for _, o := range ops {
|
|
if strings.HasPrefix(o, "umount /mnt/felhom-drives/r117sd") {
|
|
sawUmount = true
|
|
}
|
|
if o == "mount --bind /mnt/r117sd/felhom-data /mnt/felhom-drives/r117sd" {
|
|
sawBind = true
|
|
}
|
|
}
|
|
if !sawUmount || !sawBind {
|
|
t.Errorf("repair did not umount-then-rebind; ops=%v", ops)
|
|
}
|
|
}
|
|
|
|
// TestAttachDrive_AbortedFilesystem_DoesNotRebind is the §2.2 ruling. A re-bind here cannot repair
|
|
// anything (the raw mount is the same aborted superblock) and AttachDrive runs every 20 s, so re-binding
|
|
// would be an infinite silent retry that also masks the state.
|
|
func TestAttachDrive_AbortedFilesystem_DoesNotRebind(t *testing.T) {
|
|
rec := attachFixture(t, mountinfoAborted, guestSeesAborted)
|
|
b := attachBinder(rec)
|
|
|
|
got, err := b.AttachDrive(context.Background(), 9301, "/mnt/r117sd")
|
|
if err != nil {
|
|
t.Fatalf("AttachDrive returned an error for an aborted filesystem: %v — it must be a quiet no-op; "+
|
|
"an error here would log `reconcile: AttachDrive failed` every 20 s", err)
|
|
}
|
|
if got != "/mnt/felhom-drives/r117sd" {
|
|
t.Errorf("stable path = %q", got)
|
|
}
|
|
if ops := rec.mountOps(); len(ops) != 0 {
|
|
t.Errorf("AttachDrive re-bound an ABORTED filesystem: %v\n"+
|
|
"A re-bind lands on the SAME dead superblock, and this runs every 20 s — an infinite silent "+
|
|
"retry, which is R-117a's silence with more CPU. The state must SURFACE via "+
|
|
"BoundUnderParent=false so the gate stops the apps and alarms.", ops)
|
|
}
|
|
}
|
|
|
|
// TestAttachDrive_Healthy_IsStillANoOp — the idempotency the reconcile ticker depends on. If this
|
|
// regressed, every tick would umount and re-bind a working drive, re-firing propagation into the guest
|
|
// 4320 times a day.
|
|
func TestAttachDrive_Healthy_IsStillANoOp(t *testing.T) {
|
|
rec := attachFixture(t, mountinfoHealthy, guestSeesHealthy)
|
|
b := attachBinder(rec)
|
|
|
|
if _, err := b.AttachDrive(context.Background(), 9301, "/mnt/r117sd"); err != nil {
|
|
t.Fatalf("AttachDrive: %v", err)
|
|
}
|
|
if ops := rec.mountOps(); len(ops) != 0 {
|
|
t.Errorf("a healthy bind was disturbed: %v — the 20 s reconcile must stay a no-op", ops)
|
|
}
|
|
}
|
|
|
|
// TestAttachDrive_UnknownLiveness_IsANoOp — cannot-tell must not trigger churn either. An unreadable
|
|
// mount table making the agent umount and re-bind every 20 s would be a self-inflicted outage.
|
|
func TestAttachDrive_UnknownLiveness_IsANoOp(t *testing.T) {
|
|
// Host table healthy (so n == 1) but on a filesystem whose abort vocabulary we cannot read.
|
|
rec := attachFixture(t, mountinfoUnknownFS,
|
|
`759 1176 8:16 /felhom-data /mnt/felhom-drives/r117sd rw,relatime shared:459 master:450 - btrfs /dev/sdb rw
|
|
`)
|
|
b := attachBinder(rec)
|
|
|
|
if _, err := b.AttachDrive(context.Background(), 9301, "/mnt/r117sd"); err != nil {
|
|
t.Fatalf("AttachDrive: %v", err)
|
|
}
|
|
if ops := rec.mountOps(); len(ops) != 0 {
|
|
t.Errorf("an UNKNOWN verdict caused a re-bind: %v — cannot-tell must never churn a live mount", ops)
|
|
}
|
|
}
|