Skip to content

Commit 1434cf5

Browse files
prekshivyasgithub-actions[bot]senthilr-nv
authored
fix(inference): recover interrupted managed vLLM install (#9656)
## Summary Recover a validated managed vLLM container that remains after an interrupted install instead of reporting its fixed port as an unrelated process. Reject incomplete or mismatched ownership evidence before the installer can pull an image, remove a container, or launch a runtime. ## Related Issue Fixes #9582 ## Changes - Validate the existing host-local managed vLLM container through its receipt, bindings, serving identity, and API-key fingerprint before allowing the fixed-port guard to continue. - Reuse the existing ID-pinned replacement path after validation. The focused tests cover both replacement of the validated container and denial of invalid recovery evidence. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: self-review confirms receipt, container-ID, binding, serving-identity, and API-key-fingerprint validation remains required before replacement; the negative test proves invalid evidence does not trigger pull, removal, or launch. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project cli src/lib/inference/vllm.test.ts src/lib/inference/vllm-serving-port.test.ts` — 71 passed; `npm run typecheck:cli`; `npm run test:changed`. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: prekshivyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved installation recovery when the configured serving port is occupied by a managed vLLM runtime. - Validated interrupted managed runtimes can be replaced automatically after ownership and endpoint verification. - **Bug Fixes** - Prevented replacement when the recovered runtime changes or no longer matches the expected managed runtime. - Prevented recovery when runtime ownership, endpoint, or container validation fails. - Port conflicts and recovery failures now produce clear installation errors instead of proceeding unsafely. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: prekshivyas <prekshiv@nvidia.com> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
1 parent b251570 commit 1434cf5

4 files changed

Lines changed: 170 additions & 9 deletions

File tree

‎src/lib/inference/serving/vllm-host-local-lifecycle.test.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ describe("host-local managed vLLM recovery", () => {
114114
dockerCapture: capture,
115115
loadApiKey: () => API_KEY,
116116
}),
117-
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
117+
).toEqual({
118+
baseUrl: "http://127.0.0.1:8000",
119+
apiKey: API_KEY,
120+
containerId: "a".repeat(64),
121+
});
118122

119123
expect(capture).toHaveBeenCalledOnce();
120124
const dockerOptions = capture.mock.calls[0]?.[1];
@@ -133,17 +137,26 @@ describe("host-local managed vLLM recovery", () => {
133137
loadApiKey: () => API_KEY,
134138
onManagedContainerObserved: observed,
135139
}),
136-
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
140+
).toEqual({
141+
baseUrl: "http://127.0.0.1:8000",
142+
apiKey: API_KEY,
143+
containerId: "a".repeat(64),
144+
});
137145
expect(observed).toHaveBeenCalledOnce();
138146
});
139147

140148
it("recovers the exact configured host port from the bounded Docker bindings", () => {
141149
expect(
142150
recoverHostLocalManagedVllmEndpoint({
143-
dockerInspect: () => inspect(API_KEY, runtimeAuthFingerprint(API_KEY), {}, "172.18.0.1", "19000"),
151+
dockerInspect: () =>
152+
inspect(API_KEY, runtimeAuthFingerprint(API_KEY), {}, "172.18.0.1", "19000"),
144153
loadApiKey: () => API_KEY,
145154
}),
146-
).toEqual({ baseUrl: "http://127.0.0.1:19000", apiKey: API_KEY });
155+
).toEqual({
156+
baseUrl: "http://127.0.0.1:19000",
157+
apiKey: API_KEY,
158+
containerId: "a".repeat(64),
159+
});
147160
});
148161

149162
it.each(["80", "1e4", "019000", "65536"])(
@@ -194,7 +207,11 @@ describe("host-local managed vLLM recovery", () => {
194207
loadApiKey: () => API_KEY,
195208
stateDir: directory,
196209
}),
197-
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
210+
).toEqual({
211+
baseUrl: "http://127.0.0.1:8000",
212+
apiKey: API_KEY,
213+
containerId: "a".repeat(64),
214+
});
198215
});
199216

200217
it("rejects a profile-labeled runtime when its ownership receipt is missing", () => {

‎src/lib/inference/serving/vllm-host-local-lifecycle.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ function inspectHostLocalContainer(
276276
/** Recover only the exact authenticated host-local container with bounded host bindings. */
277277
export function recoverHostLocalManagedVllmEndpoint(
278278
options: RecoverHostLocalManagedVllmOptions = {},
279-
): { baseUrl: string; apiKey: string } | null {
279+
): { baseUrl: string; apiKey: string; containerId: string } | null {
280280
const capture = options.dockerCapture ?? dockerCapture;
281281
const dockerEnv = buildLocalManagedVllmDockerEnv();
282282
const source = (
@@ -380,5 +380,9 @@ export function recoverHostLocalManagedVllmEndpoint(
380380
) {
381381
throw new Error("Managed host-local vLLM authentication is missing or mismatched.");
382382
}
383-
return { baseUrl: `http://127.0.0.1:${String(loopbackHostPort)}`, apiKey };
383+
return {
384+
baseUrl: `http://127.0.0.1:${String(loopbackHostPort)}`,
385+
apiKey,
386+
containerId: row.Id,
387+
};
384388
}

‎src/lib/inference/vllm-serving-port.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const mocks = vi.hoisted(() => ({
2020
measureDirectorySizeBytes: vi.fn(),
2121
probeDockerStorage: vi.fn(),
2222
probeHostStorage: vi.fn(),
23+
recoverHostLocalManagedVllmEndpoint: vi.fn(),
2324
runCapture: vi.fn(),
2425
tryInstallManagedClusterManagedVllm: vi.fn(async () => ({ kind: "not-selected" as const })),
2526
}));
@@ -59,6 +60,7 @@ vi.mock("./serving/vllm-managed-support", async (importOriginal) => {
5960
return {
6061
...actual,
6162
ensureDualStationVllmApiKey: mocks.ensureDualStationVllmApiKey,
63+
recoverHostLocalManagedVllmEndpoint: mocks.recoverHostLocalManagedVllmEndpoint,
6264
tryInstallManagedClusterManagedVllm: mocks.tryInstallManagedClusterManagedVllm,
6365
};
6466
});
@@ -72,9 +74,11 @@ import {
7274
import {
7375
applyVllmInstallProbeDefaults,
7476
createVllmInstallSpies,
77+
MANAGED_CONTAINER_ID,
7578
mockSuccessfulVllmInstall,
7679
resetVllmInstallEnv,
7780
type VllmInstallSpies,
81+
vllmContainerRow,
7882
withVllmInstallTestReadiness,
7983
} from "./vllm-install.test-support";
8084

@@ -91,6 +95,7 @@ describe("managed vLLM serving-port guard (#8685)", () => {
9195
vi.clearAllMocks();
9296
applyVllmInstallProbeDefaults(mocks);
9397
mocks.getGpuIndicesByName.mockReturnValue([0]);
98+
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);
9499
mocks.tryInstallManagedClusterManagedVllm.mockResolvedValue({ kind: "not-selected" });
95100
({ errSpy, restore: restoreSpies } = createVllmInstallSpies());
96101
resetVllmInstallEnv();
@@ -128,6 +133,108 @@ describe("managed vLLM serving-port guard (#8685)", () => {
128133
expect(reported).not.toContain("exit 125");
129134
});
130135

136+
it("replaces a validated interrupted managed container that holds the serving port", async () => {
137+
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
138+
const managed = vllmContainerRow(profile.containerName);
139+
mockSuccessfulVllmInstall(mocks, profile.containerName, [() => managed, () => managed]);
140+
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
141+
baseUrl: "http://127.0.0.1:8000",
142+
apiKey: "b".repeat(64),
143+
containerId: MANAGED_CONTAINER_ID,
144+
});
145+
const checkServingPort = vi.fn(async () => ({ ok: false, reason: "port 8000 is held" }));
146+
147+
const result = await installVllm(profile, {
148+
hasImage: true,
149+
nonInteractive: true,
150+
promptFn: vi.fn<(q: string) => Promise<string>>(),
151+
checkServingPort,
152+
});
153+
154+
expect(result).toEqual({ ok: true });
155+
expect(checkServingPort).toHaveBeenCalledWith(8000);
156+
expect(mocks.recoverHostLocalManagedVllmEndpoint).toHaveBeenCalledOnce();
157+
expect(mocks.dockerForceRm).toHaveBeenCalledWith(
158+
MANAGED_CONTAINER_ID,
159+
expect.objectContaining({ ignoreError: true, suppressOutput: true }),
160+
);
161+
expect(mocks.dockerRunDetached).toHaveBeenCalled();
162+
expect(errSpy.mock.calls.flat().join("\n")).not.toContain("another process");
163+
});
164+
165+
it("rejects a recovered managed container bound to a different port", async () => {
166+
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
167+
mockSuccessfulVllmInstall(mocks, profile.containerName);
168+
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
169+
baseUrl: "http://127.0.0.1:19000",
170+
apiKey: "b".repeat(64),
171+
containerId: MANAGED_CONTAINER_ID,
172+
});
173+
174+
const result = await installVllm(profile, {
175+
hasImage: true,
176+
nonInteractive: true,
177+
promptFn: vi.fn<(q: string) => Promise<string>>(),
178+
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
179+
});
180+
181+
expect(result).toEqual({ ok: false });
182+
expect(mocks.recoverHostLocalManagedVllmEndpoint).toHaveBeenCalledOnce();
183+
expect(mocks.dockerPullWithProgressWatchdog).not.toHaveBeenCalled();
184+
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
185+
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
186+
expect(errSpy.mock.calls.flat().join("\n")).toContain("port 8000 is already in use");
187+
});
188+
189+
it("fails closed when the managed container changes immediately before replacement", async () => {
190+
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
191+
mockSuccessfulVllmInstall(mocks, profile.containerName, [
192+
() => vllmContainerRow(profile.containerName),
193+
() => vllmContainerRow(profile.containerName, { id: "c".repeat(64) }),
194+
]);
195+
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
196+
baseUrl: "http://127.0.0.1:8000",
197+
apiKey: "b".repeat(64),
198+
containerId: MANAGED_CONTAINER_ID,
199+
});
200+
201+
const result = await installVllm(profile, {
202+
hasImage: true,
203+
nonInteractive: true,
204+
promptFn: vi.fn<(q: string) => Promise<string>>(),
205+
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
206+
});
207+
208+
expect(result).toEqual({ ok: false });
209+
expect(mocks.dockerPullWithProgressWatchdog).toHaveBeenCalled();
210+
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
211+
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
212+
expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("changed after recovery"));
213+
});
214+
215+
it("fails closed when a managed container holding the serving port cannot be recovered", async () => {
216+
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
217+
mockSuccessfulVllmInstall(mocks, profile.containerName);
218+
mocks.recoverHostLocalManagedVllmEndpoint.mockImplementation(() => {
219+
throw new Error("Managed host-local vLLM runtime does not match its ownership receipt.");
220+
});
221+
222+
const result = await installVllm(profile, {
223+
hasImage: true,
224+
nonInteractive: true,
225+
promptFn: vi.fn<(q: string) => Promise<string>>(),
226+
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
227+
});
228+
229+
expect(result).toEqual({ ok: false });
230+
expect(mocks.dockerPullWithProgressWatchdog).not.toHaveBeenCalled();
231+
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
232+
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
233+
expect(errSpy.mock.calls.flat().join("\n")).toContain(
234+
"managed host-local vLLM recovery could not verify the container",
235+
);
236+
});
237+
131238
it("rejects the port before the storage decisions or the cache directory", async () => {
132239
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
133240
mockSuccessfulVllmInstall(mocks, profile.containerName);

‎src/lib/inference/vllm.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,7 @@ function inspectVllmContainerOwnership(containerName: string): VllmContainerOwne
827827
function vllmContainerReplacementTarget(
828828
containerName: string,
829829
dockerEnv?: Record<string, string>,
830+
expectedContainerId?: string,
830831
): { ok: true; containerId?: string } | { ok: false; reason: string } {
831832
const ownership = dockerEnv
832833
? inspectVllmContainerOwnershipInDockerEnv(containerName, dockerEnv)
@@ -851,6 +852,15 @@ function vllmContainerReplacementTarget(
851852
`Refusing single-host replacement because it would orphan the peer worker. Restore ${NEMOCLAW_DGX_STATION_PEER_ENV} and select Nemotron Ultra to manage the pair.`,
852853
};
853854
}
855+
if (
856+
expectedContainerId &&
857+
(ownership.kind !== "managed" || ownership.containerId !== expectedContainerId)
858+
) {
859+
return {
860+
ok: false,
861+
reason: `Managed vLLM container "${containerName}" changed after recovery. NemoClaw will not remove it. Retry onboarding.`,
862+
};
863+
}
854864
return ownership.kind === "managed"
855865
? { ok: true, containerId: ownership.containerId }
856866
: { ok: true };
@@ -957,6 +967,7 @@ function startContainer(
957967
dockerEnv: Record<string, string> = buildVllmDockerEnv(),
958968
resolveBridgeHost: (dockerEnv: Record<string, string>) => string = (env) =>
959969
resolveManagedVllmBridgeHost(dockerCapture, env),
970+
expectedReplacementContainerId?: string,
960971
): { ok: true; containerId: string } | { ok: false; reason: string } {
961972
emit(`Starting vLLM container (${profile.containerName})`);
962973
// The explicit download completed before this long-lived container starts,
@@ -987,6 +998,7 @@ function startContainer(
987998
const replacement = vllmContainerReplacementTarget(
988999
profile.containerName,
9891000
model.managedBearerAuth ? dockerEnv : undefined,
1001+
expectedReplacementContainerId,
9901002
);
9911003
if (!replacement.ok) return replacement;
9921004
if (replacement.containerId) {
@@ -1873,10 +1885,29 @@ async function runVllmInstall(
18731885
// the guard first keeps a refused install free of both side effects.
18741886
// Port 25000 is not checked here: it belongs to the managed-cluster
18751887
// rendezvous contract and this single-node path never binds it.
1888+
let recoveredHostLocalContainerId: string | undefined;
18761889
const servingPort = await opts.checkServingPort?.(VLLM_PORT);
18771890
if (servingPort && !servingPort.ok) {
1878-
printServingPortConflict(servingPort);
1879-
return { ok: false };
1891+
// An interrupted host-local install can leave its authenticated managed
1892+
// container holding the fixed port. Admit that state only after the
1893+
// lifecycle recovery check validates the exact receipt, bindings, and
1894+
// credential fingerprint. The replacement guard below then removes the
1895+
// inspected container ID immediately before the new launch.
1896+
try {
1897+
const recovered = recoverHostLocalManagedVllmEndpoint();
1898+
if (recovered?.baseUrl === `http://127.0.0.1:${String(VLLM_PORT)}`) {
1899+
recoveredHostLocalContainerId = recovered.containerId;
1900+
// Continue through the ordinary managed-container replacement path.
1901+
} else {
1902+
printServingPortConflict(servingPort);
1903+
return { ok: false };
1904+
}
1905+
} catch (error) {
1906+
console.error(
1907+
` vLLM install failed: managed host-local vLLM recovery could not verify the container: ${(error as Error).message}`,
1908+
);
1909+
return { ok: false };
1910+
}
18801911
}
18811912

18821913
let hostLocalApiKey: string | null = null;
@@ -1956,6 +1987,7 @@ async function runVllmInstall(
19561987
const replacement = vllmContainerReplacementTarget(
19571988
runtimeProfile.containerName,
19581989
model.managedBearerAuth ? localDockerEnv : undefined,
1990+
recoveredHostLocalContainerId,
19591991
);
19601992
if (!replacement.ok) {
19611993
console.error(` vLLM install failed: ${replacement.reason}`);
@@ -2200,6 +2232,7 @@ async function runVllmInstall(
22002232
model,
22012233
localDockerEnv,
22022234
opts.resolveManagedBridgeHost,
2235+
recoveredHostLocalContainerId,
22032236
);
22042237
if (!start.ok) {
22052238
console.error(` vLLM install failed: ${String(start.reason)}`);

0 commit comments

Comments
 (0)