Experimental BuildKit support #37151

tonistiigi · 2018-05-25T03:13:19Z

This PR adds experimental support for using moby/buildkit as a backend for docker build. #32925 #34227

The support is only enabled in the experimental daemon, and user needs to opt-in in docker/cli to start using it.

# in daemon
dockerd -D --experimental

# in Docker CLI
export DOCKER_BUILDKIT=1
docker build .

Docker CLI branch with BuildKit support enabled is in https://github.com/tonistiigi/docker-cli/tree/experimental-buildkit

In the API, version field allows switching between different build backends.

This enables lots of new capabilities, including parallel build steps and requests, skipping unused stages, efficient incremental builds, build context file detection, remote cache support, graceful cancellation, build cache issues, etc. It also enables using experimental Dockerfile features without requiring to update Moby by using the BuildKit frontend images.

The technical side of this integration is a temporary (and somewhat limited) solution as BuildKit is based on containerd storage and that has not been integrated into Moby yet. The current solution provides adapters to some components to provide compatibility between things like containerd snapshots and Moby layerstore/storage drivers or image store. When containerd storage support lands in Moby, all these adapters should be removed entirely, and BuildKit can use containerd directly. This temporary solution allows us to get feedback before we feel comfortable to move it out of experimental.

The tests will not currently use BuildKit in the PR as it requires opt-in and a new CLI binary. We have run the tests manually. Some of them need to be reworked for output change or a side-effect removal. We will continue addressing some of the test results that we are still investigating.

Known issues (some of them will not make it into 18.06 release):

no automatic GC (manual system df and system prune work)
~~pulling schema1 images during build~~
~~automatic path detection in dockerfile doesn't detect unused symlinks~~
resource limitation and --net options: execution completely bypasses the daemon, net is host.
windows support (depending on containerd runtime integration)
--pull (currently existing images in image store always take precedence)
USER doesn't set additional groups
--iidfile is not enabled in CLI
No way to export remote build cache. Build cache exported with standalone BuildKit can be imported with --cache-from.

@tiborvass @AkihiroSuda

depends on #36895

AkihiroSuda · 2018-05-25T03:28:44Z

builder/builder-next/adapters/snapshot/snapshot.go

+var _ snapshot.SnapshotterBase = &snapshotter{}
+
+// NewSnapshotter creates a new snapshotter
+func NewSnapshotter(opt Opt) (snapshot.SnapshotterBase, error) {


Can we split this to a separate package that wraps Moby graphdriver as a containerd snapshotter?

cc @dmcgowan

Currently, the issue is that containerd mounts are stateless while some graphdrives can't produce stateless mounts. @dmcgowan has a plan for that for Moby integration that is somewhat similar to the buildkit/snapshot.Mountable abstraction.

AkihiroSuda · 2018-05-25T03:30:06Z

builder/builder-next/exporter/writer.go

@@ -0,0 +1,179 @@
+package containerimage


Can we dedupe writer.go with https://github.com/moby/buildkit/blob/master/exporter/containerimage/writer.go via some util pkg?

The reason I couldn't use it directly was that buildkit uses the blobsum-diffid pairs here while Moby doesn't yet have an understanding of managing layer blobs. But with a careful refactor, some parts of it can definitely be shared.

AkihiroSuda · 2018-05-25T03:31:43Z

api/types/client.go

+	BuilderV1 BuilderVersion = "1"
+	// BuilderBuildKit is builder based on moby/buildkit project
+	BuilderBuildKit = "2"
+)


Nit: can we just use strings rather than numbers? e.g. legacy and buildkit

@AkihiroSuda I don't like the word legacy (which is what it was initially). With all the respect I have for buildkit, with time everything becomes legacy and eventually you'll have to differentiate between legacy legacy and legacy. This problem is solved with versions, so I believe it's a better tradeoff this way. Happy to hear your concerns though.

AkihiroSuda · 2018-05-25T03:32:34Z

api/types/client.go

