From 50c55a773eb88394255e9eea0d92b04aeaf8b8cd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 21:33:12 +0200 Subject: [PATCH] Mark that the build agent takes work, so its health check can see it ready (novox/hq issue 544) The agent listens on nothing, so the node-engine could judge it only alive and held it starting for the whole grace on every machine a walk moved it to. It now writes a mark once it is bound to its worker, after its seat verbs are served, and 'mesh-builder ready' passes only while the mark is there. --- cmd/mesh-builder/main.go | 14 +++++++++- cmd/mesh-builder/ready.go | 44 +++++++++++++++++++++++++++++++ cmd/mesh-builder/ready_test.go | 32 ++++++++++++++++++++++ internal/link/builds_nats.go | 7 +++++ internal/link/builds_nats_test.go | 33 +++++++++++++++++++++++ 5 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 cmd/mesh-builder/ready.go create mode 100644 cmd/mesh-builder/ready_test.go diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 3faccf9a..e8ece0d8 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -66,6 +66,11 @@ broker to take work from or a registry to publish into: Without --registry the artifacts stay in this machine's container runtime, named by the digest of their own configuration. The result is printed as JSON. + + mesh-builder ready + +exits 0 once the agent running in this container is bound to its build work and pulls it, and +1 before: its container's health check. ` func run() error { @@ -76,12 +81,18 @@ func run() error { return nil case "build": return buildOnce(context.Background(), os.Args[2:]) + case "ready": + // The container's health check (novox/hq issue 544): exits non-zero until the agent takes work. + return ready() default: fmt.Print(usage) return nil } } + if err := notReady(); err != nil { + return err + } credential, err := brokerFrom() if err != nil { return err @@ -134,7 +145,8 @@ func run() error { // **What an earlier holder here left goes before anything is taken** (novox/hq issue 306): a holder // recreated mid-check left the check's store and bus running, and the ask went to another machine. h.removeLeftBehind() - machine := link.MachineOverNATSWith(js, on, seat, link.MachineOptions{Paused: h.Paused}) + machine := link.MachineOverNATSWith(js, on, seat, link.MachineOptions{Paused: h.Paused, + Taking: func() { taking(seat) }}) defer machine.Close() // The seat's verbs, on this machine's subjects. Only the seat that declares them: the retired diff --git a/cmd/mesh-builder/ready.go b/cmd/mesh-builder/ready.go new file mode 100644 index 00000000..09b770a6 --- /dev/null +++ b/cmd/mesh-builder/ready.go @@ -0,0 +1,44 @@ +package main + +import ( + "errors" + "fmt" + "os" + "path/filepath" +) + +// **The build agent says it is ready** (novox/hq issue 544). It listens on nothing, so no http or tcp +// check can see it work; it writes a mark once it is bound to its worker and pulls build work, and its +// container's health check runs `mesh-builder ready`, which passes only while the mark is there. Without +// it the node-engine could judge the agent only alive, and every walk that moved it waited out the whole +// grace on each machine. +// +// The mark lives in the container's own temporary directory, never in the workspace the machine keeps: +// a new container starts without it, and a restarted one removes it before it binds again. +func readyMark() string { return filepath.Join(os.TempDir(), "mesh-builder.ready") } + +// notReady removes the mark: a holder starting is not taking work until it says so again. +func notReady() error { + if err := os.Remove(readyMark()); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("cannot remove the mark that this agent takes build work (%s): %w", readyMark(), err) + } + return nil +} + +// taking writes the mark. A mark that cannot be written is said, and the agent goes on building: its +// check then says it is not ready, which is the truth the gate should hear. +func taking(seat string) { + if err := os.WriteFile(readyMark(), []byte(seat+"\n"), 0o644); err != nil { + fmt.Fprintf(os.Stderr, "cannot mark that this agent takes build work (%s): %v\n", readyMark(), err) + return + } + fmt.Fprintf(os.Stderr, "taking build work from %s\n", seat) +} + +// ready is the health check: nil when this container's agent is bound and pulling build work. +func ready() error { + if _, err := os.Stat(readyMark()); err != nil { + return fmt.Errorf("not taking build work yet: %s is not there", readyMark()) + } + return nil +} diff --git a/cmd/mesh-builder/ready_test.go b/cmd/mesh-builder/ready_test.go new file mode 100644 index 00000000..4e96679a --- /dev/null +++ b/cmd/mesh-builder/ready_test.go @@ -0,0 +1,32 @@ +package main + +import ( + "os" + "testing" +) + +// The health check passes only between the agent saying it takes work and its next start (novox/hq +// issue 544). +func TestTheAgentIsReadyOnlyOnceItTakesWork(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + if err := ready(); err == nil { + t.Fatal("an agent that never took work is ready") + } + taking("mesh-build-machine") + if err := ready(); err != nil { + t.Fatalf("an agent taking work is not ready: %v", err) + } + // Started again, it is not ready until it binds again. + if err := notReady(); err != nil { + t.Fatal(err) + } + if err := ready(); err == nil { + t.Fatal("an agent starting again is ready before it takes work") + } + if err := notReady(); err != nil { + t.Fatalf("removing a mark that is not there failed: %v", err) + } + if _, err := os.Stat(readyMark()); err == nil { + t.Fatal("the mark is still there") + } +} diff --git a/internal/link/builds_nats.go b/internal/link/builds_nats.go index 1358dd92..5e253bac 100644 --- a/internal/link/builds_nats.go +++ b/internal/link/builds_nats.go @@ -167,6 +167,10 @@ type MachineOptions struct { // Paused is asked before every fetch: while it says so, nothing new is taken, and a build // already running finishes. Nil is never paused. Paused func() bool + // Taking is called once, when this machine is bound to its worker and pulls work: from then on it + // can take a build, which is what its container's health check reads (novox/hq issue 544). Nil is + // told nothing. + Taking func() } // MachineOverNATS takes build work from the current build role. @@ -232,6 +236,9 @@ func (m *natsMachine) Take(ctx context.Context, do func(context.Context, Build)) "the mesh's to answer, not this machine's", m.seat, err) } m.sub = sub + if m.opts.Taking != nil { + m.opts.Taking() + } for { if ctx.Err() != nil { diff --git a/internal/link/builds_nats_test.go b/internal/link/builds_nats_test.go index 6b405d42..8928c60c 100644 --- a/internal/link/builds_nats_test.go +++ b/internal/link/builds_nats_test.go @@ -368,3 +368,36 @@ func TestNatsAMachineOnTheRetiredBuildRoleTakesThatRolesAsks(t *testing.T) { t.Errorf("the retired role's holder did not answer: %+v", result) } } + +// A holder says once that it takes work, and only once it is bound to its worker (novox/hq issue 544): +// its container's health check reads that, so a holder that could not bind is never said ready. +func TestNatsAHolderSaysItTakesWorkOnceItIsBound(t *testing.T) { + js := aBusWithTheBuildRole(t) + ctx, stop := context.WithCancel(context.Background()) + defer stop() + said := make(chan struct{}, 4) + machine := MachineOverNATSWith(js, "anchor", TheBuildMachine, MachineOptions{Taking: func() { said <- struct{}{} }}) + defer machine.Close() + go func() { _ = machine.Take(ctx, func(context.Context, Build) {}) }() + select { + case <-said: + case <-time.After(5 * time.Second): + t.Fatal("a holder bound to its worker never said it takes work") + } + time.Sleep(300 * time.Millisecond) + if len(said) != 0 { + t.Fatalf("a holder said it takes work %d more time(s)", len(said)) + } + + // A role the mesh raised no worker for cannot be bound, and its holder says nothing. + bounded, end := context.WithTimeout(ctx, 10*time.Second) + defer end() + other := MachineOverNATSWith(js, "anchor", "mesh-no-such-seat", MachineOptions{Taking: func() { said <- struct{}{} }}) + defer other.Close() + if err := other.Take(bounded, func(context.Context, Build) {}); err == nil { + t.Fatal("a machine with no worker took work") + } + if len(said) != 0 { + t.Fatal("a machine that could not bind said it takes work") + } +} -- 2.54.0