internal/apply/apply.go

@@ -241,7 +241,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru
 	case *declaration.Package:
 		return applyPackage(ctx, sys, res, run)
 	case *declaration.Container:
-		return applyContainer(ctx, res, run)
+		return applyContainer(ctx, res, run, changed)
 	case *declaration.User:
 		return applyUser(ctx, sys, res, run)
 	case *declaration.Archive:
@@ -521,13 +521,7 @@ func reflects(r *declaration.Service, changed map[string]bool) bool {
 // reflected is which of them changed, so the outcome can say why the service was restarted. A
 // restart with no reason given is indistinguishable from a service that keeps falling over.
 func reflected(r *declaration.Service, changed map[string]bool) []string {
-	var which []string
+	return restartedBy(r.RestartOn, changed)
-	for _, id := range r.RestartOn {
-		if changed[id] {
-			which = append(which, id)
-		}
-	}
-	return which
 }
 
 func applyService(ctx context.Context, sys system.System, r *declaration.Service, run Runner,
@@ -886,7 +880,7 @@ func applyNetwork(ctx context.Context, r *declaration.Network, run Runner) (Outc
 	return out, nil
 }
 
-func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (Outcome, error) {
+func applyContainer(ctx context.Context, r *declaration.Container, run Runner, changed map[string]bool) (Outcome, error) {
 	out := begin(r)
 	want := containerSpec(r)
 
@@ -898,8 +892,16 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (
 	before, err := containerState(ctx, r.Name, run)
 	existed := err == nil
 
+	// A container reads a mounted file once, at start. When one of its restart-on resources changed
+	// this pass — a settings-merged config the runtime read, say — the file on disk is new and the
+	// running process still holds the old value, and the spec (image, env, volumes) has not moved,
+	// so the plain "spec matches, leave it" below would keep the stale process for ever
+	// (novox/hq 04-ISSUES/009). Recreating is how a container gets restart-on, which a service
+	// already has.
+	reasons := restartedBy(r.RestartOn, changed)
+
 	switch {
-	case existed && before.Spec == want && before.Running:
+	case existed && before.Spec == want && before.Running && len(reasons) == 0:
 		out.Action = "unchanged"
 		return out, nil
 	case existed:
@@ -959,11 +961,27 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner) (
 	out.Action = "created"
 	if existed {
 		out.Action = "updated"
-		out.Detail = "replaced; a container's configuration is fixed when it is created"
+		if len(reasons) > 0 {
+			out.Detail = "recreated to pick up " + strings.Join(reasons, ", ")
+		} else {
+			out.Detail = "replaced; a container's configuration is fixed when it is created"
+		}
 	}
 	return out, nil
 }
 
+// restartedBy is which of the named resources changed this pass — the reason a container or service
+// must be brought back rather than left as it is (novox/hq 04-ISSUES/009).
+func restartedBy(restartOn []string, changed map[string]bool) []string {
+	var which []string
+	for _, id := range restartOn {
+		if changed[id] {
+			which = append(which, id)
+		}
+	}
+	return which
+}
+
 func sortedKeys(m map[string]string) []string {
 	keys := make([]string, 0, len(m))
 	for k := range m {

internal/apply/apply_test.go

@@ -730,6 +730,67 @@ func TestAContainerThatMatchesIsLeftAlone(t *testing.T) {
 	}
 }
 
+func TestAContainerIsRecreatedWhenARestartOnResourceChanged(t *testing.T) {
+	// A container reads a mounted file once, at start. When the file changed this pass but the
+	// container's spec did not, the plain "spec matches, leave it" rule would keep the process
+	// holding the old value for ever, with every check passing (novox/hq 04-ISSUES/009). A
+	// container names the resources it must reflect in restart-on, the same as a service, and the
+	// host recreates it. Here the config file is fresh, so it is written this pass, and the
+	// already-running-and-matching container must still be replaced.
+	dir := t.TempDir()
+	conf := filepath.Join(dir, "config.json")
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"config","type":"file","path":"`+conf+`","content":"{\"token\":\"new\"}\n"},
+	  {"id":"app","type":"container","name":"app","image":"`+pinned+`","restart-on":["config"]}
+	]}`)
+	spec := containerSpec(d.Resources[1].(*declaration.Container))
+
+	var removed, created bool
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		switch args[0] {
+		case "info":
+			return "27.0\n", nil
+		case "inspect":
+			// The container already exists, running, with exactly the spec it is declared with —
+			// only the mounted file changed.
+			return "true\t" + spec, nil
+		case "rm":
+			removed = true
+			return "", nil
+		case "run":
+			created = true
+			return "deadbeef\n", nil
+		}
+		return "", nil
+	}
+
+	report, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
+	if err != nil {
+		t.Fatalf("apply failed: %v", err)
+	}
+	if !removed || !created {
+		t.Fatalf("a container was not recreated when its restart-on file changed (removed=%v created=%v)", removed, created)
+	}
+	app := report.Outcomes[len(report.Outcomes)-1]
+	if app.Action != "updated" || !strings.Contains(app.Detail, "config") {
+		t.Errorf("the recreation did not name why it happened: %+v", app)
+	}
+}
+
+func TestRestartOnFiresOnlyForResourcesThatChanged(t *testing.T) {
+	// restart-on must not mean "always restart": it names resources, and only a resource that
+	// changed this pass is a reason. This is the guard shared by containers and services
+	// (novox/hq 04-ISSUES/009), so a container that reflects an unchanged file is left running.
+	changed := map[string]bool{"other": true}
+	if got := restartedBy([]string{"config"}, changed); len(got) != 0 {
+		t.Errorf("an unchanged resource was treated as a reason to restart: %v", got)
+	}
+	changed["config"] = true
+	if got := restartedBy([]string{"config", "missing"}, changed); len(got) != 1 || got[0] != "config" {
+		t.Errorf("restart-on did not name exactly the changed resource: %v", got)
+	}
+}
+
 // --- boot state (novox/hq: a unit started but not enabled stops being true at the next reboot) ---
 
 // systemctlStub answers `show` and `is-enabled` the way systemd does, and records the verbs it

internal/declaration/declaration.go

@@ -462,6 +462,16 @@ type Container struct {
 	// and guessing an address that works from inside a container, which is the same thing with a
 	// worse failure mode.
 	Network string `json:"network,omitempty"`
+
+	// RestartOn names resources whose change means this container must be recreated — the same
+	// field a service has, for the same reason (novox/hq 04-ISSUES/009). A container reads a
+	// mounted file once at start; a changed file leaves the running process holding the old value,
+	// while every check passes because the file on disk is right. The container's spec — image,
+	// env, volumes — does not include a mounted file's *content*, so a settings change that
+	// re-renders that file is invisible to the ordinary spec diff. This closes that: the host
+	// recreates the container when one of these resources changed this pass, even if the spec
+	// matches.
+	RestartOn []string `json:"restart-on,omitempty"`
 }
 
 func (c *Container) Identity() string { return c.ID }