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

build: Optionally use git for all docker builds with BUILDKIT #11513

Merged

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented May 12, 2020

Make building with DOCKER_BUILDKIT and using a shallow git clone as
the build context opt-in: the developer must define DOCKER_BUILDKIT to
use these. When DOCKER_BUILDKIT is defined, the following takes place:

  1. A separate docker build directory for all docker image targets is
    used. This makes sure the build context only contains the files in git
    and none of the other files that may exist in the working
    directory. '_build' in the repo is used as the root of the build
    directory by default (can be overridden by defining BUILD_DIR). A
    shallow bare git clone of the main repo is created in '_build/.git',
    which is checked out into '_build/context'.

The build context is subsequently synced with git fetch to avoid
overwriting all files. This makes docker build context sync much
faster and allows docker to use cached build stages.

  1. The .git directory is left out of the build context, so that any
    changes in there will not affect docker build stage caching. During
    the build context sync a file '_build/context/.git' points to
    'build.git', but that is removed before the context is handed to
    docker.

  2. BUILD_DIR can be passed in to specify an alternate build root
    (replacing the default '_build'). This is beneficial in keeping the
    main repo directory cleaner, and also to avoid slow file I/O in a
    shared file system (e.g, between the host and a CI VM).

A new make target veryclean that guts docker caches has been added.
This may help resolve the issue where Docker fails to actually load an image
(e.g., cilium-builder) which leads to weird issues like /bin/sh not found.

Testing this on a MacBook Pro yielded the following results:

  • Clean and build cilium, cilium-docker-plugin, cilium-operator, and hubble-relay

Before these changes:
$ time make docker-image
8m17.558s

After:
$ time make docker-image
5m35.515s

$ DOCKER_BUILDKIT=1 time make docker-image
2m49.41s

Fixes: #11443

@jrajahalme jrajahalme added kind/enhancement This would improve or streamline existing functionality. release-note/misc This PR makes changes that have no direct user impact. area/build Anything to do with the build, more general then area/CI labels May 12, 2020
@jrajahalme jrajahalme requested a review from a team as a code owner May 12, 2020 22:12
@jrajahalme jrajahalme requested a review from a team May 12, 2020 22:12
@jrajahalme jrajahalme requested a review from a team as a code owner May 12, 2020 22:12
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 12, 2020
@jrajahalme
Copy link
Member Author

test-me-please

@coveralls
Copy link

coveralls commented May 12, 2020

Coverage Status

Coverage decreased (-0.01%) to 37.08% when pulling d95f85f on pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds into 5aee704 on master.

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch from 5b899d0 to 232658f Compare May 13, 2020 04:34
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

Failed on K8s-1.11-Kernel-netnext:

22:15:37  failed to solve with frontend dockerfile.v0: failed to solve with frontend
gateway.v0: rpc error: code = Unknown desc = failed to build LLB: executor failed
running [/bin/sh -c groupadd -f cilium     && echo ". /etc/profile.d/bash_completion.sh"
>> /etc/bash.bashrc]: runc did not terminate sucessfully

I did see this once locally as well, maybe DOCKER_BUILDKIT is not good for prime time yet?

@@ -1,6 +1,9 @@
#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to ask, why the # on top of all files?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes it a bit easier to add the required stanza to the first line without risk of doing it if the Dockerfile already contains it.

@jrajahalme jrajahalme marked this pull request as draft May 13, 2020 15:48
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch from 232658f to 9e183d2 Compare May 15, 2020 08:24
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch from 9e183d2 to 5999d7a Compare May 15, 2020 17:16
@jrajahalme
Copy link
Member Author

test-me-please

1 similar comment
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch 2 times, most recently from 0ed7685 to 00f14f3 Compare May 16, 2020 03:09
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch from 00f14f3 to f4d9fd2 Compare May 16, 2020 03:23
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme marked this pull request as ready for review May 18, 2020 19:28
@jrajahalme jrajahalme requested a review from a team as a code owner May 18, 2020 19:28
@jrajahalme jrajahalme requested a review from aanm May 18, 2020 19:29
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch from f4d9fd2 to 10c0579 Compare May 18, 2020 19:41
@jrajahalme
Copy link
Member Author

test-me-please

