From c0d94e04f3fc4fcc2cb281d3bf2c33c4bcf4aaae Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 23 Sep 2026 23:12:52 +0200 Subject: [PATCH] Recreate a container when the content of a file it reads at creation changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host decided whether a container was still the one declared by a digest of its declaration, and the declaration names an env-file's path and a mount's path — never what is in them. So when the store was given a new port, the host rewrote the forge's and the analytics service's environment files, correctly, and left both containers running with the old port in their environment: a container reads its env-file when it is CREATED, and `docker restart` hands it the same environment again. Both looked healthy until they answered 502. What a running container takes in at creation is now part of its spec, by content: every env-file, a file bind-mounted into it, and every file this host wrote at or under a directory bind-mounted into it — the secrets, bindings and configs under a module's state directories. The digest is the one the store already records for a file the host wrote (`wrote`), read from the state as it stands when the container is reached, so a file rewritten earlier in the same apply is already the new one; a file the host has no record of — an env-file a predecessor left, the superuser secret genesis writes before any declaration names it — is read from disk, which is what keeps adopting a running store in place a reconcile and not a recreate. Deliberately not part of it: what else is in a bind-mounted directory, which is the service's own data and changes while it runs; a named volume; a seed created once, which digests as the seed the host wrote and not as what has grown in it; and a step — a run-once or scheduled container reads its files when it runs and runs fresh each time. On an adopted node a held container is held before any of this is looked at. The host records what each container was created reading, per file, so the recreate can say which file changed — "recreated: changed" in the report and, now with its detail, in the log. A container made before this record existed is recreated once and says so. novox/hq 04-ISSUES/103 --- internal/apply/apply.go | 162 ++++++++++++++++++-- internal/apply/apply_test.go | 12 +- internal/apply/reads_test.go | 221 ++++++++++++++++++++++++++++ internal/apply/runonce_test.go | 10 +- internal/apply/schedule.go | 2 +- internal/declaration/declaration.go | 16 +- internal/store/store.go | 29 ++++ 7 files changed, 423 insertions(+), 29 deletions(-) create mode 100644 internal/apply/reads_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index d473f3f..184d9a0 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -52,6 +52,9 @@ type Outcome struct { wrote string // into is what a file written into held before the mesh's keys (novox/hq ADR 0102). into *store.Into + // reads is, for a container, the digest of each file it was created reading, by path — so + // the next apply can say which one changed (novox/hq 04-ISSUES/103). + reads map[string]string } // Report is what an apply did, in the order it did it. @@ -266,9 +269,16 @@ func ApplyKeeping( // machine reports success, and what is inside is using a credential the mesh has replaced // (novox/hq 04-ISSUES/045). Folding these into the container's spec makes the comparison a // standing one instead. - declares := map[string]string{} + // + // And what each file a container reads at creation holds — its env-files and what is mounted + // into it — by the digest this host recorded when it wrote the file, read from `known` as it + // stands when the container is reached, so a file rewritten earlier in this same apply is + // already the new one (novox/hq 04-ISSUES/103). That needs the file applied before the + // container, which is the declared order; a container declared ahead of its file sees the + // change one apply late, and never misses it. + in := inputs{declares: map[string]string{}, known: &known} for _, resource := range d.Resources { - declares[resource.Identity()] = declaredDigest(resource) + in.declares[resource.Identity()] = declaredDigest(resource) } // Everything is attempted, and every failure is reported. @@ -340,7 +350,7 @@ func ApplyKeeping( !known.Recorded(string(declaration.TypeFile), f.Path) { keepFound = keep } - outcome, err = applyOne(ctx, sys, resource, run, changed, declares, was, unseal, keepFound) + outcome, err = applyOne(ctx, sys, resource, run, changed, in, was, unseal, keepFound) } if err != nil { failed := &Error{Resource: resource.Identity(), Err: err, Done: report} @@ -386,6 +396,7 @@ func ApplyKeeping( Target: outcome.Target, AppliedAt: time.Now().UTC(), Wrote: outcome.wrote, Into: outcome.into, + Reads: outcome.reads, Holds: holds(resource), }) // Its module has been taken, and what was held for it is now the mesh's. @@ -396,7 +407,14 @@ func ApplyKeeping( report.Outcomes = append(report.Outcomes, outcome) if outcome.Action != "unchanged" { changed[resource.Identity()] = true - log(fmt.Sprintf(" %s %s (%s)", outcome.Action, outcome.ID, outcome.Target)) + // With the detail, when there is one: "updated app" says a container was replaced; + // which file made that happen is what somebody reading the log at the time needs + // (novox/hq 04-ISSUES/103). + line := fmt.Sprintf(" %s %s (%s)", outcome.Action, outcome.ID, outcome.Target) + if outcome.Detail != "" { + line += ": " + outcome.Detail + } + log(line) } } @@ -440,7 +458,7 @@ const guardPrefix = declaration.AdoptionPrefix + "guard" type Unseal func(sealed string) ([]byte, error) func applyOne(ctx context.Context, sys system.System, r declaration.Resource, run Runner, - changed map[string]bool, declares map[string]string, previous store.Applied, + changed map[string]bool, in inputs, previous store.Applied, unseal Unseal, keepFound Keep) (Outcome, error) { switch res := r.(type) { case *declaration.Directory: @@ -452,7 +470,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru case *declaration.Package: return applyPackage(ctx, sys, res, run) case *declaration.Container: - return applyContainer(ctx, res, run, changed, declares, previous) + return applyContainer(ctx, res, run, changed, in, previous) case *declaration.User: return applyUser(ctx, sys, res, run) case *declaration.Archive: @@ -1129,9 +1147,99 @@ const ( idLabel = "mesh-host.id" ) +// inputs is what a container takes in when it is created beyond its own declaration: what each +// resource it names under restart-on currently declares, and what the files it reads hold. +type inputs struct { + // declares is each resource's declared digest, by id (declaredDigest). + declares map[string]string + // known is the node's state as it stands when the container is reached — so a file applied + // earlier in the same pass is already its new self. Nil where nothing was written: a test, + // or a scheduled fire, which reads no file at creation. + known *store.State +} + +// fileDigest is what a file the container reads holds, by digest. +// +// **What this host wrote when it has a record of writing it, and what is on disk when it has +// not.** The record is preferred because it is what the host means by the file: a seed created +// once digests as the seed, not as whatever the service has grown in it, and a file written into +// digests as the mesh's keys, not the machine's (novox/hq ADR 0102). A file the host never wrote +// — an env-file a predecessor left, the superuser secret genesis writes before any declaration +// names it — is read, so the digest is the same one the host records when it later writes the +// same bytes there, and adopting a running store in place stays a reconcile rather than a +// recreate (bootstrap phase three). Empty when there is nothing readable there: the runtime +// refuses an absent env-file itself, with a better message than this could give. +func (in inputs) fileDigest(path string) string { + if in.known != nil { + for _, f := range in.known.FilesUnder(path) { + if f.Target == path { + return f.Wrote + } + } + } + info, err := os.Stat(path) + if err != nil || !info.Mode().IsRegular() { + return "" + } + content, err := os.ReadFile(path) + if err != nil { + return "" + } + return digestOf(string(content)) +} + +// reads is every file a running container takes in when it is created, by path and digest +// (novox/hq 04-ISSUES/103). +// +// - every env-file: the runtime reads it once, at create, and `docker restart` hands the +// container the same environment it had. +// - a file bind-mounted into it, by its content — and, for a directory bind-mounted into it, +// every file THIS HOST wrote at or beneath the source: the secrets, bindings and configs it +// put under the module's state directories. What else is in a mounted directory is the +// service's own data, which changes while it runs and is nothing to recreate it for. +// +// A step is not here: a run-once or scheduled container reads its files when it runs, and +// runs fresh each time. Only a container that stays running holds what it read. +func (in inputs) reads(r *declaration.Container) map[string]string { + if r.RunOnce || r.Schedule != "" { + return nil + } + out := map[string]string{} + for _, path := range r.EnvFile { + out[path] = in.fileDigest(path) + } + for _, v := range r.Volumes { + src := mountSource(v) + if !strings.HasPrefix(src, "/") { + continue // a named volume: the runtime's, holding data + } + if in.known != nil { + for _, f := range in.known.FilesUnder(src) { + out[f.Target] = f.Wrote + } + } + if _, recorded := out[src]; !recorded { + if digest := in.fileDigest(src); digest != "" { + out[src] = digest + } + } + } + if len(out) == 0 { + return nil + } + return out +} + // containerSpec is the identity of a declared container: everything that, if changed, means // the running container is no longer what was asked for. -func containerSpec(r *declaration.Container, declares map[string]string) string { +func containerSpec(r *declaration.Container, in inputs) string { + return containerSpecReading(r, in.declares, in.reads(r)) +} + +// containerSpecReading is containerSpec with what the container reads already read — so an +// applier that also records those digests reads each file once, and the label and the record +// cannot disagree about a file that moved between two reads. +func containerSpecReading(r *declaration.Container, declares, reads map[string]string) string { keys := make([]string, 0, len(r.Env)) for k := range r.Env { keys = append(keys, k) @@ -1171,6 +1279,15 @@ func containerSpec(r *declaration.Container, declares map[string]string) string for _, id := range depends { b.WriteString("reads " + id + "=" + declares[id] + "\n") } + // And what the files it reads at creation hold — not only their paths, which `volume` and the + // env-file arguments already name. The spec named the env-file's path and not its content, + // so the host rewrote two environment files with the store's new port and left both + // containers running with the old one, healthy-looking, until they answered 502 (novox/hq + // 04-ISSUES/103). Added only when there is something read, so a container that reads nothing + // keeps the digest it had. + for _, path := range sortedKeys(reads) { + b.WriteString("file " + path + "=" + reads[path] + "\n") + } return fmt.Sprintf("%x", sha256.Sum256([]byte(b.String()))) } @@ -1236,9 +1353,12 @@ func applyNetwork(ctx context.Context, r *declaration.Network, run Runner) (Outc } func applyContainer(ctx context.Context, r *declaration.Container, run Runner, - changed map[string]bool, declares map[string]string, previous store.Applied) (Outcome, error) { + changed map[string]bool, in inputs, previous store.Applied) (Outcome, error) { out := begin(r) - want := containerSpec(r, declares) + // Read once, so the spec and the record agree on what was read even if a file moves under them. + reads := in.reads(r) + want := containerSpecReading(r, in.declares, reads) + out.reads = reads cri, err := containerRuntime(ctx, run) if err != nil { @@ -1279,6 +1399,16 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, // already has. reasons := restartedBy(r.RestartOn, changed) + // Which of the files it reads no longer hold what it was created reading. The spec label says + // only that SOMETHING moved; the record of what was read says what — and that is the line a + // person needs when a service went stale without a word (novox/hq 04-ISSUES/103). + var changedFiles []string + for _, path := range sortedKeys(previous.Reads) { + if now, still := reads[path]; still && now != previous.Reads[path] { + changedFiles = append(changedFiles, path) + } + } + switch { case existed && before.Spec == want && before.Running && len(reasons) == 0: out.Action = "unchanged" @@ -1340,9 +1470,19 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, out.Action = "created" if existed { out.Action = "updated" - if len(reasons) > 0 { + switch { + case len(changedFiles) > 0: + out.Detail = "recreated: " + strings.Join(changedFiles, ", ") + " changed" + if len(reasons) > 0 { + out.Detail += "; and to pick up " + strings.Join(reasons, ", ") + } + case len(reasons) > 0: out.Detail = "recreated to pick up " + strings.Join(reasons, ", ") - } else { + case previous.Reads == nil && len(reads) > 0: + // A container made before this host kept what it read: whether the files it holds + // are the ones on disk cannot be known, so it is recreated once and from now on can. + out.Detail = "recreated: what it reads was not on record, so what it holds could not be checked" + default: out.Detail = "replaced; a container's configuration is fixed when it is created" } } diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index ba5c5aa..fea616b 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -666,7 +666,7 @@ func TestAContainerWhoseDeclarationChangedIsReplaced(t *testing.T) { d := parseTrusted(t, `{"declaration":1,"resources":[ {"id":"store","type":"container","name":"store","image":"`+pinned+`","env":{"PGDATA":"/data"}} ]}`) - want := containerSpec(d.Resources[0].(*declaration.Container), nil) + want := containerSpec(d.Resources[0].(*declaration.Container), inputs{}) var removed, created bool run := func(ctx context.Context, name string, args ...string) (string, error) { @@ -704,7 +704,7 @@ func TestAContainerThatMatchesIsLeftAlone(t *testing.T) { d := parseTrusted(t, `{"declaration":1,"resources":[ {"id":"store","type":"container","name":"store","image":"`+pinned+`","env":{"PGDATA":"/data"}} ]}`) - spec := containerSpec(d.Resources[0].(*declaration.Container), nil) + spec := containerSpec(d.Resources[0].(*declaration.Container), inputs{}) var touched bool run := func(ctx context.Context, name string, args ...string) (string, error) { @@ -748,8 +748,8 @@ func TestAContainerIsRecreatedWhenARestartOnResourceChanged(t *testing.T) { // what a container reads is part of what it is, so the old content yields a different spec. was := map[string]string{"config": declaredDigest(&declaration.File{Content: "{\"token\":\"old\"}\n"})} now := map[string]string{"config": declaredDigest(d.Resources[0].(*declaration.File))} - stale := containerSpec(d.Resources[1].(*declaration.Container), was) - fresh := containerSpec(d.Resources[1].(*declaration.Container), now) + stale := containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: was}) + fresh := containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: now}) var removed, created bool run := func(ctx context.Context, name string, args ...string) (string, error) { @@ -1538,8 +1538,8 @@ func TestAContainerStaleFromAnEarlierApplyIsReplaced(t *testing.T) { // The container was created when the file said something else. was := map[string]string{"env": declaredDigest(&declaration.File{Content: "PASSWORD=old\n"})} now := map[string]string{"env": declaredDigest(d.Resources[0].(*declaration.File))} - stale := containerSpec(d.Resources[1].(*declaration.Container), was) - fresh := containerSpec(d.Resources[1].(*declaration.Container), now) + stale := containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: was}) + fresh := containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: now}) var removed, created bool run := func(ctx context.Context, name string, args ...string) (string, error) { diff --git a/internal/apply/reads_test.go b/internal/apply/reads_test.go new file mode 100644 index 0000000..a55507b --- /dev/null +++ b/internal/apply/reads_test.go @@ -0,0 +1,221 @@ +package apply + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq 04-ISSUES/103: a container is recreated when the CONTENT of a file it reads at +// creation changes, not only when its path does. A container takes its env-file and its mounted +// files in once, when it is created; `docker restart` hands it the same environment again, so +// only a recreate carries a rewritten file into the process. +// +// The runtime here is the `machine` fake: it keeps the spec label the host gave a container and +// hands it back on inspect, so the comparison under test is the one the host really makes, +// against what it really wrote — not against a spec a test imagined. + +func applyCarried(t *testing.T, d *declaration.Declaration, known store.State, m *machine, + log func(string)) (Report, store.State) { + t.Helper() + report, state, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, m.run, log, nil) + if err != nil { + t.Fatalf("apply failed: %v", err) + } + return report, state +} + +func TestAContainerIsRecreatedWhenItsEnvFileChanged(t *testing.T) { + // The night of the issue: the store was given a new port, the host rewrote the forge's + // environment file with it — and left the forge running with the old one. + dir := t.TempDir() + env := filepath.Join(dir, "forge.env") + declare := func(port string) *declaration.Declaration { + return parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"forge.env","type":"file","path":"`+env+`","content":"DATABASE_PORT=`+port+`\n","mode":"0600"}, + {"id":"forge.server","type":"container","name":"forge","image":"`+pinned+`","env-file":["`+env+`"]} + ]}`) + } + m := &machine{containers: map[string]*fakeContainer{}} + var logged []string + log := func(line string) { logged = append(logged, line) } + + report, state := applyCarried(t, declare("5432"), store.State{}, m, log) + if o := outcomeOf(report, "forge.server"); o.Action != "created" { + t.Fatalf("the container was not created: %+v", report.Outcomes) + } + + // The store moved. The file is rewritten in this apply, before the container is reached, and + // the container must follow it in the same pass. + m.asked, logged = nil, nil + report, state = applyCarried(t, declare("5433"), state, m, log) + if !m.removed("forge") || !m.did("docker run") { + t.Fatalf("the container kept running with the old environment after its env-file changed: %v", m.asked) + } + o := outcomeOf(report, "forge.server") + if o.Action != "updated" || o.Detail != "recreated: "+env+" changed" { + t.Errorf("the recreate did not say which file changed: %+v", o) + } + var said bool + for _, line := range logged { + if strings.Contains(line, "updated forge.server") && strings.Contains(line, "recreated: "+env+" changed") { + said = true + } + } + if !said { + t.Errorf("the log did not say which file made the container recreate: %q", logged) + } + + // And with nothing moved, it is left alone: content is part of the identity, not a tripwire. + m.asked = nil + report, _ = applyCarried(t, declare("5433"), state, m, log) + if m.did("docker rm") || m.did("docker run") || report.Changed() { + t.Errorf("a container whose env-file did not change was recreated: %v %+v", m.asked, report.Outcomes) + } +} + +func TestAnEnvFileTheHostDidNotWriteIsStillReadForWhatItHolds(t *testing.T) { + // The host has no record of this file — a predecessor left it, or something else on the + // machine maintains it — and the container still reads it once. Its content is read from the + // disk, so a change is a recreate exactly as for a file the host wrote. + dir := t.TempDir() + env := filepath.Join(dir, "app.env") + if err := os.WriteFile(env, []byte("TOKEN=old\n"), 0o600); err != nil { + t.Fatal(err) + } + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"app.server","type":"container","name":"app","image":"`+pinned+`","env-file":["`+env+`"]} + ]}`) + m := &machine{containers: map[string]*fakeContainer{}} + _, state := applyCarried(t, d, store.State{}, m, nil) + + if err := os.WriteFile(env, []byte("TOKEN=new\n"), 0o600); err != nil { + t.Fatal(err) + } + m.asked = nil + report, _ := applyCarried(t, d, state, m, nil) + if !m.removed("app") || !m.did("docker run") { + t.Fatalf("a container reading an env-file the host did not write was not recreated when it changed: %v", m.asked) + } + if o := outcomeOf(report, "app.server"); o.Detail != "recreated: "+env+" changed" { + t.Errorf("the recreate did not name the file: %+v", o) + } +} + +func TestAContainerIsRecreatedWhenAMountedSecretChanged(t *testing.T) { + // A rotated credential has the same shape as a moved port: the host writes the file the + // container mounts, and the process holds the value it was created with. + dir := t.TempDir() + secret := filepath.Join(dir, "db.secret") + declare := func(value string) *declaration.Declaration { + return parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"app.secret","type":"file","path":"`+secret+`","content":"`+value+`","mode":"0600"}, + {"id":"app.server","type":"container","name":"app","image":"`+pinned+`", + "volumes":["`+secret+`:/run/secrets/db:ro"]} + ]}`) + } + m := &machine{containers: map[string]*fakeContainer{}} + _, state := applyCarried(t, declare("hunter2"), store.State{}, m, nil) + + m.asked = nil + report, _ := applyCarried(t, declare("correct-horse-battery-staple"), state, m, nil) + if !m.removed("app") || !m.did("docker run") { + t.Fatalf("the container kept the secret it was created with after the mounted file changed: %v", m.asked) + } + if o := outcomeOf(report, "app.server"); o.Action != "updated" || o.Detail != "recreated: "+secret+" changed" { + t.Errorf("the recreate did not say which file changed: %+v", o) + } +} + +func TestWhatAServiceWritesInAMountedDirectoryIsNotPartOfWhatItIs(t *testing.T) { + // A bind-mounted directory is the service's data: it changes while the service runs, and + // recreating for it would restart a database for every row it wrote. What the HOST wrote + // under that directory — a config it rendered there — is another matter: the service read + // that once, at start. + dir := t.TempDir() + state := filepath.Join(dir, "state") + config := filepath.Join(state, "config.toml") + declare := func(level string) *declaration.Declaration { + return parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"app.state","type":"directory","path":"`+state+`"}, + {"id":"app.config","type":"file","path":"`+config+`","content":"level = \"`+level+`\"\n"}, + {"id":"app.server","type":"container","name":"app","image":"`+pinned+`", + "volumes":["`+state+`:/var/lib/app"]} + ]}`) + } + m := &machine{containers: map[string]*fakeContainer{}} + _, known := applyCarried(t, declare("info"), store.State{}, m, nil) + + // The service grows its data in the directory it was given. + if err := os.WriteFile(filepath.Join(state, "app.db"), []byte("rows"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(state, "cache"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(state, "cache", "index"), []byte("entries"), 0o600); err != nil { + t.Fatal(err) + } + m.asked = nil + report, known := applyCarried(t, declare("info"), known, m, nil) + if m.did("docker rm") || m.did("docker run") || report.Changed() { + t.Errorf("a container was recreated for data its service wrote in a mounted directory: %v %+v", + m.asked, report.Outcomes) + } + + // The host's own config under the same directory changes: that, the service read at start. + m.asked = nil + report, _ = applyCarried(t, declare("debug"), known, m, nil) + if !m.removed("app") || !m.did("docker run") { + t.Fatalf("a config the host wrote under a mounted directory changed and the container was not recreated: %v", m.asked) + } + if o := outcomeOf(report, "app.server"); o.Detail != "recreated: "+config+" changed" { + t.Errorf("the recreate did not name the config: %+v", o) + } +} + +func TestAHeldContainerIsNotRecreatedByAChangedHeldFile(t *testing.T) { + // On an adopted node the predecessor's container and the file it reads are both held as + // found (novox/hq ADR 0100). The predecessor rewriting its own file is reported on the file + // — and is nothing to recreate the container for: it is not the host's to recreate. + dir := t.TempDir() + env := filepath.Join(dir, "hello.env") + if err := os.WriteFile(env, []byte("PORT=5432\n"), 0o600); err != nil { + t.Fatal(err) + } + m := &machine{containers: map[string]*fakeContainer{ + "hello-web": {id: "predecessor-id", running: true}, + }} + d := adopted(t, untaken("hello-web.env", "hello-web.server"), + `{"id":"hello-web.env","type":"file","path":"`+env+`","content":"PORT=5433\n","mode":"0600"}, + {"id":"hello-web.server","type":"container","name":"hello-web","image":"`+pinned+`","env-file":["`+env+`"]}`) + report, state := applyAdopted(t, d, store.State{}, m, dir) + if outcomeOf(report, "hello-web.env").Action != "held" || outcomeOf(report, "hello-web.server").Action != "held" { + t.Fatalf("the predecessor's file and container were not held: %+v", report.Outcomes) + } + + if err := os.WriteFile(env, []byte("PORT=5434\n"), 0o600); err != nil { + t.Fatal(err) + } + m.asked = nil + report, state = applyAdopted(t, d, state, m, dir) + for _, a := range m.asked { + if strings.HasPrefix(a, "docker run") || strings.HasPrefix(a, "docker rm") { + t.Fatalf("a held container was acted on because a held file changed: %s", a) + } + } + if o := outcomeOf(report, "hello-web.server"); o.Action != "held" { + t.Errorf("the container is no longer held: %+v", o) + } + if h, _ := state.HeldAt("hello-web.env"); h.Changed != "rewritten" { + t.Errorf("the predecessor's rewrite was not reported on the file: %+v", h) + } + if h, _ := state.HeldAt("hello-web.server"); h.Changed != "" { + t.Errorf("a file change was charged to the container: %+v", h) + } +} diff --git a/internal/apply/runonce_test.go b/internal/apply/runonce_test.go index e728e7b..16b3919 100644 --- a/internal/apply/runonce_test.go +++ b/internal/apply/runonce_test.go @@ -69,7 +69,7 @@ func TestARunOnceStepIsRunToCompletionNotLeftRunning(t *testing.T) { if !ok { t.Fatal("a completed run-once step was not recorded") } - if applied.Wrote != containerSpec(d.Resources[0].(*declaration.Container), nil) { + if applied.Wrote != containerSpec(d.Resources[0].(*declaration.Container), inputs{}) { t.Errorf("the run-once record is not the declaration's digest: %q", applied.Wrote) } } @@ -155,7 +155,7 @@ func TestARunOnceStepAlreadyCompletedIsNotReRun(t *testing.T) { d := parseTrusted(t, `{"declaration":1,"resources":[ {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true} ]}`) - want := containerSpec(d.Resources[0].(*declaration.Container), nil) + want := containerSpec(d.Resources[0].(*declaration.Container), inputs{}) var ran bool run := func(ctx context.Context, name string, args ...string) (string, error) { @@ -232,7 +232,7 @@ func TestARunOnceStepRunsAgainWhenWhatItReadsChanged(t *testing.T) { was := map[string]string{"env": declaredDigest(&declaration.File{Content: "ACME_ROOTS=https://10.0.0.1/roots.pem\n"})} known := store.State{} known.Record(store.Applied{ID: "trust", Type: "container", Origin: store.OriginCarried, Target: "trust", - Wrote: containerSpec(d.Resources[1].(*declaration.Container), was)}) + Wrote: containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: was})}) var ran bool run := func(ctx context.Context, name string, args ...string) (string, error) { @@ -262,7 +262,7 @@ func TestARunOnceStepRunsAgainWhenWhatItReadsChanged(t *testing.T) { settled := store.State{} settled.Record(store.Applied{ID: "env", Type: "file", Origin: store.OriginCarried, Target: env, Wrote: now["env"]}) settled.Record(store.Applied{ID: "trust", Type: "container", Origin: store.OriginCarried, Target: "trust", - Wrote: containerSpec(d.Resources[1].(*declaration.Container), now)}) + Wrote: containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: now})}) if _, _, err := Apply(context.Background(), archHost(t), d, settled, store.OriginCarried, run, nil, nil); err != nil { t.Fatalf("re-apply failed: %v", err) } @@ -280,7 +280,7 @@ func TestAContainerNamingARunOnceStepIsRecreatedWhenItRan(t *testing.T) { {"id":"server","type":"container","name":"server","image":"`+pinned+`","restart-on":["trust"]} ]}`) declares := map[string]string{"trust": declaredDigest(d.Resources[0].(*declaration.Container))} - spec := containerSpec(d.Resources[1].(*declaration.Container), declares) + spec := containerSpec(d.Resources[1].(*declaration.Container), inputs{declares: declares}) var removed, created bool run := func(ctx context.Context, name string, args ...string) (string, error) { diff --git a/internal/apply/schedule.go b/internal/apply/schedule.go index ccede49..6c80de1 100644 --- a/internal/apply/schedule.go +++ b/internal/apply/schedule.go @@ -122,7 +122,7 @@ func (s *Scheduler) Sync(d *declaration.Declaration, held map[string]bool) { // Nothing to read: a scheduled container may not declare restart-on — it runs to completion // on its cadence rather than staying running to be restarted — so its identity cannot // depend on another resource's content and there is nothing to pass. - spec := containerSpec(c, nil) + spec := containerSpec(c, inputs{}) if existing := s.jobs[c.Identity()]; existing != nil && existing.spec == spec { // Unchanged: keep where it is in its cadence, refresh the declaration pointer only. existing.container = c diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index c0bc7af..42aea06 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -782,12 +782,16 @@ type Container struct { // RestartOn names resources whose change means this container must be recreated — the same // field a service has, for the same reason (novox/hq 04-ISSUES/009). A container reads a // mounted file once at start; a changed file leaves the running process holding the old value, - // while every check passes because the file on disk is right. The container's spec — image, - // env, volumes — does not include a mounted file's *content*, so a settings change that - // re-renders that file is invisible to the ordinary spec diff. This closes that: the host - // recreates the container when one of these resources changed this pass, even if the spec - // matches. On a run-once step it means *run again*: a step that fetches a fact from a provider - // names the binding it reads, and is run again when the provider moved (novox/hq ADR 0099). + // while every check passes because the file on disk is right. The host recreates the container + // when one of these resources changed this pass, even if the spec matches. + // + // What a running container reads at creation — its env-files, a file mounted into it, and the + // files the host wrote under a directory mounted into it — is part of its spec by content + // since novox/hq 04-ISSUES/103, and needs no naming here. RestartOn is for what the spec + // cannot see: a resource the container reflects without reading it directly, a step it + // consumes the result of. On a run-once step it means *run again*: a step that fetches a fact + // from a provider names the binding it reads, and is run again when the provider moved + // (novox/hq ADR 0099). RestartOn []string `json:"restart-on,omitempty"` // RunOnce marks a container the host runs to completion rather than leaves running: a step, diff --git a/internal/store/store.go b/internal/store/store.go index d2a2da7..063d676 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "sort" + "strings" "time" ) @@ -74,6 +75,14 @@ type Applied struct { // each of the mesh's keys held before it set them, which of them were absent, and whether the // file itself was — so undeclaring it gives the machine back exactly what it had. Into *Into `json:"into,omitempty"` + + // Reads is, for a container, the digest of each file it was created reading — its env-files + // and the files mounted into it — by path (novox/hq 04-ISSUES/103). + // + // A container takes those in once, when it is created, and the digest of the whole is in the + // container's spec label; this is the same information kept per file, so that when the spec + // no longer matches the host can say WHICH file changed rather than only that something did. + Reads map[string]string `json:"reads,omitempty"` } // Into is what a file written into held before the mesh's keys. @@ -172,6 +181,26 @@ func (s State) Recorded(kind, target string) bool { return false } +// FilesUnder returns every file this host has a record of writing at a path or beneath it, of any +// origin and under any id, in path order. +// +// By path rather than by id because a container names what it reads by path: the id a file was +// declared under may change — the bundle's, then a module's, for the same file — and the container +// reading it does not care which. +func (s State) FilesUnder(path string) []Applied { + var out []Applied + for _, r := range s.Resources { + if r.Type != "file" { + continue + } + if r.Target == path || strings.HasPrefix(r.Target, strings.TrimSuffix(path, "/")+"/") { + out = append(out, r) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].Target < out[j].Target }) + return out +} + // HeldAt returns what is held under a resource id. func (s State) HeldAt(id string) (Held, bool) { for _, h := range s.Held {