docker / buildx

Docker CLI plugin for extended build capabilities with BuildKit
Apache License 2.0
3.44k stars 466 forks source link

Build details link not displayed in experimental mode #2382

Open crazy-max opened 5 months ago

crazy-max commented 5 months ago

related to https://github.com/docker/buildx/pull/2376#discussion_r1547788058

Build details link is not displayed for a build that fails through controller: https://github.com/docker/buildx/actions/runs/8522265808/job/23342157852?pr=2376#step:7:771

FROM busybox
RUN exit 1

Works without experimental mode enabled:

$ docker buildx build .
#0 building with "default" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 61B done
#1 DONE 0.0s

#2 [internal] load metadata for docker.io/library/busybox:latest
#2 DONE 0.5s

#3 [internal] load .dockerignore
#3 transferring context: 2B done
#3 DONE 0.0s

#4 [1/2] FROM docker.io/library/busybox:latest@sha256:c3839dd800b9eb7603340509769c43e146a74c63dca3045a8e7dc8ee07e53966
#4 CACHED

#5 [2/2] RUN exit 1
#5 ERROR: process "/bin/sh -c exit 1" did not complete successfully: exit code: 1
------
 > [2/2] RUN exit 1:
------
Dockerfile:2
--------------------
   1 |     FROM busybox
   2 | >>> RUN exit 1
   3 |
--------------------
ERROR: failed to solve: process "/bin/sh -c exit 1" did not complete successfully: exit code: 1

View build details: docker-desktop://dashboard/build/default/default/vetboddpu4fpa38o97opuyj0a

But not displayed when enabled:

$ BUILDX_EXPERIMENTAL=1 docker buildx build .
#0 building with "default" instance using docker driver

#1 [internal] connecting to local controller
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 61B done
#2 DONE 0.1s

#3 [internal] load metadata for docker.io/library/busybox:latest
#3 ...

#4 [auth] library/busybox:pull token for registry-1.docker.io
#4 DONE 0.0s

#3 [internal] load metadata for docker.io/library/busybox:latest
#3 DONE 1.0s

#5 [internal] load .dockerignore
#5 transferring context: 2B done
#5 DONE 0.1s

#6 [1/2] FROM docker.io/library/busybox:latest@sha256:c3839dd800b9eb7603340509769c43e146a74c63dca3045a8e7dc8ee07e53966
#6 CACHED

#7 [2/2] RUN exit 1
#7 ERROR: process "/bin/sh -c exit 1" did not complete successfully: exit code: 1
------
 > [2/2] RUN exit 1:
------
Dockerfile:2
--------------------
   1 |     FROM busybox
   2 | >>> RUN exit 1
   3 |
--------------------
ERROR: process "/bin/sh -c exit 1" did not complete successfully: exit code: 1
jsternberg commented 2 months ago

This seems to be an artifact of a few different systems that likely need to be refactored coming together to cause the output to not be displayed.

First, there's two ways the details get printed. First, if it's a successful build, we have this part: https://github.com/docker/buildx/blob/68076909b925af8ad12f845ccbd4a6713127c998/commands/build.go#L363

This line is never hit on a failure. Instead, there's some special code to handle the display output. runBasicBuild will wrap the error in a special error type here: https://github.com/docker/buildx/blob/68076909b925af8ad12f845ccbd4a6713127c998/build/build.go#L460-L468. This error gets eventually printed here: https://github.com/docker/buildx/blob/68076909b925af8ad12f845ccbd4a6713127c998/cmd/buildx/main.go#L109-L111

I currently suspect that the reason this doesn't work with the controller build is because it serializes the error over GRPC and this specific error doesn't deserialize over GRPC.

It might be possible to fix that, but it also seems problematic that there is such a seemingly convoluted and different path for printing the build details depending on a success or failure. It might be nice if printing the build details was done in a more consistent manner. I'll look into an appropriate way to do that.

jsternberg commented 1 month ago

We're going to take a look at this when we go and review the controller and its use cases. It seems like doing that is a better use of time than to fix individual bugs related to the controller. Since the controller is only activated through turning on experimental and this bug isn't a thing that prevents usage of the command, we'll leave it alone for now.

If someone runs into this and it's really bothering them and they want it fixed, please make a comment on this issue so we know to change our plans on this.