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

buildx: speed the language having builtin build #230

Merged
merged 3 commits into from
Nov 18, 2021

Conversation

llhuii
Copy link

@llhuii llhuii commented Nov 3, 2021

We use docker-buildx to build our components images for different
platforms.

But for some languages, such as golang, have good builtin build for
multi platforms, and buildx support that.

In Sedna, we have GM/LC written by golang. This commit supports this
function.

Signed-off-by: llhuii [email protected]

@kubeedge-bot kubeedge-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 3, 2021
@llhuii
Copy link
Author

llhuii commented Nov 3, 2021

Time to make gm image for linux/arm64 in github action(amd64 machine) from 706s => 127s.

before: Publish Docker Images before cross build speed , total 1h 0m 9s

2021-11-01T10:27:02.2002778Z #25 [linux/arm64 builder 6/6] RUN make build WHAT=gm GO_LDFLAGS="" OUT_DIR=_output
2021-11-01T10:36:54.2842595Z #25 706.6 + set +x
2021-11-01T10:36:54.4349622Z #25 DONE 706.6s

after: Publish Docker Images after cross build speed, total 28m 39s

2021-11-01T09:38:31.2355847Z #26 [linux/amd64 builder 8/8] RUN make build WHAT=gm GO_LDFLAGS="" OUT_DIR=_output
2021-11-01T09:38:41.2728924Z #26 127.3 + set +x
2021-11-01T09:38:41.3949048Z #26 DONE 127.5s

@kubeedge-bot kubeedge-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2021
@llhuii
Copy link
Author

llhuii commented Nov 3, 2021

/assign @JimmyYang20
/cc @khalid-jobs

We use docker-buildx to build our components images for different
platforms.

But for some languages, such as golang, have good builtin build for
multi platforms, and buildx support that.
In Sedna, we have GM/LC written by golang. This commit supports this
function.

Signed-off-by: llhuii <[email protected]>
@llhuii llhuii force-pushed the docker-buildx-speed branch from 8efcaac to d629f43 Compare November 3, 2021 02:27
@llhuii
Copy link
Author

llhuii commented Nov 4, 2021

/hold
Because LC has built sqlite3 which requires CGO with CGO_ENABLED=1, and CGO can't support cross-compilation, there are two options to solve it:

  1. No buildx speed for LC, i.e. no speed tag for build/lc/Dockerfile.
  2. Add cross compiler for arm64(in alphine world, https://musl.cc/ can help).

@kubeedge-bot kubeedge-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2021
Because LC has built sqlite3 which requires CGO with CGO_ENABLED=1, and
CGO can't support cross-compilation, this speed improvement can't used
in LC.

Signed-off-by: llhuii <[email protected]>
@llhuii llhuii force-pushed the docker-buildx-speed branch from e6de43d to 88508f8 Compare November 4, 2021 11:25
@kubeedge-bot kubeedge-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 4, 2021
Add a tag '_speed_buildx_for_cgo_alpine' for Dockerfiles:
1. require golang with CGO_ENABLED=1
2. use alpine image.

So used in 'build/lc/Dockerfile'

Signed-off-by: llhuii <[email protected]>
@llhuii llhuii force-pushed the docker-buildx-speed branch from 88508f8 to afdd6a2 Compare November 4, 2021 11:45
@llhuii
Copy link
Author

llhuii commented Nov 4, 2021

/hold cancel
Use option 2

/hold Because LC has built sqlite3 which requires CGO with CGO_ENABLED=1, and CGO can't support cross-compilation, there are two options to solve it:

  1. No buildx speed for LC, i.e. no speed tag for build/lc/Dockerfile.
  2. Add cross compiler for arm64(in alphine world, https://musl.cc/ can help).

@kubeedge-bot kubeedge-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2021
@llhuii
Copy link
Author

llhuii commented Nov 4, 2021

In the commit, it dynamically downloads the corresponding platform cross compiler in Dockerfile.

In future, we may build a image dedicated to build/cross-build for golang, then base it to do cross build.

@llhuii
Copy link
Author

llhuii commented Nov 5, 2021

Tested push-images, finished in 32 min
https://github.com/llhuii/sedna/runs/4104540636?check_suite_focus=true

@llhuii
Copy link
Author

llhuii commented Nov 5, 2021

/cc Poorunga

@kubeedge-bot kubeedge-bot requested a review from Poorunga November 5, 2021 03:59
@JimmyYang20
Copy link

/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2021
@llhuii
Copy link
Author

llhuii commented Nov 18, 2021

/approve

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: llhuii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2021
@kubeedge-bot kubeedge-bot merged commit 31812f8 into kubeedge:main Nov 18, 2021
@llhuii llhuii deleted the docker-buildx-speed branch November 23, 2021 03:11
@llhuii
Copy link
Author

llhuii commented Nov 23, 2021

Add more debug info:
new added in build/lc/Dockerfile when cross build:

# generated by sedna::buildx                                                                 

# These ARGs are built in docker build
# see https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope
ARG TARGETPLATFORM BUILDPLATFORM TARGETARCH TARGETOS

RUN echo "I am running on $BUILDPLATFORM, building for $TARGETPLATFORM(=$TARGETOS/$TARGETARCH).."

RUN test "$TARGETPLATFORM" = "$BUILDPLATFORM" || (apk add curl; \
arch=$(echo $TARGETARCH | sed "s@arm64@aarch64@"); \
cc_url=$(curl musl.cc | grep /${arch}-${TARGETOS}-.*-cross); \
cc_filename=$(basename $cc_url)  ; \
cc_name=$(echo $cc_filename | cut -f1 -d.)  ; \
curl -O $cc_url; tar xf $cc_filename; \
cc_bin=$PWD/$cc_name/bin  ; \
export PATH="$cc_bin:$PATH"; \
go env -w CC=$(echo $cc_bin/$arch-$TARGETOS-*-cc) CGO_ENABLED=1; \
go env -w GOOS="$TARGETOS" GOARCH="$TARGETARCH"; \
)

build/gm/Dockerfile:

# generated by sedna::buildx
             
# These ARGs are built in docker build
# see https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope
ARG TARGETPLATFORM BUILDPLATFORM TARGETARCH TARGETOS         

RUN echo "I am running on $BUILDPLATFORM, building for $TARGETPLATFORM(=$TARGETOS/$TARGETARCH).."
                                                                                                          
RUN test "$TARGETPLATFORM" = "$BUILDPLATFORM" || (go env -w GOOS="$TARGETOS" GOARCH="$TARGETARCH"; \
)               
# end generated by sedna::buildx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants