Merge pull request 'nfs-server: the look below a share runs beside the pass, so a long walk no longer fails its health (issue 1971)' (#616) from fix/1971-nfs-walk-beside-look into main

modules/nfs-server/cmd/nfs-server/machine.go

@@ -105,9 +105,11 @@ type Machine struct {
 	// ownerOf reads a file's owner from its inode; nil is the inode as the kernel says it. Tests set it
 	// so the folders they make stand for a non-root owner, whoever runs them.
 	ownerOf func(info os.FileInfo) (uid, gid int)
-	// clean is when a look below each real path last found no backup repository (LookKept).
+	// Beside runs a look below a share beside the pass; nil is a goroutine. Tests run it in place.
-	clean   map[string]time.Time
+	Beside func(func())
-	cleanMu sync.Mutex
+	// looks is the last look below each real path (LookKept).
+	looks   map[string]*lookBelow
+	looksMu 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

modules/nfs-server/cmd/nfs-server/nfs_test.go

@@ -32,7 +32,7 @@ func newFake(t *testing.T) *fake {
 }
 
 func (f *fake) machine() *Machine {
-	return &Machine{Root: f.root, UID: f.uid, owners: f.owners, ownerOf: asTheLibrarysOwner, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran {
+	return &Machine{Root: f.root, UID: f.uid, owners: f.owners, ownerOf: asTheLibrarysOwner, Beside: func(f func()) { f() }, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran {
 		line := strings.Join(append([]string{name}, args...), " ")
 		f.ran = append(f.ran, line)
 		if (line == "exportfs -ra" || line == "sudo -n exportfs -ra") && f.onExportfs != nil {

modules/nfs-server/cmd/nfs-server/process.go

@@ -61,9 +61,8 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status {
 	if _, note := m.Backup(); note != "" {
 		s.Warnings = append(s.Warnings, note)
 	}
-	m.setWalk(ctx)
+	m.setWalk(ctx) // stays set: a look beside the pass ends when the process does
 	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")

modules/nfs-server/cmd/nfs-server/review_test.go

@@ -432,3 +432,89 @@ func TestAHangingCommandStillLetsStoppedRemoveTheFileInTime(t *testing.T) {
 		t.Errorf("the export file stays after an unassignment whose commands hung")
 	}
 }
+
+// Issue 1971: the look below a share runs beside the pass. A pass whose walk has not finished still writes
+// its status, refuses the share until a look has finished, keeps believing a clean look while it is renewed,
+// and a walk ended by the process stopping leaves no result.
+func TestTheLookBelowRunsBesideThePassAndNeverHoldsItsStatus(t *testing.T) {
+	f := newFake(t)
+	f.mountinfo("/")
+	f.dir("/srv/clean/films/one")
+	f.file(ConfigFile, grantsConfig("clean=/srv/clean", "clean=ws:rw"))
+	f.roster("ws 10.0.0.3")
+	f.wants(map[string][]Want{"ws": {{Share: "clean", Access: "rw"}}})
+	m := f.machine()
+	var walks []func()
+	m.Beside = func(w func()) { walks = append(walks, w) } // the walk is still running
+	now := time.Date(2026, 11, 5, 5, 0, 0, 0, time.UTC)
+	m.Now = func() time.Time { return now }
+
+	s := m.Pass(context.Background(), nil)
+	if !s.At.Equal(now) || f.read(StatusFile) == "" || len(walks) != 1 {
+		t.Fatalf("the pass waited for the walk: at %v, walks %d", s.At, len(walks))
+	}
+	if !strings.Contains(refusedWhy(s, "clean"), "not been looked through yet") {
+		t.Errorf("a share whose look is unfinished is exported: %+v", s.Refused)
+	}
+	m.Pass(context.Background(), &s)
+	if len(walks) != 1 {
+		t.Errorf("a second walk started while one runs: %d", len(walks))
+	}
+	walks[0]()
+	if s = m.Pass(context.Background(), &s); refusedWhy(s, "clean") != "" {
+		t.Fatalf("the finished look was not used: %+v", s.Refused)
+	}
+
+	now = now.Add(LookKept)
+	s = m.Pass(context.Background(), &s)
+	if len(walks) != 2 || refusedWhy(s, "clean") != "" {
+		t.Errorf("the renewal must run beside the pass and the clean look stand: walks %d, %+v", len(walks), s.Refused)
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	m.Pass(ctx, &s)
+	cancel()
+	f.restic("/srv/clean/films/one/backup")
+	walks[1]()
+	if s = m.Pass(context.Background(), &s); refusedWhy(s, "clean") != "" {
+		t.Errorf("a walk ended by the stop left a result: %+v", s.Refused)
+	}
+	walks[len(walks)-1]()
+	if s = m.Pass(context.Background(), &s); !strings.Contains(refusedWhy(s, "clean"), "/srv/clean/films/one/backup") {
+		t.Errorf("the repository was not found by the next look: %+v", s.Refused)
+	}
+}
+
+// After a restart no look is remembered; a share the previous process's export file carries stays exported,
+// and nothing is reloaded, while its first look runs (a delivery never unexports a mounted share).
+func TestARestartKeepsTheExportedShareWhileItsFirstLookRuns(t *testing.T) {
+	f := newFake(t)
+	f.mountinfo("/", "/srv/clean")
+	f.dir("/srv/clean/films/one")
+	f.file(ConfigFile, grantsConfig("clean=/srv/clean", "clean=ws:rw"))
+	f.roster("ws 10.0.0.3")
+	f.wants(map[string][]Want{"ws": {{Share: "clean", Access: "rw"}}})
+	f.kernelReads()
+	first := f.machine()
+	s := first.Pass(context.Background(), nil)
+	before := f.read(ExportFile)
+	if refusedWhy(s, "clean") != "" || !strings.Contains(before, "/srv/clean ") {
+		t.Fatalf("the share was not exported to begin with: %+v %q", s.Refused, before)
+	}
+	m := f.machine()
+	var held []func()
+	m.Beside = func(w func()) { held = append(held, w) }
+	f.ran = nil
+	s = m.Pass(context.Background(), &s)
+	if len(held) != 1 || refusedWhy(s, "clean") != "" || f.read(ExportFile) != before || countExportfs(f.ran) != 0 {
+		t.Errorf("a restart with an unfinished walk unexported the share: %+v, ran %v", s.Refused, f.ran)
+	}
+	// The same name, a new folder holding a repository, the walk unfinished: refused, not carried by name.
+	f.restic("/srv/other/backup")
+	f.file(ConfigFile, grantsConfig("clean=/srv/other", "clean=ws:rw"))
+	m3 := f.machine()
+	m3.Beside = func(func()) {}
+	s = m3.Pass(context.Background(), &s)
+	if refusedWhy(s, "clean") == "" || strings.Contains(f.read(ExportFile), "/srv/other") {
+		t.Errorf("a share moved to a new folder was exported unlooked: %+v", s.Refused)
+	}
+}

modules/nfs-server/cmd/nfs-server/shares.go

@@ -244,33 +244,72 @@ const (
 )
 
 // LookKept is how long a look below a share that found no backup repository is believed. The look walks up
-// to WalkLimit folders, which on a large library is too much for every 30 s pass; a repository put under a
+// to WalkLimit folders, which on a large library takes minutes on a cold disk; a repository put under a
 // share is found within this long. A look that found one, or was cut short, is never kept: the share stays
 // refused and is looked at again on the next pass.
 const LookKept = time.Hour
 
-// repositoryBelow finds a backup repository within WalkDepth levels below dir. complete is false when
+// lookBelow is the last look below one folder, and whether another runs now.
-// the look stopped at its bound before it saw everything.
+type lookBelow struct {
-func (m *Machine) repositoryBelow(dir string) (found string, complete bool) {
+	found    string
-	now := time.Now()
+	complete bool
-	if m.Now != nil {
+	at       time.Time
-		now = m.Now()
+	done     bool // a look has finished; until then there is no result
-	}
+	running  bool
-	m.cleanMu.Lock()
+}
-	defer m.cleanMu.Unlock()
+
-	if at, kept := m.clean[dir]; kept && now.Sub(at) < LookKept {
+// repositoryBelow answers the last look below dir for a backup repository, and starts the next one beside
-		return "", true
+// the caller when there is none, or it was not clean, or LookKept has passed. The pass therefore never waits
-	}
+// for the walk (issue 1971: a walk of minutes held the status's time back until the module was found
-	found, complete = m.walkBelow(dir)
+// unhealthy). pending is true while no look has finished yet; the share is refused until then, and while a
-	if found == "" && complete {
+// kept clean look is being renewed the old result stands. complete is false when the look stopped at its
-		if m.clean == nil {
+// bound or with the process.
-			m.clean = map[string]time.Time{}
+func (m *Machine) repositoryBelow(dir string) (found string, complete, pending bool) {
+	m.looksMu.Lock()
+	r := m.looks[dir]
+	if r == nil {
+		if m.looks == nil {
+			m.looks = map[string]*lookBelow{}
 		}
-		m.clean[dir] = now
+		r = &lookBelow{}
-	} else {
+		m.looks[dir] = r
-		delete(m.clean, dir)
 	}
-	return found, complete
+	fresh := r.done && r.found == "" && r.complete && m.clock().Sub(r.at) < LookKept
+	start := !fresh && !r.running
+	if start {
+		r.running = true
+	}
+	m.looksMu.Unlock()
+	if start {
+		walk := func() {
+			f, c := m.walkBelow(dir)
+			m.looksMu.Lock()
+			defer m.looksMu.Unlock()
+			r.running = false
+			if w := m.walking(); w != nil && w.Err() != nil {
+				return // ended by the process stopping: no result, the old one stands
+			}
+			r.found, r.complete, r.at, r.done = f, c, m.clock(), true
+		}
+		if m.Beside != nil {
+			m.Beside(walk)
+		} else {
+			go walk()
+		}
+	}
+	m.looksMu.Lock()
+	defer m.looksMu.Unlock()
+	if !r.done {
+		return "", false, true
+	}
+	return r.found, r.complete, false
+}
+
+func (m *Machine) clock() time.Time {
+	if m.Now != nil {
+		return m.Now()
+	}
+	return time.Now()
 }
 
 // walkBelow is the look itself, breadth first, bounded by WalkDepth and WalkLimit.
@@ -342,7 +381,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
+	onDisk, _ := m.Read(ExportFile) // what the previous process judged
+	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 {
@@ -411,12 +451,16 @@ 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 inside the backup repository %s, and backups are never exported.", s.Name, real, via, inside))
 			continue
 		}
-		holds, complete := m.repositoryBelow(real)
+		holds, complete, pending := m.repositoryBelow(real)
 		if holds != "" {
 			no(fmt.Sprintf("The share %s's folder %s%s holds a backup repository (%s), and backups are never exported: share the folders beside it instead.", s.Name, real, via, holds))
 			continue
 		}
-		if !complete {
+		if pending && !carries(onDisk, real) {
+			no(fmt.Sprintf("The share %s's folder %s%s has not been looked through yet for a backup repository (the look runs beside the pass and takes a while on a large folder), so it is not exported yet.", s.Name, real, via))
+			continue
+		}
+		if !pending && !complete {
 			no(fmt.Sprintf("The share %s's folder %s%s holds more folders than the module looks through (%d levels, at most %d folders), so it could not make sure no backup repository is under it: share smaller folders.", s.Name, real, via, WalkDepth, m.limit()))
 			continue
 		}
@@ -487,6 +531,18 @@ func (m *Machine) Judge(c Config, roster map[string]string, wants map[string][]W
 	return out, refused, said
 }
 
+// carries says whether the export file already holds the line for this real folder (a share exported to no
+// node has none and waits for its look; a share whose folder changed is a new folder). While a share's first look is unfinished (after a restart) it stays as the previous process judged
+// it, so a delivery never unexports a share clients have mounted.
+func carries(file, real string) bool {
+	for _, l := range strings.Split(file, "\n") {
+		if strings.HasPrefix(l, real+" ") {
+			return true
+		}
+	}
+	return false
+}
+
 func (m *Machine) limit() int {
 	if m.WalkLimit > 0 {
 		return m.WalkLimit