@@ -181,8 +181,20 @@ type ImageBuildOptions struct {
 	Target      string
 	SessionID   string
 	Platform    string
+	Version     BuilderVersion
+	BuildID     string


Could you add godoc?

vdemeester

On high level, definitely Design LGTM 😻

In this PR it seems it's not possible to choose a different frontend yet right ? if yes, is it something we plan to allow before going out of experimental ?
I see the gateway.v0 frontend is there so it seems it's already possible to use custom images as frontend (which is awesome 💓) but I'm not sure how to specify it from the API standpoint.

Also, should we open a PR in docker/cli to discuss the cli design in parallel ; and also easily have a docker binary built to make manual testing easier (without having to clone and build the cli)

tonistiigi · 2018-05-25T17:19:21Z

In this PR it seems it's not possible to choose a different frontend yet right ? if yes, is it something we plan to allow before going out of experimental ? I see the gateway.v0 frontend is there so it seems it's already possible to use custom images as frontend (which is awesome 💓) but I'm not sure how to specify it from the API standpoint.

Custom Dockerfile implementations can be used with a Dockerfile directive moby/buildkit#384 but generic frontends (or raw LLB) is not currently enabled in HTTP API. As this would be a new feature, it probably makes sense to do discuss/PR that separately. It may definitely happen in experimental is maintainers agree.

Also, should we open a PR in docker/cli to discuss the cli design in parallel ; and also easily have a docker binary built to make manual testing easier (without having to clone and build the cli)

There's still some cleanup stuff that needs to happen there. You can expect a PR early next week.

codecov · 2018-05-26T00:36:57Z

Codecov Report

❗ No coverage uploaded for pull request base (master@0c4ed4e). Click here to learn what that means.
The diff coverage is 2.77%.

@@            Coverage Diff            @@
##             master   #37151   +/-   ##
=========================================
  Coverage          ?   35.33%           
=========================================
  Files             ?      609           
  Lines             ?    45000           
  Branches          ?        0           
=========================================
  Hits              ?    15901           
  Misses            ?    26947           
  Partials          ?     2152

cpuguy83 · 2018-05-30T20:29:28Z

Design LGTM

thaJeztah · 2018-06-07T23:48:34Z

vendor.conf

@@ -131,7 +135,7 @@ github.com/google/certificate-transparency d90e65c3a07988180c5b1ece71791c0b65068
 golang.org/x/crypto 1a580b3eff7814fc9b40602fd35256c63b50f491
 golang.org/x/time a4bde12657593d5e90d0533a3e4fd95e635124cb
 github.com/hashicorp/go-memdb cb9a474f84cc5e41b273b20c6927680b2a8776ad
-github.com/hashicorp/go-immutable-radix 8e8ed81f8f0bf1bdd829593fdd5c29922c1ea990
+github.com/hashicorp/go-immutable-radix 826af9ccf0feeee615d546d69b11f8e98da8c8f1 git://github.com/tonistiigi/go-immutable-radix.git


Is this something we can upstream? I didn't see a pull request; https://github.com/hashicorp/go-immutable-radix/pulls (looks like its this change; tonistiigi/go-immutable-radix@826af9c)

Not sure if they are interested and the project isn't very active. I can make a proper fork if needed (although we use this project for swarmkit as well). There is no functionality divergence, just an extra method that is only used by buildkit.

We could try if they accept the PR, then we'd be set (unless we think we'll be adding more features and a fork would help)

Ok, I'll make sure to do that and replace if it gets merged

thaJeztah · 2018-06-07T23:55:52Z

vendor.conf

@@ -31,6 +31,10 @@ github.com/moby/buildkit 43e758232a0ac7d50c6a11413186e16684fc1e4f
 github.com/tonistiigi/fsutil dc68c74458923f357474a9178bd198aa3ed11a5f
 github.com/grpc-ecosystem/grpc-opentracing 8e809c8a86450a29b90dcc9efbf062d0fe6d9746
 github.com/opentracing/opentracing-go 1361b9cd60be79c4c3a7fa9841b3c132e40066a7
