build: set default context builder if not specified

[crazy-max]

- What I did

if build subcommand has changed to a builder alias (buildx),
user would expect docker build to always create a local docker
image (default context builder). this is for better backward
compatibility in case where a user could switch to a docker container
builder with docker buildx --use foo which does not --load by default.
also makes sure that an arbitrary builder name is not being set in the
command line or in the environment before setting the default context.

- How I did it

- How to verify it

Default builder:

$ docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
default * docker
default default running 20.10.16 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
$ docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
$ docker build -t foo -<<EOFFROM busyboxRUN echo "hello world"EOF#1 [internal] load build definition from Dockerfile#1 transferring dockerfile: 73B done#1 DONE 0.1s
...
#7 exporting to image#7 exporting layers 0.0s done#7 writing image sha256:ee2ed9777638b584951486ec450f8959a8a69a59f9f1f55e0320a79d5cf4d462 done#7 naming to docker.io/library/foo done#7 DONE 0.1s
$ docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
foo latest ee2ed9777638 5 seconds ago 1.24MB

Create and use docker-container builder:

$ docker buildx create --name foo --use --bootstrap
$ docker buildx ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
foo * docker-container
foo0 unix:///var/run/docker.sock running v0.10.3 linux/amd64, linux/amd64/v2, linux/amd64/v3, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/mips64le, linux/mips64, linux/arm/v7, linux/arm/v6
default docker
default default running 20.10.16 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
$ docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
moby/buildkit buildx-stable-1 a2c9241854f2 5 weeks ago 142MB
# specify container builder
$ docker build --builder foo -t foo -<<EOFFROM busyboxRUN echo "hello world"EOF
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
...
#6 [2/2] RUN echo "hello world"#6 0.106 hello world#6 DONE 0.1s# image not loaded as expected
$ docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
moby/buildkit buildx-stable-1 a2c9241854f2 5 weeks ago 142MB
# now building without specifying a builder should build from default builder and load the image
$ docker build -t foo -<<EOFFROM busyboxRUN echo "hello world"EOF
$ docker image ls
REPOSITORY TAG IMAGE ID CREATED SIZE
foo latest ee2ed9777638 5 minutes ago 1.24MB
moby/buildkit buildx-stable-1 a2c9241854f2 5 weeks ago 142MB

- Description for the changelog

build: set default context builder if not specified

cc @tonistiigi@jedevc

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

[codecov-commenter]

Codecov Report

Merging #3676 (b9a1276) into master (3dfef76) will increase coverage by 0.03%.
The diff coverage is 60.97%.

Additional details and impacted files

@@ Coverage Diff @@## master #3676 +/- ##
==========================================
+ Coverage 58.81% 58.84% +0.03% 
==========================================
Files 286 286 Lines 24640 24664 +24 ==========================================
+ Hits 14491 14514 +23 - Misses 9265 9266 +1 
Partials 884 884 

[jedevc]

I'm not sure, does this work across multiple contexts? e.g. on dd4l, the desktop-linux docker driver is used, while the default docker driver is in a different context.

So always forcing to the default builder might not be the right call here, since the context we're in might not always have one of that name? I'm not super familiar with using multiple contexts, so I'm not 100% sure.

[crazy-max]

Good point, we might trigger this error if using default. Will switch to use the default context.

[sudo-bmitch]

So this means docker buildx use now only applies to docker buildx commands, and not docker build? Seems reasonable.

The other option I was thinking of involved changing the default flags when running docker build for a docker-container driver to mirror the backwards compatible defaults. Effectively requiring docker build --load=false if you didn't want the compatible behavior.

Once this and --link are fixed, I'll be happy to give them a test.

[crazy-max]

So this means docker buildx use now only applies to docker buildx commands, and not docker build? Seems reasonable.

Yes that's it

The other option I was thinking of involved changing the default flags when running docker build for a docker-container driver to mirror the backwards compatible defaults.

This can be tricky as user might want to push its image which would not work as multi-exporters are not currently supported with BuildKit: moby/buildkit#1555

[jedevc]

Will switch to use the default context.

Seems like the right behavior now! 🎉

[thaJeztah]

Hmmm.. bit on the fence; so IIRC, the intent was to make docker build a full alias for docker buildx build, and provide all the enhancements that buildx added. From that perspective, I'd expect that if a custom builder was set to be the active builder, docker build to also use that builder.

However, if a container-builder is the active builder, and no useful output was given (neither --push, --load or --output), then the result is definitely confusing.

Wondering if we can do something with that information (no output given), and use --load as a default if none was set? 🤔

[crazy-max]

As discussed on Slack, I think it still makes sense to use the default context for docker build to keep the same experience as 20.10.

However, if a container-builder is the active builder, and no useful output was given (neither --push, --load or --output), then the result is definitely confusing.

Don't think this is an issue as users can still use buildx build to rely on their current builder as they use to currently.

Wondering if we can do something with that information (no output given), and use --load as a default if none was set? 🤔

--load is dockerd specific and adding specific flags might not be ideal/future proof with buildx I think. Also loading for every build might not be expected if someone doesn't want to output anything like if you have a stage that just run tests for your project or linting, vendor validation, etc...

Let me know if you have the same or other concerns and how we can improve that.

[sudo-bmitch]

However, if a container-builder is the active builder, and no useful output was given (neither --push, --load or --output), then the result is definitely confusing.

My bigger concern is when running something like a Makefile from a cloned git repo, the docker build inside that Makefile should work on all the machines where it's run, without customizing the flags for each environment.

[thaJeztah]

--load is dockerd specific and adding specific flags might not be ideal/future proof with buildx I think. Also loading for every build might not be expected if someone doesn't want to output anything like if you have a stage that just run tests for your project or linting, vendor validation, etc...

