diff --git a/go/compose/frombus/answer.go b/go/compose/frombus/answer.go index cefd190..078dbbc 100644 --- a/go/compose/frombus/answer.go +++ b/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) } } diff --git a/go/compose/frombus/answer_rows_test.go b/go/compose/frombus/answer_rows_test.go index 2a361af..74b1601 100644 --- a/go/compose/frombus/answer_rows_test.go +++ b/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) { diff --git a/go/compose/frombus/frombus_test.go b/go/compose/frombus/frombus_test.go index ee7697d..e02fa56 100644 --- a/go/compose/frombus/frombus_test.go +++ b/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 { diff --git a/go/compose/network.go b/go/compose/network.go index afc8363..f51d6d7 100644 --- a/go/compose/network.go +++ b/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{}} } diff --git a/go/compose/resolve.go b/go/compose/resolve.go index d2fffd8..a481909 100644 --- a/go/compose/resolve.go +++ b/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} } diff --git a/go/compose/rest.go b/go/compose/rest.go index f7509e2..a5caf70 100644 --- a/go/compose/rest.go +++ b/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) {