Review: a failure is the same by resource id, not by the host's words; bound files and /run/docker.sock are declared

The host's error text may carry a duration or a counter, and a resource looping on
it would never have read as stuck. The previous row is read and compared here.
Stuck needs a start to say. A container may mount the file a binding lands in; the
runtime socket is declared under both of its spellings; the catalogue-wide test
takes MESH_CATALOG.

internal/catalogue/manifest.go

@@ -1098,7 +1098,8 @@ func (m Manifest) MachineSide(port int) (at int, mayAssign bool) {
 // facilitiesOf is what each capability lets a container mount: paths the machine owns and a module
 // is granted the use of by declaring the capability, never by declaring them as its own.
 var facilitiesOf = map[string][]string{
-	"container-runtime": {"/var/run/docker.sock"},
+	// Both spellings: /var/run is a link to /run on every machine the mesh runs on.
+	"container-runtime": {"/var/run/docker.sock", "/run/docker.sock"},
 }
 
 // undeclaredMounts is every bind-mount source no declaration covers — see the check above.
@@ -1127,6 +1128,9 @@ func (m Manifest) undeclaredMounts() []string {
 	for _, where := range m.Grants {
 		claim(where)
 	}
+	for _, where := range m.Binds {
+		claim(where)
+	}
 	for _, a := range m.Accesses {
 		claim(a.Path)
 	}

internal/catalogue/mounts_test.go

@@ -70,9 +70,13 @@ func TestTheRuntimeSocketIsGrantedByTheCapabilityAndNotOtherwise(t *testing.T) {
 // **Every manifest in the catalogue beside this checkout passes**, so the rule is not one the
 // catalogue is already breaking. Skipped, aloud, where the catalogue is not there.
 func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) {
-	files, _ := filepath.Glob("../../../mesh-catalog/modules/*/module.json")
+	root := os.Getenv("MESH_CATALOG")
+	if root == "" {
+		root = "../../../mesh-catalog"
+	}
+	files, _ := filepath.Glob(filepath.Join(root, "modules", "*", "module.json"))
 	if len(files) == 0 {
-		t.Skip("the catalogue is not beside this checkout")
+		t.Skipf("no catalogue at %s (set MESH_CATALOG to a checkout)", root)
 	}
 	for _, file := range files {
 		raw, err := os.ReadFile(file)
@@ -84,3 +88,13 @@ func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) {
 		}
 	}
 }
+
+// Where a bound fact lands is the mesh's file too, and a container may mount it directly.
+func TestAMountOfABoundFactIsAccepted(t *testing.T) {
+	_, err := ParseManifest([]byte(`{"module":"store","requires":["model-access"],` +
+		`"binds":{"model-access":"/var/lib/store/model.json"},"resources":[` +
+		strings.Replace(aContainerMounting, "%s", "/var/lib/store/model.json", 1) + `]}`))
+	if err != nil {
+		t.Fatalf("a mount of the file the mesh writes a binding to was refused: %v", err)
+	}
+}

internal/inventory/doing_test.go

@@ -284,6 +284,20 @@ func TestTheSameFailureReportedAgainIsCountedNotRestarted(t *testing.T) {
 		t.Fatalf("the failure began at %s and the row now says %s", *first.Since, *again.Since)
 	}
 
+	// The same resource failing with different words — a duration, a counter — is still the same
+	// failure: it is the resource that loops, not the sentence.
+	reworded := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "img", Error: "no such image (after 31s)"}}}
+	if err := inv.RecordDoing(ctx, id, reworded); err != nil {
+		t.Fatal(err)
+	}
+	still, _, err := inv.DoingOf(ctx, "looping")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if still.Times != StuckAfter+1 || !still.Stuck() {
+		t.Fatalf("the same resource failing in other words restarted the count: %+v", still)
+	}
+
 	// A different failure is a new situation, not a longer one.
 	other := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "svc", Error: "unit not found"}}}
 	if err := inv.RecordDoing(ctx, id, other); err != nil {

internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql

@@ -7,8 +7,9 @@
 -- when its dependency arrives" from "failed identically for ever", and nothing escalated the second.
 --
 -- Still one row per node. What is added is how long the CURRENT failure has been the same one:
--- when it first appeared, and how many reports in a row have said it. A report that says something
--- different starts the count again; a clean apply clears it.
+-- when it first appeared, and how many reports in a row have said it -- the same outcome, the same
+-- refusal, the same failed resources by id (not by the host's words, which may carry a duration).
+-- A report that says something different starts the count again; a clean apply clears it.
 
 alter table node_report
     add column failing_since timestamptz,

internal/inventory/nodes.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 	"time"
 
@@ -468,7 +469,7 @@ type Doing struct {
 	Declared string
 
 	// Since is when the machine first reported THIS failure — the same outcome, the same refusal,
-	// the same failed resources — and Times is how many reports in a row have said it. A node
+	// the same failed resources by id — and Times is how many reports in a row have said it. A node
 	// re-applies on a steady interval and reports each time (novox/hq ADR 0010), so a failure
 	// that will never succeed arrives as the same report over and over, indistinguishable from
 	// one that just happened until somebody counts (novox/hq 04-ISSUES/065). Nil and zero for a
@@ -496,7 +497,32 @@ const StuckAfter = 3
 
 // Stuck reports whether this machine has been failing the same way for long enough that waiting
 // is no longer a plan. The failure is still the host's own words; this only says it is not new.
-func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter }
+func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter && d.Since != nil }
+
+// sameFailure is whether two reports describe one failure: the same outcome, the same refusal, and
+// the same failed resources BY ID. Not by the host's words: an error that carries a duration, a
+// counter or a temporary path would read as new on every report, and the resource looping on it —
+// which is what stuck is for — would never be said to be (novox/hq 04-ISSUES/065).
+func sameFailure(a, b Doing) bool {
+	if a.Outcome != b.Outcome || a.Refused != b.Refused || len(a.Failed) != len(b.Failed) {
+		return false
+	}
+	ids := func(d Doing) []string {
+		out := make([]string, 0, len(d.Failed))
+		for _, f := range d.Failed {
+			out = append(out, f.ID)
+		}
+		sort.Strings(out)
+		return out
+	}
+	x, y := ids(a), ids(b)
+	for i := range x {
+		if x[i] != y[i] {
+			return false
+		}
+	}
+	return true
+}
 
 // RecordDoing keeps what a node said it did.
 //
@@ -505,35 +531,49 @@ func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter }
 // would bury the ones that matter.
 //
 // **What the row also keeps is whether this failure is the one before.** The same outcome, the
