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.

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

[v0.13] create --append with missing builder changed from warn to error #2347

Open
3 tasks done
dixneuf19 opened this issue Mar 19, 2024 · 5 comments
Open
3 tasks done

Comments

@dixneuf19
Copy link

dixneuf19 commented Mar 19, 2024

Contributing guidelines

I've found a bug and checked that ...

  • ... the documentation does not mention anything about my problem
  • ... there are no open or closed issues that are related to my problem

Description

Hi,
We are extensively using buildx with remote buildkit nodes, notably for its speed and scheduling of build on native CPU architectures (arm64 build on arm64 VMs, same for amd64)

Before the version 0.13.0, we used the following syntax in our CI/CD pipelines

docker buildx create \
  --append \
  --name buildkit \
  --driver remote \
  --driver-opt cacert=/certs/ca.pem,cert=/certs/cert.pem,key=/certs/key.pem \
  --platform arm64 \
  tcp://buildkit-arm64.buildkit.svc.cluster.local:1234 ;
  
  docker buildx create \ 
  --append \
  --name buildkit \
  --driver remote \
  --driver-opt cacert=/certs/ca.pem,cert=/certs/cert.pem,key=/certs/key.pem \
  --platform amd64 \
  tcp://buildkit-amd64.buildkit.svc.cluster.local:1234 ;

 docker buildx build \ 
  --platform amd64,arm64 \
...

Note that we use the same syntax docker buildx create --append both time. This is not well documentend but worked nicely, with a warning for the first command

WARNING: failed to find "buildkit" for append, creating a new instance instead buildkit

With the 0.13.0 release, this behavious was broken, with the following error

ERROR: failed to find instance "buildkit" for append

It seems that this is linked to a refactoring of the create command on this commit (@crazy-max )

Since buildx is automatically shipped with docker, the 0.13.0 release was unknowingly shipped with a daily rebuild of docker:24-cli and broke our CI/CD.

While this is an undocumented feature, and we might want to explicitely fail on this usage, I find the usage of an idempotent command to add node to a builder very useful.

Indeed, in our case, we dynamically generate the CI depending on how many CPU archs we want to build, either 1 or more. So we would be forced to implement a logic like "if this is the first command, do not put --append, otherwise do. While this is doable, idempotent command are truly useful. Imagine a world without kubectl apply but only kubectl create and kubectl patch !

If you agree with my proposal, I might draft a PR, seems it looks not that complicated to reintroduce this logic. What do you think ?

Expected behaviour

docker buildx create \
  --append \
  --name buildkit \
  --driver remote \
  --driver-opt cacert=/certs/ca.pem,cert=/certs/cert.pem,key=/certs/key.pem \
  --platform arm64 \
  tcp://buildkit-arm64.buildkit.svc.cluster.local:1234 ;
  WARNING: failed to find "buildkit" for append, creating a new instance instead buildkit
  
  docker buildx create \ 
  --append \
  --name buildkit \
  --driver remote \
  --driver-opt cacert=/certs/ca.pem,cert=/certs/cert.pem,key=/certs/key.pem \
  --platform amd64 \
  tcp://buildkit-amd64.buildkit.svc.cluster.local:1234 ;

Actual behaviour

docker buildx create \
  --append \
  --name buildkit \
  --driver remote \
  --driver-opt cacert=/certs/ca.pem,cert=/certs/cert.pem,key=/certs/key.pem \
  --platform arm64 \
  tcp://buildkit-arm64.buildkit.svc.cluster.local:1234 ;
ERROR: failed to find instance "buildkit" for append

Buildx version

v0.13.0

Docker info

No response

Builders list

N/A

Configuration

FROM alpine

Build logs

No response

Additional info

No response

@dixneuf19
Copy link
Author

dixneuf19 commented Mar 19, 2024

#2346 seems to be linked

@crazy-max
Copy link
Member

Yes we bail out early since v0.13 as the instance should already exists when appending. This was changed specifically for programmatic invocations of builder creation.

@tonistiigi
Copy link
Member

I think as a default, erroring behavior makes sense. The user is asking to append a node to a named builder that does not exist. We might consider adding some way to signal the "append-or-create" mode, but not sure what way exactly this could be passed.

@tonistiigi tonistiigi changed the title Breaking change: cannot create a builder instance with the --apend flag [v0.13] create --append with missing builder changed from warn to error Mar 19, 2024
@dixneuf19
Copy link
Author

Ok thanks at least the behaviour is now explicit. However having an idempotent command would still be useful.

So maybe something like

docker buildx create \
  --append-or-create \
  --name buildkit \

or

docker buildx create \
  --append --create-if-not-exists \
  --name buildkit \

I am not very good at naming things, sorry 😅

@crazy-max
Copy link
Member

Ok thanks at least the behaviour is now explicit. However having an idempotent command would still be useful.

So maybe something like

docker buildx create \
  --append-or-create \
  --name buildkit \

or

docker buildx create \
  --append --create-if-not-exists \
  --name buildkit \

I am not very good at naming things, sorry 😅

I think we could add a new flag --create-if-missing that communicates the intent to create the builder if it doesn't exist or short one --ensure indicating that the builder should be ensured to exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants