Never hold a module because a module it reads from is held: it composes with what the held one last offered (issue 1960)
mesh/delivery merged: source.main moved to novox/mesh-sdk@307b6234e6bb
mesh/delivery-group group fix/1960-no-hold-spread merged: every member is merged
mesh/check pass dbcaa29a1a38 on 394fec8a63c1 | unit 3318 run, 1951 kept, 38 skipped | integration 3994 run, 2125 kept, 38 skipped | over bound: unit, …
mesh/delivery merged: source.main moved to novox/mesh-sdk@307b6234e6bb
mesh/delivery-group group fix/1960-no-hold-spread merged: every member is merged
mesh/check pass dbcaa29a1a38 on 394fec8a63c1 | unit 3318 run, 1951 kept, 38 skipped | integration 3994 run, 2125 kept, 38 skipped | over bound: unit, …
On novox on 2026-10-29 de-spiegel, held for an unset config value, held mailu, route-proxy and nftables ("it reads
what de-spiegel contributes or listens on"), and mailu held postgres. mesh-sdk #128 (issue 1126) stopped the hold
spreading to consumers and collectors but kept dependents(): the provider of what a held module contributes to, and
the filter where it listens, were held behind it, transitively.
- Quick fix: drop dependents() alone. The filter and the proxy would then read a held module's new manifest, which
may not be what runs (review of the apply, round 2).
- Thorough fix, chosen: no module is held behind another, in the apply and in the answer (dependents, heldBehind and
the held-behind path of heldAsToday removed). A held module offers what it last offered: its contributions and
listens from its manifest at the build its own composition runs at (ranOf), else the manifest it is composed from.
Only the part of its offer that does not compose is left out, by provision (culprit): de-spiegel's routes leave
route-proxy without them, while mailu keeps its smtp account; the hold names it.
Tests: rows changed in the existing answer test (de-spiegel held, mailu and proxy composed; site held, the filter
composed with what runs; leave-out by provision). No new test function.
This commit is contained in:
@@ -104,12 +104,12 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
if part.Verdict != delivery.CandidateRefused || len(part.Refusals) != 2 || len(part.Held) != 1 || part.Held[0].Module != "api" {
|
||||
t.Errorf("held as today in part: %+v", part)
|
||||
}
|
||||
// A module held only behind one kept today is kept as today too, saying behind which (novox's mailu behind de-spiegel).
|
||||
// No module is held behind one kept today (issue 1960): mailu's own refusal stays a refusal beside de-spiegel's hold.
|
||||
if a := (delivery.CandidateAnswer{Refusals: []delivery.ModuleRefused{{Module: "de-spiegel", Why: "a value is unset"},
|
||||
{Module: "mailu", Why: heldBehind("de-spiegel")}, {Module: "proxy", Why: heldBehind("mailu")}, {Module: "web", Why: "no build"}}}); true {
|
||||
{Module: "mailu", Why: "no build"}}}); true {
|
||||
heldAsToday(&a, map[string]string{"de-spiegel": "runs a build of the old form"})
|
||||
if len(a.Refusals) != 1 || a.Refusals[0].Module != "web" || len(a.Held) != 3 || a.Held[2].Today != "held behind mailu" {
|
||||
t.Errorf("held behind a module kept today: %+v", a)
|
||||
if len(a.Refusals) != 1 || a.Refusals[0].Module != "mailu" || len(a.Held) != 1 {
|
||||
t.Errorf("held as today spreads: %+v", a)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -330,23 +330,26 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
// de-spiegel folded, and its manifest at the indexer's commit (e25e1beb, captured) asks four required config values
|
||||
// for its route names that nothing sets. novox answered the whole candidate refused ("de-spiegel contributing
|
||||
// praktijk to route says ${setting:praktijk-name} ...": its stub still contributed the route). Only de-spiegel is
|
||||
// refused, in the config form's words, and proxy, which receives its routes (what it would compose lacks them: review
|
||||
// of the apply, round 1); the rest is judged.
|
||||
// refused, in the config form's words; the rest is judged. Issue 1960, novox's chain on 2026-10-29: the hold spread
|
||||
// to mailu (its smtp provider) and proxy (its route provider), and from them on. Neither is held now: proxy composes
|
||||
// without de-spiegel's routes, which do not compose, and mailu with its smtp account, which does.
|
||||
spiegel, err := os.ReadFile("testdata/de-spiegel-e25e1beb.module.json")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
proxy := []byte(`{"module":"proxy","version":"1","capabilities":["systemd"],"provides":[{"name":"route","scope":"mesh"},{"name":"smtp","scope":"mesh"}],` +
|
||||
proxy := []byte(`{"module":"proxy","version":"1","capabilities":["systemd"],"provides":[{"name":"route","scope":"mesh"}],` +
|
||||
`"resources":[{"id":"service","type":"service","unit":"proxy.service","state":"running","boot":"enabled"}]}`)
|
||||
b = mk("lemurs", overlay.Name, "proxy", "de-spiegel")
|
||||
for m, raw := range map[string][]byte{"de-spiegel": spiegel, "proxy": proxy} {
|
||||
mailu := []byte(`{"module":"mailu","version":"1","capabilities":["systemd"],"provides":[{"name":"smtp","scope":"mesh"}],` +
|
||||
`"resources":[{"id":"service","type":"service","unit":"mailu.service","state":"running","boot":"enabled"}]}`)
|
||||
b = mk("lemurs", overlay.Name, "mailu", "proxy", "de-spiegel")
|
||||
for m, raw := range map[string][]byte{"de-spiegel": spiegel, "proxy": proxy, "mailu": mailu} {
|
||||
b.manifests[delivery.ManifestDigest(raw)] = raw
|
||||
b.modules[m] = delivery.Module{Manifest: delivery.ManifestDigest(raw)}
|
||||
}
|
||||
p = b.placements["n1"]
|
||||
p.Decided.Endpoint = "n1.example:51820"
|
||||
b.placements["n1"] = p
|
||||
fps = map[string]string{"lemurs": fp("e"), "proxy": fp("f")}
|
||||
fps = map[string]string{"lemurs": fp("e"), "proxy": fp("f"), "mailu": fp("d")}
|
||||
a = Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), InOwn: []string{"de-spiegel"}}, fps, nil, "p-h1", 3)
|
||||
refused = nil
|
||||
for _, r := range a.Refusals {
|
||||
@@ -355,11 +358,13 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
t.Errorf("de-spiegel is refused in other words than its config form's: %s", r.Why)
|
||||
}
|
||||
}
|
||||
if a.Refused != "" || !reflect.DeepEqual(refused, []string{"de-spiegel", "proxy"}) {
|
||||
t.Errorf("a module's own refusal: %+v, want de-spiegel refused with proxy, which receives its routes, the node not", a)
|
||||
if a.Refused != "" || !reflect.DeepEqual(refused, []string{"de-spiegel"}) {
|
||||
t.Errorf("a module's own refusal: %+v, want de-spiegel refused alone, the node, mailu and proxy not", a)
|
||||
}
|
||||
if !slices.ContainsFunc(a.Added, func(m delivery.ModuleAdded) bool { return m.Module == "lemurs" }) {
|
||||
t.Errorf("lemurs, which composes, is not judged beside de-spiegel's refusal: %+v", a)
|
||||
for _, m := range []string{"lemurs", "mailu", "proxy"} {
|
||||
if !slices.ContainsFunc(a.Added, func(x delivery.ModuleAdded) bool { return x.Module == m }) {
|
||||
t.Errorf("%s, which composes, is not judged beside de-spiegel's refusal: %+v", m, a)
|
||||
}
|
||||
}
|
||||
|
||||
// (E) Issue 1126, the applying path (review of the seed): the composition novox applies follows the same rule. It
|
||||
@@ -393,16 +398,13 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("de-spiegel's own refusal refuses the node's whole composition: %v", err)
|
||||
}
|
||||
if len(held) != 2 || !strings.Contains(held["de-spiegel"], "${config:praktijk-name}") || !strings.Contains(held["proxy"], "de-spiegel") {
|
||||
t.Errorf("held %v, want de-spiegel in its config form's words, and proxy for its routes", held)
|
||||
if len(held) != 1 || !strings.Contains(held["de-spiegel"], "${config:praktijk-name}") || !strings.Contains(held["de-spiegel"], "without what it contributes to route") {
|
||||
t.Errorf("held %v, want de-spiegel alone, in its config form's words, composed without its routes", held)
|
||||
}
|
||||
lemursKey, _ := delivery.ComposedKey("n1", "lemurs", fps["lemurs"])
|
||||
proxyKey, _ := delivery.ComposedKey("n1", "proxy", fps["proxy"])
|
||||
if _, ok := got[lemursKey]; !ok {
|
||||
t.Errorf("lemurs, which composes, is not composed beside de-spiegel's refusal: %v", sortedKeys(got))
|
||||
}
|
||||
if _, ok := got[proxyKey]; ok {
|
||||
t.Errorf("proxy is composed without the routes of held de-spiegel: %v", sortedKeys(got))
|
||||
for _, m := range []string{"lemurs", "mailu", "proxy"} {
|
||||
if key, _ := delivery.ComposedKey("n1", m, fps[m]); got[key].Module != m {
|
||||
t.Errorf("%s is not composed beside de-spiegel's hold: %v", m, sortedKeys(got))
|
||||
}
|
||||
}
|
||||
carries := func(c delivery.Composed, what string) {
|
||||
t.Helper()
|
||||
@@ -434,7 +436,7 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
// (G) A held module that runs a composition of its own (review of the apply, round 1, finding 2): what the others
|
||||
// compose of it is what runs, never its stub's. site listens on 8080, placed at the machine's 20005, and its own
|
||||
// secret was never issued here: held, the filter must not open 8080 and drop the 20005 that runs. The filter, which
|
||||
// reads what site's manifest listens on, is held with it (dependents).
|
||||
// reads what site listens on, composes with it (issue 1960: a hold never spreads).
|
||||
site := []byte(`{"module":"site","version":"1","capabilities":["container-runtime"],` +
|
||||
`"listens":[{"port":8080,"from":"mesh","why":"the site"}],"own-secrets":{"key":"${dir:state}/key"},` +
|
||||
`"resources":[{"id":"state","type":"directory","path":"/var/lib/site","mode":"0700"},` +
|
||||
@@ -460,8 +462,12 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
json.RawMessage(`{"id":"site.state","module":"site","type":"directory","path":"/var/lib/site","mode":"0700"}`),
|
||||
json.RawMessage(`{"id":"site.server","module":"site","type":"container","name":"site","image":"registry.example/site@sha256:` + strings.Repeat("c", 64) + `","ports":["10.77.0.1:20005:8080"]}`)}}
|
||||
got, _, held, err = Compose(ctx, b, Mesh{}, Scope{Node: "n1", Running: []delivery.Composed{siteRuns}}, fps)
|
||||
if err != nil || held["site"] == "" || held["nft"] == "" {
|
||||
t.Fatalf("site, its own secret never issued: %v, held %v; want site held, and the filter, which depends on it", err, held)
|
||||
nftKey, _ := delivery.ComposedKey("n1", "nft", fps["nft"])
|
||||
if err != nil || held["site"] == "" || held["nft"] != "" || got[nftKey].Module != "nft" {
|
||||
t.Fatalf("site, its own secret never issued: %v, held %v; want site held alone, and the filter composed", err, held)
|
||||
}
|
||||
if rules, _ := json.Marshal(got[nftKey].Resources); !strings.Contains(string(rules), "dport 20005") {
|
||||
t.Errorf("the filter drops held site's opening where it runs (20005): %s", rules)
|
||||
}
|
||||
for key, c := range got {
|
||||
for _, raw := range c.Resources {
|
||||
@@ -501,26 +507,25 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
t.Errorf("app's binding does not reach held site where it runs (20005): %s", bound)
|
||||
}
|
||||
delete(fps, "app")
|
||||
// A held feeder of a collector holds no collector: its items reach their collection through its own plan, which it
|
||||
// keeps as it runs. Only a provider it contributes to in that provider's own files is held behind it (the proxy its
|
||||
// routes), as is the filter where it listens.
|
||||
withCollector := compose.Input{Catalogue: map[string]catalogue.Manifest{
|
||||
"feeder": {Module: "feeder", Items: []catalogue.Item{{Collection: "routes", Content: "x"}}},
|
||||
"col": {Module: "col", Collections: []catalogue.Collection{{Name: "routes"}}},
|
||||
"proxy": {Module: "proxy", Provides: []catalogue.Offer{{Name: "route"}}}},
|
||||
Nodes: []compose.Node{{Name: "n1", Assigned: []string{"feeder", "col", "proxy"}}}}
|
||||
feeder := withCollector.Catalogue["feeder"]
|
||||
if d := dependents(withCollector, "n1", feeder, map[string]string{"feeder": "held"}); len(d) != 0 {
|
||||
t.Errorf("a held feeder of a collector holds %v, want none", d)
|
||||
}
|
||||
feeder.Contributes = map[string]map[string]any{"route": {"name": "x"}}
|
||||
if d := dependents(withCollector, "n1", feeder, map[string]string{"feeder": "held"}); !reflect.DeepEqual(d, []string{"proxy"}) {
|
||||
t.Errorf("a held module contributing routes holds %v, want the proxy", d)
|
||||
// A held module's offer is left out by provision, never whole when one part composes (issue 1960): de-spiegel's
|
||||
// routes, its names unset, leave the proxy without them, and its smtp account stays with mailu.
|
||||
withProxy := compose.Input{Catalogue: map[string]catalogue.Manifest{},
|
||||
Nodes: []compose.Node{{Name: "n1", Assigned: []string{"feeder"}}}}
|
||||
feeder := catalogue.Manifest{Module: "feeder", Contributes: map[string]map[string]any{"route": {"name": "x"}, "smtp": {"account": "feeder"}},
|
||||
OwnSecrets: catalogue.OwnSecrets{"key": {Path: "/k"}}}
|
||||
for _, row := range []struct {
|
||||
left map[string]bool
|
||||
route, smtp, own bool
|
||||
}{{nil, true, true, true}, {map[string]bool{"route": true}, false, true, true}, {map[string]bool{leaveAll: true}, false, false, false}} {
|
||||
k := heldAs(withProxy, "n1", "feeder", feeder, row.left, nil).Nodes[0].Kept["feeder"]
|
||||
if _, r := k.Contributes["route"]; r != row.route || (k.Contributes["smtp"] != nil) != row.smtp || (len(k.OwnSecrets) > 0) != row.own {
|
||||
t.Errorf("held leaving out %v offers %+v, own secrets %v", row.left, k.Contributes, k.OwnSecrets)
|
||||
}
|
||||
}
|
||||
// (H) Review of the apply, round 2: a held module is composed from its new manifest, so what the others read of its
|
||||
// manifest (its listens, what it contributes) may not be what runs. site's new manifest moves it to 9090 while the
|
||||
// build that runs listens on 8080: the filter, the holder of what site listens on, is held with it, and nothing
|
||||
// applied opens 9090.
|
||||
// build that runs (7) listens on 8080. The filter composes with what site last offered, its manifest at the build
|
||||
// that runs (issue 1960: a hold never spreads), so it opens 8080 and nothing applied opens 9090.
|
||||
moved := []byte(strings.Replace(string(site), `"port":8080`, `"port":9090`, 1))
|
||||
b.manifests[delivery.ManifestDigest(moved)] = moved
|
||||
b.modules["site"] = delivery.Module{Manifest: delivery.ManifestDigest(moved)}
|
||||
@@ -529,9 +534,15 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) {
|
||||
}
|
||||
b.placements["n1"] = p
|
||||
siteRuns.Resources[1] = json.RawMessage(`{"id":"site.server","module":"site","type":"container","name":"site","image":"registry.example/site@sha256:` + strings.Repeat("c", 64) + `","ports":["8080"]}`)
|
||||
siteRuns.FP = fp("7")
|
||||
b.manifests[delivery.ManifestDigest(site)] = site
|
||||
b.builds["site "+fp("7")] = &delivery.BuilderBuild{State: delivery.BuildDone, Manifest: delivery.ManifestDigest(site)}
|
||||
got, _, held, err = Compose(ctx, b, Mesh{}, Scope{Node: "n1", Running: []delivery.Composed{siteRuns}}, fps)
|
||||
if err != nil || held["site"] == "" || held["nft"] == "" {
|
||||
t.Errorf("site moved to 9090 in its new manifest: %v, held %v; want site held, and the filter with it", err, held)
|
||||
if err != nil || held["site"] == "" || held["nft"] != "" {
|
||||
t.Errorf("site moved to 9090 in its new manifest: %v, held %v; want site held alone", err, held)
|
||||
}
|
||||
if rules, _ := json.Marshal(got[nftKey].Resources); !strings.Contains(string(rules), "dport 8080") {
|
||||
t.Errorf("the filter drops what held site last offered (8080): %s", rules)
|
||||
}
|
||||
for key, c := range got {
|
||||
for _, raw := range c.Resources {
|
||||
|
||||
Reference in New Issue
Block a user