novox/mesh-host
18
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

ENG-7: every new image of a module of two containers or more is pulled before any of its containers is set aside, and a pull that fails replaces nothing (design 51, issue 1966)

Browse Source 
Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig
This commit is contained in:
jschoubben
2026-11-04 11:43:17 +01:00
parent 2779f5312b
commit 05418b4fe5
2 changed files with 133 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/apply/apply.go
+76 -6
View File
@@ -939,6 +939,13 @@ func applyScoped(
}
}
setRunnerWarnings(warnings)
// **Every new image of a module is on the machine before any of its containers is set aside** (design 51, issue 1966
// ENG-7). A container replaced after the first one's turn pulled its image only then, and for a module whose first
// container serves the store its second image comes from, that pull met the stopped one. A pull that fails
// replaces nothing of the module: its containers keep running what they ran, the failure names the image, and what
// follows in the module is skipped as it is for any step that did not complete.
in.pulled = map[string]bool{}
pullFirst(ctx, ordered, d, in.pulled, run, &failures, gated, gatedWhy, log)
for i, resource := range ordered {
if !orphansRemoved && i == guardFirst {
// **Only a guard that is up may let the filter go.** Removing the derived filter's
@@ -2759,6 +2766,9 @@ type inputs struct {
files map[string]bool
// handed is every group a record that went this apply handed to a resource still asking for it.
handed Handed
// pulled is every image this apply found on the machine, or pulled, before any container was replaced (pullFirst):
// applyContainer does not ask for it again.
pulled map[string]bool
}
// fileDigest is what a file the container reads holds, by digest.
@@ -3165,8 +3175,10 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
// 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 !in.pulled[r.Image] {
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)
}
}
// **The running container is set aside, never removed, until its replacement runs** (issue 1945): stopped and
// renamed, so its name and ports are free, and started again under its own name when the replacement does not
@@ -3469,16 +3481,16 @@ func ensureImage(ctx context.Context, cri, image string, run Runner) error {
// already hold — built here, or handed over. There is no registry that answers for it, so
// pulling would fail somewhere that names a network problem instead of a missing image.
if strings.HasPrefix(image, "sha256:") {
return fmt.Errorf(
return &PullError{Image: image, Err: fmt.Errorf(
"image %s is not on this machine, and an image named by its own digest cannot be "+
"fetched: nothing serves it. Build it here, or load it, before applying this",
image)
image)}
}
if _, err := run(ctx, cri, "pull", image); err != nil {
return fmt.Errorf("pulling image %s: %w", image, err)
return &PullError{Image: image, Err: 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 &PullError{Image: image, Err: fmt.Errorf("image %s is not present after pulling it: %w", image, err)}
}
return nil
}
@@ -4340,3 +4352,61 @@ func ownerText(info os.FileInfo) string {
}
return fmt.Sprintf("%d:%d", uid, gid)
}
// pullFirst finds on the machine, or pulls, the image of each container the declaration runs before any is replaced
// (ENG-7) for a module of two containers or more. A container that is run once is left to its marker, and a runtime that
// does not answer to applyContainer, which says so. The first image that cannot be had fails its container with the image named and gates its module, so none of
// its containers is touched; the images found before it stay pulled.
func pullFirst(ctx context.Context, ordered []declaration.Resource, d *declaration.Declaration, pulled map[string]bool,
run Runner, failures *[]*Error, gated map[string]bool, gatedWhy map[string]string, log func(string)) {
// A module of one container already pulls its image before it sets that container aside (applyContainer), and nothing
// of it comes before: only a module of two or more has an image pulled after one of its containers was replaced.
count := map[string]int{}
for _, resource := range ordered {
if c, isContainer := resource.(*declaration.Container); isContainer && c.Image != "" && !c.RunOnce && c.Schedule == "" {
module, _ := d.ModuleOfID(c.Identity())
count[module]++
}
}
var cri string
for _, resource := range ordered {
c, isContainer := resource.(*declaration.Container)
if !isContainer || c.Image == "" || c.RunOnce || c.Schedule != "" || pulled[c.Image] {
continue
}
module, ours := d.ModuleOfID(c.Identity())
if count[module] < 2 {
continue
}
if ours && gated[module] {
continue
}
if cri == "" {
var err error
if cri, err = containerRuntime(ctx, run); err != nil {
return
}
}
if err := ensureImage(ctx, cri, c.Image, run); err != nil {
err = fmt.Errorf("not replacing container %s, or any other container of its module, which keep running: %w", c.Name, err)
*failures = append(*failures, &Error{Resource: c.Identity(), Err: err})
log(fmt.Sprintf(" failed %s (%s): %v", c.Identity(), c.Target(), err))
if ours {
gated[module] = true
gatedWhy[module] = "its image " + c.Image + " could not be had, so none of its containers was replaced"
}
continue
}
pulled[c.Image] = true
}
}
// PullError is an image that could not be found or pulled, as ensureImage fails: what the node agent reads to tell a failed
// pull from another failed apply (ENG-8).
type PullError struct {
Image string
Err error
}
func (e *PullError) Error() string { return e.Err.Error() }
func (e *PullError) Unwrap() error { return e.Err }
internal/apply/replace_image_test.go
+57
View File
@@ -47,6 +47,63 @@ func TestARunningContainerIsKeptWhenItsReplacementImageCannotBeObtained(t *testi
if !strings.Contains(err.Error(), pinned) {
t.Errorf("the failure does not name the image %s: %v", pinned, err)
}
var pull *PullError
if !errors.As(err, &pull) || pull.Image != pinned {
t.Errorf("the failure is not a pull error naming %s: %v", pinned, err)
}
// design 51, ENG-7: a module of two containers whose second new image cannot be pulled replaces neither, and says the
// image; with both pullable, both are pulled before the first is set aside.
second := strings.Replace(pinned, "postgres", "other", 1)
two := `{"declaration":1,"resources":[
{"id":"a.first","type":"container","name":"first","image":"` + pinned + `","env":{"X":"1"}},
{"id":"a.second","type":"container","name":"second","image":"` + second + `","env":{"X":"2"}}
]}`
for _, c := range []struct {
name string
unpullable string
wantTouched bool
wantPulled string
}{
{"the second image cannot be pulled", second, false, "pull " + pinned},
{"both can", "", true, "pull " + pinned + "; pull " + second},
} {
d := parseTrusted(t, two)
var steps []string
present := map[string]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":
if present[args[len(args)-1]] {
return "", nil
}
return "", errors.New("no such image")
case "pull":
if args[1] == c.unpullable {
return "", errors.New("manifest unknown")
}
present[args[1]] = true
steps = append(steps, "pull "+args[1])
case "rm", "rename", "stop", "run":
steps = append(steps, args[0]+" "+strings.Join(args[1:], " "))
}
return "", nil
}
_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
got := strings.Join(steps, "; ")
touched := strings.Contains(got, "rename") || strings.Contains(got, "run ")
if touched != c.wantTouched || !strings.HasPrefix(got, c.wantPulled) {
t.Errorf("%s: steps %q (touched %v), want pulls %q first and touched=%v", c.name, got, touched, c.wantPulled, c.wantTouched)
}
if !c.wantTouched && (err == nil || !strings.Contains(err.Error(), second)) {
t.Errorf("%s: the failure does not name %s: %v", c.name, second, err)
}
}
}
func TestAReplacementImageIsPresentBeforeTheRunningContainerIsRemoved(t *testing.T) {