Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1878,7 +1878,6 @@ async function startGatewayWithOptions(
}s for first-time startup.`,
);
}

if (
await waitForGatewayHealth({
attachGatewayMetadataIfNeeded,
Expand All @@ -1887,7 +1886,8 @@ async function startGatewayWithOptions(
healthPollCount: healthWait.count,
healthPollIntervalSeconds: healthWait.interval,
isGatewayHealthy,
isGatewayHttpReady,
isGatewayHttpReady: (signal) =>
isGatewayHttpReady(undefined, undefined, undefined, signal),
repairGatewayBootstrapSecrets,
runCaptureOpenshell,
sleepSeconds,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/onboard/docker-driver-gateway-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { spawnSync, type SpawnSyncOptions } from "node:child_process";
import { type SpawnSyncOptions, spawnSync } from "node:child_process";
import fs from "node:fs";
import path from "node:path";

Expand Down
28 changes: 28 additions & 0 deletions src/lib/onboard/gateway-binding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe("dynamic gateway runtime helpers", () => {
undefined,
"http://127.0.0.1:8080/",
undefined,
undefined,
);
expect(probeDockerDriverGatewayHttpReady).toHaveBeenLastCalledWith(
undefined,
Expand All @@ -72,6 +73,7 @@ describe("dynamic gateway runtime helpers", () => {
undefined,
"http://127.0.0.1:8081/",
undefined,
undefined,
);
expect(getGatewayClusterImageDrift).toHaveBeenLastCalledWith({
gatewayName: "nemoclaw-8081",
Expand Down Expand Up @@ -100,12 +102,38 @@ describe("dynamic gateway runtime helpers", () => {
25,
"https://probe.example/health",
"POST",
undefined,
);
await expect(helpers.waitForGatewayHttpReady()).resolves.toBe(true);
expect(probeGatewayHttpReady).toHaveBeenLastCalledWith(
undefined,
"http://127.0.0.1:9090/",
undefined,
undefined,
);
});

it("forwards explicit HTTP readiness abort signals", async () => {
const probeGatewayHttpReady = vi.fn(async () => true);
const helpers = createDynamicGatewayRuntimeHelpers({
getGatewayName: () => "nemoclaw-9090",
getGatewayPort: () => 9090,
getDockerDriverGatewayEndpoint: (port) => `http://127.0.0.1:${port}`,
getGatewayClusterImageDrift: vi.fn(() => null),
probeGatewayHttpReady,
probeDockerDriverGatewayHttpReady: vi.fn(async () => true),
waitForGatewayHttpReadyBase: vi.fn(async () => true),
probeGatewayTcpReady: vi.fn(async () => true),
});
const controller = new AbortController();

await helpers.isGatewayHttpReady(25, "https://probe.example/health", "POST", controller.signal);

