diff --git a/modules/nfs-server/README.md b/modules/nfs-server/README.md index 4630ce55..3c17f47a 100644 --- a/modules/nfs-server/README.md +++ b/modules/nfs-server/README.md @@ -29,7 +29,20 @@ another node requires. - **Never the backups.** A folder that is the machine's root, is not there, is not a folder, is inside a backup (restic) repository (any parent up to `/`), or holds one within four levels below it is refused, said by the tools, and makes the module unhealthy. The look below is bounded (200 000 folders); when it - is cut short, the share is refused rather than exported unchecked. + is cut short, the share is refused rather than exported unchecked. A clean look is believed for an hour. +- **Never where the backup seat keeps its data, checked on every pass.** So that the hour cannot hide a + repository started inside it, the tools ask the controller over the bus, every five minutes, which module + holds `node-backup` on this machine (`seat:mesh-controller.seats`) and every path that module keeps data + in here (`seat:mesh-controller.data`; on the home server the restic repository and its state). They + leave it in `/var/lib/nfs-server/from-bus/backup.json`, and every pass refuses a share at, inside or + holding one of those paths, compared after resolving symbolic links, with nothing kept. A failed ask + keeps the last paths, and the status and the health warn that they are the last answer, with its age + and why. Until a list has been learnt, what is exported stays exported and the health warns that the + list is not known: a slow bus is no reason to take the library away, and the look below still guards. + A warning is said in the health without making the module unhealthy. **This is a deliberate choice**: + while the backup seat's paths are unknown the module stays healthy with a warning, rather than + unhealthy or refusing, because the look below every share still refuses one that holds a repository, + and taking the library away from every node because the bus was slow would be the larger harm. - **Only while its filesystem is mounted.** A share that is a filesystem of its own is exported with `mp`, a folder inside one with `mp=`, so an unmounted dataset never exports the empty folder under it. A folder of the root filesystem carries neither. @@ -94,6 +107,37 @@ its config file, so on unassignment the config file is gone when the process is removes its export file — the one removal its code makes — and runs `exportfs -ra`, so the kernel lets the shares go. A plain restart, a push or a shutdown leaves the config file in place, and the exports stay. +**Two signs of being taken away.** The process takes a stop as an unassignment when its config file is +gone, or when its own unit (`nfs-server-exports.service`) is disabled: the node-engine stops a process it +removes with `systemctl disable --now`, which disables the unit first, while a restart, a push or a +shutdown leaves it enabled. The second sign covers a first apply that failed and recorded the process +after its config (mesh-host #62), when the config is still there at the stop. + +**Within the stop timeout.** The service manager kills a process that has not stopped 90 s after SIGTERM +(mesh-host #62), and a killed process cleans up nothing. So SIGTERM ends a running pass at once (its +commands and its look below a share are cancelled, and it writes nothing), and the clean-up has its own +bound of 10 s: the export file is removed first, which needs no command, then `exportfs -ra` runs within +that bound. Whatever it could not finish is logged as `nfs-server: UNASSIGNED, NOT CLEAN: …`, with what a +person must do. + +**If it is killed anyway** (it did not stop within the service manager's 90 s), nothing cleans up and the +module never starts again. Then, on the home server as root: remove `/etc/exports.d/mesh.exports` and run +`exportfs -ra`; `exportfs -v` should list no client of the private network's range for the module's folders. + +## The kernel's table follows the file + +The process has the kernel read the export files again (`exportfs -ra`) on its first pass, after any +pass whose reload failed, when the file changed, when a share it exports is missing from the kernel's +table, and when the kernel exports one of the module's folders to a client the file no longer has — a +client inside the private network's range that no other export file names. While the kernel still +exports such an entry, the module is unhealthy and says which. An export another file makes (ZFS's own, +`/etc/exports`) is not the module's and is left alone. + +**A hand-made export does not survive.** `exportfs -ra` makes the kernel's table exactly what the export +files say, so an export a person added by hand with `exportfs -o …` (in no file) is dropped by the first +pass of every start of the process, and by any later reload. An export meant to last belongs in a file: +the module's settings for a share of the mesh, or `/etc/exports` for anything else. + ## What it leaves - **A subfolder export shares its filesystem.** A share that is a folder inside a larger filesystem (not a filesystem of its own) is exported with `no_subtree_check`, so a client that forges file handles could diff --git a/modules/nfs-server/cmd/nfs-server/backup.go b/modules/nfs-server/cmd/nfs-server/backup.go new file mode 100644 index 00000000..8bf3d6fd --- /dev/null +++ b/modules/nfs-server/cmd/nfs-server/backup.go @@ -0,0 +1,197 @@ +package main + +// Where this machine's backup seat keeps its data, from the bus (follow-up to the review of +// mesh-catalog #136, point 4). +// +// The look below a share for a backup repository walks the folders and is believed for LookKept, so a +// repository that the backup seat starts keeping under a share inside that hour would be exported until the +// look is taken again. So the module also learns, over the bus, which module holds `node-backup` on this +// machine and every path that module keeps data in (its repository among them), and every pass judges +// every share against those paths with nothing kept: a share at, under or above one of them is refused. +// +// The tools ask — the root process does not reach the bus — every BackupEvery, and leave what they learnt +// in BackupFile for the root process. A failed ask keeps the last paths and says why. Until a list has +// been read, what is exported stays exported and the status says the list is not known: the bus being +// slow is no reason to take the library away, and the look below still guards against a repository. +// +// The parsing of mesh-controller.data's answer is a small copy of the mounts module's (DataFrom). Shared +// code belongs in mesh-sdk: moving both there is mesh-sdk issue 13. + +import ( + "encoding/json" + "fmt" + "os" + "path" + "sort" + "time" +) + +// BackupFile is what the tools learnt about the backup seat on this machine. +const BackupFile = WantsDir + "/backup.json" + +// BackupEvery is how often the tools ask. +const BackupEvery = 5 * time.Minute + +// BackupSeat is the seat whose holder keeps this machine's backups. +const BackupSeat = "node-backup" + +// BackupFromBus is the backup seat's holder on this machine and the paths it keeps data in. +type BackupFromBus struct { + Holder string `json:"holder"` + Paths []string `json:"paths"` + Looked string `json:"looked,omitempty"` + // Learnt is when an ask last succeeded: how old the holder and paths are. + Learnt string `json:"learnt,omitempty"` + // Error is why the last ask got no answer; the holder and paths are then the last answer's. + Error string `json:"error,omitempty"` +} + +// BackupFrom reads the holder of node-backup on one machine from mesh-controller.seats' answer, and the +// paths that module keeps data in on that machine from mesh-controller.data's. A machine whose backup seat +// nobody holds, and an answer with no data list, are errors: never an empty list read as "nothing to fear". +func BackupFrom(machine string, seats, data json.RawMessage) (string, []string, error) { + var s struct { + Answer *struct { + Seats []struct { + Seat string `json:"seat"` + Holders []struct { + Module string `json:"module"` + Node string `json:"node"` + } `json:"holders"` + } `json:"seats"` + } `json:"answer"` + } + if err := json.Unmarshal(seats, &s); err != nil || s.Answer == nil { + return "", nil, fmt.Errorf("the controller's seats answer could not be read: %v", err) + } + holder := "" + for _, seat := range s.Answer.Seats { + if seat.Seat != BackupSeat { + continue + } + for _, h := range seat.Holders { + if h.Node == machine { + holder = h.Module + } + } + } + if holder == "" { + return "", nil, fmt.Errorf("no module holds %s on %s", BackupSeat, machine) + } + var d struct { + Answer *struct { + Data *[]struct { + Machine string `json:"machine"` + Module string `json:"module"` + Path string `json:"path"` + } `json:"data"` + } `json:"answer"` + } + if err := json.Unmarshal(data, &d); err != nil || d.Answer == nil || d.Answer.Data == nil { + return "", nil, fmt.Errorf("the controller's data answer holds no data list (%v)", err) + } + var paths []string + for _, item := range *d.Answer.Data { + if item.Module == holder && (item.Machine == "" || item.Machine == machine) && path.IsAbs(item.Path) { + paths = append(paths, path.Clean(item.Path)) + } + } + sort.Strings(paths) + return holder, paths, nil +} + +// TakeBackup asks the controller over the bus, as the module, through the runtime. +func (m *Machine) TakeBackup(machine string) { + m.TakeBackupWith(machine, askBus) +} + +// TakeBackupWith is TakeBackup with the ask given, for tests. A failed ask keeps the last holder and +// paths and adds why. +func (m *Machine) TakeBackupWith(machine string, ask func(key string, body any) (json.RawMessage, error)) { + var last BackupFromBus + if text, err := m.Read(BackupFile); err == nil { + _ = json.Unmarshal([]byte(text), &last) + } + next := last + next.Looked = m.Now().UTC().Format(time.RFC3339) + next.Error = "" + seats, err := ask("seat:mesh-controller.seats", map[string]any{}) + var data json.RawMessage + if err == nil { + data, err = ask("seat:mesh-controller.data", map[string]any{"machine": machine}) + } + if err == nil { + var holder string + var paths []string + holder, paths, err = BackupFrom(machine, seats, data) + if err == nil { + next.Holder, next.Paths, next.Learnt = holder, paths, next.Looked + } + } + if err != nil { + next.Error = err.Error() + fmt.Fprintln(os.Stderr, "nfs-server: where the backup seat keeps its data could not be learnt:", err) + } + raw, _ := json.MarshalIndent(next, "", " ") + if werr := m.Write(BackupFile, string(raw)+"\n"); werr != nil { + fmt.Fprintln(os.Stderr, "nfs-server: the backup seat's paths could not be written:", werr) + } +} + +// Backup is what the tools last learnt, and a sentence when it cannot be relied on as it stands: nothing +// usable learnt yet, or a later ask failed and what is used is the last answer, with its age. +func (m *Machine) Backup() (BackupFromBus, string) { + text, err := m.Read(BackupFile) + if err != nil { + return BackupFromBus{}, "where this machine's backup seat keeps its repository has not been learnt from the bus yet; the shares are judged by the look below them alone until it is" + } + var b BackupFromBus + if err := json.Unmarshal([]byte(text), &b); err != nil { + return BackupFromBus{}, fmt.Sprintf("what was learnt about the backup seat (%s) could not be read: %v", BackupFile, err) + } + if len(b.Paths) == 0 { + why := "no path has been learnt" + if b.Error != "" { + why = b.Error + } + return b, "where this machine's backup seat keeps its repository is not known (" + why + "); the shares are judged by the look below them alone until it is" + } + if b.Error != "" { + age := "of unknown age" + if at, err := time.Parse(time.RFC3339, b.Learnt); err == nil { + age = fmt.Sprintf("learnt %s ago, at %s", m.Now().Sub(at).Round(time.Second), at.Local().Format("15:04")) + } + return b, fmt.Sprintf("where this machine's backup seat (%s) keeps its data is the last answer, %s: the "+ + "latest ask failed (%s)", b.Holder, age, b.Error) + } + return b, "" +} + +// realPaths are paths as the kernel resolves them, so a backup path named through a symbolic link is +// compared where it really is; one that cannot be resolved is kept as named. +func (m *Machine) realPaths(paths []string) []string { + out := make([]string, 0, len(paths)) + for _, p := range paths { + if real, err := m.Real(p); err == nil { + out = append(out, real) + } else { + out = append(out, p) + } + } + return out +} + +// nearBackup is the backup seat's path a real path is at, under or above, or "". +func nearBackup(real string, b BackupFromBus) (string, string) { + for _, p := range b.Paths { + switch { + case real == p: + return p, "is" + case len(real) > len(p) && real[:len(p)] == p && (p == "/" || real[len(p)] == '/'): + return p, "is inside" + case len(p) > len(real) && p[:len(real)] == real && (real == "/" || p[len(real)] == '/'): + return p, "holds" + } + } + return "", "" +} diff --git a/modules/nfs-server/cmd/nfs-server/backup_test.go b/modules/nfs-server/cmd/nfs-server/backup_test.go new file mode 100644 index 00000000..7ae3298b --- /dev/null +++ b/modules/nfs-server/cmd/nfs-server/backup_test.go @@ -0,0 +1,173 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + "time" +) + +// Follow-up to the review (point 4): the one-hour look below a share must not hide a backup repository +// that appears later. Where this machine's backup seat keeps its repository is read from the bus and every +// share is judged against it on every pass. + +func (f *fake) backup(holder string, paths ...string) { + raw, _ := json.Marshal(BackupFromBus{Holder: holder, Paths: paths}) + f.file(BackupFile, string(raw)) +} + +func TestAShareHoldingTheBackupSeatsRepositoryIsRefusedOnTheNextPass(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/pool/media") + f.dir("/srv/pool/restic") + f.file(ConfigFile, grantsConfig("pool=/srv/pool media=/srv/pool/media", "pool=ws:rw media=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "pool", Access: "rw"}, {Share: "media", Access: "rw"}}}) + m := f.machine() + now := time.Date(2026, 10, 8, 20, 0, 0, 0, time.UTC) + m.Now = func() time.Time { return now } + // The repository is not there yet: the look below finds nothing and is kept for an hour. + if s := m.Pass(context.Background(), nil); refusedWhy(s, "pool") != "" { + t.Fatalf("the pool is refused before any repository exists: %+v", s.Refused) + } + // The backup seat starts keeping its repository there; a minute later is well inside LookKept. + f.backup("restic", "/srv/pool/restic", "/var/lib/restic") + now = now.Add(time.Minute) + s := m.Pass(context.Background(), nil) + if why := refusedWhy(s, "pool"); !strings.Contains(why, "/srv/pool/restic") || !strings.Contains(why, "restic") { + t.Errorf("a share holding the backup seat's repository is exported inside the kept look: %q", why) + } + if refusedWhy(s, "media") != "" { + t.Errorf("a share beside the repository is refused: %q", refusedWhy(s, "media")) + } + if strings.Contains(f.read(ExportFile), "/srv/pool ") { + t.Errorf("the export file still names the pool:\n%s", f.read(ExportFile)) + } +} + +func TestAShareInsideTheBackupSeatsRepositoryIsRefused(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/backups/data") + f.file(ConfigFile, grantsConfig("inside=/srv/backups/data", "inside=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "inside", Access: "rw"}}}) + f.backup("restic", "/srv/backups") + s := f.machine().Pass(context.Background(), nil) + if !strings.Contains(refusedWhy(s, "inside"), "/srv/backups") { + t.Errorf("a share inside the backup seat's repository is not refused: %+v", s.Refused) + } +} + +// Until a list has been read, what is exported stays exported — the bus being slow is no reason to take +// the library away — and the status says the list is not known. +func TestWithoutTheBackupListTheExportsStayAndItIsSaid(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + s := f.machine().Pass(context.Background(), nil) + if refusedWhy(s, "library") != "" || len(s.Shares) != 1 { + t.Fatalf("the library is not exported while the backup list is unknown: %+v", s) + } + if !strings.Contains(strings.Join(s.Warnings, "\n"), "backup seat") { + t.Errorf("the status does not warn that the backup seat's repository is not known: %v", s.Warnings) + } +} + +func TestBackupFromTheSeatsAndTheData(t *testing.T) { + seats := json.RawMessage(`{"answer":{"seats":[{"seat":"node-backup","scope":"node","holders":[{"module":"restic","node":"ace"},{"module":"borg","node":"shanks"}]}]},"ok":true}`) + data := json.RawMessage(`{"answer":{"data":[{"machine":"ace","module":"restic","item":"repository","path":"/storage/restic"},{"machine":"ace","module":"restic","item":"state","path":"/var/lib/restic"},{"machine":"ace","module":"plex","item":"data","path":"/mnt/plex/data"}]},"ok":true}`) + holder, paths, err := BackupFrom("ace", seats, data) + if err != nil || holder != "restic" || strings.Join(paths, " ") != "/storage/restic /var/lib/restic" { + t.Errorf("got %q %v %v", holder, paths, err) + } + if _, _, err := BackupFrom("g14", seats, data); err == nil { + t.Errorf("a machine whose backup seat nobody holds is not said") + } + if _, _, err := BackupFrom("ace", seats, json.RawMessage(`{"ok":true}`)); err == nil { + t.Errorf("an answer with no data list reads as an empty list") + } +} + +// A failed ask keeps the last paths: the file is not emptied, only the error is added. +func TestAFailedAskKeepsTheLastBackupPaths(t *testing.T) { + f := newFake(t) + m := f.machine() + m.TakeBackupWith("ace", func(string, any) (json.RawMessage, error) { + return nil, errors.New("no responders") + }) + f.backup("restic", "/storage/restic") + m.TakeBackupWith("ace", func(string, any) (json.RawMessage, error) { + return nil, errors.New("timeout") + }) + var b BackupFromBus + if err := json.Unmarshal([]byte(f.read(BackupFile)), &b); err != nil { + t.Fatal(err) + } + if strings.Join(b.Paths, " ") != "/storage/restic" || !strings.Contains(b.Error, "timeout") { + t.Errorf("a failed ask lost the last paths or did not say why: %+v", b) + } +} + +// The review of #142: the backup seat's path is compared as the kernel resolves it. A repository the data +// list names through a symbolic link is still found under the share that really holds it. +func TestTheBackupPathIsComparedAfterItsSymlinks(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/pool/restic") + f.symlink("pool/restic", "/srv/backups") // /srv/backups -> /srv/pool/restic + f.file(ConfigFile, grantsConfig("pool=/srv/pool", "pool=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "pool", Access: "rw"}}}) + f.backup("restic", "/srv/backups") + s := f.machine().Pass(context.Background(), nil) + if why := refusedWhy(s, "pool"); !strings.Contains(why, "/srv/pool/restic") { + t.Errorf("a share holding the backup seat's repository, named through a symlink, is not refused: %q", why) + } +} + +// The review of #142: a list the tools could not refresh is said in the status and the health with its +// age, and a list not known yet is a warning in the health — neither is a plain "healthy". +func TestAStaleOrUnknownBackupListIsSaidWithItsAge(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + now := time.Date(2026, 10, 8, 20, 0, 0, 0, time.UTC) + m.Now = func() time.Time { return now } + + m.Pass(context.Background(), nil) + h := m.Health() + if !strings.Contains(h.Why, "Warning") || !strings.Contains(h.Why, "backup seat") { + t.Errorf("an unknown backup list is not a warning in the health: %+v", h) + } + + ok := func(key string, body any) (json.RawMessage, error) { + if strings.Contains(key, "seats") { + return json.RawMessage(`{"answer":{"seats":[{"seat":"node-backup","holders":[{"module":"restic","node":"hs"}]}]}}`), nil + } + return json.RawMessage(`{"answer":{"data":[{"machine":"hs","module":"restic","path":"/srv/restic"}]}}`), nil + } + m.TakeBackupWith("hs", ok) + now = now.Add(2 * time.Hour) + m.TakeBackupWith("hs", func(string, any) (json.RawMessage, error) { return nil, errors.New("the bus did not answer") }) + s := m.Pass(context.Background(), nil) + said := strings.Join(append(append([]string{}, s.Said...), s.Warnings...), "\n") + if !strings.Contains(said, "2h0m0s") || !strings.Contains(said, "the bus did not answer") { + t.Errorf("the status does not say the backup list is 2 h old and why: %q", said) + } + h = m.Health() + if !strings.Contains(h.Why, "2h0m0s") { + t.Errorf("the health does not say the backup list's age: %+v", h) + } +} diff --git a/modules/nfs-server/cmd/nfs-server/guards_test.go b/modules/nfs-server/cmd/nfs-server/guards_test.go new file mode 100644 index 00000000..add729ff --- /dev/null +++ b/modules/nfs-server/cmd/nfs-server/guards_test.go @@ -0,0 +1,111 @@ +package main + +// One test per guard of the review of #142, each failing when its guard alone is taken away. + +import ( + "context" + "encoding/json" + "strings" + "testing" +) + +func TestStrayFindsAnExportOfTheModulesThatNoFileCarries(t *testing.T) { + live := []Entry{{Path: "/srv/library", Client: "10.0.0.4"}} + if got := Stray(live, nil, nil, meshRange); len(got) != 1 { + t.Errorf("an export inside the mesh range that no file carries is the module's leftover: %+v", got) + } +} + +// The foreign-file guard alone: the client is inside the mesh's range, so only the other file tells it apart. +func TestStrayLeavesAnExportAnotherFileCarriesEvenInsideTheMeshRange(t *testing.T) { + // ZFS's own file exports the library to a client inside the mesh's range. + foreign := ParseExports("/srv/library 10.0.0.9(rw,no_subtree_check)\n", "/etc/exports.d/zfs.exports") + live := []Entry{{Path: "/srv/library", Client: "10.0.0.9", Options: "rw"}} + if got := Stray(live, nil, foreign, meshRange); len(got) != 0 { + t.Errorf("what ZFS's file exports is not the module's, whatever its client: %+v", got) + } +} + +// The range guard alone: no file carries the client, so only the range tells it apart. +func TestStrayLeavesAClientOutsideTheMeshRange(t *testing.T) { + live := []Entry{{Path: "/srv/library", Client: "192.0.2.0/24"}, {Path: "/srv/library", Client: "192.0.2.7"}, + {Path: "/srv/library", Client: "10.0.0.0/16"}} + if got := Stray(live, nil, nil, meshRange); len(got) != 0 { + t.Errorf("a client the module never exports to (outside or wider than the mesh's range) is not its leftover: %+v", got) + } +} + +// The failed-reload retry alone: the file is unchanged, every share is live and nothing is stray, so the +// last pass's failed reload is the only reason to reload. +func TestAFailedReloadIsTriedAgainWhenNothingElseAsks(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/library") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + s := m.Pass(context.Background(), nil) + if !s.Shares[0].Live { + t.Fatalf("the library was not exported: %+v", s) + } + + f.ran = nil + if s = m.Pass(context.Background(), &s); countExportfs(f.ran) != 0 { + t.Fatalf("with nothing to do a pass reloads: %v", f.ran) + } + s.ReloadError = "exportfs: stuck" + f.ran = nil + m.Pass(context.Background(), &s) + if countExportfs(f.ran) == 0 { + t.Errorf("the last pass's failed reload was not tried again: %v", f.ran) + } +} + +func TestBackupFromRefusesAnAnswerWithoutADataList(t *testing.T) { + seats := json.RawMessage(`{"answer":{"seats":[{"seat":"node-backup","holders":[{"module":"restic","node":"ace"}]}]}}`) + for _, data := range []string{`{"answer":{}}`, `{"answer":{"data":null}}`} { + if _, paths, err := BackupFrom("ace", seats, json.RawMessage(data)); err == nil { + t.Errorf("%s was read as the empty list %v: never \"nothing to fear\"", data, paths) + } + } +} + +func TestTheLookBelowStopsWhenTheProcessStops(t *testing.T) { + f := newFake(t) + f.dir("/srv/library/movies/a") + m := f.machine() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + m.setWalk(ctx) + if found, complete := m.walkBelow("/srv/library"); complete || found != "" { + t.Errorf("a look the process ended is unfinished, so the share is refused rather than exported: %q %v", found, complete) + } +} + +func TestAPassEndedByStoppingWritesAndReloadsNothing(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/library") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + s := m.Pass(context.Background(), nil) + file, status := f.read(ExportFile), f.read(StatusFile) + + // The grant is withdrawn, and the process is stopping as the next pass runs. + f.file(ConfigFile, grantsConfig("library=/srv/library", "none")) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + f.ran = nil + got := m.Pass(ctx, &s) + if f.read(ExportFile) != file || f.read(StatusFile) != status || countExportfs(f.ran) != 0 { + t.Errorf("a pass ended by stopping wrote or reloaded: file %q, ran %v", f.read(ExportFile), f.ran) + } + if !strings.Contains(strings.Join(got.Problems, ";"), "nothing was written") { + t.Errorf("it says so: %+v", got.Problems) + } +} diff --git a/modules/nfs-server/cmd/nfs-server/machine.go b/modules/nfs-server/cmd/nfs-server/machine.go index 857a50ed..fdd948bf 100644 --- a/modules/nfs-server/cmd/nfs-server/machine.go +++ b/modules/nfs-server/cmd/nfs-server/machine.go @@ -108,6 +108,21 @@ type Machine struct { // clean is when a look below each real path last found no backup repository (LookKept). clean map[string]time.Time cleanMu sync.Mutex + // walk is the running pass's context, which ends the look below a share when the process stops. + walk context.Context + walkMu sync.Mutex +} + +func (m *Machine) setWalk(ctx context.Context) { + m.walkMu.Lock() + m.walk = ctx + m.walkMu.Unlock() +} + +func (m *Machine) walking() context.Context { + m.walkMu.Lock() + defer m.walkMu.Unlock() + return m.walk } // Here is the machine this process runs on. @@ -232,11 +247,11 @@ func (m *Machine) Glob(pattern string) []string { } // written are the only paths Write may touch. -var written = map[string]bool{ExportFile: true, StatusFile: true, WantsFile: true} +var written = map[string]bool{ExportFile: true, StatusFile: true, WantsFile: true, BackupFile: true} // Write replaces one of the module's own files, atomically. Every other path is refused: the module owns -// these three files (the export file and the status, written as root; the wants, written by the tools) -// and nothing else on the machine. +// exactly these four files and nothing else on the machine: the export file and the status, written by the +// root process; the wants and what was learnt about the backup seat, written by the tools from the bus. func (m *Machine) Write(path, content string) error { if !written[path] { return fmt.Errorf("the module writes only %s, %s and %s, not %s", ExportFile, StatusFile, WantsFile, path) @@ -272,8 +287,17 @@ func (m *Machine) Escalated(name string, args ...string) (string, []string) { // AsRoot runs what only root may do, and names a refusal by how it failed. func (m *Machine) AsRoot(name string, args ...string) (Ran, error) { + return m.AsRootWithin(context.Background(), name, args...) +} + +// AsRootWithin is AsRoot ended with its context: the process's pass is cancelled by SIGTERM, and Stopped +// runs within StopTimeout. +func (m *Machine) AsRootWithin(ctx context.Context, name string, args ...string) (Ran, error) { program, argv := m.Escalated(name, args...) - r := m.Run(context.Background(), program, argv...) + r := m.Run(ctx, program, argv...) + if err := ctx.Err(); err != nil && r.Code != 0 { + return r, fmt.Errorf("%s %s did not finish: %v", name, strings.Join(args, " "), err) + } if r.Code != 0 { why := r.Err if why == "" { diff --git a/modules/nfs-server/cmd/nfs-server/main.go b/modules/nfs-server/cmd/nfs-server/main.go index 12315c53..3406d5a8 100644 --- a/modules/nfs-server/cmd/nfs-server/main.go +++ b/modules/nfs-server/cmd/nfs-server/main.go @@ -74,12 +74,25 @@ func ShareExports(m *Machine) { } // The SDK reaches the bus once Serve runs. time.Sleep(2 * time.Second) + m.TakeBackup(machine) put() - for range time.Tick(Every) { - put() + backup := time.Tick(BackupEvery) + every := time.Tick(Every) + for { + select { + case <-backup: + m.TakeBackup(machine) + case <-every: + put() + } } } +// askBus calls a seat's verb over the bus as this module, through the runtime. +func askBus(key string, body any) (json.RawMessage, error) { + return stdio.Ask(key, body) +} + // TakeWants reads every node's `mounts` state from the bus and writes what is asked of this machine to // the wants file, for the root process. When the bus does not answer, the last wants stay. func (m *Machine) TakeWants(machine string) { diff --git a/modules/nfs-server/cmd/nfs-server/manifest_test.go b/modules/nfs-server/cmd/nfs-server/manifest_test.go index 548371d3..f9e12319 100644 --- a/modules/nfs-server/cmd/nfs-server/manifest_test.go +++ b/modules/nfs-server/cmd/nfs-server/manifest_test.go @@ -162,3 +162,17 @@ func TestTheManifestCarriesGrantsTheRosterTheWantsAndTheRemovalOrder(t *testing. t.Errorf("the process must be declared before its config, so unassignment removes the config first: %v", order) } } + +// Stopped asks whether the process's own unit is disabled; that unit is the manifest's process name. +func TestTheProcessUnitIsTheManifestsProcess(t *testing.T) { + m, _ := readManifest(t) + for _, r := range m.Resources { + if r["type"] == "process" && r["id"] == "exports" { + if got := r["name"].(string) + ".service"; got != ProcessUnit { + t.Errorf("the exports process runs as %s, and Stopped asks about %s", got, ProcessUnit) + } + return + } + } + t.Error("the manifest has no exports process") +} diff --git a/modules/nfs-server/cmd/nfs-server/process.go b/modules/nfs-server/cmd/nfs-server/process.go index b28b933d..c6c6172e 100644 --- a/modules/nfs-server/cmd/nfs-server/process.go +++ b/modules/nfs-server/cmd/nfs-server/process.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "fmt" + "net/netip" "os" + "path" "strings" "time" ) @@ -26,6 +28,9 @@ type Status struct { // Said is what does not match between the grants and the wants: a node wanting a share it is not // granted, a grant nobody wants, a share exported to no node. Said []string `json:"said,omitempty"` + // Warnings make the module no less healthy but are said in its health: what it could not learn, and + // how old what it relies on is. + Warnings []string `json:"warnings,omitempty"` // Reloaded is when the process last had the kernel read the export files again, and why. Reloaded *time.Time `json:"reloaded,omitempty"` ReloadWhy string `json:"reload_why,omitempty"` @@ -53,7 +58,17 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status { if note != "" { s.Said = append(s.Said, note) } + if _, note := m.Backup(); note != "" { + s.Warnings = append(s.Warnings, note) + } + m.setWalk(ctx) exports, refused, said := m.Judge(c, roster, wants) + m.setWalk(nil) + if ctx.Err() != nil { + // Stopping: write nothing and reload nothing, so Stopped starts at once. + s.Problems = append(s.Problems, "the pass was ended by the process stopping; nothing was written") + return s + } s.Refused = refused s.Said = append(s.Said, said...) @@ -68,6 +83,14 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status { } } live, note := m.Live() + foreign, _, _ := m.Foreign() + mine := ParseExports(want, ExportFile) + if reload == "" && last == nil { + reload = "the process started, and the kernel's table is made to match the file" + } + if reload == "" && last != nil && last.ReloadError != "" { + reload = "the last reload failed: " + last.ReloadError + } if reload == "" { for _, e := range exports { if !SharedLive(live, e) { @@ -76,8 +99,13 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status { } } } + if reload == "" { + if stray := Stray(live, mine, foreign, c.Range); len(stray) > 0 { + reload = fmt.Sprintf("the kernel exports %s to %s, which the export file no longer carries", stray[0].Path, stray[0].Client) + } + } if reload != "" { - if _, err := m.AsRoot("exportfs", "-ra"); err != nil { + if _, err := m.AsRootWithin(ctx, "exportfs", "-ra"); err != nil { s.ReloadError = err.Error() } else { at := m.Now().UTC() @@ -88,6 +116,10 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status { if note != "" && len(exports) > 0 { s.Problems = append(s.Problems, note) } + for _, e := range Stray(live, mine, foreign, c.Range) { + s.Problems = append(s.Problems, fmt.Sprintf("the kernel still exports %s to %s, which the module's export file "+ + "no longer carries: a reload has to take it away (exportfs -ra as root)", e.Path, e.Client)) + } for i := range exports { exports[i].Live = SharedLive(live, exports[i]) } @@ -99,6 +131,13 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status { return s } +// ProcessUnit is the service the node-engine runs this process as: the manifest's process name, `.service`. +const ProcessUnit = "nfs-server-exports.service" + +// StopTimeout bounds Stopped and the commands it runs: well under the service manager's stop timeout (90 s, +// then SIGKILL), so the export file is gone before the process could be killed (mesh-host #62). +var StopTimeout = 10 * time.Second + // RunExports is the process: a pass now and every Every until the context ends, then Stopped. func (m *Machine) RunExports(ctx context.Context) error { last := m.Pass(ctx, nil) @@ -172,20 +211,80 @@ func SharedLive(live []Entry, e Exported) bool { return true } -// Stopped is the process ending. When the module was unassigned — the node-engine removes the module's -// files in reverse declaration order, so the config file, declared after this process, is already gone -// when the process is stopped (mesh-host internal/apply: orphans removed in reverse order) — the -// module's export file goes and the kernel lets the shares go. A plain restart, a push or the machine -// shutting down leaves the config file, and the exports stay. +// Stopped is the process ending. When the module was unassigned (see unassigned) the module's export file +// goes and the kernel lets the shares go. A plain restart, a push or the machine shutting down leaves the +// config file and the unit enabled, and the exports stay. func (m *Machine) Stopped() { - if _, err := m.Read(ConfigFile); err == nil || !os.IsNotExist(err) { + ctx, cancel := context.WithTimeout(context.Background(), StopTimeout) + defer cancel() + if !m.unassigned(ctx) { return } + // The file first: removing it needs no command, so a hanging sudo or exportfs cannot keep it. if err := m.unexport(); err != nil { - fmt.Fprintln(os.Stderr, "nfs-server:", err) + loud("the module was unassigned and its export file is still there: " + err.Error() + + "; remove " + ExportFile + " and run exportfs -ra as root") return } - if _, err := m.AsRoot("exportfs", "-ra"); err != nil { - fmt.Fprintln(os.Stderr, "nfs-server: the export file is gone, and exportfs -ra failed:", err) + fmt.Fprintln(os.Stderr, "nfs-server: unassigned: the export file "+ExportFile+" is removed") + if _, err := m.AsRootWithin(ctx, "exportfs", "-ra"); err != nil { + loud("the module was unassigned and its export file is removed, but the kernel may still export its " + + "shares until exportfs -ra runs as root: " + err.Error()) + return } + fmt.Fprintln(os.Stderr, "nfs-server: unassigned: the kernel let the shares go (exportfs -ra)") +} + +// unassigned says whether this stop is the module being taken away: its config file is gone (the node-engine +// removes resources in reverse of their record, and the config is recorded after this process), or this +// process's own unit is disabled. The node-engine stops a process it removes with `systemctl disable +// --now`, which disables the unit before it stops it, while a restart, a push or a shutdown leaves it +// enabled — so the second sign holds even when a failed first apply recorded the process after its config +// (mesh-host #62) and the config is still there. +func (m *Machine) unassigned(ctx context.Context) bool { + if _, err := m.Read(ConfigFile); os.IsNotExist(err) { + return true + } + r := m.Run(ctx, "systemctl", "is-enabled", ProcessUnit) + return strings.TrimSpace(r.Out) == "disabled" +} + +// loud says, on the process's log where the service manager's journal keeps it, what an unassignment left +// undone, so a person finds it rather than an export that outlived its module. +func loud(what string) { + fmt.Fprintln(os.Stderr, "nfs-server: UNASSIGNED, NOT CLEAN: "+what) +} + +// Stray is what the kernel exports that is the module's and that its file no longer carries: an entry to a +// client inside the private network's range — the only clients the module ever exports to — that neither +// the module's file nor any other export file (/etc/exports, ZFS's own) has. What another file exports is +// not the module's, whatever its client. +func Stray(live, mine, foreign []Entry, rng string) []Entry { + prefix, err := netip.ParsePrefix(rng) + if err != nil { + return nil + } + has := map[string]bool{} + for _, e := range append(append([]Entry{}, mine...), foreign...) { + has[path.Clean(e.Path)+" "+e.Client] = true + } + var out []Entry + for _, e := range live { + if has[path.Clean(e.Path)+" "+e.Client] || !insideRange(e.Client, prefix) { + continue + } + out = append(out, e) + } + return out +} + +// insideRange says whether a client — one address, or a network — lies wholly inside a range. +func insideRange(client string, prefix netip.Prefix) bool { + if a, err := netip.ParseAddr(client); err == nil { + return prefix.Contains(a) + } + if p, err := netip.ParsePrefix(client); err == nil { + return p.Bits() >= prefix.Bits() && prefix.Contains(p.Addr()) + } + return false } diff --git a/modules/nfs-server/cmd/nfs-server/reconcile_test.go b/modules/nfs-server/cmd/nfs-server/reconcile_test.go new file mode 100644 index 00000000..d6be1ada --- /dev/null +++ b/modules/nfs-server/cmd/nfs-server/reconcile_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" +) + +func countExportfs(ran []string) int { + n := 0 + for _, r := range ran { + if strings.Contains(r, "exportfs -ra") { + n++ + } + } + return n +} + +// The review of #142: pass 1 exports /srv/pool; pass 2 no longer exports it and exportfs fails; pass 3 ran nothing, +// and the kernel still exported the refused folder while the module said it was healthy. A failed reload +// is tried again on the next pass, and while the kernel exports anything of the module's that the file no +// longer carries, the module is unhealthy and says what. +func TestARefusalWhoseReloadFailedIsReloadedAgainAndIsUnhealthyUntilItIs(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/pool") + f.dir("/srv/pool") + f.file(ConfigFile, grantsConfig("pool=/srv/pool", "pool=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "pool", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + s := m.Pass(context.Background(), nil) + if !strings.Contains(f.read(Etab), "/srv/pool") { + t.Fatalf("pass 1 did not export the pool: %+v", s) + } + + f.file(ConfigFile, grantsConfig("pool=/srv/pool", "none")) // pass 2: the grant is taken away + f.onExportfs = nil + f.answers["exportfs -ra"] = Ran{Code: 1, Err: "exportfs: stuck"} + f.answers["sudo -n exportfs -ra"] = Ran{Code: 1, Err: "exportfs: stuck"} + s = m.Pass(context.Background(), &s) + if s.ReloadError == "" { + t.Fatalf("pass 2's reload did not fail as arranged: %+v", s) + } + + f.ran = nil + s = m.Pass(context.Background(), &s) + if countExportfs(f.ran) == 0 { + t.Errorf("pass 3 did not try the failed reload again: %v", f.ran) + } + h := m.Health() + if h.Healthy || !strings.Contains(h.Why, "/srv/pool") { + t.Errorf("the kernel still exports the refused /srv/pool and the module says: %+v", h) + } + + delete(f.answers, "exportfs -ra") + delete(f.answers, "sudo -n exportfs -ra") + f.kernelReads() + s = m.Pass(context.Background(), &s) + if strings.Contains(f.read(Etab), "/srv/pool") { + t.Errorf("a reload that works again did not let the refused pool go: %q", f.read(Etab)) + } + for _, p := range s.Problems { + if strings.Contains(p, "/srv/pool") { + t.Errorf("a problem stays after the kernel let the pool go: %q", p) + } + } +} + +// A grant taken away while the kernel still holds it — exported before the process started, or a reload +// that never happened — is reconciled: the first pass always reloads, a later pass reloads when the kernel +// exports one of the module's folders to a client the file no longer has, and the module is unhealthy +// while it cannot. +func TestAWithdrawnGrantIsTakenFromTheKernel(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/library") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3", "laptop 10.0.0.4") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}, "laptop": {{Share: "library", Access: "ro"}}}) + f.kernelReads() + m := f.machine() + s := m.Pass(context.Background(), nil) + // The file and the kernel agree on ws only; the laptop's old export is left in the kernel's table, as + // a grant withdrawn while the process was down would be. + f.file(Etab, f.read(Etab)+"/srv/library\t10.0.0.4(ro,sync)\n") + + f.ran = nil + m2 := f.machine() // a restarted process: its first pass + m2.Pass(context.Background(), nil) + if countExportfs(f.ran) == 0 { + t.Errorf("the first pass of a process did not reload: %v", f.ran) + } + if strings.Contains(f.read(Etab), "10.0.0.4") { + t.Errorf("the withdrawn grant stays in the kernel after the first pass: %q", f.read(Etab)) + } + + f.file(Etab, f.read(Etab)+"/srv/library\t10.0.0.4(ro,sync)\n") + f.ran = nil + s = m.Pass(context.Background(), &s) + if countExportfs(f.ran) == 0 { + t.Errorf("a later pass did not reload while the kernel exported a folder of the module's to a client the file no longer has: %v", f.ran) + } + if strings.Contains(f.read(Etab), "10.0.0.4") { + t.Errorf("the withdrawn grant stays in the kernel: %q", f.read(Etab)) + } + + // A client the module never exported to — the home network's range, from ZFS's own file — is not + // the module's, and makes no reload and no finding. + f.file("/etc/exports.d/zfs.exports", "/srv/library 192.0.2.0/24(rw)\n") + f.file(Etab, f.read(Etab)+"/srv/library\t192.0.2.0/24(rw)\n") + f.ran = nil + s = m.Pass(context.Background(), &s) + if n := countExportfs(f.ran); n != 0 { + t.Errorf("an export that is not the module's made a reload: %v", f.ran) + } + if h := m.Health(); !h.Healthy { + t.Errorf("an export that is not the module's made it unhealthy: %+v", h) + } +} + +// The review of #142: SIGTERM in the middle of a pass on a plain restart (the config is still there) ends +// the pass, and the exports stay: the export file is unchanged and the kernel's table keeps the share. +func TestSIGTERMDuringAPassOnARestartLeavesTheExports(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/library") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + m.Pass(context.Background(), nil) + file, table := f.read(ExportFile), f.read(Etab) + if !strings.Contains(table, "/srv/library") { + t.Fatalf("the library was not exported: %q", table) + } + + inner := m.Run + hung := make(chan struct{}, 8) + m.Run = func(ctx context.Context, name string, args ...string) Ran { + if strings.Contains(strings.Join(append([]string{name}, args...), " "), "exportfs") { + hung <- struct{}{} + <-ctx.Done() + return Ran{Code: -1, Err: ctx.Err().Error()} + } + return inner(ctx, name, args...) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- m.RunExports(ctx) }() // its first pass reloads, and hangs there + select { + case <-hung: + case <-time.After(5 * time.Second): + t.Fatal("the pass never reached exportfs") + } + cancel() + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("the process did not stop after SIGTERM") + } + if got := f.read(ExportFile); got != file { + t.Errorf("a restart changed the export file:\nwas %q\nnow %q", file, got) + } + if got := f.read(Etab); got != table { + t.Errorf("a restart changed what the kernel exports:\nwas %q\nnow %q", table, got) + } +} + +// mesh-host #62's caveat: when the process's first apply failed, it can be recorded after its config, and +// unassignment then stops it with the config still there. The node-engine disables a process's unit before +// it stops it (`systemctl disable --now`), and a restart or a shutdown leaves it enabled, so a process +// whose own unit is disabled when it is stopped is being taken away, whatever the config says. +func TestAProcessWhoseUnitIsDisabledAtStopCleansUpWithTheConfigStillThere(t *testing.T) { + f := newFake(t) + f.mountinfo("/", "/srv/library") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + f.kernelReads() + m := f.machine() + m.Pass(context.Background(), nil) + + f.answers["systemctl is-enabled "+ProcessUnit] = Ran{Out: "enabled\n"} + m.Stopped() + if !strings.Contains(f.read(ExportFile), "/srv/library") { + t.Fatalf("a stop with the unit enabled (a restart) removed the exports") + } + + f.answers["systemctl is-enabled "+ProcessUnit] = Ran{Code: 1, Out: "disabled\n"} + m.Stopped() + if strings.Contains(f.read(ExportFile), "/srv/library") { + t.Errorf("a stop with the unit disabled (unassigned, config still there) left the export file") + } + if strings.Contains(f.read(Etab), "/srv/library") { + t.Errorf("the kernel still exports the library after an unassignment with the config in place: %q", f.read(Etab)) + } +} diff --git a/modules/nfs-server/cmd/nfs-server/review_test.go b/modules/nfs-server/cmd/nfs-server/review_test.go index c0893d11..86128e7a 100644 --- a/modules/nfs-server/cmd/nfs-server/review_test.go +++ b/modules/nfs-server/cmd/nfs-server/review_test.go @@ -379,3 +379,56 @@ func TestTheLookBelowIsKeptForAWhileAndThenTakenAgain(t *testing.T) { t.Errorf("a repository put there later was not found once the kept look expired: %+v", s.Refused) } } + +// mesh-host #62 found that a stop the service manager ends with SIGKILL after 90 s leaves the export file +// behind. A pass that hangs in a command (sudo, exportfs: 20 s each) must not hold the cleanup: SIGTERM +// cancels the running pass, and Stopped has its own short bound, so the file is gone well inside the stop +// timeout even when every command hangs. +func TestAHangingCommandStillLetsStoppedRemoveTheFileInTime(t *testing.T) { + f := newFake(t) + f.mountinfo("/") + f.dir("/srv/library") + f.file(ConfigFile, grantsConfig("library=/srv/library", "library=ws:rw")) + f.roster("ws 10.0.0.3") + f.wants(map[string][]Want{"ws": {{Share: "library", Access: "rw"}}}) + m := f.machine() + inner := m.Run + hung := make(chan struct{}, 8) + m.Run = func(ctx context.Context, name string, args ...string) Ran { + line := strings.Join(append([]string{name}, args...), " ") + if strings.Contains(line, "exportfs") { + hung <- struct{}{} + <-ctx.Done() // hangs until its context ends, as a stuck sudo or exportfs would + return Ran{Code: -1, Err: ctx.Err().Error()} + } + return inner(ctx, name, args...) + } + saved := StopTimeout + StopTimeout = 300 * time.Millisecond + defer func() { StopTimeout = saved }() + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- m.RunExports(ctx) }() + select { + case <-hung: // the first pass is stuck in exportfs + case <-time.After(5 * time.Second): + t.Fatal("the first pass never reached exportfs") + } + if err := os.Remove(filepath.Join(f.root, ConfigFile)); err != nil { // unassigned: config removed first + t.Fatal(err) + } + start := time.Now() + cancel() // SIGTERM + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("the process did not stop within 3 s of SIGTERM while a command hung") + } + if took := time.Since(start); took > 2*time.Second { + t.Errorf("stopping took %s with a hanging command; the bound is StopTimeout (%s)", took, StopTimeout) + } + if _, err := os.Stat(filepath.Join(f.root, ExportFile)); err == nil { + t.Errorf("the export file stays after an unassignment whose commands hung") + } +} diff --git a/modules/nfs-server/cmd/nfs-server/shares.go b/modules/nfs-server/cmd/nfs-server/shares.go index eb656442..073ee0cb 100644 --- a/modules/nfs-server/cmd/nfs-server/shares.go +++ b/modules/nfs-server/cmd/nfs-server/shares.go @@ -281,9 +281,13 @@ func (m *Machine) walkBelow(dir string) (found string, complete bool) { } seen := 0 level := []string{dir} + walk := m.walking() for depth := 1; depth <= WalkDepth && len(level) > 0; depth++ { var next []string for _, d := range level { + if walk != nil && walk.Err() != nil { + return "", false // ended by the process stopping: unfinished, so refused + } for _, child := range m.Children(d) { seen++ if seen > limit { @@ -338,6 +342,8 @@ func (m *Machine) Judge(c Config, roster map[string]string, wants map[string][]W prefix, _ = netip.ParsePrefix(c.Range) } points := m.MountPoints() + backup, _ := m.Backup() // judged on every pass, never kept: see backup.go + backup.Paths = m.realPaths(backup.Paths) shares := map[string]bool{} for _, s := range c.Shares { shares[s.Name] = true @@ -383,6 +389,10 @@ func (m *Machine) Judge(c Config, roster map[string]string, wants map[string][]W no(fmt.Sprintf("The share %s's folder %s%s is owned by root (%d:%d), and every client would act as root there: give it an owner of its own.", s.Name, real, via, o.UID, o.GID)) continue } + if p, how := nearBackup(real, backup); p != "" { + no(fmt.Sprintf("The share %s's folder %s%s %s %s, where this machine's backup seat (%s) keeps its data, and backups are never exported.", s.Name, real, via, how, p, backup.Holder)) + continue + } if m.IsBackupRepository(real) { no(fmt.Sprintf("The share %s's folder %s%s is a backup repository, and backups are never exported.", s.Name, real, via)) continue diff --git a/modules/nfs-server/cmd/nfs-server/tools.go b/modules/nfs-server/cmd/nfs-server/tools.go index 114fce40..596bc2f8 100644 --- a/modules/nfs-server/cmd/nfs-server/tools.go +++ b/modules/nfs-server/cmd/nfs-server/tools.go @@ -126,6 +126,7 @@ func (m *Machine) Exports() ExportsAnswer { a.Shares = append(a.Shares, e) } a.Notes = append(a.Notes, st.Problems...) + a.Notes = append(a.Notes, st.Warnings...) a.Unmatched = st.Said } entries, sets, notes := m.Foreign() @@ -437,8 +438,12 @@ func (m *Machine) Health() Health { } } sort.Strings(found) + warned := "" + for _, w := range st.Warnings { + warned += " Warning: " + sentence(w) + } if len(found) > 0 { - return Health{Healthy: false, Why: strings.Join(found, " "), Found: found} + return Health{Healthy: false, Why: strings.Join(found, " ") + warned, Found: found} } var parts []string for _, e := range st.Shares { @@ -447,9 +452,9 @@ func (m *Machine) Health() Health { } } if len(parts) == 0 { - return Health{Healthy: true, Why: "The NFS server is active, and the mesh exports nothing to any node here."} + return Health{Healthy: true, Why: "The NFS server is active, and the mesh exports nothing to any node here." + warned} } - return Health{Healthy: true, Why: fmt.Sprintf("The NFS server is active and exports %s.", strings.Join(parts, "; "))} + return Health{Healthy: true, Why: fmt.Sprintf("The NFS server is active and exports %s.", strings.Join(parts, "; ")) + warned} } func sentence(s string) string { diff --git a/modules/nfs-server/module.json b/modules/nfs-server/module.json index f36e133c..d67cddde 100644 --- a/modules/nfs-server/module.json +++ b/modules/nfs-server/module.json @@ -47,6 +47,10 @@ "reads": [ "mounts.shares" ], + "invokes": [ + "seat:mesh-controller.seats", + "seat:mesh-controller.data" + ], "settings": { "grants": { "kind": "preference",