Merge pull request 'frombus: blame only the modules a refusal names, and say which providers another node cannot run (issue 1126, group fix/1126-shadow-bodies)' (#113) from fix/1126-shadow-bodies into main

go/compose/frombus/answer.go

@@ -212,6 +212,9 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st
 	return a
 }
 
+// maxCulprits bounds the modules one refusal is tried on, each a whole composition.
+const maxCulprits = 8
+
 // MaxShown bounds the bytes of shown resource bodies in one answer (delivery.ModuleDiffers.Bodies).
 const MaxShown = 256 << 10
 
@@ -251,15 +254,12 @@ func culprit(in compose.Input, node string, fps map[string]string, failed map[st
 			assigned = n.Assigned
 		}
 	}
-	// The modules the error names first: the words of a module's refusal open with its name.
-	order := make([]string, 0, len(assigned))
+	// Only the modules the error names: the words of a module's refusal name it. Each try is a whole composition, so a
+	// refusal naming none is no module's, answered at once rather than after a composition per module (issue 1126: ace's
+	// answer took 7m46s).
+	var order []string
 	for _, m := range assigned {
-		if strings.Contains(err.Error(), m) {
-			order = append(order, m)
-		}
-	}
-	for _, m := range assigned {
-		if !strings.Contains(err.Error(), m) {
+		if strings.Contains(err.Error(), m) && len(order) < maxCulprits {
 			order = append(order, m)
 		}
 	}

go/compose/frombus/answer_rows_test.go

@@ -3,6 +3,7 @@ package frombus
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"slices"
 	"strings"
@@ -107,6 +108,11 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	// A refusal naming no module placed here is no module's: no compose is spent stubbing each in turn (issue 1126: ace's
+	// answer took 7m46s, one compose per module), though here a stub would compose.
+	if m := culprit(in, "n1", fps, map[string]string{}, fmt.Errorf("nothing in this mesh provides \"artifact-store\"")); m != "" {
+		t.Errorf("a refusal naming no placed module was laid on %s", m)
+	}
 	ownKey, _ := delivery.ComposedNodeKey("n1")
 	own := base[ownKey]
 	carriedRes := []json.RawMessage{
@@ -161,6 +167,32 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
 	}
 }
 
