A container reflects its config: restart-on for containers (04-ISSUES/009)
A container reads a mounted file once, at start; its spec (image, env, volumes) does not include a mounted file's content, so a settings change that re-renders the file left the running process holding the old value while every check passed. Give Container the restart-on field a Service already has, and recreate the container when a named resource changed this pass. Unit-tested (recreated on change, left alone otherwise) and proven in the mesh-lab: a running grafana runtime picked up a token change on the next push. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF
This commit is contained in:
+29
-11
@@ -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
|
||||
for _, id := range r.RestartOn {
|
||||
if changed[id] {
|
||||
which = append(which, id)
|
||||
}
|
||||
}
|
||||
return which
|
||||
return restartedBy(r.RestartOn, changed)
|
||||
}
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 }
|
||||
|
||||
Reference in New Issue
Block a user