diff --git a/src/lib/adapters/docker/container.ts b/src/lib/adapters/docker/container.ts index afe8d71ec3e..0a949e920e7 100644 --- a/src/lib/adapters/docker/container.ts +++ b/src/lib/adapters/docker/container.ts @@ -7,6 +7,27 @@ export function dockerStop(containerName: string, opts: DockerRunOptions = {}) { return dockerRun(["stop", containerName], opts); } +// NIM (and most Python services) write errors to stderr, so a stdout-only +// capture would silently lose the auth-error tail. Use dockerRun so we can +// read both streams from the SpawnResult and concatenate them. Bounded by a +// short timeout — callers (e.g. waitForNimHealth's fast-fail) rely on this +// not blocking when the Docker daemon is unresponsive. +const DOCKER_LOGS_DEFAULT_TIMEOUT_MS = 5000; + +export function dockerLogs( + containerName: string, + { tail = 30, timeout = DOCKER_LOGS_DEFAULT_TIMEOUT_MS }: { tail?: number; timeout?: number } = {}, +): string { + const result = dockerRun(["logs", "--tail", String(tail), containerName], { + ignoreError: true, + suppressOutput: true, + timeout, + }); + const stdout = result?.stdout ? result.stdout.toString("utf-8") : ""; + const stderr = result?.stderr ? result.stderr.toString("utf-8") : ""; + return `${stdout}${stderr}`.trim(); +} + export function dockerRm(containerName: string, opts: DockerRunOptions = {}) { return dockerRun(["rm", containerName], opts); } diff --git a/src/lib/inference/nim.test.ts b/src/lib/inference/nim.test.ts index 3fbf9d76b78..11edd48e28c 100644 --- a/src/lib/inference/nim.test.ts +++ b/src/lib/inference/nim.test.ts @@ -27,18 +27,20 @@ function withFirmwareModel(model: string, fn: () => void): void { } } -function loadNimWithMockedRunner(runCapture: Mock) { +function loadNimWithMockedRunner(runCapture: Mock, run?: Mock) { const runner = require(RUNNER_PATH); const originalRun = runner.run; const originalRunCapture = runner.runCapture; delete require.cache[NIM_DIST_PATH]; - runner.run = vi.fn(); + const runMock = run ?? vi.fn(); + runner.run = runMock; runner.runCapture = runCapture; const nimModule = require(NIM_DIST_PATH); return { nimModule, + run: runMock, restore() { delete require.cache[NIM_DIST_PATH]; runner.run = originalRun; @@ -413,6 +415,186 @@ describe("nim", () => { restore(); } }); + + // Regression #3333: if the container exits (typically NGC auth failure), + // stop polling immediately and surface the last log lines so the user sees + // the cause instead of a generic 5-minute timeout. NIM's Python logger + // writes errors to stderr, so the dockerLogs adapter must capture both + // streams or the tail will be empty in the real failure mode. + it("short-circuits when the container has exited and surfaces stderr", () => { + const errorMsg = "Console output from stdout"; + const stderrMsg = "ERROR Authentication Error\nShutting down services..."; + const consoleErrors: string[] = []; + const origError = console.error; + console.error = (...args: unknown[]) => { + consoleErrors.push(args.map(String).join(" ")); + }; + const runCapture = vi.fn((cmd: string | string[]) => { + if (!Array.isArray(cmd)) throw new Error("expected argv array"); + if (cmd[0] === "curl") return ""; + if (cmd[0] === "docker" && cmd.includes("inspect")) return "exited"; + return ""; + }); + const run = vi.fn((cmd: string[]) => { + if (cmd[0] === "docker" && cmd.includes("logs")) { + return { stdout: Buffer.from(errorMsg), stderr: Buffer.from(stderrMsg), status: 0 }; + } + return { stdout: Buffer.from(""), stderr: Buffer.from(""), status: 0 }; + }); + const { nimModule, restore } = loadNimWithMockedRunner(runCapture, run); + + try { + const started = Date.now(); + expect(nimModule.waitForNimHealth(9000, 60, { container: "nemoclaw-nim-test" })).toBe( + false, + ); + expect(Date.now() - started).toBeLessThan(15_000); + const inspectCalls = runCapture.mock.calls.map(([c]: [string | string[]]) => c); + const runCalls = run.mock.calls.map(([c]: [string | string[]]) => c); + expect( + inspectCalls.some( + (c) => + c[0] === "docker" && c.includes("inspect") && c.includes("nemoclaw-nim-test"), + ), + ).toBe(true); + expect( + runCalls.some( + (c) => c[0] === "docker" && c.includes("logs") && c.includes("nemoclaw-nim-test"), + ), + ).toBe(true); + // The stderr line is the actual NIM auth error in production; this + // assertion guards against losing it on a future runCapture refactor. + expect(consoleErrors.some((line) => line.includes("Authentication Error"))).toBe(true); + } finally { + console.error = origError; + restore(); + } + }); + }); + + describe("startNimContainerByName", () => { + // Regression #3333 (and original fix in #219 that was lost in the + // string→argv refactor): the NIM container must receive NGC_API_KEY and + // NIM_NGC_API_KEY so it can download model manifests from NGC. Without + // these the container exits 0 a few seconds in with "Authentication Error". + // The value is passed through the spawn env (not argv) so it does not + // leak via `ps`/audit logs. + type RunCall = [string[], { env?: Record } | undefined]; + + function dockerRunCall(run: Mock): RunCall | undefined { + const found = run.mock.calls.find((c) => { + const argv = c[0] as string[]; + return Array.isArray(argv) && argv[0] === "docker" && argv[1] === "run"; + }); + return found as RunCall | undefined; + } + + function hasEnvFlag(argv: string[], envName: string): boolean { + for (let i = 0; i < argv.length - 1; i++) { + if (argv[i] === "-e" && argv[i + 1] === envName) return true; + } + return false; + } + + function argvContainsValue(argv: string[], value: string): boolean { + return argv.some((a) => a.includes(value)); + } + + it("passes NGC_API_KEY and NIM_NGC_API_KEY through spawn env, not argv", () => { + const run = vi.fn(); + const { nimModule, restore } = loadNimWithMockedRunner(vi.fn(() => ""), run); + try { + nimModule.startNimContainerByName( + "nemoclaw-nim-test", + "nvidia/nemotron-3-nano-30b-a3b", + 8000, + { ngcApiKey: "nvapi-abc123" }, + ); + const call = dockerRunCall(run); + expect(call).toBeDefined(); + const [argv, opts] = call!; + expect(hasEnvFlag(argv, "NGC_API_KEY")).toBe(true); + expect(hasEnvFlag(argv, "NIM_NGC_API_KEY")).toBe(true); + // Secret must not appear in argv (visible via ps/audit logs). + expect(argvContainsValue(argv, "nvapi-abc123")).toBe(false); + expect(opts?.env).toMatchObject({ + NGC_API_KEY: "nvapi-abc123", + NIM_NGC_API_KEY: "nvapi-abc123", + }); + } finally { + restore(); + } + }); + + it("falls back to process.env.NGC_API_KEY when no opts key is supplied", () => { + const prev = { ngc: process.env.NGC_API_KEY, nv: process.env.NVIDIA_API_KEY }; + process.env.NGC_API_KEY = "nvapi-env-ngc"; + delete process.env.NVIDIA_API_KEY; + const run = vi.fn(); + const { nimModule, restore } = loadNimWithMockedRunner(vi.fn(() => ""), run); + try { + nimModule.startNimContainerByName( + "nemoclaw-nim-test", + "nvidia/nemotron-3-nano-30b-a3b", + 8000, + ); + const call = dockerRunCall(run); + expect(call?.[1]?.env).toMatchObject({ + NGC_API_KEY: "nvapi-env-ngc", + NIM_NGC_API_KEY: "nvapi-env-ngc", + }); + } finally { + restore(); + if (prev.ngc === undefined) delete process.env.NGC_API_KEY; + else process.env.NGC_API_KEY = prev.ngc; + if (prev.nv !== undefined) process.env.NVIDIA_API_KEY = prev.nv; + } + }); + + it("falls back to process.env.NVIDIA_API_KEY when NGC_API_KEY is unset", () => { + const prev = { ngc: process.env.NGC_API_KEY, nv: process.env.NVIDIA_API_KEY }; + delete process.env.NGC_API_KEY; + process.env.NVIDIA_API_KEY = "nvapi-env-nvidia"; + const run = vi.fn(); + const { nimModule, restore } = loadNimWithMockedRunner(vi.fn(() => ""), run); + try { + nimModule.startNimContainerByName( + "nemoclaw-nim-test", + "nvidia/nemotron-3-nano-30b-a3b", + 8000, + ); + const call = dockerRunCall(run); + expect(call?.[1]?.env?.NGC_API_KEY).toBe("nvapi-env-nvidia"); + } finally { + restore(); + if (prev.ngc !== undefined) process.env.NGC_API_KEY = prev.ngc; + if (prev.nv === undefined) delete process.env.NVIDIA_API_KEY; + else process.env.NVIDIA_API_KEY = prev.nv; + } + }); + + it("omits env flags when no key is available", () => { + const prev = { ngc: process.env.NGC_API_KEY, nv: process.env.NVIDIA_API_KEY }; + delete process.env.NGC_API_KEY; + delete process.env.NVIDIA_API_KEY; + const run = vi.fn(); + const { nimModule, restore } = loadNimWithMockedRunner(vi.fn(() => ""), run); + try { + nimModule.startNimContainerByName( + "nemoclaw-nim-test", + "nvidia/nemotron-3-nano-30b-a3b", + 8000, + ); + const call = dockerRunCall(run); + expect(hasEnvFlag(call![0], "NGC_API_KEY")).toBe(false); + expect(hasEnvFlag(call![0], "NIM_NGC_API_KEY")).toBe(false); + expect(call?.[1]?.env).toBeUndefined(); + } finally { + restore(); + if (prev.ngc !== undefined) process.env.NGC_API_KEY = prev.ngc; + if (prev.nv !== undefined) process.env.NVIDIA_API_KEY = prev.nv; + } + }); }); describe("nimStatusByName", () => { diff --git a/src/lib/inference/nim.ts b/src/lib/inference/nim.ts index 429fde23f36..d309b134c73 100644 --- a/src/lib/inference/nim.ts +++ b/src/lib/inference/nim.ts @@ -9,6 +9,7 @@ const { dockerContainerInspectFormat, dockerForceRm, dockerLoginPasswordStdin, + dockerLogs, dockerPort, dockerPull, dockerRm, @@ -326,39 +327,86 @@ export function pullNimImage(model: string): string { return image; } -export function startNimContainer(sandboxName: string, model: string, port = VLLM_PORT): string { +export interface NimStartOptions { + ngcApiKey?: string; +} + +export function startNimContainer( + sandboxName: string, + model: string, + port = VLLM_PORT, + opts: NimStartOptions = {}, +): string { const name = containerName(sandboxName); - return startNimContainerByName(name, model, port); + return startNimContainerByName(name, model, port, opts); } -export function startNimContainerByName(name: string, model: string, port = VLLM_PORT): string { +export function startNimContainerByName( + name: string, + model: string, + port = VLLM_PORT, + opts: NimStartOptions = {}, +): string { const image = getImageForModel(model); if (!image) { console.error(` Unknown model: ${model}`); process.exit(1); } + // Resolve the NGC key: explicit arg wins, then NGC_API_KEY, then NVIDIA_API_KEY + // (covers users who only set the NVIDIA key for cloud inference but reuse it + // against NGC). Without this, NIM's in-container model-manifest download + // returns "Authentication Error" and the container exits 0 a few seconds in. + // Regression of #210 — see #3333. + const ngcApiKey = opts.ngcApiKey ?? process.env.NGC_API_KEY ?? process.env.NVIDIA_API_KEY ?? ""; + // Use `-e KEY` (no value) so the secret never appears in argv; pass the + // value through the spawn env instead. Docker reads each named var from + // its own process env and forwards it to the container. + const envFlags = ngcApiKey ? ["-e", "NGC_API_KEY", "-e", "NIM_NGC_API_KEY"] : []; + const runEnv = ngcApiKey + ? { NGC_API_KEY: ngcApiKey, NIM_NGC_API_KEY: ngcApiKey } + : undefined; + if (!ngcApiKey) { + console.warn( + " No NGC API key available; NIM will fail to download model weights. " + + "Set NGC_API_KEY or pass it through onboard.", + ); + } + dockerForceRm(name, { ignoreError: true }); console.log(` Starting NIM container: ${name}`); - dockerRunDetached([ - "--gpus", - "all", - "-p", - `${Number(port)}:8000`, - "--name", - name, - "--shm-size", - "16g", - image, - ]); + dockerRunDetached( + [ + "--gpus", + "all", + "-p", + `${Number(port)}:8000`, + "--name", + name, + "--shm-size", + "16g", + ...envFlags, + image, + ], + runEnv ? { env: runEnv } : {}, + ); return name; } -export function waitForNimHealth(port = VLLM_PORT, timeout = 300): boolean { +export interface WaitForNimHealthOptions { + container?: string; +} + +export function waitForNimHealth( + port = VLLM_PORT, + timeout = 300, + opts: WaitForNimHealthOptions = {}, +): boolean { const start = Date.now(); const intervalSec = 5; const hostPort = Number(port); + const { container } = opts; console.log(` Waiting for NIM health on port ${hostPort} (timeout: ${timeout}s)...`); while ((Date.now() - start) / 1000 < timeout) { @@ -382,6 +430,26 @@ export function waitForNimHealth(port = VLLM_PORT, timeout = 300): boolean { } catch { /* ignored */ } + // Short-circuit if the container has already exited — typically NGC auth + // failure or OOM during model load. Without this, the wizard polls the + // full timeout (default 300s) against a dead container. See #3333. + if (container) { + const state = dockerContainerInspectFormat("{{.State.Status}}", container, { + ignoreError: true, + timeout: NIM_STATUS_PROBE_TIMEOUT_MS, + }); + if (state && state !== "running" && state !== "created" && state !== "restarting") { + console.error(` NIM container ${container} is ${state}; aborting health wait.`); + const tail = dockerLogs(container, { tail: 30 }); + if (tail) { + console.error(" Last container output:"); + for (const line of tail.split("\n")) { + if (line) console.error(` ${line}`); + } + } + return false; + } + } sleepSeconds(intervalSec); } console.error(` NIM did not become healthy within ${timeout}s.`); diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 2985e19dafe..f537a19128e 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -6786,6 +6786,9 @@ async function setupNim( model = sel.name; // Ensure Docker is logged in to NGC registry before pulling NIM images. + // The key is also forwarded into the NIM container at runtime (#3333), + // so we hoist it out of the not-logged-in branch. + let ngcApiKey: string | null = null; if (!nim.isNgcLoggedIn()) { if (isNonInteractive()) { console.error( @@ -6813,16 +6816,39 @@ async function setupNim( process.exit(1); } } + ngcApiKey = ngcKey; + } else { + // Docker is already logged in, but NIM still needs the key in its + // container env to download model manifests. Users hit by the + // original #3333 bug typically have a cached docker login from + // the earlier broken attempt while the NGC key was never saved + // anywhere, so a passive lookup would silently reproduce the + // failure. Try env first, then prompt interactively; an empty + // answer falls through to startNimContainerByName's warning so + // we don't double-fail in non-interactive callers. + ngcApiKey = + hydrateCredentialEnv("NGC_API_KEY") || hydrateCredentialEnv("NVIDIA_API_KEY"); + if (!ngcApiKey && !isNonInteractive()) { + console.log(""); + console.log(" NGC API Key required to download NIM model weights at runtime."); + console.log(" (Docker is logged in to nvcr.io, but the key was not saved.)"); + ngcApiKey = normalizeCredentialValue( + await prompt(" NGC API Key: ", { secret: true }), + ); + } } console.log(` Pulling NIM image for ${model}...`); nim.pullNimImage(model); console.log(" Starting NIM container..."); - nimContainer = nim.startNimContainerByName(nim.containerName(GATEWAY_NAME), model); + const nimContainerNameLocal = nim.containerName(GATEWAY_NAME); + nimContainer = nim.startNimContainerByName(nimContainerNameLocal, model, undefined, { + ngcApiKey: ngcApiKey ?? undefined, + }); console.log(" Waiting for NIM to become healthy..."); - if (!nim.waitForNimHealth()) { + if (!nim.waitForNimHealth(undefined, undefined, { container: nimContainerNameLocal })) { console.error(" NIM failed to start. Falling back to cloud API."); model = null; nimContainer = null;