diff --git a/internal/apply/runonce_test.go b/internal/apply/runonce_test.go index 106eb44..e728e7b 100644 --- a/internal/apply/runonce_test.go +++ b/internal/apply/runonce_test.go @@ -3,6 +3,7 @@ package apply import ( "context" "errors" + "path/filepath" "strings" "testing" @@ -216,18 +217,102 @@ func TestARunOnceStepReRunsWhenItsDeclarationChanged(t *testing.T) { } } -func TestARunOnceContainerCannotAlsoDeclareRestartOn(t *testing.T) { - // restart-on brings a running container back when a file it read changed; a run-once step does - // not stay running. The two lifecycles contradict, so the parser refuses the pair rather than - // silently resolving to one. - _, err := declaration.Parse([]byte(`{"declaration":1,"resources":[ - {"id":"f","type":"file","path":"/tmp/x","content":"y"}, - {"id":"seed","type":"container","name":"seed","image":"` + pinned + `","run-once":true,"restart-on":["f"]} - ]}`)) - if err == nil { - t.Fatal("a run-once container that also declared restart-on was accepted") +func TestARunOnceStepRunsAgainWhenWhatItReadsChanged(t *testing.T) { + // A step that fetches a fact from a provider names the binding file it reads. What a + // container reads is part of its digest, so when the provider moved and the mesh rewrote the + // file, the step's marker no longer matches and it runs again (novox/hq ADR 0099) — the same + // rule that recreates a running container, read as "again" for a step. + dir := t.TempDir() + env := filepath.Join(dir, "acme.env") + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"env","type":"file","path":"`+env+`","content":"ACME_ROOTS=https://10.0.0.2/roots.pem\n"}, + {"id":"trust","type":"container","name":"trust","image":"`+pinned+`","run-once":true,"restart-on":["env"]} + ]}`) + // The record from when the provider was elsewhere: the step's digest against the old file. + 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)}) + + var ran bool + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "run": + ran = true + return "", nil + } + return "", nil } - if !strings.Contains(err.Error(), "run-once") { - t.Errorf("refused for the wrong reason: %v", err) + report, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("apply failed: %v", err) + } + if !ran { + t.Fatal("a run-once step whose named file changed was not run again") + } + if report.Outcomes[1].Action != "created" { + t.Errorf("the re-run step was not reported as having run: %+v", report.Outcomes[1]) + } + + // And with the file unchanged, the step stays done: "again" is when something changed. + ran = false + now := map[string]string{"env": declaredDigest(d.Resources[0].(*declaration.File))} + 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)}) + if _, _, err := Apply(context.Background(), archHost(t), d, settled, store.OriginCarried, run, nil, nil); err != nil { + t.Fatalf("re-apply failed: %v", err) + } + if ran { + t.Error("a run-once step whose named file did not change was run again") + } +} + +func TestAContainerNamingARunOnceStepIsRecreatedWhenItRan(t *testing.T) { + // The service that consumes what a step made names the step: when the step ran this pass — + // fetched a new root — the running container holds the old one, and its spec did not move, + // so restart-on is what brings it back with the new one (novox/hq ADR 0099). + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"trust","type":"container","name":"trust","image":"`+pinned+`","run-once":true}, + {"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) + + var removed, created bool + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "inspect": + // The server is up, made from exactly this spec — nothing but the step's run says + // it must be replaced. + return "true\t" + spec, nil + case "rm": + if len(args) > 0 && args[len(args)-1] == "server" { + removed = true + } + return "", nil + case "run": + if args[len(args)-1] != "trust" && strings.Contains(strings.Join(args, " "), "--name server") { + created = true + } + return "deadbeef\n", nil + } + return "", nil + } + report, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("apply failed: %v", err) + } + if !removed || !created { + t.Fatalf("the container naming the step was not recreated after the step ran (removed=%v created=%v)", removed, created) + } + server := report.Outcomes[len(report.Outcomes)-1] + if server.Action != "updated" || !strings.Contains(server.Detail, "trust") { + t.Errorf("the recreation did not name the step as its reason: %+v", server) } } diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index f4831fb..cd7cbd3 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -678,7 +678,8 @@ type Container struct { // 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. + // 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). RestartOn []string `json:"restart-on,omitempty"` // RunOnce marks a container the host runs to completion rather than leaves running: a step, @@ -713,13 +714,9 @@ func (c *Container) validate(where string, _ bool) []string { if c.Name == "" { problems = append(problems, where+": a container needs a name") } - // restart-on brings a *running* container back when a file it read changed; a run-once step - // does not stay running to be brought back. Declaring both asks for two contradictory - // lifecycles at once, so it is refused rather than silently resolved to one of them. - if c.RunOnce && len(c.RestartOn) > 0 { - problems = append(problems, where+": a run-once container cannot also declare restart-on; "+ - "it runs to completion rather than staying running to be restarted") - } + // A run-once step may name what it reads under restart-on. For a step the word means *run + // again*: what a container reads is part of its digest, so a step whose named resource changed + // is a different step and runs again (novox/hq ADR 0099). Not refused. // A container runs once and gates, on a cadence, or stays up — never two of these // (novox/hq ADR 0053). run-once and schedule are the two "runs to completion" lifecycles and // contradict each other, and a scheduled step does not stay running to be brought back by