From c3308d1954bfc294b73f0c0fc47c57c4367090c5 Mon Sep 17 00:00:00 2001 From: spinloop-agent Date: Fri, 18 Sep 2026 22:25:43 +0100 Subject: [PATCH] fix(gateway): carry a remote node's served name into its wake stats The stats Lambda already read the deploy config's servedModelName but never included it in its reply, so a stopped remote node's wake match only had its bare model id to compare against. A caller that had been serving requests under the node's ALIAS while it was running lost that match the moment it stopped, and the gateway refused to wake it. --- docs/openapi.yaml | 7 ++++++ internal/daemon/daemon.go | 3 ++- internal/fleet/remote_node.go | 1 + internal/gateway/gateway.go | 9 +++---- internal/gateway/gateway_test.go | 40 ++++++++++++++++++++++++++++---- internal/metrics/metrics.go | 11 ++++++--- internal/remote/remote.go | 19 +++++++++------ remote/lambda/shared/stats.ts | 7 ++++++ remote/lambda/stats/index.ts | 2 ++ remote/test/stats.test.ts | 23 ++++++++++++++++++ 10 files changed, 103 insertions(+), 19 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 2286b28c..1b2f02b0 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -398,6 +398,13 @@ components: type: string modelId: type: string + servedName: + type: string + description: | + The name the engine answers to beside the model id, mirroring + StatusResponse's `servedName` from the same record — so a caller + resolving a stopped host's model from its metrics gets the same + name a running host reports on its status. uptimeSeconds: type: integer tokens: diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 3af88053..a37e74c9 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -427,11 +427,12 @@ func (d *Daemon) activity() (lastActiveAt string, idleSeconds int) { // Errors; an absent source is simply omitted, per the engine-metrics spec. func (d *Daemon) Metrics(ctx context.Context) metrics.Stats { state, _, uptime := d.Sup.Status() - runner, model, _ := d.served() + runner, model, servedName := d.served() stats := metrics.Stats{ State: string(state), Runner: runner, ModelID: model, + ServedName: servedName, UptimeSeconds: uptime, } if state == StateRunning { diff --git a/internal/fleet/remote_node.go b/internal/fleet/remote_node.go index 0b73548f..9887146d 100644 --- a/internal/fleet/remote_node.go +++ b/internal/fleet/remote_node.go @@ -229,6 +229,7 @@ func statsFromRemote(resp remote.StatsResponse) metrics.Stats { State: resp.State, Runner: resp.Runner, ModelID: resp.ModelID, + ServedName: resp.ServedName, UptimeSeconds: resp.UptimeSeconds, Tokens: resp.Tokens, GPUs: resp.GPUs, diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index c8f2213d..ade0c016 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -329,9 +329,10 @@ func (h *Handler) wakeableModels(ctx context.Context) map[string]string { // 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. +// what "nothing deployed" looks like here. It carries the served name +// alongside the model id too, the same field the deploy config's ALIAS +// sets, so a stopped remote node's wakeable name matches what it reported +// while running rather than falling back to the bare model id. func (h *Handler) remoteConfigFor(ctx context.Context) fleet.ConfigFor { return func(entry fleet.NodeConfig) (inference.DeployConfig, error) { node, err := h.cfg.NewNode(entry) @@ -342,7 +343,7 @@ func (h *Handler) remoteConfigFor(ctx context.Context) fleet.ConfigFor { 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 + return inference.DeployConfig{ModelID: stats.ModelID, ServedModelName: stats.ServedName}, nil } } diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index 8efdaea2..94994272 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -471,7 +471,10 @@ func TestModelsListsOnlyWhatRunsWhenWakeIsOff(t *testing.T) { // 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. +// So it is wakeable and listed the same as a daemon node's, under the same +// served-name-first naming: the stats reply carries the served name beside +// the model id, so a stopped environment lists the same name it would report +// while running rather than the bare model id. func TestModelsListsADeployedRemoteEnvironment(t *testing.T) { url, _ := remoteControlServer(t) registerRemoteEnv(t, "env", url) @@ -480,8 +483,8 @@ func TestModelsListsADeployedRemoteEnvironment(t *testing.T) { }} 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) + if got := m["env"]; got != "deployed" { + t.Errorf("wakeableModels()[env] = %q, want the served name from its stats reply", got) } } @@ -1200,7 +1203,7 @@ func remoteControlServer(t *testing.T) (url string, started func() bool) { }) 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"}`) + fmt.Fprint(w, `{"state":"stopped","runner":"llamacpp","modelId":"org/deployed","servedName":"deployed"}`) }) mux.HandleFunc("POST /", func(w http.ResponseWriter, r *http.Request) { mu.Lock() @@ -1256,6 +1259,35 @@ func TestColdRequestWakesADeployedRemoteNode(t *testing.T) { } } +// A request for a deployed-but-stopped remote node's served name — the +// alias a caller knew it by while it was running, rather than its bare model +// id — still matches: the stats reply the wake path reads carries the served +// name from the deploy config, the same field the status reply carries while +// running, so a caller need not learn a second name once the node stops. +func TestColdRequestWakesADeployedRemoteNodeByItsServedName(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":"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. diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 860c0bfc..99af154f 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -13,9 +13,14 @@ package metrics // response field-for-field (minus the Lambda's transport fields), so the // existing `spinloop remote metrics` formats render it unchanged. type Stats struct { - State string `json:"state"` - Runner string `json:"runner,omitempty"` - ModelID string `json:"modelId,omitempty"` + State string `json:"state"` + Runner string `json:"runner,omitempty"` + ModelID string `json:"modelId,omitempty"` + // ServedName is the name the engine answers to beside the model id — + // mirroring daemon.StatusResponse.ServedName from the same record — so a + // caller resolving a stopped host's model from its metrics gets the same + // name a running host reports on its status. + ServedName string `json:"servedName,omitempty"` UptimeSeconds int `json:"uptimeSeconds,omitempty"` Tokens *TokenStats `json:"tokens,omitempty"` GPUs []GpuStat `json:"gpus,omitempty"` diff --git a/internal/remote/remote.go b/internal/remote/remote.go index 8cdf5444..3b71f2af 100644 --- a/internal/remote/remote.go +++ b/internal/remote/remote.go @@ -727,13 +727,18 @@ type StatsResponse struct { // Message carries a rejection reason on a non-success reply — including the // authorizer's own text on a 403 — so an expired-credential rejection can be // classified even though the stats fields are empty. - Message string `json:"message"` - Environment string `json:"environment"` - State string `json:"state"` - InstanceID string `json:"instanceId"` - InstanceType string `json:"instanceType"` - Runner string `json:"runner"` - ModelID string `json:"modelId"` + Message string `json:"message"` + Environment string `json:"environment"` + State string `json:"state"` + InstanceID string `json:"instanceId"` + InstanceType string `json:"instanceType"` + Runner string `json:"runner"` + ModelID string `json:"modelId"` + // ServedName is the name the engine answers to beside the model id, + // relayed from the environment's deploy config — the same field the + // status reply's Response.ServedName carries, so a stopped environment's + // wakeable name matches what it reported while running. + ServedName string `json:"servedName"` UptimeSeconds int `json:"uptimeSeconds"` Tokens *TokenStats `json:"tokens"` GPUs []GpuStat `json:"gpus"` diff --git a/remote/lambda/shared/stats.ts b/remote/lambda/shared/stats.ts index 75232e29..a8be9c19 100644 --- a/remote/lambda/shared/stats.ts +++ b/remote/lambda/shared/stats.ts @@ -79,6 +79,13 @@ export interface StatsResult { runner?: string; /** Model id from deploy config. */ modelId?: string; + /** + * The name the engine answers to beside the model id, from the deploy + * config's ALIAS — the same field the start/env Lambdas' status reply + * carries, so a stopped environment's wakeable name matches what it + * reported while running. + */ + servedName?: string; /** Uptime in seconds since launch. */ uptimeSeconds?: number; /** Token/request metrics from the daemon's engine scrape. */ diff --git a/remote/lambda/stats/index.ts b/remote/lambda/stats/index.ts index 64c0c943..05f75dec 100644 --- a/remote/lambda/stats/index.ts +++ b/remote/lambda/stats/index.ts @@ -82,6 +82,7 @@ export async function handler(event: LambdaFunctionURLEvent): Promise { const futureTag = '2030-01-02T04:00:00.000Z'; const pastTag = '2020-01-02T04:00:00.000Z'; +describe('servedName', () => { + it('is relayed from the deploy config on a running instance', async () => { + findManagedInstance.mockResolvedValue({ instanceId: 'i-run', state: 'running' }); + readDeployConfig.mockResolvedValue({ runner: 'llamacpp', modelId: 'org/m', servedModelName: 'the-alias' }); + + const result = await handler(statsEvent({ env: 'dev' })); + const body = bodyOf(result); + expect(statusOf(result)).toBe(200); + expect(body.servedName).toBe('the-alias'); + }); + + it('is relayed from the deploy config on a stopped instance, so a wake matches it', async () => { + findManagedInstance.mockResolvedValue({ instanceId: 'i-stopped', state: 'stopped' }); + readDeployConfig.mockResolvedValue({ runner: 'llamacpp', modelId: 'org/m', servedModelName: 'the-alias' }); + + const result = await handler(statsEvent({ env: 'dev' })); + const body = bodyOf(result); + expect(statusOf(result)).toBe(200); + expect(body.state).toBe('stopped'); + expect(body.servedName).toBe('the-alias'); + }); +}); + describe('retainUntil', () => { it('is present on a running instance whose tag is in the future', async () => { findManagedInstance.mockResolvedValue({