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.

Sign up for GitHub

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

Jump to bottom

builder: fix default docker context behavior#1454

Merged
merged 1 commit into from
Dec 7, 2022
Merged

builder: fix default docker context behavior #1454

merged 1 commit into from
Dec 7, 2022

Conversation

crazy-max
Copy link
Member

With changes from #1430 we are now setting the expected builder name and not default for docker builders:

buildx/builder/builder.go

Lines 100 to 102 in fe8d562

if b.NodeGroup.Name == "default" && len(b.NodeGroup.Nodes) == 1 {
b.NodeGroup.Name = b.NodeGroup.Nodes[0].Endpoint
}

When switching to another context other than default like docker context use foo, build will not work as it still expects default to be the active context:

$ docker buildx build.
ERROR: use `docker --context=foo buildx` to switch to context "foo"

Another issue also linked to this change is that enabled context builder other than default (here docker2010) appears twice in the list and also docker nodes are always set to default:

$ docker buildx ls
NAME/NODE         DRIVER/ENDPOINT             STATUS   BUILDKIT PLATFORMS
attest            docker-container
  attest0         unix:///var/run/docker.sock stopped
builder2          docker-container
  builder20       unix:///var/run/docker.sock running  7615120  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
eager_benz        docker-container
  eager_benz0     unix:///var/run/docker.sock running  v0.10.5  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
docker2010        docker
  default         docker2010                  running  20.10.21 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
desktop-linux     docker
  default         desktop-linux               error
desktop-windows   docker
  default         desktop-windows             error
docker1903        docker
  default         docker1903                  running  19.03.15 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
docker2010        docker
  default         docker2010                  running  20.10.21 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6

And as you can see the ls command doesn't show the current builder (with *) anymore when switching to another docker context builder. Node name for docker builders is also not correct.

This PR avoids setting the builder name in the new builder client and lets the store doing it. To make a clear distinction with context builders for validation, a new field DockerContext has been added. This also avoids unnecessary loading of contexts for validation:

$ docker buildx ls
NAME/NODE         DRIVER/ENDPOINT             STATUS   BUILDKIT PLATFORMS
attest            docker-container
  attest0         unix:///var/run/docker.sock stopped
builder2          docker-container
  builder20       unix:///var/run/docker.sock running  7615120  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
eager_benz        docker-container
  eager_benz0     unix:///var/run/docker.sock running  v0.10.5  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.21 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
desktop-linux     docker
  desktop-linux   desktop-linux               error
desktop-windows   docker
  desktop-windows desktop-windows             error
docker1903        docker
  docker1903      docker1903                  running  19.03.15 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6
docker2010 *      docker
  docker2010      docker2010                  running  20.10.21 linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6

Also found out that buildx inspect command does not return the correct builder when switching to another context other than default in current 0.9.1:

$ docker buildx inspect
Name:   default
Driver: docker

Nodes:
Name:      default
Endpoint:  docker2010
Status:    running
Buildkit:  20.10.21
Platforms: linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6

With this PR:

$ docker buildx inspect
Name:   docker2010
Driver: docker

Nodes:
Name:      docker2010
Endpoint:  docker2010
Status:    running
Buildkit:  20.10.21
Platforms: linux/amd64, linux/arm64, linux/riscv64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6

@crazy-max
builder: fix default docker context behavior
ebdd883 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max merged commit aa6fd3d into docker:master Dec 7, 2022
@crazy-max crazy-max deleted the builder-fix-default branch December 7, 2022 10:03
@crazy-max crazy-max mentioned this pull request Dec 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedevc jedevc jedevc approved these changes

@tonistiigi tonistiigi Awaiting requested review from tonistiigi
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@crazy-max @jedevc