diff --git a/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go b/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go index 7ff120c7..07fa1e5b 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go @@ -70,7 +70,7 @@ func newMachine(t *testing.T) (*Module, *fakeMachine, string) { t.Helper() home, cache, work := t.TempDir(), t.TempDir(), t.TempDir() f := &fakeMachine{now: time.Unix(1_000_000, 0), cost: map[string]time.Duration{}, answer: map[string]forgekey.Ran{}} - m := &Module{Node: "ace", Recipe: Recipe{Steps: "1", Node: "v26.10.0", Lock: "lock"}, + m := &Module{Node: "ace", Recipe: Recipe{Steps: "1", Lock: "lock"}, Toolchain: &Toolchain{Paths: Paths{Home: home, Cache: cache}, Registry: func() (Registry, error) { return Registry{URL: "https://git.novox.be/api/packages/novox/npm/"}, nil }}, diff --git a/modules/mesh-build-node/cmd/mesh-build-node/module.go b/modules/mesh-build-node/cmd/mesh-build-node/module.go index f237ca62..f5364cab 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/module.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/module.go @@ -53,23 +53,35 @@ func (m *Module) recipe() Recipe { return m.Recipe } -func (m *Module) guard(r Recipe) buildstep.Guard { - return buildstep.Guard{Language: "typescript", Want: r.Node, Recipe: r.Version(), Probe: m.Probe, Node: m.Node, Put: m.reportingPut()} +// guard is the SDK guard for the version the probe just read: the module names no Node version (ADR 0477 §2), so the +// version wanted is the one installed and the guard refuses only when node cannot be run, writing the installed +// version to .typescript of the toolchains state every time. +func (m *Module) guard(r Recipe, installed string, probeErr error) buildstep.Guard { + return buildstep.Guard{Language: "typescript", Want: installed, Recipe: r.Version(), Node: m.Node, Put: m.reportingPut(), + Probe: func(context.Context) (string, error) { return installed, probeErr }} } // Check reads the installed toolchain, writes .typescript of the toolchains state and answers nil, or the -// refusal (buildstep.Guard.Check). It runs at start, before every step, at every health look and every CheckRound. +// refusal when node cannot be run (buildstep.Guard.Check); another version than another node's is no refusal (ADR 0477 +// §2). It runs at start, before every step, at every health look and every CheckRound. func (m *Module) Check(ctx context.Context) error { + _, err := m.check(ctx) + return err +} + +// check is Check that also answers the version it found installed. +func (m *Module) check(ctx context.Context) (string, error) { if m.Reload != nil { r, err := m.Reload() if err != nil { - return &buildstep.Refusal{Text: err.Error(), Want: RecipeNode} + return "", &buildstep.Refusal{Text: err.Error()} } m.mu.Lock() m.Recipe = r m.mu.Unlock() } - return m.guard(m.recipe()).Check(ctx) + installed, err := m.Probe(ctx) + return installed, m.guard(m.recipe(), installed, err).Check(ctx) } // Prepare is Check and then the toolchain directory of the recipe in force: what every step runs behind. @@ -99,20 +111,20 @@ func (m *Module) Start(ctx context.Context) { } } -// Health is the module's health (the runtime-health tool, ADR 0366): unhealthy while the toolchain installed is not -// the recipe's, saying both versions, so the controller raises the node's condition. It looks again each time it is +// Health is the module's health (the runtime-health tool, ADR 0366): unhealthy only while node cannot be run (or the lock cannot +// be read), saying why, so the controller raises the node's condition; healthy it names the installed version. It looks again each time it is // asked. func (m *Module) Health(ctx context.Context) health.Answer { - if err := m.Check(ctx); err != nil { + installed, err := m.check(ctx) + if err != nil { return health.Unhealthy(err.Error()) } - r := m.recipe() - return health.Healthy(fmt.Sprintf("node %s installed, as the recipe %s builds with", r.Node, r.Version())) + return health.Healthy(fmt.Sprintf("node %s installed, the version the module builds with (recipe %s)", installed, m.recipe().Version())) } // Long is a tool that runs a long step (ADR 0469 §2): it answers at once with the step's id, and the step ends on -// its key in the steps state. The guard runs inside the step, before run, so a node that refuses ends every step -// refused with both versions named. +// its key in the steps state. The guard runs inside the step, before run, so a node whose node cannot be run ends every +// step refused with the cause named. func (m *Module) Long(tool string, run func(ctx context.Context, args map[string]any) (any, error)) func(map[string]any) (any, error) { return func(args map[string]any) (any, error) { id, err := buildstep.Start(m.Ctx, m.Steps, m.Node, tool, func(ctx context.Context) (any, error) { @@ -145,7 +157,7 @@ func (m *Module) Status(ctx context.Context) map[string]any { err := m.Check(ctx) installed, _ := m.Probe(ctx) r := m.recipe() - out := map[string]any{"node": m.Node, "recipe": r.Version(), "node-version": r.Node, "installed": installed, "refusing": err != nil} + out := map[string]any{"node": m.Node, "recipe": r.Version(), "installed": installed, "refusing": err != nil} if m.Toolchain != nil { out["toolchain-dir"] = m.Toolchain.Paths.Dir(r.Version()) out["toolchain-installed"] = m.Toolchain.Present(r.Version()) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/module_test.go b/modules/mesh-build-node/cmd/mesh-build-node/module_test.go index a698a184..552c1048 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/module_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/module_test.go @@ -74,7 +74,7 @@ func (s *keptStates) waitEnd(t *testing.T) delivery.Step { } func moduleWith(states *keptStates, installed string) *Module { - return &Module{Node: "ace", Recipe: Recipe{Steps: "1", Node: "v26.10.0", Lock: "lock"}, + return &Module{Node: "ace", Recipe: Recipe{Steps: "1", Lock: "lock"}, Probe: func(context.Context) (string, error) { return installed, nil }, Toolchains: states.putToolchain, Steps: states.putStep, Ctx: context.Background()} } @@ -102,11 +102,14 @@ func TestStartWritesTheToolchainAndHealthIsHealthy(t *testing.T) { } } -// 1488/AC3: a probe answering v26.11.0: every step tool's step ends refused naming both versions, ace.typescript -// holds v26.11.0, and the health tool answers unhealthy naming both. -func TestAnotherNodeRefusesEveryStepAndHealthIsUnhealthy(t *testing.T) { +// 1572/AC2 (ADR 0477 §2): a node that cannot be run: every step tool's step ends refused naming the cause, and the +// health tool answers unhealthy naming it. +func TestANodeThatCannotBeRunRefusesEveryStepAndHealthIsUnhealthy(t *testing.T) { states := newStates() - m := moduleWith(states, "v26.11.0") + m := moduleWith(states, "") + m.Probe = func(context.Context) (string, error) { + return "", errors.New("exec: node: executable file not found in $PATH") + } ran := false run := func(context.Context, map[string]any) (any, error) { ran = true; return "done", nil } steps := append([]StepTool{ @@ -137,19 +140,16 @@ func TestAnotherNodeRefusesEveryStepAndHealthIsUnhealthy(t *testing.T) { } reason = err.Error() } - if !strings.Contains(reason, "v26.11.0") || !strings.Contains(reason, "v26.10.0") { - t.Errorf("%s: the refusal %q does not name both versions", tool.Name, reason) + if !strings.Contains(reason, "executable file not found") { + t.Errorf("%s: the refusal %q does not name the cause", tool.Name, reason) } } if ran { t.Errorf("a step ran on a node that refuses") } - if tc, _ := states.toolchain("ace.typescript"); tc.Version != "v26.11.0" { - t.Errorf("ace.typescript holds %q, want the installed v26.11.0", tc.Version) - } a := m.Health(context.Background()) - if a.State != health.StateUnhealthy || !strings.Contains(a.Why, "v26.11.0") || !strings.Contains(a.Why, "v26.10.0") { - t.Errorf("health = %+v, want unhealthy naming both versions", a) + if a.State != health.StateUnhealthy || !strings.Contains(a.Why, "executable file not found") { + t.Errorf("health = %+v, want unhealthy naming the cause", a) } } @@ -162,19 +162,18 @@ func isLong(steps []StepTool, name string) bool { return false } -// 1488/AC2: the recipe version moves with one byte of the lock and with the Node version, and with the recipe's own -// version; equal recipes have the same one. +// 1488/AC2, 1572/AC3: the recipe version moves with one byte of the lock and with the recipe's own version; equal +// recipes have the same one. func TestRecipeVersionMovesWithEachOfItsValues(t *testing.T) { lock := []byte(`{"lockfileVersion":3}`) - base := Recipe{Steps: "1", Node: "v26.10.0", Lock: LockHash(lock)} - if base.Version() != (Recipe{Steps: "1", Node: "v26.10.0", Lock: LockHash(lock)}).Version() { + base := Recipe{Steps: "1", Lock: LockHash(lock)} + if base.Version() != (Recipe{Steps: "1", Lock: LockHash(lock)}).Version() { t.Errorf("one recipe has two versions") } seen := map[string]string{base.Version(): "base"} for name, r := range map[string]Recipe{ - "one byte of the lock": {Steps: "1", Node: "v26.10.0", Lock: LockHash([]byte(`{"lockfileVersion":4}`))}, - "node": {Steps: "1", Node: "v26.11.0", Lock: LockHash(lock)}, - "steps": {Steps: "2", Node: "v26.10.0", Lock: LockHash(lock)}, + "one byte of the lock": {Steps: "1", Lock: LockHash([]byte(`{"lockfileVersion":4}`))}, + "steps": {Steps: "2", Lock: LockHash(lock)}, } { v := r.Version() if was, dup := seen[v]; dup { @@ -193,7 +192,7 @@ func TestReadRecipeHashesTheLockFile(t *testing.T) { } os.WriteFile(path, []byte("a"), 0o600) a, err := ReadRecipe(path) - if err != nil || a.Node != "v26.10.0" { + if err != nil || a.Steps != RecipeSteps { t.Fatalf("= %+v, %v", a, err) } os.WriteFile(path, []byte("b"), 0o600) @@ -207,7 +206,7 @@ func TestReadRecipeHashesTheLockFile(t *testing.T) { func TestACheckReadsTheRecipeAgain(t *testing.T) { states := newStates() m := moduleWith(states, "v26.10.0") - r2 := Recipe{Steps: "1", Node: "v26.10.0", Lock: "other"} + r2 := Recipe{Steps: "1", Lock: "other"} m.Reload = func() (Recipe, error) { return r2, nil } if err := m.Check(context.Background()); err != nil { t.Fatal(err) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/probe.go b/modules/mesh-build-node/cmd/mesh-build-node/probe.go index e2f99dff..70ddd21b 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/probe.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/probe.go @@ -11,7 +11,7 @@ import ( // ProbeTimeout bounds `node --version`. const ProbeTimeout = 15 * time.Second -// ProbeNode reads the installed Node version: what `node --version` prints ("v26.10.0"), as RecipeNode is written. +// ProbeNode reads the installed Node version: what `node --version` prints ("v26.10.0"), as the toolchains state holds it. func ProbeNode(ctx context.Context) (string, error) { ctx, cancel := context.WithTimeout(ctx, ProbeTimeout) defer cancel() diff --git a/modules/mesh-build-node/cmd/mesh-build-node/recipe.go b/modules/mesh-build-node/cmd/mesh-build-node/recipe.go index b024df00..c83fead8 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/recipe.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/recipe.go @@ -1,9 +1,9 @@ package main -// The recipe of the Node build module (novox/hq ADR 0405 §1 and §7, ADR 0469 §1 and §4; issue 1488): the one Node -// version it builds with, and the recipe's own version, a hash of the recipe's version, that Node version and the -// bytes of the lock file that names the TypeScript toolchain. A node whose installed Node differs from the recipe's -// refuses every step (module.go), so all builders run this version or none builds. +// The recipe of the Node build module (novox/hq ADR 0405 §7, ADR 0469 §4, ADR 0477 §1-§3; issues 1488, 1572): the +// recipe's own version, and the recipe version, a hash of that and the bytes of the lock file that names the +// TypeScript toolchain. The module names no Node version: it builds with the installed nodejs and npm, reports the +// version installed to the toolchains state, and refuses only when node cannot be run (module.go). import ( "crypto/sha256" @@ -12,20 +12,14 @@ import ( "os" ) -// RecipeNode is the Node version the recipe builds with: the text `node --version` prints for it, which is what the -// guard compares and what the indexer reads from .typescript of the toolchains state. -const RecipeNode = "v26.10.0" - // RecipeSteps is the recipe's own version: changed whenever the steps of fetch, test or build change in a way that -// can change what they produce (ADR 0405 §7). It is one of the three values the recipe version hashes. +// can change what they produce (ADR 0405 §7). It is one of the two values the recipe version hashes. const RecipeSteps = "1" // Recipe is every value that makes up the recipe version. type Recipe struct { // Steps is RecipeSteps. Steps string - // Node is the toolchain's version as `node --version` prints it. - Node string // Lock is the hex sha256 of the bytes of toolchain/package-lock.json (LockHash). Lock string } @@ -43,15 +37,16 @@ func ReadRecipe(lockPath string) (Recipe, error) { if err != nil { return Recipe{}, fmt.Errorf("the toolchain's lock file cannot be read: %w", err) } - return Recipe{Steps: RecipeSteps, Node: RecipeNode, Lock: LockHash(raw)}, nil + return Recipe{Steps: RecipeSteps, Lock: LockHash(raw)}, nil } -// Version is the recipe version: a hash of the recipe's own version, the Node version and the lock file's hash, so a -// change of any of the three moves it and with it every fingerprint that holds it (ADR 0405 §7, ADR 0465 §2). The +// Version is the recipe version: a hash of the recipe's own version and the lock file's hash, never a Node version +// (ADR 0477 §3): the toolchain enters each fingerprint once, as the version the build nodes agree on. A change of +// either moves it and with it every fingerprint that holds it (ADR 0405 §7, ADR 0465 §2). The // fields are named in the hashed text, so no two recipes share a text. func (r Recipe) Version() string { h := sha256.New() - for _, f := range [][2]string{{"steps", r.Steps}, {"node", r.Node}, {"lock", r.Lock}} { + for _, f := range [][2]string{{"steps", r.Steps}, {"lock", r.Lock}} { h.Write([]byte(f[0] + "=" + f[1] + "\n")) } return hex.EncodeToString(h.Sum(nil))[:16] diff --git a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_test.go b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_test.go index 618bc421..ccf06d4c 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/toolchain_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/toolchain_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "os" "path/filepath" "strings" @@ -57,7 +58,7 @@ func toolchainIn(t *testing.T, fake *fakeNpm) Toolchain { func TestAMissingToolchainDirectoryIsInstalledOnceFromTheLock(t *testing.T) { fake := &fakeNpm{} tc := toolchainIn(t, fake) - version := Recipe{Steps: "1", Node: "v26.10.0", Lock: "x"}.Version() + version := Recipe{Steps: "1", Lock: "x"}.Version() dir := filepath.Join(tc.Paths.Cache, "toolchain", version) if tc.Present(version) { t.Fatal("present before any install") @@ -137,7 +138,8 @@ func TestPrepareInstallsOnlyBehindAPassingGuard(t *testing.T) { fake := &fakeNpm{} tc := toolchainIn(t, fake) states := newStates() - m := moduleWith(states, "v26.11.0") + m := moduleWith(states, "") + m.Probe = func(context.Context) (string, error) { return "", errors.New("no node") } m.Toolchain = &tc if err := m.Prepare(context.Background()); err == nil || len(fake.calls) != 0 { t.Fatalf("a refusing node: %v, %d npm calls", err, len(fake.calls))