57/F3: Keep the assignments and each module's declared config on the bus (issue 652) #315
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-controller#315
Reference in New Issue
Block a user
What changed
mesh-controller_assignments, key<node>.<module>, value{node, module, assigned}; andmesh-controller_declared-config, key<module>, value{module, settings}where each declared key is built from named fields:kind,default,why(ADR 0289 §12). A field a manifest declaration gains does not reach the bus until it is listed (package K1 widens it deliberately). Every registered module has a key ({}when it declares none); a module no longer registered loses its key. One value per key, no age, 64 KiB a value, 64 MiB a bucket, a purge marker lives a day.internal/recordmirror): every pass asks for the lease first and writes nothing without it (ErrNotHeld); it re-reads the bucket every pass and compares it byte for byte with the store, so a value written behind its back (a former lease holder) is put back from the store; it asks for the lease again before each write and each removal; it writes by compare-and-set, purges a key whose record is gone, and raises one event after each successful write:assignment-changed/declared-config-changedon the controller's seat (mirror, key, change, the top-level fields that changed, revision, epoch, time; never the value). A refused event is tried once more, then counted lost and logged.cmd/mesh-controller/record_mirrors.go) runs one pass for both mirrors every 10 s from the store, only while it holds the lease.ControllerStates, the seat's emits). A module reads them throughreadsasmesh-controller.assignments/mesh-controller.declared-config;catalogue.ControllerMirrors(the controller's list of its mirrors) lets that read pass the catalogue check. Nobody else may write them.TestNoPathDeletesALogOrABucketallowsPurgeininternal/recordmirror(a removal is a purge of the key, design 54 §2).The record
ADR 0356 is accepted (version 5). It records where this differs from ADR 0289 and design 54: the mirrors are written by a pass over committed records at most 10 s apart (§1); the writer reads its own mirror to write it (§2); the mirrors are listed by the controller, not declared as state in its manifest, granted to readers through
readsand not reported as undeclared (§3); and the declared-config mirror is a sixth mirror (§4). Each of its seven checks has a test below.Why
mesh-config (package K3, issue 662) resolves a module's config values per node from which module runs where and what each module declares (ADR 0348 clauses 2, 4); both lived only in the controller's store.
How it is proven
internal/recordmirror:TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent(store and real bus; intestdata/store-tests): a registration and an assignment each change one key and raise one event; the same records again change nothing; a changed declaration namessettings; an unassignment and a module no longer registered are each one purge and oneremoved; the declaration's bytes are its named fields.TestNatsAMirrorChangesOneKeyAndRaisesOneEventPerChange,TestNatsAWriterStartedAgainWritesOnlyWhatDiffers,TestNatsAWriteRefusedByCompareAndSetReadsAgain.TestNatsAForeignWriteIsCorrectedOnTheNextPass(ADR 0356 §2).TestNatsAWriterWithoutTheLeaseWritesNothing(no lease at the pass, and lost mid-pass) andTestNatsARemovalWithoutTheLeaseRemovesNothing(the removal loop's own check).TestOnlyTheAuditAndAMirrorsWriterReadAMirror(ADR 0289 §19 as ADR 0356 §2 changes it). It catches a name standing for a mirror qualified by its package under any import name (broker.AssignmentsBucket, an aliasedb.DeclaredConfigBucket,recordmirror.Assignments), the same names bare in a file of package broker or recordmirror, and a string literal holding a mirror's bucket name, in any non-test file ofcmdandinternaloutside the allowed files (the mirrors' definition, grants and writer). It does not catch a bucket name built at run time (BucketName(ControllerSeat, "assignments"), concatenated literals) or a mirror reached through a value handed over from an allowed file; the test says so. In package broker, declaring a field named like a mirror is flagged, on the safe side.TestTheReaderTestFindsPlantedReaders: each of those planted readers (qualified, aliased, bare in broker, nested bare, literal) is named; allowed files and a selector on a value are not.TestAKeyTheBusCannotHoldIsRefusedAlone,TestTheFieldsThatChangedAreNamed.internal/inventoryTestTheDeclaredConfigMirrorCarriesOnlyItsListedFields(ADR 0289 §12): fails on a declaration field the list does not name.internal/broker:TestAModuleReadsTheControllersMirrorsAndWritesNone,TestAListedMirrorIsNotReportedAsUndeclared(ADR 0201 §16),TestTheMirrorsListAndTheWritersTableAgree; the states agreement test holds the catalogue's and the bus's mirror lists equal.internal/catalogueTestAReadOfTheControllersMirrorsIsAReadOfKeptState.7a691edd) merged with feat/57 at its head (dc36536):git merge(no conflict), thenMESH_TEST_POSTGRES=… sh merge-check.sh: exit 0 (gofmt, go vet, the whole suite under -race, every listed store test against a throwaway PostgreSQL over TCP).Deliberately left
mirror-drifted,mirror-event-lost), the other three mirrors, the rest of an assignment's fields, and cutting a value over 64 KiB to its summary (ADR 0289 §16): issue 419. The audit joins the reader test's allowed files when it is built.config(withsettingsas its alias) once the manifest field is renamed, and listing the new declaration fields: package K1 (issue 660), ininternal/inventory/mirrors.go.Reviewer's checklist
composed.conf, one line).Delivery
novox/mesh-controller@7a691edd3a6c— rejected since 2026-10-14T01:45:20ZDelivery plan — builds nothing: the change touches no module of the mesh's graph
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
8563b909Judged: head
8563b90918fbmerged into trunkbfc79c64d989; read beside it: mesh-catalogebf7f3773dc7, mesh-controller-main2d18f438c516, mesh-host1a2677882380, mesh-tools954e1a53eef9mesh/merge-gate: PASS — the change touches no module of the mesh's graphmesh/repo-check: FAIL — its merge-check.sh failed: --- FAIL: TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose (0.66s)Run by the build seat on ace as
build-1791938134235979340(builds --log build-1791938134235979340).Verdict: CHANGES at
8563b90918Independent review of package F3 (issue 652) at head
8563b90918, together with mesh-host#110 at c89206225165.Checks I ran in my own worktree: gofmt -l clean; go vet clean;
sh merge-check.shexit 0 against a throwaway PostgreSQL (MESH_TEST_POSTGRES over 127.0.0.1, stopped afterwards), under -race; the bus tests ran on the embedded nats-server v2.11.17 (internal/testbus). TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent passed. The forge's mesh/repo-check is red on this head (TestTheInstallersFirstUserList...) because the check clones mesh-host's feat/57 beside it, which lacks mesh-host#110; I reproduced that: the test fails with mesh-host at origin/feat/57 and passes with mesh-host at c8920622.Must change:
{"a":9}behind a writer's back after it wrote{"a":1}, and the next Sync with the store still at{"a":1}did{Written:0}and left{"a":9}in the bucket. With no audit yet (D17/H6 are issue 419), nothing repairs it. Since this design already re-reads the store every 10 s, refill the memory when the bucket's last sequence differs from the last one this writer wrote (one Status call per pass), or refill every pass; add the test.Holdssays false; it consultsHoldsonly after a compare-and-set refusal. A Writer withHolds: func() bool { return false }wrote 1 key in my probe. Design 54 §2 and ADR 0289 §9: a controller without the lease writes none. Checkholds()before each put and purge (cheap: a mutex and a clock), and test it.settingswhole as raw JSON. ADR 0289 §12: each value is built from a list of named fields, and a test fails on a field the list does not name. Today the stored manifest is the marshalledSettingDeclaration(kind, default, why), and a default is refused for secret-named keys (setting_defaults.go), so no secret can reach the bucket today; but K1 (issue 660) adds fields and renamessettingstoconfig, and this value would carry them unreviewed. Build it from a struct of its own with the listed fields, and add the test the ADR names.knowledge-base.proposewithinsight), named in this PR. (a) ADR 0289 §18 / design 54 §5 say "declared as the controller's state in its manifest". I agree with the deviation: astatedeclaration grants the mesh-controller module's own principal (KindModule) publish on$KV.<bucket>.>, which the writers table rows this PR adds would refuse at composition; readers' grants come frombucketOfRead, not the declaration.catalogue.ControllerMirrorsplus the agreement test is sound. (b) ADR 0289 §7 / design 54 §2 say "after each commit"; a 10 s pass from the store is sound (only committed state is read, every change path is covered, it stays inside §38's reader bound), but the record must say so.Should (may follow, issue 419 or K1):
config(withsettingsas the alias); say so in issue 660.Endless rewrite loop: none found. Values are deterministic (time in UTC, maps sorted, numbers as written), the refused-write path takes an equal value's revision without writing, and nothing a mirror write causes changes the store.
Merge check at
69367aa7Judged: head
69367aa78642merged into trunk617881c3093d; read beside it: mesh-catalogebf7f3773dc7, mesh-controller-maindf9e5728a925, mesh-host1a2677882380, mesh-tools954e1a53eef9mesh/merge-gate: PASS — the change touches no module of the mesh's graphmesh/repo-check: FAIL — its merge-check.sh failed: --- FAIL: TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose (0.62s)Run by the build seat on ace as
build-1791939690718064235(builds --log build-1791939690718064235).Out of date when it landed: out of date: judged against trunk
617881c309, the trunk is now 71362c196dbc; judged again on a recheck or when it is mergedVerdict: CHANGES at
69367aa786Second review of package F3 (issue 652) at head
69367aa786, following review 246. The code at this head needs no change. What must change is the base, the record and this pull request's description.Checks I ran in my own worktree: gofmt -l is clean. go vet is clean.
sh merge-check.shexited 0 against a throwaway PostgreSQL over 127.0.0.1 (stopped afterwards), under -race, with every listed store test passing. The bus tests ran on the vendored nats-server (internal/testbus). TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose passes with mesh-host at origin/feat/57 (c18d5a8, which includes mesh-host#110), using MESH_CHECK_BESIDE.Review 246's findings, each fix checked by reverting it locally:
{Written:0}and the bucket still held{"a":9}. Fixed.holds()with nil Holds now means not held (:291). The production writer passesholds(record_mirrors.go:85). When I reverted holds() and the checks, TestNatsAWriterWithoutTheLeaseWritesNothing failed with 2 writes, 2 removals and 11 bucket writes; with only the per-write checks removed, it failed too. Fixed.I re-ran the earlier probes on this head. A foreign
{"a":9}is put back to{"a":1}. A foreign purge is re-created. A foreign extra key is removed. The next pass writes nothing. A writer with Holds false writes 0 keys and returns ErrNotHeld.Must change:
71362c1. Rebase onto it and keep both sides' lines. I merged it locally that way, and merge-check.sh on the result exited 0. The forge's merge-gate and repo-check say "out of date" (pending), and mesh/delivery is failure, so the rebased head needs both checks green.settingsis "carried whole" and that the writer is "filled from the bucket on its first pass", and it does not list the three new tests. Update it.Should (may follow):
config) still stand for issue 419 and K1.Verdict: CHANGES at
35c7662f89Third review of package F3 (issue 652) at head
35c7662f89, following reviews 246 and 251. Review 246's findings 1 to 3 and review 251's findings 1 and 5 are answered in code. One gap remains in the new reader test, and the description has two stale lines.The record. ADR 0356 is accepted. The knowledge base serves version 5, which accepts version 4's text after its review approved it. Its "How it is checked" section lists seven checks, and this head has a test for each.
Merging. The branch merges cleanly into feat/57 at
dc36536.git merge-tree --write-treegives tree6b42d95with no conflict. A local merge gives the same tree, with the store-tests line moved so that it no longer collides with F2's line.Checks I ran on the merged tree.
gofmt -l cmd internal examplesprinted nothing.MESH_TEST_POSTGRES=postgres://postgres@127.0.0.1:55433/… sh merge-check.shexited 0. That run includes go vet and the whole suite under -race, and every listed store test passed against a throwaway PostgreSQL over TCP (dynamic_shared_memory_type=mmap, stopped afterwards). The listed store tests include the extended TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent, in which a forgotten module is one purge and oneremovedevent.What I reverted or planted at the head, and what happened:
broker.AssignmentsBucketin cmd/mesh-controller and as the literal "mesh-controller_declared-config" in internal/inventory. TestOnlyTheAuditAndAMirrorsWriterReadAMirror failed on both and named the file. It passes on the real tree, so it flags no legitimate code today.Must change:
broker.X,recordmirror.X). A reader inside package broker uses the bare name, so the test misses it. I planted internal/broker/zz_reader.go withfunc zzR() string { return AssignmentsBucket }, and the test passed. ADR 0356's check says the test fails when any package other than the audit or a mirror's writer reads a mirror, and broker is the controller's bus package. Fix: in a file of package broker that is not on mayNameAMirror, also match an*ast.Identnamed AssignmentsBucket, DeclaredConfigBucket, MirrorAssignments, MirrorDeclaredConfig or ControllerMirrors. Today only controller_buckets.go and writers.go use these names, and both are on the allow list, so the change flags no current code.dc36536.Should (may follow):
b "…/broker"; b.DeclaredConfigBucket),broker.BucketName(broker.ControllerSeat, "assignments")and a concatenated literal. I planted each, and the test passed. These are deliberate evasions, not natural code, so naming the limit in the test's comment is enough. Matching on the imported path instead of the local package name would close the alias case cheaply.config) still stand for issue 419 and K1 (issue 660).Merge check at
35c7662fJudged: head
35c7662f895fmerged into trunkdc36536e199e; read beside it: mesh-catalogebf7f3773dc7, mesh-controller-main2959c8143536, mesh-host1a2677882380, mesh-tools954e1a53eef9mesh/merge-gate: PASS — the change touches no module of the mesh's graphmesh/repo-check: FAIL — its merge-check.sh failed: --- FAIL: TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose (1.06s)Run by the build seat on novox as
build-1791941207220036292(builds --log build-1791941207220036292).Merge check at
7a691eddJudged: head
7a691edd3a6cmerged into trunkdc36536e199e; read beside it: mesh-catalogebf7f3773dc7, mesh-controller-main2959c8143536, mesh-host1a2677882380, mesh-tools954e1a53eef9mesh/merge-gate: PASS — the change touches no module of the mesh's graphmesh/repo-check: FAIL — its merge-check.sh failed: --- FAIL: TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose (0.79s)Run by the build seat on shanks as
build-1791942053809789385(builds --log build-1791942053809789385).Verdict: APPROVE at
7a691edd3aFourth review of package F3 (issue 652), following reviews 246, 251 and 257. Both points of review 257 are answered.
return AssignmentsBucket) and internal/q/a.go (import bb ".../broker"; bb.DeclaredConfigBucket): TestOnlyTheAuditAndAMirrorsWriterReadAMirror failed and named both files. On the real tree it passes and flags nothing. TestTheReaderTestFindsPlantedReaders passes (qualified, aliased, bare in broker, nested bare, literal). The test's comment names what it still misses: names built at run time, concatenated literals, a mirror reached through a value handed over from an allowed file.dc36536.Nothing else changed since
35c7662:git diff --stat 35c7662..7a691edis the one file readers_test.go (108 insertions, 9 deletions).Merging:
git merge-tree --write-tree origin/feat/57-config-and-provisioning 7a691edgives tree75632f5with no conflict; a local merge gives the same tree.Checks on the merged tree:
gofmt -l cmd internal examplesprinted nothing;go vet ./...clean;MESH_TEST_POSTGRES=postgres://postgres@127.0.0.1:55434/... sh merge-check.shexit 0, no FAIL line, against a throwaway PostgreSQL over TCP (dynamic_shared_memory_type=mmap, stopped afterwards).Open findings that may follow, unchanged from earlier reviews: cutting a value over 64 KiB (ADR 0289 section 16) stays with issue 419, and K1 (issue 660) reads
config.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.