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
150 changes: 150 additions & 0 deletions src/lib/inference/vllm-serving-port.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ describe("managed vLLM serving-port guard (#8685)", () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const managed = vllmContainerRow(profile.containerName);
mockSuccessfulVllmInstall(mocks, profile.containerName, [() => managed, () => managed]);
mockDefaultDockerOwnership(managed);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
baseUrl: "http://127.0.0.1:8000",
apiKey: "b".repeat(64),
Expand Down Expand Up @@ -286,6 +287,10 @@ describe("managed vLLM serving-port guard (#8685)", () => {
() => vllmContainerRow(profile.containerName),
() => vllmContainerRow(profile.containerName, { id: "c".repeat(64) }),
]);
mockDefaultDockerOwnership(
vllmContainerRow(profile.containerName),
vllmContainerRow(profile.containerName, { id: "c".repeat(64) }),
);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue({
baseUrl: "http://127.0.0.1:8000",
apiKey: "b".repeat(64),
Expand Down Expand Up @@ -415,4 +420,149 @@ describe("managed vLLM serving-port guard (#8685)", () => {
expect(mocks.dockerRunDetached).toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).not.toContain("already in use");
});
it("adopts its interrupted container on the default Docker context (#11426)", async () => {
process.env.DOCKER_HOST = "ssh://remote-builder.example.test";
process.env.DOCKER_CONTEXT = "remote-builder";
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const managed = vllmContainerRow(profile.containerName, { state: "running" });
// Three responses: the ownership check that classifies the port holder, then
// the ordinary replacement guard's own inspection.
mockSuccessfulVllmInstall(mocks, profile.containerName, [
() => managed,
() => managed,
() => managed,
]);
mockDefaultDockerOwnership(managed);
// An interrupted install persists no runtime receipt, and a profile without
// managed bearer auth carries no auth label, so lifecycle recovery cannot
// admit the container this very install left behind.
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);
publishContainerBindings("0.0.0.0:8000", "[::]:8000");
const checkServingPort = vi.fn(async () => ({
ok: false,
reason: "port 8000 is held by docker-proxy (PID 4242)",
}));

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

expect(result).toEqual({ ok: true });
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");
const dockerOptions = [
...mocks.dockerImageInspectFormat.mock.calls.map((call) => call[2]),
...mocks.dockerPullWithProgressWatchdog.mock.calls.map((call) => call[1]),
...mocks.dockerSpawn.mock.calls.map((call) => call[1]),
...mocks.dockerForceRm.mock.calls.map((call) => call[1]),
...mocks.dockerRunDetached.mock.calls.map((call) => call[1]),
...mocks.dockerCapture.mock.calls.map((call) => call[1]),
];
expect(dockerOptions.length).toBeGreaterThan(0);
expect(new Set(dockerOptions.map((options) => options?.env?.DOCKER_CONTEXT))).toEqual(
new Set(["default"]),
);
expect(new Set(dockerOptions.map((options) => options?.env?.DOCKER_HOST))).toEqual(
new Set([undefined]),
);
});

it("still refuses when an unlabeled container holds the serving port", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const foreign = vllmContainerRow(profile.containerName, { label: "", state: "running" });
mockSuccessfulVllmInstall(mocks, profile.containerName, [() => foreign, () => foreign]);
mockDefaultDockerOwnership(foreign);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);

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.dockerForceRm).not.toHaveBeenCalled();
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).toContain("already in use");
});
/** Return each ownership row from the physical host's default Docker context. */
function mockDefaultDockerOwnership(...rows: string[]): void {
const base = mocks.dockerCapture.getMockImplementation();
let rowIndex = 0;
mocks.dockerCapture.mockImplementation(
(args: readonly string[], options?: { env?: NodeJS.ProcessEnv }) =>
args[0] === "container" && options?.env?.DOCKER_CONTEXT === "default"
? (rows[Math.min(rowIndex++, rows.length - 1)] ?? "")
: (base?.(args, options) ?? ""),
);
}

/** Report the host bindings the managed container publishes for container 8000. */
function publishContainerBindings(...bindings: string[]): void {
const base = mocks.dockerCapture.getMockImplementation();
mocks.dockerCapture.mockImplementation(
(args: readonly string[], options?: { env?: NodeJS.ProcessEnv }) =>
args[0] === "port" ? `${bindings.join("\n")}\n` : (base?.(args, options) ?? ""),
);
}

it("refuses a managed container published on a different host port", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const managed = vllmContainerRow(profile.containerName, { state: "running" });
mockSuccessfulVllmInstall(mocks, profile.containerName, [
() => managed,
() => managed,
() => managed,
]);
mockDefaultDockerOwnership(managed);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);
// Our managed container serves another port, so an unrelated process owns
// the port that failed. Removing this container would free nothing.
publishContainerBindings("0.0.0.0:19000");

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.dockerForceRm).not.toHaveBeenCalled();
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).toContain("already in use");
});