This still feels as a very specific use-case, and it's odd that this was chosen to be the default, and not an opt-in (more below).

to keep the same experience as 20.10.

So, it depends how you ran it in 20.10. If I didn't opt-in to use buildx (and thus not into the advanced features), then the default builder would be used;

With the built-in BuildKit client (on 20.10);

DOCKER_BUILDKIT=1 docker build -t foo -<<'EOF'FROM busyboxRUN echo helloEOF
[+] Building 4.7s (6/6) FINISHED
=> [internal] load build definition from Dockerfile 0.3s
=> => transferring dockerfile: 70B 0.1s
=> [internal] load .dockerignore 0.2s
=> => transferring context: 2B 0.0s
=> [internal] load metadata for docker.io/library/busybox:latest 0.0s
=> CACHED [1/2] FROM docker.io/library/busybox 0.0s
=> [2/2] RUN echo hello 3.9s
=> exporting to image 0.3s
=> => exporting layers 0.2s
=> => writing image sha256:66ed75cd29b095e1bdcee7720f7960cdcdedae33f6b322c3d2e57e3f24e17ee2 0.0s
=> => naming to docker.io/library/foo

However, if I opted-in to using buildx (docker buildx install) to get the advanced features, and if I switched the current builder to a container buidler, then a warning is printed (generated in buildx;

docker buildx install
docker build -t foo -<<'EOF'FROM busyboxRUN echo helloEOF
[+] Building 6.2s (5/5) FINISHED
=> [internal] load build definition from Dockerfile 0.8s
=> => transferring dockerfile: 65B 0.1s
=> [internal] load .dockerignore 0.6s
=> => transferring context: 2B 0.1s
=> [internal] load metadata for docker.io/library/busybox:latest 3.7s
=> [1/2] FROM docker.io/library/busybox@sha256:3614ca5eacf0a3a1bcc361c939202a974b4902b9334ff36eb29ffe9011aaad83 0.9s
=> => resolve docker.io/library/busybox@sha256:3614ca5eacf0a3a1bcc361c939202a974b4902b9334ff36eb29ffe9011aaad83 0.0s
=> => sha256:19d511225f94f9b5cbf3836eb02b5273c01b95da50735742560e3e45b8c8bfcc 773.26kB / 773.26kB 0.4s
=> => extracting sha256:19d511225f94f9b5cbf3836eb02b5273c01b95da50735742560e3e45b8c8bfcc 0.3s
=> [2/2] RUN echo hello 0.4s
WARNING: No output specified fordocker-container driver. Build result will only remainin the build cache. To push result image into registry use --push or to load image into docker use --load

Overall, I prefer the latter over the earlier one (I even would've preferred it to be an error at the start - when dealing with ambiguous situations, it's better to produce an error to prevent people from using it, than not; an error can later turned into a feature, a "non-error" will result in a backward-incompatible change).

• The above would only occur if; the current builder was changed to not be the default; this is not something that happens automatically, so this would've been caused by a user action
• IMO, there can be only one "current" builder; if I use docker builder use <X> / docker buildx use <X>, I'd expect that builder to be used always (unless I override with --builder)
• The docker build command has always been to build an image, with the expectation to have that image appear in docker image ls (docker build --help shows "Build an image from a Dockerfile")
• BuildKit / buildx added additional features to docker build (--output, --push), which were opt-in
• However, when a container-builder (or other driver) is used, that's not the case (output is now "optional") <-- this is the situation described here
• AFAIK, the intent is still for docker build to be the canonical subcommand to perform a build
• And (with buildx being the default client for build), for docker build to become more powerful with new features added that were not available in the old, built-in, BuildKit client.

Some options;

1. Produce a WARNING (no output specified) - I think that's the current situation? (Ideally, we'd print that warning at the start of the build as well, so that the user is notified early)
2. When invoking a build through docker build, produce an error but continue to allow it when invoked explicitly through docker buildx build. This would allow the user to explicitly opt-on to this advanced feature by invoking the build through docker buildx build
3. When invoking a build through docker build, automatically set --load

Of the above;

• both 2. and 3. would be a (breaking) change in behavior
• 2. would have the advantage that it allows for this feature to be enabled in future (as described earlier).
• 3. is the most "convenient" one, but effectively prevents us from ever allowing "no output as default".
• Question is; do we want this as a default (depending on the driver) - even when invoked through docker buildx build? It feels inconsistent for the result of docker build to differ depending on what build happened to be used; if we prefer "explicit" over "implicit", I'd rather have an error (no output specified), an require an explicit --output=none (e.g.).

[tonistiigi]

@thaJeztah If you insist, we could keep the old behavior of buildx install. When user types that, then they say that they want docker build and docker buildx build to always be identical. This way we have covered all the possible backward compatibility angles.

After buildx is default I think most of the appeal of buildx install is going away anyway and we can start to gradually sunset that command.

[thaJeztah]

Ah, sorry, I meant to reply; I wasn't 100% sure what the proposal was 😅 ☺️

On 22.06 (buildx as default), I would consider that the equivalent as docker buildx install on older versions; buildx build is set up to replace build (with all the advantages/features that brings), which would mean;

docker builder create --use --name foo --driver=docker-container
echo"FROM alpine"| docker build -
...
WARNING: No output specified fordocker-container driver. Build result will only remainin the build cache. To push result image into registry use --push or to load image into docker use --load

If the proposal is to make that the behaviour, that sounds good to me (making it a hard error if no explicit output is set would still be my preference, but I realise that requires more changes).

Can we print the warning (both?) at the start (and at the end)?

[crazy-max]

As discussed, do not enforce the default context if a builder alias is set ("buildx install") so user keeps the same build experience.