builder: extra init error handling by milas · Pull Request #1732 · docker/buildx

milas · 2023-04-11T16:22:48Z

Return errors from creating the NodeGroup
Ensure that b.NodeGroup != nil before reading from it during validation

Details

This was reported by a Compose user:

[BUG] Segfault during buildx initialization if no builder available compose#10453

milas · 2023-04-13T20:30:14Z

I'm curious of the builder configuration

@crazy-max: Got a response here:

$ docker buildx inspect
docker: 'buildx' is not a docker command.
See 'docker --help'

$ docker -v
Docker version 23.0.3, build 3e7cbfd

I guess in this case it's Engine but without buildx plugin, which was my suspicion.

So if we can return a predictable error here, Compose can also show a nice error, kind of like docker/cli does:

ubuntu@primary:~$ DOCKER_BUILDKIT=1 docker build --help
ERROR: BuildKit is enabled but the buildx component is missing or broken.
       Install the buildx component to build images with BuildKit:
       https://docs.docker.com/go/buildx/

crazy-max · 2023-04-13T21:32:00Z

I'm curious of the builder configuration

@crazy-max: Got a response here:
$ docker buildx inspect
docker: 'buildx' is not a docker command.
See 'docker --help'

$ docker -v
Docker version 23.0.3, build 3e7cbfd

This is not from the same user reporting the issue with Docker 20.10. This is one is with Docker 23 but I'm not able to repro on my side, see docker/compose#10453 (comment)

milas · 2023-04-16T14:23:16Z

I managed to reproduce this purely by chance this weekend while working on a personal project in a really silly way:

docker --context=doesnotexist buildx bake foo

I'm doing cursed things that require me to switch back and forth between an AMD64 + ARM64 Docker context so kept explicitly passing Docker context. FWIW I'm using docker driver with Engine v23.x.

So making a typo here is an easy way to trigger it at least 🙃

crazy-max · 2023-04-17T09:23:08Z

I managed to reproduce this purely by chance this weekend while working on a personal project in a really silly way:

Ok that's interesting. With Docker Desktop I'm not able to repro:

$ docker --context=doesnotexist buildx bake foo
unable to resolve docker endpoint: context "doesnotexist" does not exist

$ cat ~/.docker/buildx/current
{"Key":"unix:///var/run/docker.sock","Name":"","Global":false}

But on a server or using a dind image I repro:

$ docker run --rm -it docker:23.0-dind sh

/ # docker --context=doesnotexist buildx bake foo
[+] Building 0.0s (0/0)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x39 pc=0x10ca4c9]

goroutine 1 [running]:
github.com/docker/buildx/builder.(*Builder).Validate(0x238f4c0?)
        github.com/docker/buildx/builder/builder.go:111 +0x29
github.com/docker/buildx/builder.New({0x238f4c0?, 0xc000060800}, {0xc0006e7970, 0x2, 0x2380ac0?})
        github.com/docker/buildx/builder/builder.go:101 +0x27f
github.com/docker/buildx/commands.runBake({0x238f4c0, 0xc000060800}, {0xc0005b60c0, 0x1, 0x1}, {{0x3347d28, 0x0, 0x0}, {0x0, 0x0, ...}, ...})
        github.com/docker/buildx/commands/bake.go:107 +0xc5d
github.com/docker/buildx/commands.bakeCmd.func1(0xc000281800?, {0xc0005b60c0, 0x1, 0x1})
        github.com/docker/buildx/commands/bake.go:202 +0x205
github.com/spf13/cobra.(*Command).execute(0xc000281800, {0xc0004af7e0, 0x1, 0x2})
        github.com/spf13/[email protected]/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc00058c900)
        github.com/spf13/[email protected]/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/[email protected]/command.go:968
github.com/docker/cli/cli-plugins/plugin.RunPlugin(0x1d0af20?, 0xc000281200, {{0x2010c0a, 0x5}, {0x201ae58, 0xb}, {0x2359f50, 0x7}, {0x0, 0x0}, ...})
        github.com/docker/[email protected]+incompatible/cli-plugins/plugin/plugin.go:51 +0x130
main.runPlugin(0x2054ae6?)
        github.com/docker/buildx/cmd/buildx/main.go:45 +0x105
main.main()
        github.com/docker/buildx/cmd/buildx/main.go:62 +0xf9

/ # cat ~/.docker/buildx/current
{"Key":"doesnotexist","Name":"","Global":false}

On Docker Desktop if I invoke buildx in standalone mode I'm able to repro:

$ DOCKER_CONTEXT=doesnotexist /usr/local/lib/docker/cli-plugins/docker-buildx bake foo
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x39 pc=0x10ca4c9]