expect(probeGatewayHttpReady).toHaveBeenLastCalledWith(
25,
"https://probe.example/health",
"POST",
controller.signal,
);
});
});
Expand Down
15 changes: 9 additions & 6 deletions src/lib/onboard/gateway-binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,15 @@ export function createDynamicGatewayRuntimeHelpers(deps: DynamicGatewayRuntimeDe
deps.getDockerDriverGatewayEndpoint(deps.getGatewayPort());
const getGatewayClusterImageDrift = () =>
deps.getGatewayClusterImageDrift({ gatewayName: deps.getGatewayName() });
const isGatewayHttpReady = (timeoutMs?: number, url?: string, method?: "GET" | "POST") =>
deps.probeGatewayHttpReady(
timeoutMs,
url ?? `${deps.getDockerDriverGatewayEndpoint(deps.getGatewayPort())}/`,
method,
);
const isGatewayHttpReady = (
timeoutMs?: number,
url?: string,
method?: "GET" | "POST",
signal?: AbortSignal,
) => {
const targetUrl = url ?? `${deps.getDockerDriverGatewayEndpoint(deps.getGatewayPort())}/`;
return deps.probeGatewayHttpReady(timeoutMs, targetUrl, method, signal);
};
const isDockerDriverGatewayHttpReady = (timeoutMs?: number, url?: string) =>
deps.probeDockerDriverGatewayHttpReady(
timeoutMs,
Expand Down
63 changes: 61 additions & 2 deletions src/lib/onboard/gateway-health-wait.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("waitForGatewayHealth", () => {
await expect(waitForGatewayHealth(options)).resolves.toBe(true);

expect(isGatewayHealthy).toHaveBeenCalledTimes(2);
expect(isGatewayHttpReady).toHaveBeenCalledTimes(1);
expect(isGatewayHttpReady).toHaveBeenCalledTimes(2);
expect(options.sleepSeconds).toHaveBeenCalledTimes(1);
expect(options.sleepSeconds).toHaveBeenCalledWith(2);
});
Expand Down Expand Up @@ -94,7 +94,7 @@ describe("waitForGatewayHealth", () => {
await expect(waitForGatewayHealth(options)).resolves.toBe(false);

expect(options.isGatewayHealthy).toHaveBeenCalledTimes(3);
expect(options.isGatewayHttpReady).not.toHaveBeenCalled();
expect(options.isGatewayHttpReady).toHaveBeenCalledTimes(3);
expect(options.sleepSeconds).toHaveBeenCalledTimes(2);
expect(options.sleepSeconds).toHaveBeenNthCalledWith(1, 2);
expect(options.sleepSeconds).toHaveBeenNthCalledWith(2, 2);
Expand Down Expand Up @@ -137,4 +137,63 @@ describe("waitForGatewayHealth", () => {
});
expect(isGatewayHealthy).toHaveBeenCalledWith("status", "named", "current");
});

it("starts the HTTP readiness probe before collecting OpenShell metadata", async () => {
const events: string[] = [];
const openshellOutputByCommand = new Map([
["status", "status"],
["gateway info -g nemoclaw", "named"],
["gateway info", "current"],
]);
const options = buildOptions({
isGatewayHttpReady: vi.fn(async () => {
events.push("http");
return true;
}),
runCaptureOpenshell: vi.fn((args: string[]) => {
const command = args.join(" ");
events.push(command);
return openshellOutputByCommand.get(command) ?? "";
}),
});

await expect(waitForGatewayHealth(options)).resolves.toBe(true);

expect(events).toEqual([
"http",
"gateway select nemoclaw",
"status",
"gateway info -g nemoclaw",
"gateway info",
]);
});

it("aborts the HTTP readiness probe when OpenShell metadata is unhealthy", async () => {
let observedSignal: AbortSignal | undefined;
let aborted = false;
const options = buildOptions({
healthPollCount: 1,
isGatewayHealthy: vi.fn(() => false),
isGatewayHttpReady: vi.fn(
(signal?: AbortSignal) =>
new Promise<boolean>((resolve) => {
observedSignal = signal;
signal?.addEventListener(
"abort",
() => {
aborted = true;
resolve(false);
},
{ once: true },
);
}),
),
});

await expect(waitForGatewayHealth(options)).resolves.toBe(false);

expect(options.isGatewayHttpReady).toHaveBeenCalledOnce();
expect(observedSignal?.aborted).toBe(true);
expect(aborted).toBe(true);
});
});
29 changes: 27 additions & 2 deletions src/lib/onboard/gateway-health-wait.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,32 @@ export interface GatewayHealthWaitOptions {
healthPollCount: number;
healthPollIntervalSeconds: number;
isGatewayHealthy: (status: string, namedInfo: string, currentInfo: string) => boolean;
isGatewayHttpReady: () => Promise<boolean>;
isGatewayHttpReady: (signal?: AbortSignal) => Promise<boolean>;
repairGatewayBootstrapSecrets: () => { repaired: boolean };
runCaptureOpenshell: RunCaptureOpenshell;
sleepSeconds: (seconds: number) => void;
}

function startAbortableGatewayHttpProbe(
isGatewayHttpReady: GatewayHealthWaitOptions["isGatewayHttpReady"],
): { abort: () => void; ready: Promise<boolean> } {
const controller = new AbortController();
let started: Promise<boolean>;
try {
started = Promise.resolve(isGatewayHttpReady(controller.signal));
} catch (error) {
started = Promise.reject(error);
}
const ready = started.catch((error: unknown) => {
if (controller.signal.aborted) return false;
throw error;
});
return {
abort: () => controller.abort(),
ready,
};
}

