Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
18 changes: 15 additions & 3 deletions src/lib/inference/serving/vllm-host-local-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ describe("host-local managed vLLM recovery", () => {
dockerCapture: capture,
loadApiKey: () => API_KEY,
}),
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
).toEqual({
baseUrl: "http://127.0.0.1:8000",
apiKey: API_KEY,
containerId: "a".repeat(64),
});

expect(capture).toHaveBeenCalledOnce();
const dockerOptions = capture.mock.calls[0]?.[1];
Expand All @@ -133,7 +137,11 @@ describe("host-local managed vLLM recovery", () => {
loadApiKey: () => API_KEY,
onManagedContainerObserved: observed,
}),
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
).toEqual({
baseUrl: "http://127.0.0.1:8000",
apiKey: API_KEY,
containerId: "a".repeat(64),
});
expect(observed).toHaveBeenCalledOnce();
});

Expand Down Expand Up @@ -194,7 +202,11 @@ describe("host-local managed vLLM recovery", () => {
loadApiKey: () => API_KEY,
stateDir: directory,
}),
).toEqual({ baseUrl: "http://127.0.0.1:8000", apiKey: API_KEY });
).toEqual({
baseUrl: "http://127.0.0.1:8000",
apiKey: API_KEY,
containerId: "a".repeat(64),
});
});

it("rejects a profile-labeled runtime when its ownership receipt is missing", () => {
Expand Down
8 changes: 6 additions & 2 deletions src/lib/inference/serving/vllm-host-local-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ function inspectHostLocalContainer(
/** Recover only the exact authenticated host-local container with bounded host bindings. */
export function recoverHostLocalManagedVllmEndpoint(
options: RecoverHostLocalManagedVllmOptions = {},
): { baseUrl: string; apiKey: string } | null {
): { baseUrl: string; apiKey: string; containerId: string } | null {
const capture = options.dockerCapture ?? dockerCapture;
const dockerEnv = buildLocalManagedVllmDockerEnv();
const source = (
Expand Down Expand Up @@ -380,5 +380,9 @@ export function recoverHostLocalManagedVllmEndpoint(
) {
throw new Error("Managed host-local vLLM authentication is missing or mismatched.");
}
return { baseUrl: `http://127.0.0.1:${String(loopbackHostPort)}`, apiKey };
return {
baseUrl: `http://127.0.0.1:${String(loopbackHostPort)}`,
apiKey,
containerId: row.Id,
};
}
82 changes: 82 additions & 0 deletions src/lib/inference/vllm-serving-port.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const mocks = vi.hoisted(() => ({
measureDirectorySizeBytes: vi.fn(),
probeDockerStorage: vi.fn(),
probeHostStorage: vi.fn(),
recoverHostLocalManagedVllmEndpoint: vi.fn(),
runCapture: vi.fn(),
tryInstallManagedClusterManagedVllm: vi.fn(async () => ({ kind: "not-selected" as const })),
}));
Expand Down Expand Up @@ -59,6 +60,7 @@ vi.mock("./serving/vllm-managed-support", async (importOriginal) => {
return {
...actual,
ensureDualStationVllmApiKey: mocks.ensureDualStationVllmApiKey,
recoverHostLocalManagedVllmEndpoint: mocks.recoverHostLocalManagedVllmEndpoint,
tryInstallManagedClusterManagedVllm: mocks.tryInstallManagedClusterManagedVllm,
};
});
Expand All @@ -72,9 +74,11 @@ import {
import {
applyVllmInstallProbeDefaults,
createVllmInstallSpies,
MANAGED_CONTAINER_ID,
mockSuccessfulVllmInstall,
resetVllmInstallEnv,
type VllmInstallSpies,
vllmContainerRow,
withVllmInstallTestReadiness,
} from "./vllm-install.test-support";

Expand All @@ -91,6 +95,7 @@ describe("managed vLLM serving-port guard (#8685)", () => {
vi.clearAllMocks();
applyVllmInstallProbeDefaults(mocks);
mocks.getGpuIndicesByName.mockReturnValue([0]);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);
mocks.tryInstallManagedClusterManagedVllm.mockResolvedValue({ kind: "not-selected" });
({ errSpy, restore: restoreSpies } = createVllmInstallSpies());
resetVllmInstallEnv();
Expand Down Expand Up @@ -128,6 +133,83 @@ describe("managed vLLM serving-port guard (#8685)", () => {
expect(reported).not.toContain("exit 125");
});

it("replaces a validated interrupted managed container that holds the serving port", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const managed = vllmContainerRow(profile.containerName);
mockSuccessfulVllmInstall(mocks, profile.containerName, [() => managed, () => managed]);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
baseUrl: "http://127.0.0.1:8000",
apiKey: "b".repeat(64),
containerId: MANAGED_CONTAINER_ID,
});
const checkServingPort = vi.fn(async () => ({ ok: false, reason: "port 8000 is held" }));

const result = await installVllm(profile, {
hasImage: true,
nonInteractive: true,
promptFn: vi.fn<(q: string) => Promise<string>>(),
checkServingPort,
});

expect(result).toEqual({ ok: true });
expect(checkServingPort).toHaveBeenCalledWith(8000);
expect(mocks.recoverHostLocalManagedVllmEndpoint).toHaveBeenCalledOnce();
expect(mocks.dockerForceRm).toHaveBeenCalledWith(
MANAGED_CONTAINER_ID,
expect.objectContaining({ ignoreError: true, suppressOutput: true }),
);
expect(mocks.dockerRunDetached).toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).not.toContain("another process");
});

it("fails closed when the managed container changes after recovery", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
mockSuccessfulVllmInstall(mocks, profile.containerName, [
() => vllmContainerRow(profile.containerName, { id: "c".repeat(64) }),
]);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
baseUrl: "http://127.0.0.1:8000",
apiKey: "b".repeat(64),
containerId: MANAGED_CONTAINER_ID,
});

const result = await installVllm(profile, {
hasImage: true,
nonInteractive: true,
promptFn: vi.fn<(q: string) => Promise<string>>(),
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
});

expect(result).toEqual({ ok: false });
expect(mocks.dockerPullWithProgressWatchdog).not.toHaveBeenCalled();
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
expect(errSpy).toHaveBeenCalledWith(expect.stringContaining("changed after recovery"));
});

it("fails closed when a managed container holding the serving port cannot be recovered", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
mockSuccessfulVllmInstall(mocks, profile.containerName);
mocks.recoverHostLocalManagedVllmEndpoint.mockImplementation(() => {
throw new Error("Managed host-local vLLM runtime does not match its ownership receipt.");
});

const result = await installVllm(profile, {
hasImage: true,
nonInteractive: true,
promptFn: vi.fn<(q: string) => Promise<string>>(),
checkServingPort: async () => ({ ok: false, reason: "port 8000 is held" }),
});

expect(result).toEqual({ ok: false });
expect(mocks.dockerPullWithProgressWatchdog).not.toHaveBeenCalled();
expect(mocks.dockerForceRm).not.toHaveBeenCalled();
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).toContain(
"managed host-local vLLM recovery could not verify the container",
);
});

it("rejects the port before the storage decisions or the cache directory", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
mockSuccessfulVllmInstall(mocks, profile.containerName);
Expand Down
37 changes: 35 additions & 2 deletions src/lib/inference/vllm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ function inspectVllmContainerOwnership(containerName: string): VllmContainerOwne
function vllmContainerReplacementTarget(
containerName: string,
dockerEnv?: Record<string, string>,
expectedContainerId?: string,
): { ok: true; containerId?: string } | { ok: false; reason: string } {
const ownership = dockerEnv
? inspectVllmContainerOwnershipInDockerEnv(containerName, dockerEnv)
Expand All @@ -851,6 +852,15 @@ function vllmContainerReplacementTarget(
`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.`,
};
}
if (
expectedContainerId &&
(ownership.kind !== "managed" || ownership.containerId !== expectedContainerId)
) {
return {
ok: false,
reason: `Managed vLLM container "${containerName}" changed after recovery. NemoClaw will not remove it. Retry onboarding.`,
};
}
return ownership.kind === "managed"
? { ok: true, containerId: ownership.containerId }
: { ok: true };
Expand Down Expand Up @@ -957,6 +967,7 @@ function startContainer(
dockerEnv: Record<string, string> = buildVllmDockerEnv(),
resolveBridgeHost: (dockerEnv: Record<string, string>) => string = (env) =>
resolveManagedVllmBridgeHost(dockerCapture, env),
expectedReplacementContainerId?: string,
): { ok: true; containerId: string } | { ok: false; reason: string } {
emit(`Starting vLLM container (${profile.containerName})`);
// The explicit download completed before this long-lived container starts,
Expand Down Expand Up @@ -987,6 +998,7 @@ function startContainer(
const replacement = vllmContainerReplacementTarget(
profile.containerName,
model.managedBearerAuth ? dockerEnv : undefined,
expectedReplacementContainerId,
);
if (!replacement.ok) return replacement;
if (replacement.containerId) {
Expand Down Expand Up @@ -1873,10 +1885,29 @@ async function runVllmInstall(
// the guard first keeps a refused install free of both side effects.
// Port 25000 is not checked here: it belongs to the managed-cluster
// rendezvous contract and this single-node path never binds it.
let recoveredHostLocalContainerId: string | undefined;
const servingPort = await opts.checkServingPort?.(VLLM_PORT);
if (servingPort && !servingPort.ok) {
printServingPortConflict(servingPort);
return { ok: false };
// An interrupted host-local install can leave its authenticated managed
// container holding the fixed port. Admit that state only after the
// lifecycle recovery check validates the exact receipt, bindings, and
// credential fingerprint. The replacement guard below then removes the
// inspected container ID immediately before the new launch.
try {
const recovered = recoverHostLocalManagedVllmEndpoint();
if (recovered) {
recoveredHostLocalContainerId = recovered.containerId;
// Continue through the ordinary managed-container replacement path.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
printServingPortConflict(servingPort);
return { ok: false };
}
} catch (error) {
console.error(
` vLLM install failed: managed host-local vLLM recovery could not verify the container: ${(error as Error).message}`,
);
return { ok: false };
}
}

let hostLocalApiKey: string | null = null;
Expand Down Expand Up @@ -1956,6 +1987,7 @@ async function runVllmInstall(
const replacement = vllmContainerReplacementTarget(
runtimeProfile.containerName,
model.managedBearerAuth ? localDockerEnv : undefined,
recoveredHostLocalContainerId,
);
if (!replacement.ok) {
console.error(` vLLM install failed: ${replacement.reason}`);
Expand Down Expand Up @@ -2200,6 +2232,7 @@ async function runVllmInstall(
model,
localDockerEnv,
opts.resolveManagedBridgeHost,
recoveredHostLocalContainerId,
);
if (!start.ok) {
console.error(` vLLM install failed: ${String(start.reason)}`);
Expand Down