Makefile Outdated
ifdef DOCKER_BUILDKIT
# Export with value expected by docker
export DOCKER_BUILDKIT=1
BUILDKIT_DOCKERFILE_FILTER := | sed -e "1s|^\#.*|\# syntax = docker/dockerfile:experimental|" -e "s|^RUN\(.*\)make|RUN --mount=type=cache,target=/root/.cache/go-build\1make|"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would much rather see this added permanently, what earliest version of Docker supports this option?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spoke with @nebril and it sounds like upgrading Docker in Jenkins wouldn't be a big deal :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev VM has docker 19.03 and it still has a bug where it sometimes fails to pull the base image on the FROM line, behaving as if it was pulled, but then "make" or "/bin/sh" can't be found (see @joestringer's comments below). At the least, enabling DOCKER_BUILDKIT in the CI takes some testing in addition updates (if needed).

@jrajahalme
Copy link
Member Author

@tklauser This is ready to merge.

@joestringer joestringer merged commit cfeaf6e into master May 20, 2020
1.8.0 automation moved this from In progress to Merged May 20, 2020
@joestringer joestringer deleted the pr/jrajahalme/docker-use-buildkit-also-for-non-dev-builds branch May 20, 2020 20:33
jrajahalme added a commit that referenced this pull request Jun 16, 2020
Remove support for user-defined build dir paths when using Docker
buildkit. This was dangerous as there was no easy way to make sure the
user supplied paths were pointing to directories that can be safely
deleted and overwritten.

Expand the BUILD_DIR and DOCKER_BUILD_DIR in Makefile.buildkit to make
it easier to understand and to protect against accidental damage when
BUILD_DIR = '.', which would mess up user's main git repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
jrajahalme added a commit that referenced this pull request Jun 16, 2020
Docker buildkit often fails to actaully pull the FROM image and then
fails wit errors like:

 > [builder 4/4] RUN make ... clean-container build-container install-container:
#13 0.344 /bin/sh: 1: make: not found

Fix this by pulling all FROM images before docker build. This depends
on the FROM image names not be based on build ARGs, which is the case
for all the current Dockerfiles in the repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
jrajahalme added a commit that referenced this pull request Jun 16, 2020
'make' builds the first target, make sure 'all' is the first target
regardless what is defined in the included makefiles.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
jrajahalme added a commit that referenced this pull request Jun 16, 2020
Use BPF_SRCFILES to compute CILIUM_DATAPATH_SHA as BPF_FILES is not
available in the buildkit context due to buildkit context not having
the '.git' directory.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
jrajahalme added a commit that referenced this pull request Jun 16, 2020
_build directory may already exist, and the shallow clone may already
have been applied, so don't fail if git clone fails.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
nebril pushed a commit that referenced this pull request Jun 20, 2020
Remove support for user-defined build dir paths when using Docker
buildkit. This was dangerous as there was no easy way to make sure the
user supplied paths were pointing to directories that can be safely
deleted and overwritten.

Expand the BUILD_DIR and DOCKER_BUILD_DIR in Makefile.buildkit to make
it easier to understand and to protect against accidental damage when
BUILD_DIR = '.', which would mess up user's main git repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
nebril pushed a commit that referenced this pull request Jun 20, 2020
Docker buildkit often fails to actaully pull the FROM image and then
fails wit errors like:

 > [builder 4/4] RUN make ... clean-container build-container install-container:
#13 0.344 /bin/sh: 1: make: not found

Fix this by pulling all FROM images before docker build. This depends
on the FROM image names not be based on build ARGs, which is the case
for all the current Dockerfiles in the repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
nebril pushed a commit that referenced this pull request Jun 20, 2020
'make' builds the first target, make sure 'all' is the first target
regardless what is defined in the included makefiles.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
nebril pushed a commit that referenced this pull request Jun 20, 2020
Use BPF_SRCFILES to compute CILIUM_DATAPATH_SHA as BPF_FILES is not
available in the buildkit context due to buildkit context not having
the '.git' directory.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
nebril pushed a commit that referenced this pull request Jun 20, 2020
_build directory may already exist, and the shallow clone may already
have been applied, so don't fail if git clone fails.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
aanm pushed a commit that referenced this pull request Jun 21, 2020
[ upstream commit 4167a4c ]

Remove support for user-defined build dir paths when using Docker
buildkit. This was dangerous as there was no easy way to make sure the
user supplied paths were pointing to directories that can be safely
deleted and overwritten.

Expand the BUILD_DIR and DOCKER_BUILD_DIR in Makefile.buildkit to make
it easier to understand and to protect against accidental damage when
BUILD_DIR = '.', which would mess up user's main git repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 21, 2020
[ upstream commit ae5d7c1 ]

Docker buildkit often fails to actaully pull the FROM image and then
fails wit errors like:

 > [builder 4/4] RUN make ... clean-container build-container install-container:
#13 0.344 /bin/sh: 1: make: not found

Fix this by pulling all FROM images before docker build. This depends
on the FROM image names not be based on build ARGs, which is the case
for all the current Dockerfiles in the repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 21, 2020
[ upstream commit 8eced25 ]

'make' builds the first target, make sure 'all' is the first target
regardless what is defined in the included makefiles.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 21, 2020
[ upstream commit 959de2b ]

Use BPF_SRCFILES to compute CILIUM_DATAPATH_SHA as BPF_FILES is not
available in the buildkit context due to buildkit context not having
the '.git' directory.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 21, 2020
[ upstream commit a7293ef ]

_build directory may already exist, and the shallow clone may already
have been applied, so don't fail if git clone fails.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 22, 2020
[ upstream commit 4167a4c ]

Remove support for user-defined build dir paths when using Docker
buildkit. This was dangerous as there was no easy way to make sure the
user supplied paths were pointing to directories that can be safely
deleted and overwritten.

Expand the BUILD_DIR and DOCKER_BUILD_DIR in Makefile.buildkit to make
it easier to understand and to protect against accidental damage when
BUILD_DIR = '.', which would mess up user's main git repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 22, 2020
[ upstream commit ae5d7c1 ]

Docker buildkit often fails to actaully pull the FROM image and then
fails wit errors like:

 > [builder 4/4] RUN make ... clean-container build-container install-container:
#13 0.344 /bin/sh: 1: make: not found

Fix this by pulling all FROM images before docker build. This depends
on the FROM image names not be based on build ARGs, which is the case
for all the current Dockerfiles in the repo.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 22, 2020
[ upstream commit 8eced25 ]

'make' builds the first target, make sure 'all' is the first target
regardless what is defined in the included makefiles.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 22, 2020
[ upstream commit 959de2b ]

Use BPF_SRCFILES to compute CILIUM_DATAPATH_SHA as BPF_FILES is not
available in the buildkit context due to buildkit context not having
the '.git' directory.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jun 22, 2020
[ upstream commit a7293ef ]

_build directory may already exist, and the shallow clone may already
have been applied, so don't fail if git clone fails.

Fixes: #11513
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Signed-off-by: André Martins <andre@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Anything to do with the build, more general then area/CI kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants