Leave no leftover for a folder kept only while empty, and name filesystems by more than their mount point (hq ADR 0325 third review)

A folder kept because it was empty, then covered by its disk, was said as unhealthy
leftover entries for ever: its entries sit on a hidden empty folder and grant nothing,
so they are now forgotten, and only entries walked onto data under a warrant are kept
to be taken out. A warrant and a kept folder now name their filesystem by mount point,
source and type, so another pool at the same point is a move; records naming a mount
point are read as its name. A record with no access ids is said under a path-free name.

internal/apply/keep.go

@@ -244,6 +244,9 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken
 	if pl.Away {
 		return folderVerdict{kind: folderWaitDisk, mount: pl.Mount}
 	}
+	// Another filesystem at the same mount point (another pool, another disk) is a move too.
+	here := PlacedID(pl.Mount)
+	moved := pl.Moved || (rec.Placed != "" && rec.Mount == pl.Mount && rec.Placed != here)
 	facts, err := readFolder(path)
 	if err != nil {
 		return folderVerdict{err: errors.New("the folder cannot be read: " + pathless(err))}
@@ -253,30 +256,41 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken
 	}
 	// The controller's word that the folder was taken holds for the filesystem the warrant was carried out on: a
 	// folder moved to another is not taken, and asks again (§8).
-	takenHere := taken && (rec.TakenOn == pl.Mount || (rec.TakenOn == "" && !pl.Moved))
+	takenHere := taken && (takenOnID(rec.TakenOn) == here || (rec.TakenOn == "" && !moved))
 	switch {
-	case rec.Kept && !pl.Moved:
+	case rec.Kept && !moved:
 		return folderVerdict{kind: folderKeep, mount: pl.Mount, how: rec.How}
 	case takenHere:
-		return folderVerdict{kind: folderKeep, mount: pl.Mount, how: store.KeptByWarrant, moved: pl.Moved}
+		return folderVerdict{kind: folderKeep, mount: pl.Mount, how: store.KeptByWarrant, moved: moved}
 	case facts.Empty:
-		return folderVerdict{kind: folderKeep, mount: pl.Mount, how: store.KeptEmpty, moved: pl.Moved}
+		return folderVerdict{kind: folderKeep, mount: pl.Mount, how: store.KeptEmpty, moved: moved}
 	}
 	// What is there now holds data the mesh never took — a disk mounted over a folder kept while it was empty
 	// below it included (a move): it waits for the operator's word.
-	return folderVerdict{kind: folderWaitKeep, mount: pl.Mount, moved: pl.Moved}
+	return folderVerdict{kind: folderWaitKeep, mount: pl.Mount, moved: moved}
 }
 
