From 09c77d57b82304ae39771d6c288de712464ac75d Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:16:32 +0200 Subject: [PATCH] Have a replacement container's image before removing the running one, so an unobtainable image leaves the service up (issue 1742) --- internal/apply/apply.go | 8 +++ internal/apply/hold_test.go | 7 +++ internal/apply/replace_image_test.go | 93 ++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 internal/apply/replace_image_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index ed13d6d..5162f19 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -2833,6 +2833,14 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, return out, nil } } + // **The replacement's image is on the machine before the running container is removed** + // (novox/hq issue 1742). `docker run` pulls on its own, but only after the old container is + // gone: an image the registry does not hold then left the service down and every later apply + // failing. Pulled (or found present) first, an unobtainable image fails this resource loudly, + // naming the image, and the old container keeps running. + if err := ensureImage(ctx, cri, r.Image, run); err != nil { + return out, fmt.Errorf("not replacing container %s, which keeps running: %w", r.Name, err) + } if _, err := run(ctx, cri, "rm", "-f", r.Name); err != nil { return out, fmt.Errorf("replacing container %s: %w", r.Name, err) } diff --git a/internal/apply/hold_test.go b/internal/apply/hold_test.go index c0195d3..e8ba6bd 100644 --- a/internal/apply/hold_test.go +++ b/internal/apply/hold_test.go @@ -185,6 +185,13 @@ func (m *machine) run(_ context.Context, name string, args ...string) (string, e return "", errors.New("no such image") } return "", nil + case "pull": + // A real runtime leaves the image present after a pull. + if m.images == nil { + m.images = map[string]string{} + } + m.images[args[len(args)-1]] = "2026-01-01T00:00:00Z" + return "", nil case "network": if len(args) > 1 && args[1] == "inspect" { return strings.Join(m.members[args[len(args)-1]], ",") + ",\n", nil diff --git a/internal/apply/replace_image_test.go b/internal/apply/replace_image_test.go new file mode 100644 index 0000000..f653274 --- /dev/null +++ b/internal/apply/replace_image_test.go @@ -0,0 +1,93 @@ +package apply + +import ( + "context" + "errors" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// Issue 1742: the engine removed a running container and only then found the replacement image +// unobtainable, leaving the service down. The replacement image comes first. +func TestARunningContainerIsKeptWhenItsReplacementImageCannotBeObtained(t *testing.T) { + d := parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"store","type":"container","name":"store","image":"`+pinned+`","env":{"PGDATA":"/data"}} + ]}`) + var removed, created bool + run := func(ctx context.Context, name string, args ...string) (string, error) { + args = oldShape(name, args) + switch args[0] { + case "info": + return "27.0\n", nil + case "container": + return "true\tsome-older-spec", nil + case "image": + return "", errors.New("no such image") + case "pull": + return "", errors.New("manifest unknown") + case "rm": + removed = true + return "", nil + case "run": + created = true + return "deadbeef\n", nil + } + return "", nil + } + _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if removed || created { + t.Fatalf("the running container was touched although its image is unobtainable (removed=%v created=%v)", removed, created) + } + if err == nil { + t.Fatal("an unobtainable image failed nothing") + } + if !strings.Contains(err.Error(), pinned) { + t.Errorf("the failure does not name the image %s: %v", pinned, err) + } +} + +func TestAReplacementImageIsPresentBeforeTheRunningContainerIsRemoved(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), inputs{}) + var order []string + created := false + run := func(ctx context.Context, name string, args ...string) (string, error) { + args = oldShape(name, args) + switch args[0] { + case "info": + return "27.0\n", nil + case "container": + if created { + return "true\t" + want, nil + } + return "true\tsome-older-spec", nil + case "image": + order = append(order, "inspect") + if len(order) == 1 { + return "", errors.New("no such image") + } + return "", nil + case "pull": + order = append(order, "pull") + case "rm": + order = append(order, "rm") + case "run": + order = append(order, "run") + created = true + return "deadbeef\n", nil + } + return "", nil + } + if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil); err != nil { + t.Fatalf("apply failed: %v", err) + } + got := strings.Join(order, ",") + if !strings.HasPrefix(got, "inspect,pull,inspect,rm,run") { + t.Errorf("order of runtime calls = %s, want the image pulled before rm", got) + } +}