Judge a filled write with the roots the last apply read, asking no runtime per file (issue 1138)

internal/apply/filled.go

@@ -29,6 +29,11 @@ import (
 //
 // The caller holds the apply's lock: a write and an apply never write one file at once. log is said the path, the
 // action and the digest, never the content.
+//
+// **It does not refresh the runtimes' data roots.** A filled write is one file, not an apply; the placement guard
+// judges it with the roots the last apply read (runtimeDataRoots), falling back to the runtimes' configuration
+// files when no apply has read them yet — the runtimes themselves are asked once, at an apply's start, not once
+// per file (novox/hq issue 1138).
 func WriteFilled(
 	ctx context.Context,
 	sys system.System,
@@ -68,7 +73,6 @@ func WriteFilled(
 
 	in := inputs{declares: map[string]string{}, known: &known, windows: windows, away: newStoreAway(windows, log),
 		groups: groupsWanted(d.Resources), handed: Handed{}}
-	refreshRuntimeRoots(ctx)
 	in.refused = refusedPlaces(d.Resources)
 	in.agentHomes = rootNeverHomes(d)
 	in.dirsBefore = map[string]bool{}

internal/apply/filled_runtime_roots_test.go

@@ -0,0 +1,168 @@
+package apply
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// A filled write is one file, not an apply: it asks no container runtime, and judges where nothing is placed
+// (issue 339) with the roots the last apply read, falling back to the runtimes' configuration files when no apply
+// has read them yet (novox/hq issue 1138). These tests set the package's runtimeRoots and AskRuntimes, so none
+// runs in parallel with another that reads or sets them, and each restores what it changed.
+
+// setRuntimeRoots makes runtimeDataRoots answer roots, as if an apply had just read them (or, with nil, as if none
+// ever had), restoring what a test before it left there once this one is done.
+func setRuntimeRoots(t *testing.T, roots []string) {
+	t.Helper()
+	runtimeRootsMu.Lock()
+	before := runtimeRoots
+	runtimeRoots = roots
+	runtimeRootsMu.Unlock()
+	t.Cleanup(func() {
+		runtimeRootsMu.Lock()
+		runtimeRoots = before
+		runtimeRootsMu.Unlock()
+	})
+}
+
+// askNoRuntime makes AskRuntimes record the name of every runtime it is asked for, answering as one that is not
+// running does (nothing, no error): were it consulted, that is what would say so. Restored once the test is done.
+func askNoRuntime(t *testing.T, asked *[]string) {
+	t.Helper()
+	before := AskRuntimes
+	AskRuntimes = func(_ context.Context, name string, _ ...string) (string, error) {
+		*asked = append(*asked, name)
+		return "", nil
+	}
+	t.Cleanup(func() { AskRuntimes = before })
+}
+
+// aRuntimeConfig points the runtimes' configuration at a fresh, unmounted root below a temp directory, restoring
+// the engine's real configuration once the test is done.
+func aRuntimeConfig(t *testing.T) (dir, root string) {
+	t.Helper()
+	dir = t.TempDir()
+	root = filepath.Join(dir, "docker")
+	daemon := filepath.Join(dir, "daemon.json")
+	if err := os.WriteFile(daemon, []byte(`{"data-root":"`+root+`"}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	before := runtimeConfigs
+	runtimeConfigs = runtimeFiles{daemonJSON: daemon, storageConf: filepath.Join(dir, "none.conf")}
+	t.Cleanup(func() { runtimeConfigs = before })
+	return dir, root
+}
+
+// refusedAsPlacement fails the test unless err is a PlacementRefusedError.
+func refusedAsPlacement(t *testing.T, err error) {
+	t.Helper()
+	var refusal *PlacementRefusedError
+	if err == nil || !errors.As(err, &refusal) {
+		t.Fatalf("a filled file below a refused runtime root was not refused: %v", err)
+	}
+}
+
+// 1138/AC1: a filled write asks no runtime, and judges with the roots the last apply read.
+func TestAFilledWriteAsksNoRuntimeAndUsesTheLastApplysRoots(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "docker-root")
+	setRuntimeRoots(t, []string{root}) // as if an apply had just read it
+
+	var asked []string
+	askNoRuntime(t, &asked)
+
+	path := filepath.Join(root, "app.conf")
+	d := filledDecl(t, path, `"content":"","owner-at-runtime":"owned"`, "")
+	_, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, path, []byte("x\n"), noServices,
+		nil, nil, nil)
+
+	refusedAsPlacement(t, err)
+	if len(asked) != 0 {
+		t.Errorf("a filled write asked the container runtimes: %v", asked)
+	}
+}
+
+// 1138/AC2: given a real apply that read a runtime data root, a filled write below it is refused as today, with
+// no apply run again in between.
+func TestAFilledWriteBelowARootARealApplyReadIsRefused(t *testing.T) {
+	dir, root := aRuntimeConfig(t)
+	t.Cleanup(func() {
+		runtimeRootsMu.Lock()
+		runtimeRoots = nil
+		runtimeRootsMu.Unlock()
+	})
+
+	// A full apply of something unrelated reads the runtimes' configuration once, at its start.
+	elsewhere := filepath.Join(dir, "elsewhere.conf")
+	if _, _, err := Apply(context.Background(), archHost(t), declare(t,
+		fmt.Sprintf(`{"id":"m.other","type":"file","path":%q,"content":"x\n"}`, elsewhere)),
+		store.State{}, store.OriginDeclared, noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+
+	// A filled write below the runtime's root, with no apply run since, is still refused.
+	path := filepath.Join(root, "sub", "app.conf")
+	d := filledDecl(t, path, `"content":"","owner-at-runtime":"owned"`, "")
+	_, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, path, []byte("x\n"), noServices,
+		nil, nil, nil)
+	refusedAsPlacement(t, err)
+}
+
+// 1138/AC3: with no apply having read the roots yet, a filled write judges by the runtimes' configuration files,
+// asking no runtime.
+func TestAFilledWriteWithNoApplyYetUsesTheRuntimesConfigFiles(t *testing.T) {
+	setRuntimeRoots(t, nil) // no apply has read the roots yet
+	_, root := aRuntimeConfig(t)
+
+	var asked []string
+	askNoRuntime(t, &asked)
+
+	path := filepath.Join(root, "sub", "app.conf")
+	d := filledDecl(t, path, `"content":"","owner-at-runtime":"owned"`, "")
+	_, _, err := WriteFilled(context.Background(), archHost(t), d, store.State{}, path, []byte("x\n"), noServices,
+		nil, nil, nil)
+
+	refusedAsPlacement(t, err)
+	if len(asked) != 0 {
+		t.Errorf("a filled write with no apply yet asked the container runtimes: %v", asked)
+	}
+}
+
+// 1138/AC4: a full apply still asks each runtime once, at its start, whatever the number of files it writes.
+func TestAFullApplyAsksEachRuntimeOnceRegardlessOfFileCount(t *testing.T) {
+	dir := t.TempDir()
+	var asked []string
+	before := AskRuntimes
+	AskRuntimes = func(_ context.Context, name string, _ ...string) (string, error) {
+		asked = append(asked, name)
+		return "", errors.New("not running in this test")
+	}
+	t.Cleanup(func() {
+		AskRuntimes = before
+		runtimeRootsMu.Lock()
+		runtimeRoots = nil
+		runtimeRootsMu.Unlock()
+	})
+
+	var resources []string
+	for i := 0; i < 3; i++ {
+		resources = append(resources, fmt.Sprintf(`{"id":"m.f%d","type":"file","path":%q,"content":"x\n"}`, i,
+			filepath.Join(dir, fmt.Sprintf("f%d.conf", i))))
+	}
+	if _, _, err := Apply(context.Background(), archHost(t), declare(t, strings.Join(resources, ",")),
+		store.State{}, store.OriginDeclared, noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	if got := count(asked, "docker"); got != 1 {
+		t.Errorf("docker was asked %d times for one apply of 3 files; want 1: %v", got, asked)
+	}
+	if got := count(asked, "podman"); got != 1 {
+		t.Errorf("podman was asked %d times for one apply of 3 files; want 1: %v", got, asked)
+	}
+}