Merge pull request 'mounts: never leave a failed mount unit when a source is away, or the failure of a unit it takes over (hq ADR 0263 rules 4 and 6)' (#162) from fix/mounts-leaves-no-failed-unit into main

modules/mounts/README.md

@@ -42,6 +42,17 @@ an autofs mount, so nothing writes into the empty directory under it.
 | an SMB source | `//<host>/<share>`, `Type=cifs` | `credentials=<file>,iocharset=utf8,_netdev,nofail,uid=…,gid=…` | 10 |
 | a disk | `/dev/disk/by-uuid/<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.

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
+}

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")
+	}
+}

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
+}