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

Merge pull request 'A service is restarted only after every file it names is written (hq issue 260)' (#25) from fix/a-service-restarts-after-every-file-it-reads into main

Browse Source
This commit was merged in pull request #25.
This commit is contained in:
mesh-admin
2026-10-05 19:57:22 +00:00
parent 89a7796afe 101f0c0d61
commit a566add815
2 changed files with 180 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/apply/apply.go
+71 -9
View File
@@ -339,21 +339,20 @@ func ApplyMindingWindows(
}
orphans = append(orphans, orphan)
}
ordered := d.Resources
ordered := afterWhatTheyRead(d.Resources)
guardFirst := 0
if d.Adoption != nil {
ordered = nil
// Each part ordered on its own, so a guard service never leaves the part applied first.
var guard, rest []declaration.Resource
for _, r := range d.Resources {
if strings.HasPrefix(r.Identity(), guardPrefix) {
ordered = append(ordered, r)
}
}
guardFirst = len(ordered)
for _, r := range d.Resources {
if !strings.HasPrefix(r.Identity(), guardPrefix) {
ordered = append(ordered, r)
guard = append(guard, r)
} else {
rest = append(rest, r)
}
}
ordered = append(afterWhatTheyRead(guard), afterWhatTheyRead(rest)...)
guardFirst = len(guard)
}
orphansRemoved := false
removeOrphans := func() error {
@@ -775,6 +774,69 @@ func ApplyMindingWindows(
return report, known, nil
}
// afterWhatTheyRead is the resources in declared order, except that **a service comes after every
// resource it names under restart-on or reload-on** (novox/hq issue 260).
//
// A service is started, restarted or reloaded where it is reached, and what it reads must be on the
// machine by then. Declared ahead of one of its files, it was restarted for the file before it while
// the other did not exist yet — the resolver, restarted on its configuration before the zones file
// that configuration names was written, failed its first start on every machine. And a change to a
// file applied after its service was never acted on at all: what moved is only known within one
// apply, and the service had already been passed.
//
// Only a service moves, and only as far as the last of what it names; everything else keeps its
// declared place. A name not in the list is ignored, as it is when the restart is decided. Only
// what is declared after a service is waited for, so no ring can form and nothing is left out.
func afterWhatTheyRead(resources []declaration.Resource) []declaration.Resource {
at := map[string]int{}
for i, r := range resources {
at[r.Identity()] = i
}
// What each service still waits for: the resources it names that come after it.
waits := map[int]map[string]bool{}
for i, r := range resources {
svc, ok := r.(*declaration.Service)
if !ok {
continue
}
for _, id := range append(append([]string{}, svc.RestartOn...), svc.ReloadOn...) {
if j, declared := at[id]; declared && j > i {
if waits[i] == nil {
waits[i] = map[string]bool{}
}
waits[i][id] = true
}
}
}
if len(waits) == 0 {
return resources
}
ordered := make([]declaration.Resource, 0, len(resources))
placed := map[int]bool{}
var place func(i int)
place = func(i int) {
placed[i] = true
ordered = append(ordered, resources[i])
id := resources[i].Identity()
// Whatever was waiting only for this comes now, in declared order.
for j := range resources {
if w := waits[j]; w != nil && w[id] {
delete(w, id)
if len(w) == 0 && !placed[j] {
delete(waits, j)
place(j)
}
}
}
}
for i := range resources {
if !placed[i] && len(waits[i]) == 0 {
place(i)
}
}
return ordered
}
// guardPrefix is the ids of the mesh's guard on an adopted node: its package, table, unit and
// service (novox/hq ADR 0100).
const guardPrefix = declaration.AdoptionPrefix + "guard"
internal/apply/restart_order_test.go
+109
View File
@@ -0,0 +1,109 @@
package apply
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"github.com/novox/mesh-host/internal/store"
)
// The resolver's configuration named a second file, the mesh's zones, declared after the service —
// as a module's facts are, after its resources. The host restarted the service for the
// configuration, the daemon could not read the zones file the same apply had not written yet, and
// the apply failed on every machine (novox/hq issue 260). A service is restarted only once every
// resource it names under restart-on has been applied.
func TestAServiceIsRestartedOnlyOnceEveryFileItNamesIsWritten(t *testing.T) {
dir := t.TempDir()
conf := filepath.Join(dir, "resolver.conf")
zones := filepath.Join(dir, "zones.conf")
declare := func(zonesContent string) string {
return fmt.Sprintf(`{"declaration":1,"resources":[
{"id":"resolver.config","type":"file","path":%q,"content":"conf-file=%s\n","mode":"0644"},
{"id":"resolver.service","type":"service","unit":"resolver.service","state":"running",
"restart-on":["resolver.config","resolver.fact-zones"]},
{"id":"resolver.fact-zones","type":"file","path":%q,"content":%q,"mode":"0644"}
]}`, conf, zones, zones, zonesContent)
}
// A daemon that cannot start without both of its files, as dnsmasq cannot.
var commands []string
services := recordingServices(&commands)
run := func(ctx context.Context, name string, args ...string) (string, error) {
if strings.Contains(strings.Join(args, " "), "start resolver.service") {
if _, err := os.Stat(zones); err != nil {
commands = append(commands, name+" "+strings.Join(args, " "))
return "", errors.New("cannot read " + zones + ": no such file or directory")
}
}
return services(ctx, name, args...)
}
report, state, err := Apply(context.Background(), archHost(t), parse(t, declare("server=/a/1\n")),
store.State{}, store.OriginCarried, run, nil, nil)
if err != nil {
t.Fatalf("the service was restarted before every file it reads was written: %v", err)
}
var detail string
for _, o := range report.Outcomes {
if o.ID == "resolver.service" {
detail = o.Detail
}
}
if !strings.Contains(detail, "resolver.config") || !strings.Contains(detail, "resolver.fact-zones") {
t.Errorf("one restart for both files was expected, the outcome says %q", detail)
}
restarts := 0
for _, c := range commands {
if strings.Contains(c, "stop resolver.service") {
restarts++
}
}
if restarts != 1 {
t.Errorf("the service was restarted %d times; once, after both files: %v", restarts, commands)
}
// And a change to the file declared after the service alone is acted on in the apply that
// makes it — not passed by because the service was reached first.
commands = nil
if _, _, err := Apply(context.Background(), archHost(t), parse(t, declare("server=/b/2\n")),
state, store.OriginCarried, run, nil, nil); err != nil {
t.Fatal(err)
}
restarted := false
for _, c := range commands {
if strings.Contains(c, "stop resolver.service") {
restarted = true
}
}
if !restarted {
t.Errorf("the zones file changed and the service was not restarted: %v", commands)
}
}
// Only a service moves, and only to just after the last resource it names — under restart-on or
// reload-on, in its own module or another's. Everything else keeps its declared place, and a name
// not in the declaration is no reason to move.
func TestAServiceIsOrderedAfterWhatItNamesAndNothingElseMoves(t *testing.T) {
d := parse(t, `{"declaration":1,"resources":[
{"id":"a.dir","type":"directory","path":"/tmp/a"},
{"id":"a.runtime","type":"service","unit":"docker.service","state":"running","reload-on":["b.daemon"]},
{"id":"a.svc","type":"service","unit":"a.service","state":"running","restart-on":["a.conf","gone"]},
{"id":"a.conf","type":"file","path":"/tmp/a/conf","content":"x\n"},
{"id":"b.daemon","type":"file","path":"/tmp/b/daemon.json","content":"{}\n"},
{"id":"b.after","type":"file","path":"/tmp/b/after","content":"y\n"},
{"id":"b.svc","type":"service","unit":"b.service","state":"running","restart-on":["a.conf"]}
]}`)
var got []string
for _, r := range afterWhatTheyRead(d.Resources) {
got = append(got, r.Identity())
}
want := "a.dir a.conf a.svc b.daemon a.runtime b.after b.svc"
if strings.Join(got, " ") != want {
t.Errorf("ordered\n %s\nwant\n %s", strings.Join(got, " "), want)
}
}