From 391378af3a15e28707a1e73acb951891c782a2be Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Fri, 17 Jul 2026 21:31:35 -0400 Subject: [PATCH] fix(onboard): preserve structured mounts during recreation --- .../docker-gpu-patch-fixtures.ts | 12 ++ .../onboard/docker-gpu-patch-clone.test.ts | 24 +++- src/lib/onboard/docker-gpu-patch-clone.ts | 115 ++++++++++++++++++ src/lib/onboard/docker-gpu-patch-types.ts | 19 +++ .../docker-gpu-patch-validation.test.ts | 38 ++++++ 5 files changed, 207 insertions(+), 1 deletion(-) diff --git a/src/lib/onboard/__test-helpers__/docker-gpu-patch-fixtures.ts b/src/lib/onboard/__test-helpers__/docker-gpu-patch-fixtures.ts index 8c6566a5600..0c0f3f0a89a 100644 --- a/src/lib/onboard/__test-helpers__/docker-gpu-patch-fixtures.ts +++ b/src/lib/onboard/__test-helpers__/docker-gpu-patch-fixtures.ts @@ -43,6 +43,18 @@ export function createDockerGpuInspectFixture(): DockerContainerInspect { }, HostConfig: { Binds: ["/host:/container:rw"], + Mounts: [ + { + Type: "tmpfs", + Target: "/tmp/nemoclaw-exact-main-driver-config", + ReadOnly: false, + TmpfsOptions: { + Options: [["noexec"]], + SizeBytes: 16_777_216, + Mode: 0o1777, + }, + }, + ], NetworkMode: "openshell-docker", RestartPolicy: { Name: "unless-stopped" }, CapAdd: ["SYS_ADMIN", "NET_ADMIN"], diff --git a/src/lib/onboard/docker-gpu-patch-clone.test.ts b/src/lib/onboard/docker-gpu-patch-clone.test.ts index b49d77f970c..a8239863931 100644 --- a/src/lib/onboard/docker-gpu-patch-clone.test.ts +++ b/src/lib/onboard/docker-gpu-patch-clone.test.ts @@ -11,7 +11,7 @@ import { } from "./docker-gpu-patch"; describe("Docker GPU clone envelope", () => { - it("builds clone args that preserve OpenShell labels and runtime settings", () => { + it("builds clone args that preserve OpenShell labels, mounts, and runtime settings", () => { const args = buildDockerGpuCloneRunArgs(inspectFixture(), buildDockerGpuMode("gpus")); expect(args).toEqual( @@ -32,6 +32,8 @@ describe("Docker GPU clone envelope", () => { "openshell.ai/sandbox-name=alpha", "--volume", "/host:/container:rw", + "--tmpfs", + "/tmp/nemoclaw-exact-main-driver-config:noexec,size=16777216,mode=1777", "--network", "openshell-docker", "--network-alias", @@ -56,6 +58,26 @@ describe("Docker GPU clone envelope", () => { expect(args).not.toEqual(expect.arrayContaining(["--env", "NVIDIA_VISIBLE_DEVICES=void"])); }); + it("preserves OpenShell structured volume options", () => { + const inspect = inspectFixture(); + inspect.HostConfig!.Mounts!.push({ + Type: "volume", + Source: "sandbox-cache", + Target: "/sandbox/cache", + ReadOnly: true, + VolumeOptions: { NoCopy: true, Subpath: "project" }, + }); + + const args = buildDockerGpuCloneRunArgs(inspect, buildDockerGpuMode("startup-command")); + + expect(args).toEqual( + expect.arrayContaining([ + "--mount", + "type=volume,src=sandbox-cache,dst=/sandbox/cache,readonly,volume-nocopy,volume-subpath=project", + ]), + ); + }); + it("adds OpenShell's sandbox command env when the inspected container lacks one", () => { const inspect = inspectFixture(); inspect.Config!.Env = inspect.Config!.Env!.filter( diff --git a/src/lib/onboard/docker-gpu-patch-clone.ts b/src/lib/onboard/docker-gpu-patch-clone.ts index cb904068318..e6da25d1591 100644 --- a/src/lib/onboard/docker-gpu-patch-clone.ts +++ b/src/lib/onboard/docker-gpu-patch-clone.ts @@ -16,6 +16,9 @@ const GPU_ENV_KEYS = new Set([ "NVIDIA_REQUIRE_CUDA", "NVIDIA_DISABLE_REQUIRE", ]); +type DockerStructuredMount = NonNullable< + NonNullable["Mounts"] +>[number]; export const DOCKER_GPU_PATCH_NETWORK_ENV = "NEMOCLAW_DOCKER_GPU_PATCH_NETWORK"; @@ -115,6 +118,117 @@ function dockerUlimits( return [...merged.values()]; } +function mountValue(value: unknown, label: string): string { + if (typeof value !== "string" || value.length === 0 || value !== value.trim()) { + throw new Error(`Docker structured mount ${label} must be a non-empty trimmed string.`); + } + if (/[\0,:]/u.test(value)) { + throw new Error(`Docker structured mount ${label} contains an unsupported delimiter.`); + } + return value; +} + +function optionalMountBoolean(value: unknown, label: string): boolean { + if (value === undefined || value === null) return false; + if (typeof value !== "boolean") { + throw new Error(`Docker structured mount ${label} must be a boolean.`); + } + return value; +} + +function assertUnusedMountOption(value: unknown, label: string): void { + if (value !== undefined && value !== null) { + throw new Error(`Docker structured mount has unexpected ${label}.`); + } +} + +function dockerTmpfsMountValue(mount: DockerStructuredMount): string { + if (String(mount.Source ?? "") !== "") { + throw new Error("Docker tmpfs mount must not include a source."); + } + if (String(mount.Consistency ?? "") !== "") { + throw new Error("Docker tmpfs mount consistency is not supported during recreation."); + } + assertUnusedMountOption(mount.BindOptions, "BindOptions for a tmpfs mount"); + assertUnusedMountOption(mount.VolumeOptions, "VolumeOptions for a tmpfs mount"); + + const target = mountValue(mount.Target, "target"); + if (!target.startsWith("/")) { + throw new Error("Docker structured mount target must be an absolute container path."); + } + const options: string[] = []; + if (optionalMountBoolean(mount.ReadOnly, "ReadOnly")) options.push("ro"); + for (const parts of mount.TmpfsOptions?.Options ?? []) { + if (!Array.isArray(parts) || parts.length < 1 || parts.length > 2) { + throw new Error("Docker tmpfs mount options must contain one or two values."); + } + options.push(parts.map((part) => mountValue(part, "tmpfs option")).join("=")); + } + + const sizeBytes = mount.TmpfsOptions?.SizeBytes; + if (sizeBytes !== undefined && sizeBytes !== null) { + if (!Number.isSafeInteger(sizeBytes) || sizeBytes <= 0) { + throw new Error("Docker tmpfs mount size must be a positive safe integer."); + } + options.push(`size=${sizeBytes}`); + } + const mode = mount.TmpfsOptions?.Mode; + if (mode !== undefined && mode !== null) { + if (!Number.isSafeInteger(mode) || mode < 0 || mode > 0o7777) { + throw new Error("Docker tmpfs mount mode must be a valid non-negative file mode."); + } + options.push(`mode=${mode.toString(8)}`); + } + return options.length > 0 ? `${target}:${options.join(",")}` : target; +} + +function dockerVolumeMountValue(mount: DockerStructuredMount): string { + if (String(mount.Consistency ?? "") !== "") { + throw new Error("Docker volume mount consistency is not supported during recreation."); + } + assertUnusedMountOption(mount.BindOptions, "BindOptions for a volume mount"); + assertUnusedMountOption(mount.TmpfsOptions, "TmpfsOptions for a volume mount"); + + const source = mountValue(mount.Source, "volume source"); + const target = mountValue(mount.Target, "target"); + if (!target.startsWith("/")) { + throw new Error("Docker structured mount target must be an absolute container path."); + } + const values = [`type=volume`, `src=${source}`, `dst=${target}`]; + if (optionalMountBoolean(mount.ReadOnly, "ReadOnly")) values.push("readonly"); + const volumeOptions = mount.VolumeOptions; + if (optionalMountBoolean(volumeOptions?.NoCopy, "VolumeOptions.NoCopy")) { + values.push("volume-nocopy"); + } + if (volumeOptions?.Subpath) { + values.push(`volume-subpath=${mountValue(volumeOptions.Subpath, "volume subpath")}`); + } + if (volumeOptions?.Labels && Object.keys(volumeOptions.Labels).length > 0) { + throw new Error("Docker volume mount labels are not supported during recreation."); + } + assertUnusedMountOption(volumeOptions?.DriverConfig, "VolumeOptions.DriverConfig"); + return values.join(","); +} + +function dockerStructuredMountArgs(inspect: DockerContainerInspect): string[] { + const args: string[] = []; + for (const mount of inspect.HostConfig?.Mounts ?? []) { + switch (mount.Type) { + case "tmpfs": + // The --tmpfs form preserves OpenShell's arbitrary TmpfsOptions.Options, + // such as noexec, in addition to the structured size and mode fields. + args.push("--tmpfs", dockerTmpfsMountValue(mount)); + break; + case "volume": + args.push("--mount", dockerVolumeMountValue(mount)); + break; + default: + throw new Error(`Unsupported Docker structured mount type '${String(mount.Type)}'.`); + } + } + return args; +} + function pushNumberFlag(args: string[], flag: string, value: unknown): void { if (typeof value === "number" && Number.isFinite(value) && value > 0) { args.push(flag, String(value)); @@ -235,6 +349,7 @@ export function buildDockerGpuCloneRunArgs( if (value !== undefined && value !== null) args.push("--label", `${key}=${value}`); } for (const bind of stringArray(host.Binds)) args.push("--volume", bind); + args.push(...dockerStructuredMountArgs(inspect)); const networkMode = options.networkMode ?? host.NetworkMode; pushStringFlag(args, "--network", networkMode); for (const alias of dockerNetworkAliases(inspect, networkMode)) diff --git a/src/lib/onboard/docker-gpu-patch-types.ts b/src/lib/onboard/docker-gpu-patch-types.ts index 723dec4bd5f..2fab00dd636 100644 --- a/src/lib/onboard/docker-gpu-patch-types.ts +++ b/src/lib/onboard/docker-gpu-patch-types.ts @@ -188,6 +188,25 @@ export type DockerContainerInspect = { } | null; HostConfig?: { Binds?: string[] | null; + Mounts?: Array<{ + Type?: string; + Source?: string; + Target?: string; + ReadOnly?: boolean; + Consistency?: string; + BindOptions?: unknown; + VolumeOptions?: { + NoCopy?: boolean; + Labels?: Record | null; + Subpath?: string; + DriverConfig?: unknown; + } | null; + TmpfsOptions?: { + SizeBytes?: number; + Mode?: number; + Options?: string[][] | null; + } | null; + }> | null; NetworkMode?: string; RestartPolicy?: { Name?: string; MaximumRetryCount?: number } | null; CapAdd?: string[] | null; diff --git a/src/lib/onboard/docker-gpu-patch-validation.test.ts b/src/lib/onboard/docker-gpu-patch-validation.test.ts index e1a270b3b9f..b80e2834fd3 100644 --- a/src/lib/onboard/docker-gpu-patch-validation.test.ts +++ b/src/lib/onboard/docker-gpu-patch-validation.test.ts @@ -143,6 +143,44 @@ describe("Docker GPU startup command validation (#6110)", () => { expect(dockerRunDetached).not.toHaveBeenCalled(); }); + it("rejects unsupported structured mounts before touching the original container", () => { + const inspect = inspectFixture(); + inspect.HostConfig!.Mounts = [{ Type: "bind", Source: "/host/path", Target: "/sandbox/path" }]; + const dockerStop = vi.fn(() => ({ status: 0 })); + const dockerRename = vi.fn(() => ({ status: 0 })); + const dockerRunDetached = vi.fn(() => ({ status: 0, stdout: "new-container-id\n" })); + + expect(() => + recreateOpenShellDockerSandboxWithGpu( + { + sandboxName: "alpha", + timeoutSecs: 1, + openshellSandboxCommand: ["env", "nemoclaw-start"], + }, + { + dockerCapture: vi.fn((args: readonly string[]) => + args[0] === "ps" + ? "old-container-id\n" + : args[0] === "inspect" + ? JSON.stringify([inspect]) + : "", + ), + detectSandboxFallbackDns: vi.fn(() => null), + dockerRun: vi.fn(() => ({ status: 0, stdout: "probe-id\n" })), + dockerRunDetached, + dockerRename, + dockerRm: vi.fn(() => ({ status: 0 })), + dockerStop, + readDir: vi.fn(() => null), + readFile: vi.fn(() => null), + }, + ), + ).toThrow("Unsupported Docker structured mount type 'bind'"); + expect(dockerStop).not.toHaveBeenCalled(); + expect(dockerRename).not.toHaveBeenCalled(); + expect(dockerRunDetached).not.toHaveBeenCalled(); + }); + it.each([ ";", "&&",