From 77af82cfa2fc537a0c60bf95e75823e14432ade3 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 16:11:07 +0200 Subject: [PATCH] Keep the asks state and write the ask about a held fingerprint, so the view's and the teller's read of mesh-delivery-assigner.asks is granted and has something to answer (issue 1297, review round 1 item 1; ADR 0528 clause 6) --- .../integration_test.go | 32 +++++++++++++++++-- .../cmd/mesh-delivery-assigner/main.go | 7 ++-- .../cmd/mesh-delivery-assigner/pass.go | 15 +++++++++ .../internal/assign/asks.go | 29 +++++++++++++++++ .../internal/assign/asks_test.go | 28 ++++++++++++++++ .../mesh-delivery-assigner/manifest_test.go | 2 +- modules/mesh-delivery-assigner/module.json | 1 + 7 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 modules/mesh-delivery-assigner/internal/assign/asks.go create mode 100644 modules/mesh-delivery-assigner/internal/assign/asks_test.go diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go index 2682b37..6a79f7c 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go @@ -99,7 +99,7 @@ func (b *bed) start(t *testing.T) <-chan error { return done } -// 1297/B2b: started whole on its credential, the assigner says on the bus the three states it keeps, each readable, +// 1297/B2b: started whole on its credential, the assigner says on the bus the four states it keeps, each readable, // having opened every bucket it names. func TestTheAssignerStartsAndSaysWhatItKeeps(t *testing.T) { b := startBed(t, "") @@ -124,9 +124,9 @@ func TestTheAssignerStartsAndSaysWhatItKeeps(t *testing.T) { } break } - if said.Module != "mesh-delivery-assigner" || said.Node != "n1" || len(said.States) != 3 || + if said.Module != "mesh-delivery-assigner" || said.Node != "n1" || len(said.States) != 4 || !said.Readable("asks") || !said.Readable("wants") || !said.Readable("differences") || !said.Readable("compositions") { - t.Fatalf("the assigner said %+v, want its three states, each readable", said) + t.Fatalf("the assigner said %+v, want its four states, each readable", said) } } @@ -274,3 +274,29 @@ func TestWantsAndTheirDifferencesOnTheBus(t *testing.T) { t.Errorf("g14, held back by the canary with no want before, was given one: %s", e.Value()) } } + +// 1297/B9 (the assigner's half, ADR 0528 clause 6): started whole, the assigner holds a fingerprint whose health check +// failed on a node and writes the ask about it in its own asks, which the view and the teller read; its asks bucket is +// the one its manifest declares (ownBuckets, held equal to the manifest by TestOpensWhatItsManifestNames). +func TestAHeldFingerprintIsAskedAbout(t *testing.T) { + b := startBed(t, "") + fNew := delivery.SourcePrefix2 + strings.Repeat("c", 64) + digest := b.manifest(t, "web", `{"module":"web","version":"1"}`) + mod, _ := delivery.EncodeModule(delivery.Module{FP2: fNew, Manifest: digest, Repository: "novox/web", Commit: strings.Repeat("1", 40), Path: delivery.RootPath}) + b.putJSON(t, delivery.ModulesBucket, "web", mod) + on, _ := delivery.EncodePresence(delivery.Presence{Online: true}) + b.putJSON(t, delivery.PresenceBucket, "ace", on) + failed, _ := delivery.EncodeHave(delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthFailed, Detail: "connection refused"}) + b.putJSON(t, delivery.HaveBucket, "ace.web", failed) + b.putJSON(t, provider.AssignmentsBucket, "ace.web", []byte(`{"node":"ace","module":"web","build":"b-1"}`)) + b.start(t) + hkey, _ := delivery.HeldKey("web", fNew) + if h, err := delivery.DecodeHeld(b.get(t, delivery.AssignerWantsBucket, hkey)); err != nil || h.Node != "ace" { + t.Fatalf("held = %+v, %v", h, err) + } + akey, _ := delivery.HoldAskKey("web", fNew) + a, err := delivery.DecodeAssignerAsk(akey, b.get(t, delivery.AssignerAsksBucket, akey)) + if err != nil || a.Ask.ID != delivery.AskingID(akey, 0) || len(a.Ask.Options) != 2 { + t.Fatalf("the ask = %+v, %v", a, err) + } +} diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go index dc34b09..8900bd8 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/main.go @@ -50,11 +50,13 @@ var readBuckets = []string{ } // ownBuckets are the assigner's own states, as its manifest declares them. -var ownBuckets = []string{delivery.AssignerCompositionsBucket, delivery.AssignerDifferencesBucket, delivery.AssignerWantsBucket} +var ownBuckets = []string{delivery.AssignerAsksBucket, delivery.AssignerCompositionsBucket, delivery.AssignerDifferencesBucket, delivery.AssignerWantsBucket} // keptStates is what the assigner keeps and who may read it (ADR 0495 §2-§3): its wants by the watchman and the view -// (and the node agent from release 6), its differences by the view, its compositions by the checker. +// (and the node agent from release 6), its differences by the view, its compositions by the checker, its asks by the view +// and the teller. var keptStates = []announce.State{ + {Name: "asks", Readable: true}, {Name: "compositions", Readable: true}, {Name: "differences", Readable: true}, {Name: "wants", Readable: true}, @@ -135,6 +137,7 @@ var watched = []string{ delivery.ModulesBucket, delivery.PresenceBucket, delivery.HaveBucket, + delivery.AssignerAsksBucket, delivery.AssignerCompositionsBucket, delivery.AssignerDifferencesBucket, delivery.AssignerWantsBucket, diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 754a812..a626f95 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -137,6 +137,9 @@ func (p *passer) wants(ctx context.Context) map[string]map[string]assign.Holding p.put(ctx, delivery.AssignerWantsBucket, key, func() ([]byte, error) { return delivery.EncodeHeld(held) }) } } + if out.Held != nil || m.Held { + p.holdAsk(ctx, name, fp) + } for node, want := range out.Wants { key, err := delivery.WantKey(node, name) if err != nil { @@ -205,3 +208,15 @@ func (p *passer) put(ctx context.Context, bucket, key string, encode func() ([]b } p.w.put(bucket, key, value) } + +// holdAsk writes, once, the ask about a held fingerprint (ADR 0528 clause 6, ADR 0437 clause 10): the first ask that needs +// no proof of who answered, which the view may answer and the teller fills. In release 5 an answer changes nothing on a +// node (wants are only compared, ADR 0429 §7), and the options say so. +func (p *passer) holdAsk(ctx context.Context, module, fp string) { + key, err := delivery.HoldAskKey(module, fp) + if err != nil || p.w.has(delivery.AssignerAsksBucket, key) { + return + } + a := assign.HoldAsk(key, module, fp, time.Now()) + p.put(ctx, delivery.AssignerAsksBucket, key, func() ([]byte, error) { return delivery.EncodeAssignerAsk(key, a) }) +} diff --git a/modules/mesh-delivery-assigner/internal/assign/asks.go b/modules/mesh-delivery-assigner/internal/assign/asks.go new file mode 100644 index 0000000..52d7cb7 --- /dev/null +++ b/modules/mesh-delivery-assigner/internal/assign/asks.go @@ -0,0 +1,29 @@ +package assign + +import ( + "fmt" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// HoldAsk is the ask about a module's fingerprint the assigner holds, at its key (delivery.HoldAskKey), first asking: keep +// it held or release it. Both options acknowledge and bind nothing: in release 5 the wants are only compared, so no answer +// changes what a node runs (ADR 0429 §7); the node agent acts on the hold from release 6. +func HoldAsk(key, module, fp string, now time.Time) delivery.AssignerAsk { + return delivery.AssignerAsk{Ask: asks.Ask{ + ID: delivery.AskingID(key, 0), + Headline: fmt.Sprintf("%s is held at %s", module, short(fp)), + Explanation: fmt.Sprintf("A node's health check of %s failed at %s, so the assigner holds that version and wants "+ + "no node to move to it. Until release 6 this changes nothing a node runs: the controller still sends.", module, short(fp)), + Options: []asks.Option{ + {ID: "keep", Label: "Keep it held", Does: "the version stays held", Level: asks.Acknowledge}, + {ID: "release", Label: "Release it", Does: "the version is released when wants are applied, from release 6", Level: asks.Acknowledge}, + }, + Who: asks.Operator, + Expires: now.Add(7 * 24 * time.Hour), + OnExpiry: "the version stays held", + About: key, + }} +} diff --git a/modules/mesh-delivery-assigner/internal/assign/asks_test.go b/modules/mesh-delivery-assigner/internal/assign/asks_test.go new file mode 100644 index 0000000..7de1348 --- /dev/null +++ b/modules/mesh-delivery-assigner/internal/assign/asks_test.go @@ -0,0 +1,28 @@ +package assign + +import ( + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// 1297/B9 (the assigner's half): the ask about a held fingerprint is a valid ask of the router and of mesh-sdk's layout at +// its key, of a kind the view may answer. +func TestTheHoldAskIsValid(t *testing.T) { + key, err := delivery.HoldAskKey("mail", fNew) + if err != nil { + t.Fatal(err) + } + now := time.Now() + a := HoldAsk(key, "mail", fNew, now) + if err := a.Ask.Check(now); err != nil { + t.Errorf("the router refuses the ask: %v", err) + } + if _, err := delivery.EncodeAssignerAsk(key, a); err != nil { + t.Errorf("the ask cannot be written: %v", err) + } + if _, err := delivery.ViewAnswerKey(key); err != nil { + t.Errorf("the view may not answer it: %v", err) + } +} diff --git a/modules/mesh-delivery-assigner/manifest_test.go b/modules/mesh-delivery-assigner/manifest_test.go index 417ac68..03b879a 100644 --- a/modules/mesh-delivery-assigner/manifest_test.go +++ b/modules/mesh-delivery-assigner/manifest_test.go @@ -42,7 +42,7 @@ func TestManifest(t *testing.T) { for _, s := range m.State { states[s.Name] = s.History } - if want := map[string]int{"compositions": 5, "differences": 64, "wants": 20}; !reflect.DeepEqual(states, want) { + if want := map[string]int{"asks": 5, "compositions": 5, "differences": 64, "wants": 20}; !reflect.DeepEqual(states, want) { t.Errorf("state = %v, want %v", states, want) } if _, ok := m.OwnSecrets["broker"]; !ok || len(m.OwnSecrets) != 1 { diff --git a/modules/mesh-delivery-assigner/module.json b/modules/mesh-delivery-assigner/module.json index 599c3f6..418e8fb 100644 --- a/modules/mesh-delivery-assigner/module.json +++ b/modules/mesh-delivery-assigner/module.json @@ -14,6 +14,7 @@ "node-engine.have" ], "state": [ + { "name": "asks", "history": 5 }, { "name": "compositions", "history": 5 }, { "name": "differences", "history": 64 }, { "name": "wants", "history": 20 }