E-2 Part 1: the backup-target role on StoragePath (foundation, not yet wired)
Foundation only, no version bump -- nothing customer-visible changes yet. The
offer UI (Part 3), the degraded banner (Part 4) and the controller half of the
absent-target signal (Part 5) are NOT here; they are tracked as E-2 in
OPEN-ITEMS.md so this cannot become a sixth "seam built but never wired". The
fifth was found by E-2's own Phase 0: NotifyStorageDisconnected is defined and
called nowhere, so a drive going absent emits no event at all today.
StoragePath gains BackupTarget bool -- the sibling role to Schedulable/
IsDefault/Kind. It is INTENT, not truth: the authority is the agent's
backup.local_backup_target, and truth is read from GET /backup/tiers. This
records what the customer ASSIGNED so the controller can render the state,
notice the drive going absent, and detect drift.
Invariants, each pinned by a test asserting the CONSEQUENCE not the mechanism:
- a drive NEVER acquires the role by appearing (registration does not set it)
- exactly one carrier; assigning moves rather than duplicates
- sticky: a new bigger/faster drive does not steal an assigned target
- an absent target STAYS assigned -- clearing on disconnect would be a silent
retarget by omission ("no target configured" instead of "drive missing")
- a network share is refused (R-108 risk model; the role is about LOCAL disk
failure)
Red-proof C: adding auto-elevation to AddStoragePath fails
TestRegisteringDrivesNeverAssignsTheBackupTarget with
registering drives assigned the backup target "/mnt/hdd_1"
Attributes may suggest and refuse the absurd, never select: demo-felhom's backup
drive is an external USB HDD and BOTH demo boxes report removable=0, so a
transport rule disqualifies the reference drive and a removable rule finds no
candidate at all.
Green gate: build + vet + test rc=0, run separately from this commit.
This commit is contained in:
@@ -1,5 +1,42 @@
|
||||
## Changelog
|
||||
|
||||
### UNRELEASED — E-2 Part 1: the backup-target role (foundation; NOT yet wired to a UI)
|
||||
|
||||
**Status: foundation only. No version bump — nothing customer-visible changes yet.** The field is
|
||||
written by `SetBackupTarget` and read by `BackupTargetPath`/`BackupTargetAssigned`, and by nothing
|
||||
else. **The offer UI (Part 3), the degraded banner (Part 4) and the absent-target signal's
|
||||
controller half (Part 5) are NOT in this commit** — tracked as E-2 in `OPEN-ITEMS.md` so this cannot
|
||||
become a sixth "seam built but never wired" (the fifth, `NotifyStorageDisconnected`, was found by
|
||||
E-2's own Phase 0 and is one of the things still to wire).
|
||||
|
||||
`StoragePath` gains `BackupTarget bool` — the sibling role to `Schedulable`/`IsDefault`/`Kind`,
|
||||
marking the drive the whole-guest vzdump is written to.
|
||||
|
||||
**It is INTENT, not truth.** The authority is the agent's `backup.local_backup_target`; this records
|
||||
what the customer ASSIGNED so the controller can render the state, notice the drive going absent,
|
||||
and detect drift. Truth comes from the agent's `GET /backup/tiers`.
|
||||
|
||||
Invariants, each pinned by a test asserting the CONSEQUENCE rather than the mechanism:
|
||||
|
||||
- **A drive never acquires the role by appearing.** Registration does not set it; only an explicit
|
||||
customer choice through `SetBackupTarget` does. Red-proofed: adding auto-elevation to
|
||||
`AddStoragePath` fails `TestRegisteringDrivesNeverAssignsTheBackupTarget` with
|
||||
`registering drives assigned the backup target "/mnt/hdd_1"`.
|
||||
- **Exactly one carrier** — assigning moves the role rather than duplicating it.
|
||||
- **Sticky** — a new, bigger, faster drive appearing does not steal an assigned target.
|
||||
- **An absent target stays assigned.** Clearing on disconnect would be a silent retarget by
|
||||
omission: the box would read "no target configured" instead of "your target drive is missing".
|
||||
- **A network share is refused** — the role exists to survive a LOCAL disk failure, and a remote,
|
||||
credential-bound share mounted at its own root (R-108) is a different risk model.
|
||||
|
||||
Attributes may suggest and may refuse the absurd; they may never select. The reference hardware
|
||||
settles it: demo-felhom's backup drive is an external **USB HDD**, and **both** demo boxes' drives
|
||||
report `removable=0` — a transport rule would disqualify the reference drive, a removable rule would
|
||||
find no candidate at all.
|
||||
|
||||
Green gate: `go build` + `go vet` + `go test ./internal/{settings,web,quiesce}` all rc=0, run
|
||||
separately from the commit.
|
||||
|
||||
### v0.183.0 — C9-F1 + C9-F2: a restore that restored nothing, and a crash loop nobody saw (2026-07-28)
|
||||
|
||||
Both are the same shape — the system reporting healthy while the customer is not — and both were
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
package settings
|
||||
|
||||
import (
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func roleTestSettings(t *testing.T) *Settings {
|
||||
t.Helper()
|
||||
s, err := Load(filepath.Join(t.TempDir(), "settings.json"), log.New(os.Stderr, "", 0))
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// SCENARIO C, the core of E-2 §3: a drive NEVER acquires the backup-target role by appearing.
|
||||
//
|
||||
// This is the invariant the whole role model exists for. Registration is the path a newly-plugged
|
||||
// drive travels, and the failure it guards against is a USB stick silently becoming the destination
|
||||
// of the whole-system backup. The test asserts the CONSEQUENCE (no target is assigned after
|
||||
// registering drives) rather than the mechanism, so it still fails if a future caller sets the field
|
||||
// through some other route in the registration path.
|
||||
func TestRegisteringDrivesNeverAssignsTheBackupTarget(t *testing.T) {
|
||||
s := roleTestSettings(t)
|
||||
for _, p := range []string{"/mnt/hdd_1", "/mnt/usb_stick", "/mnt/nvme-1tb"} {
|
||||
if err := s.AddStoragePath(StoragePath{Path: p, Label: p, Schedulable: true}); err != nil {
|
||||
t.Fatalf("add %s: %v", p, err)
|
||||
}
|
||||
}
|
||||
if got := s.BackupTargetPath(); got != "" {
|
||||
t.Fatalf("registering drives assigned the backup target %q — a drive must never acquire a role "+
|
||||
"by appearing (E-2 §3); only an explicit customer choice may assign it", got)
|
||||
}
|
||||
if s.BackupTargetAssigned() {
|
||||
t.Fatal("BackupTargetAssigned() is true after registration alone")
|
||||
}
|
||||
}
|
||||
|
||||
// Exactly one target at a time — assigning a second must move the role, not duplicate it. Two paths
|
||||
// both claiming to be the destination is not a cosmetic defect: the renderer and the absent-check
|
||||
// would each pick a different one.
|
||||
func TestBackupTargetIsExclusive(t *testing.T) {
|
||||
s := roleTestSettings(t)
|
||||
for _, p := range []string{"/mnt/a", "/mnt/b"} {
|
||||
if err := s.AddStoragePath(StoragePath{Path: p, Schedulable: true}); err != nil {
|
||||
t.Fatalf("add: %v", err)
|
||||
}
|
||||
}
|
||||
if err := s.SetBackupTarget("/mnt/a"); err != nil {
|
||||
t.Fatalf("set a: %v", err)
|
||||
}
|
||||
if err := s.SetBackupTarget("/mnt/b"); err != nil {
|
||||
t.Fatalf("set b: %v", err)
|
||||
}
|
||||
var carriers []string
|
||||
for _, sp := range s.GetStoragePaths() {
|
||||
if sp.BackupTarget {
|
||||
carriers = append(carriers, sp.Path)
|
||||
}
|
||||
}
|
||||
if len(carriers) != 1 || carriers[0] != "/mnt/b" {
|
||||
t.Fatalf("expected exactly one carrier (/mnt/b), got %v", carriers)
|
||||
}
|
||||
}
|
||||
|
||||
// STICKINESS (E-2 §3): a new drive appearing must not move an assigned target. This is the other half
|
||||
// of "never by appearing" — the first covers an unassigned box, this covers an assigned one.
|
||||
func TestNewDriveDoesNotStealAnAssignedTarget(t *testing.T) {
|
||||
s := roleTestSettings(t)
|
||||
if err := s.AddStoragePath(StoragePath{Path: "/mnt/hdd_1", Schedulable: true}); err != nil {
|
||||
t.Fatalf("add: %v", err)
|
||||
}
|
||||
if err := s.SetBackupTarget("/mnt/hdd_1"); err != nil {
|
||||
t.Fatalf("set: %v", err)
|
||||
}
|
||||
// A bigger, faster, newer drive appears. It must change nothing.
|
||||
if err := s.AddStoragePath(StoragePath{Path: "/mnt/nvme_huge", Schedulable: true}); err != nil {
|
||||
t.Fatalf("add: %v", err)
|
||||
}
|
||||
if got := s.BackupTargetPath(); got != "/mnt/hdd_1" {
|
||||
t.Fatalf("target moved to %q when a new drive appeared — the role is sticky", got)
|
||||
}
|
||||
}
|
||||
|
||||
// An ABSENT target stays assigned. Clearing it on disconnect would be a silent retarget by omission:
|
||||
// the box would read as "no target configured" (degraded) instead of "your target drive is missing"
|
||||
// (alarm), and reconnecting would not restore the assignment.
|
||||
func TestDisconnectDoesNotClearTheTarget(t *testing.T) {
|
||||
s := roleTestSettings(t)
|
||||
if err := s.AddStoragePath(StoragePath{Path: "/mnt/hdd_1", Schedulable: true}); err != nil {
|
||||
t.Fatalf("add: %v", err)
|
||||
}
|
||||
if err := s.SetBackupTarget("/mnt/hdd_1"); err != nil {
|
||||
t.Fatalf("set: %v", err)
|
||||
}
|
||||
if err := s.SetDisconnected("/mnt/hdd_1", true, nil); err != nil {
|
||||
t.Fatalf("disconnect: %v", err)
|
||||
}
|
||||
if got := s.BackupTargetPath(); got != "/mnt/hdd_1" {
|
||||
t.Fatalf("disconnect cleared the target (got %q) — an absent target must stay assigned and "+
|
||||
"alarm, never silently become 'unconfigured'", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A network share can never be the target: the role exists to survive a LOCAL disk failure, and a
|
||||
// remote, credential-bound share mounted at its own root (R-108) is a different risk model.
|
||||
func TestNetworkShareRefusedAsBackupTarget(t *testing.T) {
|
||||
s := roleTestSettings(t)
|
||||
if err := s.AddStoragePath(StoragePath{Path: "/mnt/felhom-drives/nas", Kind: StorageKindNetwork}); err != nil {
|
||||
t.Fatalf("add: %v", err)
|
||||
}
|
||||
if err := s.SetBackupTarget("/mnt/felhom-drives/nas"); err == nil {
|
||||
t.Fatal("a network share was accepted as the backup target")
|
||||
}
|
||||
if s.BackupTargetAssigned() {
|
||||
t.Fatal("a refused assignment still set the role")
|
||||
}
|
||||
}
|
||||
@@ -287,6 +287,28 @@ type StoragePath struct {
|
||||
// Kind discriminates a physical drive ("" / "drive") from a NAS network share ("network"). A network
|
||||
// share is bulk-media only and carries the fields below; the drive lifecycle does NOT apply to it.
|
||||
Kind string `json:"kind,omitempty"`
|
||||
|
||||
// BackupTarget (E-2) marks the drive the WHOLE-GUEST vzdump is written to — the sibling role to
|
||||
// Schedulable/IsDefault/Kind.
|
||||
//
|
||||
// THIS FIELD IS INTENT, NOT TRUTH. The authority is the agent's `backup.local_backup_target` in
|
||||
// agent.json; this records which drive the customer ASSIGNED so the controller can (a) render the
|
||||
// degraded/healthy state, (b) notice when the assigned drive goes absent, and (c) detect drift from
|
||||
// what the agent is actually doing. Read the agent's `GET /backup/tiers` for truth; never assume
|
||||
// this flag and the agent agree.
|
||||
//
|
||||
// THE RULE THIS EXISTS TO ENFORCE: a drive NEVER acquires this role by appearing. Nothing in the
|
||||
// registration path sets it — only an explicit customer choice through SetBackupTarget does.
|
||||
// Attributes (transport, removable, size) may SUGGEST a candidate and may refuse the absurd; they
|
||||
// may never select one. The reference hardware settles it: demo-felhom's backup drive is an
|
||||
// external USB HDD and BOTH demo boxes' drives report removable=0, so a transport rule would
|
||||
// disqualify the reference drive and a removable rule would find no candidate at all.
|
||||
//
|
||||
// Exactly one path may carry it (SetBackupTarget clears the others), and it is STICKY: it does not
|
||||
// move because a new drive appeared, and it is never cleared just because the drive is absent —
|
||||
// an absent target must stay assigned and ALARM (E-2 Part 5), because silently retargeting is how
|
||||
// a backup lands somewhere nobody expects. See [[storage-authz-redesign]] for the role vocabulary.
|
||||
BackupTarget bool `json:"backup_target,omitempty"`
|
||||
// Network-storage descriptors (Kind=="network" only; mirror the agent A1 add request). NO password
|
||||
// is stored — the SMB credential is passed through to the agent at add-time and never persisted here.
|
||||
Protocol string `json:"protocol,omitempty"` // nfs | smb
|
||||
@@ -1018,6 +1040,75 @@ func (s *Settings) SetSchedulable(path string, schedulable bool) error {
|
||||
return fmt.Errorf("storage path %q not found", path)
|
||||
}
|
||||
|
||||
// SetBackupTarget assigns the whole-guest backup-target role to exactly one storage path, clearing it
|
||||
// from every other. This is the ONLY writer of StoragePath.BackupTarget — registration must never set
|
||||
// it (E-2 §3: a drive never acquires a role by appearing).
|
||||
//
|
||||
// Refusals, both structural rather than advisory:
|
||||
// - a NETWORK share can never be the target. vzdump writes a multi-GB archive through the host, and
|
||||
// the whole point of the role is surviving a local disk failure — a share that is itself remote,
|
||||
// credential-bound and mounted at its own root (R-108) is a different risk model entirely.
|
||||
// - a path that is not registered cannot hold a role.
|
||||
//
|
||||
// It deliberately does NOT refuse a DISCONNECTED path: reassigning to a drive that is currently absent
|
||||
// is a legitimate recovery order ("this is the drive, go find it"), and the absent-target alarm is what
|
||||
// covers the gap. Refusing here would instead force a silent retarget elsewhere.
|
||||
func (s *Settings) SetBackupTarget(path string) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
idx := -1
|
||||
for i := range s.StoragePaths {
|
||||
if s.StoragePaths[i].Path == path {
|
||||
idx = i
|
||||
break
|
||||
}
|
||||
}
|
||||
if idx < 0 {
|
||||
return fmt.Errorf("storage path %q not found", path)
|
||||
}
|
||||
if s.StoragePaths[idx].IsNetwork() {
|
||||
return fmt.Errorf("a hálózati tárhely nem lehet a rendszermentés célja: %s", path)
|
||||
}
|
||||
for i := range s.StoragePaths {
|
||||
s.StoragePaths[i].BackupTarget = i == idx
|
||||
}
|
||||
if s.log != nil {
|
||||
s.log.Printf("[INFO] [settings] backup target assigned: %s", path)
|
||||
}
|
||||
return s.save()
|
||||
}
|
||||
|
||||
// ClearBackupTarget removes the role from every path, leaving the box with no assigned target (the
|
||||
// degraded state). Separate from SetBackupTarget on purpose: "no target" is a real, nameable state
|
||||
// that must be reachable deliberately, not an accident of passing an empty string to a setter.
|
||||
func (s *Settings) ClearBackupTarget() error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
for i := range s.StoragePaths {
|
||||
s.StoragePaths[i].BackupTarget = false
|
||||
}
|
||||
if s.log != nil {
|
||||
s.log.Printf("[INFO] [settings] backup target cleared — the box is now in the degraded (system-drive) state")
|
||||
}
|
||||
return s.save()
|
||||
}
|
||||
|
||||
// BackupTargetPath returns the assigned target's path, or "" when none is assigned (degraded).
|
||||
func (s *Settings) BackupTargetPath() string {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
for _, sp := range s.StoragePaths {
|
||||
if sp.BackupTarget {
|
||||
return sp.Path
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// BackupTargetAssigned reports whether ANY path carries the role. The negation is the degraded state
|
||||
// the customer must be told about (E-2 Part 4).
|
||||
func (s *Settings) BackupTargetAssigned() bool { return s.BackupTargetPath() != "" }
|
||||
|
||||
// SetStorageLabel updates the label for a storage path.
|
||||
func (s *Settings) SetStorageLabel(path, label string) error {
|
||||
s.mu.Lock()
|
||||
|
||||
Reference in New Issue
Block a user