it("refuses a managed container bound away from the probed loopback address", async () => {
const profile = detectVllmProfile({ platform: "spark", type: "nvidia" })!;
const managed = vllmContainerRow(profile.containerName, { state: "running" });
mockSuccessfulVllmInstall(mocks, profile.containerName, [
() => managed,
() => managed,
() => managed,
]);
mockDefaultDockerOwnership(managed);
mocks.recoverHostLocalManagedVllmEndpoint.mockReturnValue(null);
publishContainerBindings("127.0.0.2:8000", "192.168.1.10:8000");

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.dockerForceRm).not.toHaveBeenCalled();
expect(mocks.dockerRunDetached).not.toHaveBeenCalled();
expect(errSpy.mock.calls.flat().join("\n")).toContain("already in use");
});
});
66 changes: 59 additions & 7 deletions src/lib/inference/vllm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ function startContainer(
// avoids deleting an unrelated same-name container if the name changes hands.
const replacement = vllmContainerReplacementTarget(
profile.containerName,
model.managedBearerAuth ? dockerEnv : undefined,
model.managedBearerAuth || expectedReplacementContainerId !== undefined ? dockerEnv : undefined,
expectedReplacementContainerId,
);
if (!replacement.ok) return replacement;
Expand Down Expand Up @@ -1949,6 +1949,46 @@ function applyRequestedVllmGpuDevice(
return requestedGpuDevice ? selectVllmGpuDevice(profile, requestedGpuDevice) : profile;
}

/**
* Container id of this install's own managed container when that container is
* what holds the serving port.
*
* Lifecycle recovery admits only a completed authenticated install: it needs
* the runtime receipt written after startup and the auth label that a profile
* adds only for managed bearer auth. An install interrupted before either
* exists leaves a running managed container that recovery can never claim, so
* ownership labels decide instead.
*
* Ownership alone is not enough. A managed container published on a different
* host port is not the process holding this port, and removing it would free
* nothing while destroying an unrelated runtime, so one published binding must
* cover the probed loopback address and match the port that failed. Every other
* state — a foreign or unlabeled holder, an ambiguous inspection, a distributed
* head or worker, a container that is not running, and an unreadable or
* address-mismatched binding — remains a conflict.
*/
function adoptableServingPortHolder(
containerName: string,
servingPort: number,
dockerEnv: Record<string, string>,
): string | undefined {
const ownership = inspectVllmContainerOwnershipInDockerEnv(containerName, dockerEnv);
if (ownership.kind !== "managed" || !ownership.running) return undefined;
// The managed container always publishes the fixed container port 8000.
const published = dockerCapture(["port", containerName, "8000"], {
env: dockerEnv,
ignoreError: true,
timeout: 10_000,
})
?.split(/\r?\n/u)
.some((binding) => {
const endpoint = binding.trim().match(/^(127[.]0[.]0[.]1|0[.]0[.]0[.]0):(\d+)$/u);
return endpoint !== null && Number(endpoint[2]) === servingPort;
});
if (!published) return undefined;
return ownership.containerId;
}

/**
* Name the process holding the serving port so the operator can act, matching
* how the Ollama auth proxy reports its own port conflict.
Expand Down Expand Up @@ -2315,6 +2355,7 @@ async function runVllmInstall(
// 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;
let recoveredHostLocalDockerEnv: Record<string, string> | undefined;
const servingPort = await opts.checkServingPort?.(VLLM_PORT);
if (servingPort && !servingPort.ok) {
// An interrupted host-local install can leave its authenticated managed
Expand All @@ -2323,14 +2364,24 @@ async function runVllmInstall(
// credential fingerprint. The replacement guard below then removes the
// inspected container ID immediately before the new launch.
try {
const hostLocalDockerEnv = buildLocalManagedVllmDockerEnv();
const recovered = recoverHostLocalManagedVllmEndpoint();
if (recovered?.baseUrl === `http://127.0.0.1:${String(VLLM_PORT)}`) {
recoveredHostLocalContainerId = recovered.containerId;
// Continue through the ordinary managed-container replacement path.
} else {
printServingPortConflict(servingPort);
return { ok: false };
const adopted = adoptableServingPortHolder(
runtimeProfile.containerName,
VLLM_PORT,
hostLocalDockerEnv,
);
if (adopted === undefined) {
printServingPortConflict(servingPort);
return { ok: false };
}
recoveredHostLocalContainerId = adopted;
}
recoveredHostLocalDockerEnv = hostLocalDockerEnv;
} catch (error) {
console.error(
` vLLM install failed: managed host-local vLLM recovery could not verify the container: ${(error as Error).message}`,
Expand Down Expand Up @@ -2373,9 +2424,8 @@ async function runVllmInstall(
let hostLocalApiKey: string | null = null;
let localDockerEnv = dualStationPlan
? buildLocalDualStationDockerEnv()
: model.managedBearerAuth
? buildLocalManagedVllmDockerEnv()
: buildVllmDockerEnv();
: (recoveredHostLocalDockerEnv ??
(model.managedBearerAuth ? buildLocalManagedVllmDockerEnv() : buildVllmDockerEnv()));
let gpuMemoryWarningShown = false;
const reportGpuMemoryWarning = (result: GpuMemoryPreflightResult): void => {
if (!result.ok || !result.warning || gpuMemoryWarningShown) return;
Expand Down Expand Up @@ -2426,7 +2476,9 @@ async function runVllmInstall(
} else {
const replacement = vllmContainerReplacementTarget(
runtimeProfile.containerName,
model.managedBearerAuth ? localDockerEnv : undefined,
model.managedBearerAuth || recoveredHostLocalContainerId !== undefined
? localDockerEnv
: undefined,
recoveredHostLocalContainerId,
);
if (!replacement.ok) {
Expand Down
Loading