Spawn docker build relative to the Dockerfile #9729

404Wolf · 2025-06-24T19:04:19Z

Set build context relative to the Dockerfile so that COPYs do not break.
Also just cleaning up some duplicated helpers tests

Tests
- TODO (before merge)
- Tests included
- Tests not necessary because:
Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required
- Not required because:
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Documentation not necessary because: bugfix
Wrangler V3 Backport
- TODO (before merge)
- Wrangler PR:
- Not necessary because: not in v3

changeset-bot · 2025-06-24T19:04:24Z

🦋 Changeset detected

Latest commit: bcacff9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
wrangler	Patch
@cloudflare/vitest-pool-workers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

skepticfx · 2025-06-24T19:09:14Z

Thank you 🚀

pkg-pr-new · 2025-06-24T19:10:41Z

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9729

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9729

miniflare

npm i https://pkg.pr.new/miniflare@9729

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9729

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9729

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9729

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9729

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9729

wrangler

npm i https://pkg.pr.new/wrangler@9729

commit: bcacff9

404Wolf · 2025-06-24T19:12:27Z

Realized that we want the dirname of the dockerfile path, not the dockerfile path :/

emily-shen · 2025-06-26T10:01:08Z

hey, thanks for the contribution! would you be able to add a regression test for this? probably in e2e/containers.dev.test.ts
let me know if you need any help with this :)

emily-shen · 2025-06-26T10:50:22Z

also i'm not sure if misunderstanding the problem, but do we want to spawn docker relative to the dockerfile? the dockerfile and image_build_context is defined in the wrangler config relative to the wrangler config, so it would make sense to spawn docker relative to the wrangler config too, and pass in the paths defined there. there might be a bug with how we resolve paths, but i'm struggling to reproduce.

can you give an example of something that fails without this fix and works with? (i guess this comment is also just still asking for the regression test :p)

404Wolf · 2025-06-26T18:07:28Z

also i'm not sure if misunderstanding the problem, but do we want to spawn docker relative to the dockerfile? the dockerfile and image_build_context is defined in the wrangler config relative to the wrangler config, so it would make sense to spawn docker relative to the wrangler config too, and pass in the paths defined there. there might be a bug with how we resolve paths, but i'm struggling to reproduce.

can you give an example of something that fails without this fix and works with? (i guess this comment is also just still asking for the regression test :p)

Hey Emily, I should have more time to look into this tomorrow, but the general issue is for when you have a setup that looks like:

cloudflare/wrangler.jsonc
cloudflare/main.ts
server/main.ts
Dockerfile

And you need the Dockerfile in the scope of the docker build, so you specify ".." for the image, but the build fails / COPYs fail because docker is run from CWD=./cloudflare

According to the docs:

image_build_context: "The build context of the application, by default it is the directory of image."
And if I have image set to ".." I feel it should spawn docker from ".."

emily-shen · 2025-06-26T19:46:43Z

ah i see if i deploy with that file structure, I was confused because it doesn't seem to happen in dev. I think the actual bug is on this line:

workers-sdk/packages/wrangler/src/cloudchamber/deploy.ts

Line 152 in 869b755

buildContext: container.image_build_context ?? ".",

it should be falling back to the directory of the dockerfile rather than the cwd - buildContext is what gets passed into the docker build command, and the build command itself can spawn from wherever.

also, does setting image_build_context fix the current issue, or is it also broken somehow? here is a quick repro i've been working off, where setting image_build_context to ".." seems to fix the issue - https://github.com/emily-shen/ancient-firefly-1260/tree/main

sorry for all the back and forth, just making sure we've not messed this up in multiple places! thanks so much for the contribution 🧡

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

.changeset/clever-regions-build.md

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

vicb · 2025-07-04T09:02:56Z

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

+							},
+						} as unknown as ChildProcess;
+					})
+					// 2. docker image inspect


Does this test need 2, 3, 4, 5 ?

Sounds like what we want to test is the build path ("resolve the docker build context path based on the dockerfile location, if image_build_context is not provided")

i wanted to test all the config resolution up until the build, but I don't think there's a good way to test just that part of the deploy path?

I agree that ideally we should test that pathToDockerFile is correctly set by the build command. And then add another test testing that the build command reflects the passed value (explicit path or default to Dockerfile)

With the current, there are ~100 LOC that are not related to what we are trying to test and that might need to be updated when something unrelated to what we want to test changes.

If there is no easy/quick way to fix this, I don't think it should block merging the PR but we should revisit later. It could benefit test speed, coverage and maintainability.

packages/wrangler/src/cloudchamber/deploy.ts

vicb

LGTM with a few suggestions

vicb · 2025-07-04T15:31:54Z

packages/containers-shared/src/build.ts

@@ -28,7 +28,7 @@ export async function constructBuildCommand(
 	const dockerfile = readFileSync(options.pathToDockerfile, "utf-8");
 	// pipe in the dockerfile
 	buildCmd.push("-f", "-");
-	buildCmd.push(options.buildContext);
+	buildCmd.push(options.buildContext ?? path.dirname(options.pathToDockerfile));


Could you add some JSDoc to the function to describe this behavior?

vicb · 2025-07-04T15:32:58Z

packages/containers-shared/src/types.ts

@@ -10,7 +10,8 @@ export type BuildArgs = {
 	/** image tag in the format `name:tag`, where tag is optional */
 	tag: string;
 	pathToDockerfile: string;
-	buildContext: string;
+	/** image_build_context or args.PATH. if not provided, defaults to the dockerfile directory */


+1 for updating the doc

holopin-bot · 2025-07-04T17:00:52Z

Congratulations @404Wolf, the maintainer of this repository has issued you a holobyte! Here it is: https://holopin.io/holobyte/cmcp29fn9367107i264bxxicw

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

404Wolf requested a review from a team as a code owner June 24, 2025 19:04

workers-devprod added this to workers-sdk Jun 24, 2025

github-project-automation bot moved this to Untriaged in workers-sdk Jun 24, 2025

IRCody requested a review from a team June 24, 2025 19:08

skepticfx approved these changes Jun 24, 2025

View reviewed changes

emily-shen added the e2e Run wrangler + vite-plugin e2e tests on a PR label Jun 26, 2025

emily-shen force-pushed the spawn-docker-correctly branch from 1fe108a to ed5604a Compare July 3, 2025 19:04

404Wolf and others added 7 commits July 4, 2025 09:33

Spawn docker build relative to the Dockerfile

cbf77cd

Create changeset

4b7ae1a

Use dirname

f5bba8d

fix image_build_context fallback

4072cd5

add tests

6cc072f

update changeset

3901914

merge conflict fixups

ae4b2a5

emily-shen force-pushed the spawn-docker-correctly branch from ed5604a to ae4b2a5 Compare July 4, 2025 08:42

emily-shen added skip-pr-description-validation Skip validation of the required PR description format skip-v3-pr Skip validation of presence of a v3 backport PR labels Jul 4, 2025

vicb reviewed Jul 4, 2025

View reviewed changes

packages/wrangler/src/__tests__/deploy.test.ts Show resolved Hide resolved

vicb reviewed Jul 4, 2025

View reviewed changes

.changeset/clever-regions-build.md Outdated Show resolved Hide resolved

vicb reviewed Jul 4, 2025

View reviewed changes

packages/wrangler/src/__tests__/deploy.test.ts Show resolved Hide resolved

vicb reviewed Jul 4, 2025

View reviewed changes

packages/wrangler/src/cloudchamber/deploy.ts Show resolved Hide resolved

vicb approved these changes Jul 4, 2025

View reviewed changes

github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jul 4, 2025

emily-shen added 2 commits July 4, 2025 14:35

set default in one place

5b7f6c7

check cwd

bcacff9

emily-shen enabled auto-merge July 4, 2025 14:55

vicb reviewed Jul 4, 2025

View reviewed changes

emily-shen added this pull request to the merge queue Jul 4, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 4, 2025

emily-shen added this pull request to the merge queue Jul 4, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 4, 2025

emily-shen added this pull request to the merge queue Jul 4, 2025

Merged via the queue into cloudflare:main with commit 1b3a2b7 Jul 4, 2025
66 of 72 checks passed

github-project-automation bot moved this from Approved to Done in workers-sdk Jul 4, 2025

workers-devprod added the contribution [Holopin] Recognizes an open-source contribution, big or small label Jul 4, 2025

workers-devprod mentioned this pull request Jul 4, 2025

Version Packages #9841

Merged

Spawn docker build relative to the Dockerfile #9729

Spawn docker build relative to the Dockerfile #9729

Conversation

404Wolf commented Jun 24, 2025 • edited by emily-shen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

skepticfx commented Jun 24, 2025

Uh oh!

pkg-pr-new bot commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

404Wolf commented Jun 24, 2025

Uh oh!

emily-shen commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emily-shen commented Jun 26, 2025

Uh oh!

404Wolf commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emily-shen commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vicb Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

emily-shen Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

vicb Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vicb left a comment

Choose a reason for hiding this comment

Uh oh!

vicb Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

vicb Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

holopin-bot bot commented Jul 4, 2025

Uh oh!

Uh oh!

404Wolf commented Jun 24, 2025 •

edited by emily-shen

Loading

changeset-bot bot commented Jun 24, 2025 •

edited

Loading

pkg-pr-new bot commented Jun 24, 2025 •

edited

Loading

emily-shen commented Jun 26, 2025 •

edited

Loading

404Wolf commented Jun 26, 2025 •

edited

Loading

emily-shen commented Jun 26, 2025 •

edited

Loading