-// movedRecord is f judged afresh on the filesystem now at mount: what the mesh added on the one it kept is left
-// to be taken out once that filesystem holds the folder again (Left), the warrant's filesystem is kept so the
-// controller's word is not read for the new one, and nothing else of what was kept there holds.
+// movedRecord is f judged afresh on the filesystem now at mount: the warrant's filesystem is kept so the
+// controller's word is not read for the new one, and nothing else of what was kept there holds. What the mesh
+// added on the old filesystem is left to be taken out once it holds the folder again (Left) only where it was
+// walked onto data under a warrant: a folder kept because it was empty, or made, carries entries on that empty
+// folder alone, hidden under the mount point and granting nothing on data, and they are forgotten (said once, by
+// the apply that saw the move).
 func movedRecord(f store.KeptFolder, mount string) store.KeptFolder {
 	left := f.Left
-	if len(f.Users) > 0 && f.Mount != "" {
+	if f.How == store.KeptByWarrant && len(f.Users) > 0 && f.Mount != "" {
 		left = addLeft(left, f.Mount, f.Users)
 	}
-	return store.KeptFolder{Path: f.Path, Mount: mount, Want: f.Want, IDs: f.IDs, Act: f.Act, TakenOn: f.TakenOn,
-		Left: left}
+	return store.KeptFolder{Path: f.Path, Mount: mount, Placed: PlacedID(mount), Want: f.Want, IDs: f.IDs,
+		Act: f.Act, TakenOn: f.TakenOn, Left: left}
+}
+
+// takenOnID is a record's TakenOn as a filesystem's name: one written before names held the mount point itself.
+func takenOnID(takenOn string) string {
+	if strings.HasPrefix(takenOn, "/") {
+		return PlacedID(takenOn)
+	}
+	return takenOn
 }
 
 // addLeft adds users' entries left on mount, once per mount and uid.
@@ -435,6 +449,10 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O
 	note := ""
 	if v.moved {
 		note = "; what is at it now is on another filesystem than what the mesh kept there, so it was judged afresh"
+		if rec.How != store.KeptByWarrant {
+			note += "; the entries on the empty folder kept below are forgotten: hidden under the mount, they grant " +
+				"nothing on data"
+		}
 	}
 	if len(r.Grant) > 0 {
 		note += "; its grant is no longer acted on: the mesh puts no account in a group for data (novox/hq ADR 0325 §5)"
@@ -469,7 +487,7 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O
 			return out, nil
 		}
 		_ = KeptFolders.Update(path, func(f *store.KeptFolder, _ bool) bool {
-			f.Mount, f.Want, f.IDs = v.mount, users, want.ids
+			f.Mount, f.Placed, f.Want, f.IDs = v.mount, PlacedID(v.mount), users, want.ids
 			return true
 		})
 		out.Action = "waiting"
@@ -497,7 +515,7 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O
 	changed, err := keepTop(path, usersOf(users))
 	if errors.Is(err, ErrNoACL) {
 		_ = KeptFolders.Update(path, func(f *store.KeptFolder, _ bool) bool {
-			f.Mount, f.Want = v.mount, users
+			f.Mount, f.Placed, f.Want = v.mount, PlacedID(v.mount), users
 			return true
 		})
 		out.Action = "waiting"
@@ -511,11 +529,12 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O
 	var pending string
 	_ = KeptFolders.Update(path, func(f *store.KeptFolder, had bool) bool {
 		first := !f.Kept
-		f.Kept, f.Mount, f.Want, f.IDs = true, v.mount, users, want.ids
+		f.Kept, f.Mount, f.Placed, f.Want, f.IDs = true, v.mount, PlacedID(v.mount), users, want.ids
 		if v.how == store.KeptByWarrant && f.TakenOn == "" {
 			// The controller says it was taken, and this engine has no record of where: here, now.
-			f.TakenOn = v.mount
+			f.TakenOn = PlacedID(v.mount)
 		}
+		f.TakenOn = takenOnID(f.TakenOn)
 		if first {
 			f.How, f.At = v.how, time.Now().UTC()
 			// A folder that held nothing has nothing under it to walk; one the controller says was taken
@@ -758,18 +777,15 @@ func Leftovers() []Leftover {
 	}
 	var out []Leftover
 	for path, f := range all {
-		if len(f.IDs) == 0 {
-			continue
-		}
-		module, name := accessName(f.IDs[0])
+		module, name, access := recordName(path, f)
 		dropping := f.Kept && len(f.Want) == 0 && len(f.Users) > 0 && f.Act != nil && f.Act.State == "waiting"
 		switch {
 		case dropping:
-			out = append(out, Leftover{Module: module, Access: f.IDs[0], Name: name, Path: path, Why: "the mesh's entries for " +
+			out = append(out, Leftover{Module: module, Access: access, Name: name, Path: path, Why: "the mesh's entries for " +
 				name + " could not be removed: its disk is not there; they are removed once it is (novox/hq ADR " +
 				"0325 §8)"})
 		case len(f.Left) > 0:
-			out = append(out, Leftover{Module: module, Access: f.IDs[0], Name: name, Path: path, Why: "the mesh's entries for " +
+			out = append(out, Leftover{Module: module, Access: access, Name: name, Path: path, Why: "the mesh's entries for " +
 				name + " on a filesystem no longer at its folder could not be removed: they are removed once that " +
 				"filesystem is back (novox/hq ADR 0325 §8)"})
 		}
@@ -777,3 +793,21 @@ func Leftovers() []Leftover {
 	sort.Slice(out, func(i, j int) bool { return out[i].Access < out[j].Access })
 	return out
 }
+
+// recordName is the module, the name and the access a kept folder's record is said under: the accesses that last
+// named it, or for a record written before it kept them, a name with no path — the module of a container identity
+// among its users, and a folder named by a digest of its path.
+func recordName(path string, f store.KeptFolder) (module, name, access string) {
+	if len(f.IDs) > 0 {
+		module, name = accessName(f.IDs[0])
+		return module, name, f.IDs[0]
+	}
+	sum := sha256.Sum256([]byte(path))
+	name = "folder-" + hex.EncodeToString(sum[:4])
+	for _, u := range append(append([]store.KeptUser(nil), f.Users...), f.Want...) {
+		if m, ok := strings.CutPrefix(u.Identity, declaration.IdentityPrefix); ok && u.From == declaration.FromContainer {
+			return m, name, m + "." + name
+		}
+	}
+	return "", name, name
+}

internal/apply/keeper_linux.go

@@ -272,8 +272,8 @@ func TakeFolder(ctx context.Context, d *declaration.Declaration, id, path, warra
 			f.Kept, f.How, f.At, f.Users = true, store.KeptByWarrant, started, nil
 		}
 		f.Warrant, f.By, f.Mount, f.Want, f.IDs = warrant, by, v.mount, users, want.ids
-		// The warrant is for the filesystem it was carried out on (§8).
-		f.TakenOn = v.mount
+		// The warrant is for the filesystem it was carried out on (§8), named as a wait names it.
+		f.Placed, f.TakenOn = PlacedID(v.mount), PlacedID(v.mount)
 		f.Act = &store.KeptAct{Act: "take", Warrant: warrant, By: by, State: "running", Started: started}
 		return true
 	})

internal/apply/kept_test.go

@@ -937,9 +937,51 @@ func TestEntriesThatCouldNotBeDroppedAreSaidUntilTheyAre(t *testing.T) {
 	}
 }
 
-// Review L3: a disk mounted over a folder kept below it leaves the mesh's entries there recorded and said; once that
-// filesystem holds the folder again, they are taken out.
+// Review L3: a disk mounted over a folder taken below it on a warrant leaves the mesh's entries on that data
+// recorded and said; once that filesystem holds the folder again, they are taken out.
 func TestEntriesLeftOnAFilesystemNoLongerAtTheFolderAreDroppedWhenItIsBack(t *testing.T) {
+	dir := aclFolder(t)
+	keeping(t, nil, nil)
+	pool := filepath.Join(dir, "pool")
+	lib := filepath.Join(pool, "media")
+	if err := os.MkdirAll(filepath.Join(lib, "old"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	a := accessJSON("radarr.media", lib, "read-write", false, "mesh-radarr:61001:read-write:container")
+	if _, _, err := applyAccesses(t, store.State{}, a); err != nil {
+		t.Fatal(err)
+	}
+	done, _, err := TakeFolder(context.Background(), declare(t, a), "radarr.media", lib, "w-1", "op", noServices)
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-done
+	film := filepath.Join(lib, "film")
+	if err := os.WriteFile(film, []byte("data"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_ = unix.Removexattr(film, aclAccessXattr)
+	MountPoints = func() []string { return []string{"/", pool} }
+	if _, _, err := applyAccesses(t, store.State{}, a); err != nil {
+		t.Fatal(err)
+	}
+	if left := Leftovers(); len(left) != 1 || !strings.Contains(left[0].Why, "no longer at its folder") {
+		t.Fatalf("entries left on the data taken below are not said: %+v", left)
+	}
+	// The pool goes: the filesystem below holds the folder again, and the entries left there are taken out.
+	MountPoints = func() []string { return []string{"/"} }
+	ReconcileKept(context.Background())
+	if _, still := func() (uint16, bool) { a, _, _ := readACLs(t, filepath.Join(lib, "old")); return a.perm(aclUser, radarr) }(); still {
+		t.Fatal("the entries left on the filesystem that came back were not taken out")
+	}
+	if left := Leftovers(); len(left) != 0 {
+		t.Fatalf("still said after they were taken out: %+v", left)
+	}
+}
+
+// Review (third) M: a folder kept only because it was empty leaves nothing said when a disk mounts over it — its
+// entries sit on an empty folder hidden under the mount and grant nothing on data.
+func TestAFolderKeptEmptyLeavesNoLeftoverWhenADiskMountsOverIt(t *testing.T) {
 	dir := aclFolder(t)
 	keeping(t, nil, nil)
 	pool := filepath.Join(dir, "pool")
@@ -951,26 +993,79 @@ func TestEntriesLeftOnAFilesystemNoLongerAtTheFolderAreDroppedWhenItIsBack(t *te
 	if _, _, err := applyAccesses(t, store.State{}, a); err != nil {
 		t.Fatal(err)
 	}
-	film := filepath.Join(lib, "film")
-	if err := os.WriteFile(film, []byte("data"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	_ = unix.Removexattr(film, aclAccessXattr)
 	MountPoints = func() []string { return []string{"/", pool} }
-	if _, _, err := applyAccesses(t, store.State{}, a); err != nil {
+	report, _, err := applyAccesses(t, store.State{}, a)
+	if err != nil {
 		t.Fatal(err)
 	}
-	if left := Leftovers(); len(left) != 1 || !strings.Contains(left[0].Why, "no longer at its folder") {
-		t.Fatalf("entries left on the filesystem below are not said: %+v", left)
-	}
-	// The pool goes: the filesystem below holds the folder again, and the entries left there are taken out.
-	MountPoints = func() []string { return []string{"/"} }
-	ReconcileKept(context.Background())
-	if _, still := func() (uint16, bool) { a, _, _ := readACLs(t, lib); return a.perm(aclUser, radarr) }(); still {
-		t.Fatal("the entries left on the filesystem that came back were not taken out")
-	}
 	if left := Leftovers(); len(left) != 0 {
-		t.Fatalf("still said after they were taken out: %+v", left)
+		t.Fatalf("a folder kept while empty is said as leftover entries for ever: %+v", left)
+	}
+	if rec, _ := keptRecord(lib); len(rec.Left) != 0 {
+		t.Errorf("entries on the hidden empty folder are recorded: %+v", rec.Left)
+	}
+	if o := outcomeOf(report, "radarr.media"); !strings.Contains(o.Detail, "forgotten") {
+		t.Errorf("the forgetting is not said: %+v", o)
+	}
+}
+
+// Review (third) low: a record written before it kept the accesses' ids is said too, under a name with no path.
+func TestALeftoverWithNoAccessIdsIsSaidUnderAPathFreeName(t *testing.T) {
+	keeping(t, nil, nil)
+	path := "/srv/media/old"
+	_ = KeptFolders.Update(path, func(f *store.KeptFolder, _ bool) bool {
+		f.Kept, f.Users = true, []store.KeptUser{{Identity: "mesh-radarr", UID: radarr, Use: "read-write", From: "container"}}
+		f.Act = &store.KeptAct{State: "waiting"}
+		return true
+	})
+	left := Leftovers()
+	if len(left) != 1 || left[0].Module != "radarr" || strings.Contains(left[0].Why, path) ||
+		strings.Contains(left[0].Access, "/") {
+		t.Fatalf("a record without ids is not said, path-free: %+v", left)
+	}
+}
+
+// Review (third) low: another filesystem mounted at the same point is not the one the warrant was for.
+func TestAnotherFilesystemAtTheSameMountPointIsNotTaken(t *testing.T) {
+	dir := aclFolder(t)
+	pool := filepath.Join(dir, "pool")
+	lib := filepath.Join(pool, "media")
+	if err := os.MkdirAll(filepath.Join(lib, "old"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	keeping(t, []string{pool}, nil)
+	source := "tank/a"
+	was := MountSource
+	MountSource = func(point string) (string, string) {
+		if point == pool {
+			return source, "zfs"
+		}
+		return "", ""
+	}
+	defer func() { MountSource = was }()
+	notTaken := accessJSON("radarr.media", lib, "read-write", false, "mesh-radarr:61001:read-write:container")
+	if _, _, err := applyAccesses(t, store.State{}, notTaken); err != nil {
+		t.Fatal(err)
+	}
+	done, _, err := TakeFolder(context.Background(), declare(t, notTaken), "radarr.media", lib, "w-1", "op", noServices)
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-done
+	taken := accessJSON("radarr.media", lib, "read-write", true, "mesh-radarr:61001:read-write:container")
+	if report, _, _ := applyAccesses(t, store.State{}, taken); outcomeOf(report, "radarr.media").Action == "waiting" {
+		t.Fatal("the folder taken waits on its own filesystem")
+	}
+	source = "tank/b" // another pool imported at the same mount point
+	report, _, err := applyAccesses(t, store.State{}, taken)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "radarr.media"); o.Action != "waiting" {
+		t.Fatalf("another filesystem at the same mount point was kept on the old warrant: %+v", o)
+	}
+	if v := AccessVerdict(context.Background(), lib, true, true); v.Kind != VerdictWaitKeep {
+		t.Fatalf("the look judges it %s", v.Kind)
 	}
 }
 

internal/apply/placed.go

@@ -147,7 +147,11 @@ func mountinfoPoints(raw string) []string {
 
 // mountSourceOf is the source and filesystem type of the mount at point, from mountinfo: for a ZFS dataset, its
 // name. Empty when the machine says nothing of it.
-func mountSourceOf(point string) (source, fstype string) {
+func mountSourceOf(point string) (source, fstype string) { return MountSource(point) }
+
+// MountSource is mountSourceOf as the machine answers it. A variable so a test can stand in for another filesystem
+// mounted at the same point.
+var MountSource = func(point string) (source, fstype string) {
 	raw, err := os.ReadFile("/proc/self/mountinfo")
 	if err != nil {
 		return "", ""

internal/store/kept.go

@@ -61,8 +61,12 @@ type KeptFolder struct {
 	Want  []KeptUser `json:"want,omitempty"`
 	// IDs are the accesses that last named the folder: what a leftover is said under once none does.
 	IDs []string `json:"ids,omitempty"`
-	// TakenOn is the mount point the operator's warrant was carried out on (ADR 0325 §8): the controller's word
-	// that the folder was taken holds for that filesystem alone, and a folder moved to another asks again.
+	// Placed names the filesystem the folder was kept on (the engine's PlacedID: its mount point, source and type),
+	// so another filesystem mounted at the same point is a move, as another mount point is.
+	Placed string `json:"placed,omitempty"`
+	// TakenOn names, as Placed does, the filesystem the operator's warrant was carried out on (ADR 0325 §8): the
+	// controller's word that the folder was taken holds for that filesystem alone, and a folder moved to another
+	// asks again. A record written before held the mount point here, and is read as that mount's name.
 	TakenOn string `json:"taken_on,omitempty"`
 	// Left is the entries the mesh added on a filesystem no longer at the folder (a disk mounted over it, or
 	// one gone): taken out by a drop walk once that filesystem holds the folder again, said until then.