Reload, not restart, the huge pages unit when the setting changes (hq issue 746)

A restart ran restore then apply, so every change passed back through the
found mode and rotated the journal again. Reload applies the new value
directly; restore also drops its state; no rotation while the machine boots;
the unit is sandboxed.

modules/kernel/cmd/kernel-tools/manifest_test.go

@@ -2,8 +2,9 @@ package main
 
 // The module's shape (novox/hq issue 746): the setting's default is what the kernel boots with, so
 // assigning the module changes nothing until a machine sets another value; the script is written into the
-// module's own state and run by a oneshot unit that is restarted whenever the script (and so the setting)
-// or the unit changes; and the script itself sets, verifies, gives back and refuses as it says.
+// module's own state and run by a sandboxed oneshot unit that is reloaded (applied again) when the script,
+// and so the setting, changes and restarted only when the unit does; and the script itself sets, verifies,
+// gives back and refuses as it says.
 
 import (
 	"os"
@@ -34,7 +35,9 @@ func TestTheDefaultChangesNothingAndTheUnitFollowsTheSetting(t *testing.T) {
 	}
 	unit := m.resource(t, "unit")["content"].(string)
 	for _, line := range []string{"Type=oneshot", "RemainAfterExit=yes", "ExecStart=${dir:state}/huge-pages apply",
-		"ExecStop=${dir:state}/huge-pages restore", "WantedBy=sysinit.target", "DefaultDependencies=no"} {
+		"ExecReload=${dir:state}/huge-pages apply", "ExecStop=${dir:state}/huge-pages restore", "WantedBy=sysinit.target",
+		"DefaultDependencies=no", "ProtectSystem=strict", "ReadWritePaths=${dir:state}", "ProtectHome=yes", "PrivateNetwork=yes",
+		"NoNewPrivileges=yes"} {
 		if !strings.Contains(unit, line+"\n") {
 			t.Errorf("the unit lacks %q", line)
 		}
@@ -43,9 +46,14 @@ func TestTheDefaultChangesNothingAndTheUnitFollowsTheSetting(t *testing.T) {
 	if apply["unit"] != "mesh-kernel-huge-pages.service" || apply["state"] != "running" || apply["boot"] != "enabled" {
 		t.Fatalf("apply: %v", apply)
 	}
-	if on := apply["restart-on"].([]any); len(on) != 2 || on[0] != "huge-pages" || on[1] != "unit" {
+	// A new value reloads (apply again); only a new unit restarts, whose stop would pass back through the
+	// mode the machine was found with.
+	if on := apply["restart-on"].([]any); len(on) != 1 || on[0] != "unit" {
 		t.Fatalf("restart-on: %v", on)
 	}
+	if on := apply["reload-on"].([]any); len(on) != 1 || on[0] != "huge-pages" {
+		t.Fatalf("reload-on: %v", on)
+	}
 	if h := apply["health"].(map[string]any); h["kind"] != "unit" {
 		t.Fatalf("health: %v", h)
 	}
@@ -54,12 +62,17 @@ func TestTheDefaultChangesNothingAndTheUnitFollowsTheSetting(t *testing.T) {
 // bed is the script as the mesh writes it for one value of the setting, a stand-in transparent_hugepage
 // directory, the module's state, and a PATH whose journalctl records that it was asked to rotate.
 type bed struct {
-	script, sys, state, rotated, path string
+	script, sys, state, rotated, path, bin string
 }
 
 func newBed(t *testing.T, want, found string) bed {
+	return newBedIn(t, t.TempDir(), want, found)
+}
+
+// newBedIn writes the script for want into dir; a second bed in the same dir shares the machine and the
+// state, as a reload with a new value does.
+func newBedIn(t *testing.T, dir, want, found string) bed {
 	t.Helper()
-	dir := t.TempDir()
 	b := bed{script: filepath.Join(dir, "huge-pages"), sys: filepath.Join(dir, "thp"), state: filepath.Join(dir, "state"),
 		rotated: filepath.Join(dir, "rotated")}
 	content := manifest(t).resource(t, "huge-pages")["content"].(string)
@@ -75,12 +88,22 @@ func newBed(t *testing.T, want, found string) bed {
 		}
 	}
 	write(t, b.script, content, 0o755)
-	write(t, filepath.Join(b.sys, "enabled"), found, 0o644)
+	if found != "" {
+		write(t, filepath.Join(b.sys, "enabled"), found, 0o644)
+	}
 	write(t, filepath.Join(bin, "journalctl"), "#!/bin/sh\necho \"$@\" >>"+b.rotated+"\n", 0o755)
+	b.bin = bin
+	b.booted(t, "running")
 	b.path = bin + string(os.PathListSeparator) + os.Getenv("PATH")
 	return b
 }
 
+// booted makes the service manager say state for is-system-running.
+func (b bed) booted(t *testing.T, state string) {
+	t.Helper()
+	write(t, filepath.Join(b.bin, "systemctl"), "#!/bin/sh\n[ \"$1\" = is-system-running ] && echo "+state+"\n", 0o755)
+}
+
 func write(t *testing.T, p, content string, mode os.FileMode) {
 	t.Helper()
 	if err := os.WriteFile(p, []byte(content), mode); err != nil {
@@ -134,6 +157,44 @@ func TestApplySetsTheModeRotatesTheJournalOnceAndRestoreGivesBackWhatWasFound(t
 	if got := b.file(t, filepath.Join(b.sys, "enabled")); got != "always\n" {
 		t.Fatalf("restored: %q", got)
 	}
+	// Nothing of the module is kept once it is stopped.
+	if _, err := os.Stat(filepath.Join(b.state, "found")); !os.IsNotExist(err) {
+		t.Fatalf("found kept after restore: %v", err)
+	}
+}
+
+func TestAReloadWithANewValueGoesStraightThereAndNeverRewritesFound(t *testing.T) {
+	dir := t.TempDir()
+	b := newBedIn(t, dir, "madvise", "[always] madvise never\n")
+	if out, err := b.run(t, "apply"); err != nil {
+		t.Fatalf("apply: %v %s", err, out)
+	}
+	// The setting changes to never: the mesh rewrites the script and reloads the unit, which applies again.
+	n := newBedIn(t, dir, "never", "")
+	out, err := n.run(t, "apply")
+	if err != nil || !strings.Contains(out, "madvise -> never") {
+		t.Fatalf("reload: %v %s", err, out)
+	}
+	if got := n.file(t, filepath.Join(n.state, "found")); got != "always\n" {
+		t.Fatalf("found rewritten by a reload: %q", got)
+	}
+}
+
+func TestNoRotationWhileTheMachineIsStarting(t *testing.T) {
+	for _, state := range []string{"initializing", "starting"} {
+		b := newBed(t, "madvise", "[always] madvise never\n")
+		b.booted(t, state)
+		out, err := b.run(t, "apply")
+		if err != nil || !strings.Contains(out, "still starting") {
+			t.Fatalf("%s: %v %s", state, err, out)
+		}
+		if got := b.file(t, filepath.Join(b.sys, "enabled")); got != "madvise\n" {
+			t.Fatalf("%s: not set: %q", state, got)
+		}
+		if got := b.file(t, b.rotated); got != "" {
+			t.Fatalf("%s: rotated at boot: %q", state, got)
+		}
+	}
 }
 
 func TestAlwaysIsSetWithoutRotating(t *testing.T) {
@@ -172,9 +233,6 @@ func TestAWriteThatDoesNotTakeFailsTheUnit(t *testing.T) {
 
 func TestAKernelWithoutHugePagesIsSaid(t *testing.T) {
 	b := newBed(t, "madvise", "")
-	if err := os.Remove(filepath.Join(b.sys, "enabled")); err != nil {
-		t.Fatal(err)
-	}
 	if out, err := b.run(t, "apply"); err == nil || !strings.Contains(out, "built without transparent huge pages") {
 		t.Fatalf("%v %s", err, out)
 	}