novox/mesh-controller
8
0
Fork 0
Code Issues Pull Requests 5 Releases Wiki Activity

Build a check's builds for its merge where the host's container engine sees them (hq issue 713)

Browse Source 
The check built in a temporary directory of the build holder's container, which the
host's engine mounted empty: every check build failed and every walk built again.
This commit is contained in:
jschoubben
2026-10-14 20:14:20 +02:00
parent 622fb1e9cb
commit afed887217
3 changed files with 160 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/builder/check.go
+30 -15
View File
@@ -406,13 +406,16 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
// **What the merge would move is built from a checkout nothing of the check touches** (novox/hq issue 703): the
// gate and the repository's own check run in containers that mount the workspace read-write, so they can change
// its tree, its objects and its caches. The builds are made from a copy of the merge's objects, cloned before
// anything of the change runs, outside the workspace — so what is kept under a key is what the key describes.
building, buildingTree := "", ""
// anything of the change runs, outside the workspace — so what is kept under a key is what the key describes. Each
// build's working copy is cloned from it into the workspace only after the check has ended (forTheMerge): the
// host's container engine sees no other directory of the holder's (novox/hq issue 713).
buildingTree := ""
if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
at := judged.Merge
if at == "" {
at = spec.Ref
}
var building string
building, buildingTree, err = checkoutForTheBuilds(ctx, run, tree, at, workspace)
if err != nil {
return CheckVerdict{}, err
@@ -750,23 +753,35 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
// **What the merge would move, built once, here** (novox/hq ADR 0350 §1): after the judging, from the merge it
// judged, into the scratch namespace — so the walk of the merge commit takes it rather than building again.
if len(spec.Builds) > 0 && spec.BuildWith != nil && len(spec.Group) == 0 {
if why := skipsTheBuilds(v); why != "" {
say("check", "%s", why)
} else {
began := time.Now()
v.Built = buildForTheMerge(ctx, run, spec, *spec.BuildWith, building, buildingTree, judged.Merge, judged, forge,
func(step, message string) {
if log != nil {
log(step, redact.redact(message))
}
}, say)
say("check", "built for the merge: %d of %d module(s) (%s)", builtCount(v.Built), len(v.Built),
time.Since(began).Round(time.Second))
}
v.Built = forTheMerge(ctx, run, spec, v, workspace, buildingTree, judged, forge, func(step, message string) {
if log != nil {
log(step, redact.redact(message))
}
}, say)
}
return named(v), nil
}
// forTheMerge builds what the merge moves once the check has judged it, from merged — the copy of the merge
// checkoutForTheBuilds made outside the workspace — **in the builder's own workspace** (novox/hq issue 713). A build's
// containers are run by the host's container engine, which mounts a path as the host names it: the build seat's
// holder runs in a container that shares only its workspace with the host, at the same path, so a working copy in
// its own temporary directory reaches the toolchain as an empty directory ("go.mod file not found"), every check
// build failed, and every walk built again. The copy stays outside the workspace, so nothing the tests ran reaches it;
// the working copy cloned from it is made only now, after every container of the check has ended.
func forTheMerge(ctx context.Context, run Runner, spec CheckSpec, verdict CheckVerdict, workspace, merged string,
judged CheckVerdict, forge GitCredential, log Log, say func(step, format string, args ...any)) []CheckBuilt {
if why := skipsTheBuilds(verdict); why != "" {
say("check", "%s", why)
return nil
}
began := time.Now()
built := buildForTheMerge(ctx, run, spec, *spec.BuildWith, workspace, merged, judged.Merge, judged, forge, log, say)
say("check", "built for the merge: %d of %d module(s) (%s)", builtCount(built), len(built),
time.Since(began).Round(time.Second))
return built
}
// checkoutForTheBuilds is a clone of the commit a check builds for its merge, made before anything of the check runs,
// outside the workspace its containers mount (novox/hq issue 703): the directory to remove, and the clone in it.
// Cloned over git's own transport, never by linking the objects: what the check's containers later write into the
internal/builder/check_builds.go
+5 -1
View File
@@ -70,7 +70,9 @@ func skipsTheBuilds(v CheckVerdict) string {
// buildForTheMerge builds each module the plan moves, tier by tier, into the scratch namespace, and keeps what it
// built under its keys. A module standing after one not built is not built either: what it stands on in the walk
// is not what it would stand on here. tree is the check's clone and merge the merge commit it made there.
// is not what it would stand on here. tree is the check's clone and merge the merge commit it made there; workspace
// is where each build's working copy is made, a directory the host's container engine sees at the same path — the
// builder's own workspace, never a temporary directory of its container (novox/hq issue 713).
func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with CheckBuilding, workspace, tree, merge string,
judged CheckVerdict, forge GitCredential, log Log, say func(step, format string, args ...any)) []CheckBuilt {
if len(spec.Builds) == 0 || with.Publish == nil || with.Scratch == nil {
@@ -80,6 +82,8 @@ func buildForTheMerge(ctx context.Context, run Runner, spec CheckSpec, with Chec
if ref == "" {
ref = spec.Ref
}
// What an earlier check left, ended before it could remove it, is not built on.
_ = os.RemoveAll(filepath.Join(workspace, "for-the-merge"))
held := map[string]string{}
for k, v := range with.Held {
held[k] = v
internal/builder/replay713_test.go
+125
View File
@@ -0,0 +1,125 @@
package builder
import (
"context"
"errors"
"path/filepath"
"strings"
"testing"
)
// novox/hq issue 713: after ADR 0350 the walk of novox/mesh-controller#331 built mesh-controller, build-agent and
// route-proxy again, each "built: no check of novox/mesh-controller#331 built it". The check had built none: its
// build log said "built for the merge: 0 of 3 module(s)", mesh-controller's go bundle failing with "go.mod file not
// found". The build seat's holder runs in a container sharing only its workspace with the host, and a build's
// toolchain container is run by the host's container engine, which mounts a path as the host names it. The check
// built in a temporary directory of the holder's own (issue 703's copy), which the engine mounted as an empty one.
// aHostsEngine runs as the host's container engine does for a holder in a container: a mount of a path outside the
// workspace the holder shares with the host is an empty directory, and a Go toolchain finds no go.mod in it.
type aHostsEngine struct {
onTrunk
shared string
}
func (h aHostsEngine) run(ctx context.Context, dir, name string, args ...string) (string, error) {
if name == "docker" && len(args) > 0 && args[0] == "run" {
for i, a := range args {
if a != "--volume" || i+1 >= len(args) {
continue
}
from, _, _ := strings.Cut(args[i+1], ":")
if !withinDir(h.shared, from) {
return "", errors.New("go: go.mod file not found in current directory or any parent directory")
}
}
}
return h.onTrunk.run(ctx, dir, name, args...)
}
// aGoModule is a module of one Go bundle built from a repository's root.
func aGoModule(module string) map[string]string {
files := aSharedRepository("one", "1")
files[ManifestName] = `{"module":"` + module + `","version":"1","build":{"artifacts":[
{"name":"server","kind":"bundle","language":"go","system":"arch","from":"cmd/ctl","binary":"ctl"}]},
"resources":[{"id":"server","type":"process","name":"` + module + `","artifact":"server","run":["./ctl"]}]}`
return files
}
// A controller change whose walk moves mesh-controller, build-agent and route-proxy (#331's) has its check build all
// three, in the workspace the host's engine sees, and the walk of its merge takes each from the check by its key.
func TestReplay713TheWalkOfAControllerChangeTakesAllThreeFromItsCheck(t *testing.T) {
withVersion(t, "builder-1")
pull := PullRef{Owner: "novox", Repo: "mesh-controller", Number: 331}
held := map[string]string{"mesh-tools-go/build": "registry.invalid/mesh-tools-go/build@sha256:" + strings.Repeat("b", 64)}
repositories := map[string]string{"build-agent": "https://forge.invalid/mesh-catalog-build-agent.git",
"route-proxy": "https://forge.invalid/mesh-catalog-route-proxy.git"}
checked := func(shared string) (*recorded, aHostsEngine) {
r := &recorded{tree: "tree-of-the-merge", contents: aGoModule("mesh-controller"),
secondary: map[string]map[string]string{}}
for module, repository := range repositories {
r.secondary[repository] = aGoModule(module)
}
return r, aHostsEngine{onTrunk: onTrunk{recorded: r}, shared: shared}
}
spec := CheckSpec{ID: "build-331", Owner: pull.Owner, Repo: pull.Repo, Number: pull.Number,
Builds: []CheckBuildSpec{{Module: "mesh-controller", Merged: true, Tier: 0},
{Module: "build-agent", Repository: repositories["build-agent"], Ref: "main", Tier: 1},
{Module: "route-proxy", Repository: repositories["route-proxy"], Ref: "main", Tier: 2}}}
judged := CheckVerdict{Head: "98e856874913", Trunk: "c975dbf7b053", Merge: "2d22279b1d07"}
passed := CheckVerdict{Gate: &Layer{Verdict: "pass"}, Repo: &Layer{Verdict: "pass"}}
// As it ran: the builds made in the holder's own temporary directory, which the host's engine does not see.
shared, outside := t.TempDir(), t.TempDir()
merged := filepath.Join(outside, "merged")
r, engine := checked(shared)
asItRan := buildForTheMerge(t.Context(), engine.run, spec, CheckBuilding{Publish: r, Scratch: newScratch(), Held: held},
outside, merged, judged.Merge, judged, GitCredential{}, nil, logging(nil))
if builtCount(asItRan) != 0 {
t.Fatalf("the engine of this test sees the holder's temporary directory: %+v", asItRan)
}
// The check as it builds now: every module its merge moves, kept under its key.
scratch := newScratch()
spec.BuildWith = &CheckBuilding{Publish: r, Scratch: scratch, Held: held}
built := forTheMerge(t.Context(), engine.run, spec, passed, shared, merged, judged, GitCredential{}, nil,
logging(nil))
if len(built) != 3 || builtCount(built) != 3 {
t.Fatalf("the check built %d of %d module(s) its merge moves: %+v", builtCount(built), len(built), built)
}
var names []NamedBuild
for _, b := range built {
n := NamedBuild{Pull: pull, Check: spec.ID, Module: b.Module, Key: b.Key, Digests: map[string]string{}}
for _, m := range b.Made {
d := m.Digest
if d == "" {
d = DigestOf(m.Reference)
}
n.Digests[m.Name] = d
}
names = append(names, n)
}
// The walk of the merge commit, tier by tier: each module taken from the check, none built.
walked := map[string]string{"mesh-controller": "https://forge.invalid/mesh-controller.git"}
for module, repository := range repositories {
walked[module] = repository
}
for _, module := range []string{"mesh-controller", "build-agent", "route-proxy"} {
w, walk := checked(shared)
w.secondary[walked["mesh-controller"]] = aGoModule("mesh-controller")
got, err := BuildReusing(t.Context(), walk.run, w, Reuse{Scratch: scratch, Pulls: []PullRef{pull}, Named: names},
walked[module], "", "main", t.TempDir(), held, Npmrc{}, GitCredential{}, nil)
if err != nil {
t.Fatalf("%s: %v", module, err)
}
if got.Reused == nil || got.Why != "" {
t.Fatalf("the walk built %s again: %s", module, got.Why)
}
for _, line := range w.ran {
if strings.HasPrefix(line, "docker run") && strings.Contains(line, "go build") {
t.Fatalf("the walk compiled %s: %s", module, line)
}
}
}
}