+github.com/google/shlex 6f45313302b9c56850fc17f99e40caebce98c716
+github.com/opentracing-contrib/go-stdlib  b1a47cfbdd7543e70e9ef3e73d0802ad306cc1cc
+github.com/BurntSushi/locker a6e239ea1c69bff1cfdb20c4b73dadf52f784b6a


This is using UNLICENSE; looks like that could have some implications (but IANAL); https://softwareengineering.stackexchange.com/a/147120, docopt/docopt.rs#1

tiborvass

LGTM

tiborvass · 2018-06-11T16:41:18Z

It's really unfortunate to have all these unrelated CI issues.

AkihiroSuda

LGTM if the regression is fixed now.

tiborvass · 2018-06-12T18:53:52Z

@AkihiroSuda yes, regression was on docker CLI.

vdemeester

LGTM 🦁

tiborvass · 2018-06-12T20:31:37Z

CI for powerpc and s390x are known to have issues /cc @andrewhsu @seemethere.

lowenna · 2018-06-13T16:08:19Z

This has broken master on Windows:

PS C:\> dockerd
INFO[2018-06-13T09:04:23.208583500-07:00] Windows default isolation mode: process
INFO[2018-06-13T09:04:23.294671100-07:00] Loading containers: start.
INFO[2018-06-13T09:04:23.298646300-07:00] Restoring existing overlay networks from HNS into docker
INFO[2018-06-13T09:04:23.399254200-07:00] Loading containers: done.
INFO[2018-06-13T09:04:23.402261600-07:00] Docker daemon                                 commit=692df4699c graphdriver(s)=windowsfilter version=0.0.0-dev
INFO[2018-06-13T09:04:23.405258500-07:00] Daemon has completed initialization
failed to find git binary: exec: "git": executable file not found in %PATH%
PS C:\>

tiborvass · 2018-06-13T16:50:29Z

