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

Make docker build faster #1074

Merged
merged 7 commits into from
Mar 18, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Enable docker buildkit in CI
Set DOCKER_BUILDKIT in github actions
  • Loading branch information
georgysavva committed Mar 17, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit fdbe5e1d1339a8ed8888a634d1f8ef61cf673cef
9 changes: 9 additions & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ jobs:
name: autopeering
env:
TEST_NAME: autopeering
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -46,6 +47,7 @@ jobs:
name: common
env:
TEST_NAME: common
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -80,6 +82,7 @@ jobs:
name: consensus
env:
TEST_NAME: consensus
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -115,6 +118,7 @@ jobs:
name: drng
env:
TEST_NAME: drng
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -151,6 +155,7 @@ jobs:
name: message
env:
TEST_NAME: message
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -187,6 +192,7 @@ jobs:
name: value
env:
TEST_NAME: value
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -221,6 +227,7 @@ jobs:
name: faucet
env:
TEST_NAME: faucet
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -255,6 +262,7 @@ jobs:
name: syncbeacon
env:
TEST_NAME: syncbeacon
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

@@ -289,6 +297,7 @@ jobs:
name: mana
env:
TEST_NAME: mana
DOCKER_BUILDKIT: 1
runs-on: ubuntu-latest
steps:

5 changes: 2 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -20,13 +20,12 @@ ENV GO111MODULE=on
RUN go mod download
RUN go mod verify

COPY . .

# 1. Mount everything from the current directory to the PWD(Present Working Directory) inside the container
# 2. Mount the build cache volume
# 3. Build the binary
# 4. Verify that goshimmer binary is statically linked
RUN --mount=type=cache,target=/root/.cache/go-build \
RUN --mount=target=. \
--mount=type=cache,target=/root/.cache/go-build \
Copy link
Contributor

Choose a reason for hiding this comment

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

This should significantly speed up local builds and thus make development for integration tests much less painful. But we won't be able to leverage the cache on GH Actions since this is not persisted anywhere and we'd need to share it across builds as suggested as a "hack" here See this issue moby/buildkit#1512.
Another option would be using one of the techniques described in #890.

But in general I think this is already great and we should maybe first implement static peering to speed up overall execution of the integration tests before investing too much time into CI optimizations/caching

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree with you. Reusing RUN --mount=type=cache in GH actions seems to be hard right now. let's postpone it.
The docker layer caching is pretty doable: here are the official docker GH actions for that: https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md
and a third-party article https://evilmartians.com/chronicles/build-images-on-github-actions-with-docker-layer-caching

Copy link
Contributor

Choose a reason for hiding this comment

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

How is the mounting interacting with the docker build cache?
Before with a COPY . . it was assured that if none of the files (outside of .dockerignore) changed, all the successive build steps were skipped.
Is the mount . doing the same and is it also skipping files dockerignore? I think it could make sense to still keep the COPY for the sources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It provides the same features as COPY . .: if none of the files changed: everything is cached in docker
Yes, it also respects the .dockerignore file.
This tips if from the official Docker article for Go applications https://www.docker.com/blog/containerize-your-go-developer-environment-part-3/:

Performing a COPY will create an extra layer in the container image which slows things down and uses extra disk space. This can be avoided by using RUN --mount and bind mounting from the build context, from a stage, or an image. Adopting this pattern, the resulting Dockerfile is as follows.
The default mount type is a read only bind mount from the context that you pass with the docker build command. This means that you can replace the COPY . . with a RUN --mount=target=. wherever you need the files from your context to run a command but do not need them to persist in the final image.

CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build \
-ldflags='-w -s -extldflags "-static"' \
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you removed the -a (force rebuilding of packages that are already up-to-date.) flag. Was there any specific reason we had this in first place? @Wollac knows maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only reason that I see is to always rebuild everything including the std to be more sure that the resulting binary is statically linked. But build-cache doesn't work in that case, so I disabled it and added an additional check after the build that the resulting binary is indeed statically linked so we could see an error during the build if it's not true for some mysterious reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually add the -a just to be on the safe side. However, the Go compiler should be able to correctly detect necessary rebuilds for everything.

-o /go/bin/goshimmer; \