apply: ensure a scheduled container's image is present at apply, without running it
A schedule: container (ADR 0053) is installed as present state and never run at apply — the Scheduler fires it later on its cadence. But a service or run-once container only gets its image as a side effect of docker run, so a scheduled step's image was not pulled until its first scheduled fire: absent from the node right after a successful apply, so the first run paid the whole pull latency and tooling that expects the image present after apply found it missing. applyContainer now probes the runtime and ensures the pinned image present for a scheduled step before recording it. A new ensureImage helper inspects the image and pulls it only if absent, then reads back (ADR 0018). Ensuring an image is not running it: no docker run fires the container, so the no-run invariant of ADR 0053 holds. The runtime probe, previously skipped for a schedule, now runs because a pull needs it — the schedule.go comment is updated to match. Tests: the install-does-not-run test is extended to allow the image-ensure while asserting no fire and no needless pull; a new test applies a scheduled container whose image is absent and asserts it is pulled and still not started. go build, go vet, go test ./... all pass. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF
This commit is contained in:
+40
-7
@@ -948,21 +948,30 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, c
|
|||||||
out := begin(r)
|
out := begin(r)
|
||||||
want := containerSpec(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).
|
// 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
|
// 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
|
// 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
|
// 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
|
// RUN the container.
|
||||||
// for exactly that reason.
|
//
|
||||||
|
// 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 r.Schedule != "" {
|
||||||
|
if err := ensureImage(ctx, cri, r.Image, run); err != nil {
|
||||||
|
return out, err
|
||||||
|
}
|
||||||
return applySchedule(r, want, previous)
|
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 {
|
if r.RunOnce {
|
||||||
return applyRunOnce(ctx, r, run, cri, want, previous)
|
return applyRunOnce(ctx, r, run, cri, want, previous)
|
||||||
}
|
}
|
||||||
@@ -1130,6 +1139,30 @@ func foregroundRunArgs(r *declaration.Container, want string) []string {
|
|||||||
return args
|
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
|
// applySchedule installs a scheduled step: it records the schedule as present and reports the node
|
||||||
// current, without running anything (novox/hq ADR 0053).
|
// current, without running anything (novox/hq ADR 0053).
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -203,8 +203,9 @@ func (s *Scheduler) fire(ctx context.Context, j *scheduledJob) {
|
|||||||
s.log(fmt.Sprintf("scheduled step %s: run completed", j.id))
|
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
|
// runtime detects the container runtime once and caches it. The Scheduler needs one only to fire a
|
||||||
// installed (see applySchedule), only to be fired, so detection is deferred to here.
|
// 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) {
|
func (s *Scheduler) runtime(ctx context.Context) (string, error) {
|
||||||
s.mu.Lock()
|
s.mu.Lock()
|
||||||
cached := s.cri
|
cached := s.cri
|
||||||
|
|||||||
@@ -85,11 +85,25 @@ func scheduledContainer(t *testing.T, schedule string) *declaration.Container {
|
|||||||
|
|
||||||
func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) {
|
func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) {
|
||||||
// The deliberate inversion of run-once: the schedule is state that is present, so the apply is
|
// 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.
|
// current as soon as it is recorded. Installing it ensures the pinned image is present (a
|
||||||
var calls []string
|
// 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) {
|
run := func(ctx context.Context, name string, args ...string) (string, error) {
|
||||||
calls = append(calls, name+" "+strings.Join(args, " "))
|
switch args[0] {
|
||||||
return "", errors.New("installing a schedule must not run any command")
|
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":[
|
d := parseTrusted(t, `{"declaration":1,"resources":[
|
||||||
{"id":"sync","type":"container","name":"sync","image":"`+pinned+`","schedule":"0 3 * * *"}
|
{"id":"sync","type":"container","name":"sync","image":"`+pinned+`","schedule":"0 3 * * *"}
|
||||||
@@ -99,8 +113,11 @@ func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("installing a schedule failed the apply: %v", err)
|
t.Fatalf("installing a schedule failed the apply: %v", err)
|
||||||
}
|
}
|
||||||
if len(calls) != 0 {
|
if fired {
|
||||||
t.Errorf("installing a schedule ran commands, so it did more than record state: %v", calls)
|
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" {
|
if report.Outcomes[0].Action != "created" {
|
||||||
t.Errorf("an installed schedule was not reported created: %+v", report.Outcomes[0])
|
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")
|
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)
|
report2, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
@@ -122,6 +139,59 @@ func TestInstallingAScheduleDoesNotRunItAndReportsCurrent(t *testing.T) {
|
|||||||
if report2.Changed() {
|
if report2.Changed() {
|
||||||
t.Errorf("re-installing the same schedule reported a change: %+v", report2.Outcomes)
|
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 <image>`
|
||||||
|
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) {
|
func TestAScheduledStepDoesNotGateWhatFollows(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user