build: allow external Dockerfile on remote context by corhere · Pull Request #994 · docker/buildx (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

corhere

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

@thaJeztah

nice!! thank you for working on this!

crazy-max

Choose a reason for hiding this comment

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

LGTM

@thaJeztah

@tonistiigi

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

@tonistiigi

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

@thaJeztah

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

@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

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

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

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.

@corhere

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 csnider@mirantis.com

tonistiigi