goroutine 1 [running]:
github.com/docker/buildx/builder.(*Builder).Validate(0x238f4c0?)
        github.com/docker/buildx/builder/builder.go:111 +0x29
github.com/docker/buildx/builder.New({0x238f4c0?, 0xc000148800}, {0xc00081fa90, 0x2, 0x2380ac0?})
        github.com/docker/buildx/builder/builder.go:101 +0x27f
github.com/docker/buildx/commands.runBake({0x238f4c0, 0xc000148800}, {0xc00011f7d0, 0x1, 0x1}, {{0x3347d28, 0x0, 0x0}, {0x0, 0x0, ...}, ...})
        github.com/docker/buildx/commands/bake.go:107 +0xc5d
github.com/docker/buildx/commands.bakeCmd.func1(0xc00023f800?, {0xc00011f7d0, 0x1, 0x1})
        github.com/docker/buildx/commands/bake.go:202 +0x205
github.com/spf13/cobra.(*Command).execute(0xc00023f800, {0xc000120170, 0x1, 0x1})
        github.com/spf13/[email protected]/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc00023f200)
        github.com/spf13/[email protected]/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/[email protected]/command.go:968
main.runStandalone(0x2054ae6?)
        github.com/docker/buildx/cmd/buildx/main.go:40 +0x75
main.main()
        github.com/docker/buildx/cmd/buildx/main.go:60 +0x108

$ cat ~/.docker/buildx/current 
{"Key":"doesnotexist","Name":"","Global":false}

So my guess is the proxy on DD does something with the context.

milas · 2023-04-25T15:09:45Z

@thaJeztah Do you think it'd be reasonable for docker/cli to return early if asked to use a non-existent context? Or is there a pre-existing method to validate the context that plugins should be calling before trying to use it? [I can make a GitHub issue over there for further discussion if interesting]

milas · 2023-04-25T15:11:35Z

So my guess is the proxy on DD does something with the context

Yep, not the proxy, but compose-cli is handling this error early, presumably since it needs to interact with the context in some cases 🙃

* Return errors from creating the `NodeGroup` * Ensure that `b.NodeGroup != nil` before reading from it during validation Signed-off-by: Milas Bowman <[email protected]>

crazy-max

LGTM thanks

milas changed the title ~~builder: extra initi error handling~~ builder: extra init error handling Apr 11, 2023

milas force-pushed the builder-init-panic branch from 60f3d87 to e4a27ec Compare April 11, 2023 16:23

jedevc reviewed Apr 11, 2023

View reviewed changes

Comment thread builder/builder.go Outdated

milas commented Apr 11, 2023

View reviewed changes

Comment thread builder/builder.go Outdated

milas force-pushed the builder-init-panic branch from e4a27ec to 27878f7 Compare April 11, 2023 16:30

milas force-pushed the builder-init-panic branch from 27878f7 to bff9d2a Compare April 25, 2023 15:01

builder: extra init error handling

14f5d49

* Return errors from creating the `NodeGroup` * Ensure that `b.NodeGroup != nil` before reading from it during validation Signed-off-by: Milas Bowman <[email protected]>

milas force-pushed the builder-init-panic branch from bff9d2a to 14f5d49 Compare April 25, 2023 15:33

crazy-max approved these changes Apr 25, 2023

View reviewed changes

crazy-max added the needs-cherry-pick/v0.10 label Apr 25, 2023

crazy-max merged commit e14e052 into docker:master Apr 26, 2023

ndeloof mentioned this pull request May 12, 2023

[BUG] Segfault during buildx initialization if no builder available docker/compose#10453

Closed

crazy-max removed the needs-cherry-pick/v0.10 label Jul 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

builder: extra init error handling#1732

builder: extra init error handling#1732
crazy-max merged 1 commit intodocker:masterfrom
milas:builder-init-panic

milas commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

milas commented Apr 13, 2023

Uh oh!

crazy-max commented Apr 13, 2023

Uh oh!

milas commented Apr 16, 2023

Uh oh!

crazy-max commented Apr 17, 2023

Uh oh!

milas commented Apr 25, 2023

Uh oh!

milas commented Apr 25, 2023

Uh oh!

crazy-max left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

milas commented Apr 11, 2023

Details

Uh oh!

Uh oh!

Uh oh!

milas commented Apr 13, 2023

Uh oh!

crazy-max commented Apr 13, 2023

Uh oh!

milas commented Apr 16, 2023

Uh oh!

crazy-max commented Apr 17, 2023

Uh oh!

milas commented Apr 25, 2023

Uh oh!

milas commented Apr 25, 2023

Uh oh!

crazy-max left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants