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

H1 engine-stop-at-failure: a module's apply stops at its first failed resource

Browse Source 
A failed resource of a module gates the rest of that module, which is reported
skipped with the failed resource named; the module's result names each one left.
A resource of no module, and other modules, go on as before. One table test
replaces two in apply_test.go; two fixtures adapted to the rule.

Builds: design 42 zfs-owns-zfs §1 (ADR 0679 §16 (a)), A-22
This commit is contained in:
jschoubben
2026-11-13 00:19:49 +01:00
parent 0193949efe
commit fb492da7e7
5 changed files with 105 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/mesh-host/probe_test.go
+5 -4
View File
@@ -88,11 +88,12 @@ func TestThePackagesGoInWhenTheApplyThatRestoresThePackageManagerWroteIt(t *test
conf := filepath.Join(dir, "pacman.conf")
pacmanAnsweringOnce(t, conf)
opts := options{state: filepath.Join(dir, "state.json")}
// The package first: within one pass it is asked about before the file is written, which is the order that
// leaves it to a second pass.
// The file first, as the pacman module declares it: a module's apply stops at its first failed resource
// (design 42 zfs-owns-zfs §1, answer A-26), so the file that makes the package manager answer comes before the
// package that needs it.
raw := []byte(`{"declaration":1,"resources":[` +
`{"id":"pacman.base","type":"package","package":"base"},` +
`{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"}]}`)
`{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"},` +
`{"id":"pacman.base","type":"package","package":"base"}]}`)
report := applyAndKeep(context.Background(), opts, raw, nil, nil, nil)
internal/apply/apply.go
+10
View File
@@ -1162,6 +1162,16 @@ func applyScoped(
if p, ok := resource.(*declaration.Process); ok && p.RunOnce {
gates = true
}
// **A module's apply stops at its first failed resource** (design 42 zfs-owns-zfs §1, ADR 0679 §16 (a)): any
// other resource of a module that failed gates the rest of that module, which is reported skipped with this
// reason. What it applied before stays applied; other modules, and a resource of no module, go on as before.
if module, ours := d.ModuleOfID(resource.Identity()); ours && !gates {
failed.Gated = true
gated[module] = true
gatedWhy[module] = fmt.Sprintf("%s failed, so nothing more of this module was attempted", resource.Identity())
log(fmt.Sprintf(" gated %s.*: %s failed, so the rest of it was not attempted", module, resource.Identity()))
continue
}
if gates {
failed.Gated = true
// **A step that failed has not completed, whatever it did before** (novox/hq issue 467). A
internal/apply/apply_test.go
+80 -64
View File
@@ -182,77 +182,93 @@ func TestARenameToTheSamePathDoesNotDeleteTheNewFile(t *testing.T) {
}
}
func TestAFailedStepFailsTheApplyAndTheRestIsStillAttempted(t *testing.T) {
// The apply fails, names the resource, and carries what did happen — because the machine is
// in whatever state the apply reached and the only honest thing to hand back is that list.
//
// **And everything is attempted.** It used to stop at the first failure, which made one
// broken resource hold the whole machine hostage: a module declaring a package that does not
// exist meant every module after it was never applied, for ever
// (novox/hq 04-ISSUES/011). The case for stopping was that a later resource may depend on an
// earlier one — and it still may, and it then fails its own check and is reported, which is
// more information than not attempting it.
// A failed resource fails the apply and names itself, and what else is attempted depends on whose it is
// (novox/hq 04-ISSUES/011; design 42 zfs-owns-zfs §1, ADR 0679 §16 (a)): the resources of no module are all
// attempted, so one broken resource holds no machine hostage and every failure is counted, not just the first;
// a module's apply stops at its first failed resource, reports each one left, and no other module is touched.
func TestAFailedResourceFailsTheApplyAndWhatFollowsIsAttemptedByWhoseItIs(t *testing.T) {
dir := t.TempDir()
blocker := filepath.Join(dir, "blocker")
if err := os.WriteFile(blocker, []byte("i am a file\n"), 0o644); err != nil {
t.Fatal(err)
}
d := parse(t, `{"declaration":1,"resources":[
{"id":"fine","type":"file","path":"`+filepath.Join(dir, "fine.conf")+`","content":"a\n"},
{"id":"doomed","type":"directory","path":"`+blocker+`"},
{"id":"after","type":"file","path":"`+filepath.Join(dir, "after.conf")+`","content":"b\n"}
]}`)
_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, noServices, nil, nil)
if err == nil {
t.Fatal("an impossible resource did not fail the apply")
}
var applyErr *Error
if !errors.As(err, &applyErr) {
t.Fatalf("expected an apply error, got %T", err)
}
if applyErr.Resource != "doomed" {
t.Errorf("the failure names %q, not the resource that failed", applyErr.Resource)
}
// Everything that worked is in the report, before and after the failure.
if len(applyErr.Done.Outcomes) != 2 {
t.Errorf("the error does not carry what was applied: %+v", applyErr.Done.Outcomes)
}
if _, err := os.Stat(filepath.Join(dir, "after.conf")); err != nil {
t.Error("a resource after the failing one was never attempted, so one broken module " +
"still blocks every module after it")
}
}
func TestEveryFailureIsCountedNotJustTheFirst(t *testing.T) {
// "One thing failed" and "eleven things failed" are different machines, and the first line is
// what somebody reads.
dir := t.TempDir()
for _, name := range []string{"one", "two"} {
if err := os.WriteFile(filepath.Join(dir, name), []byte("a file\n"), 0o644); err != nil {
for _, name := range []string{"blocker", "one", "two"} {
if err := os.WriteFile(filepath.Join(dir, name), []byte("i am a file\n"), 0o644); err != nil {
t.Fatal(err)
}
}
d := parse(t, `{"declaration":1,"resources":[
{"id":"first","type":"directory","path":"`+filepath.Join(dir, "one")+`"},
{"id":"second","type":"directory","path":"`+filepath.Join(dir, "two")+`"}
]}`)
_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, noServices, nil, nil)
if err == nil {
t.Fatal("two impossible resources did not fail the apply")
file := func(id, name string) string {
return `{"id":"` + id + `","type":"file","path":"` + filepath.Join(dir, name) + `","content":"a\n"}`
}
var applyErr *Error
if !errors.As(err, &applyErr) {
t.Fatalf("got %T", err)
unwritable := func(id, name string) string { // a file below a file: it cannot be made
return `{"id":"` + id + `","type":"file","path":"` + filepath.Join(dir, name, "x.conf") + `","content":"a\n"}`
}
if applyErr.Others != 1 {
t.Errorf("the failure says %d others also failed, and one did", applyErr.Others)
doomed := func(id, name string) string {
return `{"id":"` + id + `","type":"directory","path":"` + filepath.Join(dir, name) + `"}`
}
if !strings.Contains(err.Error(), "one other resource also failed") {
t.Errorf("the message does not say others failed: %v", err)
exists := func(name string) bool {
_, err := os.Stat(filepath.Join(dir, name))
return err == nil
}
for _, tc := range []struct {
name string
resources []string
failed string
others int
applied []string // files that must exist
left []string // files that must not, reported skipped
detail string // what the module's result says, when it is one module's
}{
{"of no module, the rest is attempted", []string{file("fine", "fine.conf"), doomed("doomed", "blocker"), file("after", "after.conf")},
"doomed", 0, []string{"fine.conf", "after.conf"}, nil, ""},
{"of no module, every failure is counted", []string{doomed("first", "one"), doomed("second", "two")},
"first", 1, nil, nil, ""},
{"of a module, it stops there and names each one left",
[]string{file("m.first", "first.conf"), unwritable("m.second", "blocker"), file("m.third", "third.conf"), file("m.fourth", "fourth.conf"), file("o.other", "other.conf")},
"m.second", 0, []string{"first.conf", "other.conf"}, []string{"third.conf", "fourth.conf"},
"m.second failed"},
} {
t.Run(tc.name, func(t *testing.T) {
d := parse(t, `{"declaration":1,"resources":[`+strings.Join(tc.resources, ",")+`]}`)
report, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, noServices, nil, nil)
var applyErr *Error
if !errors.As(err, &applyErr) {
t.Fatalf("expected an apply error, got %v", err)
}
if applyErr.Resource != tc.failed || applyErr.Others != tc.others {
t.Errorf("the failure names %q with %d others, want %q with %d", applyErr.Resource, applyErr.Others, tc.failed, tc.others)
}
if tc.others == 1 && !strings.Contains(err.Error(), "one other resource also failed") {
t.Errorf("the message does not say others failed: %v", err)
}
for _, name := range tc.applied {
if !exists(name) {
t.Errorf("%s was not applied", name)
}
}
for _, name := range tc.left {
if exists(name) {
t.Errorf("%s was applied after its module failed", name)
}
}
if tc.detail == "" {
return
}
outcomes := applyErr.Done.Outcomes
skipped := 0
for _, o := range outcomes {
if o.Action == "skipped" {
skipped++
}
}
if skipped != len(tc.left) {
t.Errorf("%d resources reported skipped, want %d: %+v", skipped, len(tc.left), outcomes)
}
result := ModuleResults(d, report, err)["m"]
if !result.Failed || !strings.Contains(result.Detail, tc.detail) ||
!strings.Contains(result.Detail, "m.third") || !strings.Contains(result.Detail, "m.fourth") {
t.Errorf("the module's result does not name the failed resource and each one left: %+v", result)
}
if other := ModuleResults(d, report, err)["o"]; other.Failed {
t.Errorf("another module was failed by it: %+v", other)
}
})
}
}
internal/apply/module_results.go
+7
View File
@@ -41,6 +41,7 @@ func ModuleResults(d *declaration.Declaration, report Report, err error) map[str
outcomes = append(append([]Outcome(nil), outcomes...), failed.Done.Outcomes...)
}
skipped := map[string]string{}
left := map[string][]string{}
for _, o := range outcomes {
module, ok := d.ModuleOfID(o.ID)
if !ok {
@@ -48,6 +49,7 @@ func ModuleResults(d *declaration.Declaration, report Report, err error) map[str
}
if o.Action == "skipped" {
skipped[module] = o.Detail
left[module] = append(left[module], o.ID)
continue
}
attempted[o.ID] = true
@@ -92,6 +94,11 @@ func ModuleResults(d *declaration.Declaration, report Report, err error) map[str
}
}
for module, why := range reasons {
// Each resource left unapplied is named (design 42 zfs-owns-zfs §1).
if ids := left[module]; len(ids) > 0 {
sort.Strings(ids)
why = append(why, "left unapplied: "+strings.Join(ids, ", "))
}
sort.Strings(why)
out[module] = ModuleResult{Failed: true, Detail: strings.Join(why, "; ")}
}
internal/apply/refused_directory_test.go
+3 -3
View File
@@ -118,10 +118,10 @@ func TestAFileIsJudgedByTheSameGuardAsADirectory(t *testing.T) {
if _, statErr := os.Stat(filepath.Join(home, "data", "notes.conf")); statErr == nil {
t.Error("a file was written below a directory the engine refused")
}
// A refused file has no outcome: it is a failure. Declared first, it is the failure the apply reports,
// and the directory the other.
// A refused file has no outcome: it is a failure. Declared first, it is the failure the apply reports, and the
// directory after it is left unapplied, as the rest of its module is (design 42 zfs-owns-zfs §1).
var applyErr *Error
if !errors.As(err, &applyErr) || applyErr.Resource != "notes.conf" || applyErr.Others != 1 {
if !errors.As(err, &applyErr) || applyErr.Resource != "notes.conf" || applyErr.Others != 0 {
t.Errorf("the file below a refused directory is not among the failures: %v", err)
}
if o := outcomeOf(report, "notes.conf"); o.ID != "" {