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

Add buildx option for daemon-less BuildKit support #8172

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ebekebe
Copy link

@ebekebe ebekebe commented Nov 29, 2022

Related:
#6732

Description

docker buildx build can be configured to execute against a remote buildkit instance. No docker daemon is necessary for this to work.

This PR removes dependencies on docker daemon if buildx is used, so skaffold build can be used in a CI without docker daemon.

I appreciate feedback on this PR before I start writing unit tests:

  • should we only add an option daemonLess instead of buildx?
  • buildx is only supported for skaffold build, not skaffold dev. How to best handle this?
  • is there a better way of getting rid of the docker daemon in CI while sharing the docker config for local development?

User facing changes

A new option build.local.buildx is added.

ToDo:

  • unit tests

@google-cla
Copy link

google-cla bot commented Nov 29, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ebekebe ebekebe marked this pull request as ready for review November 29, 2022 15:14
@gsquared94 gsquared94 self-requested a review November 29, 2022 18:47
@gsquared94 gsquared94 self-assigned this Nov 29, 2022
@gsquared94
Copy link
Contributor

Thanks @ebekebe for creating this PR. We've been discussing how buildx support should be implemented in Skaffold. We think we'd require a more detailed design document answering these questions:

  • Being able to set the buildx builder instances in the skaffold config
  • Actionable error messages if the target builder instance doesn't support the specified target platforms
  • Actionable error messages when buildx is not available or the specified instance doesn't exist.
  • Can the docker daemon store built multi-arch image, or should it always push to a container registry

Feel free to take a stab at writing this doc. We can't merge just the set of changes in this PR unfortunately.

@ebekebe
Copy link
Author

ebekebe commented Dec 1, 2022

Hi @gsquared94, I can see that there is no straight-forward design and more discussion is required. This PR is merely a solution to a more fundamental challenge:

We would like to use skaffold.yaml as a common build recipe for a repo that shares as much config between local and CI as possible. Locally, most of our devs use Docker and so it is natural to provide a skaffold.yaml with a docker build configuration. In CI, we are somehow limited and don't have access to a Docker daemon. So we thought about the following options:

  1. Create a custom build step that is only used in CI (e.g. via profile). AFAIK this is the recommended way to talk to buildkit. The drawback is that we need to maintain two build configs, one local docker config and a custom config for CI. Since the custom build step doesn't support all config values that the Docker step supports (e.g. dockerfile), we need to pass those manually in a non-standardised form (at least not standardised by skaffold). Given that we wanted to simplify and standardise local and CI builds among our repos, this doesn't feel right.

  2. We could write a yaml transform that translates the docker build config into a custom build config. This could be done in CI just before the skaffold build command is issued. That way, we wouldn't repeat the build config parameters in the checked-in skaffold.yaml. The drawbacks here are maintaining the custom build script, additional complexity and it becomes harder to run the CI build locally.

  3. Since docker build and docker buildx build essentially share the same options, we could create a shell script called docker in the shell PATH that masks the genuine docker executable. This script could execute /usr/bin/docker buildx $@ --push as its single action. This has the effect of rewriting skaffold's docker build ... command to docker buildx build ... --push. I tried this out with useDockerCLI=true and it works well enough for our purposes. There is only one issue: after building and pushing from buildx, skaffold tries to contact the docker daemon again (e.g. here ). This results in an error which makes skaffold return a non-zero exit code and this is bad for CI.
    So this is how I started looking into ways to tell skaffold to not interact with docker after issuing the build command.

  4. So when I have to change skaffold code anyways, I could also just add the buildx command directly without the docker-script hack. This is how this PR came about.

So given our use case, it would be totally fine with me to reduce the scope of the PR from "buildx" to "don't contact docker-daemon after build". Would that be something that could be done in smaller amount of time?

Maybe there is an option we haven't considered yet and that could also help us on our quest of sharing as much config between CI and local dev. Do you have a recommendation?

Finally, I feel this PR is not the right place to discuss these high-level design matters. Can you point me to where the discussion should happen (slack, mailing list, issue)?

Thanks

`docker buildx build` can be configured to execute a remote buildkit instance.
No docker daemon is necessary for this to work.

This commit removes dependencies on docker daemon if `buildx` is used, so `skaffold build` can be used in a CI without docker daemon.
@ebekebe
Copy link
Author

ebekebe commented May 19, 2023

Since I haven't received a reply for months now, we moved forward and worked with a patched version of skaffold that always uses docker buildx build when useDockerCLI: true. This patched version also ensures that no docker daemon is required for operation. Only buildkit needs to be configured. In the patch, only about 13 lines needed to be changed or added and I was able to rebase regularly on the latest releases without conflicts so far (fingers crossed).

Our setup looks like this:

  1. Developers use the official skaffold version locally and are instructed to set useDockerCLI: true and useBuildkit: true in their skaffold.yaml files for better compatibility with the patched version.
  2. In CI, we provide a base image for CI jobs that contains the patched skaffold version. From our developers perspective this version works the same as their local version and they can develop and test their skaffold.yaml locally before pushing changes to the CI.

This has worked great for us, but we would like to not maintain the patched version. So I hope there will be a discussion around the points I listed above.

In the meantime, if anyone is interested, here is the patch for v2.4.1 (which I cleaned up today): ebekebe@1c1fdeb

reingart added a commit to reingart/skaffold that referenced this pull request Jan 11, 2025
…Tools#6732)

* detect-buildx global config option for backward compatibility
* cache-tag global config option to customize cache destination
* new CacheTo in DockerArtifact in configuration yaml (for docker build --cache-to)

* export LoadDockerConfig to read ~/.docker/config.json for buildx detection
* fix avoid loading image via buildx if no docker daemon is accessible
* fix remote lookup / import missing in buildx workaround
* fix image import if no docker daemon is available (under buildx)
* adjust cache reference preserving tag and default cacheTo if not given
* parse buildx metadata to extract ImageID digest

Initially based on ebekebe's GoogleContainerTools#8172 patch
ebekebe@1c1fdeb

Signed-off-by: reingart@gmail.com
@mepi262
Copy link

mepi262 commented Jan 22, 2025

@ebekebe
It seems that this branch has conflicts.
Would you resolve it?

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

Successfully merging this pull request may close these issues.

3 participants