-// same refusal, the same failed resources with the same words: then the failure did not just
-// happen, it is still happening, and the row keeps when it began and counts one more report. Any
-// difference starts again — a machine failing on a new resource is a new situation, not a longer
-// one — and a clean apply clears both (novox/hq 04-ISSUES/065).
+// same refusal, the same failed resources by id: then the failure did not just happen, it is
+// still happening, and the row keeps when it began and counts one more report. Any difference
+// starts again — a machine failing on a new resource is a new situation, not a longer one — and
+// a clean apply clears both (novox/hq 04-ISSUES/065). The previous row is read first and the
+// comparison made here, so "the same" is a rule this package states rather than a jsonb equality
+// that would restart the count on a changed word in an error.
 func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) error {
 	failed, err := json.Marshal(d.Failed)
 	if err != nil {
 		return err
 	}
+	var since *time.Time
+	times := 0
+	if d.Outcome != OutcomeApplied {
+		var before Doing
+		var beforeFailed []byte
+		err := i.store.Pool().QueryRow(ctx,
+			`select outcome, refused, failed, failing_since, failures from node_report where node = $1`,
+			node).Scan(&before.Outcome, &before.Refused, &beforeFailed, &before.Since, &before.Times)
+		switch {
+		case errors.Is(err, pgx.ErrNoRows):
+		case err != nil:
+			return err
+		default:
+			if err := json.Unmarshal(beforeFailed, &before.Failed); err != nil {
+				return err
+			}
+		}
+		now := time.Now()
+		since, times = &now, 1
+		if err == nil && sameFailure(before, d) && before.Since != nil {
+			since, times = before.Since, before.Times+1
+		}
+	}
 	_, err = i.store.Pool().Exec(ctx,
 		`insert into node_report (node, outcome, refused, failed, applied, at, declared,
 		                          failing_since, failures)
-		 values ($1, $2, $3, $4, $5, now(), $6,
-		         case when $2 <> $7 then now() end,
-		         case when $2 <> $7 then 1 else 0 end)
+		 values ($1, $2, $3, $4, $5, now(), $6, $7, $8)
 		 on conflict (node) do update set outcome = excluded.outcome, refused = excluded.refused,
 		   failed = excluded.failed, applied = excluded.applied, at = excluded.at,
 		   declared = excluded.declared,
-		   failing_since = case
-		     when excluded.outcome = $7 then null
-		     when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused
-		          and node_report.failed = excluded.failed then node_report.failing_since
-		     else excluded.at end,
-		   failures = case
-		     when excluded.outcome = $7 then 0
-		     when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused
-		          and node_report.failed = excluded.failed then node_report.failures + 1
-		     else 1 end`,
-		node, d.Outcome, d.Refused, failed, d.Applied, d.Declared, OutcomeApplied)
+		   failing_since = excluded.failing_since, failures = excluded.failures`,
+		node, d.Outcome, d.Refused, failed, d.Applied, d.Declared, since, times)
 	return err
 }