novox/mesh-delivery
19
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Build each fingerprint on one node and drop the second-node retake, as ADR 0478 §4 has it from R4e (issue 1587, B9)

Browse Source 
A key done on one node before R4e is not built again for a second; a key failed for differing digests stays as it is.
This commit is contained in:
jschoubben
2026-10-22 18:18:59 +02:00
parent 9a3f38629f
commit 4bb04a47f9
9 changed files with 84 additions and 336 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-delivery-builder/cmd/mesh-delivery-builder/integration_test.go
+15 -40
View File
@@ -56,10 +56,10 @@ type fakeBuildModules struct {
failBuild atomic.Bool
// refusePush makes every bundle and archive build fail as a push the artifact store refused (ADR 0498 clause 2).
refusePush atomic.Bool
mu sync.Mutex
calls map[string]int // by `<module>.<tool>`
fetches []map[string]any
builds []map[string]any // the arguments of every build call
mu sync.Mutex
calls map[string]int // by `<module>.<tool>`
fetches []map[string]any
builds []map[string]any // the arguments of every build call
}
func (f *fakeBuildModules) count(tool string) int {
@@ -121,7 +121,7 @@ func (f *fakeBuildModules) serve(t *testing.T, nc *nats.Conn, module string) {
merge = `,"merge":"` + strings.Repeat("9", 40) + `"`
}
step.State, step.Result = delivery.StepPassed, json.RawMessage(`{"workspace":"w`+id+`","commit":"`+commit+`"`+merge+`}`)
case f.failBuild.Load() && node == "n1":
case f.failBuild.Load() && tool != "build_image_copy": // every node's build fails: one node builds a fingerprint
step.State, step.Reason = delivery.StepFailed, fmt.Sprintf("go build of %v: boom", args["artifact"])
case tool == "build_image_copy": // the upstream copy answers the digest and the registry's reference (ADR 0498 clause 4)
d := digestOf(fmt.Sprint(args["artifact"]))
@@ -374,7 +374,7 @@ func isState(state string) func(delivery.BuilderBuild) bool {
// TestBuilderMakesADoneKeyWithTheDigests: modules.netcheck (a Go and a TypeScript bundle) and its manifest are in the
// indexer's buckets, and two nodes report each build module's toolchain; the program writes builds.netcheck.<fp> done,
// the digests of both bundles, both build modules' toolchains, after a fetch and a build on each of the two nodes.
// the digests of both bundles, both build modules' toolchains, after a fetch and a build on one node (ADR 0478 clause 4).
func TestBuilderMakesADoneKeyWithTheDigests(t *testing.T) { // 1607/AC7
b := startBed(t)
digest := b.module(t, "netcheck", "catalogue-a67f0196-netcheck.module.json", "modules/netcheck")
@@ -391,9 +391,9 @@ func TestBuilderMakesADoneKeyWithTheDigests(t *testing.T) { // 1607/AC7
if v.Toolchains["go"].Version != "go1.27.1" || v.Toolchains["typescript"].Version != "tsc5" {
t.Errorf("toolchains = %+v", v.Toolchains)
}
for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 2, "mesh-build-go.build_go_build": 2, "mesh-build-node.build_node_fetch": 2, "mesh-build-node.build_node_build": 2} {
for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 1, "mesh-build-go.build_go_build": 1, "mesh-build-node.build_node_fetch": 1, "mesh-build-node.build_node_build": 1} {
if got := b.fake.count(tool); got != want {
t.Errorf("%s called %d times, want %d (once on each of two nodes)", tool, got, want)
t.Errorf("%s called %d times, want %d (on one node, ADR 0478 clause 4)", tool, got, want)
}
}
}
@@ -410,8 +410,8 @@ func TestTwoBuilderInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { //
if len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "archive" || v.Artifacts[0].Digest != digestOf("files") {
t.Fatalf("build = %+v, want the archive's digest", v)
}
if n := b.fake.count("mesh-build-go.build_go_build"); n < 2 || n%2 != 0 { // each instance that built, on two nodes
t.Errorf("build_go_build was called %d times, want two for each instance that built", n)
if n := b.fake.count("mesh-build-go.build_go_build"); n < 1 { // each instance that built, on one node
t.Errorf("build_go_build was called %d times, want one for each instance that built", n)
}
}
@@ -423,32 +423,7 @@ func TestAnArchiveOnlyModuleNamingNoToolchainIsBuilt(t *testing.T) {
b.start(t, "n1", nil)
v := b.await(t, "ca-trust", isState(delivery.BuildDone))
if len(v.Toolchains) != 0 || len(v.Artifacts) != 1 || strings.HasPrefix(v.Step, "one node (") {
t.Errorf("build = %+v, want done on two nodes naming no toolchain", v)
}
}
// TestAKeyBuiltOnOneNodeIsBuiltAgainWhenASecondNodeComesOnline: n2 is offline, so the key is done on n1 alone and marked;
// n2 coming online makes the builder build it again on both nodes, as a new revision (ADR 0478 clause 4).
func TestAKeyBuiltOnOneNodeIsBuiltAgainWhenASecondNodeComesOnline(t *testing.T) {
b := startBed(t)
b.setOnline(t, "n2", false)
b.archive(t, "ca-trust", "modules/ca-trust", commit)
b.start(t, "n1", nil)
one := b.await(t, "ca-trust", isState(delivery.BuildDone))
if !strings.HasPrefix(one.Step, "one node (") {
t.Fatalf("build = %+v, want it marked as built on one node", one)
}
key := mustKey(t, "ca-trust")
e, err := b.builds.Get(b.ctx, key)
if err != nil {
t.Fatal(err)
}
b.setOnline(t, "n2", true)
two := b.await(t, "ca-trust", func(v delivery.BuilderBuild) bool {
return v.State == delivery.BuildDone && !strings.HasPrefix(v.Step, "one node (")
})
if e2, _ := b.builds.Get(b.ctx, key); e2.Revision() <= e.Revision() || !strings.Contains(two.Step, "n1 and n2") {
t.Errorf("build = %+v at revision %d (was %d), want a new revision built on n1 and n2", two, e2.Revision(), e.Revision())
t.Errorf("build = %+v, want done naming no toolchain", v)
}
}
@@ -459,7 +434,7 @@ func TestANodeWhoseBuildModuleIsUnhealthyIsNotUsed(t *testing.T) {
b.archive(t, "ca-trust", "modules/ca-trust", commit)
b.start(t, "n1", nil)
v := b.await(t, "ca-trust", isState(delivery.BuildDone))
if !strings.Contains(v.Step, "on one node, n1") {
if !strings.Contains(v.Step, "on n1") {
t.Errorf("build = %+v, want it built on n1 alone", v)
}
}
@@ -473,7 +448,7 @@ func TestAFailedKeyIsBuiltAgainAsANewRevisionFromANewModulesValueWithTheSameFing
b.archive(t, "ca-trust", "modules/ca-trust", commit)
b.start(t, "n1", nil)
v := b.await(t, "ca-trust", isState(delivery.BuildFailed))
if !strings.Contains(v.Error, "n1") || !strings.Contains(v.Error, "boom") {
if !strings.Contains(v.Error, " on n") || !strings.Contains(v.Error, "boom") {
t.Errorf("build = %+v, want failed naming the node and the step's reason", v)
}
key, _ := delivery.BuilderBuildKey("ca-trust", fp)
@@ -510,8 +485,8 @@ func TestAToolchainChangeDoesNotBuildTheOldFingerprint(t *testing.T) { // 1615/A
if _, err := b.builds.Get(b.ctx, mustKey(t, "netcheck")); err == nil {
t.Error("the old fingerprint, naming go1.27.1, was built with go1.27.2")
}
if n := b.fake.count("mesh-build-go.build_go_build"); n != 2 {
t.Errorf("build_go_build was called %d times, want 2 (the new fingerprint on two nodes only)", n)
if n := b.fake.count("mesh-build-go.build_go_build"); n != 1 {
t.Errorf("build_go_build was called %d times, want 1 (the new fingerprint on one node only)", n)
}
}
modules/mesh-delivery-builder/cmd/mesh-delivery-builder/main.go
-3
View File
@@ -35,8 +35,6 @@ const (
// defaultStepBound is how long the builder waits for one step of a build module (a fetch downloads the Go modules, a
// build compiles).
defaultStepBound = 30 * time.Minute
// nodesPerFingerprint is how many build nodes build each fingerprint until release R4e (ADR 0478 clause 4).
nodesPerFingerprint = 2
)
func main() {
@@ -178,7 +176,6 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro
Health: have,
Places: &buildrun.Places{Capacity: capacity},
Manifests: manifestReader{manifests},
Nodes: nodesPerFingerprint,
Logf: logger.Printf,
}
// The toolchains first, so a build that starts from the first modules value finds the nodes.
modules/mesh-delivery-builder/internal/build/candidates_test.go
+3 -3
View File
@@ -54,8 +54,8 @@ func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoThe
t.Fatalf("build = %+v, want the candidate's fingerprint built", v)
}
fetches := r.bm.callsOf("build_go_fetch")
if len(fetches) != 2 { // one on each of two nodes
t.Fatalf("%d fetches, want one per node", len(fetches))
if len(fetches) != 1 { // one node per fingerprint (ADR 0478 clause 4)
t.Fatalf("%d fetches, want one", len(fetches))
}
f := fetches[0].body.(map[string]any)
if f["commit"] != headX || f["ref"] != "refs/pull/7/head" || f["base"] != "7a74cee83f88e6585a5511c3cc53bdf0f5303d20" || f["repository"] != "novox/mesh-catalog" {
@@ -67,7 +67,7 @@ func TestACandidatesFingerprintIsBuiltOnceAsAKeyOfItsOwnFromTheHeadMergedOntoThe
// Two heads carrying one fingerprint are one build.
r.b.Candidate(context.Background(), "feat%2Fy", candidateValue(t, "feat/y", 8, headY))
r.b.Wait()
if n := len(r.bm.callsOf("build_go_fetch")); n != 2 {
if n := len(r.bm.callsOf("build_go_fetch")); n != 1 {
t.Errorf("%d fetches after a second head with the same fingerprint, want no more", n)
}
}
modules/mesh-delivery-builder/internal/build/claim.go
+4 -27
View File
@@ -39,32 +39,11 @@ type Claim struct {
Build delivery.BuilderBuild
}
// OneNodePrefix opens the Step of a done build that a build module built on one node only, because no second node
// qualified (ADR 0478 clause 4); the build modules that did so follow in parentheses, then ": ". Claim reads it to build
// the key again, as a new revision, once a second node qualifies.
const OneNodePrefix = "one node ("
// OneNodeModules are the build modules a done build built on one node only, or none when it is not such a build.
func OneNodeModules(b delivery.BuilderBuild) []string {
if b.State != delivery.BuildDone || !strings.HasPrefix(b.Step, OneNodePrefix) {
return nil
}
list, _, ok := strings.Cut(strings.TrimPrefix(b.Step, OneNodePrefix), "): ")
if !ok || list == "" {
return nil
}
return strings.Split(list, ", ")
}
// ClaimOptions say what the builder knows about the moment of a claim.
type ClaimOptions struct {
// TakeOver takes a key found building whoever holds it: the builder has just started and looks at the key for the first
// time, and a build left building by a builder that is gone, on this node or another, is nobody's (ADR 0478 clause 6).
TakeOver bool
// MoreNodes says whether one of these build modules, which built a done key on one node only, has a second node that
// qualifies now and the key may be built again (it is not held back after a failed second build, issue 1664). Nil means
// none does.
MoreNodes func(modules []string) bool
// ImageBuilder says whether a node runs a healthy image build module now: a key failed for NoImageBuilderPrefix is taken
// again only then. Nil means none does.
ImageBuilder func() bool
@@ -135,9 +114,10 @@ func Unreferenced(b delivery.BuilderBuild) bool {
// - no key: it is created building, by this builder; a create that finds the key made meanwhile gets nothing;
// - a done key is not taken again (the same source is never built twice), except one whose bundle, archive or upstream
// copy names no reference (Unreferenced, a key done before R4e: ADR 0498 clause 8), which is claimed building as a new
// revision and built whole, and one built on one node only (OneNodePrefix)
// when a second node qualifies now (ClaimOptions.MoreNodes): that is taken as it stands, with the done value, and its
// builder writes the new revision when the second build ends (ADR 0478 clause 4);
// revision and built whole, and one whose image or package was built unpublished as a candidate's once the trunk
// names it (ClaimOptions.Trunk): that is taken as it stands, with the done value, and its builder writes the new
// revision when the publishing build ends (ADR 0498 clause 6). From R4e one node builds each fingerprint (ADR 0478
// clause 4), so a key built on one node is never built again for a second;
// - a failed key is taken again as a new revision of the same key (ADR 0429 clause 18), except one failed for a kind no
// builder builds, for an upstream pinned by no digest or a package version taken (delivery.FailedUntilChanged: ADR 0478
// clause 2, ADR 0498 clauses 4 and 5) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4),
@@ -169,9 +149,6 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string
if Unreferenced(cur) {
return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) })
}
if modules := OneNodeModules(cur); len(modules) > 0 && opt.MoreNodes != nil && opt.MoreNodes(modules) {
return &Claim{Key: key, Rev: rev, Build: cur}, nil
}
if opt.Trunk && len(publishable(cur, opt.ImageBuilder)) > 0 {
return &Claim{Key: key, Rev: rev, Build: cur}, nil
}
modules/mesh-delivery-builder/internal/build/followups_test.go
-37
View File
@@ -8,43 +8,6 @@ import (
"git.novox.be/novox/mesh-sdk/go/delivery"
)
// 1664: a done key built on one node is not built again, over and over, while the second node's build keeps failing.
func TestASecondNodeBuildThatFailsIsNotRetriedUntilTheNodesChange(t *testing.T) { // issue 1664
r := newRig(t, goBundle)
r.pres.Put("ace", presenceValue(t, false), false) // only novox qualifies
first := r.consider(t, "modules/m")
r.bm.fails["ace/build_go_fetch"] = "ace is out of disk"
r.pres.Put("ace", presenceValue(t, true), false)
r.b.NodesChanged(context.Background()) // ace came online: one more try
r.b.Wait()
tries := len(r.bm.callsOf("build_go_fetch"))
if tries < 2 {
t.Fatalf("%d fetches: the second node was never tried", tries)
}
if v := valueOf(t, r.st, r.key); !delivery.SameBuilderBuild(v, first) {
t.Fatalf("the done key changed: %+v", v)
}
// Nothing about the nodes changed: a modules value, a Reconsider and a start of looking again build nothing.
for i := 0; i < 3; i++ {
r.b.Module(context.Background(), "m", moduleValue(t, "modules/m"))
r.b.Reconsider(context.Background())
r.b.Wait()
}
if n := len(r.bm.callsOf("build_go_fetch")); n != tries {
t.Errorf("%d fetches after looking again, was %d: the failed second build was retried in a loop", n, tries)
}
// A node changing (ace goes and comes back) is a reason to try again, and ace now builds.
delete(r.bm.fails, "ace/build_go_fetch")
r.pres.Put("ace", presenceValue(t, false), false)
r.b.NodesChanged(context.Background())
r.pres.Put("ace", presenceValue(t, true), false)
r.b.NodesChanged(context.Background())
r.b.Wait()
if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone || strings.HasPrefix(v.Step, OneNodePrefix) || !strings.Contains(v.Step, "ace and novox") {
t.Errorf("build = %+v, want it built on both nodes once ace could", v)
}
}
// 1660: at start the builder closes a key left building for a fingerprint the module no longer has.
func TestAKeyLeftBuildingForAFingerprintTheModuleNoLongerHasIsClosedAtStart(t *testing.T) { // issue 1660
r := newRig(t, goBundle)
modules/mesh-delivery-builder/internal/build/r4e_test.go
+28
View File
@@ -332,3 +332,31 @@ func TestTheFetchOfAModuleNamingItsPathsCarriesItsDirectoryAndThosePaths(t *test
}
func hex40(c byte) string { return strings.Repeat(string(c), 40) }
func TestOneNodeBuildsEachFingerprint(t *testing.T) { // B9, ADR 0478 clause 4: "From R4e one node builds each fingerprint"
r := newRig(t, goBundle, archiveA, tsBundle) // ace and novox both qualify for both build modules
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || strings.Contains(v.Step, " and ") || strings.HasPrefix(v.Step, "one node (") {
t.Fatalf("build = %+v, want done on one node of each build module, unmarked", v)
}
if f, b := len(r.bm.callsOf("build_go_fetch")), len(r.bm.callsOf("build_go_build")); f != 1 || b != 2 {
t.Errorf("go fetches %d, builds %d; want one fetch and one build of each artifact, on one node", f, b)
}
if f := len(r.bm.callsOf("build_node_fetch")); f != 1 {
t.Errorf("node fetches %d, want one", f)
}
}
func TestAKeyDoneOnOneNodeIsNeverBuiltAgainForASecondNode(t *testing.T) { // B9: the second-node retake is gone
r := newRig(t, goBundle)
putBuild(t, r.st, r.key, delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.builder", Manifest: manifestD,
Step: "one node (mesh-build-go): built mesh-build-go on one node, novox",
Artifacts: []delivery.BuiltArtifact{{Name: "tool", Kind: "bundle", Digest: digest, Reference: "artifact-store://m/tool/blobs/" + digest}}})
writes := r.st.writes
r.b.Module(context.Background(), "m", moduleValue(t, "modules/m"))
r.b.NodesChanged(context.Background())
r.b.Wait()
if r.st.writes != writes || len(r.bm.calls) != 0 {
t.Errorf("a key done on one node was built again (%d writes, was %d; %d calls)", r.st.writes, writes, len(r.bm.calls))
}
}
modules/mesh-delivery-builder/internal/build/round4_test.go
+5 -85
View File
@@ -34,8 +34,8 @@ func TestAnArchiveOnlyModuleIsBuiltAlthoughItsFingerprintNamesNoToolchain(t *tes
if v.State != delivery.BuildDone || len(v.Artifacts) != 1 || v.Artifacts[0].Kind != "archive" || len(v.Toolchains) != 0 {
t.Fatalf("build = %+v, want done with the archive and no toolchain named", v)
}
if n := len(r.bm.callsOf("build_go_build")); n != 2 {
t.Errorf("%d go builds, want one on each of two nodes", n)
if n := len(r.bm.callsOf("build_go_build")); n != 1 {
t.Errorf("%d go builds, want one, on one node (ADR 0478 clause 4)", n)
}
}
@@ -54,8 +54,8 @@ func TestAnArchiveOnlyModuleNeedsNoAgreedToolchainButNeedsNodesThatQualify(t *te
r.b.Reconsider(context.Background())
r.b.Wait()
v := valueOf(t, r.st, r.key)
if v.State != delivery.BuildDone || !strings.HasPrefix(v.Step, OneNodePrefix) {
t.Errorf("build = %+v, want done on novox, the one node that qualifies, marked as one node", v)
if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") {
t.Errorf("build = %+v, want done on novox, the one node that qualifies", v)
}
}
@@ -70,25 +70,6 @@ func TestABundleAndAnArchiveOfOneBuildModuleStillNeedTheAgreedToolchain(t *testi
}
}
// 2. The digests-differ failure opens with its prefix, and nothing builds it again.
func TestDifferingDigestsFailTheKeyWithThePrefixAndNothingBuildsItAgain(t *testing.T) { // review round 4, 2
r := newRig(t, goBundle)
r.bm.digests["ace/tool"] = "sha256:" + hex64('9')
v := r.consider(t, "modules/m")
if v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, "digests differ:") {
t.Fatalf("build = %+v, want a failure opening with exactly %q", v, "digests differ:")
}
calls, writes := len(r.bm.calls), r.st.writes
r.bm.digests = map[string]string{} // the nodes would now agree
r.b.Reconsider(context.Background())
r.b.Wait()
r.b.Module(context.Background(), "m", moduleValue(t, "modules/m"))
r.b.Wait()
if len(r.bm.calls) != calls || r.st.writes != writes {
t.Errorf("a failed-for-digests key was built again: %d calls (was %d), %d writes (was %d)", len(r.bm.calls), calls, r.st.writes, writes)
}
}
func TestClaimLeavesAKeyFailedForDifferingDigestsAlone(t *testing.T) { // review round 4, 2
st := newMem()
seed(t, st, delivery.BuilderBuild{State: delivery.BuildFailed, Error: buildrun.DigestsDifferPrefix + " the nodes built tool to different digests: ace x; novox y"})
@@ -99,67 +80,6 @@ func TestClaimLeavesAKeyFailedForDifferingDigestsAlone(t *testing.T) { // review
}
}
// 3. A done key built on one node is built again on a second node once one qualifies.
func TestADoneKeyOfOneNodeIsBuiltAgainOnASecondNodeAndTheDigestsCompared(t *testing.T) { // review round 4, 3
r := newRig(t, goBundle)
r.pres.Put("ace", presenceValue(t, false), false) // only novox qualifies
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || !strings.HasPrefix(v.Step, OneNodePrefix+buildrun.GoBuildModule+"): ") || !strings.Contains(v.Step, "on one node, novox") {
t.Fatalf("build = %+v, want done on one node, marked", v)
}
if got := OneNodeModules(v); len(got) != 1 || got[0] != buildrun.GoBuildModule {
t.Errorf("OneNodeModules = %v", got)
}
rev, calls := r.st.revs[r.key], len(r.bm.callsOf("build_go_build"))
// Nothing changes while one node qualifies: looking again builds nothing.
r.b.Reconsider(context.Background())
r.b.Wait()
if r.st.revs[r.key] != rev || len(r.bm.callsOf("build_go_build")) != calls {
t.Fatal("a one-node key was built again with no second node")
}
// ace comes online: the key is built on ace and novox, as a new revision.
r.pres.Put("ace", presenceValue(t, true), false)
r.b.Reconsider(context.Background())
r.b.Wait()
v = valueOf(t, r.st, r.key)
if v.State != delivery.BuildDone || strings.HasPrefix(v.Step, OneNodePrefix) || !strings.Contains(v.Step, "ace and novox") {
t.Errorf("build = %+v, want done on ace and novox", v)
}
if r.st.revs[r.key] <= rev {
t.Errorf("revision %d, was %d: not a new revision", r.st.revs[r.key], rev)
}
builds := r.bm.callsOf("build_go_build")
if len(builds) != calls+2 || builds[len(builds)-2].node == builds[len(builds)-1].node {
t.Errorf("%d builds (was %d), want one on each of two nodes", len(builds), calls)
}
}
func TestASecondNodeThatDisagreesFailsTheDoneKeyWithTheDigestsPrefix(t *testing.T) { // review round 4, 3
r := newRig(t, goBundle)
r.pres.Put("ace", presenceValue(t, false), false)
r.consider(t, "modules/m")
r.bm.digests["ace/tool"] = "sha256:" + hex64('9')
r.pres.Put("ace", presenceValue(t, true), false)
r.b.Reconsider(context.Background())
r.b.Wait()
if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, buildrun.DigestsDifferPrefix) {
t.Errorf("build = %+v, want failed with digests differ", v)
}
}
func TestASecondBuildThatFailsForAnotherReasonLeavesTheDoneKeyStanding(t *testing.T) { // review round 4, 3
r := newRig(t, goBundle)
r.pres.Put("ace", presenceValue(t, false), false)
first := r.consider(t, "modules/m")
r.bm.fails["ace/build_go_fetch"] = "ace is out of disk"
r.pres.Put("ace", presenceValue(t, true), false)
r.b.Reconsider(context.Background())
r.b.Wait()
if v := valueOf(t, r.st, r.key); !delivery.SameBuilderBuild(v, first) {
t.Errorf("build = %+v, want the first done build to stand: %+v", v, first)
}
}
// 4. A step never waits on a full node while another qualifying node has room.
func TestChoosesMadeAtOnceSeeEachOthersReservationsSoNoLaneWaitsWhileANodeHasRoom(t *testing.T) { // review round 4, 4
p := &buildrun.Places{Capacity: map[string]int{"ace": 2, "g14": 1, "novox": 1, "shanks": 3}}
@@ -252,7 +172,7 @@ func TestANodeWhoseBuildModuleIsNotHealthyIsNotChosen(t *testing.T) { // review
have.Put("novox.mesh-build-go", haveValue(t, delivery.PhaseRunning, delivery.HealthOK), false)
r.b.Health = have
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on one node, novox") {
if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") {
t.Fatalf("build = %+v, want done on novox alone", v)
}
for _, c := range r.bm.calls {
modules/mesh-delivery-builder/internal/build/run.go
+11 -71
View File
@@ -42,8 +42,6 @@ type Builder struct {
}
// buildrun.Places holds a step back while its node is at its build capacity; a Builder with none runs one step at a time on each node.
Places *buildrun.Places
// Nodes is how many build nodes build each fingerprint: two until release R4e (ADR 0478 clause 4).
Nodes int
// Limit is how many builds run at once; four when 0.
Limit int
Logf func(format string, args ...any)
@@ -60,10 +58,6 @@ type Builder struct {
via map[string]via // by build key: a candidate's fetch (candidates.go)
trunkFP map[string]bool // by build key: a trunk module value carries this fingerprint
tried map[string]bool // by build key: looked at since this process started
// epoch counts the changes of the nodes (online, healthy, a toolchain) seen since this process started; a done key
// whose second-node build failed is held back until it moves (issue 1664).
epoch uint64
secondAt map[string]uint64 // by build key: the epoch a second-node build of a one-node key failed at
// published are the package versions `<name>@<version>` a done key of this bucket names as a package's reference: a key
// waiting for one is built once it is here (ADR 0500 clause 6).
published map[string]bool
@@ -83,7 +77,7 @@ func (b *Builder) init() {
b.sem = make(chan struct{}, limit)
b.cands, b.via, b.trunkFP = map[string]delivery.Candidate{}, map[string]via{}, map[string]bool{}
b.known, b.inflight, b.again, b.plans, b.tried = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{}, map[string]bool{}
b.secondAt, b.published = map[string]uint64{}, map[string]bool{}
b.published = map[string]bool{}
if b.Places == nil {
b.Places = &buildrun.Places{}
}
@@ -120,15 +114,8 @@ func (b *Builder) Module(ctx context.Context, key string, value []byte) {
}
// NodesChanged is Reconsider after a change of the nodes: one came online or went, a build module became healthy or
// stopped being, or a toolchain moved. A done key built on one node whose second-node build failed is tried again only
// after such a change, not at every modules value (issue 1664).
func (b *Builder) NodesChanged(ctx context.Context) {
b.init()
b.mu.Lock()
b.epoch++
b.mu.Unlock()
b.Reconsider(ctx)
}
// stopped being, or a toolchain moved, so a build that waited for a node may go on.
func (b *Builder) NodesChanged(ctx context.Context) { b.Reconsider(ctx) }
// Sweep reads, at start, every key of the bucket: it keeps the package versions done keys name as published (a key
// waiting for one is then looked at again, ADR 0500 clause 6), and closes every key left building for a fingerprint its
@@ -323,14 +310,8 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) {
trunk := b.trunkFP[key]
first := !b.tried[key]
b.tried[key] = true
epoch := b.epoch
failedAt, failedBefore := b.secondAt[key]
b.mu.Unlock()
moreNodes := func(modules []string) bool {
// A second build that failed is not tried again until the nodes change (issue 1664).
return (!failedBefore || epoch > failedAt) && b.moreNodes(modules)
}
cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: moreNodes, ImageBuilder: b.imageBuilderRuns, Trunk: trunk, Published: b.isPublished})
cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, ImageBuilder: b.imageBuilderRuns, Trunk: trunk, Published: b.isPublished})
if err != nil {
b.logf("%s: %v", module, err)
return
@@ -338,16 +319,13 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) {
if cl == nil {
return
}
// A done key taken again builds only what is missing: the build modules that ran on one node, and the image when it was
// built as a candidate's; the other artifacts stay as the done key has them (ADR 0487 clauses 5 and 8).
// A done key taken again builds only what is missing: the image or the package built unpublished as a candidate's; the
// other artifacts stay as the done key has them (ADR 0487 clauses 5 and 8, ADR 0498 clause 6).
var prior *delivery.BuilderBuild
var only map[string]bool
publishing := false
if cl.Build.State == delivery.BuildDone {
prior, only = &cl.Build, map[string]bool{}
for _, bm := range OneNodeModules(cl.Build) {
only[bm] = true
}
if trunk {
for _, bm := range publishable(cl.Build, b.imageBuilderRuns) {
only[bm] = true
@@ -375,14 +353,6 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) {
// The trunk's image or package was built only as a candidate's and its push or publish failed: the key says so, so the builds view, the
// checker and the failed-key retake see it, and a look does not build the image again for nothing.
result.Error = keyError([]string{"publishing what the merged commit built failed: " + result.Error})
} else if cl.Build.State == delivery.BuildDone && result.State == delivery.BuildFailed && !strings.HasPrefix(result.Error, buildrun.DigestsDifferPrefix) {
// A done key built again on a second node keeps its done value when the new build fails for another reason than
// the digests: the first build stands, and a later change of the nodes looks again.
b.logf("%s: built again on a second node and failed, the done build stands: %s", cl.Key, result.Error)
b.mu.Lock()
b.secondAt[cl.Key] = epoch
b.mu.Unlock()
return
}
if err := b.Claimer.Finish(ctx, cl, result); err != nil {
b.logf("%s: the build could not be written: %v", cl.Key, err)
@@ -444,7 +414,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p
repository := delivery.RepositoryName(m.Repository)
var lanes []*lane
used := map[string]delivery.ToolchainUsed{}
var steps, oneNode []string
var steps []string
// Every node chosen holds a reservation until its lane takes the place; a build that ends before its lanes start gives them up.
giveUp := func(nodes ...string) {
for _, n := range nodes {
@@ -477,7 +447,7 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p
}
nodes = ns
} else if plan.NeedsToolchain(bm) {
ns, tc, err := buildrun.ChooseNodes(language, entries, b.Places, qualifies, b.Nodes)
ns, tc, err := buildrun.ChooseNodes(language, entries, b.Places, qualifies, 1)
if err != nil {
giveUpLanes()
return failed("%s", err)
@@ -491,23 +461,15 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p
nodes = ns
} else {
// An archive is packed with no toolchain (the fingerprint names none): any node of the build module that qualifies.
ns, err := buildrun.ChooseAnyNodes(language, entries, b.Places, qualifies, b.Nodes)
ns, err := buildrun.ChooseAnyNodes(language, entries, b.Places, qualifies, 1)
if err != nil {
giveUpLanes()
return failed("%s", err)
}
nodes = ns
}
if bm == ImageBuildModule {
steps = append(steps, fmt.Sprintf("%s on %s", bm, nodes[0]))
} else if len(nodes) == 1 {
oneNode = append(oneNode, bm)
steps = append(steps, fmt.Sprintf("%s on one node, %s", bm, nodes[0]))
} else {
sorted := append([]string(nil), nodes...)
sort.Strings(sorted)
steps = append(steps, fmt.Sprintf("%s on %s", bm, strings.Join(sorted, " and ")))
}
// One node builds each fingerprint (ADR 0478 clause 4, from R4e).
steps = append(steps, fmt.Sprintf("%s on %s", bm, nodes[0]))
var tasks []Task
for _, t := range plan.Tasks {
if t.Module == bm {
@@ -592,10 +554,6 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p
}
}
step := "built " + strings.Join(steps, "; ")
if len(oneNode) > 0 {
// Marked so that Claim builds the key again once a second node qualifies (ADR 0478 clause 4).
step = OneNodePrefix + strings.Join(oneNode, ", ") + "): " + step
}
if len(used) == 0 {
used = nil
}
@@ -696,24 +654,6 @@ func (b *Builder) imageBuilderRuns() bool {
return false
}
// moreNodes is whether any of these build modules has two nodes that qualify now.
func (b *Builder) moreNodes(modules []string) bool {
for _, bm := range modules {
seen := map[string]bool{}
n := 0
for _, e := range b.Toolchains.Entries(buildrun.LanguageOf[bm]) {
if !seen[e.Node] && b.qualifies(e.Node, bm) {
seen[e.Node] = true
n++
}
}
if n >= 2 {
return true
}
}
return false
}
// step runs one step of a build module; the lane that runs it holds its node's place.
func (b *Builder) step(ctx context.Context, bm, node, tool string, args map[string]any) (json.RawMessage, error) {
return b.Runner.Run(ctx, bm, node, toolPrefix[bm]+tool, args)
modules/mesh-delivery-builder/internal/build/run_test.go
+18 -70
View File
@@ -139,7 +139,7 @@ func newRig(t *testing.T, artifacts ...catalogue.Artifact) *rig {
}
r.b = &Builder{
Claimer: Claimer{Store: r.st, By: "novox.builder"}, Runner: buildrun.Runner{Caller: r.bm, Steps: r.bm.steps, Bound: time.Second},
Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Nodes: 2, Logf: t.Logf,
Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Logf: t.Logf,
}
var err error
if r.key, err = delivery.BuilderBuildKey("m", fp1); err != nil {
@@ -180,7 +180,7 @@ var (
manifestD = mdigest('a')
)
func TestABundleAndAnArchiveAreBuiltOnTwoNodesAndDoneWhenTheDigestsAgree(t *testing.T) { // 1607/AC7, ADR 0478 clauses 1, 4
func TestABundleAndAnArchiveAreBuiltOnOneNodeAndDone(t *testing.T) { // 1607/AC7, ADR 0478 clauses 1, 4
r := newRig(t, goBundle, archiveA, tsBundle)
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || v.By != "novox.builder" || v.Manifest != manifestD || len(v.Artifacts) != 3 {
@@ -200,15 +200,15 @@ func TestABundleAndAnArchiveAreBuiltOnTwoNodesAndDoneWhenTheDigestsAgree(t *test
if got := v.Toolchains["typescript"]; got.Version != "tsc5" {
t.Errorf("toolchains = %+v", v.Toolchains)
}
// Two fetches per build module, one on each node; every build names the module's directory.
if n := len(r.bm.callsOf("build_go_fetch")); n != 2 {
t.Errorf("%d go fetches, want one per node", n)
// One fetch per build module, on one node (ADR 0478 clause 4); every build names the module's directory.
if n := len(r.bm.callsOf("build_go_fetch")); n != 1 {
t.Errorf("%d go fetches, want one", n)
}
if n := len(r.bm.callsOf("build_go_build")); n != 4 { // tool and files, each on two nodes
t.Errorf("%d go builds, want 4", n)
if n := len(r.bm.callsOf("build_go_build")); n != 2 { // tool and files
t.Errorf("%d go builds, want 2", n)
}
if n := len(r.bm.callsOf("build_node_build")); n != 2 {
t.Errorf("%d node builds, want 2", n)
if n := len(r.bm.callsOf("build_node_build")); n != 1 {
t.Errorf("%d node builds, want 1", n)
}
c := r.bm.callsOf("build_go_build")[0]
if args := c.body.(map[string]any); args["module"] != "modules/m" || !strings.HasPrefix(fmt.Sprint(args["workspace"]), "w-") {
@@ -220,21 +220,12 @@ func TestABundleAndAnArchiveAreBuiltOnTwoNodesAndDoneWhenTheDigestsAgree(t *test
}
}
func TestDifferingDigestsFailTheBuildNamingBothNodes(t *testing.T) { // 1607/AC4, ADR 0478 clause 4
r := newRig(t, goBundle)
r.bm.digests["ace/tool"] = "sha256:" + hex64('9')
v := r.consider(t, "modules/m")
if v.State != delivery.BuildFailed || !strings.Contains(v.Error, "ace") || !strings.Contains(v.Error, "novox") || !strings.Contains(v.Error, digest) || !strings.Contains(v.Error, hex64('9')) {
t.Errorf("build = %+v, want failed naming both nodes and both digests", v)
}
}
func TestOneQualifyingNodeBuildsOnceAndTheKeySaysSo(t *testing.T) { // 1607/AC4, ADR 0478 clause 4
func TestOneQualifyingNodeBuildsOnce(t *testing.T) { // 1607/AC4, ADR 0478 clause 4
r := newRig(t, goBundle)
r.tools.Put(buildrun.GoBuildModule, "ace.go", nil, true)
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || !strings.Contains(v.Step, "one node") || !strings.Contains(v.Step, "novox") {
t.Errorf("build = %+v, want done with its step saying one node, novox", v)
if v.State != delivery.BuildDone || !strings.Contains(v.Step, "on novox") {
t.Errorf("build = %+v, want done with its step saying novox", v)
}
if n := len(r.bm.callsOf("build_go_build")); n != 1 {
t.Errorf("%d builds, want one", n)
@@ -277,9 +268,6 @@ func TestAnImageGoesToOneNodeAndGetsOneDoneKeyWithItsReference(t *testing.T) { /
if v.Artifacts[0].Reference == "" {
t.Errorf("a trunk image has no reference: %+v", v.Artifacts[0])
}
if strings.HasPrefix(v.Step, OneNodePrefix) || len(OneNodeModules(v)) != 0 {
t.Errorf("step = %q, want no one-node mark: an image is never built again on a second node", v.Step)
}
if n := len(r.bm.callsOf("build_image_fetch")); n != 1 {
t.Errorf("%d image fetches, want one", n)
}
@@ -300,22 +288,19 @@ func TestAnImageIsBuiltOnlyWhereMeshBuildImageIsHealthy(t *testing.T) { // 1705a
}
}
func TestABundleAndAnImageOfOneModuleBuildOnTwoNodesAndOne(t *testing.T) { // 1705a
func TestABundleAndAnImageOfOneModuleBuildOnOneNodeEach(t *testing.T) { // 1705a
r := newRig(t, goBundle, imageArt)
r.b.Health = imageHave(t, "ace", "novox")
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || len(v.Artifacts) != 2 {
t.Fatalf("build = %+v", v)
}
if n := len(r.bm.callsOf("build_go_build")); n != 2 {
t.Errorf("%d go builds, want 2", n)
if n := len(r.bm.callsOf("build_go_build")); n != 1 {
t.Errorf("%d go builds, want 1 (one node per fingerprint, ADR 0478 clause 4)", n)
}
if n := len(r.bm.callsOf("build_image_build")); n != 1 {
t.Errorf("%d image builds, want 1", n)
}
if modules := OneNodeModules(v); len(modules) != 0 {
t.Errorf("one-node modules = %v, want none", modules)
}
}
func TestWithNoImageNodeTheKeyFailsNamingTheProvisionAndIsNotRetakenUntilANodeReportsTheModule(t *testing.T) { // 1705b, lead's decision
@@ -454,7 +439,7 @@ func TestAKeyTheBuilderLeftBuildingIsBuiltAtStartAndDoneKeysAreNotBuiltTwice(t *
// whose compare-and-set comes first, and the other's write is refused, never overwriting it.
func TestTwoInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/AC7, review round 4
r := newRig(t, goBundle)
other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Nodes: 2, Logf: t.Logf}
other := &Builder{Claimer: Claimer{Store: r.st, By: "ace.builder"}, Runner: r.b.Runner, Toolchains: r.tools, Presence: r.pres, Manifests: r.mf, Logf: t.Logf}
value := moduleValue(t, "modules/m")
var wg sync.WaitGroup
for _, b := range []*Builder{r.b, other} {
@@ -465,8 +450,8 @@ func TestTwoInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/A
if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone || len(v.Artifacts) != 1 {
t.Errorf("build = %+v, want one done key", v)
}
if n := len(r.bm.callsOf("build_go_build")); n < 2 || n%2 != 0 {
t.Errorf("%d builds, want two for each instance that built", n)
if n := len(r.bm.callsOf("build_go_build")); n < 1 {
t.Errorf("%d builds, want one for each instance that built", n)
}
}
@@ -603,43 +588,6 @@ func TestACandidatesImageIsPublishedOnceWhenTheTrunkNamesItsFingerprint(t *testi
}
}
func TestASecondGoNodeDoesNotBuildOrPushTheImageAgain(t *testing.T) { // review r1, mesh-delivery 2; ADR 0487 clause 8
r := newRig(t, goBundle, imageArt)
r.b.Health = imageHave(t, "ace", "novox")
r.pres.Put("ace", presenceValue(t, false), false) // only novox qualifies for Go (and the image)
v := r.consider(t, "modules/m")
if v.State != delivery.BuildDone || len(OneNodeModules(v)) != 1 {
t.Fatalf("build = %+v, want done with the Go bundle on one node", v)
}
var image delivery.BuiltArtifact
for _, a := range v.Artifacts {
if a.Kind == "image" {
image = a
}
}
if image.Reference == "" {
t.Fatalf("artifacts = %+v, want a published image", v.Artifacts)
}
r.pres.Put("ace", presenceValue(t, true), false)
r.b.Reconsider(context.Background())
r.b.Wait()
v = valueOf(t, r.st, r.key)
if len(OneNodeModules(v)) != 0 || len(v.Artifacts) != 2 {
t.Fatalf("build = %+v, want done on two Go nodes", v)
}
if n := len(r.bm.callsOf("build_image_build")); n != 1 {
t.Errorf("%d image builds, want the one of the first build only", n)
}
for _, a := range v.Artifacts {
if a.Kind == "image" && a != image {
t.Errorf("image = %+v, was %+v: the done key was rewritten with another image", a, image)
}
}
if n := len(r.bm.callsOf("build_go_build")); n != 3 { // one, then two
t.Errorf("%d go builds, want 3", n)
}
}
func TestAFailedPublishOfACandidatesImageIsWrittenToTheKey(t *testing.T) { // review r2, mesh-delivery 1; ADR 0487 clause 5
r := newRig(t, imageArt)
r.b.Health = imageHave(t, "ace")