Take a container left set aside as the next apply's, and bound an apply to 30 minutes (issue 1945)

cmd/mesh-host/apply_context_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"testing"
+	"time"
 )
 
 // An apply runs in a context the bus's loss cannot end: the link's context ends when the bus goes, and the bus may go
@@ -12,7 +13,8 @@ import (
 func TestAnApplyOutlivesTheLinksContext(t *testing.T) {
 	type key struct{}
 	link, cancel := context.WithCancel(context.WithValue(context.Background(), key{}, "v"))
-	ctx := applyContext(link)
+	ctx, done := applyContext(link)
+	defer done()
 	cancel()
 	if ctx.Err() != nil {
 		t.Fatalf("the apply's context ended with the link's: %v", ctx.Err())
@@ -21,3 +23,17 @@ func TestAnApplyOutlivesTheLinksContext(t *testing.T) {
 		t.Fatal("the apply's context lost the link's values")
 	}
 }
+
+// An apply is bounded all the same: a step that hangs (a docker call that never returns) cannot hold the node agent for
+// ever (the review of issue 1945).
+func TestAnApplyIsBounded(t *testing.T) {
+	ctx, done := applyContext(context.Background())
+	defer done()
+	at, ok := ctx.Deadline()
+	if !ok {
+		t.Fatal("an apply has no bound")
+	}
+	if left := time.Until(at); left > applyBound || left < applyBound-time.Minute {
+		t.Fatalf("the bound is %s away, want about %s", left, applyBound)
+	}
+}

cmd/mesh-host/nodeagent.go

@@ -298,13 +298,17 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, sess
 			Backup:   backupNode(session),
 			Say:      say,
 			Apply: func(ctx context.Context, c delivery.Composed, others []delivery.Composed) agent.Result {
-				res := applyComposition(applyContext(session.WithLink(ctx)), opts, c, others, sched, say)
+				actx, done := applyContext(session.WithLink(ctx))
+				res := applyComposition(actx, opts, c, others, sched, say)
+				done()
 				afterApply()
 				sayFacts(ctx, ag)
 				return res
 			},
 			Remove: func(ctx context.Context, module string, others []delivery.Composed) agent.Result {
-				res := removeComposition(applyContext(session.WithLink(ctx)), opts, module, others, sched, say)
+				actx, done := applyContext(session.WithLink(ctx))
+				res := removeComposition(actx, opts, module, others, sched, say)
+				done()
 				afterApply()
 				sayFacts(ctx, ag)
 				return res
@@ -568,8 +572,14 @@ func declaresNothingToBackUp(said json.RawMessage, module string) (nothing bool,
 	return true, true
 }
 
-// applyContext is the context an apply or a removal runs in: the link's values, never its end. The link's context ends
+// applyBound is how long one apply or removal may take: long enough for the slowest image pull and backup step, short
-// when the bus goes, and the bus may go because of the apply itself (it replaces the bus's own container), which must
+// enough that a step that hangs cannot hold the node agent for ever.
-// then finish: a container removed and never made again took the mesh down (issue 1945, live on 2026-10-25 at 11:23).
+const applyBound = 30 * time.Minute
-// An apply is bounded by its own steps' bounds, not by the link.
+
-func applyContext(ctx context.Context) context.Context { return context.WithoutCancel(ctx) }
+// applyContext is the context an apply or a removal runs in: the link's values, never its end, and a bound of its own.
+// The link's context ends when the bus goes, and the bus may go because of the apply itself (it replaces the bus's own
+// container), which must then finish: a container removed and never made again took the mesh down (issue 1945, live on
+// 2026-10-25 at 11:23). applyBound keeps a hung step from blocking the agent for ever.
+func applyContext(ctx context.Context) (context.Context, context.CancelFunc) {
+	return context.WithTimeout(context.WithoutCancel(ctx), applyBound)
+}

internal/apply/apply.go

@@ -2965,6 +2965,13 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner,
 	}
 
 	aside := ""
+	if !existed {
+		// A container an engine set aside and then stopped before it ran the replacement (it died between the two):
+		// the set-aside one of this apply, put back if the replacement fails and removed once it runs (issue 1945).
+		if _, err := containerState(ctx, r.Name+asideSuffix, run); err == nil {
+			aside = r.Name + asideSuffix
+		}
+	}
 	switch {
 	case existed && (before.Spec == want || legacy) && before.Running && len(reasons) == 0:
 		out.Action = "unchanged"

internal/apply/containers_kept_test.go

@@ -20,6 +20,9 @@ func TestAContainerJoinsTheNetworksItKeeps(t *testing.T) {
 		if name != "docker" {
 			return "", errors.New("not installed")
 		}
+		if len(args) > 0 && args[0] == "container" && strings.HasSuffix(args[len(args)-1], asideSuffix) {
+			return "", errors.New("no such container") // nothing was left set aside
+		}
 		ran = append(ran, strings.Join(args, " "))
 		switch args[0] {
 		case "info":

internal/apply/replace_image_test.go

@@ -162,3 +162,58 @@ func TestAReplacementThatRunsRemovesTheContainerSetAside(t *testing.T) {
 		t.Fatalf("the steps were %q: the set-aside container is removed after the replacement runs", got)
 	}
 }
+
+// A container left set aside by an engine that stopped between setting it aside and running its replacement is the
+// set-aside one of the next apply: put back when the replacement fails, removed once it runs (the review of issue
+// 1945).
+func TestAContainerLeftAsideIsPutBackOrRemovedByTheNextApply(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{})
+	for _, failRun := range []bool{true, false} {
+		var order []string
+		created := false
+		run := func(ctx context.Context, name string, args ...string) (string, error) {
+			switch args[0] {
+			case "info":
+				return "27.0\n", nil
+			case "container":
+				switch args[len(args)-1] {
+				case "store":
+					if created {
+						return "true\t" + want, nil
+					}
+					return "", errors.New("no such container")
+				case "store.replaced":
+					return "false\tsome-older-spec", nil
+				}
+				return "", errors.New("no such container")
+			case "image":
+				return "", nil
+			case "rm", "rename", "stop", "start":
+				order = append(order, strings.Join(args, " "))
+				return "", nil
+			case "run":
+				order = append(order, "run")
+				if failRun {
+					return "", errors.New("port is already allocated")
+				}
+				created = true
+				return "deadbeef\n", nil
+			}
+			return "", nil
+		}
+		_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
+		got := strings.Join(order, "; ")
+		if failRun {
+			if err == nil || !strings.Contains(got, "rename store.replaced store; start store") {
+				t.Fatalf("a failed run beside a container left aside: %v; steps %q", err, got)
+			}
+			continue
+		}
+		if err != nil || !strings.Contains(got, "run; rm -f store.replaced") {
+			t.Fatalf("a run beside a container left aside: %v; steps %q", err, got)
+		}
+	}
+}