From 9b8f39857698de56f174e72d0b3e6a29f69d1031 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 18:05:08 +0200 Subject: [PATCH] Judge a filled write with the roots the last apply read, asking no runtime per file (issue 1138) --- internal/apply/filled.go | 6 +- internal/apply/filled_runtime_roots_test.go | 168 ++++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 internal/apply/filled_runtime_roots_test.go diff --git a/internal/apply/filled.go b/internal/apply/filled.go index d95edd3..6759e9f 100644 --- a/internal/apply/filled.go +++ b/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{} diff --git a/internal/apply/filled_runtime_roots_test.go b/internal/apply/filled_runtime_roots_test.go new file mode 100644 index 0000000..947704a --- /dev/null +++ b/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) + } +}