diff --git a/internal/bootstrap/enrol.go b/internal/bootstrap/enrol.go index 2387ec3..facbda5 100644 --- a/internal/bootstrap/enrol.go +++ b/internal/bootstrap/enrol.go @@ -154,14 +154,32 @@ func Enrol(ctx context.Context, o Options, sys system.System, control controlPla func runTheHost(ctx context.Context, o Options, sys system.System, control controlPlane, say func(string)) (string, error) { - // Asked first, and of the mesh rather than of the process table. What matters is not that a - // process exists but that the mesh has heard from it, and only one of those two is the thing - // every later step depends on. + // **Asked of both, because either one alone lies.** + // + // A process in the table may be wedged and never collect anything, which is why this asked the + // mesh instead. But the mesh having heard from a node proves only that something spoke to it + // *once*, and `mesh-host enrol` — run moments earlier, by this very step — is itself that + // something. So straight after enrolling, the mesh has always heard from this machine and no + // agent is running; skipping the start on that signal alone is exactly wrong. + // + // What it costs is silent and total. Every later step is the control plane being *told* + // things, and nothing it is told reaches a machine with no agent to collect it: the push at + // step 7 is accepted, the mesh records the module, and no container is ever created. It + // surfaces three minutes later as "the registry is not there at all" — one step from its + // cause, looking nothing like it. if heard, err := heardFrom(ctx, control, o.Node); err != nil { return "", err } else if heard { - say(" host running the mesh has heard from " + o.Node) - return "already running", nil + running, err := agentIsRunning(ctx, o, sys, control) + if err != nil { + return "", err + } + if running { + say(" host running the mesh has heard from " + o.Node) + return "already running", nil + } + say(" host running the mesh has heard from " + o.Node + ", and no agent is running " + + "here — enrolling speaks once, which is not the same thing") } how := "" @@ -242,6 +260,28 @@ func runTheHost(ctx context.Context, o Options, sys system.System, control contr } } +// agentIsRunning is whether a host agent is on this machine now — the other half of the question +// the mesh cannot answer. +// +// Deliberately not an error when there is nothing to find: "no unit here" and "not running" are +// both simply *not running* to this caller, and the branch that starts one says far more about a +// missing unit than this could, naming what to install. +func agentIsRunning(ctx context.Context, o Options, sys system.System, control controlPlane) (bool, error) { + if o.HostInBackground { + // No unit to ask, so the process table is all there is. The exit status is the whole + // answer; anything it printed is not this function's business. + if _, err := control.run(ctx, "pgrep", "-f", o.Host+" run"); err != nil { + return false, nil + } + return true, nil + } + state, err := sys.ServiceState(ctx, control.run, o.HostService) + if err != nil { + return false, nil + } + return state == "running", nil +} + // heardFrom asks the mesh whether this node has spoken to it. func heardFrom(ctx context.Context, control controlPlane, node string) (bool, error) { listing, err := control.tell(ctx, "node", "list") diff --git a/internal/bootstrap/enrol_test.go b/internal/bootstrap/enrol_test.go index 7561d56..cad2fa9 100644 --- a/internal/bootstrap/enrol_test.go +++ b/internal/bootstrap/enrol_test.go @@ -59,12 +59,17 @@ func TestAMachineThatHasAlreadyEnrolledIsNotEnrolledAgain(t *testing.T) { switch { case strings.Contains(joined, "node list"): return "anchor here 01J0\n", nil + // An agent is running here too. Both halves are needed: enrolling makes the mesh hear from + // a machine once, so the first answer alone also describes a machine with no agent at all. + case name == "pgrep": + return "4242\n", nil } return "", fmt.Errorf("unexpected: %s %v", name, args) }} out, err := Enrol(context.Background(), Options{ Node: "anchor", State: alreadyEnrolled(t, "anchor"), Timeout: time.Second, + Host: "/usr/local/bin/mesh-host", HostInBackground: true, }, arch(t), controlPlane{container: "temp-mesh-control", run: runtime.run, timeout: time.Second}, func(string) {}) if err != nil { @@ -82,6 +87,41 @@ func TestAMachineThatHasAlreadyEnrolledIsNotEnrolledAgain(t *testing.T) { } } +// The mesh having heard from a machine is not the same as an agent running on it, and enrolling is +// itself the thing that makes the mesh hear. Taken as proof of life it ends the step believing an +// agent it never started, and everything after is the control plane being told things that never +// reach the machine: the next push is accepted, recorded, and applied by nobody. +func TestAMeshThatHasHeardFromAMachineWithNoAgentStartsOne(t *testing.T) { + runtime := &asked{answer: func(name string, args []string) (string, error) { + joined := strings.Join(args, " ") + switch { + case strings.Contains(joined, "node list"): + // Exactly what enrolling leaves behind, with nothing running. + return "anchor here 01J0\n", nil + case name == "pgrep": + return "", fmt.Errorf("exit status 1") + case name == "sh": + return "", nil + } + return "", fmt.Errorf("unexpected: %s %v", name, args) + }} + + out, err := Enrol(context.Background(), Options{ + Node: "anchor", State: alreadyEnrolled(t, "anchor"), Timeout: time.Second, + Host: "/usr/local/bin/mesh-host", HostInBackground: true, + }, arch(t), controlPlane{container: "temp-mesh-control", run: runtime.run, timeout: time.Second}, + func(string) {}) + if err != nil { + t.Fatal(err) + } + if out.Agent == "already running" { + t.Fatal("a machine with no agent was reported as already running it") + } + if !runtime.ran("nohup") { + t.Errorf("no agent was started on a machine that has none: %v", runtime.commands) + } +} + // A machine already enrolled under ANOTHER name is refused, with what to do about it. Re-enrolling // replaces the identity the mesh recorded, which is a deliberate act and not something an // installer does on its own.