Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: allow external Dockerfile on remote context #994

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

corhere
Copy link
Contributor

@corhere corhere commented Mar 9, 2022

Support Dockerfile from stdin or a local file with remote contexts. Fixes moby/moby#38254 now that buildx is the only BuildKit client included in the Docker CLI (docker/cli#3314).

Support for external Dockerfile on remote contexts was added to BuildKit in moby/buildkit#947, which is included in the BuildKit v0.5.0 release and first vendored into Moby in moby/moby#39132, included in v19.03.0. The client side was the only missing piece.

@corhere
Copy link
Contributor Author

corhere commented Mar 9, 2022

@thaJeztah @tonistiigi @tiborvass PTAL

@thaJeztah
Copy link
Member

nice!! thank you for working on this!

@thaJeztah thaJeztah requested a review from tonistiigi March 10, 2022 21:09
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

@tonistiigi PTAL

@tonistiigi
Copy link
Member

(no specific issue, just want to make sure I review this before we merge)

@tonistiigi
Copy link
Member

This is currently breaking:

» docker buildx build "https://github.com/moby/buildkit.git" -f ./frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile
[+] Building 0.0s (0/0)
error: could not find frontend/dockerfile/cmd/dockerfile-frontend: stat frontend/dockerfile/cmd/dockerfile-frontend: no such file or directory

while the current version works fine. I think the definition of -f for such cases needs to remain as is.

What we can support are cases like:

docker buildx build -f - "git://" < ./path/to/Dockerfile
docker buildx build -f ./path/to/Docerfile - < ./path/to/context.tar

@tonistiigi tonistiigi added this to the v0.9.0 milestone May 12, 2022
@thaJeztah
Copy link
Member

I think for this case;

» docker buildx build "https://github.com/moby/buildkit.git" -f ./frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile
[+] Building 0.0s (0/0)
error: could not find frontend/dockerfile/cmd/dockerfile-frontend: stat frontend/dockerfile/cmd/dockerfile-frontend: no such file or directory

In the classic builder we implicitly changed the path to - ("read from stdin") and then read the Dockerfile content; would a similar workaround work here?

@tonistiigi
Copy link
Member

@thaJeztah I don't understand but the command above works in legacy builder as well. -f means file path in relation to the git repository.

@thaJeztah
Copy link
Member

Ah, you're right; I thought it would take the local file (due to ./)

Maybe I misunderstood your "I think the definition of -f for such cases needs to remain as is." (I interpreted your comment as "won't fix")

So, the issue to fix is "remote context" but file from stdin; so my example from the issue;

docker buildx build -t statx --no-cache -f- https://github.com/whotwagner/statx-fun.git <<-EOF
FROM ubuntu:18.04
RUN apt-get update && apt-get install -y gcc make
RUN mkdir /src/
WORKDIR /src/
COPY . .
RUN make
EOF

@corhere
Copy link
Contributor Author

corhere commented May 12, 2022

If I'm understanding it correctly, the desired behavior is that the -f argument should always be interpreted as either a path relative to the root of the context or STDIN?

docker buildx build ... Interpretation
-f - "git://context" < custom-dockerfile Dockerfile from STDIN applied to remote context
-f ./path/to/custom-dockerfile "git://context" Dockerfile read from git://context/path/to/custom-dockerfile
-f ./path/to/custom-dockerfile - < context.tar Dockerfile read from file named path/to/custom-dockerfile in context.tar?

@tonistiigi
Copy link
Member

If context is a local dir then -f is relative to the working dir. Otherwise it is relative to the context (and can't be outside context). If -f - then it is just data and path does not matter. If context comes from stdin then I guess we can't use external Dockerfile as stdin is already used, although API should support that case as well.

BuildKit has supported external Dockerfile on remote contexts since
v0.5.0, included in Moby v19.03.0. The client side was the only missing
piece.

Signed-off-by: Cory Snider <[email protected]>
@corhere corhere force-pushed the local-dockerfile-remote-context branch from a290f73 to ca35076 Compare May 16, 2022 20:47
@tonistiigi tonistiigi merged commit 766653f into docker:master Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BuildKit doesn't handle Dockerfile from stdin, when using remote context
4 participants