diff --git a/modules/mounts/README.md b/modules/mounts/README.md index 0e1a1911..b9c3cc9f 100644 --- a/modules/mounts/README.md +++ b/modules/mounts/README.md @@ -42,6 +42,17 @@ an autofs mount, so nothing writes into the empty directory under it. | an SMB source | `///`, `Type=cifs` | `credentials=,iocharset=utf8,_netdev,nofail,uid=…,gid=…` | 10 | | a disk | `/dev/disk/by-uuid/`; the mount `BindsTo=` the disk's device | `nofail` | 10 | +A mount of the module's own that failed because its source was not there — an occasional source that is +off, or a node's share whose server is away, tried when someone opened the mount point — is cleared +(`systemctl reset-failed`) by `apply` within five seconds, and so is the failure a unit of the same name +carried before the module wrote it (the unit fstab's generator makes until the line is adopted). The module +says such a share as `absent` or `unreachable`; left failed, the unit would also be said as a failed unit of +the machine's own, with a notification each time it is touched. A mount that failed while its source answers +is left failed: that is a fault, and the share is said `failed`. An automount that gave up after repeated +failed mounts (`mount-start-limit-hit`) while its source was away is cleared and armed again the same way. +Only a unit the module takes over has its earlier failure cleared regardless of the source; a pair of its +own that it rewrites keeps its failure. + A share is read-only when the setting says `:ro` or the sharing machine exports it read-only. A read-write share is mounted hard, so a write is never dropped while the server is away. @@ -102,6 +113,8 @@ Each refusal is said as the share's state `failed`, with why, and nothing is wri - a mount point the system owns (`/`, `/etc`, `/usr`, `/boot`, `/proc`, `/sys`, `/dev`, `/run` and what is under them, and `/var`, `/home`, `/root`, `/tmp` themselves); - a mount point that something else is mounted at, or an automount that is not the module's; +- a mount point whose unit someone made at run time (`/run/systemd/transient`, from `systemd-mount` or the + like): that is somebody's mount now; - a folder that holds files: a mount would hide them. Only a folder not in the kernel's mount table is opened, so a server that is away cannot hold the look; - a share that this machine serves itself. diff --git a/modules/mounts/cmd/mounts/apply.go b/modules/mounts/cmd/mounts/apply.go index c2df2630..3376f26a 100644 --- a/modules/mounts/cmd/mounts/apply.go +++ b/modules/mounts/cmd/mounts/apply.go @@ -33,6 +33,11 @@ import ( // LookEvery is how often the process looks. const LookEvery = 30 * time.Second +// ClearEvery is how often the acting process clears a failed mount of its own whose source does not +// answer (clearFailed). Shorter than the node-engine's two looks of 15 s, so a mount that failed because +// its source is away is never read as a failed unit of the machine's own. +const ClearEvery = 5 * time.Second + // ProbeTimeout bounds one try of a source. const ProbeTimeout = 2 * time.Second @@ -89,6 +94,11 @@ type Applier struct { Dial func(addr string, timeout time.Duration) error Now func() time.Time Lookup func(account string) (int, int, error) + // written is every pair the last pass wrote and armed, by its mount unit: what clearFailed looks at + // between passes. The acting process's own memory; nothing else reads it. + written map[string]Pair + // LookInterval and ClearInterval are LookEvery and ClearEvery, or a test's own when set. + LookInterval, ClearInterval time.Duration } // NewApplier is the process on this machine. @@ -326,6 +336,9 @@ func (a *Applier) Look(ctx context.Context) Status { own := a.ownUnits() mounted := ParseMountinfo(func() string { t, _ := a.M.read("/proc/self/mountinfo"); return t }()) changed := false + // The mount units this pass wrote where no unit file of the module's was before: a unit of the same + // name that was there — the one fstab's generator makes until the line is adopted — is taken over. + takenOver := map[string]bool{} var written []Pair for _, p := range pairs { ours := own[p.MountUnit] && own[p.AutoUnit] @@ -362,6 +375,9 @@ func (a *Applier) Look(ctx context.Context) Status { State: StateFailed, Why: fmt.Sprintf("its units could not be written: %v %v", err1, err2)} continue } + if c1 && !own[p.MountUnit] { + takenOver[p.MountUnit] = true + } if c1 || c2 { changed = true st.Changed = append(st.Changed, "the units of "+p.Name) @@ -448,6 +464,24 @@ func (a *Applier) Look(ctx context.Context) Status { } } } + if a.Act { + // A mount unit the module wrote for the first time, over a unit of the same name that had already + // failed — the one fstab's generator makes until the line is adopted — carries a failure that is not + // the module's pair's. It is cleared, so the pair starts armed and is judged on its own mounts (ADR + // 0263 rule 4). A pair the module already had and rewrote keeps its failure: that one is its own. + for _, p := range written { + if takenOver[p.MountUnit] && states[p.MountUnit]["ActiveState"] == "failed" { + if _, err := a.M.Run(ctx, "systemctl", "reset-failed", p.MountUnit); err == nil { + st.Changed = append(st.Changed, "cleared "+p.MountUnit+", which had failed before the module wrote it") + } + } + } + a.written = map[string]Pair{} + for _, p := range written { + a.written[p.MountUnit] = p + } + st.Changed = append(st.Changed, a.clearFailed(ctx)...) + } if !a.Judge { a.finish(&st, notSet) return st @@ -522,6 +556,12 @@ func (a *Applier) finish(st *Status, notSet map[string]ShareStatus) { // foreign refuses a pair whose unit file exists and is not the module's. func (a *Applier) foreign(p Pair, own map[string]bool) error { for _, unit := range []string{p.MountUnit, p.AutoUnit} { + // A transient unit of the same name — made by `systemd-mount` or `mount` on the command line — is + // someone's mount now, whatever the module's own file says. + transient := filepath.Join(TransientDir, unit) + if _, err := os.Lstat(a.path(transient)); err == nil { + return fmt.Errorf("%s has a transient unit that is not the module's (%s); it is left as it is, and this share is not mounted here", p.Mountpoint, transient) + } if own[unit] { continue } @@ -532,6 +572,9 @@ func (a *Applier) foreign(p Pair, own map[string]bool) error { return nil } +// TransientDir is where the service manager keeps the units made at run time rather than from a file. +const TransientDir = "/run/systemd/transient" + // judge is one pair's state. The source is tried first, without touching the mount point. func (a *Applier) judge(p Pair, states map[string]map[string]string, mounted map[string]Mounted) (string, string) { there := true @@ -569,17 +612,91 @@ func (a *Applier) writeStatus(st *Status) { } } -// RunApply looks every LookEvery until the context ends, saying on stderr what it changed. +// clearFailed clears every failed mount of the module's own pairs whose source does not answer now: an +// occasional source that is away, or a node's share whose server is. Such a mount failed because its +// source was not there, which the module already says as the share's state (absent, or unreachable, and +// for a share that should always be there its health after five minutes, ADR 0263 rule 8); left failed, +// the unit would be said a second time as a failed unit of the machine's own, with a notification each +// time it is touched — what rules 4 and 6 promise never happens. A mount that failed while its source +// answers is left failed: that is a fault, and the share is said failed until a person looks. +func (a *Applier) clearFailed(ctx context.Context) []string { + if len(a.written) == 0 { + return nil + } + var mounts, units []string + for unit, p := range a.written { + mounts = append(mounts, unit) + units = append(units, unit, p.AutoUnit) + } + sort.Strings(mounts) + states := a.unitStates(ctx, units) + var cleared []string + for _, unit := range mounts { + p := a.written[unit] + mountFailed := states[unit]["ActiveState"] == "failed" + // After repeated failed mounts the automount gives up too (mount-start-limit-hit) and stops + // answering its point: cleared and armed again, or the share stays dead until the next pass. + autoFailed := states[p.AutoUnit]["ActiveState"] == "failed" + if !mountFailed && !autoFailed { + continue + } + if a.sourceAnswers(p) { + continue + } + if mountFailed { + if _, err := a.M.Run(ctx, "systemctl", "reset-failed", unit); err == nil { + cleared = append(cleared, "cleared "+unit+": "+p.Name+" was not there to mount") + } + } + if autoFailed { + if _, err := a.M.Run(ctx, "systemctl", "reset-failed", p.AutoUnit); err == nil { + if _, err := a.M.Run(ctx, "systemctl", "start", p.AutoUnit); err == nil { + cleared = append(cleared, "cleared and armed "+p.AutoUnit+" again ("+orDefault(states[p.AutoUnit]["Result"], "failed")+"): "+p.Name+" was not there to mount") + } + } + } + } + return cleared +} + +// sourceAnswers tries a pair's source the way the judging process does, without touching its mount point. +func (a *Applier) sourceAnswers(p Pair) bool { + if p.Kind == "disk" { + return a.M.present(p.Probe) + } + return a.Dial != nil && a.Dial(p.Probe, ProbeTimeout) == nil +} + +// RunApply looks every LookEvery until the context ends, saying on stderr what it changed. Between looks +// the acting process clears, every ClearEvery, a mount of its own that failed because its source was away. func RunApply(ctx context.Context, a *Applier) { for { st := a.Look(ctx) for _, c := range st.Changed { fmt.Fprintln(os.Stderr, "mounts:", c) } - select { - case <-ctx.Done(): - return - case <-time.After(LookEvery): + next := time.After(orInterval(a.LookInterval, LookEvery)) + between: + for { + select { + case <-ctx.Done(): + return + case <-next: + break between + case <-time.After(orInterval(a.ClearInterval, ClearEvery)): + if a.Act { + for _, c := range a.clearFailed(ctx) { + fmt.Fprintln(os.Stderr, "mounts:", c) + } + } + } } } } + +func orInterval(d, otherwise time.Duration) time.Duration { + if d > 0 { + return d + } + return otherwise +} diff --git a/modules/mounts/cmd/mounts/failed_test.go b/modules/mounts/cmd/mounts/failed_test.go new file mode 100644 index 00000000..2b03ae2e --- /dev/null +++ b/modules/mounts/cmd/mounts/failed_test.go @@ -0,0 +1,237 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" +) + +// A mount the module took over, or one whose source was away when it was touched, is never left as a +// failed unit: the node-engine would say it as a failed unit of the machine's own, with a notification each +// time (hq ADR 0263 rules 4 and 6). A mount that failed while its source answers stays failed: a fault. + +func failedLook(t *testing.T, f *fakeSystem) map[string]ShareStatus { + t.Helper() + st := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)).Look(context.Background()) + out := map[string]ShareStatus{} + for _, s := range st.Shares { + out[s.Name] = s + } + return out +} + +func TestTheFailureOfTheUnitTheModuleTakesOverIsCleared(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "shares=library=/srv/library\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + f.write(ExportsFile, exportsAnswer(t, library)) + f.reachable["192.0.2.10:2049"] = true + // fstab's generator made srv-library.mount for the same mount point, and it timed out at boot. + f.units["srv-library.mount"] = map[string]string{"ActiveState": "failed", "Result": "timeout"} + + states := failedLook(t, f) + + if !f.called("systemctl reset-failed srv-library.mount") { + t.Fatalf("the generator's failure is cleared once the module's unit is written: %q", f.calls) + } + if s := states["library"]; s.State != StateArmed { + t.Errorf("the share is judged on its own mounts, not on the generator's: %+v", s) + } +} + +func TestAMountThatFailedBecauseItsSourceWasAwayIsCleared(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "shares=library=/srv/library\nsources=games=smb://192.0.2.76/Share@/mnt/games\n"+ + "server=homeserver\nat=192.0.2.10\nnode=workstation\naccount=operator\n"+ + "secret=/var/lib/mesh/mounts/smb-credentials.secret\nstate=/var/lib/mesh/mounts\n") + f.write(ExportsFile, exportsAnswer(t, library)) + f.write("/var/lib/mesh/mounts/smb-credentials.secret", "[games]\nusername=root\npassword="+testPassword+"\n") + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) // writes and arms both pairs; neither source answers + f.calls = nil + + // Someone opened both mount points: the gaming device is off and the server is away, so both mounts failed. + f.units["mnt-games.mount"] = map[string]string{"ActiveState": "failed", "Result": "timeout"} + f.units["srv-library.mount"] = map[string]string{"ActiveState": "failed", "Result": "timeout"} + cleared := a.clearFailed(context.Background()) + + for _, unit := range []string{"mnt-games.mount", "srv-library.mount"} { + if !f.called("systemctl reset-failed " + unit) { + t.Errorf("%s failed because its source was away and is cleared: %q", unit, f.calls) + } + } + if len(cleared) != 2 || !strings.Contains(strings.Join(cleared, "; "), "games was not there to mount") { + t.Errorf("each clearing is said: %q", cleared) + } + states := failedLook(t, f) + if states["games"].State != StateAbsent || states["library"].State != StateUnreachable { + t.Errorf("the module still says each as absent or unreachable: %+v", states) + } +} + +func TestAMountThatFailedWhileItsSourceAnswersStaysFailed(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "shares=library=/srv/library\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + f.write(ExportsFile, exportsAnswer(t, library)) + f.reachable["192.0.2.10:2049"] = true + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) + f.calls = nil + + f.units["srv-library.mount"] = map[string]string{"ActiveState": "failed", "Result": "exit-code"} + if cleared := a.clearFailed(context.Background()); len(cleared) != 0 || f.called("systemctl reset-failed") { + t.Fatalf("a mount refused by a server that answers is a fault and is left failed: %q %q", cleared, f.calls) + } + if s := failedLook(t, f)["library"]; s.State != StateFailed { + t.Errorf("it is said failed: %+v", s) + } +} + +func TestOnlyTheActingProcessClears(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "shares=library=/srv/library\nserver=homeserver\nat=192.0.2.10\nnode=workstation\n") + f.write(ExportsFile, exportsAnswer(t, library)) + f.units["srv-library.mount"] = map[string]string{"ActiveState": "failed", "Result": "timeout"} + a := f.applier(time.Now()) + a.Act = false + a.Look(context.Background()) + if f.called("systemctl reset-failed") { + t.Errorf("the judging process changes nothing: %q", f.calls) + } +} + +// waitFor polls until cond holds or the deadline passes. +func waitFor(t *testing.T, within time.Duration, cond func() bool) bool { + t.Helper() + deadline := time.Now().Add(within) + for time.Now().Before(deadline) { + if cond() { + return true + } + time.Sleep(5 * time.Millisecond) + } + return cond() +} + +func TestTheActingProcessClearsBetweenItsLooks(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Judge = false + // The next look is an hour away: only the loop between looks can clear what fails now. + a.LookInterval, a.ClearInterval = time.Hour, 10*time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { RunApply(ctx, a); close(done) }() + defer func() { cancel(); <-done }() + + if !waitFor(t, 2*time.Second, func() bool { return f.called("systemctl enable --now srv-library.automount") }) { + t.Fatal("the first look did not arm the pair") + } + // The server is away and someone opened the mount point: the mount failed after the look. + f.setUnit("srv-library.mount", map[string]string{"ActiveState": "failed", "Result": "timeout"}) + if !waitFor(t, 2*time.Second, func() bool { return f.called("systemctl reset-failed srv-library.mount") }) { + t.Fatal("a mount that failed between looks, its server away, is cleared before the next look") + } +} + +func TestALookClearsAMountOfItsOwnThatFailedWhileItsSourceWasAway(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) + f.setUnit("srv-library.mount", map[string]string{"ActiveState": "failed", "Result": "timeout"}) + f.calls = nil + + // A second look, with nothing to write: the pair is the module's own already, so this is no takeover. + a.Look(context.Background()) + if !f.called("systemctl reset-failed srv-library.mount") { + t.Errorf("the look itself clears it: %q", f.calls) + } +} + +func TestAnAbsentDiskIsClearedAndAPresentOneIsNot(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, "sources=usb=disk:1234-ABCD@/mnt/usb\nnode=workstation\n") + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) + + f.setUnit("mnt-usb.mount", map[string]string{"ActiveState": "failed", "Result": "timeout"}) + f.calls = nil + if a.clearFailed(context.Background()); !f.called("systemctl reset-failed mnt-usb.mount") { + t.Errorf("the disk is not plugged in: its failed mount is cleared: %q", f.calls) + } + + // Plugged in, and the mount still failed: a fault, left as it is. + f.write("/dev/disk/by-uuid/1234-ABCD", "") + f.setUnit("mnt-usb.mount", map[string]string{"ActiveState": "failed", "Result": "exit-code"}) + f.calls = nil + if a.clearFailed(context.Background()); f.called("systemctl reset-failed") { + t.Errorf("a disk that is there and failed to mount is left failed: %q", f.calls) + } +} + +func TestARewrittenPairOfItsOwnKeepsItsFailure(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + f.reachable["192.0.2.10:2049"] = true + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) + + // The server moved the library and refuses the mount while it answers: the module's own failure. + f.setUnit("srv-library.mount", map[string]string{"ActiveState": "failed", "Result": "exit-code"}) + moved := library + moved.Path = "/tank/library" + f.write(ExportsFile, exportsAnswer(t, moved)) + f.calls = nil + a.Look(context.Background()) + if !strings.Contains(f.read(UnitDir+"/srv-library.mount"), "What=192.0.2.10:/tank/library") { + t.Fatal("the pair was not rewritten") + } + if f.called("systemctl reset-failed") { + t.Errorf("only a unit the module takes over has its failure cleared, not one of its own it rewrites: %q", f.calls) + } +} + +func TestAnAutomountThatGaveUpIsClearedAndArmedAgain(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + a := f.applier(time.Date(2026, 10, 9, 14, 0, 0, 0, time.UTC)) + a.Look(context.Background()) + + f.setUnit("srv-library.automount", map[string]string{"ActiveState": "failed", "Result": "mount-start-limit-hit", "UnitFileState": "enabled"}) + f.calls = nil + cleared := a.clearFailed(context.Background()) + if !f.called("systemctl reset-failed srv-library.automount") || !f.called("systemctl start srv-library.automount") { + t.Fatalf("the automount is cleared and started: %q", f.calls) + } + if !strings.Contains(strings.Join(cleared, ";"), "mount-start-limit-hit") { + t.Errorf("said with its result: %q", cleared) + } + + // With the server answering, an automount that gave up is a fault: left as it is. + f.reachable["192.0.2.10:2049"] = true + f.setUnit("srv-library.automount", map[string]string{"ActiveState": "failed", "Result": "mount-start-limit-hit", "UnitFileState": "enabled"}) + f.calls = nil + if a.clearFailed(context.Background()); f.called("systemctl reset-failed") || f.called("systemctl start") { + t.Errorf("left failed while the server answers: %q", f.calls) + } +} + +func TestATransientUnitOfTheSameNameIsNotTheModules(t *testing.T) { + f := newSystem(t) + f.write(ConfigFile, libraryOnly) + f.write(ExportsFile, exportsAnswer(t, library)) + f.write(TransientDir+"/srv-library.mount", "[Mount]\nWhat=elsewhere\n") + states := failedLook(t, f) + if s := states["library"]; s.State != StateFailed || !strings.Contains(s.Why, "transient") { + t.Errorf("a share whose unit someone made at run time is refused: %+v", s) + } + if f.called("systemctl enable --now srv-library.automount") { + t.Error("and not armed") + } +} diff --git a/modules/mounts/cmd/mounts/write_test.go b/modules/mounts/cmd/mounts/write_test.go index bf06d3d4..1446ff6f 100644 --- a/modules/mounts/cmd/mounts/write_test.go +++ b/modules/mounts/cmd/mounts/write_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "time" ) @@ -16,6 +17,8 @@ const testPassword = "s3cr3t-Pa55" // fakeSystem is a machine under a temporary root with a recorded service manager. type fakeSystem struct { + // mu guards calls, units and reachable, for a test that runs RunApply beside itself. + mu sync.Mutex t *testing.T root string calls []string @@ -55,6 +58,8 @@ func (f *fakeSystem) read(p string) string { } func (f *fakeSystem) run(_ context.Context, name string, args ...string) (string, error) { + f.mu.Lock() + defer f.mu.Unlock() f.calls = append(f.calls, name+" "+strings.Join(args, " ")) if name != "systemctl" || len(args) == 0 { return "", nil @@ -77,6 +82,17 @@ func (f *fakeSystem) run(_ context.Context, name string, args ...string) (string case "enable": u := args[len(args)-1] f.units[u] = map[string]string{"ActiveState": "active", "UnitFileState": "enabled"} + case "start": + u := args[len(args)-1] + if f.units[u] == nil { + f.units[u] = map[string]string{} + } + f.units[u]["ActiveState"], f.units[u]["Result"] = "active", "success" + case "reset-failed": + u := args[len(args)-1] + if f.units[u]["ActiveState"] == "failed" { + f.units[u]["ActiveState"], f.units[u]["Result"] = "inactive", "success" + } } return "", nil } @@ -88,6 +104,8 @@ func (f *fakeSystem) machine() *Machine { func (f *fakeSystem) applier(now time.Time) *Applier { return &Applier{M: f.machine(), Act: true, Judge: true, Dial: func(addr string, _ time.Duration) error { + f.mu.Lock() + defer f.mu.Unlock() if f.reachable[addr] { return nil } @@ -99,6 +117,8 @@ func (f *fakeSystem) applier(now time.Time) *Applier { } func (f *fakeSystem) called(prefix string) bool { + f.mu.Lock() + defer f.mu.Unlock() for _, c := range f.calls { if strings.HasPrefix(c, prefix) { return true @@ -560,3 +580,10 @@ func TestApplyActsAndWatchJudgesAndNeitherDoesTheOthersWork(t *testing.T) { t.Error("the password leaked") } } + +// setUnit sets one unit's state as the service manager answers it, under the lock. +func (f *fakeSystem) setUnit(unit string, state map[string]string) { + f.mu.Lock() + defer f.mu.Unlock() + f.units[unit] = state +}