diff --git a/internal/apply/apply.go b/internal/apply/apply.go index ad21d29..6b2c65d 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -948,21 +948,30 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, c out := begin(r) want := containerSpec(r) + cri, err := containerRuntime(ctx, run) + if err != nil { + return out, fmt.Errorf("%w, so nothing can be said about %q", err, r.Name) + } + // A scheduled step is state that is present, not a container to start (novox/hq ADR 0053). // Installing it records the schedule and reports the node current at once — the deliberate // inversion of run-once, which gates. The recurring run is fired by the host's Scheduler off the // clock, re-established from this declaration each apply, and NEVER here — so installing does not - // run the container and does not even need a runtime present. Checked before the runtime probe - // for exactly that reason. + // RUN the container. + // + // It does, however, ensure the pinned image is present now. A service or a run-once container + // gets its image as a side effect of `docker run`; a scheduled step is never run at apply, so + // without this its image would be absent from the node until the first scheduled fire — which + // would pay the whole pull latency then, and leave tooling that expects the image present after + // apply looking at a node that does not have it. Ensuring the image is not running it, so the + // no-run invariant holds. if r.Schedule != "" { + if err := ensureImage(ctx, cri, r.Image, run); err != nil { + return out, err + } return applySchedule(r, want, previous) } - cri, err := containerRuntime(ctx, run) - if err != nil { - 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) } @@ -1130,6 +1139,30 @@ func foregroundRunArgs(r *declaration.Container, want string) []string { return args } +// ensureImage makes the pinned image present on the node without running anything. +// +// A service or a run-once container gets its image as a side effect of `docker run` — the first run +// pulls it. A scheduled step is installed but deliberately never run at apply (novox/hq ADR 0053), so +// nothing would pull its image until the first scheduled fire: the image is absent from the node +// right after a successful apply, the first run pays the whole pull latency, and tooling that expects +// the image present after apply finds it missing. This fetches the same bytes `docker run` would, and +// stops short of starting the container. +// +// Idempotent, and it reads back (novox/hq ADR 0018): an image already present is left as is, and a +// pull that reported success but left nothing there is a failure, not a convergence. +func ensureImage(ctx context.Context, cri, image string, run Runner) error { + if _, err := run(ctx, cri, "image", "inspect", image); err == nil { + return nil + } + if _, err := run(ctx, cri, "pull", image); err != nil { + return fmt.Errorf("pulling image %s: %w", image, err) + } + if _, err := run(ctx, cri, "image", "inspect", image); err != nil { + return fmt.Errorf("image %s is not present after pulling it: %w", image, err) + } + return nil +} + // applySchedule installs a scheduled step: it records the schedule as present and reports the node // current, without running anything (novox/hq ADR 0053). // diff --git a/internal/apply/schedule.go b/internal/apply/schedule.go index 3357128..039385a 100644 --- a/internal/apply/schedule.go +++ b/internal/apply/schedule.go @@ -203,8 +203,9 @@ func (s *Scheduler) fire(ctx context.Context, j *scheduledJob) { s.log(fmt.Sprintf("scheduled step %s: run completed", j.id)) } -// runtime detects the container runtime once and caches it. A scheduled step does not need one to be -// installed (see applySchedule), only to be fired, so detection is deferred to here. +// runtime detects the container runtime once and caches it. The Scheduler needs one only to fire a +// run, so detection is deferred to here — separate from the apply, which does its own probe when it +// ensures a scheduled step's image is present (see applyContainer). func (s *Scheduler) runtime(ctx context.Context) (string, error) { s.mu.Lock() cached := s.cri diff --git a/internal/apply/schedule_test.go b/internal/apply/schedule_test.go index fb30900..bd0813c 100644 --- a/internal/apply/schedule_test.go +++ b/internal/apply/schedule_test.go @@ -85,11 +85,25 @@ func scheduledContainer(t *testing.T, schedule string) *declaration.Container { func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) { // The deliberate inversion of run-once: the schedule is state that is present, so the apply is - // current as soon as it is recorded — nothing is run, and no runtime is even probed. - var calls []string + // current as soon as it is recorded. Installing it ensures the pinned image is present (a + // scheduled step is never run at apply, so nothing else pulls it), but it NEVER runs the + // container — no `docker run` fires it, which is the invariant that matters (novox/hq ADR 0053). + var fired bool // a `docker run` — the container was started + var pulled bool // an image reported present was pulled anyway run := func(ctx context.Context, name string, args ...string) (string, error) { - calls = append(calls, name+" "+strings.Join(args, " ")) - return "", errors.New("installing a schedule must not run any command") + switch args[0] { + case "info": + return "27.0\n", nil // a container runtime answers the apply's probe + case "image": + return "", nil // `image inspect`: the pinned image is already present + case "pull": + pulled = true + return "", nil + case "run": + fired = true + return "", errors.New("installing a schedule must not run the container") + } + return "", nil } d := parseTrusted(t, `{"declaration":1,"resources":[ {"id":"sync","type":"container","name":"sync","image":"`+pinned+`","schedule":"0 3 * * *"} @@ -99,8 +113,11 @@ func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) { if err != nil { t.Fatalf("installing a schedule failed the apply: %v", err) } - if len(calls) != 0 { - t.Errorf("installing a schedule ran commands, so it did more than record state: %v", calls) + if fired { + t.Error("installing a schedule ran the container — it installs state, it does not fire it") + } + if pulled { + t.Error("installing a schedule pulled an image it had just found present") } if report.Outcomes[0].Action != "created" { t.Errorf("an installed schedule was not reported created: %+v", report.Outcomes[0]) @@ -114,7 +131,7 @@ func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) { t.Error("an installed schedule recorded no marker, so a re-apply cannot tell it is unchanged") } - // And a re-apply of the same declaration is unchanged and still runs nothing. + // And a re-apply of the same declaration is unchanged and still fires nothing. report2, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil) if err != nil { t.Fatal(err) @@ -122,6 +139,59 @@ func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) { if report2.Changed() { t.Errorf("re-installing the same schedule reported a change: %+v", report2.Outcomes) } + if fired { + t.Error("re-installing a schedule ran the container") + } +} + +func TestInstallingAScheduleEnsuresItsImageIsPresentWithoutStartingIt(t *testing.T) { + // A scheduled step is never run at apply, so `docker run` — which is what pulls a service's or a + // run-once step's image — never fetches it. Without an explicit pull the image is absent from the + // node until the first scheduled fire, which then pays the whole pull latency and, until it runs, + // leaves tooling that expects the image present after apply looking at a node without it. So the + // apply ensures the image present: when it is absent it is pulled, and still nothing is run. + imagePresent := false + var pulledImage string + var fired bool + run := func(ctx context.Context, name string, args ...string) (string, error) { + switch args[0] { + case "info": + return "27.0\n", nil + case "image": // `image inspect ` + if imagePresent { + return "", nil + } + return "", errors.New("no such image") + case "pull": + pulledImage = args[len(args)-1] + imagePresent = true // a real runtime leaves the image present after a pull + return "", nil + case "run": + fired = true + return "", nil + } + return "", nil + } + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"sync","type":"container","name":"sync","image":"`+pinned+`","schedule":"0 3 * * *"} + ]}`) + + report, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("installing a schedule whose image was absent failed the apply: %v", err) + } + if pulledImage != pinned { + t.Errorf("installing a schedule did not pull its pinned image (pulled %q, want %q)", pulledImage, pinned) + } + if fired { + t.Error("installing a schedule ran the container — ensuring the image is present is not running it") + } + if report.Outcomes[0].Action != "created" { + t.Errorf("an installed schedule was not reported created: %+v", report.Outcomes[0]) + } + if _, ok := state.Find("sync"); !ok { + t.Fatal("an installed schedule was not recorded as applied") + } } func TestAScheduledStepDoesNotGateWhatFollows(t *testing.T) {