Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Switch to using buildkit with go caching #2731

Closed
wants to merge 1 commit into from

Conversation

randomvariable
Copy link
Contributor

@randomvariable randomvariable commented Jun 23, 2022

What this PR does / why we need it

Makes the following changes:

  • Use buildkit with Docker caching for container builds. This means single line changes do not cause go mod download to re-execute
  • Remove -a from go build options as the compiler produces reproducible builds already.

Signed-off-by: Naadir Jeewa jeewan@vmware.com

Which issue(s) this PR fixes

Fixes #2730

Describe testing done for PR

Release note

Container builds now use buildkit to speed up builds

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@randomvariable randomvariable force-pushed the buildkit branch 4 times, most recently from d362786 to d11af77 Compare June 23, 2022 14:04
@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #2731 (8a8d8bc) into main (e9025ea) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2731      +/-   ##
==========================================
+ Coverage   44.91%   44.98%   +0.06%     
==========================================
  Files         412      412              
  Lines       41733    41733              
==========================================
+ Hits        18745    18774      +29     
+ Misses      21262    21240      -22     
+ Partials     1726     1719       -7     
Impacted Files Coverage Δ
...ons/controllers/packageinstallstatus_controller.go 79.15% <0.00%> (+1.15%) ⬆️
addons/controllers/clusterbootstrap_controller.go 65.57% <0.00%> (+2.29%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@randomvariable randomvariable marked this pull request as ready for review June 23, 2022 15:35
@randomvariable randomvariable requested review from a team and prkalle as code owners June 23, 2022 15:35
addons/pinniped/post-deploy/Dockerfile Show resolved Hide resolved
addons/Dockerfile Outdated Show resolved Hide resolved
@rajathagasthya
Copy link
Member

I just tried this on my Mac running rootless podman and it seems to be having permission issues creating the cache directory.

$ cd pkg/v1/sdk/features
$ make docker-build
cd ../../../../ && docker build -t featuregates-controller-manager:latest -f pkg/v1/sdk/features/Dockerfile --build-arg LD_FLAGS="-s -w -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.Date=$(date -u +"%Y-%m-%d")' -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.SHA=$(git describe --match= --always --dirty)' -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.Version=v0.24.0-dev'" .
[1/2] STEP 1/11: FROM golang:1.17 AS builder
[1/2] STEP 2/11: WORKDIR /workspace
--> Using cache 9cc48d86755de0069fd8afe3926c7ce1124537a4f23b25e2a42248cd9040e81b
--> 9cc48d86755
[1/2] STEP 3/11: COPY go.mod go.mod
--> Using cache 66c02ec87c44b9db02979f35744bf504800851d1e87773e2faf782403a43372b
--> 66c02ec87c4
[1/2] STEP 4/11: COPY go.sum go.sum
--> Using cache 3dca6526b9ef8a7745280c83973ebd7c988e1f715b5ca87ff30a4f4af11d77b8
--> 3dca6526b9e
[1/2] STEP 5/11: RUN  --mount=type=cache,target=/root/.local/share/golang   --mount=type=cache,target=/go/pkg/mod   go mod download
go: writing go.mod cache: mkdir /go/pkg/mod/cache: permission denied
go: writing go.mod cache: mkdir /go/pkg/mod/cache: permission denied

Has this got something to do with podman?

@randomvariable
Copy link
Contributor Author

randomvariable commented Jun 23, 2022

I just tried this on my Mac running rootless podman and it seems to be having permission issues creating the cache directory.

Podman is not fully compatible with buildkit, but this feels like a tradeoff worth making?

Makes the following changes:

* Use buildkit with Docker caching for container builds
  This means single line changes do not cause `go mod download` to
re-execute
* Remove `-a` from go build options as the compiler produces
reproducible builds already.

Signed-off-by: Naadir Jeewa <jeewan@vmware.com>
@randomvariable
Copy link
Contributor Author

This has been rebased

@randomvariable
Copy link
Contributor Author

Testing it on one infra should be sufficient as a test. It will either explode or not.

/test install-aws

@vijaykatam
Copy link
Contributor

Verified build works in docker for mac

➜  addons git:(pr/2731) ✗ IMG_DEFAULT_NAME=projects-stg.registry.vmware.com/tkg/vkatam/addons-controller make docker-build
cd .. && docker build -t projects-stg.registry.vmware.com/tkg/vkatam/addons-controller:v0.26.0-dev-57-g8a8d8bc2 -f addons/Dockerfile --build-arg LD_FLAGS="-s -w -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.Date=$(date -u +"%Y-%m-%d")' -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.SHA=$(git describe --match= --always --dirty)' -X 'github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo.Version=v0.26.0-dev'" .
[+] Building 99.7s (17/17) FINISHED                                                                                                                              
 => [internal] load build definition from Dockerfile                                                                                                        0.0s
 => => transferring dockerfile: 1.20kB                                                                                                                      0.0s
 => [internal] load .dockerignore                                                                                                                           0.0s
 => => transferring context: 34B                                                                                                                            0.0s
 => resolve image config for docker.io/docker/dockerfile:1.4                                                                                                1.7s
 => docker-image://docker.io/docker/dockerfile:1.4@sha256:443aab4ca21183e069e7d8b2dc68006594f40bddf1b15bbd83f5137bd93e80e2                                  0.8s
 => => resolve docker.io/docker/dockerfile:1.4@sha256:443aab4ca21183e069e7d8b2dc68006594f40bddf1b15bbd83f5137bd93e80e2                                      0.0s
 => => sha256:24d064a369eda7bc7839b6c1c227eac7212d06ca09a8235a4bed467f8acf180d 528B / 528B                                                                  0.0s
 => => sha256:84495a15555de1a8f4738f58268fa8949547068198f8d0fa2a3e3a693d7f923f 2.37kB / 2.37kB                                                              0.0s
 => => sha256:09768fef35f2ee387f57e401ae685727d12d1c70c6fd8545a422850167bf1940 9.94MB / 9.94MB                                                              0.5s
 => => sha256:443aab4ca21183e069e7d8b2dc68006594f40bddf1b15bbd83f5137bd93e80e2 2.00kB / 2.00kB                                                              0.0s
 => => extracting sha256:09768fef35f2ee387f57e401ae685727d12d1c70c6fd8545a422850167bf1940                                                                   0.2s
 => [internal] load build definition from Dockerfile                                                                                                        0.0s
 => [internal] load .dockerignore                                                                                                                           0.0s
 => [internal] load metadata for gcr.io/distroless/static:nonroot                                                                                           0.2s
 => [internal] load metadata for docker.io/library/golang:1.17                                                                                              0.0s
 => [internal] load build context                                                                                                                           7.5s
 => => transferring context: 389.80MB                                                                                                                       6.9s
 => CACHED [stage-1 1/3] FROM gcr.io/distroless/static:nonroot@sha256:59d91a17dbdd8b785e61da81c9095b78099cad8d7757cc108f49e4fb564ef8b3                      0.0s
 => [builder 1/5] FROM docker.io/library/golang:1.17                                                                                                        0.0s
 => CACHED [builder 2/5] WORKDIR /workspace                                                                                                                 0.0s
 => [builder 3/5] COPY ./ ./                                                                                                                                3.6s
 => [builder 4/5] RUN  --mount=type=cache,target=/root/.local/share/golang   --mount=type=cache,target=/go/pkg/mod   go mod download                       31.6s
 => [builder 5/5] RUN --mount=type=cache,target=/root/.cache/go-build   --mount=type=cache,target=/go/pkg/mod   --mount=type=cache,target=/root/.local/sh  53.4s
 => [stage-1 2/3] COPY --from=builder /workspace/addons/bin/manager .                                                                                       0.1s 
 => exporting to image                                                                                                                                      0.3s
 => => exporting layers                                                                                                                                     0.2s
 => => writing image sha256:83e4f54c0ad440d18a7e0b8f670389e7842fb1b27eb20c8e5ce62e26b827b050                                                                0.0s
 => => naming to projects-stg.registry.vmware.com/tkg/vkatam/addons-controller:v0.26.0-dev-57-g8a8d8bc2      

Copy link
Contributor

@vijaykatam vijaykatam left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM. Lets merge once we get a passing signal from downstream CI.

@vijaykatam
Copy link
Contributor

/test install-vc7

@randomvariable
Copy link
Contributor Author

Didn't get merged. Don't have bandwidth to rebase.

@jayunit100
Copy link
Contributor

have a WIP copy of this w/ the new dir structure ..... trying to test it now ! thanks Naadir #3972

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why do we build using go build -a and have removed buildkit caching?
5 participants