diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 8cba0e3..838b2ed 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -189,7 +189,16 @@ func Apply( // to do with a file on the other side of the declaration, and stopping there is what // made one broken module hold a whole machine hostage // (novox/hq 04-ISSUES/011). - if resource.Kind() == declaration.TypeAction { + // A run-once container is a step with the same purpose as an action: to make + // something true *before* the next thing needs it (novox/hq ADR 0052). A broker whose + // dynsec store was not seeded must not be started, so a run-once step that did not + // complete gates what follows exactly as a failed action does — that is the whole of + // how "before that container starts" is enforced, since the step is declared before it. + gates := resource.Kind() == declaration.TypeAction + if c, ok := resource.(*declaration.Container); ok && c.RunOnce { + gates = true + } + if gates { failed.Done = report failed.Others = len(failures) - 1 failed.Gated = true @@ -241,7 +250,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) + return applyContainer(ctx, res, run, changed, previous) case *declaration.User: return applyUser(ctx, sys, res, run) case *declaration.Archive: @@ -929,7 +938,7 @@ func applyNetwork(ctx context.Context, r *declaration.Network, run Runner) (Outc return out, nil } -func applyContainer(ctx context.Context, r *declaration.Container, run Runner, changed map[string]bool) (Outcome, error) { +func applyContainer(ctx context.Context, r *declaration.Container, run Runner, changed map[string]bool, previous store.Applied) (Outcome, error) { out := begin(r) want := containerSpec(r) @@ -938,6 +947,10 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, c return out, fmt.Errorf("%w, so nothing can be said about %q", err, r.Name) } + if r.RunOnce { + return applyRunOnce(ctx, r, run, cri, want, previous) + } + before, err := containerState(ctx, r.Name, run) existed := err == nil @@ -1019,6 +1032,79 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, c return out, nil } +// applyRunOnce runs a container to completion, once, and requires it to exit 0 (novox/hq ADR 0052). +// +// It is a step, not a service: the module's own code seeding a store, migrating a schema or +// gating on health, under the module's own account (ADR 0047), before the container that depends +// on it. Three things make it a step rather than an ordinary container: +// +// - It is run in the foreground, so the runtime returns the container's exit code. A non-zero +// exit is an error here, and — because a run-once step gates the apply the way a failed action +// does — that error stops everything the declaration places after it. That is how "before the +// broker starts" is enforced: the step is declared first, and the broker is never reached +// until it has completed. +// - Its record of having happened is the digest of its declaration, recorded by the caller only +// after it exits 0 (ADR 0018). The step leaves nothing running to inspect, so the persisted +// digest — not a live container — is the marker. A re-apply whose declaration digest already +// matches does nothing; a changed declaration re-runs it. +// - It writes a seed only if it is absent and never reconciles it, so what a running program +// grows in that seed afterward is never wiped (04-ISSUES/035). The host's marker keeps the +// step from re-running; the step's own code keeps it from clobbering on the pass it does run. +func applyRunOnce(ctx context.Context, r *declaration.Container, run Runner, cri, want string, previous store.Applied) (Outcome, error) { + out := begin(r) + + // Already completed for this exact declaration. The marker is the store, because a run-once + // step leaves nothing running to ask. + if previous.Wrote != "" && previous.Wrote == want { + out.Action = "unchanged" + out.Detail = "run-once step already completed for this declaration" + out.wrote = want + return out, nil + } + + // A container by this name from a previous, different declaration must not linger and be + // mistaken for this run. Removing a name that is not there is the state we want, so its error + // is ignored. + _, _ = run(ctx, cri, "rm", "-f", r.Name) + + // Run in the foreground so the runtime waits for the container and hands back its exit code. + // No --detach and no --restart: a step that is restarted is not a step. + args := []string{"run", "--name", r.Name} + for _, file := range r.EnvFile { + args = append(args, "--env-file", file) + } + if r.Network != "" { + args = append(args, "--network", r.Network) + } + args = append(args, "--label", specLabel+"="+want, "--label", idLabel+"="+r.ID) + for _, k := range sortedKeys(r.Env) { + args = append(args, "--env", k+"="+r.Env[k]) + } + for _, v := range r.Volumes { + args = append(args, "--volume", v) + } + for _, h := range r.Hosts { + args = append(args, "--add-host", h) + } + args = append(args, r.Image) + args = append(args, r.Args...) + + if _, err := run(ctx, cri, args...); err != nil { + // A non-zero exit or a runtime that could not start it. Either way the step did not make + // the machine ready, so the apply must not go on to the container that needs it. + return out, fmt.Errorf("run-once step %s did not complete: %w", r.Name, err) + } + + // It completed. Remove the exited container so a later apply is not confused by a stopped one; + // the record that it ran is the digest below, which the caller persists after the fact. + _, _ = run(ctx, cri, "rm", "-f", r.Name) + + out.Action = "created" + out.Detail = "run-once step completed" + out.wrote = want + return out, nil +} + // restartedBy is which of the named resources changed this pass — the reason a container or service // must be brought back rather than left as it is (novox/hq 04-ISSUES/009). func restartedBy(restartOn []string, changed map[string]bool) []string { diff --git a/internal/apply/runonce_test.go b/internal/apply/runonce_test.go new file mode 100644 index 0000000..264b1d5 --- /dev/null +++ b/internal/apply/runonce_test.go @@ -0,0 +1,233 @@ +package apply + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// A run-once container is a step, not a service (novox/hq ADR 0052): the host runs it to +// completion, requires exit 0, records that it ran, and — because a failed step gates the apply — +// starts whatever the declaration places after it only once the step has finished. These tests +// defend that, each named for the claim it holds up. + +// nameOf returns the value after --name in a docker run argument list. +func nameOf(args []string) string { + for i, a := range args { + if a == "--name" && i+1 < len(args) { + return args[i+1] + } + } + return "" +} + +func TestARunOnceStepIsRunToCompletionNotLeftRunning(t *testing.T) { + // The difference between a step and a service is that a step is run in the foreground and its + // exit code is the answer. So the host must not pass --detach (which returns before the + // container exits) nor --restart (a step that is restarted is not a step). + var runArgs []string + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "run": + runArgs = args + return "", nil // ran and exited 0 + case "rm": + return "", nil + } + return "", nil + } + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true} + ]}`) + + report, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("a run-once step that exited 0 failed the apply: %v", err) + } + if runArgs == nil { + t.Fatal("the run-once step was never run") + } + if got := strings.Join(runArgs, " "); strings.Contains(got, "--detach") { + t.Errorf("a run-once step was detached, so its exit could not be observed: %s", got) + } + if got := strings.Join(runArgs, " "); strings.Contains(got, "--restart") { + t.Errorf("a run-once step was given a restart policy, which makes it a service: %s", got) + } + if report.Outcomes[0].Action != "created" { + t.Errorf("a completed run-once step was not reported created: %+v", report.Outcomes[0]) + } + // It ran, so it was recorded — and the record is the digest of the declaration, which is what + // keeps a re-apply from running it again. + applied, ok := state.Find("seed") + if !ok { + t.Fatal("a completed run-once step was not recorded") + } + if applied.Wrote != containerSpec(d.Resources[0].(*declaration.Container)) { + t.Errorf("the run-once record is not the declaration's digest: %q", applied.Wrote) + } +} + +func TestARunOnceStepThatExitsNonZeroFailsTheApply(t *testing.T) { + // Run in the foreground, a non-zero exit is an error the runtime hands back. That must fail + // the apply, not be swallowed — a seed that did not happen leaves the machine unready. + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "run": + return "", errors.New("exit status 1") + case "rm": + return "", nil + } + return "", nil + } + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true} + ]}`) + + _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err == nil { + t.Fatal("a run-once step that did not complete was accepted") + } + if !strings.Contains(err.Error(), "did not complete") { + t.Errorf("failed for the wrong reason: %v", err) + } + if _, recorded := state.Find("seed"); recorded { + t.Error("a run-once step that did not complete was recorded as done") + } +} + +func TestAFailedRunOnceStepGatesWhatFollows(t *testing.T) { + // The whole of how "before the broker starts" is enforced: the step is declared first, and a + // step that did not complete stops the apply reaching the container that depends on it — the + // mirror of a failed action stopping what follows. + var startedNames []string + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "inspect": + return "false\t\n", errors.New("no such container") + case "run": + startedNames = append(startedNames, nameOf(args)) + if nameOf(args) == "seed" { + return "", errors.New("exit status 1") // the step fails + } + return "deadbeef\n", nil + case "rm": + return "", nil + } + return "", nil + } + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true}, + {"id":"broker","type":"container","name":"broker","image":"`+pinned+`"} + ]}`) + + _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err == nil { + t.Fatal("a failed run-once step did not fail the apply") + } + var applyErr *Error + if !errors.As(err, &applyErr) { + t.Fatalf("got %T", err) + } + if !applyErr.Gated { + t.Error("the failure does not say that nothing after the step was attempted") + } + for _, n := range startedNames { + if n == "broker" { + t.Fatal("the broker was started even though its run-once step did not complete") + } + } +} + +func TestARunOnceStepAlreadyCompletedIsNotReRun(t *testing.T) { + // Its record of having happened is the digest of its declaration. A re-apply that finds the + // digest already recorded does nothing — a step is not reconciled toward, it is run once. + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true} + ]}`) + want := containerSpec(d.Resources[0].(*declaration.Container)) + + 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 + case "rm": + return "", nil + } + return "", nil + } + known := store.State{} + known.Record(store.Applied{ID: "seed", Type: "container", Origin: store.OriginCarried, Target: "seed", Wrote: want}) + + report, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("re-applying a completed run-once step failed: %v", err) + } + if ran { + t.Error("a run-once step already completed for this declaration was run again") + } + if report.Changed() { + t.Errorf("a re-applied run-once step reported a change: %+v", report.Outcomes) + } +} + +func TestARunOnceStepReRunsWhenItsDeclarationChanged(t *testing.T) { + // The marker is the declaration's digest, so a changed image or environment moves it and the + // step runs again — a migration or a seed that changed is a different step. + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"seed","type":"container","name":"seed","image":"`+pinned+`","run-once":true,"env":{"CLIENT":"new-admin"}} + ]}`) + + 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 + case "rm": + return "", nil + } + return "", nil + } + // A record from an earlier, different declaration of the same step. + known := store.State{} + known.Record(store.Applied{ID: "seed", Type: "container", Origin: store.OriginCarried, Target: "seed", Wrote: "an-older-digest"}) + + if _, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil); err != nil { + t.Fatalf("apply failed: %v", err) + } + if !ran { + t.Error("a run-once step whose declaration changed was not run again") + } +} + +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") + } + if !strings.Contains(err.Error(), "run-once") { + t.Errorf("refused for the wrong reason: %v", err) + } +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 0ce7338..c86bd65 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -523,6 +523,16 @@ type Container struct { // recreates the container when one of these resources changed this pass, even if the spec // matches. RestartOn []string `json:"restart-on,omitempty"` + + // RunOnce marks a container the host runs to completion rather than leaves running: a step, + // not a service (novox/hq ADR 0052). The host runs it, requires it to exit 0, and records that + // it did — and because the declaration is applied in order and a failed step halts the apply, + // whatever is declared after a run-once container starts only once the step has finished. It is + // how a module runs its own code at first boot — seed a store, migrate, health-gate — under its + // own account (ADR 0047), before the container that depends on it. The record that it ran is + // the digest of this declaration, so a re-apply does not re-run it unless the declaration + // changed. + RunOnce bool `json:"run-once,omitempty"` } func (c *Container) Identity() string { return c.ID } @@ -534,6 +544,13 @@ 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") + } return append(problems, checkImage(where, c.Image)...) }