+// A module another node cannot run in this composition offers nothing in it: the refusal names it and why
+// (issue 1126, probe #532: ace and g14 answered "nothing in this mesh provides artifact-store … assign distribution to a
+// node" while novox runs it). And a refusal no module's stub changes is answered at once, not after a compose per module
+// (ace took 7m46s).
+func otherNodeNotResolvedIsNamed(t *testing.T) {
+	ctx := context.Background()
+	prov := []byte(`{"module":"prov","version":"1","capabilities":["container-runtime"],"provides":[{"name":"thing","scope":"mesh"}]}`)
+	cons := []byte(`{"module":"cons","version":"1","requires":["thing"]}`)
+	// n2 runs prov, which needs a container runtime n2's record does not say it has: n2's set does not resolve here.
+	b := &bus{placements: map[string]delivery.Placement{
+		"n1": {Node: "n1", Decided: delivery.NodeDecided{Hub: true, Address: "10.77.0.1", Endpoint: "n1.example:51820"}, Modules: []delivery.PlacedModule{{Module: "cons"}, {Module: overlay.Name}}},
+		"n2": {Node: "n2", Decided: delivery.NodeDecided{Address: "10.77.0.2"}, Modules: []delivery.PlacedModule{{Module: "prov"}, {Module: overlay.Name}}}},
+		records:   map[string]delivery.NodeRecord{"n1": {Node: "n1", TunnelKey: publicKey(t)}, "n2": {Node: "n2", TunnelKey: publicKey(t)}},
+		modules:   map[string]delivery.Module{"prov": {Manifest: delivery.ManifestDigest(prov)}, "cons": {Manifest: delivery.ManifestDigest(cons)}},
+		manifests: map[string][]byte{delivery.ManifestDigest(prov): prov, delivery.ManifestDigest(cons): cons}}
+	given := map[string]*delivery.Placement{}
+	for n, p := range b.placements {
+		p := p
+		given[n] = &p
+	}
+	a := Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given}, map[string]string{"cons": "src2:" + strings.Repeat("1", 64)}, nil, "h", 1)
+	if a.Verdict != delivery.CandidateRefused || !strings.Contains(a.Refused, "prov on n2 lacks container-runtime") {
+		t.Errorf("a refusal for want of n2's provider = %+v", a)
+	}
+}
+
 // A node that reaches no artifact store says what its composition holds of one (issue 1126: ace and g14 answered
 // "reaches no artifact store", and nothing said why).
 func noStoreReachedSaysWhy(t *testing.T) {

go/compose/frombus/frombus_test.go

@@ -123,6 +123,7 @@ func TestInput(t *testing.T) {
 	// saying why (answer_rows_test.go).
 	t.Run("every refusal, and an old build left to the node", answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode)
 	t.Run("no store reached says why", noStoreReachedSaysWhy)
+	t.Run("another node not resolved is named", otherNodeNotResolvedIsNamed)
 	ctx := context.Background()
 	xterm, err := os.ReadFile("../../catalogue/testdata/catalogue-835bf22b-xterm.module.json")
 	if err != nil {

go/compose/network.go

@@ -24,6 +24,9 @@ type composer struct {
 	in   Input
 	said []string
 	seen map[string]bool
+	// unhostable is, from the last look at the rest of the mesh, each module assigned to another node that this
+	// composition finds that node cannot run: what it would have offered is missing here (issue 1126, probe #532).
+	unhostable []string
 }
 
 func newComposer(in Input) *composer { return &composer{in: in, seen: map[string]bool{}} }

go/compose/resolve.go

@@ -3,6 +3,7 @@ package compose
 import (
 	"fmt"
 	"slices"
+	"strings"
 
 	"git.novox.be/novox/mesh-sdk/go/catalogue"
 )
@@ -203,7 +204,7 @@ func (c *composer) resolution(nodeName string) (catalogue.Resolution, error) {
 		return catalogue.Resolution{}, err
 	}
 	shelf := c.shelfFor(node)
-	world, _, _, err := c.theRestOfTheMesh(nodeName)
+	world, _, left, err := c.theRestOfTheMesh(nodeName)
 	if err != nil {
 		return catalogue.Resolution{}, err
 	}
@@ -226,6 +227,19 @@ func (c *composer) resolution(nodeName string) (catalogue.Resolution, error) {
 			Account: node.Account, AccountHome: node.AccountHome,
 			AgentAccount: node.AgentAccount, AgentAccountHome: node.AgentAccountHome}, world)
 	if err != nil {
+		// What another node's set would have offered is missing when that set does not resolve here: said with the
+		// refusal, so "nothing provides" is read beside why (issue 1126, probe #532).
+		if len(left) > 0 {
+			var why []string
+			for _, l := range left {
+				why = append(why, l.String())
+			}
+			err = fmt.Errorf("%w\n\nand %d node(s) offer nothing here, as they could not be resolved: %s", err, len(left), strings.Join(why, "; "))
+		}
+		if len(c.unhostable) > 0 {
+			err = fmt.Errorf("%w\n\nand these modules offer nothing here, as their node cannot run them in this composition: %s",
+				err, strings.Join(c.unhostable, "; "))
+		}
 		// The node's own set does not compose: the only failure a mesh-wide gatherer may pass over.
 		return catalogue.Resolution{}, notResolvable{err}
 	}

go/compose/rest.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"sort"
+	"strings"
 
 	"git.novox.be/novox/mesh-sdk/go/catalogue"
 )
@@ -100,6 +101,7 @@ func (c *composer) theRestOfTheMesh(exclude string) (catalogue.World, Passes, []
 	var left []LeftOut
 	offered := map[string][]catalogue.Provider{}
 	var firstHeld []catalogue.Held
+	c.unhostable = nil
 	for _, o := range others {
 		got, err := catalogue.Resolve(shelf, o.assigned, o.node, catalogue.World{Unchecked: true, Holdings: holdings})
 		if err != nil {
@@ -107,6 +109,9 @@ func (c *composer) theRestOfTheMesh(exclude string) (catalogue.World, Passes, []
 			left = append(left, LeftOut{Node: o.node.Name, Err: err})
 			continue
 		}
+		for _, u := range got.Unhostable {
+			c.unhostable = append(c.unhostable, fmt.Sprintf("%s on %s lacks %s", u.Module, o.node.Name, strings.Join(u.Missing, ", ")))
+		}
 		firstHeld = append(firstHeld, got.Claims...)
 		for _, m := range got.Modules {
 			for _, name := range m.OffersAt(catalogue.ScopeMesh) {