diff --git a/.changeset/full-pugs-say.md b/.changeset/full-pugs-say.md new file mode 100644 index 000000000000..9ca89b3039b0 --- /dev/null +++ b/.changeset/full-pugs-say.md @@ -0,0 +1,10 @@ +--- +"@cloudflare/containers-shared": patch +"wrangler": patch +--- + +feat(containers): try to automatically get the socket path that the container engine is listening on. + +Currently, if your container engine isn't set up to listen on `unix:///var/run/docker.sock` (or isn't symlinked to that), then you have to manually set this via the `dev.containerEngine` field in your Wrangler config, or via the env vars `WRANGLER_DOCKER_HOST`. This change means that we will try and get the socket of the current context automatically. This should reduce the occurrence of opaque `internal error`s thrown by the runtime when the daemon is not listening on `unix:///var/run/docker.sock`. + +In addition to `WRANGLER_DOCKER_HOST`, `DOCKER_HOST` can now also be used to set the container engine socket address. diff --git a/.changeset/yummy-coins-greet.md b/.changeset/yummy-coins-greet.md new file mode 100644 index 000000000000..cbbc317d473d --- /dev/null +++ b/.changeset/yummy-coins-greet.md @@ -0,0 +1,7 @@ +--- +"@cloudflare/vite-plugin": patch +--- + +fix: properly set the socket path that the container engine is listening on. + +Previously, this was only picking up the value set in Wrangler config under `dev.containerEngine`, but this value can also be set from env vars or automatically read from the current docker context. diff --git a/packages/containers-shared/src/utils.ts b/packages/containers-shared/src/utils.ts index 238f19772680..c23117139c07 100644 --- a/packages/containers-shared/src/utils.ts +++ b/packages/containers-shared/src/utils.ts @@ -1,4 +1,4 @@ -import { execFile, spawn } from "child_process"; +import { execFileSync, spawn } from "child_process"; import { randomUUID } from "crypto"; import { existsSync, statSync } from "fs"; import path from "path"; @@ -61,22 +61,15 @@ export const runDockerCmd = ( }; }; -export const runDockerCmdWithOutput = async ( - dockerPath: string, - args: string[] -): Promise => { - return new Promise((resolve, reject) => { - execFile(dockerPath, args, (error, stdout) => { - if (error) { - return reject( - new Error( - `Failed running docker command: ${error.message}. Command: ${dockerPath} ${args.join(" ")}` - ) - ); - } - return resolve(stdout.trim()); - }); - }); +export const runDockerCmdWithOutput = (dockerPath: string, args: string[]) => { + try { + const stdout = execFileSync(dockerPath, args, { encoding: "utf8" }); + return stdout.trim(); + } catch (error) { + throw new Error( + `Failed running docker command: ${(error as Error).message}. Command: ${dockerPath} ${args.join(" ")}` + ); + } }; /** throws when docker is not installed */ @@ -209,7 +202,7 @@ export const getContainerIdsFromImage = async ( dockerPath: string, ancestorImage: string ) => { - const output = await runDockerCmdWithOutput(dockerPath, [ + const output = runDockerCmdWithOutput(dockerPath, [ "ps", "-a", "--filter", @@ -250,3 +243,85 @@ export async function checkExposedPorts( export function generateContainerBuildId() { return randomUUID().slice(0, 8); } + +/** + * Output of docker context ls --format json + */ +type DockerContext = { + Current: boolean; + Description: string; + DockerEndpoint: string; + Error: string; + Name: string; +}; + +/** + * Run `docker context ls` to get the socket from the currently active Docker context + * @returns The socket path or null if we are not able to determine it + */ +export function getDockerSocketFromContext(dockerPath: string): string | null { + try { + const output = runDockerCmdWithOutput(dockerPath, [ + "context", + "ls", + "--format", + "json", + ]); + + // Parse each line as a separate JSON object + const lines = output.trim().split("\n"); + const contexts: DockerContext[] = lines.map((line) => JSON.parse(line)); + + // Find the current context + const currentContext = contexts.find((context) => context.Current === true); + + if (currentContext && currentContext.DockerEndpoint) { + return currentContext.DockerEndpoint; + } + } catch { + // Fall back to null if docker context inspection fails so that we can use platform defaults + } + return null; +} +/** + * Resolve Docker host as follows: + * 1. Check WRANGLER_DOCKER_HOST environment variable + * 2. Check DOCKER_HOST environment variable + * 3. Try to get socket from active Docker context + * 4. Fall back to platform-specific defaults + */ +export function resolveDockerHost(dockerPath: string): string { + if (process.env.WRANGLER_DOCKER_HOST) { + return process.env.WRANGLER_DOCKER_HOST; + } + + if (process.env.DOCKER_HOST) { + return process.env.DOCKER_HOST; + } + + // 3. Try to get socket from by running `docker context ls` + + const contextSocket = getDockerSocketFromContext(dockerPath); + if (contextSocket) { + return contextSocket; + } + + // 4. Fall back to platform-specific defaults + // (note windows doesn't work yet due to a runtime limitation) + return process.platform === "win32" + ? "//./pipe/docker_engine" + : "unix:///var/run/docker.sock"; +} + +/** + * + * Get docker host from environment variables or platform defaults. + * Does not use the docker context ls command, so we + */ +export const getDockerHostFromEnv = (): string => { + const fromEnv = process.env.WRANGLER_DOCKER_HOST ?? process.env.DOCKER_HOST; + + return fromEnv ?? process.platform === "win32" + ? "//./pipe/docker_engine" + : "unix:///var/run/docker.sock"; +}; diff --git a/packages/containers-shared/tests/docker-context.test.ts b/packages/containers-shared/tests/docker-context.test.ts new file mode 100644 index 000000000000..5ff0f8c951d7 --- /dev/null +++ b/packages/containers-shared/tests/docker-context.test.ts @@ -0,0 +1,51 @@ +import { execFileSync } from "child_process"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { resolveDockerHost } from "../src/utils"; + +vi.mock("node:child_process"); +// We can only really run these tests on Linux, because we build our images for linux/amd64, +// and github runners don't really support container virtualization in any sane way +describe.skipIf(process.platform !== "linux" && process.env.CI === "true")( + "resolveDockerHost", + () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("should return WRANGLER_DOCKER_HOST when set", async () => { + vi.stubEnv( + "WRANGLER_DOCKER_HOST", + "unix:///FROM/WRANGLER/DOCKER/HOST/wrangler/socket" + ); + vi.stubEnv("DOCKER_HOST", "unix:///FROM/DOCKER/HOST/docker/socket"); + + const result = resolveDockerHost("/no/op/docker"); + expect(result).toBe("unix:///FROM/WRANGLER/DOCKER/HOST/wrangler/socket"); + }); + + it("should return DOCKER_HOST when WRANGLER_DOCKER_HOST is not set", async () => { + vi.stubEnv("WRANGLER_DOCKER_HOST", undefined); + vi.stubEnv("DOCKER_HOST", "unix:///FROM/DOCKER/HOST/docker/socket"); + + const result = resolveDockerHost("/no/op/docker"); + expect(result).toBe("unix:///FROM/DOCKER/HOST/docker/socket"); + }); + + it("should use Docker context when no env vars are set", async () => { + vi.mocked(execFileSync) + .mockReturnValue(`{"Current":true,"Description":"Current DOCKER_HOST based configuration","DockerEndpoint":"unix:///FROM/CURRENT/CONTEXT/run/docker.sock","Error":"","Name":"default"} +{"Current":false,"Description":"Docker Desktop","DockerEndpoint":"unix:///FROM/OTHER/CONTEXT/run/docker.sock","Error":"","Name":"desktop-linux"}`); + const result = resolveDockerHost("/no/op/docker"); + expect(result).toBe("unix:///FROM/CURRENT/CONTEXT/run/docker.sock"); + }); + + it("should fall back to platform default when context fails", () => { + vi.mocked(execFileSync).mockImplementation(() => { + throw new Error("Docker command failed"); + }); + + const result = resolveDockerHost("/no/op/docker"); + expect(result).toBe("unix:///var/run/docker.sock"); + }); + } +); diff --git a/packages/vite-plugin-cloudflare/src/index.ts b/packages/vite-plugin-cloudflare/src/index.ts index b0b05922e6b0..de28fb830d9b 100644 --- a/packages/vite-plugin-cloudflare/src/index.ts +++ b/packages/vite-plugin-cloudflare/src/index.ts @@ -4,6 +4,7 @@ import * as path from "node:path"; import { generateContainerBuildId, getContainerIdsByImageTags, + resolveDockerHost, } from "@cloudflare/containers-shared/src/utils"; import { generateStaticRoutingRuleMatcher } from "@cloudflare/workers-shared/asset-worker/src/utils/rules-engine"; import replace from "@rollup/plugin-replace"; @@ -370,9 +371,12 @@ if (import.meta.hot) { const hasDevContainers = entryWorkerConfig?.containers?.length && entryWorkerConfig.dev.enable_containers; + const dockerPath = getDockerPath(); if (hasDevContainers) { containerBuildId = generateContainerBuildId(); + entryWorkerConfig.dev.container_engine = + resolveDockerHost(dockerPath); } const miniflareDevOptions = await getDevMiniflareOptions({ @@ -445,8 +449,6 @@ if (import.meta.hot) { } if (hasDevContainers) { - const dockerPath = getDockerPath(); - containerImageTagsSeen = await prepareContainerImages({ containersConfig: entryWorkerConfig.containers, containerBuildId, diff --git a/packages/wrangler/src/__tests__/dev.test.ts b/packages/wrangler/src/__tests__/dev.test.ts index 7a83e1f6df4d..373d4eae9ad9 100644 --- a/packages/wrangler/src/__tests__/dev.test.ts +++ b/packages/wrangler/src/__tests__/dev.test.ts @@ -34,7 +34,7 @@ import type { Mock, MockInstance } from "vitest"; vi.mock("../api/startDevWorker/ConfigController", (importOriginal) => importOriginal() ); - +vi.mock("node:child_process"); vi.mock("../dev/hotkeys"); vi.mock("@cloudflare/containers-shared", async (importOriginal) => { @@ -1245,16 +1245,44 @@ describe.sequential("wrangler dev", () => { }); describe("container engine", () => { - it("should default to docker socket", async () => { + const minimalContainerConfig = { + durable_objects: { + bindings: [ + { + name: "EXAMPLE_DO_BINDING", + class_name: "ExampleDurableObject", + }, + ], + }, + migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], + containers: [ + { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "docker.io/hello:world", + }, + ], + }; + let mockExecFileSync: ReturnType; + const mockedDockerContextLsOutput = `{"Current":true,"Description":"Current DOCKER_HOST based configuration","DockerEndpoint":"unix:///current/run/docker.sock","Error":"","Name":"default"} +{"Current":false,"Description":"Docker Desktop","DockerEndpoint":"unix:///other/run/docker.sock","Error":"","Name":"desktop-linux"}`; + + beforeEach(async () => { + const childProcess = await import("node:child_process"); + mockExecFileSync = vi.mocked(childProcess.execFileSync); + + mockExecFileSync.mockReturnValue(mockedDockerContextLsOutput); + }); + it("should default to socket of current docker context", async () => { writeWranglerConfig({ main: "index.js", + ...minimalContainerConfig, }); fs.writeFileSync("index.js", `export default {};`); const config = await runWranglerUntilConfig("dev"); expect(config.dev.containerEngine).toEqual( - process.platform === "win32" - ? "//./pipe/docker_engine" - : "unix:///var/run/docker.sock" + "unix:///current/run/docker.sock" ); }); @@ -1265,6 +1293,7 @@ describe.sequential("wrangler dev", () => { port: 8888, container_engine: "test.sock", }, + ...minimalContainerConfig, }); fs.writeFileSync("index.js", `export default {};`); @@ -1277,6 +1306,7 @@ describe.sequential("wrangler dev", () => { dev: { port: 8888, }, + ...minimalContainerConfig, }); fs.writeFileSync("index.js", `export default {};`); vi.stubEnv("WRANGLER_DOCKER_HOST", "blah.sock"); diff --git a/packages/wrangler/src/api/dev.ts b/packages/wrangler/src/api/dev.ts index 0059fdb79b2d..fbe39b509235 100644 --- a/packages/wrangler/src/api/dev.ts +++ b/packages/wrangler/src/api/dev.ts @@ -1,10 +1,7 @@ import events from "node:events"; import { fetch, Request } from "undici"; import { startDev } from "../dev"; -import { - getDockerHost, - getDockerPath, -} from "../environment-variables/misc-variables"; +import { getDockerPath } from "../environment-variables/misc-variables"; import { run } from "../experimental-flags"; import { logger } from "../logger"; import type { Environment } from "../config"; @@ -165,6 +162,8 @@ export async function unstable_dev( const defaultLogLevel = testMode ? "warn" : "log"; const local = options?.local ?? true; + const dockerPath = options?.experimental?.dockerPath ?? getDockerPath(); + const devOptions: StartDevOptions = { script: script, inspect: false, @@ -227,8 +226,8 @@ export async function unstable_dev( enableIpc: options?.experimental?.enableIpc, nodeCompat: undefined, enableContainers: options?.experimental?.enableContainers ?? false, - dockerPath: options?.experimental?.dockerPath ?? getDockerPath(), - containerEngine: options?.experimental?.containerEngine ?? getDockerHost(), + dockerPath, + containerEngine: options?.experimental?.containerEngine, }; //outside of test mode, rebuilds work fine, but only one instance of wrangler will work at a time diff --git a/packages/wrangler/src/api/integrations/platform/index.ts b/packages/wrangler/src/api/integrations/platform/index.ts index cbb778eadc37..6efa41151ca4 100644 --- a/packages/wrangler/src/api/integrations/platform/index.ts +++ b/packages/wrangler/src/api/integrations/platform/index.ts @@ -1,3 +1,4 @@ +import { resolveDockerHost } from "@cloudflare/containers-shared"; import { kCurrentWorker, Miniflare } from "miniflare"; import { getAssetsOptions, NonExistentAssetsDirError } from "../../../assets"; import { readConfig } from "../../../config"; @@ -12,7 +13,7 @@ import { buildSitesOptions, getImageNameFromDOClassName, } from "../../../dev/miniflare"; -import { getDockerHost } from "../../../environment-variables/misc-variables"; +import { getDockerPath } from "../../../environment-variables/misc-variables"; import { logger } from "../../../logger"; import { getSiteAssetPaths } from "../../../sites"; import { dedent } from "../../../utils/dedent"; @@ -460,11 +461,15 @@ export function unstable_getMiniflareWorkerOptions( ? buildAssetOptions({ assets: processedAssetOptions }) : {}; + const useContainers = + config.dev?.enable_containers && config.containers?.length; const workerOptions: SourcelessWorkerOptions = { compatibilityDate: config.compatibility_date, compatibilityFlags: config.compatibility_flags, modulesRules, - containerEngine: config.dev.container_engine ?? getDockerHost(), + containerEngine: useContainers + ? config.dev.container_engine ?? resolveDockerHost(getDockerPath()) + : undefined, ...bindingOptions, ...sitesOptions, diff --git a/packages/wrangler/src/api/startDevWorker/ConfigController.ts b/packages/wrangler/src/api/startDevWorker/ConfigController.ts index faba79452ae5..07d27e361ed3 100644 --- a/packages/wrangler/src/api/startDevWorker/ConfigController.ts +++ b/packages/wrangler/src/api/startDevWorker/ConfigController.ts @@ -1,5 +1,6 @@ import assert from "node:assert"; import path from "node:path"; +import { resolveDockerHost } from "@cloudflare/containers-shared"; import { watch } from "chokidar"; import { getAssetsOptions, validateAssetsArgsAndConfig } from "../../assets"; import { fillOpenAPIConfiguration } from "../../cloudchamber/common"; @@ -15,10 +16,7 @@ import { } from "../../dev"; import { getClassNamesWhichUseSQLite } from "../../dev/class-names-sqlite"; import { getLocalPersistencePath } from "../../dev/get-local-persistence-path"; -import { - getDockerHost, - getDockerPath, -} from "../../environment-variables/misc-variables"; +import { getDockerPath } from "../../environment-variables/misc-variables"; import { UserError } from "../../errors"; import { getFlag } from "../../experimental-flags"; import { logger, runWithLogLevel } from "../../logger"; @@ -119,6 +117,9 @@ async function resolveDevConfig( const initialIpListenCheck = initialIp === "*" ? "0.0.0.0" : initialIp; + const useContainers = + config.dev.enable_containers && config.containers?.length; + return { auth, remote: input.dev?.remote, @@ -160,10 +161,11 @@ async function resolveDevConfig( enableContainers: input.dev?.enableContainers ?? config.dev.enable_containers, dockerPath: input.dev?.dockerPath ?? getDockerPath(), - containerEngine: - input.dev?.containerEngine ?? - config.dev.container_engine ?? - getDockerHost(), + containerEngine: useContainers + ? input.dev?.containerEngine ?? + config.dev.container_engine ?? + resolveDockerHost(input.dev?.dockerPath ?? getDockerPath()) + : undefined, containerBuildId: input.dev?.containerBuildId, } satisfies StartDevWorkerOptions["dev"]; } diff --git a/packages/wrangler/src/environment-variables/factory.ts b/packages/wrangler/src/environment-variables/factory.ts index 152e5cb7d320..2076b7fd1f25 100644 --- a/packages/wrangler/src/environment-variables/factory.ts +++ b/packages/wrangler/src/environment-variables/factory.ts @@ -35,7 +35,10 @@ type VariableNames = | "WRANGLER_REGISTRY_PATH" | "WRANGLER_D1_EXTRA_LOCATION_CHOICES" | "WRANGLER_DOCKER_BIN" - | "WRANGLER_DOCKER_HOST"; + // We don't get the following using the environment variable factory, + // but including here so that all environment variables are documented here: + | "WRANGLER_DOCKER_HOST" + | "DOCKER_HOST"; type DeprecatedNames = | "CF_ACCOUNT_ID" diff --git a/packages/wrangler/src/environment-variables/misc-variables.ts b/packages/wrangler/src/environment-variables/misc-variables.ts index e2b44aa21525..c6f80ce6ef5a 100644 --- a/packages/wrangler/src/environment-variables/misc-variables.ts +++ b/packages/wrangler/src/environment-variables/misc-variables.ts @@ -273,15 +273,3 @@ export const getDockerPath = getEnvironmentVariableFactory({ return "docker"; }, }); - -/** - * `WRANGLER_DOCKER_HOST` specifies the Docker socket to connect to. - */ -export const getDockerHost = getEnvironmentVariableFactory({ - variableName: "WRANGLER_DOCKER_HOST", - defaultValue() { - return process.platform === "win32" - ? "//./pipe/docker_engine" - : "unix:///var/run/docker.sock"; - }, -}); diff --git a/packages/wrangler/turbo.json b/packages/wrangler/turbo.json index dbd769c9da95..455084feeef1 100644 --- a/packages/wrangler/turbo.json +++ b/packages/wrangler/turbo.json @@ -50,7 +50,9 @@ "WRANGLER_DISABLE_REQUEST_BODY_DRAINING", "WRANGLER_LOG", "WRANGLER_SEND_METRICS", - "WRANGLER_WORKER_REGISTRY_PORT" + "WRANGLER_WORKER_REGISTRY_PORT", + "DOCKER_HOST", + "WRANGLER_DOCKER_HOST" ] }, "test:ci": { diff --git a/turbo.json b/turbo.json index fd96d198f399..17b4075dcc71 100644 --- a/turbo.json +++ b/turbo.json @@ -10,7 +10,9 @@ "VSCODE_INSPECTOR_OPTIONS", "WRANGLER_LOG_PATH", "TEST_REPORT_PATH", - "CLOUDFLARE_CONTAINER_REGISTRY" + "CLOUDFLARE_CONTAINER_REGISTRY", + "DOCKER_HOST", + "WRANGLER_DOCKER_HOST" ], "tasks": { "dev": {