diff --git a/cmd/spinloop/route.go b/cmd/spinloop/route.go index 9df3a3df..7dd1b047 100644 --- a/cmd/spinloop/route.go +++ b/cmd/spinloop/route.go @@ -135,11 +135,11 @@ func routeThroughFleet(sel spinloop.Selection, spinloopPath string, opts routeOp if opts.noWake { return nil, fmt.Errorf("%w\nStart one with `spinloop fleet start `, or drop --no-wake to have spinloop do it", err) } - if !cfg.Wakes() { - // The fleet file says the machines are not to be started on demand. The - // refusal still names the node that would have been woken, the way a - // --no-wake refusal does: the setting decides whether to wake, not what - // would be woken. + if !cfg.AnyNodeWakes() { + // No node in the fleet may be woken — the fleet-wide setting, since + // no node's own overrides it. The refusal still names the node that + // would have been woken, the way a --no-wake refusal does: the + // setting decides whether to wake, not what would be woken. if wake, ok := cfg.WouldWake(none.Results, fleet.ConstantConfig(dc, dcErr)); ok { return nil, fmt.Errorf( "%w\nwake is off in %s: start %s with `spinloop fleet start %s`", diff --git a/docs/commands/fleet.md b/docs/commands/fleet.md index c671823c..4bd85b9e 100644 --- a/docs/commands/fleet.md +++ b/docs/commands/fleet.md @@ -250,6 +250,26 @@ An explicit `--no-wake` still refuses to start anything, whatever the file says; an explicit `spinloop fleet start` does the opposite — it always starts, because it was asked. +A node MAY declare its own `wake`, overriding the file's setting for that +node alone: + +```yaml +wake: on +nodes: + - name: gpu-box + host: 198.51.100.7 + - name: prod + kind: remote + wake: off # this one node stays asleep even though the fleet wakes +``` + +This matters most for a `kind: remote` node, whose wake boots a billed cloud +instance rather than starting a process on a machine you already run — so you +can leave the fleet's daemons on `wake: on` while deciding a given remote +environment's waking separately, in either direction: `wake: off` on one node +under a fleet that otherwise wakes, or `wake: on` on one node under a fleet +that otherwise does not. + ### Tags A node's `tags` name the kind of work the node takes on — key/value pairs the diff --git a/docs/commands/gateway.md b/docs/commands/gateway.md index bc5407ef..dca266d0 100644 --- a/docs/commands/gateway.md +++ b/docs/commands/gateway.md @@ -73,7 +73,7 @@ picker and a second gateway does not overwrite this one. See | Path | Meaning | | ---- | ------- | | `GET /health` | That the gateway is up. It touches no node on purpose — it is how you tell the gateway down from the fleet down. | -| `GET /v1/models` | The OpenAI list of what a request can reach: what the running nodes report (the served name when a node reports one, else the model id), and — when [wake](#waking-a-node) is on — what a stopped node's own source describes, the model a request would start it with. Duplicates once. Nothing reachable is an empty list, not an error. | +| `GET /v1/models` | The OpenAI list of what a request can reach: what the running nodes report (the served name when a node reports one, else the model id), and, for a stopped node [waking can reach](#waking-a-node), the model it would start with — its own Spinloop source for a `kind: daemon` node, its own stats reply for a `kind: remote` one. Duplicates once. Nothing reachable is an empty list, not an error. | | `POST /v1/chat/completions` | Routed to the node serving the request's `model`, the way a launch routes. | | `POST /v1/completions` | The same, for the completions endpoint. | | `GET /v1/fleet` | The fleet's [topology](#the-fleets-topology) — what a [`spinloop orchestrator`](orchestrator.md) reads to work its backlog. | @@ -83,10 +83,15 @@ not serve is refused with a `404` naming the ones it does. The list is what a request can reach, so it is bounded by what the gateway can start: a running node contributes only what it reports — a running engine is -never displaced to make room — and a `kind: remote` environment contributes -nothing beyond what it runs, because a request never wakes one. With -`wake: off`, only what is running is listed. Each node's source is read at most -once in a short window, so a poll of the models list is cheap. +never displaced to make room. A deployed-but-stopped `kind: remote` +environment contributes the model id its own stats reply reports — read +directly from its stored deploy config, the way `spinloop remote metrics` +already reads it, since its status reply carries no such facts while +stopped — since the gateway can wake it the same way it wakes a +`kind: daemon` node; one with nothing deployed contributes nothing, and +neither does any node whose own `wake` (or the file's, when it names none) +is off. Each node's source is read at most once in a short window, so a +poll of the models list is cheap. ### Routing a request @@ -114,8 +119,8 @@ fleet-level settings. Each node's entry carries its name, kind, [tags](fleet.md#tags), state, what it serves (the served name where a running engine reports one, else the model id), whether it has answered its own health check, when it last did work — and, for a node that is not running, the model -a request would start it with, where its own source describes one and the -file's [wake policy](fleet.md#waking) allows it. The file's `wake` and +a request would start it with, where the node describes one and +[waking is allowed](fleet.md#waking) for it. The file's `wake` and `prefer` settings and its [concurrency](fleet.md#concurrency) limits ride along, each absent where the file declares none. A node that does not answer is reported in its place — the way the fleet's own views report it — rather @@ -127,20 +132,36 @@ of the fleet: the orchestrator takes no fleet file of its own. ### Waking a node -When no running node serves the model and the fleet file's -[`wake`](fleet.md#waking) setting allows it, the gateway starts a node with -the config that node's own Spinloop source resolves to — only nodes whose -source describes the requested model are candidates, and a node whose stored -config already matches is tried first — and holds the request until the engine -answers, bounded by `--wake-timeout` (default 5m). A timeout fails the request -saying so and leaves the engine running, so a slow load is not thrown away. -Concurrent requests for the same model wake at most one engine: a request that -loses the start to the daemon's "already running" answer takes the node the -other one started. - -With `wake: off`, or when no node's source describes the model, a request -nothing is serving fails without starting anything, naming the node and the -`spinloop fleet start ` command that would start it. +When no running node serves the model and [waking is allowed](fleet.md#waking) +for at least one candidate, the gateway starts one and holds the request until +its engine answers. What a node is started with, and how it is picked, depends +on its kind — only nodes describing the requested model are candidates, and +one whose stored config already matches is tried first: + +- A **`kind: daemon`** node is started with the config its own Spinloop + source resolves to. +- A **deployed-but-stopped `kind: remote`** node is booted as it is: its own + stored deploy config — set by `spinloop remote deploy`, not by this wake — + decides what it serves, and the gateway pushes nothing new. An + **undeployed** environment is never a candidate: it has nothing to serve + yet, and choosing what to deploy is `spinloop remote deploy`'s call, not a + request's. + +The wait is bounded by `--wake-timeout` (default 5m); a timeout fails the +request saying so and leaves the engine running, so a slow load — or, for a +remote node, a slow boot — is not thrown away. Concurrent requests for the +same model wake at most one engine: the gateway coalesces two requests +racing to wake the same node into a single start, so a request that arrives +mid-wake joins the one already under way rather than starting a second +engine of its own — a daemon node's control API would refuse the second +start anyway, but a remote environment's control plane does not, so this is +what keeps a burst of requests from booting (and billing for) more than one +instance. + +A request nothing is serving fails without starting anything, naming the node +and the `spinloop fleet start ` command that would start it, when no +candidate node may be woken — its own `wake`, or the file's when it names +none, is off — or when no node describes the model at all. The gateway needs the same environment a machine running `spinloop fleet start` would: the tokens the fleet file names, set in its diff --git a/go.mod b/go.mod index f41df338..4a8f58ba 100644 --- a/go.mod +++ b/go.mod @@ -25,6 +25,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 github.com/zalando/go-keyring v0.2.8 + golang.org/x/sync v0.16.0 golang.org/x/term v0.45.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 3dd3395d..1563b54a 100644 --- a/go.sum +++ b/go.sum @@ -139,6 +139,8 @@ go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= +golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= diff --git a/internal/fleet/config.go b/internal/fleet/config.go index 53011276..a0454456 100644 --- a/internal/fleet/config.go +++ b/internal/fleet/config.go @@ -104,6 +104,33 @@ func (c *Config) Wakes() bool { return c.WakePolicy != WakeOff } +// NodeWakes reports whether routing may start an engine on entry specifically: +// entry's own wake setting when it names one, taking precedence over the +// fleet-wide policy; the fleet-wide policy otherwise. This is the check a +// candidate search makes per node — Wakes alone answers for a fleet that +// names no per-node override anywhere. +func (c *Config) NodeWakes(entry NodeConfig) bool { + if entry.WakePolicy != "" { + return entry.WakePolicy != WakeOff + } + return c.Wakes() +} + +// AnyNodeWakes reports whether waking is allowed for at least one node in +// the fleet. A caller that pre-empts Wake with a friendlier refusal when +// nothing at all may be woken (naming the node whose config already matches, +// if one does) uses this to decide whether that pre-emption still applies — +// a fleet-wide `wake: off` no longer means nothing wakes, once one node's +// own setting overrides it. +func (c *Config) AnyNodeWakes() bool { + for _, entry := range c.Nodes { + if c.NodeWakes(entry) { + return true + } + } + return false +} + // Concurrency is the fleet's declared capacity: how much work it may take at // once. It sits in the file beside wake and prefer for the same reason — how // much work the fleet's machines will take is a property of the fleet, owned @@ -257,6 +284,14 @@ type NodeConfig struct { // never changes what the node's engine runs. Named key=value where tags // are named in a limit or an item; HasTag matches on that form. Tags map[string]string `yaml:"tags"` + // WakePolicy overrides the fleet-wide wake policy for this node alone, + // in the same `on`/`off` shape. Empty means the fleet-wide setting + // decides for this node, as it always has. It exists because waking is + // not free the same way on every node — a remote environment's wake + // boots a cloud instance, unlike a local daemon's engine — so an + // operator may want to decide one node's waking on its own terms rather + // than through a single fleet-wide switch. + WakePolicy WakePolicy `yaml:"wake,omitempty"` } // HasTag reports whether the node carries the tag named key=value. A name @@ -497,6 +532,11 @@ func (c *Config) validate() error { if n.Kind == "" { n.Kind = KindDaemon } + if n.WakePolicy != "" { + if _, err := ParseWakePolicy(string(n.WakePolicy)); err != nil { + return fmt.Errorf("node %q: %w", n.Name, err) + } + } for key, value := range n.Tags { if key == "" || value == "" { return fmt.Errorf( diff --git a/internal/fleet/config_test.go b/internal/fleet/config_test.go index e52be6e0..69a7b50a 100644 --- a/internal/fleet/config_test.go +++ b/internal/fleet/config_test.go @@ -664,6 +664,83 @@ func TestWakeRejectsUnknownValue(t *testing.T) { } } +// A node's own wake setting overrides the fleet-wide one for that node +// alone; a node naming none is governed by the fleet-wide setting, exactly +// as before per-node overrides existed. +func TestNodeWakeOverride(t *testing.T) { + cases := []struct { + name string + fleet string + wantFleet bool + wantNode bool + }{ + {"node off overrides fleet on", + "wake: on\nnodes:\n - name: a\n host: a.local\n wake: off\n", true, false}, + {"node on overrides fleet off", + "wake: off\nnodes:\n - name: a\n host: a.local\n wake: on\n", false, true}, + {"node names none, fleet on", + "wake: on\nnodes:\n - name: a\n host: a.local\n", true, true}, + {"node names none, fleet off", + "wake: off\nnodes:\n - name: a\n host: a.local\n", false, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cfg, err := Load(writeFleet(t, c.fleet, "")) + if err != nil { + t.Fatal(err) + } + if got := cfg.Wakes(); got != c.wantFleet { + t.Errorf("Wakes() = %v, want %v", got, c.wantFleet) + } + if got := cfg.NodeWakes(cfg.Nodes[0]); got != c.wantNode { + t.Errorf("NodeWakes(a) = %v, want %v", got, c.wantNode) + } + }) + } +} + +// AnyNodeWakes is true whenever at least one node may be woken, whichever +// level decides it for that node — not just when the fleet-wide setting is +// on. +func TestAnyNodeWakes(t *testing.T) { + cases := []struct { + name string + file string + want bool + }{ + {"fleet wakes, no override", "wake: on\nnodes:\n - name: a\n host: a.local\n", true}, + {"fleet off, no override anywhere", + "wake: off\nnodes:\n - name: a\n host: a.local\n - name: b\n host: b.local\n", false}, + {"fleet off, one node opts in", + "wake: off\nnodes:\n - name: a\n host: a.local\n - name: b\n host: b.local\n wake: on\n", true}, + {"fleet on, every node opts out", + "wake: on\nnodes:\n - name: a\n host: a.local\n wake: off\n", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cfg, err := Load(writeFleet(t, c.file, "")) + if err != nil { + t.Fatal(err) + } + if got := cfg.AnyNodeWakes(); got != c.want { + t.Errorf("AnyNodeWakes() = %v, want %v", got, c.want) + } + }) + } +} + +func TestNodeWakeRejectsUnknownValue(t *testing.T) { + _, err := Load(writeFleet(t, "nodes:\n - name: a\n host: a.local\n wake: sometimes\n", "")) + if err == nil { + t.Fatal("an unknown per-node wake value should fail to parse") + } + for _, want := range []string{"on", "off", "a"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should name %q, got %q", want, err) + } + } +} + func TestGatewaySection(t *testing.T) { path := writeFleet(t, ` nodes: diff --git a/internal/fleet/remote_node.go b/internal/fleet/remote_node.go index 4617c5d1..0b73548f 100644 --- a/internal/fleet/remote_node.go +++ b/internal/fleet/remote_node.go @@ -90,15 +90,26 @@ func (n *remoteNode) StartWithProgress(ctx context.Context, report func(StartPha return statusFromRemote(*resp), nil } -// StartWith is how a router wakes a node to serve something. A remote environment -// is not woken: what it serves is set by `spinloop remote deploy`, a heavier flow -// (provisioning, weight seeding, ingress) that a node start must not conflate. +// StartWith is how a router wakes a node to serve something. A remote +// environment's engine is not configured by a start: what it serves and the +// key that gates it are fixed by `spinloop remote deploy`, a heavier flow +// (provisioning, weight seeding, ingress) that a node start must not +// conflate — so dc and engineKey are ignored, and this boots the instance +// exactly as Start does. +// +// An undeployed environment is not checked for here: a status read cannot +// tell a stopped-but-deployed environment from an undeployed one — the +// control plane only relays what an environment serves on its status reply +// while it is running — so that check has to happen where deployment is +// actually confirmed, by reading the environment's deploy config directly +// (its stats reply, in the gateway's own candidate matching) before a +// candidate ever reaches this call. A caller that skips that matching and +// hands an undeployed environment straight to StartWith gets the boot +// call's own answer instead, whatever that turns out to be. func (n *remoteNode) StartWith(ctx context.Context, dc *inference.DeployConfig, engineKey string) (daemon.StatusResponse, error) { _ = dc _ = engineKey - return daemon.StatusResponse{}, fmt.Errorf( - "%s is a remote environment, not a node to be woken: tell it what to serve with `spinloop remote deploy`", - n.name) + return n.StartWithProgress(ctx, func(StartPhase) {}) } func (n *remoteNode) Stop(ctx context.Context) (daemon.StatusResponse, error) { @@ -172,6 +183,16 @@ func (n *remoteNode) Logs(ctx context.Context, offset int64, limit int) (daemon. // routing resolves a remote node's address the way it resolves any node's. // Absent (a stopped or undeployed environment reports none) means no engine // address, exactly as the parts would be. +// +// Healthy is the control plane's own readiness reading — the same health +// check (hitting the engine's /health, excluding the 503 it answers while +// still loading weights) a running remote view already carries — mapped +// onto Ready the way a local daemon's own reading is, so a router waiting +// for a remote engine to answer trusts this instead of falling back to +// whether its port merely accepts a connection, which it can do well before +// the model has loaded. Absent (an older control plane, or the SSM agent +// not yet reachable) leaves Ready empty, the same "no reading yet" a local +// daemon reports before its own first check lands. func statusFromRemote(resp remote.Response) daemon.StatusResponse { s := daemon.StatusResponse{ State: resp.State, @@ -181,6 +202,13 @@ func statusFromRemote(resp remote.Response) daemon.StatusResponse { LastActiveAt: resp.LastActiveAt, IdleSeconds: resp.IdleSeconds, } + if resp.Healthy != nil { + if *resp.Healthy { + s.Ready = daemon.ReadyYes + } else { + s.Ready = daemon.ReadyNo + } + } if u, err := url.Parse(resp.BaseURL); resp.BaseURL != "" && err == nil && u.Host != "" { port, _ := strconv.Atoi(u.Port()) s.Engine = &daemon.EngineEndpoint{ diff --git a/internal/fleet/remote_node_test.go b/internal/fleet/remote_node_test.go index a39afaf9..5ac62dc3 100644 --- a/internal/fleet/remote_node_test.go +++ b/internal/fleet/remote_node_test.go @@ -269,11 +269,63 @@ func TestRemoteNodeLogsSharesTheFollowCursorWithRemoteLogsCommand(t *testing.T) } } -func TestRemoteNodeStartWithIsRefused(t *testing.T) { - node, _ := NewRemoteNode("env", remote.Config{StartURL: "http://x", StopURL: "http://x", Region: "r"}) - _, err := node.StartWith(context.Background(), &inference.DeployConfig{Runner: "llamacpp"}, "") - if err == nil || !strings.Contains(err.Error(), "spinloop remote deploy") { - t.Errorf("StartWith should refuse, naming the deploy path; got %v", err) +// StartWith boots a deployed-but-stopped environment exactly as Start does, +// ignoring the config and key it is handed: what a remote environment +// serves, and the key that gates it, are fixed by `spinloop remote deploy`, +// not by a wake call. +func TestRemoteNodeStartWithBootsADeployedEnvironment(t *testing.T) { + stubAWSCreds(t) + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"stopped"}`)) + }) + mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"ready","healthy":true}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + node, err := NewRemoteNode("env", remote.Config{StartURL: srv.URL, StopURL: srv.URL, Region: "us-east-1"}) + if err != nil { + t.Fatal(err) + } + status, err := node.StartWith(context.Background(), + &inference.DeployConfig{Runner: "llamacpp", ModelID: "org/other"}, "some-key") + if err != nil { + t.Fatalf("StartWith should boot a deployed environment: %v", err) + } + if status.State != "ready" { + t.Errorf("status = %+v, want ready", status) + } +} + +// StartWith does not itself tell a deployed environment from an undeployed +// one by reading status: the control plane's status reply carries no deploy +// facts for a stopped environment either way (only a running one's does), so +// a status read cannot distinguish them. That check belongs to the caller — +// the gateway's own candidate matching confirms deployment from a stats read +// before a candidate is ever chosen — so StartWith just boots, whatever a +// status read would have said. +func TestRemoteNodeStartWithDoesNotItselfCheckDeployment(t *testing.T) { + stubAWSCreds(t) + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"undeployed"}`)) + }) + mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"ready","healthy":true}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + node, err := NewRemoteNode("env", remote.Config{StartURL: srv.URL, StopURL: srv.URL, Region: "us-east-1"}) + if err != nil { + t.Fatal(err) + } + if _, err := node.StartWith(context.Background(), &inference.DeployConfig{Runner: "llamacpp"}, ""); err != nil { + t.Fatalf("StartWith should not refuse on a status read alone: %v", err) } } @@ -395,6 +447,25 @@ func TestStatusFromRemoteServingFacts(t *testing.T) { } } +// statusFromRemote maps the control plane's own health check onto Ready the +// way a local daemon's reading is reported, so a router waiting for a +// remote engine to answer trusts it instead of a raw TCP probe — which can +// succeed well before the model has finished loading. +func TestStatusFromRemoteMapsHealthyOntoReady(t *testing.T) { + if got := statusFromRemote(remote.Response{State: "running", Healthy: boolPtr(true)}); got.Ready != "ready" { + t.Errorf("healthy=true should map to Ready=%q, got %q", "ready", got.Ready) + } + if got := statusFromRemote(remote.Response{State: "running", Healthy: boolPtr(false)}); got.Ready != "not-ready" { + t.Errorf("healthy=false should map to Ready=%q, got %q", "not-ready", got.Ready) + } + // No healthy reading at all — an older control plane, or the branch + // where the SSM agent is not yet reachable — leaves Ready empty rather + // than claiming either answer. + if got := statusFromRemote(remote.Response{State: "running"}); got.Ready != "" { + t.Errorf("an absent healthy reading should leave Ready empty, got %q", got.Ready) + } +} + // statusFromRemote carries a running environment's engine address — the // control plane's published base url — as the engine's host, so routing can // reach it the way it reaches any node. A stopped or undeployed environment diff --git a/internal/fleet/wake.go b/internal/fleet/wake.go index 0e72b44d..565a0dfd 100644 --- a/internal/fleet/wake.go +++ b/internal/fleet/wake.go @@ -15,6 +15,8 @@ import ( "strings" "time" + "golang.org/x/sync/singleflight" + "github.com/spinloop-ai/spinloop/internal/daemon" "github.com/spinloop-ai/spinloop/internal/inference" ) @@ -27,6 +29,18 @@ var WakeTimeout = 5 * time.Minute // wakePoll is how often a waking node is re-checked. var wakePoll = 2 * time.Second +// wakeSingleflight coalesces concurrent wakes of the same node in the same +// fleet file into one actual start. Two requests racing to wake the same +// node is the ordinary shape of two agents starting near enough together, +// and a daemon node's own 409 already turns the loser into a joiner — but a +// remote environment's control plane has no equivalent guard: its instance +// lookup is eventually consistent right after a launch, so two wakes that +// race within that window can each miss the other's not-yet-visible +// instance and each launch one, doubling the bill for what should have been +// a single machine. Keyed by the fleet file's path alongside the node's +// name, so distinct fleets never coalesce across each other. +var wakeSingleflight singleflight.Group + // Waker reports progress while a node is woken. A silent five-minute pause // reads as a hang, so the caller is given something to print. type Waker func(format string, args ...any) @@ -88,6 +102,14 @@ func (c *Config) Wake(ctx context.Context, w Want, cfgFor ConfigFor, results []N var refused []string for _, cand := range cands { + if !c.NodeWakes(cand.entry) { + // Waking is off for this node — its own setting, or the fleet's + // when it names none — so it is refused here rather than + // started, the same as a node that refuses the config: another + // candidate may still serve the request. + refused = append(refused, fmt.Sprintf("%s: waking is disabled for this node", cand.entry.Name)) + continue + } dc, err := resolver.config(cand.entry) if err != nil { refused = append(refused, fmt.Sprintf("%s: %v", cand.entry.Name, err)) @@ -106,33 +128,32 @@ func (c *Config) Wake(ctx context.Context, w Want, cfgFor ConfigFor, results []N if err != nil { return nil, err } - log("Waking %s to serve %s...\n", cand.entry.Name, w.wanted()) - _, err = node.StartWith(ctx, &dc, engineKey) + // Concurrent wakes of this same node — another request racing this + // one, on the same fleet file — coalesce into one actual start via + // wakeSingleflight: only the first caller through runs startAndWait, + // and every caller for this node gets its result. + // The shared call runs on its own background context rather than + // this caller's: whichever caller happens to be first must not have + // its start-and-wait cut short by ITS OWN request being cancelled + // (a client disconnecting) while another caller is still waiting on + // the same node — waitReady bounds the wait by WakeTimeout on its + // own regardless. + key := c.Path + "\x00" + cand.entry.Name + v, err, _ := wakeSingleflight.Do(key, func() (any, error) { + return c.startAndWait(context.Background(), node, cand, dc, engineKey, w, log) + }) if err != nil { - // Another client may have woken this node first. That is - // another route to the same place, not a failure — re-read - // its state and take it if it is now serving what we want. - // The state alone is not the answer, though: the other start may - // still be loading, so the same readiness wait applies to a node - // we did not start ourselves. - if isAlreadyRunning(err) { - if status, err := node.Status(ctx); err == nil && w.matches(servingNames(status)...) { - log("%s was already started by someone else; waiting for its engine to answer...\n", cand.entry.Name) - ready, err := c.waitReady(ctx, node, cand.entry, w, log) - if err != nil { - return nil, err - } - cand.result = NodeResult{Name: cand.entry.Name, Outcome: OutcomeOK, Status: ready} - return c.choiceFor(cand, w, true, engineKey) - } + var fatal *fatalWakeError + if errors.As(err, &fatal) { + // The engine was started, or was already running, but never + // answered — it is left running, so no other candidate is + // tried: that would leave two engines up for one request. + return nil, fatal.err } refused = append(refused, fmt.Sprintf("%s: %v", cand.entry.Name, err)) continue } - ready, err := c.waitReady(ctx, node, cand.entry, w, log) - if err != nil { - return nil, err - } + ready := v.(daemon.StatusResponse) cand.result = NodeResult{Name: cand.entry.Name, Outcome: OutcomeOK, Status: ready} return c.choiceFor(cand, w, true, engineKey) } @@ -141,9 +162,55 @@ func (c *Config) Wake(ctx context.Context, w Want, cfgFor ConfigFor, results []N c.Path, w.wanted(), strings.Join(refused, "\n ")) } +// fatalWakeError marks a wake failure that must not be answered by trying +// the next candidate: the engine was started, or was found already running, +// and is left running either way, so falling through would leave two +// engines up for one request rather than one that simply took longer. +type fatalWakeError struct{ err error } + +func (e *fatalWakeError) Error() string { return e.err.Error() } +func (e *fatalWakeError) Unwrap() error { return e.err } + +// startAndWait starts cand's node — or, when another caller's start won a +// race, joins the engine that start produced — and waits for it to answer. +// It is the unit wakeSingleflight coalesces: everything from the log line a +// caller sees through the readiness wait happens at most once per node per +// overlapping set of wakes, however many requests are waiting on it. +func (c *Config) startAndWait(ctx context.Context, node Node, cand candidate, dc inference.DeployConfig, engineKey string, w Want, log Waker) (daemon.StatusResponse, error) { + log("Waking %s to serve %s...\n", cand.entry.Name, w.wanted()) + _, err := node.StartWith(ctx, &dc, engineKey) + if err != nil { + // Another client may have woken this node first. That is another + // route to the same place, not a failure — re-read its state and + // take it if it is now serving what we want. The state alone is not + // the answer, though: the other start may still be loading, so the + // same readiness wait applies to a node we did not start ourselves. + if isAlreadyRunning(err) { + if status, serr := node.Status(ctx); serr == nil && w.matches(servingNames(status)...) { + log("%s was already started by someone else; waiting for its engine to answer...\n", cand.entry.Name) + ready, werr := c.waitReady(ctx, node, cand.entry, w, log) + if werr != nil { + return daemon.StatusResponse{}, &fatalWakeError{werr} + } + return ready, nil + } + } + return daemon.StatusResponse{}, err + } + ready, werr := c.waitReady(ctx, node, cand.entry, w, log) + if werr != nil { + return daemon.StatusResponse{}, &fatalWakeError{werr} + } + return ready, nil +} + // wakeable keeps the nodes that could be started, in the order to try them: a // node whose stored config already names the wanted model first, since it has -// the weights and starts sooner. +// the weights and starts sooner. Whether waking is actually allowed for a +// given node is Wake's own concern, not this ordering's — WouldWake reports +// the node that would be tried first on config alone, regardless of policy, +// which is what lets a caller explain a wake-off refusal by naming the node +// it would otherwise have started. func wakeable(cands []candidate, resolver *configResolver) []candidate { var warm, cold []candidate for _, c := range cands { @@ -202,8 +269,14 @@ func (c *Config) WaitLoading(ctx context.Context, w Want, results []NodeResult, // // A daemon that reports its own readiness reading — the engine has answered // its health check — is taken on that word; it checked from the same machine -// the engine runs on. A daemon that reports none (older builds, or a runner -// with no known health-check convention) falls back to the TCP probe. +// the engine runs on, and a remote node's reading is the control plane's own +// equivalent check. A ReadyNo reading is taken on its word too: the engine's +// port can accept a connection well before the engine can answer a request +// — llama.cpp and vLLM both open it early and answer their own health check +// 503 while still loading — so an explicit "not ready" must not be +// second-guessed by the weaker TCP probe. That probe is the fallback only +// for a node that reports no reading at all (older builds, or a runner with +// no known health-check convention) — the one case a reading cannot settle. // // On timeout the started engine is deliberately left running: it is probably // still loading, and stopping it throws away the only expensive part. @@ -216,15 +289,21 @@ func (c *Config) waitReady(ctx context.Context, node Node, entry NodeConfig, w W if err == nil { last = status if status.State == string(daemon.StateRunning) { - if status.Ready == daemon.ReadyYes { - return status, nil - } - baseURL, urlErr := c.EngineBaseURL(entry, status) - if urlErr != nil { - return status, urlErr - } - if engineAnswers(ctx, baseURL) { + switch status.Ready { + case daemon.ReadyYes: return status, nil + case daemon.ReadyNo: + // Taken on its word: falling back to the TCP probe here + // would hand out an address the engine itself just said + // is not ready to answer. + default: + baseURL, urlErr := c.EngineBaseURL(entry, status) + if urlErr != nil { + return status, urlErr + } + if engineAnswers(ctx, baseURL) { + return status, nil + } } if !announced { log("%s is up; waiting for its engine to load...\n", entry.Name) diff --git a/internal/fleet/wake_test.go b/internal/fleet/wake_test.go index 43b0109c..c2fd3cb3 100644 --- a/internal/fleet/wake_test.go +++ b/internal/fleet/wake_test.go @@ -15,7 +15,6 @@ import ( "github.com/spinloop-ai/spinloop/internal/daemon" "github.com/spinloop-ai/spinloop/internal/inference" - "github.com/spinloop-ai/spinloop/internal/remote" ) // fakeNode is one machine's daemon plus, optionally, its engine's listener. @@ -31,13 +30,17 @@ type fakeNode struct { startStatus int // engineDelay is how long after starting before the engine listens. engineDelay time.Duration - // ready, when set, is what /v1/status reports for `ready`. - ready bool + // ready, when set (daemon.ReadyYes or daemon.ReadyNo), is what + // /v1/status reports for `ready`; empty reports no reading at all. + ready string // noEngine keeps the engine's listener down even after an accepted start: // readiness can only come from the daemon's own reading. noEngine bool // started records whether a start was accepted. started bool + // startCalls counts every /v1/start request received, accepted or + // refused — unlike started, which only says whether the last one was. + startCalls int // pushed is the deploy config the start carried. pushed *inference.DeployConfig // pushedKey is the engine key the start carried. @@ -69,8 +72,8 @@ func newFakeNode(t *testing.T, state, model string) *fakeNode { resp := daemon.StatusResponse{State: f.state, Model: f.model} if f.state == string(daemon.StateRunning) { resp.Engine = &daemon.EngineEndpoint{Port: f.enginePort} - if f.ready { - resp.Ready = "ready" + if f.ready != "" { + resp.Ready = f.ready } } json.NewEncoder(w).Encode(resp) @@ -78,6 +81,7 @@ func newFakeNode(t *testing.T, state, model string) *fakeNode { mux.HandleFunc("/v1/start", func(w http.ResponseWriter, r *http.Request) { f.mu.Lock() defer f.mu.Unlock() + f.startCalls++ if f.startErr != "" { status := f.startStatus if status == 0 { @@ -200,6 +204,70 @@ func TestWakeSkipsANodeThatRefusesTheConfig(t *testing.T) { } } +// A node whose own wake is off is never started, even though its config +// matches and the fleet otherwise wakes: another candidate is tried instead. +func TestWakeSkipsANodeWithItsOwnWakeDisabled(t *testing.T) { + shortWake(t) + disabled := newFakeNode(t, string(daemon.StateIdle), "") + enabled := newFakeNode(t, string(daemon.StateIdle), "") + cfg := fleetOf(t, []string{"disabled-box", "enabled-box"}, disabled, enabled) + cfg.Nodes[0].WakePolicy = WakeOff + + choice, err := cfg.Wake(context.Background(), Want{Model: "m"}, + ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, nil), statusOf(t, cfg), nil) + if err != nil { + t.Fatal(err) + } + if choice.Node.Name != "enabled-box" { + t.Errorf("chose %q, want the node whose own wake is not disabled", choice.Node.Name) + } + disabled.mu.Lock() + defer disabled.mu.Unlock() + if disabled.started { + t.Error("a node with its own wake disabled must not be started") + } +} + +// A node with nothing to try but a disabled node reports why, naming the +// node and that its own waking is disabled — not a generic "refuses the +// config" reason. +func TestWakeDisabledNodeNamesItselfInTheRefusal(t *testing.T) { + shortWake(t) + disabled := newFakeNode(t, string(daemon.StateIdle), "") + cfg := fleetOf(t, []string{"disabled-box"}, disabled) + cfg.Nodes[0].WakePolicy = WakeOff + + _, err := cfg.Wake(context.Background(), Want{Model: "m"}, + ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, nil), statusOf(t, cfg), nil) + if err == nil { + t.Fatal("expected a failure: the only candidate's own wake is disabled") + } + for _, want := range []string{"disabled-box", "waking is disabled"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("message should mention %q, got:\n%s", want, err) + } + } +} + +// A node that opts its own wake on is started even though the fleet as a +// whole does not wake. +func TestWakeStartsANodeThatOptsInUnderAFleetThatDoesNotWake(t *testing.T) { + shortWake(t) + node := newFakeNode(t, string(daemon.StateIdle), "") + cfg := fleetOf(t, []string{"opted-in-box"}, node) + cfg.WakePolicy = WakeOff + cfg.Nodes[0].WakePolicy = WakeOn + + choice, err := cfg.Wake(context.Background(), Want{Model: "m"}, + ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, nil), statusOf(t, cfg), nil) + if err != nil { + t.Fatal(err) + } + if choice.Node.Name != "opted-in-box" { + t.Errorf("chose %q, want the node that opted its own wake on", choice.Node.Name) + } +} + func TestWakeReportsEveryRefusal(t *testing.T) { shortWake(t) a := newFakeNode(t, string(daemon.StateIdle), "") @@ -271,6 +339,47 @@ func TestWakeTimesOutWithoutStopping(t *testing.T) { } } +// Two Wake calls racing to wake the same node coalesce into one actual +// start. This fixture's own /v1/start does not itself reject a concurrent +// call the way a real daemon's supervisor mutex does — unlike a daemon +// node, a remote environment's control plane has no such guard at all — so +// without wakeSingleflight both calls would reach StartWith and each start +// their own engine (or, for a remote node, each launch their own instance). +func TestWakeCoalescesConcurrentCallsForTheSameNode(t *testing.T) { + shortWake(t) + node := newFakeNode(t, string(daemon.StateIdle), "") + node.engineDelay = 100 * time.Millisecond + cfg := fleetOf(t, []string{"box"}, node) + cfgFor := ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, nil) + results := statusOf(t, cfg) + + var wg sync.WaitGroup + choices := make([]*Choice, 2) + errs := make([]error, 2) + for i := range 2 { + wg.Add(1) + go func(i int) { + defer wg.Done() + choices[i], errs[i] = cfg.Wake(context.Background(), Want{Model: "m"}, cfgFor, results, nil) + }(i) + } + wg.Wait() + + for i, err := range errs { + if err != nil { + t.Fatalf("call %d: %v", i, err) + } + } + if choices[0].Node.Name != "box" || choices[1].Node.Name != "box" { + t.Errorf("both calls should land on the same node, got %q and %q", choices[0].Node.Name, choices[1].Node.Name) + } + node.mu.Lock() + defer node.mu.Unlock() + if node.startCalls != 1 { + t.Errorf("the node's /v1/start was called %d times, want exactly 1 — the second wake should have joined the first rather than starting its own", node.startCalls) + } +} + // Losing the race to another client is another route to the same place. func TestWakeLosingTheRaceUsesTheNode(t *testing.T) { shortWake(t) @@ -392,16 +501,163 @@ func TestWakeWithoutAKeyIsUngated(t *testing.T) { } // The wake path stays daemon-only: a remote is never woken — what it serves is -// set by `spinloop remote deploy`, a heavier flow a node start must not conflate. -// The refusal is the contract Wake relies on to move to its next candidate. -func TestRemoteRefusesToBeWoken(t *testing.T) { - n, err := NewRemoteNode("cloud", remote.Config{StartURL: "https://s", StopURL: "https://x", Region: "us-east-1"}) +// set by `spinloop remote deploy`. Wake boots its instance and waits for its +// engine to answer the same way it does for a daemon node, without pushing +// the candidate resolver's config onto it — the environment already knows +// what it serves. +func TestWakeStartsADeployedRemoteNode(t *testing.T) { + shortWake(t) + stubAWSCreds(t) + + var ( + mu sync.Mutex + engine net.Listener + started bool + ) + statusBody := func() string { + mu.Lock() + defer mu.Unlock() + if engine == nil { + // The control plane carries no deploy facts on a stopped + // environment's status reply — only a running one's does — so + // this deliberately reports none, the way the real one does. + return `{"state":"stopped"}` + } + return fmt.Sprintf( + `{"state":"running","runner":"llamacpp","modelId":"org/m","servedName":"m","base_url":"http://%s/v1"}`, + engine.Addr()) + } + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(statusBody())) + }) + mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + started = true + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + mu.Unlock() + t.Fatal(err) + } + engine = ln + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + // remote.Start only accepts HTTP 200 with state "ready" as done; the + // engine's own running state comes from the status polls waitReady + // makes afterwards, not from this reply. + fmt.Fprintf(w, `{"state":"ready","healthy":true,"runner":"llamacpp","modelId":"org/m","servedName":"m","base_url":"http://%s/v1"}`, ln.Addr()) + }) + srv := httptest.NewServer(mux) + t.Cleanup(func() { + srv.Close() + mu.Lock() + defer mu.Unlock() + if engine != nil { + engine.Close() + } + }) + registerRemoteEnv(t, "cloud", srv.URL, srv.URL) + + path := writeFleet(t, "nodes:\n - name: cloud\n kind: remote\n", "") + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + results := statusOf(t, cfg) + if !results[0].OK() || results[0].Status.State != "stopped" { + t.Fatalf("initial status = %+v", results[0]) + } + + cfgFor := ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "org/m", ServedModelName: "m"}, nil) + choice, err := cfg.Wake(context.Background(), Want{Model: "org/m"}, cfgFor, results, nil) + if err != nil { + t.Fatal(err) + } + if choice.Node.Name != "cloud" { + t.Errorf("chose %q, want cloud", choice.Node.Name) + } + mu.Lock() + defer mu.Unlock() + if !started { + t.Error("the environment's instance was never started") + } +} + +// A remote node's wake waits for the control plane's own health check to +// say ready, not just for its port to accept a connection: this fake +// reports running-but-unhealthy for a stretch after boot, the way an engine +// that has opened its port but is still loading weights does, before +// flipping healthy. The bug this guards against would have returned as +// soon as the port opened. +func TestWakeWaitsForARemoteEngineToBecomeHealthy(t *testing.T) { + shortWake(t) + stubAWSCreds(t) + const unhealthyFor = 150 * time.Millisecond + + var ( + mu sync.Mutex + engine net.Listener + healthyAt time.Time + ) + statusBody := func() string { + mu.Lock() + defer mu.Unlock() + if engine == nil { + return `{"state":"stopped"}` + } + healthy := time.Now().After(healthyAt) + return fmt.Sprintf( + `{"state":"running","healthy":%t,"runner":"llamacpp","modelId":"org/m","servedName":"m","base_url":"http://%s/v1"}`, + healthy, engine.Addr()) + } + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(statusBody())) + }) + mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + mu.Unlock() + t.Fatal(err) + } + engine = ln + healthyAt = time.Now().Add(unhealthyFor) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"state":"ready","healthy":true,"runner":"llamacpp","modelId":"org/m","servedName":"m","base_url":"http://%s/v1"}`, ln.Addr()) + }) + srv := httptest.NewServer(mux) + t.Cleanup(func() { + srv.Close() + mu.Lock() + defer mu.Unlock() + if engine != nil { + engine.Close() + } + }) + registerRemoteEnv(t, "cloud", srv.URL, srv.URL) + + path := writeFleet(t, "nodes:\n - name: cloud\n kind: remote\n", "") + cfg, err := Load(path) if err != nil { t.Fatal(err) } - _, err = n.StartWith(context.Background(), &inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, "sk-key") - if err == nil || !strings.Contains(err.Error(), "not a node to be woken") { - t.Errorf("want the remote refusal, got %v", err) + results := statusOf(t, cfg) + + cfgFor := ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "org/m", ServedModelName: "m"}, nil) + start := time.Now() + choice, err := cfg.Wake(context.Background(), Want{Model: "org/m"}, cfgFor, results, nil) + if err != nil { + t.Fatal(err) + } + if time.Since(start) < unhealthyFor { + t.Error("wake returned before the control plane reported the engine healthy — an open TCP port alone must not be trusted") + } + if choice.Node.Name != "cloud" { + t.Errorf("chose %q, want cloud", choice.Node.Name) } } @@ -479,7 +735,7 @@ func TestWakeWaitsForARacedNodeToAnswer(t *testing.T) { func TestWakeTrustsTheDaemonReadinessReading(t *testing.T) { shortWake(t) node := newFakeNode(t, string(daemon.StateIdle), "") - node.ready = true + node.ready = daemon.ReadyYes node.noEngine = true // the probe could never succeed; only the reading could cfg := fleetOf(t, []string{"box"}, node) @@ -493,6 +749,32 @@ func TestWakeTrustsTheDaemonReadinessReading(t *testing.T) { } } +// An explicit "not ready" reading is trusted too, and is not second-guessed +// by a successful TCP probe: a runner's port can accept a connection well +// before it can answer a request — llama.cpp and vLLM both open it early +// and answer their own health check with a 503 while still loading — so an +// engine that says it is not ready must not be waved through because +// something merely answers on its port. +func TestWakeDoesNotTrustTheProbeOverAnExplicitNotReadyReading(t *testing.T) { + shortWake(t) + node := newFakeNode(t, string(daemon.StateIdle), "") + node.ready = daemon.ReadyNo + // engineDelay defaults to 0, so the "engine" starts listening almost + // immediately — the TCP probe alone would say ready straight away. The + // bug this guards against is exactly that probe overriding a reading + // that already says no. + cfg := fleetOf(t, []string{"box"}, node) + + _, err := cfg.Wake(context.Background(), Want{Model: "m"}, + ConstantConfig(inference.DeployConfig{Runner: "llamacpp", ModelID: "m"}, nil), statusOf(t, cfg), nil) + if err == nil { + t.Fatal("expected a timeout: the reading never says ready") + } + if !strings.Contains(err.Error(), "box") { + t.Errorf("message should name the node, got: %v", err) + } +} + // A variable that resolves to nothing fails before any engine is started. func TestWakeFailsOnAnUnresolvableKey(t *testing.T) { shortWake(t) diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index d08a0bb3..c8f2213d 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -249,7 +249,7 @@ func (h *Handler) handleHealth(w http.ResponseWriter, _ *http.Request) { // list, not an error. func (h *Handler) handleModels(w http.ResponseWriter, r *http.Request) { results := h.reading(r.Context()) - wakeable := h.wakeableModels() + wakeable := h.wakeableModels(r.Context()) seen := map[string]bool{} data := []map[string]any{} add := func(name string) { @@ -281,28 +281,28 @@ func (h *Handler) handleModels(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{"object": "list", "data": data}) } -// wakeableModels is what a request could start: for each node, the model its -// own source describes, under the served-name-first naming a running node -// reports. It is resolved at most once per sourcesTTL, shared by every models -// request. The gateway cannot start a remote environment — one the fleet -// names by environment and a request never wakes — so a remote node -// contributes nothing here, and neither does any node when the fleet's wake is -// off or the gateway holds no way to resolve a source. -func (h *Handler) wakeableModels() map[string]string { - if h.cfgFor == nil || !h.cfg.Wakes() { - return nil - } +// wakeableModels is what a request could start: for each node for which +// waking is allowed (its own `wake` setting, or the fleet's when it names +// none), the model it would be started with, under the served-name-first +// naming a running node reports. A daemon node's model comes from its own +// Spinloop source; a remote node's comes from its own stats reply. It is +// resolved at most once per sourcesTTL, shared by every models request — +// which bounds how often a remote node's resolution pays a live control- +// plane call, the same way it bounds how often a daemon node's pays a +// Spinloop file read. +func (h *Handler) wakeableModels(ctx context.Context) map[string]string { h.mu.Lock() defer h.mu.Unlock() if h.wakeable != nil && h.now().Sub(h.wakeableAt) < sourcesTTL { return h.wakeable } + cfgFor := h.combinedConfigFor(ctx) m := map[string]string{} for _, entry := range h.cfg.Nodes { - if entry.Kind != fleet.KindDaemon { + if !h.cfg.NodeWakes(entry) { continue } - dc, err := h.cfgFor(entry) + dc, err := cfgFor(entry) if err != nil { continue } @@ -319,6 +319,53 @@ func (h *Handler) wakeableModels() map[string]string { return m } +// remoteConfigFor resolves what a kind: remote node would be started with: +// the environment's stored deploy config, read from its stats reply. The +// status reply a fan-out already holds is no good for this — the control +// plane only relays deploy facts on the status reply while the environment +// is running (see remote-node's "A running environment's status..." +// requirement), so a stopped environment's status names nothing served even +// though it is deployed. The stats reply carries the runner and model id +// unconditionally, since the stats Lambda reads the deploy config directly +// rather than relaying it alongside instance state; an undeployed +// environment's stats read fails outright (no config to read), which is +// what "nothing deployed" looks like here. It carries no served name — the +// stats reply has none — so a stopped remote node's wakeable model is its +// model id alone, unlike a running one's served-name-first naming. +func (h *Handler) remoteConfigFor(ctx context.Context) fleet.ConfigFor { + return func(entry fleet.NodeConfig) (inference.DeployConfig, error) { + node, err := h.cfg.NewNode(entry) + if err != nil { + return inference.DeployConfig{}, err + } + stats, err := node.Metrics(ctx) + if err != nil { + return inference.DeployConfig{}, fmt.Errorf("%s: %w (run `spinloop remote deploy` if nothing is deployed)", entry.Name, err) + } + return inference.DeployConfig{ModelID: stats.ModelID}, nil + } +} + +// combinedConfigFor resolves what any node — daemon or remote — would be +// started with: a daemon node through the gateway's own cfgFor (its +// Spinloop source), a remote node through its own stats reply +// (remoteConfigFor). A daemon node fails the way it always has when the +// gateway holds no cfgFor at all; a remote node's resolution does not +// depend on cfgFor, so it still works when the gateway was built with none. +func (h *Handler) combinedConfigFor(ctx context.Context) fleet.ConfigFor { + remoteFor := h.remoteConfigFor(ctx) + return func(entry fleet.NodeConfig) (inference.DeployConfig, error) { + if entry.Kind == fleet.KindRemote { + return remoteFor(entry) + } + if h.cfgFor == nil { + return inference.DeployConfig{}, fmt.Errorf( + "this gateway can wake no node: it has no way to resolve a node's Spinloop source") + } + return h.cfgFor(entry) + } +} + // handleTopology answers with the fleet's topology: the reading the models // list and the routing take, joined with the file's claims about each node // and its fleet-level settings. A node that does not answer is reported in @@ -326,7 +373,7 @@ func (h *Handler) wakeableModels() map[string]string { // the whole reply. func (h *Handler) handleTopology(w http.ResponseWriter, r *http.Request) { results := h.reading(r.Context()) - wakeable := h.wakeableModels() + wakeable := h.wakeableModels(r.Context()) topo := Topology{Wake: h.cfg.Wakes(), Prefer: string(h.cfg.Prefer)} if c := h.cfg.Concurrency; c != nil { @@ -495,26 +542,31 @@ func (h *Handler) progress() fleet.Waker { } } -// wakeFor starts a node for a request nothing is serving, when the fleet file -// allows it, and holds the request until the engine answers. A concurrent -// request waking the same node loses its start to the daemon's 409 and takes -// the node the other one started — the same engine, the same wait. +// wakeFor starts a node for a request nothing is serving, when waking is +// allowed for at least one node in the fleet, and holds the request until +// the engine answers. A concurrent request waking the same node loses its +// start to a 409 and takes the node the other one started — the same +// engine, the same wait. +// +// A node whose resolved config matches the request but whose own waking is +// disabled is left for Wake to refuse itself, the same way it refuses a node +// whose config does not match: Wake's per-candidate loop already names it +// ("waking is disabled for this node") in the refusal it reports when +// nothing else can serve the request either. func (h *Handler) wakeFor(ctx context.Context, want fleet.Want, results []fleet.NodeResult) (*fleet.Choice, error) { none := &fleet.ErrNoneServing{Results: results, Want: want, Path: h.cfg.Path} - if h.cfgFor == nil { - return nil, fmt.Errorf("%s\nthis gateway can wake no node: it has no way to resolve a node's Spinloop source", none) - } - if !h.cfg.Wakes() { - return nil, h.refuseWake(want, none) + matching := h.matchingConfigFor(want.Model, h.combinedConfigFor(ctx)) + if !h.cfg.AnyNodeWakes() { + return nil, h.refuseWake(want, none, matching) } - return h.cfg.Wake(ctx, want, h.matchingConfigFor(want.Model), results, h.progress()) + return h.cfg.Wake(ctx, want, matching, results, h.progress()) } -// refuseWake is the wake-off answer: nothing is started, and the failure names -// the node whose source describes the model and the command that would start -// it — or, when no source describes it, that there is nothing to start. -func (h *Handler) refuseWake(want fleet.Want, none error) error { - cfgFor := h.matchingConfigFor(want.Model) +// refuseWake is the no-node-wakes answer: nothing is started, and the +// failure names the node whose source describes the model and the command +// that would start it — or, when no source describes it, that there is +// nothing to start. +func (h *Handler) refuseWake(want fleet.Want, none error, cfgFor fleet.ConfigFor) error { for _, entry := range h.cfg.Nodes { if _, err := cfgFor(entry); err == nil { return fmt.Errorf("%s\nwake is off in %s: %q's source describes %s; start it with `spinloop fleet start %s`", @@ -524,12 +576,11 @@ func (h *Handler) refuseWake(want fleet.Want, none error) error { return fmt.Errorf("%s\nwake is off in %s, and no node's source describes %s", none, h.cfg.Path, want.Model) } -// matchingConfigFor wraps the per-node source resolver with the one condition -// a wake has to meet: the source's config is the model the request asks for. -// A node whose source describes a different model is not a candidate — it -// would be started with the wrong engine — and its refusal says so. -func (h *Handler) matchingConfigFor(model string) fleet.ConfigFor { - base := h.cfgFor +// matchingConfigFor wraps a resolver with the one condition a wake has to +// meet: the resolved config is the model the request asks for. A node whose +// config describes a different model is not a candidate — it would be +// started with the wrong engine — and its refusal says so. +func (h *Handler) matchingConfigFor(model string, base fleet.ConfigFor) fleet.ConfigFor { return func(entry fleet.NodeConfig) (inference.DeployConfig, error) { dc, err := base(entry) if err != nil { diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index f00010ce..8efdaea2 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -2,6 +2,7 @@ package gateway import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -9,6 +10,8 @@ import ( "net" "net/http" "net/http/httptest" + "os" + "path/filepath" "slices" "strconv" "strings" @@ -21,6 +24,35 @@ import ( "github.com/spinloop-ai/spinloop/internal/inference" ) +// registerRemoteEnv points the environment registry (SPINLOOP_CONFIG_DIR) at +// a temp config directory and writes one environment's remote.json, whose +// control plane — start, stop and stats alike — is the server url given, and +// stubs the AWS credential chain so a signed control call reaches it. +// Reproduced from internal/fleet's own helper of the same name because it +// lives in a different package. +func registerRemoteEnv(t *testing.T, name, url string) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "AKIATESTTESTTESTTEST") + t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "no-such-file")) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + + home := t.TempDir() + t.Setenv("SPINLOOP_CONFIG_DIR", home) + envDir := filepath.Join(home, "remotes", name) + if err := os.MkdirAll(envDir, 0o755); err != nil { + t.Fatal(err) + } + body := fmt.Sprintf(`{"start_url":%q,"stop_url":%q,"stats_url":%q,"region":"us-east-1","environment":%q}`, + url, url, url+"/stats", name) + if err := os.WriteFile(filepath.Join(envDir, "remote.json"), []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + // fakeNode is one machine: its daemon's control API and, once running, its // engine's real HTTP endpoint on the port the daemon reports — so the // gateway's proxy path is exercised end to end against a listener, not a @@ -435,17 +467,77 @@ func TestModelsListsOnlyWhatRunsWhenWakeIsOff(t *testing.T) { } } -// The gateway cannot start a remote environment — a request never wakes one — -// so its source's model is not wakeable and not listed. -func TestModelsLeavesOutARemoteEnvironment(t *testing.T) { +// A deployed-but-stopped remote environment's model comes from its own +// stats reply — the environment's stored deploy config, read directly by +// the stats Lambda — not from its status reply, which carries no deploy +// facts while the environment is stopped, and not from a Spinloop source. +// So it is wakeable and listed the same as a daemon node's. +func TestModelsListsADeployedRemoteEnvironment(t *testing.T) { + url, _ := remoteControlServer(t) + registerRemoteEnv(t, "env", url) cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ {Name: "env", Kind: fleet.KindRemote}, }} - h := New(cfg, "", Options{ConfigFor: cfgForOf(t, - map[string]inference.DeployConfig{"env": {Runner: "llamacpp", ModelID: "org/cold"}}, - nil)}) - if m := h.wakeableModels(); len(m) != 0 { - t.Errorf("a remote environment's source is not a request's, got %v", m) + h := New(cfg, "", Options{}) + m := h.wakeableModels(context.Background()) + if got := m["env"]; got != "org/deployed" { + t.Errorf("wakeableModels()[env] = %q, want the model id from its stats reply", got) + } +} + +// An undeployed remote environment's stats read fails outright — the stats +// Lambda has no deploy config to read — so it has nothing to be woken with +// and is not listed. +func TestModelsLeavesOutAnUndeployedRemoteEnvironment(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"undeployed"}`)) + }) + mux.HandleFunc("GET /stats", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"cannot read deploy config: run spinloop remote deploy first"}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + registerRemoteEnv(t, "env", srv.URL) + + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "env", Kind: fleet.KindRemote}, + }} + h := New(cfg, "", Options{}) + if m := h.wakeableModels(context.Background()); len(m) != 0 { + t.Errorf("an undeployed remote environment has nothing to start it with, got %v", m) + } +} + +// A remote node with its own wake disabled is not listed, even though it is +// deployed, and even under a fleet that wakes. +func TestModelsLeavesOutARemoteEnvironmentWithWakeDisabled(t *testing.T) { + url, _ := remoteControlServer(t) + registerRemoteEnv(t, "env", url) + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "env", Kind: fleet.KindRemote, WakePolicy: fleet.WakeOff}, + }} + h := New(cfg, "", Options{}) + if m := h.wakeableModels(context.Background()); len(m) != 0 { + t.Errorf("a node with its own wake disabled should not be listed, got %v", m) + } +} + +// A remote node whose environment is not registered fails before any network +// call, naming the node the way a daemon node with no resolvable Spinloop +// source would. +func TestRemoteConfigForUnregisteredEnvironment(t *testing.T) { + t.Setenv("SPINLOOP_CONFIG_DIR", t.TempDir()) + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "env", Kind: fleet.KindRemote}, + }} + h := New(cfg, "", Options{}) + cfgFor := h.remoteConfigFor(context.Background()) + if _, err := cfgFor(fleet.NodeConfig{Name: "env", Kind: fleet.KindRemote}); err == nil || + !strings.Contains(err.Error(), "env") { + t.Errorf("an unregistered environment should fail naming it, got %v", err) } } @@ -1063,6 +1155,162 @@ func TestNothingCanServeNamesEveryRefusal(t *testing.T) { } } +// --- waking a remote node ----------------------------------------------------- + +// remoteControlServer serves a deployed-but-stopped environment's status +// until its instance is booted (POST), after which it reports running with +// an engine that actually answers an OpenAI-compatible completion — so a +// request proxied to it end to end gets a real reply, the same way it does +// against a daemon's engine. +// +// Its GET / (status) reply matches the real control plane: deploy facts +// (runner, modelId, servedName) ride along only once the environment is +// running, not while it is stopped — a stopped environment's status names +// only its state and base_url, the same gap that "gateway waking an +// undeployed node" is really about. GET /stats always carries runner and +// modelId (never servedName — the stats reply has none), since the stats +// Lambda reads the deploy config directly rather than relaying it alongside +// instance state; that is what the gateway now resolves a stopped remote +// node's wakeable model from. +func remoteControlServer(t *testing.T) (url string, started func() bool) { + t.Helper() + var ( + mu sync.Mutex + engine net.Listener + wasSent bool + ) + engineHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"id":"cmpl-1","object":"chat.completion","choices":[{"message":{"role":"assistant","content":"hello"}}]}`) + }) + statusBody := func() string { + mu.Lock() + defer mu.Unlock() + if engine == nil { + return `{"state":"stopped"}` + } + return fmt.Sprintf( + `{"state":"running","runner":"llamacpp","modelId":"org/deployed","servedName":"deployed","base_url":"http://%s/v1"}`, + engine.Addr()) + } + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(statusBody())) + }) + mux.HandleFunc("GET /stats", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"state":"stopped","runner":"llamacpp","modelId":"org/deployed"}`) + }) + mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + wasSent = true + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + mu.Unlock() + t.Fatal(err) + } + engine = ln + mu.Unlock() + go (&http.Server{Handler: engineHandler}).Serve(ln) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"state":"ready","healthy":true,"runner":"llamacpp","modelId":"org/deployed","servedName":"deployed","base_url":"http://%s/v1"}`, ln.Addr()) + }) + srv := httptest.NewServer(mux) + t.Cleanup(func() { + srv.Close() + mu.Lock() + defer mu.Unlock() + if engine != nil { + engine.Close() + } + }) + return srv.URL, func() bool { mu.Lock(); defer mu.Unlock(); return wasSent } +} + +// A request for a model only a deployed-but-stopped remote node serves is +// held and answered once that node's instance boots — the gateway sources +// its wakeable model from its own last status, not a Spinloop source, and +// StartWith boots it without pushing any config. +func TestColdRequestWakesADeployedRemoteNode(t *testing.T) { + shortWake := func(t *testing.T) { + old := fleet.WakeTimeout + fleet.WakeTimeout = 3 * time.Second + t.Cleanup(func() { fleet.WakeTimeout = old }) + } + shortWake(t) + url, started := remoteControlServer(t) + registerRemoteEnv(t, "cloud", url) + + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "cloud", Kind: fleet.KindRemote}, + }} + h := New(cfg, "", Options{}) + + resp, body := post(t, h, "", `{"model":"org/deployed"}`) + if resp.StatusCode != http.StatusOK { + t.Fatalf("HTTP %d, body %s", resp.StatusCode, body) + } + if !started() { + t.Error("the environment's instance was never started") + } +} + +// An undeployed remote node's stats read fails outright — nothing deployed +// to read — so it does not match any request and the failure says so, +// naming the deploy path, without holding the request for the wake timeout. +func TestWakeRefusesAnUndeployedRemoteNode(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"state":"undeployed"}`)) + }) + mux.HandleFunc("GET /stats", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"cannot read deploy config: run spinloop remote deploy first"}`)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + registerRemoteEnv(t, "cloud", srv.URL) + + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "cloud", Kind: fleet.KindRemote}, + }} + h := New(cfg, "", Options{}) + + resp, body := post(t, h, "", `{"model":"org/deployed"}`) + if resp.StatusCode != http.StatusServiceUnavailable { + t.Fatalf("HTTP %d, want 503: %s", resp.StatusCode, body) + } + if !strings.Contains(body, "spinloop remote deploy") { + t.Errorf("the failure should name the deploy path: %s", body) + } +} + +// A remote node whose own wake is disabled is not started even though its +// last status matches the request, and the failure says waking is off for +// it rather than that nothing can serve the model at all. +func TestWakeDisabledForAMatchingRemoteNodeNamesIt(t *testing.T) { + url, started := remoteControlServer(t) + registerRemoteEnv(t, "cloud", url) + + cfg := &fleet.Config{Path: "fleet.yaml", Dir: t.TempDir(), Nodes: []fleet.NodeConfig{ + {Name: "cloud", Kind: fleet.KindRemote, WakePolicy: fleet.WakeOff}, + }} + h := New(cfg, "", Options{}) + + resp, body := post(t, h, "", `{"model":"org/deployed"}`) + if resp.StatusCode != http.StatusServiceUnavailable { + t.Fatalf("HTTP %d, want 503: %s", resp.StatusCode, body) + } + if !strings.Contains(body, "cloud") { + t.Errorf("the failure should name the node: %s", body) + } + if started() { + t.Error("a node with its own wake disabled must not be started") + } +} + // --- logging ------------------------------------------------------------------ func TestRoutedRequestLeavesOneLogLine(t *testing.T) { diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/.openspec.yaml b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/.openspec.yaml new file mode 100644 index 00000000..a40cb63c --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-14 diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/design.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/design.md new file mode 100644 index 00000000..a3bfe2bf --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/design.md @@ -0,0 +1,270 @@ +## Context + +See proposal.md for the motivation. Three things in the current code make the +refusal happen, and the fix touches each differently: + +- `remoteNode.StartWith` (`internal/fleet/remote_node.go`) refuses + unconditionally, before anything is checked against the control plane. +- `gateway.wakeableModels()` (`internal/gateway/gateway.go`) only reads a + node's wakeable model by resolving its local Spinloop source (`h.cfgFor`, + wired up in `cmd/spinloop/gateway.go` via `resolveNodeSpinloop` + + `readSpinloop`), and explicitly skips any node whose `Kind != KindDaemon`. +- `fleet.Config.Wake` (`internal/fleet/wake.go`) resolves each candidate's + deploy config the same way, through the injected `ConfigFor`, so a remote + candidate either has no resolvable local source or, if it does, is starting + from a config that is not what the environment already has deployed. + +Separately: `spinloop fleet start ` already boots a deployed +remote environment today, through `Node.Start` (not `StartWith`) in +`cmd/spinloop/fleet.go`'s `fleetStartCall`. That path is unaffected by this +change and is the proof that booting a deployed environment with no pushed +config already works end to end. + +The remote start Lambda (`remote/lambda/start/index.ts`) already reads the +environment's own stored deploy config on every wake and already answers, +with a `503` naming `spinloop remote deploy`, when nothing is deployed +(`state: 'unconfigured'`) or nothing is provisioned (`state: 'undeployed'`). +That reply is not an immediate failure on the Go side, though: +`remote.Start` (`internal/remote/remote.go`) treats every `503` alike — +still booting, no capacity, or undeployed — and retries after the reply's +`retry_after_seconds` until its own deadline, so an undeployed environment +would otherwise hold a wake request for the whole wake timeout before +failing with a generic "gave up waiting" message. That means a candidate +must be confirmed deployed *before* `StartWith` is ever called, not by +`StartWith` itself — see the next point for why it also cannot be confirmed +from a status read. + +The status Lambda's own non-running branch (`status()`, same file) answers a +stopped or undeployed environment with only `{state, environment, healthy, +base_url}` — no runner, model id, or served name; those only ride along +(`readDeployFacts`, spread into the reply) once the instance is `running`. +So a fan-out's cached status for a stopped environment cannot say what it +would serve, deployed or not — the two look identical. The stats Lambda +(`remote/lambda/stats/index.ts`) is different: it reads the deploy config +directly (`readDeployConfig`) before it even looks at instance state, and +fails outright (no instance-state branch reached at all) when there is none +to read. Its reply carries `runner` and `modelId` in every branch it does +reach, running or not — but never a served name, which only the deploy +config's own field name (not relayed by this Lambda) would supply. + +## Goals / Non-Goals + +**Goals:** +- A deployed-but-stopped remote node becomes a wake candidate for the + gateway's `/v1/models`, its topology endpoint, and its request-time wake, + sourced from the node's own stats reply rather than a local file. +- An operator can allow or refuse waking on one node without changing the + fleet's own `wake` setting. + +**Non-Goals:** +- Undeployed remote environments stay out of scope, per the issue: the + refusal for them is preserved, just moved to where the control plane + already produces it rather than duplicated in Go. +- `spinloop fleet start`, `fleet route`, and the orchestrator's admission + (#188) are unaffected — they already treat a remote node correctly for + their own purposes and are not part of this change. +- No change to how a daemon node is woken or how its config is resolved. + +## Decisions + +### Where a remote node's wakeable config comes from + +Resolve it from a live call to the environment's stats endpoint +(`Node.Metrics`, wrapping `remote.Stats`), not from the status a fan-out +already holds and not from a local Spinloop source. The status reply is no +good for this (see Context: it carries deploy facts only while running), so +this is a genuine extra network call rather than a free read of data already +in hand — bounded the same way a daemon node's Spinloop-file read is, by the +gateway's existing `sourcesTTL` cache on `wakeableModels`, and by `Wake`'s +own per-node-name memoisation (`configResolver`) within one wake attempt. + +Rejected alternative: give a remote node entry a local Spinloop source and +resolve its config the way a daemon node's is. Rejected because a remote +environment is deployed with `spinloop remote deploy`, a separate flow that +can drift from whatever local file the fleet entry happens to point at (or +point at nothing); trusting the control plane's own record avoids a second +copy of "what does this node run" that could disagree with the first. + +Because this needs a live call — building the node from the registry and +reaching its control plane — it is a method on `Handler` +(`remoteConfigFor(ctx) fleet.ConfigFor`), not a pure function over `results`: +it needs `h.cfg.NewNode` and a `context.Context` to bound the call, neither +of which a `results []fleet.NodeResult` slice carries. `wakeableModels` and +`wakeFor` pass a request-scoped context through; the config-resolution +signature otherwise stays the same shape (`fleet.ConfigFor`), so +`internal/fleet` itself (`wake.go`, `configResolver`, `candidates`, +`WouldWake`) is still untouched — it only knows "the injected resolver said +X" and does not care whether X came from a file or a live stats call. The +existing "does the resolved config match what the request wants" check +(`gateway.matchingConfigFor`) keeps wrapping the combined resolver +unchanged, so it enforces the match for a remote candidate exactly the way +it already does for a daemon one — on `ModelID` alone for a remote node, +since the stats reply carries no served name to match on. + +### Starting a deployed remote node + +`remoteNode.StartWith` stops refusing and delegates to the same +`StartWithProgress` that `Start` already uses, ignoring the `dc` and +`engineKey` it is handed: a remote environment's engine is gated by the key +fixed at deploy time, and what it serves is fixed by its own stored deploy +config, not by anything a wake call supplies. It does not itself check +whether the environment is deployed — a status read cannot tell that apart +from stopped-and-deployed (see Context), so a check here would inherit the +same blind spot the original design mistakenly built into it. That +confirmation already happened one step earlier, in the gateway's own +candidate matching (`remoteConfigFor`, this document's other decision), +which reads the stats reply and only offers a candidate whose deploy +config actually resolved — `Wake`'s loop never reaches `StartWith` for a +node that check refused. `internal/fleet.Wake` has exactly one caller of +`StartWith` on a remote node, so there is nowhere else that gap could be +reached from today. + +Rejected alternative: check `dc` against the environment's last known model +before calling `StartWithProgress`, refusing locally on a mismatch. Rejected +as unnecessary — a remote candidate only reaches `StartWith` after the +gateway's own match check already accepted its resolved config as matching +the request, and duplicating that check here is validating a condition the +caller already enforced. + +### Per-node wake override + +Add `NodeConfig.WakePolicy` (yaml `wake`, same `on`/`off` shape and parse +validation as the fleet-wide setting) and `Config.NodeWakes(entry) bool`: +the node's own setting when it names one, else the fleet's — plus +`Config.AnyNodeWakes() bool` for a caller that needs to know whether waking +is possible at all before it tries. + +The check itself lives inside `Wake`'s own per-candidate loop, not in +`wakeable()`'s ordering: a candidate whose own wake is off is skipped there +the same way a candidate whose config doesn't match is skipped, contributing +"waking is disabled for this node" to the refusal `Wake` reports when +nothing else can serve the request either. `wakeable()` and `WouldWake` +stay exactly as they were — reporting the node that would be tried first on +config alone, regardless of policy. That is deliberate: `WouldWake` is what +`fleet route`'s and the gateway's pre-emptive refusal already used to name +the node a wake-off setting was refusing, and a caller doing that still +needs the answer to "who would this have woken" even when the answer is +"nobody, because policy said no" — folding the policy into `wakeable()` +itself would have made `WouldWake` blind to that node whenever the fleet or +the node's own setting is off, breaking the exact message it exists to +produce. + +The gateway's request-time refusal (`wakeFor`) and its advertisement +(`wakeableModels`) switch from the single `h.cfg.Wakes()` check to, +respectively, `h.cfg.AnyNodeWakes()` (is there any point trying `Wake` at +all) and `h.cfg.NodeWakes(entry)` per node (does this one contribute to what +a request could start). `cmd/spinloop/route.go`'s own pre-emptive refusal — +outside this change's stated scope, but sharing the same fleet-wide-only gate +this change touches — gets the same `AnyNodeWakes()` swap, so a per-node +override is not silently inert there. + +Rejected alternative (from the issue): a second fleet-wide flag scoped to +remote nodes only (e.g. `wakeRemote`). Rejected per the chosen direction — a +per-node override is more general (it also lets a daemon node opt out +individually) and reads directly off the node it affects rather than adding +a second global switch whose interaction with the first has to be +documented separately. + +### Coalescing concurrent wakes + +Found running this against a real fleet: two requests landed on the gateway +within half a second of each other, both wanting the same model nothing was +serving, and both logged "Waking dev-4...". A daemon node's control API +would have turned the loser into a joiner via its own `409` — that race was +never actually a problem for a daemon node, so the original design (see the +now-corrected risk note above) assumed a remote environment's conflict would +present the same way and left it alone. It does not: `remote/lambda/start`'s +only de-duplication is a `DescribeInstances` tag lookup, which is eventually +consistent right after a `RunInstances` call, so two `wake()` invocations +close enough together can each miss the other's not-yet-visible instance and +each launch one — a real double-billed launch, not a cosmetic double log +line, and never surfaced as an error either side could catch: the Lambda +doesn't propagate a failed on-instance daemon start as anything a caller +would recognise as a conflict. + +Fixed with a `golang.org/x/sync/singleflight.Group`, keyed by the fleet +file's path alongside the node's name, wrapping exactly the "start (or join) +and wait for ready" step inside `Wake`'s per-candidate loop — the same step +the daemon's `409` used to make redundant for daemon nodes and now makes +redundant for every node kind uniformly. Two concurrent calls for the same +node share one call to `startAndWait`; only the first actually starts +anything, and both receive its result. The shared call runs on its own +`context.Background()`, not either caller's request context: whichever +caller happened to be first must not have the wait cut short by its own +request being cancelled while another caller is still waiting on the same +node, and `waitReady` already bounds the wait by `WakeTimeout` on its own +regardless of context. + +A fatal outcome (the engine started, or was found already running, but +never answered) has to reach every caller sharing that result the same way +a non-fatal refusal does not: the former must not be retried against another +candidate — the engine is left running — while the latter should let each +caller's own loop move on to its own next candidate. `startAndWait` signals +this with a `*fatalWakeError` wrapper so `Wake`'s loop, after `Do` returns, +can tell the two apart without singleflight itself needing to know anything +about wake-specific semantics. + +Rejected alternative: fix the race in the remote Lambda instead (a +DynamoDB-backed idempotency lock, or Lambda reserved concurrency of 1 per +environment). Rejected for this change — it would need a coordinated +`remote/` CDK redeploy per environment, which this PR cannot cause on its +own, whereas the Go-side fix closes the gap for every already-deployed +environment the moment the gateway binary updates. A control-plane-side lock +would still be worth doing eventually, as defence in depth against a +caller outside this gateway process (a second gateway instance, a direct +script) racing the same environment — out of scope here. + +### Trusting a real readiness signal over an open port + +Found once dev-4 actually booted: the gateway routed a request to it while +the model was still loading, and the request failed. `waitReady` was built +to prefer a real readiness reading over a raw TCP probe — the probe only +checks that a port accepts a connection, which both llama.cpp and vLLM do +before they can answer a request — but its `if status.Ready == +daemon.ReadyYes { return }` fell through to the probe for *everything else*, +including an explicit `ReadyNo`, not only "no reading landed". A daemon node +rarely surfaces this: its own reading is almost always populated one way or +the other by the time a caller asks. A remote node's `Ready` was *always* +empty going into this fix, for an unrelated reason — `statusFromRemote` +never mapped the control plane's own `healthy` field (the same `/health` +check, already relayed on a running remote view) onto it at all — so every +remote wake fell through to the probe by construction, making the gap +certain rather than occasional. Fixing the mapping without also fixing the +fallthrough would have made the gap visible without closing it: a properly +populated `ReadyNo` would still have lost to a successful probe. + +Both are fixed together: `statusFromRemote` now maps `Healthy` onto `Ready` +the way a local daemon's own check is reported, and `waitReady` branches on +`Ready` three ways — `ReadyYes` returns, `ReadyNo` waits without probing, +and only an empty reading falls back to the probe, matching what the +existing routing check for an already-running node (`select.go`'s +`running()`, which already treated `ReadyNo` correctly) established was the +right rule. No design alternative was considered here — this was a +correctness bug against the design's own stated intent, not a choice. + +## Risks / Trade-offs + +- [A remote candidate's resolved config is stale by up to `wakeableModels`' + cache window on the models/topology paths (30s) — not on the wake path + itself, which resolves fresh — so a listed model could lag a re-deploy + briefly] → Mitigation: the same staleness already applies to a daemon + node's Spinloop-file read under the same cache, and the control plane's + own reply when the wake is actually attempted is still the last word. +- [Every stopped remote node now costs a live stats call each time + `wakeableModels` re-resolves (every `sourcesTTL`), instead of a free read + of already-fanned-out status] → Mitigation: bounded by the same cache a + daemon node's file read already relies on; a remote environment missing a + configured `stats_url` fails that one node's resolution the same way a + daemon node with no resolvable Spinloop source does, rather than the + whole reply. +- [A second place (`node.wake`) now decides whether a node wakes, which + could confuse debugging] → Mitigation: refusal and topology text names + which setting decided it. +- [Two requests racing to wake the same remote node could each miss the + other's not-yet-visible instance in the control plane's eventually + consistent instance lookup, launching two — this was flagged here as + "out of scope, worst case a missed optimisation" on the assumption that a + remote start's conflict would surface as a refusal the way a daemon's 409 + does; it does not (see "Coalescing concurrent wakes" below), so this was + wrong and had to be fixed rather than accepted] → Mitigation: see + "Coalescing concurrent wakes". diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/proposal.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/proposal.md new file mode 100644 index 00000000..dc122a06 --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/proposal.md @@ -0,0 +1,57 @@ +## Why + +The gateway's on-demand wake only ever starts an engine on a daemon node. +A deployed-but-stopped remote environment already knows what to serve — the +same information a status read already gets back from its control plane — +but the gateway refuses to start it and never advertises it as reachable, so +a request that could be served by waking a stopped remote environment fails +instead, even though the fleet file allows waking and the node is one +`start` call away from serving it. + +## What Changes + +- `remoteNode.StartWith` boots a deployed-but-stopped environment's instance + instead of refusing outright. An undeployed environment — nothing stored to + serve — is still refused, unchanged. +- The gateway's wakeable-model computation (`/v1/models`, the topology + endpoint, and the wake candidate search) stops skipping remote nodes. A + stopped remote node's wakeable model is read from its last status reply + (served name first, then model id) — the same fact the control plane + already reports for a deployed environment — rather than resolved from a + local Spinloop source the way a daemon node's is. +- A node entry gains an optional per-node `wake` override (`on`/`off`). When + set, it decides whether that node may be woken regardless of the fleet's + own `wake` setting; when unset, the fleet's setting applies as it does + today. This lets an operator leave a fleet's `wake` on for its daemon nodes + while opting a remote node in or out individually — starting a remote + instance costs money, so its opt-in should not be implied by the fleet's + blanket setting alone. + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `fleet-gateway`: the models list, the topology endpoint, and the wake + candidate search now consider a deployed-but-stopped remote node wakeable, + sourcing its wakeable model from its own status reply instead of a + Spinloop source. +- `remote-node`: waking a remote environment is no longer refused + unconditionally — a deployed-but-stopped environment is started; an + undeployed one is still refused. +- `fleet-config`: a node entry may declare its own `wake` setting, overriding + the fleet-wide one for that node alone. + +## Impact + +- `internal/fleet/remote_node.go` (`StartWith`), `internal/fleet/wake.go` + (candidate resolution), `internal/fleet/config.go` (`NodeConfig`, per-node + wake resolution), `internal/fleet/node.go`. +- `internal/gateway/gateway.go` (`wakeableModels`, `handleModels`, + `handleTopology`, `wakeFor`, `refuseWake`). +- `cmd/spinloop/gateway.go` (the `ConfigFor` the gateway wires up). +- No change to the orchestrator's admission (#188) or to undeployed remote + environments, which remain out of scope per the issue. diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-config/spec.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-config/spec.md new file mode 100644 index 00000000..01d39c63 --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-config/spec.md @@ -0,0 +1,70 @@ +## MODIFIED Requirements + +### Requirement: Fleet-wide wake policy + +A fleet file MAY declare a top-level `wake` value of `on` or `off`, deciding +whether routing starts an engine on a node that is not running one when no +running node serves what is wanted. It applies to every node that declares +no `wake` setting of its own — the same reason `prefer` is fleet-wide: it +describes how this cluster is to be used — may work be started on its +machines on demand, or only used where it is already running — which is a +property of the fleet, not of any one machine in it, unless that machine's +entry says otherwise. + +A node entry MAY declare its own `wake` value, `on` or `off`, in the same +shape as the fleet-wide setting. When a node names one, it decides whether +that node may be woken, taking precedence over the fleet-wide setting for +that node alone; a node naming none is governed by the fleet-wide setting as +before. This exists because waking is not free the same way on every node: a +remote environment's wake boots and pays for a cloud instance, unlike a local +daemon's engine, so an operator may want the fleet's daemons to wake freely +while deciding a remote node's waking on its own terms — opted in under a +fleet that otherwise does not wake, or opted out under one that does — +without a second fleet-wide flag governing every remote node in the file +alike. + +A file declaring nothing at either level SHALL wake, as routing does when no +setting decides otherwise: waking is the difference between a fleet that +answers a request and one that must be prepared by hand, and the file's +author is the one who owns the machines it names. A fleet-wide or per-node +`wake` declaring anything other than `on` or `off` SHALL fail to parse, +naming both accepted values, in keeping with the file's other validation. + +The setting SHALL decide whether to wake only, at whichever level decides it +for a given node. It SHALL NOT change which node is chosen, how matching +nodes are ranked, or what a wake does: a node for which waking is not +allowed still reports, when nothing is running, the node whose source +describes the wanted model and the command that would start it. + +#### Scenario: A fleet that declares nothing wakes + +- **WHEN** a fleet file declares no `wake` setting at either level and + routing finds no node serving what is wanted +- **THEN** routing starts an engine on a suitable node, as it does today + +#### Scenario: A fleet that refuses to wake + +- **WHEN** a fleet file declares `wake: off` and no node overrides it, and + routing finds no node serving what is wanted +- **THEN** nothing is started, and the failure names the node that would be + woken and the command that would start it + +#### Scenario: A node opts out under a fleet that wakes + +- **WHEN** a fleet file declares `wake: on`, one node declares its own + `wake: off`, and that node is the only one serving what is wanted +- **THEN** that node is not woken, and the failure names it and says waking + is disabled for it, even though the fleet otherwise wakes + +#### Scenario: A node opts in under a fleet that does not wake + +- **WHEN** a fleet file declares `wake: off`, one node declares its own + `wake: on`, and that node is the only one serving what is wanted +- **THEN** that node is woken, though the rest of the fleet still does not + wake + +#### Scenario: An unknown value is rejected at parse time + +- **WHEN** a fleet file declares `wake: sometimes`, at the fleet level or on + a node +- **THEN** parsing fails naming `on` and `off` diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-gateway/spec.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-gateway/spec.md new file mode 100644 index 00000000..f51bd1b1 --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/fleet-gateway/spec.md @@ -0,0 +1,285 @@ +## MODIFIED Requirements + +### Requirement: Listing the fleet's models + +The gateway SHALL serve `GET /v1/models` returning, in the OpenAI list shape, +the union of the models a request can reach. For each node whose state is +`running`, the list SHALL carry the name it reports serving — the served name +when it reports one, otherwise the model id. For each node that is not +running and is a wake candidate — waking is allowed for it (its own `wake` +setting, or the fleet-wide one when it names none) and it names a model to +start with — the list SHALL additionally carry that model, under the same +served-name-first naming a wake would start it with: a daemon node's own +Spinloop source describes it, and a remote node's own stats reply carries it +— the environment's stored deploy config, which the stats reply carries +whether the environment is running or stopped (unlike the status reply, +which only relays it while running). A running node SHALL contribute nothing +but what it reports: a running engine is never displaced, so its source's +model is not a request the gateway would answer from it. An undeployed remote +environment — one whose stats read fails outright, having no deploy config +to read — and a node for which waking is not allowed SHALL contribute +nothing beyond what is running, and duplicates SHALL be listed once. The +model a node would be started with SHALL be resolved at most once in a short +window shared by all models requests, so a burst does not re-read every +node's source or re-fetch every remote node's stats. + +#### Scenario: Running models are listed + +- **WHEN** two nodes are running, one serving a model under an alias and one + under its id, and a models request is made +- **THEN** the response lists the alias and the id, each once + +#### Scenario: A stopped node's wakeable model is listed + +- **WHEN** a node is stopped, its own source describes a model, and waking is + allowed for it, and a models request is made +- **THEN** the response lists the model the source describes, beside what the + running nodes serve + +#### Scenario: A stopped node's model is not listed when wake is off + +- **WHEN** a node is stopped and waking is not allowed for it, and a models + request is made +- **THEN** the response lists only what the running nodes serve + +#### Scenario: A deployed remote environment's model is listed + +- **WHEN** a remote environment is stopped, its stats reply reports what its + stored deploy config would serve, and waking is allowed for it, and a + models request is made +- **THEN** the response lists that model beside what the running nodes serve + +#### Scenario: A stopped remote environment's model is not listed + +- **WHEN** a remote environment is stopped and has nothing deployed, and a + models request is made +- **THEN** the response does not list it: the gateway has nothing stored to + start it with + +#### Scenario: A running node's source adds no second model + +- **WHEN** a running node reports one model and its source describes another, + and a models request is made +- **THEN** the response lists only what the node reports + +#### Scenario: A burst of models requests resolves each source once + +- **WHEN** several models requests arrive within the window in which a node's + source or a remote node's status is resolved +- **THEN** each node's source or status is read once for the burst + +#### Scenario: Nothing reachable lists nothing + +- **WHEN** no node is running and nothing is wakeable — no stopped node + describes a model, or waking is not allowed for any of them — and a models + request is made +- **THEN** the response is an empty list, not an error + +### Requirement: Waking a node for a request + +When no running node serves the model a request names, and waking is allowed +for at least one candidate, the gateway SHALL start an engine on a node that +is not running one, and SHALL hold the request until the engine answers. +Waking is allowed for a node when its own `wake` setting says so, or, +when it names none, the fleet file's wake policy does. A node is a wake +candidate when it is not running, waking is allowed for it, and it names a +model to start with, matching the one the request asks for: + +- A daemon node names one through the Spinloop source it names — its `file` + field, a registered alias named after it, or a same-named directory beside + the fleet file, resolved the way `spinloop fleet start` resolves it — + describing a config whose model or served name is the one the request asks + for. +- A remote node names one through its own stats reply, which reads the + environment's stored deploy config directly and so carries its model id + whether the environment is running or stopped — unlike its status reply, + which only relays the deploy config while running, and unlike the stats + reply itself, which carries no served name. An undeployed remote + environment's stats read fails outright, having no deploy config to read; + it names nothing and is not a candidate. + +A node is started with what it names, never with a config invented for the +request: a daemon node is started with the Spinloop source's config; a remote +node is started as it is — its stored deploy config decides what it serves, +and the gateway pushes it nothing new. Candidates whose stored config already +names the model SHALL be tried first, since they have the weights, and the +rest in fleet-file order. A node that refuses the start — a runner or model it +cannot serve — SHALL NOT fail the request while other candidates remain. + +A daemon engine started this way SHALL be gated with the key the node's fleet +entry names, supplied by the gateway: the gateway is the client that starts +the engine, so the key the client sets is the key the engine takes. A remote +environment's engine is gated by its own key, resolved the same way a request +already routed to it resolves one; the gateway does not change it. The wait +SHALL be bounded by a wake timeout, defaulting to five minutes and +overridable by `--wake-timeout`; exceeding it SHALL fail the request saying +the engine did not answer in time, and the started engine SHALL be left +running rather than stopped, so a slow load — or, for a remote node, a slow +boot — is not thrown away. + +When several requests ask for a model nothing is serving at once, the gateway +SHALL start at most one engine per node and answer every request from it: the +first request's wait is the wait the rest join, regardless of the node's +kind. A daemon node's own control API refuses a second concurrent start on +its own, but a remote environment's control plane does not, so the gateway +SHALL NOT rely on that alone: two requests racing to wake the same node +SHALL be coalesced before either reaches the node, not just reconciled after +one of them answers. A node another request woke first SHALL be used the +same way, and only once its engine answers. + +A request for a model nothing is serving, and for which waking is not allowed +on any node that names it, SHALL fail without starting anything, naming the +nodes and what they could serve, and the command that would start one. A +model no node is running and no node names — no daemon source describes it +and no remote node is deployed with it — SHALL fail the same way regardless +of any wake setting: nothing to wake with, and the failure SHALL say so +rather than trying to start a node with nothing. + +#### Scenario: A cold request wakes a node and is served + +- **WHEN** no node is running the model a request names, one node's Spinloop + source describes it, and waking is allowed for it +- **THEN** that node is started with its own config, gated with the key its + fleet entry names, and the request is answered once the engine answers + +#### Scenario: A cold request wakes a deployed remote environment + +- **WHEN** no node is running the model a request names, one remote node's + stats reply reports it is deployed to serve it, and waking is allowed for + it +- **THEN** that environment's instance is started, its own stored deploy + config decides what it serves, and the request is answered once its engine + answers + +#### Scenario: An undeployed remote node is not a wake candidate + +- **WHEN** the only node whose name could match a request is a remote + environment with nothing deployed +- **THEN** it is not started, and the failure says nothing is deployed to + serve the model, naming the deployment path + +#### Scenario: The request is held while the engine loads + +- **WHEN** the woken node reports running while its engine is still loading +- **THEN** the request waits, and is answered when the engine answers, rather + than failing against an endpoint that refuses connections + +#### Scenario: A wake that does not finish in time fails the request + +- **WHEN** a woken node's engine does not answer within the wake timeout +- **THEN** the request fails saying so, naming the node, and the engine is left + running + +#### Scenario: Concurrent cold requests share one wake + +- **WHEN** two requests arrive at once for a model nothing is serving, and one + node's source describes it +- **THEN** that node is started once, and both requests are answered from the + same engine + +#### Scenario: Concurrent cold requests share one remote wake + +- **WHEN** two requests arrive at once for a model nothing is serving, and one + remote node's stats reply reports it is deployed to serve it +- **THEN** that environment's instance is started once, not once per request, + and both requests are answered once its engine answers + +#### Scenario: A woken engine takes the gateway's key + +- **WHEN** the gateway starts an engine on a daemon node whose fleet entry + names an engine key +- **THEN** the engine is gated with that value, the gateway's requests to it + carry it, and no reply to any caller contains it + +#### Scenario: Wake refused by the fleet file + +- **WHEN** waking is not allowed for any node that could serve the model a + request names, and no node is serving it +- **THEN** nothing is started, and the request fails naming the node whose + source describes the model and the command that would start it + +#### Scenario: A remote node opted out is not woken though the fleet wakes + +- **WHEN** the fleet file's wake policy is `on`, a stopped remote node + declares its own `wake: off`, and it is the only node that names the + model a request asks for +- **THEN** it is not started, and the failure names it and says waking is + disabled for that node + +#### Scenario: Nothing can serve the model + +- **WHEN** no node is running the model a request names, no node's Spinloop + source describes it, and no remote node is deployed with it +- **THEN** the request fails, naming each node and why it cannot serve the + model, and nothing is started + +#### Scenario: A sourceless node is not woken + +- **WHEN** the only node that could take a request names no Spinloop source + that resolves +- **THEN** it is not started, and the failure names it and the ways a source + could have been given + +### Requirement: Serving the fleet's topology + +The gateway SHALL serve the fleet's topology over a read endpoint, behind the +same caller authentication as everything else it serves: a caller with the +token gets it, a caller without does not, exactly as on its other paths. + +The reply SHALL describe each node of the fleet the gateway holds: its name, +its kind, its tags as the fleet file declares them, its state, and its +serving facts — the model it serves when it is running, the name it serves +that model under where it reports one, whether its engine has answered, and +when it was last active. For a node that is not running, the reply SHALL name +the model a request would start it with, where the node names one — a daemon +node's own source, or a remote node's own stats reply — and waking is +allowed for it (its own `wake` setting, or the fleet's when it names none); a +node that names no such model, or for which waking is not allowed, SHALL +report none. A node that does not answer SHALL be reported as such in the +fleet's order, not fail the whole reply. + +The reply SHALL carry the fleet file's fleet-level settings the way the file +declares them: whether the fleet wakes, how it ranks, and its concurrency +limits where it declares any, with each absent where the file declares it +not. The gateway's fleet file remains the single source of truth for the +topology: the reply is what the file says and what the nodes report, and the +gateway holds no copy of either beyond what it already holds. + +#### Scenario: A caller with the token reads the topology + +- **WHEN** a caller sends the gateway's token to the topology endpoint +- **THEN** it gets every node with its tags, state, and serving facts, and + the file's wake policy, ranking, and concurrency limits + +#### Scenario: A caller without the token is refused + +- **WHEN** a caller sends no token, or the wrong one, to the topology + endpoint +- **THEN** it is refused the way the gateway's other paths refuse it + +#### Scenario: A stopped node reports what it would start + +- **WHEN** a node is not running, names a model to start with, and waking is + allowed for it +- **THEN** the topology names that model as what a request would start the + node with + +#### Scenario: A stopped, deployed remote node reports what it would start + +- **WHEN** a remote node is stopped, its stats reply reports its stored + deploy config, and waking is allowed for it +- **THEN** the topology names that config's model as what a request would + start it with, the same way a daemon node's is named + +#### Scenario: A dead node does not sink the reply + +- **WHEN** one of the fleet's nodes does not answer and a caller reads the + topology +- **THEN** that node is reported as not answering, in the fleet's order, and + the rest of the fleet is reported as usual + +#### Scenario: Absent settings are absent + +- **WHEN** the fleet file declares no concurrency limits +- **THEN** the topology carries none, rather than a default the file never + named diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/remote-node/spec.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/remote-node/spec.md new file mode 100644 index 00000000..f0e0f783 --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/specs/remote-node/spec.md @@ -0,0 +1,73 @@ +## MODIFIED Requirements + +### Requirement: A remote environment is a fleet node + +A registered remote environment SHALL be representable as one member of the fleet's node +set, answering the same operations a local node answers: its status, its metrics, and +being started, stopped, and read for logs. The control plane's replies SHALL be mapped +onto the same status and metrics shapes a local node yields, so downstream fan-out and +rendering treat the two identically. A running environment's status SHALL in particular +carry what its engine is serving — the model it runs, and the served name the deploy gave +it beside the model id when there is one — so a client choosing a node by model matches +it the way it matches a local node, and the fleet view and the remote view name it the +same. It SHALL also carry where its engine answers — the instance's published address, +which the control plane knows and a daemon on the instance cannot — so a client can +reach the engine, not only name it; a stopped or undeployed environment reports none. + +A remote environment that cannot be reached, or whose control call is rejected — +including a rejected AWS credential — SHALL be reported as a typed outcome against that +environment, the same way an unreachable or unauthorized node is, rather than failing the +command or being silently dropped. + +A deployed remote environment — one whose stored deploy config already describes what to +serve — SHALL answer a node-level start the same way a local node does: the instance is +booted and the call waits for it, without deploying a new configuration. Any deploy +configuration a caller supplies to the start SHALL NOT be pushed onto the environment: it +already knows what to run, and choosing what it runs is `spinloop remote deploy`'s job, not +a node start's. An undeployed remote environment — one with no stored deploy config — +SHALL still refuse a node-level start, with a message naming the deployment path, rather +than attempted: starting one would mean choosing what to serve and paying for provisioning +and weights, a heavier decision a node start must not make on a caller's behalf. + +#### Scenario: A remote environment answers status like a node + +- **WHEN** a remote environment is asked for its status as a member of a node set +- **THEN** it returns a status carrying the endpoint's state, what its engine is serving + (the model, and the served name beside it when the deploy gave one), where its engine + answers (the instance's published address), and, when the engine has done work, its + last-active time, in the same shape a local node's status carries + +#### Scenario: A freshly loaded engine shows its model before it has done work + +- **WHEN** a remote environment's engine is serving a model but has not yet answered a + request, so it reports no last-active time +- **THEN** its status still carries the model it is serving, so a router can match a + request to it before the first request has landed + +#### Scenario: A remote environment answers metrics like a node + +- **WHEN** a running remote environment is asked for its metrics as a member of a node set +- **THEN** it returns the token and system figures in the same stats shape a local node + returns + +#### Scenario: A rejected control call is a typed outcome + +- **WHEN** a remote environment's status or metrics call is rejected, for example because + the caller's credentials are not valid +- **THEN** the environment is reported with a failure outcome and the reason, and it does + not abort or blank the rest of the node set + +#### Scenario: A deployed remote environment is started + +- **WHEN** a node-level start is requested for a stopped remote environment whose stored + deploy config already describes what to serve +- **THEN** the environment's instance is booted, the call waits for it the way a local + node's start does, and the environment serves what its own stored config names, not + any config the start call carried + +#### Scenario: Waking a remote environment is refused + +- **WHEN** a node-level start is requested for a remote environment with no stored deploy + config +- **THEN** it is refused with a message naming the deployment path, and the environment + is not started diff --git a/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/tasks.md b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/tasks.md new file mode 100644 index 00000000..c272b401 --- /dev/null +++ b/openspec/changes/archive/2026-09-14-gateway-start-remote-nodes/tasks.md @@ -0,0 +1,224 @@ +## 1. Per-node wake override + +- [x] 1.1 Add `NodeConfig.WakePolicy WakePolicy` (`yaml:"wake,omitempty"`) in + `internal/fleet/config.go`, validated at parse time the same way the + fleet-wide `wake` value is (`ParseWakePolicy`, failing on anything but + `on`/`off`/absent), and verify with a parse test covering a valid and + an invalid per-node value. +- [x] 1.2 Add `Config.NodeWakes(entry NodeConfig) bool`: the node's own + `WakePolicy` when it names one, else `c.Wakes()`. Verify with unit + tests for all four combinations (fleet on/off crossed with node + unset/on/off). +- [x] 1.3 In `internal/fleet/wake.go`, skip a candidate in `Wake`'s own loop + when `c.NodeWakes(cand.entry)` is false, refusing it with a + "waking is disabled for this node" reason the same way a + config-mismatched candidate is refused — `wakeable()`'s ordering and + `WouldWake` stay policy-agnostic, since a caller (`fleet route`'s and + the gateway's pre-emptive refusal) still needs to name the node a + wake-off setting refused. Add `Config.AnyNodeWakes() bool` for that + pre-emptive check, and swap it in wherever `cfg.Wakes()` gated a + pre-`Wake` refusal, including `cmd/spinloop/route.go`. Verify with a + `Wake` test where a fleet that wakes has one node with `wake: off` + that must not be chosen, and one where a fleet that does not wake has + one node with `wake: on` that must be chosen. + +## 2. Remote node starts on wake + +- [x] 2.1 Replace `remoteNode.StartWith` in `internal/fleet/remote_node.go` + with a delegation to `StartWithProgress` (ignoring `dc` and + `engineKey`, as `Start` already does), removing the unconditional + refusal. Verify with a test that `StartWith` boots the same way + `Start` does against a fake control plane. +- [x] 2.2 Have `StartWith` read a fresh status first and refuse immediately, + naming `spinloop remote deploy`, when it reports nothing served — + `remote.Start`'s own retry loop treats an undeployed environment's + `503` the same as a capacity wait, so leaving it to that call would + hold the request for the whole wake timeout instead of failing fast. + Verify with a test that `StartWith` against a fake control plane + reporting no served model refuses immediately without calling the + boot endpoint. + +## 3. Gateway: resolving a remote node's wakeable config + +- [x] 3.1 In `internal/gateway/gateway.go`, add a resolver that builds an + `inference.DeployConfig{ModelID, ServedModelName}` for a `KindRemote` + entry from its `NodeResult.Status` (`Model`, `ServedName`) within a + given `results []fleet.NodeResult`, erroring when there is no OK + result for that node or when it reports nothing being served (nothing + deployed) — mirroring the error a daemon node with no resolvable + source already produces. Verify with a unit test covering: a deployed + status resolves; an undeployed (empty Model/ServedName) status errors + naming `spinloop remote deploy`; a missing/not-OK result errors. +- [x] 3.2 Combine that resolver with `h.cfgFor` into one `fleet.ConfigFor` + keyed by `entry.Kind`, and use the combined resolver everywhere + `h.cfgFor` is currently passed to `matchingConfigFor`, `Wake`, and the + wakeable-models loop. Verify by confirming existing daemon-path tests + for `wakeFor`/`refuseWake`/`wakeableModels` still pass unchanged. + +## 4. Gateway: advertising and waking a remote node + +- [x] 4.1 Change `wakeableModels()` to accept `results []fleet.NodeResult`, + drop the `entry.Kind != fleet.KindDaemon` skip, resolve every node's + wakeable model through the combined resolver from 3.2, and skip a + node when `!h.cfg.NodeWakes(entry)`. Update its two call sites + (`handleModels`, `handleTopology`) to pass the `results` they already + hold. Verify with a test that a deployed, stopped remote node's model + appears in `/v1/models` and in the topology's `wakeableModel`, and + that an undeployed one, and a wake-disabled one, do not. +- [x] 4.2 Change `wakeFor`/`refuseWake` to check `h.cfg.NodeWakes(entry)` per + candidate node instead of the single `h.cfg.Wakes()` gate, and to use + the combined resolver. Verify with a test that a request for a model + only a deployed, stopped remote node serves is held and answered once + that node's `StartWith` reports it running (using a fake remote + control plane, the way existing remote-node gateway tests already + fake one). +- [x] 4.3 Verify with a test that a request for a model only an undeployed + remote node's name could match still fails, naming the deployment + path, and that a request for a model only a wake-disabled remote node + serves fails naming that it is not woken — both without starting + anything. + +## 5. Specs and docs + +- [x] 5.1 Run `openspec validate --strict gateway-start-remote-nodes` (or + the store-scoped form if applicable) and fix any reported issues in + the delta specs. +- [x] 5.2 Update `docs/` and `README.md` fleet/gateway documentation, if any + describes the current "remote nodes are never woken" behaviour or the + fleet-wide-only `wake` setting, to reflect the per-node override and + remote wake support (handled by `/docs-update` after implementation). + +## 6. Full verification + +- [x] 6.1 Run `go test ./... -cover` and confirm coverage stays at or above + 80%, with the new/changed packages (`internal/fleet`, + `internal/gateway`) individually meeting it. All packages pass; + `internal/fleet` 89.2%, `internal/gateway` 93.8%, `cmd/spinloop` 91.3%. +- [x] 6.2 Run `gofmt -l .` and confirm it reports nothing. + +## 7. Bug fix: resolve a remote node's wakeable model from its stats reply + +Found running the orchestrator against a real deployed-but-stopped remote +node: the gateway's topology named no `wakeableModel` for it, and the +orchestrator's dispatch failed with "node X reports no model to run item +against". Root cause: the remote control plane's *status* reply only relays +deploy-config facts (runner, model id, served name) while the environment is +`running` — a stopped environment's status reply carries only its state and +address. Task 3.1's resolver read `NodeResult.Status`, so it always saw a +stopped-but-deployed remote node as if nothing were deployed. The *stats* +reply is the one that reads the deploy config directly regardless of run +state (and fails outright when there is none to read) — the same source +`spinloop remote metrics`/`stats` already uses. + +- [x] 7.1 Rewrite `remoteConfigFor` (`internal/gateway/gateway.go`) as a + `Handler` method taking a `context.Context`: build the node via + `h.cfg.NewNode(entry)` and resolve its `inference.DeployConfig` from + `node.Metrics(ctx)` (`stats.ModelID`; no served name — the stats reply + carries none), erroring straight through a failed `Metrics` call + (unregistered environment, no `stats_url`, or an undeployed + environment's stats read, which the control plane itself fails). + `combinedConfigFor`, `wakeableModels`, and `wakeFor` thread the context + through instead of `results`, since the resolution is no longer a pure + read of already-fanned-out data. Verify with a unit test covering an + unregistered environment, and with `handleModels`/`handleTopology` + tests using a fake control plane that only carries deploy facts on its + `/stats` route, matching the real Lambda split. +- [x] 7.2 Remove the "nothing deployed" pre-check `StartWith` + (`internal/fleet/remote_node.go`) gained in task 2.2: it read the same + status reply, so it could never actually distinguish undeployed from + stopped-and-deployed either, and would have refused every legitimate + wake once 3.1's bug was fixed elsewhere. The gateway's own candidate + matching (7.1) already confirms deployment via stats before a + candidate is chosen, and `Wake` has exactly one call site for + `StartWith` on a remote node — so nothing currently reaches `StartWith` + without that confirmation already having happened. Verify with a test + that `StartWith` boots regardless of what a status read alone would + have said. +- [x] 7.3 Update the delta specs (`fleet-gateway`), `design.md`, and + `docs/commands/gateway.md` to say "stats reply", not "status reply" or + "last status", for where a remote node's wakeable model comes from, and + to drop the served-name claim for a stopped remote node (the stats + reply carries none — model id only). +- [x] 7.4 Re-run `go test ./... -cover` and `gofmt -l .` to confirm the fix + and its test changes are clean. + +## 8. Bug fix: coalesce concurrent wakes of the same node + +Found immediately after 7's fix, still against the same real fleet: two +gateway log lines, "Waking dev-4 to serve X..." twice inside half a second, +for what was one orchestrator dispatch. A daemon node's control API turns a +racing second start into a `409` the loser joins rather than repeats — that +race was never actually a problem for a daemon node. A remote environment's +control plane has no equivalent: its instance lookup is eventually +consistent right after a launch, so two wakes racing within that window can +each miss the other's not-yet-visible instance and each launch one — a real +double-billed EC2 launch, not a cosmetic duplicate log line. + +- [x] 8.1 Add `golang.org/x/sync/singleflight` (`go get + golang.org/x/sync@v0.16.0` — pinned to the version already resolvable + in the dependency graph, then `go mod tidy`, taking care the `go` + directive in `go.mod` does not get bumped as a side effect). +- [x] 8.2 In `internal/fleet/wake.go`, add a package-level + `wakeSingleflight singleflight.Group` keyed by `fleet.Path + node + name`, and extract the "start (or join a race) and wait for ready" + step of `Wake`'s per-candidate loop into `Config.startAndWait`, called + through `wakeSingleflight.Do` so concurrent callers for the same node + share one call. Run the shared call on `context.Background()`, not + any one caller's request context, so one caller's cancelled request + cannot cut short another caller still waiting on the same node. + Signal a fatal outcome (engine started or found already running, but + never answered — must not be retried elsewhere) through a + `*fatalWakeError` wrapper the loop checks after `Do` returns, keeping + that wake-specific distinction out of singleflight's own contract. + Verify with a test asserting a fake node's `/v1/start` is called + exactly once when two `Wake` calls race for it — the fake does not + itself reject a concurrent start the way a real daemon does, so this + only passes with the coalescing in place. +- [x] 8.3 Update the `fleet-gateway` delta spec's "Waking a node for a + request" requirement and its concurrent-wake scenarios, + `docs/commands/gateway.md`, and `design.md` to describe the + coalescing as a property of every node kind, not something a daemon's + `409` happens to provide. +- [x] 8.4 Re-run `go test ./... -cover -race` (at least for + `internal/fleet` and `internal/gateway`) and `gofmt -l .`. + +## 9. Bug fix: a remote wake trusted an open port over "still loading" + +Found once dev-4 finally booted: the gateway routed a request to it and the +launched agent's own request failed with "Loading model" — the engine's +process was up and its port was accepting connections, but the model had +not finished loading. Two bugs compounded: + +- `statusFromRemote` (`internal/fleet/remote_node.go`) never mapped the + control plane's own `healthy` reading (`remote.Response.Healthy` — the + same `/health` check, excluding the 503 an engine answers while loading, + that a running remote view already surfaces) onto `daemon.StatusResponse + .Ready`, so a remote node's readiness was always unknown to `waitReady`. +- `waitReady` (`internal/fleet/wake.go`) treated any `Ready` value short of + `ReadyYes` — including an explicit `ReadyNo`, not just "no reading yet" — + as a reason to fall back to a raw TCP probe, which only checks that the + port accepts a connection. Both llama.cpp and vLLM open their port before + they can serve a request, so the probe returns true during exactly the + window `ReadyNo` exists to describe — the probe was second-guessing a + reading that had already answered. This affected a `kind: daemon` node + too, wherever its own reading said `ReadyNo`; it was simply never hit + before dev-4 made the remote gap likely on every cold wake, since a + remote node's `Ready` was always empty and so always fell through. + `internal/fleet/select.go`'s ordinary routing (an already-running node) + already got this right — only `waitReady`'s wake-time polling had the bug. + +- [x] 9.1 Map `resp.Healthy` onto `Ready` in `statusFromRemote` + (`internal/fleet/remote_node.go`): `true` to `daemon.ReadyYes`, + `false` to `daemon.ReadyNo`, `nil` (no reading landed) left empty. + Verify with a unit test covering all three. +- [x] 9.2 In `waitReady`, branch on `status.Ready` three ways instead of a + single `== ReadyYes` check with everything else falling through to + the TCP probe: `ReadyYes` returns immediately, `ReadyNo` waits without + probing, and only an empty reading (no convention to trust) falls + back to the probe. Verify with a test where a node reports `ReadyNo` + while its engine's port already answers, asserting `Wake` does not + return until the reading itself changes (here: times out, since it + never does) — and a second test carrying this through the full + remote path: a fake control plane reporting running-but-unhealthy for + a stretch after boot, asserting the wake takes at least that long. +- [x] 9.3 Re-run `go test ./... -cover -race` and `gofmt -l .`. diff --git a/openspec/specs/fleet-config/spec.md b/openspec/specs/fleet-config/spec.md index 4394112a..74a959ed 100644 --- a/openspec/specs/fleet-config/spec.md +++ b/openspec/specs/fleet-config/spec.md @@ -373,40 +373,69 @@ without a `gateway` section SHALL behave exactly as it does today. A fleet file MAY declare a top-level `wake` value of `on` or `off`, deciding whether routing starts an engine on a node that is not running one when no -running node serves what is wanted. It belongs to the file rather than to each -node, for the reason `prefer` does: it describes how this cluster is to be -used — may work be started on its machines on demand, or only used where it is -already running — which is a property of the fleet, not of any one machine in -it. - -A file declaring nothing SHALL wake, as routing does when the setting is -absent: waking is the difference between a fleet that answers a request and -one that must be prepared by hand, and the file's author is the one who owns -the machines it names. A file declaring anything other than `on` or `off` SHALL -fail to parse, naming both accepted values, in keeping with the file's other -validation. - -The setting SHALL decide whether to wake only. It SHALL NOT change which node -is chosen, how matching nodes are ranked, or what a wake does: a fleet that -declares `wake: off` still reports, when nothing is running, the node whose -source describes the wanted model and the command that would start it. +running node serves what is wanted. It applies to every node that declares +no `wake` setting of its own — the same reason `prefer` is fleet-wide: it +describes how this cluster is to be used — may work be started on its +machines on demand, or only used where it is already running — which is a +property of the fleet, not of any one machine in it, unless that machine's +entry says otherwise. + +A node entry MAY declare its own `wake` value, `on` or `off`, in the same +shape as the fleet-wide setting. When a node names one, it decides whether +that node may be woken, taking precedence over the fleet-wide setting for +that node alone; a node naming none is governed by the fleet-wide setting as +before. This exists because waking is not free the same way on every node: a +remote environment's wake boots and pays for a cloud instance, unlike a local +daemon's engine, so an operator may want the fleet's daemons to wake freely +while deciding a remote node's waking on its own terms — opted in under a +fleet that otherwise does not wake, or opted out under one that does — +without a second fleet-wide flag governing every remote node in the file +alike. + +A file declaring nothing at either level SHALL wake, as routing does when no +setting decides otherwise: waking is the difference between a fleet that +answers a request and one that must be prepared by hand, and the file's +author is the one who owns the machines it names. A fleet-wide or per-node +`wake` declaring anything other than `on` or `off` SHALL fail to parse, +naming both accepted values, in keeping with the file's other validation. + +The setting SHALL decide whether to wake only, at whichever level decides it +for a given node. It SHALL NOT change which node is chosen, how matching +nodes are ranked, or what a wake does: a node for which waking is not +allowed still reports, when nothing is running, the node whose source +describes the wanted model and the command that would start it. #### Scenario: A fleet that declares nothing wakes -- **WHEN** a fleet file declares no `wake` setting and routing finds no node - serving what is wanted +- **WHEN** a fleet file declares no `wake` setting at either level and + routing finds no node serving what is wanted - **THEN** routing starts an engine on a suitable node, as it does today #### Scenario: A fleet that refuses to wake -- **WHEN** a fleet file declares `wake: off` and routing finds no node serving - what is wanted +- **WHEN** a fleet file declares `wake: off` and no node overrides it, and + routing finds no node serving what is wanted - **THEN** nothing is started, and the failure names the node that would be woken and the command that would start it +#### Scenario: A node opts out under a fleet that wakes + +- **WHEN** a fleet file declares `wake: on`, one node declares its own + `wake: off`, and that node is the only one serving what is wanted +- **THEN** that node is not woken, and the failure names it and says waking + is disabled for it, even though the fleet otherwise wakes + +#### Scenario: A node opts in under a fleet that does not wake + +- **WHEN** a fleet file declares `wake: off`, one node declares its own + `wake: on`, and that node is the only one serving what is wanted +- **THEN** that node is woken, though the rest of the fleet still does not + wake + #### Scenario: An unknown value is rejected at parse time -- **WHEN** a fleet file declares `wake: sometimes` +- **WHEN** a fleet file declares `wake: sometimes`, at the fleet level or on + a node - **THEN** parsing fails naming `on` and `off` ### Requirement: Node tags diff --git a/openspec/specs/fleet-gateway/spec.md b/openspec/specs/fleet-gateway/spec.md index 55cdd594..b8635e66 100644 --- a/openspec/specs/fleet-gateway/spec.md +++ b/openspec/specs/fleet-gateway/spec.md @@ -91,19 +91,23 @@ fleet client does. The gateway SHALL serve `GET /v1/models` returning, in the OpenAI list shape, the union of the models a request can reach. For each node whose state is `running`, the list SHALL carry the name it reports serving — the served name -when it reports one, otherwise the model id. When the fleet's wake allows -starting an engine, the list SHALL additionally carry, for each node that is -not running and answers its status, the model that node's own Spinloop source -describes — the same served-name-first naming the wake would start it with — -since a request naming that model starts that node. A running node SHALL -contribute nothing but what it reports: a running engine is never displaced, -so its source's model is not a request the gateway would answer from it. A -node the gateway cannot start — a remote environment, which starts from -`spinloop remote deploy`, not from a request — and a fleet whose wake is off -SHALL contribute nothing beyond what is running, and duplicates SHALL be -listed once. The source a node describes SHALL be resolved at most once in a -short window shared by all models requests, so a burst does not re-read every -node's source. +when it reports one, otherwise the model id. For each node that is not +running and is a wake candidate — waking is allowed for it (its own `wake` +setting, or the fleet-wide one when it names none) and it names a model to +start with — the list SHALL additionally carry that model, under the same +served-name-first naming a wake would start it with: a daemon node's own +Spinloop source describes it, and a remote node's own stats reply carries it +— the environment's stored deploy config, which the stats reply carries +whether the environment is running or stopped (unlike the status reply, +which only relays it while running). A running node SHALL contribute nothing +but what it reports: a running engine is never displaced, so its source's +model is not a request the gateway would answer from it. An undeployed remote +environment — one whose stats read fails outright, having no deploy config +to read — and a node for which waking is not allowed SHALL contribute +nothing beyond what is running, and duplicates SHALL be listed once. The +model a node would be started with SHALL be resolved at most once in a short +window shared by all models requests, so a burst does not re-read every +node's source or re-fetch every remote node's stats. #### Scenario: Running models are listed @@ -113,22 +117,30 @@ node's source. #### Scenario: A stopped node's wakeable model is listed -- **WHEN** a node is stopped, its own source describes a model, and the fleet - allows waking, and a models request is made +- **WHEN** a node is stopped, its own source describes a model, and waking is + allowed for it, and a models request is made - **THEN** the response lists the model the source describes, beside what the running nodes serve #### Scenario: A stopped node's model is not listed when wake is off -- **WHEN** a node is stopped and the fleet's wake is off, and a models request - is made +- **WHEN** a node is stopped and waking is not allowed for it, and a models + request is made - **THEN** the response lists only what the running nodes serve +#### Scenario: A deployed remote environment's model is listed + +- **WHEN** a remote environment is stopped, its stats reply reports what its + stored deploy config would serve, and waking is allowed for it, and a + models request is made +- **THEN** the response lists that model beside what the running nodes serve + #### Scenario: A stopped remote environment's model is not listed -- **WHEN** a remote environment is stopped and a models request is made -- **THEN** the response does not list what its source describes: the gateway - cannot start it, so a request naming that model would fail +- **WHEN** a remote environment is stopped and has nothing deployed, and a + models request is made +- **THEN** the response does not list it: the gateway has nothing stored to + start it with #### Scenario: A running node's source adds no second model @@ -139,14 +151,14 @@ node's source. #### Scenario: A burst of models requests resolves each source once - **WHEN** several models requests arrive within the window in which a node's - source is resolved -- **THEN** each node's source is read once for the burst + source or a remote node's status is resolved +- **THEN** each node's source or status is read once for the burst #### Scenario: Nothing reachable lists nothing -- **WHEN** no node is running and nothing is wakeable — no stopped node's - source describes a model, or the fleet's wake is off — and a models request - is made +- **WHEN** no node is running and nothing is wakeable — no stopped node + describes a model, or waking is not allowed for any of them — and a models + request is made - **THEN** the response is an empty list, not an error ### Requirement: Routing a request to a node @@ -285,45 +297,87 @@ and the outcome — and the gateway SHALL log no request body. ### Requirement: Waking a node for a request -When no running node serves the model a request names, and the fleet file's -wake policy allows it, the gateway SHALL start an engine on a node that is not -running one, and SHALL hold the request until the engine answers. A node is a -wake candidate when it is not running and the Spinloop source it names — its -`file` field, a registered alias named after it, or a same-named directory -beside the fleet file, resolved the way `spinloop fleet start` resolves it — -describes a config whose model or served name is the one the request asks for: -a node is started with what it was told to run, never with a config invented -for the request. Candidates whose stored config already names the model SHALL -be tried first, since they have the weights, and the rest in fleet-file order. -A node that refuses the start — a runner or model it cannot serve — SHALL NOT -fail the request while other candidates remain. - -The started engine SHALL be gated with the key the node's fleet entry names, -supplied by the gateway: the gateway is the client that starts the engine, so -the key the client sets is the key the engine takes. The wait SHALL be bounded -by a wake timeout, defaulting to five minutes and overridable by -`--wake-timeout`; exceeding it SHALL fail the request saying the engine did -not answer in time, and the started engine SHALL be left running rather than -stopped, so a slow load is not thrown away. +When no running node serves the model a request names, and waking is allowed +for at least one candidate, the gateway SHALL start an engine on a node that +is not running one, and SHALL hold the request until the engine answers. +Waking is allowed for a node when its own `wake` setting says so, or, +when it names none, the fleet file's wake policy does. A node is a wake +candidate when it is not running, waking is allowed for it, and it names a +model to start with, matching the one the request asks for: + +- A daemon node names one through the Spinloop source it names — its `file` + field, a registered alias named after it, or a same-named directory beside + the fleet file, resolved the way `spinloop fleet start` resolves it — + describing a config whose model or served name is the one the request asks + for. +- A remote node names one through its own stats reply, which reads the + environment's stored deploy config directly and so carries its model id + whether the environment is running or stopped — unlike its status reply, + which only relays the deploy config while running, and unlike the stats + reply itself, which carries no served name. An undeployed remote + environment's stats read fails outright, having no deploy config to read; + it names nothing and is not a candidate. + +A node is started with what it names, never with a config invented for the +request: a daemon node is started with the Spinloop source's config; a remote +node is started as it is — its stored deploy config decides what it serves, +and the gateway pushes it nothing new. Candidates whose stored config already +names the model SHALL be tried first, since they have the weights, and the +rest in fleet-file order. A node that refuses the start — a runner or model it +cannot serve — SHALL NOT fail the request while other candidates remain. + +A daemon engine started this way SHALL be gated with the key the node's fleet +entry names, supplied by the gateway: the gateway is the client that starts +the engine, so the key the client sets is the key the engine takes. A remote +environment's engine is gated by its own key, resolved the same way a request +already routed to it resolves one; the gateway does not change it. The wait +SHALL be bounded by a wake timeout, defaulting to five minutes and +overridable by `--wake-timeout`; exceeding it SHALL fail the request saying +the engine did not answer in time, and the started engine SHALL be left +running rather than stopped, so a slow load — or, for a remote node, a slow +boot — is not thrown away. When several requests ask for a model nothing is serving at once, the gateway SHALL start at most one engine per node and answer every request from it: the -first request's wait is the wait the rest join. A node another request woke -first SHALL be used the same way, and only once its engine answers. - -With the wake policy off, a request for a model nothing is serving SHALL fail -without starting anything, naming the nodes and what they could serve, and the -command that would start one. A model no node is running and no node's source -describes SHALL fail the same way, whatever the policy: nothing to wake with, -and the failure SHALL say so rather than trying to start a node with nothing. +first request's wait is the wait the rest join, regardless of the node's +kind. A daemon node's own control API refuses a second concurrent start on +its own, but a remote environment's control plane does not, so the gateway +SHALL NOT rely on that alone: two requests racing to wake the same node +SHALL be coalesced before either reaches the node, not just reconciled after +one of them answers. A node another request woke first SHALL be used the +same way, and only once its engine answers. + +A request for a model nothing is serving, and for which waking is not allowed +on any node that names it, SHALL fail without starting anything, naming the +nodes and what they could serve, and the command that would start one. A +model no node is running and no node names — no daemon source describes it +and no remote node is deployed with it — SHALL fail the same way regardless +of any wake setting: nothing to wake with, and the failure SHALL say so +rather than trying to start a node with nothing. #### Scenario: A cold request wakes a node and is served - **WHEN** no node is running the model a request names, one node's Spinloop - source describes it, and the wake policy allows it + source describes it, and waking is allowed for it - **THEN** that node is started with its own config, gated with the key its fleet entry names, and the request is answered once the engine answers +#### Scenario: A cold request wakes a deployed remote environment + +- **WHEN** no node is running the model a request names, one remote node's + stats reply reports it is deployed to serve it, and waking is allowed for + it +- **THEN** that environment's instance is started, its own stored deploy + config decides what it serves, and the request is answered once its engine + answers + +#### Scenario: An undeployed remote node is not a wake candidate + +- **WHEN** the only node whose name could match a request is a remote + environment with nothing deployed +- **THEN** it is not started, and the failure says nothing is deployed to + serve the model, naming the deployment path + #### Scenario: The request is held while the engine loads - **WHEN** the woken node reports running while its engine is still loading @@ -343,24 +397,39 @@ and the failure SHALL say so rather than trying to start a node with nothing. - **THEN** that node is started once, and both requests are answered from the same engine +#### Scenario: Concurrent cold requests share one remote wake + +- **WHEN** two requests arrive at once for a model nothing is serving, and one + remote node's stats reply reports it is deployed to serve it +- **THEN** that environment's instance is started once, not once per request, + and both requests are answered once its engine answers + #### Scenario: A woken engine takes the gateway's key -- **WHEN** the gateway starts an engine on a node whose fleet entry names an - engine key +- **WHEN** the gateway starts an engine on a daemon node whose fleet entry + names an engine key - **THEN** the engine is gated with that value, the gateway's requests to it carry it, and no reply to any caller contains it #### Scenario: Wake refused by the fleet file -- **WHEN** the fleet file declares the wake policy off and no node is serving - the model a request names +- **WHEN** waking is not allowed for any node that could serve the model a + request names, and no node is serving it - **THEN** nothing is started, and the request fails naming the node whose source describes the model and the command that would start it +#### Scenario: A remote node opted out is not woken though the fleet wakes + +- **WHEN** the fleet file's wake policy is `on`, a stopped remote node + declares its own `wake: off`, and it is the only node that names the + model a request asks for +- **THEN** it is not started, and the failure names it and says waking is + disabled for that node + #### Scenario: Nothing can serve the model -- **WHEN** no node is running the model a request names and no node's Spinloop - source describes it +- **WHEN** no node is running the model a request names, no node's Spinloop + source describes it, and no remote node is deployed with it - **THEN** the request fails, naming each node and why it cannot serve the model, and nothing is started @@ -400,10 +469,12 @@ its kind, its tags as the fleet file declares them, its state, and its serving facts — the model it serves when it is running, the name it serves that model under where it reports one, whether its engine has answered, and when it was last active. For a node that is not running, the reply SHALL name -the model a request would start it with, where the node's own source -describes one and the fleet's wake policy allows starting it; a node whose -source describes no such model SHALL report none. A node that does not answer -SHALL be reported as such in the fleet's order, not fail the whole reply. +the model a request would start it with, where the node names one — a daemon +node's own source, or a remote node's own stats reply — and waking is +allowed for it (its own `wake` setting, or the fleet's when it names none); a +node that names no such model, or for which waking is not allowed, SHALL +report none. A node that does not answer SHALL be reported as such in the +fleet's order, not fail the whole reply. The reply SHALL carry the fleet file's fleet-level settings the way the file declares them: whether the fleet wakes, how it ranks, and its concurrency @@ -426,11 +497,18 @@ gateway holds no copy of either beyond what it already holds. #### Scenario: A stopped node reports what it would start -- **WHEN** a node is not running, its own source describes a model, and the - fleet wakes +- **WHEN** a node is not running, names a model to start with, and waking is + allowed for it - **THEN** the topology names that model as what a request would start the node with +#### Scenario: A stopped, deployed remote node reports what it would start + +- **WHEN** a remote node is stopped, its stats reply reports its stored + deploy config, and waking is allowed for it +- **THEN** the topology names that config's model as what a request would + start it with, the same way a daemon node's is named + #### Scenario: A dead node does not sink the reply - **WHEN** one of the fleet's nodes does not answer and a caller reads the diff --git a/openspec/specs/remote-node/spec.md b/openspec/specs/remote-node/spec.md index 20ecb8e5..68d3e28e 100644 --- a/openspec/specs/remote-node/spec.md +++ b/openspec/specs/remote-node/spec.md @@ -26,9 +26,15 @@ including a rejected AWS credential — SHALL be reported as a typed outcome aga environment, the same way an unreachable or unauthorized node is, rather than failing the command or being silently dropped. -Because a remote endpoint is provisioned by deployment rather than woken like a node, a -node-level start asked to run on a supplied deploy configuration SHALL be refused with a -message naming the deployment path, rather than attempted. +A deployed remote environment — one whose stored deploy config already describes what to +serve — SHALL answer a node-level start the same way a local node does: the instance is +booted and the call waits for it, without deploying a new configuration. Any deploy +configuration a caller supplies to the start SHALL NOT be pushed onto the environment: it +already knows what to run, and choosing what it runs is `spinloop remote deploy`'s job, not +a node start's. An undeployed remote environment — one with no stored deploy config — +SHALL still refuse a node-level start, with a message naming the deployment path, rather +than attempted: starting one would mean choosing what to serve and paying for provisioning +and weights, a heavier decision a node start must not make on a caller's behalf. #### Scenario: A remote environment answers status like a node @@ -58,10 +64,18 @@ message naming the deployment path, rather than attempted. - **THEN** the environment is reported with a failure outcome and the reason, and it does not abort or blank the rest of the node set +#### Scenario: A deployed remote environment is started + +- **WHEN** a node-level start is requested for a stopped remote environment whose stored + deploy config already describes what to serve +- **THEN** the environment's instance is booted, the call waits for it the way a local + node's start does, and the environment serves what its own stored config names, not + any config the start call carried + #### Scenario: Waking a remote environment is refused -- **WHEN** a node-level start is requested for a remote environment, carrying a deploy - configuration +- **WHEN** a node-level start is requested for a remote environment with no stored deploy + config - **THEN** it is refused with a message naming the deployment path, and the environment is not started