export async function waitForGatewayHealth({
attachGatewayMetadataIfNeeded,
gatewayClusterHealthcheckPassed,
Expand All @@ -41,13 +61,18 @@ export async function waitForGatewayHealth({
} else if (gatewayClusterHealthcheckPassed()) {
attachGatewayMetadataIfNeeded();
}
const httpProbe = startAbortableGatewayHttpProbe(isGatewayHttpReady);
runCaptureOpenshell(["gateway", "select", gatewayName], { ignoreError: true });
const status = runCaptureOpenshell(["status"], { ignoreError: true });
const namedInfo = runCaptureOpenshell(["gateway", "info", "-g", gatewayName], {
ignoreError: true,
});
const currentInfo = runCaptureOpenshell(["gateway", "info"], { ignoreError: true });
return isGatewayHealthy(status, namedInfo, currentInfo) && (await isGatewayHttpReady());
if (!isGatewayHealthy(status, namedInfo, currentInfo)) {
httpProbe.abort();
return false;
}
return await httpProbe.ready;
},
{
initialIntervalMs: healthPollIntervalMs,
Expand Down
86 changes: 86 additions & 0 deletions src/lib/onboard/gateway-http-readiness.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import http from "node:http";
import type { AddressInfo } from "node:net";

import { afterEach, describe, expect, it } from "vitest";

import { isGatewayHttpReady } from "./gateway-http-readiness";

const servers: http.Server[] = [];

function closeServer(server: http.Server): Promise<void> {
return new Promise((resolve, reject) => {
server.close((error) => {
error ? reject(error) : resolve();
});
});
}

interface ListeningServer {
address: AddressInfo;
url: string;
}

function listen(server: http.Server): Promise<ListeningServer> {
servers.push(server);
return new Promise((resolve, reject) => {
server.once("error", reject);
server.listen(0, "127.0.0.1", () => {
server.off("error", reject);
Promise.resolve(server.address())
.then((address) => {
const listeningAddress = address as AddressInfo;
return {
address: listeningAddress,
url: `http://127.0.0.1:${listeningAddress.port}/`,
};
})
.then(resolve, reject);
});
});
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

describe("isGatewayHttpReady abort handling", () => {
afterEach(async () => {
await Promise.all(servers.splice(0).map((server) => closeServer(server)));
});

it("returns false without opening a request when the signal is already aborted", async () => {
let requests = 0;
const { address, url } = await listen(
http.createServer((_req, res) => {
requests += 1;
res.writeHead(200).end();
}),
);
const controller = new AbortController();
controller.abort();

expect(address).toEqual(expect.objectContaining({ port: expect.any(Number) }));
await expect(isGatewayHttpReady(10_000, url, "GET", controller.signal)).resolves.toBe(false);

expect(requests).toBe(0);
});

it("returns false when an in-flight request is aborted", async () => {
let resolveRequestSeen: () => void = () => undefined;
const requestSeen = new Promise<void>((resolve) => {
resolveRequestSeen = resolve;
});
const { address, url } = await listen(
http.createServer(() => {
resolveRequestSeen();
}),
);
const controller = new AbortController();

expect(address).toEqual(expect.objectContaining({ port: expect.any(Number) }));
const probe = isGatewayHttpReady(10_000, url, "GET", controller.signal);
await requestSeen;
controller.abort();

await expect(probe).resolves.toBe(false);
});
});
26 changes: 17 additions & 9 deletions src/lib/onboard/gateway-http-readiness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
* (regression of #2020) for the original motivation.
*/

import fs from "node:fs";
import http from "node:http";
import http2 from "node:http2";
import fs from "node:fs";
import path from "node:path";

import { getGatewayHttpEndpoint, getGatewayHttpsEndpoint } from "../core/gateway-address";
Expand Down Expand Up @@ -76,16 +76,18 @@ export function isGatewayHttpReady(
timeoutMs = ISGATEWAY_HTTP_READY_DEFAULT_TIMEOUT_MS,
url = `${getGatewayHttpEndpoint(GATEWAY_PORT)}/`,
method: "GET" | "POST" = "GET",
signal?: AbortSignal,
): Promise<boolean> {
return withTraceSpan("nemoclaw.gateway.http_probe", { timeout_ms: timeoutMs, url, method }, () =>
isGatewayHttpReadyImpl(timeoutMs, url, method),
isGatewayHttpReadyImpl(timeoutMs, url, method, signal),
);
}

function isGatewayHttpReadyImpl(
timeoutMs = ISGATEWAY_HTTP_READY_DEFAULT_TIMEOUT_MS,
url = `${getGatewayHttpEndpoint(GATEWAY_PORT)}/`,
method: "GET" | "POST" = "GET",
signal?: AbortSignal,
): Promise<boolean> {
const effectiveTimeout =
Number.isFinite(timeoutMs) && timeoutMs > 0
Expand All @@ -98,13 +100,19 @@ function isGatewayHttpReadyImpl(
settled = true;
resolve(ready);
};
const request = http
.request(url, { method }, (res) => {
res.resume();
const code = res.statusCode || 0;
settle(GATEWAY_HTTP_ALIVE_CODES.has(code));
})
.on("error", () => settle(false));
let request: http.ClientRequest;
try {
request = http
.request(url, { method, signal }, (res) => {
res.resume();
const code = res.statusCode || 0;
settle(GATEWAY_HTTP_ALIVE_CODES.has(code));
})
.on("error", () => settle(false));
} catch {
settle(false);
return;
}
request.setTimeout(effectiveTimeout, () => {
request.destroy();
settle(false);
Expand Down