Skip to content

Commit 406fba5

Browse files
IRCodyemily-shen
andauthored
Verify docker installation earlier when deploying container (#9667)
* Verify docker installation earlier when deploying container When containers are configured we want the container and the worker to be updated together (ideally). Failing earlier when we know docker is not installed keeps us from getting into a disjointed state between the worker and container. * update error message and make it conditional on there being a dockerfile * clean up tests a bit --------- Co-authored-by: emily-shen <[email protected]>
1 parent 75b75f3 commit 406fba5

File tree

5 files changed

+99
-8
lines changed

5 files changed

+99
-8
lines changed

.changeset/social-dryers-feel.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
Fail earlier in the deploy process when deploying a container worker if docker is not detected.

packages/containers-shared/src/utils.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,18 @@ export const runDockerCmdWithOutput = async (
5050
};
5151

5252
/** throws when docker is not installed */
53-
export const verifyDockerInstalled = async (dockerPath: string) => {
53+
export const verifyDockerInstalled = async (
54+
dockerPath: string,
55+
isDev = true
56+
) => {
5457
try {
5558
await runDockerCmd(dockerPath, ["info"], ["inherit", "pipe", "pipe"]);
5659
} catch {
5760
// We assume this command is unlikely to fail for reasons other than the Docker daemon not running, or the Docker CLI not being installed or in the PATH.
5861
throw new Error(
59-
`The Docker CLI could not be launched. Please ensure that Docker is installed and running. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN.\n` +
60-
`Other container tooling that is compatible with the Docker CLI may work, but is not yet guaranteed to do so.\n` +
61-
`To suppress this error if you do not intend on triggering any container instances, set dev.enable_containers to false in your Wrangler config or passing in --enable-containers=false.`
62+
`The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running.\n` +
63+
`Other container tooling that is compatible with the Docker CLI and engine may work, but is not yet guaranteed to do so. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN and a socket with WRANGLER_DOCKER_HOST.` +
64+
`${isDev ? "\nTo suppress this error if you do not intend on triggering any container instances, set dev.enable_containers to false in your Wrangler config or passing in --enable-containers=false." : ""}`
6265
);
6366
}
6467
};

packages/wrangler/e2e/containers.dev.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ describe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
149149
const worker = helper.runLongLived("wrangler dev");
150150
expect(await worker.exitCode).toBe(1);
151151
expect(await worker.output).toContain(
152-
`The Docker CLI could not be launched. Please ensure that Docker is installed and running. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN.`
152+
`The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running.`
153+
);
154+
expect(await worker.output).toContain(
155+
`To suppress this error if you do not intend on triggering any container instances, set dev.enable_containers to false in your Wrangler config or passing in --enable-containers=false.`
153156
);
154157
});
155158
}

packages/wrangler/src/__tests__/deploy.test.ts

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8667,7 +8667,62 @@ addEventListener('fetch', event => {});`
86678667
expect(std.warn).toMatchInlineSnapshot(`""`);
86688668
});
86698669

8670-
it("should support durable object bindings to SQLite classes with containers (docker flow)", async () => {
8670+
it("should fail early if no docker is detected when deploying a container from a dockerfile", async () => {
8671+
vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/bad-docker-path");
8672+
8673+
fs.writeFileSync(
8674+
"index.js",
8675+
`export class ExampleDurableObject {}; export default{};`
8676+
);
8677+
fs.writeFileSync("./Dockerfile", "blah");
8678+
8679+
writeWranglerConfig({
8680+
durable_objects: {
8681+
bindings: [
8682+
{
8683+
name: "EXAMPLE_DO_BINDING",
8684+
class_name: "ExampleDurableObject",
8685+
},
8686+
],
8687+
},
8688+
containers: [
8689+
{
8690+
name: "my-container",
8691+
instances: 10,
8692+
class_name: "ExampleDurableObject",
8693+
image: "./Dockerfile",
8694+
},
8695+
],
8696+
migrations: [
8697+
{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] },
8698+
],
8699+
});
8700+
8701+
mockSubDomainRequest();
8702+
mockLegacyScriptData({
8703+
scripts: [{ id: "test-name", migration_tag: "v1" }],
8704+
});
8705+
8706+
mockUploadWorkerRequest({
8707+
expectedBindings: [
8708+
{
8709+
class_name: "ExampleDurableObject",
8710+
name: "EXAMPLE_DO_BINDING",
8711+
type: "durable_object_namespace",
8712+
},
8713+
],
8714+
useOldUploadApi: true,
8715+
expectedContainers: [{ class_name: "ExampleDurableObject" }],
8716+
});
8717+
8718+
await expect(runWrangler("deploy index.js")).rejects
8719+
.toThrowErrorMatchingInlineSnapshot(`
8720+
[Error: The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running.
8721+
Other container tooling that is compatible with the Docker CLI and engine may work, but is not yet guaranteed to do so. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN and a socket with WRANGLER_DOCKER_HOST.]
8722+
`);
8723+
});
8724+
8725+
it("should support durable object bindings to SQLite classes with containers (dockerfile flow)", async () => {
86718726
vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker");
86728727
function mockGetVersion(versionId: string) {
86738728
msw.use(
@@ -8713,6 +8768,12 @@ addEventListener('fetch', event => {});`
87138768
}
87148769

87158770
vi.mocked(spawn)
8771+
// 0. docker info
8772+
.mockImplementationOnce((cmd, args) => {
8773+
expect(cmd).toBe("/usr/bin/docker");
8774+
expect(args).toEqual(["info"]);
8775+
return defaultChildProcess();
8776+
})
87168777
// 1. docker build
87178778
.mockImplementationOnce((cmd, args) => {
87188779
expect(cmd).toBe("/usr/bin/docker");
@@ -9038,8 +9099,9 @@ addEventListener('fetch', event => {});`
90389099
expect(output).toContain("export {\n ExampleDurableObject,");
90399100
});
90409101

9041-
it("should support durable object bindings to SQLite classes with containers", async () => {
9102+
it("should support durable object bindings to SQLite classes with containers (image uri flow)", async () => {
90429103
// note no docker commands have been mocked here!
9104+
90439105
function mockGetVersion(versionId: string) {
90449106
msw.use(
90459107
http.get(

packages/wrangler/src/deploy/deploy.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
33
import path from "node:path";
44
import { URLSearchParams } from "node:url";
55
import { cancel } from "@cloudflare/cli";
6+
import {
7+
isDockerfile,
8+
verifyDockerInstalled,
9+
} from "@cloudflare/containers-shared";
610
import PQueue from "p-queue";
711
import { Response } from "undici";
812
import { syncAssets } from "../assets";
@@ -764,9 +768,23 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
764768
config.containers === undefined;
765769

766770
let workerBundle: FormData;
771+
const dockerPath = getDockerPath();
772+
773+
// lets fail earlier in the case where docker isn't installed
774+
// and we have containers so that we don't get into a
775+
// disjointed state where the worker updates but the container
776+
// fails.
777+
if (config.containers) {
778+
// if you have a registry url specified, you don't need docker
779+
const hasDockerfiles = config.containers?.some((container) =>
780+
isDockerfile(container.image ?? container.configuration?.image)
781+
);
782+
if (hasDockerfiles) {
783+
await verifyDockerInstalled(dockerPath, false);
784+
}
785+
}
767786

768787
if (props.dryRun) {
769-
const dockerPath = getDockerPath();
770788
if (config.containers) {
771789
for (const container of config.containers) {
772790
await maybeBuildContainer(

0 commit comments

Comments
 (0)