Keep a try off the forge and bound how many run (hq issue 686)
mesh/merge-gate head 3ce5a0239bb1 merged into trunk 2d18f438c516 — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 3ce5a0239bb1 merged into trunk 2d18f438c516 — pass: THE CHANGE ALTERS ITS OWN CHECK (merge-check.sh): main's version judged it; the ch…
mesh/delivery delivered
mesh/merge-gate head 3ce5a0239bb1 merged into trunk 2d18f438c516 — pass: builds build-agent, mesh-controller → ace, g14, novox, shanks; no bus step; every …
mesh/repo-check head 3ce5a0239bb1 merged into trunk 2d18f438c516 — pass: THE CHANGE ALTERS ITS OWN CHECK (merge-check.sh): main's version judged it; the ch…
mesh/delivery delivered
Review of #314: a try's start named its commit, which the forge's holder matches to an open pull request's head and marks pending. Its start now names no repository or commit. One try per commit and four in all, a try never ages ahead of deliveries or pull request checks, and its id is checked before it is a bus subject.
This commit is contained in:
@@ -87,6 +87,26 @@ func asATry(request link.BuildRequest, tests string, storeOnly bool) link.BuildR
|
|||||||
return request
|
return request
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// maxTries is how many tries may wait or run at once, mesh-wide: each holds a build machine for minutes.
|
||||||
|
const maxTries = 4
|
||||||
|
|
||||||
|
// triesIn reads the build queue for a new try of one commit: the id of a try of that commit already waiting or
|
||||||
|
// running — the answer to a second ask — and whether there is room for another.
|
||||||
|
func triesIn(q link.Queue, owner, repo, commit string) (same string, room bool) {
|
||||||
|
n := 0
|
||||||
|
for _, a := range q.Asks {
|
||||||
|
if !link.IsTry(a.Request) || (a.State != link.AskWaiting && a.State != link.AskInFlight) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
n++
|
||||||
|
c := a.Request.Check
|
||||||
|
if same == "" && strings.EqualFold(c.Owner, owner) && strings.EqualFold(c.Repo, repo) && a.Request.Ref == commit {
|
||||||
|
same = a.ID
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return same, n < maxTries
|
||||||
|
}
|
||||||
|
|
||||||
// forgeCommit asks the forge, through its module's tools on the bus, which commit a branch or commit of a
|
// forgeCommit asks the forge, through its module's tools on the bus, which commit a branch or commit of a
|
||||||
// repository is, and the repository's default branch. A variable so a test needs no forge.
|
// repository is, and the repository's default branch. A variable so a test needs no forge.
|
||||||
var forgeCommit = func(ctx context.Context, owner, repo, ref string) (commit, trunk string, err error) {
|
var forgeCommit = func(ctx context.Context, owner, repo, ref string) (commit, trunk string, err error) {
|
||||||
@@ -148,6 +168,10 @@ func tryCommand(ctx context.Context, args []string) error {
|
|||||||
if len(positionals) != 0 {
|
if len(positionals) != 0 {
|
||||||
return usage
|
return usage
|
||||||
}
|
}
|
||||||
|
// Before it is a token of a bus subject: an id, never a pattern.
|
||||||
|
if !link.SafeBuildID(*id) {
|
||||||
|
return fmt.Errorf("%q is not a try's id", *id)
|
||||||
|
}
|
||||||
return tryRead(ctx, *id)
|
return tryRead(ctx, *id)
|
||||||
}
|
}
|
||||||
if len(positionals) != 2 {
|
if len(positionals) != 2 {
|
||||||
@@ -202,6 +226,26 @@ func tryCommand(ctx context.Context, args []string) error {
|
|||||||
}
|
}
|
||||||
request = asATry(request, *tests, *storeOnly)
|
request = asATry(request, *tests, *storeOnly)
|
||||||
seat := buildSeatHeld(ctx)
|
seat := buildSeatHeld(ctx)
|
||||||
|
// **Bounded in number** (novox/hq issue 686): one try of a commit at a time, and maxTries in all.
|
||||||
|
js, err := dialTheBus()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
queue, err := link.ReadQueue(ctx, js, seat)
|
||||||
|
js.Close()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
same, room := triesIn(queue, owner, repo, commit)
|
||||||
|
if same != "" {
|
||||||
|
return printTry(tryAnswer{ID: same, State: "asked", Repository: owner + "/" + repo, Ref: ref, Commit: commit,
|
||||||
|
On: seat, Said: fmt.Sprintf("a try of %s/%s at %.12s already waits or runs, as %s: nothing new was asked. "+
|
||||||
|
"Read it with try id=%s", owner, repo, commit, same, same)})
|
||||||
|
}
|
||||||
|
if !room {
|
||||||
|
return fmt.Errorf("%d tries already wait or run on the build seat, the most at once (`queue` lists them); "+
|
||||||
|
"nothing was asked — read one of yours, or ask again when one has ended", maxTries)
|
||||||
|
}
|
||||||
ask, err := askOverOn(seat)
|
ask, err := askOverOn(seat)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|||||||
@@ -131,6 +131,38 @@ func TestATrysAnswerCarriesTheFailingLines(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// One try of a commit at a time, and four in all (novox/hq issue 686): a second ask is answered the first's id,
|
||||||
|
// a fifth is refused; a try that ended, and a pull request's check, count for nothing.
|
||||||
|
func TestTriesAreBoundedInNumber(t *testing.T) {
|
||||||
|
ask := func(id, repo, ref, state string, try bool) link.QueuedAsk {
|
||||||
|
return link.QueuedAsk{ID: id, State: state, Request: link.BuildRequest{ID: id, Ref: ref,
|
||||||
|
Check: &link.CheckRequest{Owner: "novox", Repo: repo, Try: try}}}
|
||||||
|
}
|
||||||
|
q := link.Queue{Asks: []link.QueuedAsk{
|
||||||
|
ask("t1", "a", "c1", link.AskInFlight, true), ask("t2", "a", "c2", link.AskWaiting, true),
|
||||||
|
ask("t3", "b", "c1", link.AskDead, true), ask("p1", "a", "c9", link.AskWaiting, false)}}
|
||||||
|
if same, room := triesIn(q, "novox", "a", "c2"); same != "t2" || !room {
|
||||||
|
t.Fatalf("a second try of a commit: %q %v", same, room)
|
||||||
|
}
|
||||||
|
if same, room := triesIn(q, "novox", "b", "c1"); same != "" || !room {
|
||||||
|
t.Fatalf("a dead try, or another repository's, was taken for this one: %q %v", same, room)
|
||||||
|
}
|
||||||
|
if same, _ := triesIn(q, "novox", "a", "c9"); same != "" {
|
||||||
|
t.Fatalf("a pull request's check was taken for a try: %q", same)
|
||||||
|
}
|
||||||
|
q.Asks = append(q.Asks, ask("t4", "c", "c3", link.AskWaiting, true), ask("t5", "d", "c4", link.AskWaiting, true))
|
||||||
|
if _, room := triesIn(q, "novox", "e", "c5"); room {
|
||||||
|
t.Fatal("a fifth try was let in")
|
||||||
|
}
|
||||||
|
// An id is checked before it is a token of a bus subject.
|
||||||
|
for _, id := range []string{"x.>", "*", "a b"} {
|
||||||
|
if err := tryCommand(context.Background(), []string{"--id", id}); err == nil ||
|
||||||
|
!strings.Contains(err.Error(), "not a try's id") {
|
||||||
|
t.Errorf("try --id %q: %v", id, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// The verb runs its command, and is declared with the shell habit it replaces (novox/hq ADR 0245).
|
// The verb runs its command, and is declared with the shell habit it replaces (novox/hq ADR 0245).
|
||||||
func TestTheTryVerbRunsItsCommand(t *testing.T) {
|
func TestTheTryVerbRunsItsCommand(t *testing.T) {
|
||||||
for _, c := range []struct {
|
for _, c := range []struct {
|
||||||
|
|||||||
@@ -369,11 +369,14 @@ var ControllerVerbs = []Verb{
|
|||||||
"once with an id; asked again with that id it waits up to 20 s and answers pass, fail or error, how long it " +
|
"once with an id; asked again with that id it waits up to 20 s and answers pass, fail or error, how long it " +
|
||||||
"took and the lines naming what failed — or that it is still waiting or running, to ask again. Sets no " +
|
"took and the lines naming what failed — or that it is still waiting or running, to ask again. Sets no " +
|
||||||
"status on the forge, registers and builds nothing, runs no merge gate, counts as a check in the build queue " +
|
"status on the forge, registers and builds nothing, runs no merge gate, counts as a check in the build queue " +
|
||||||
"(after deliveries) and is bounded as one. Only for a repository of the mesh; the ref must be on the forge.",
|
"(after deliveries and after every pull request's check, however long it waits) and is bounded as one. One " +
|
||||||
|
"try of a commit at a time — a second ask answers the first's id — and at most four waiting or running in " +
|
||||||
|
"all: a fifth is refused. Only for a repository of the mesh; the ref must be on the forge.",
|
||||||
Input: schema(map[string]string{
|
Input: schema(map[string]string{
|
||||||
"repository": "the repository on the forge, owner/name; with ref",
|
"repository": "the repository on the forge, owner/name; with ref",
|
||||||
"ref": "the branch or commit to try, as pushed to the forge",
|
"ref": "the branch or commit to try, as pushed to the forge",
|
||||||
"base": "the branch it is merged into (the repository's default branch when none is named)",
|
"base": "the branch it is merged into (the repository's default branch when none is named). The " +
|
||||||
|
"merge-check.sh that runs is that branch's, never the ref's own: the same trust as a pull request's base",
|
||||||
"tests": "only the tests this `go test -run` pattern names, where the repository's merge-check.sh reads " +
|
"tests": "only the tests this `go test -run` pattern names, where the repository's merge-check.sh reads " +
|
||||||
"MESH_CHECK_TESTS; a script that does not runs whole",
|
"MESH_CHECK_TESTS; a script that does not runs whole",
|
||||||
"store-only": "\"true\": only the repository's store tests, where its merge-check.sh reads " +
|
"store-only": "\"true\": only the repository's store tests, where its merge-check.sh reads " +
|
||||||
|
|||||||
@@ -96,8 +96,27 @@ type BuildStart struct {
|
|||||||
// an earlier taker left unfinished — most often a build agent replaced mid-build — and started
|
// an earlier taker left unfinished — most often a build agent replaced mid-build — and started
|
||||||
// again from the beginning (novox/hq issue 594). 0 from a machine that predates it.
|
// again from the beginning (novox/hq issue 594). 0 from a machine that predates it.
|
||||||
Delivery int `json:"delivery,omitempty"`
|
Delivery int `json:"delivery,omitempty"`
|
||||||
|
// Try marks the start of a branch tried before its pull request (novox/hq issue 686), which names no
|
||||||
|
// repository and no ref (startOf).
|
||||||
|
Try bool `json:"try,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// startOf is what a build machine says of the ask it took. **A try's start names no repository and no commit**
|
||||||
|
// (novox/hq issue 686): the forge's holder writes mesh/merge-gate pending on an open pull request whose head is
|
||||||
|
// the commit a start names, and a try of that head is no check of the pull request — nothing on the forge moves
|
||||||
|
// for a try. Its id, machine and time are all the queue reads of a start.
|
||||||
|
func startOf(r BuildRequest, on string, delivery int, at time.Time) BuildStart {
|
||||||
|
s := BuildStart{ID: r.ID, Repository: r.Repository, Path: r.Path, Ref: r.Ref, On: on,
|
||||||
|
At: at.UTC().Format(time.RFC3339Nano), Delivery: delivery}
|
||||||
|
if IsTry(r) {
|
||||||
|
s.Repository, s.Path, s.Ref, s.Try = "", "", "", true
|
||||||
|
}
|
||||||
|
return s
|
||||||
|
}
|
||||||
|
|
||||||
|
// IsTry says an ask is a branch tried before its pull request (novox/hq issue 686).
|
||||||
|
func IsTry(r BuildRequest) bool { return r.Check != nil && r.Check.Try }
|
||||||
|
|
||||||
// Restarted is how a build handed out again says so (novox/hq issue 594), in its log and in its check's
|
// Restarted is how a build handed out again says so (novox/hq issue 594), in its log and in its check's
|
||||||
// report, so a second run never reads as the first. Neutral on why: a holder replaced mid-build, a paused
|
// report, so a second run never reads as the first. Neutral on why: a holder replaced mid-build, a paused
|
||||||
// holder handing an ask back as it arrived, and a bus that held its acknowledgements past the wait all
|
// holder handing an ask back as it arrived, and a bus that held its acknowledgements past the wait all
|
||||||
|
|||||||
@@ -401,10 +401,7 @@ func (b *natsBuild) Done() error { return b.msg.Ack() }
|
|||||||
// Began publishes that this machine has taken the build, into the stream like the outcome, so a
|
// Began publishes that this machine has taken the build, into the stream like the outcome, so a
|
||||||
// reader that asks afterwards sees when it started as well as how it ended.
|
// reader that asks afterwards sees when it started as well as how it ended.
|
||||||
func (b *natsBuild) Began(ctx context.Context) error {
|
func (b *natsBuild) Began(ctx context.Context) error {
|
||||||
body, err := json.Marshal(BuildStart{
|
body, err := json.Marshal(startOf(b.request, b.on, b.delivery, time.Now()))
|
||||||
ID: b.request.ID, Repository: b.request.Repository, Path: b.request.Path,
|
|
||||||
Ref: b.request.Ref, On: b.on, At: time.Now().UTC().Format(time.RFC3339Nano), Delivery: b.delivery,
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -427,6 +427,9 @@ func ReadCheckBuilds(ctx context.Context, js *broker.JetStream, seat string, pul
|
|||||||
// safeKey is an id that can be a key, and a subject token, as it is: a build id, never a pattern.
|
// safeKey is an id that can be a key, and a subject token, as it is: a build id, never a pattern.
|
||||||
var safeKey = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
|
var safeKey = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
|
||||||
|
|
||||||
|
// SafeBuildID says an id can be a subject token as it is: a build's id, never a pattern.
|
||||||
|
func SafeBuildID(id string) bool { return safeKey.MatchString(id) }
|
||||||
|
|
||||||
// Cancelled is what the controller writes for an ask it cancelled.
|
// Cancelled is what the controller writes for an ask it cancelled.
|
||||||
type Cancelled struct {
|
type Cancelled struct {
|
||||||
At string `json:"at"`
|
At string `json:"at"`
|
||||||
|
|||||||
@@ -26,6 +26,8 @@ import (
|
|||||||
// - a check that has waited CheckWaitsAtMost goes before every delivery asked after it, so no check
|
// - a check that has waited CheckWaitsAtMost goes before every delivery asked after it, so no check
|
||||||
// waits for ever behind a stream of deliveries;
|
// waits for ever behind a stream of deliveries;
|
||||||
// - within a kind, first come, first served, by when it was asked;
|
// - within a kind, first come, first served, by when it was asked;
|
||||||
|
// - a branch tried before its pull request goes after everything else, however long it has waited: it is
|
||||||
|
// never promoted ahead of a delivery, and never ahead of a pull request's check (novox/hq issue 686);
|
||||||
// - an ask whose kind cannot be told counts as a check: a delivery taken late breaks a budget, a check
|
// - an ask whose kind cannot be told counts as a check: a delivery taken late breaks a budget, a check
|
||||||
// taken late breaks nothing.
|
// taken late breaks nothing.
|
||||||
//
|
//
|
||||||
@@ -64,8 +66,12 @@ func (a QueuedAsk) askedOrStored() time.Time {
|
|||||||
// QueueOrder is the order the waiting asks are to be handed out in, now: the deliveries and every check
|
// QueueOrder is the order the waiting asks are to be handed out in, now: the deliveries and every check
|
||||||
// that has waited CheckWaitsAtMost, by when each was asked, then the other checks, by when each was asked.
|
// that has waited CheckWaitsAtMost, by when each was asked, then the other checks, by when each was asked.
|
||||||
func QueueOrder(waiting []QueuedAsk, now time.Time) []QueuedAsk {
|
func QueueOrder(waiting []QueuedAsk, now time.Time) []QueuedAsk {
|
||||||
var first, then []QueuedAsk
|
var first, then, tries []QueuedAsk
|
||||||
for _, a := range waiting {
|
for _, a := range waiting {
|
||||||
|
if IsTry(a.Request) {
|
||||||
|
tries = append(tries, a)
|
||||||
|
continue
|
||||||
|
}
|
||||||
if KindOf(a.Request) == KindCheck && now.Sub(a.askedOrStored()) < CheckWaitsAtMost {
|
if KindOf(a.Request) == KindCheck && now.Sub(a.askedOrStored()) < CheckWaitsAtMost {
|
||||||
then = append(then, a)
|
then = append(then, a)
|
||||||
continue
|
continue
|
||||||
@@ -82,7 +88,8 @@ func QueueOrder(waiting []QueuedAsk, now time.Time) []QueuedAsk {
|
|||||||
}
|
}
|
||||||
byAsking(first)
|
byAsking(first)
|
||||||
byAsking(then)
|
byAsking(then)
|
||||||
return append(first, then...)
|
byAsking(tries)
|
||||||
|
return append(append(first, then...), tries...)
|
||||||
}
|
}
|
||||||
|
|
||||||
// toMove is the asks to move to the tail, in this order, for the queue to stand as order says: everything
|
// toMove is the asks to move to the tail, in this order, for the queue to stand as order says: everything
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
package link
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"reflect"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The start of a try names no repository and no commit (novox/hq issue 686): the forge's holder writes
|
||||||
|
// mesh/merge-gate pending on the open pull request whose head a start names, and nothing on the forge moves
|
||||||
|
// for a try. A pull request's check still names both.
|
||||||
|
func TestATrysStartNamesNoCommitTheForgeCouldMatch(t *testing.T) {
|
||||||
|
sha := strings.Repeat("c0ffee", 7)[:40]
|
||||||
|
at := time.Date(2026, 10, 14, 0, 0, 0, 0, time.UTC)
|
||||||
|
try := BuildRequest{ID: "build-1", Repository: "https://forge/novox/a.git", Ref: sha,
|
||||||
|
Check: &CheckRequest{Owner: "novox", Repo: "a", Try: true}}
|
||||||
|
body, _ := json.Marshal(startOf(try, "ace", 1, at))
|
||||||
|
if strings.Contains(string(body), sha) || strings.Contains(string(body), "novox/a") {
|
||||||
|
t.Fatalf("a try's start names what the forge matches a pull request by: %s", body)
|
||||||
|
}
|
||||||
|
if s := startOf(try, "ace", 2, at); s.ID != "build-1" || s.On != "ace" || !s.Try || s.Delivery != 2 || s.At == "" {
|
||||||
|
t.Fatalf("a try's start lost what the queue reads: %+v", s)
|
||||||
|
}
|
||||||
|
pull := try
|
||||||
|
pull.Check = &CheckRequest{Owner: "novox", Repo: "a", Number: 7}
|
||||||
|
if s := startOf(pull, "ace", 1, at); s.Ref != sha || s.Repository == "" || s.Try {
|
||||||
|
t.Fatalf("a pull request's check's start: %+v", s)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A try stays behind every delivery and every pull request's check, however long it has waited: the ten
|
||||||
|
// minutes that put a check ahead of later deliveries never promote a try (novox/hq issue 686).
|
||||||
|
func TestATryIsNeverHandedOutBeforeADeliveryOrAPullRequestsCheck(t *testing.T) {
|
||||||
|
now := time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC)
|
||||||
|
try := func(seq uint64, id string, asked time.Time) QueuedAsk {
|
||||||
|
a := anAsk(seq, id, asked, true)
|
||||||
|
a.Request.Check.Try = true
|
||||||
|
return a
|
||||||
|
}
|
||||||
|
waiting := []QueuedAsk{
|
||||||
|
try(1, "try-old", now.Add(-40*time.Minute)),
|
||||||
|
try(2, "try-young", now.Add(-time.Minute)),
|
||||||
|
anAsk(3, "check-young", now.Add(-2*time.Minute), true),
|
||||||
|
anAsk(4, "delivery", now.Add(-time.Minute), false),
|
||||||
|
}
|
||||||
|
want := []string{"delivery", "check-young", "try-old", "try-young"}
|
||||||
|
if got := idsOf(QueueOrder(waiting, now)); !reflect.DeepEqual(got, want) {
|
||||||
|
t.Fatalf("handed out as %v, want %v", got, want)
|
||||||
|
}
|
||||||
|
if !SafeBuildID("build-20261014T000000-1a2b") || SafeBuildID("x.>") || SafeBuildID("*") || SafeBuildID("") {
|
||||||
|
t.Fatal("an id that is a pattern passes for a build's id")
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user