@jhowardmsft Thanks. Sorry about that. Windows CI was green :(

lowenna · 2018-06-13T17:01:03Z

@tiborvass Yes, the CI servers have (and need) git installed :/

api/server/backend/build/backend.go

@@ -8,10 +8,12 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/builder"
+	buildkit "github.com/docker/docker/builder/builder-next"


tonistiigi requested review from dnephin, tianon and vdemeester as code owners May 25, 2018 03:13

GordonTheTurtle added the status/0-triage label May 25, 2018

tiborvass added status/1-design-review and removed status/0-triage labels May 25, 2018

AkihiroSuda mentioned this pull request May 25, 2018

Proposal: DAG image builder (can execute muti-stage build in parallel) #32550

Closed

AkihiroSuda reviewed May 25, 2018

View reviewed changes

tonistiigi mentioned this pull request May 25, 2018

dockerfile: USER doesn't set additional group moby/buildkit#412

Closed

vdemeester mentioned this pull request May 25, 2018

rfc: move client/types from api/types… #37000

Closed

5 tasks

vdemeester reviewed May 25, 2018

View reviewed changes

tonistiigi added the rebuild/janky label May 25, 2018

GordonTheTurtle removed the rebuild/janky label May 25, 2018

moby deleted a comment from codecov bot May 25, 2018

tonistiigi added the rebuild/janky label May 26, 2018

GordonTheTurtle removed the rebuild/janky label May 26, 2018

vdemeester added status/2-code-review and removed status/1-design-review labels May 30, 2018

tonistiigi force-pushed the experimental-buildkit branch from 3a6c7d4 to 71b7cd5 Compare June 4, 2018 23:31

tiborvass mentioned this pull request Jun 5, 2018

Support for experimental BuildKit docker/cli#1111

Merged

tonistiigi force-pushed the experimental-buildkit branch 2 times, most recently from 4937e74 to b00dc69 Compare June 6, 2018 22:07

thaJeztah reviewed Jun 7, 2018

View reviewed changes

thaJeztah requested changes Jun 8, 2018

View reviewed changes

tiborvass approved these changes Jun 11, 2018

View reviewed changes

tiborvass added rebuild/windowsRS1 labels Jun 11, 2018

GordonTheTurtle removed rebuild/experimental labels Jun 11, 2018

AkihiroSuda approved these changes Jun 12, 2018

View reviewed changes

vdemeester approved these changes Jun 12, 2018

View reviewed changes

tiborvass merged commit c752b09 into moby:master Jun 12, 2018

This was referenced Jun 13, 2018

Dockerd no longer starts if git is not installed after buildkit merge #37278

Closed

Don't fail to start daemon if builder source is not available #37280

Merged

thaJeztah mentioned this pull request Jun 16, 2018

Add ability to only build the stages that the target stage depends on #37297

Closed

wking mentioned this pull request Jun 22, 2018

Document Unlicense compatibility issues? github/choosealicense.com#591

Closed

RytisLT mentioned this pull request Sep 24, 2018

Error on cache query moby/buildkit#569

Closed

ybogdanov mentioned this pull request Jan 7, 2019

Pointers to other tools? Alternatives to MOUNT? grammarly/rocker#199

Closed

thaJeztah mentioned this pull request Jul 8, 2021

Update /system/df example swagger response #42605

Closed

thaJeztah added the impact/api label Sep 2, 2022

thaJeztah mentioned this pull request Jan 8, 2024

builder-next: builder uses ChecksumForGraphID, which always produces error (?) #47040

Open

smoookeeey reviewed Mar 7, 2024

View reviewed changes

api/server/backend/build/backend.go

@@ -8,10 +8,12 @@ import (

"github.com/docker/docker/api/types"

"github.com/docker/docker/api/types/backend"

"github.com/docker/docker/builder"

buildkit "github.com/docker/docker/builder/builder-next"

This comment was marked as spam.

Sign in to view

Dramaga11 mentioned this pull request Apr 29, 2024

gitHub.com/docker/repo electron/electron#41989

Closed

thaJeztah added the docs/revisit label Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental BuildKit support #37151

Experimental BuildKit support #37151

tonistiigi commented May 25, 2018 •

edited

Loading

AkihiroSuda May 25, 2018

tonistiigi May 25, 2018 •

edited

Loading

AkihiroSuda May 25, 2018

tonistiigi May 25, 2018

AkihiroSuda May 25, 2018

tiborvass May 25, 2018

AkihiroSuda May 25, 2018

vdemeester left a comment

tonistiigi commented May 25, 2018

codecov bot commented May 26, 2018 •

edited

Loading

cpuguy83 commented May 30, 2018

thaJeztah Jun 7, 2018

tonistiigi Jun 8, 2018

thaJeztah Jun 8, 2018

tonistiigi Jun 8, 2018

thaJeztah Jun 7, 2018

tonistiigi Jun 9, 2018

tiborvass left a comment

tiborvass commented Jun 11, 2018

AkihiroSuda left a comment

tiborvass commented Jun 12, 2018

vdemeester left a comment

tiborvass commented Jun 12, 2018

lowenna commented Jun 13, 2018

tiborvass commented Jun 13, 2018

lowenna commented Jun 13, 2018

This comment was marked as spam.

Experimental BuildKit support #37151

Experimental BuildKit support #37151

Conversation

tonistiigi commented May 25, 2018 • edited Loading

Choose a reason for hiding this comment

tonistiigi May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

tonistiigi commented May 25, 2018

codecov bot commented May 26, 2018 • edited Loading

Codecov Report

cpuguy83 commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborvass left a comment

Choose a reason for hiding this comment

tiborvass commented Jun 11, 2018

AkihiroSuda left a comment

Choose a reason for hiding this comment

tiborvass commented Jun 12, 2018

vdemeester left a comment

Choose a reason for hiding this comment

tiborvass commented Jun 12, 2018

lowenna commented Jun 13, 2018

tiborvass commented Jun 13, 2018

lowenna commented Jun 13, 2018

This comment was marked as spam.

tonistiigi commented May 25, 2018 •

edited

Loading

tonistiigi May 25, 2018 •

edited

Loading

codecov bot commented May 26, 2018 •

edited

Loading