Skip to content

Commit a74e1fc

Browse files
committed
fix(inference): pin recovered vLLM container identity
Signed-off-by: prekshivyas <prekshiv@nvidia.com>
1 parent 6b068bf commit a74e1fc

4 files changed

Lines changed: 65 additions & 6 deletions

File tree

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

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

118122
expect(capture).toHaveBeenCalledOnce();
119123
const dockerOptions = capture.mock.calls[0]?.[1];
@@ -132,7 +136,11 @@ describe("host-local managed vLLM recovery", () => {
132136
loadApiKey: () => API_KEY,
133137
onManagedContainerObserved: observed,
134138
}),
135-
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
139+
).toEqual({
140+
baseUrl: "http://127.0.0.1:8000",
141+
apiKey: API_KEY,
142+
containerId: "a".repeat(64),
143+
});
136144
expect(observed).toHaveBeenCalledOnce();
137145
});
138146

@@ -171,7 +179,11 @@ describe("host-local managed vLLM recovery", () => {
171179
loadApiKey: () => API_KEY,
172180
stateDir: directory,
173181
}),
174-
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
182+
).toEqual({
183+
baseUrl: "http://127.0.0.1:8000",
184+
apiKey: API_KEY,
185+
containerId: "a".repeat(64),
186+
});
175187
});
176188

177189
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 = (
@@ -364,5 +364,9 @@ export function recoverHostLocalManagedVllmEndpoint(
364364
) {
365365
throw new Error("Managed host-local vLLM authentication is missing or mismatched.");
366366
}
367-
return { baseUrl: `http://127.0.0.1:${String(HOST_LOCAL_VLLM_PORT)}`, apiKey };
367+
return {
368+
baseUrl: `http://127.0.0.1:${String(HOST_LOCAL_VLLM_PORT)}`,
369+
apiKey,
370+
containerId: row.Id,
371+
};
368372
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ describe("managed vLLM serving-port guard (#8685)", () => {
130130
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
131131
baseUrl: "http://127.0.0.1:8000",
132132
apiKey: "b".repeat(64),
133+
containerId: MANAGED_CONTAINER_ID,
133134
});
134135
const checkServingPort = vi.fn(async () => ({ ok: false, reason: "port 8000 is held" }));
135136

@@ -151,6 +152,31 @@ describe("managed vLLM serving-port guard (#8685)", () => {
151152
expect(errSpy.mock.calls.flat().join("\n")).not.toContain("another process");
152153
});
153154

155+
it("fails closed when the managed container changes after recovery", async () => {
156+
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
157+
mockSuccessfulVllmInstall(mocks, profile.containerName, [
158+
() => vllmContainerRow(profile.containerName, { id: "c".repeat(64) }),
159+
]);
160+
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
161+
baseUrl: "http://127.0.0.1:8000",
162+
apiKey: "b".repeat(64),
163+
containerId: MANAGED_CONTAINER_ID,
164+
});
165+
166+
const result = await installVllm(profile, {
167+
hasImage: true,
168+
nonInteractive: true,
169+
promptFn: vi.fn<(q: string) => Promise<string>>(),
170+
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
171+
});
172+
173+
expect(result).toEqual({ ok: false });
174+
expect(mocks.dockerPullWithProgressWatchdog).not.toHaveBeenCalled();
175+
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
176+
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
177+
expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("changed after recovery"));
178+
});
179+
154180
it("fails closed when a managed container holding the serving port cannot be recovered", async () => {
155181
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
156182
mockSuccessfulVllmInstall(mocks, profile.containerName);

src/lib/inference/vllm.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,7 @@ function inspectVllmContainerOwnership(containerName: string): VllmContainerOwne
738738
function vllmContainerReplacementTarget(
739739
containerName: string,
740740
dockerEnv?: Record<string, string>,
741+
expectedContainerId?: string,
741742
): { ok: true; containerId?: string } | { ok: false; reason: string } {
742743
const ownership = dockerEnv
743744
? inspectVllmContainerOwnershipInDockerEnv(containerName, dockerEnv)
@@ -762,6 +763,15 @@ function vllmContainerReplacementTarget(
762763
`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.`,
763764
};
764765
}
766+
if (
767+
expectedContainerId &&
768+
(ownership.kind !== "managed" || ownership.containerId !== expectedContainerId)
769+
) {
770+
return {
771+
ok: false,
772+
reason: `Managed vLLM container "${containerName}" changed after recovery. NemoClaw will not remove it. Retry onboarding.`,
773+
};
774+
}
765775
return ownership.kind === "managed"
766776
? { ok: true, containerId: ownership.containerId }
767777
: { ok: true };
@@ -868,6 +878,7 @@ function startContainer(
868878
dockerEnv: Record<string, string> = buildVllmDockerEnv(),
869879
resolveBridgeHost: (dockerEnv: Record<string, string>) => string = (env) =>
870880
resolveManagedVllmBridgeHost(dockerCapture, env),
881+
expectedReplacementContainerId?: string,
871882
): { ok: true; containerId: string } | { ok: false; reason: string } {
872883
emit(`Starting vLLM container (${profile.containerName})`);
873884
// The explicit download completed before this long-lived container starts,
@@ -892,6 +903,7 @@ function startContainer(
892903
const replacement = vllmContainerReplacementTarget(
893904
profile.containerName,
894905
model.managedBearerAuth ? dockerEnv : undefined,
906+
expectedReplacementContainerId,
895907
);
896908
if (!replacement.ok) return replacement;
897909
if (replacement.containerId) {
@@ -1735,6 +1747,7 @@ async function runVllmInstall(
17351747
// the guard first keeps a refused install free of both side effects.
17361748
// Port 25000 is not checked here: it belongs to the managed-cluster
17371749
// rendezvous contract and this single-node path never binds it.
1750+
let recoveredHostLocalContainerId: string | undefined;
17381751
const servingPort = await opts.checkServingPort?.(VLLM_PORT);
17391752
if (servingPort && !servingPort.ok) {
17401753
// An interrupted host-local install can leave its authenticated managed
@@ -1743,7 +1756,9 @@ async function runVllmInstall(
17431756
// credential fingerprint. The replacement guard below then removes the
17441757
// inspected container ID immediately before the new launch.
17451758
try {
1746-
if (recoverHostLocalManagedVllmEndpoint()) {
1759+
const recovered = recoverHostLocalManagedVllmEndpoint();
1760+
if (recovered) {
1761+
recoveredHostLocalContainerId = recovered.containerId;
17471762
// Continue through the ordinary managed-container replacement path.
17481763
} else {
17491764
printServingPortConflict(servingPort);
@@ -1830,6 +1845,7 @@ async function runVllmInstall(
18301845
const replacement = vllmContainerReplacementTarget(
18311846
runtimeProfile.containerName,
18321847
model.managedBearerAuth ? localDockerEnv : undefined,
1848+
recoveredHostLocalContainerId,
18331849
);
18341850
if (!replacement.ok) {
18351851
console.error(` vLLM install failed: ${replacement.reason}`);
@@ -2074,6 +2090,7 @@ async function runVllmInstall(
20742090
model,
20752091
localDockerEnv,
20762092
opts.resolveManagedBridgeHost,
2093+
recoveredHostLocalContainerId,
20772094
);
20782095
if (!start.ok) {
20792096
console.error(` vLLM install failed: ${String(start.reason)}`);

0 commit comments

Comments
 (0)