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

Upgrade to golang-with-libgit2:1.1.1.6 and use static libraries for in development #562

Merged
merged 7 commits into from
Feb 8, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Jan 26, 2022

Main changes:

  • Bump to golang-with-libgit2:1.1.1.6 to speed up build time when cross compiling. Previous version was compiling in emulation mode instead, which added +10x overhead.
  • Ensure that make test is executed against the exact same libraries that will be shipped on the built image.
  • Simplify Makefile to reduce its complexity.
  • Libgit2 behaviour:
    • linux-amd64 download static libraries from the official container image.
    • linux-arm64 on top of the above, requires static musl tool chain (automatically downloaded).
    • darwin-amd64 and darwin-arm64 download universal static libraries for darwin from https://github.com/fluxcd/golang-with-libgit2 releases.

Depends on:

Fix: #568

@pjbgf pjbgf force-pushed the test-alpine branch 2 times, most recently from 95ab956 to 3a14cb8 Compare January 26, 2022 19:17
@pjbgf pjbgf marked this pull request as draft January 26, 2022 19:29
@pjbgf pjbgf force-pushed the test-alpine branch 6 times, most recently from 1870e29 to cd5eefe Compare January 26, 2022 23:06
@pjbgf pjbgf changed the title Change test image to alpine Update make test to use static libraries Jan 26, 2022
@pjbgf pjbgf marked this pull request as ready for review January 26, 2022 23:20
@stefanprodan
Copy link
Member

This fails on Apple Silicon with:

$ uname -a
Darwin stefan-m1-max.lan 21.2.0 Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:41 PST 2021; root:xnu-8019.61.5~1/RELEASE_ARM64_T6000 arm64

$ make test
IMG_TAG=ghcr.io/fluxcd/golang-with-libgit2:libgit2-1.1.1-4 ./hack/extract-libraries.sh
+ IMG_TAG=ghcr.io/fluxcd/golang-with-libgit2:libgit2-1.1.1-4
+ setup_current
+ '[' -d ./hack/libgit2 ']'
+ DIR=x86_64-alpine-linux-musl
+ PLATFORM=linux/amd64
++ uname -m
+ '[' arm64 = armv7l ']'
++ uname -m
+ '[' arm64 = aarch64 ']'
+ setup linux/amd64 x86_64-alpine-linux-musl
+ PLATFORM=linux/amd64
+ DIR=x86_64-alpine-linux-musl
+ extract linux/amd64 x86_64-alpine-linux-musl
+ PLATFORM=linux/amd64
+ DIR=x86_64-alpine-linux-musl
++ docker create --platform=linux/amd64 ghcr.io/fluxcd/golang-with-libgit2:libgit2-1.1.1-4
+ id=1a11d719c1ebf1b0b014a457670c22388b736a3c9926546133b69b2f738949c4
+ docker cp 1a11d719c1ebf1b0b014a457670c22388b736a3c9926546133b69b2f738949c4:/usr/local -
+ docker rm -v 1a11d719c1ebf1b0b014a457670c22388b736a3c9926546133b69b2f738949c4
1a11d719c1ebf1b0b014a457670c22388b736a3c9926546133b69b2f738949c4
+ tar -xf output.tar.gz local/x86_64-alpine-linux-musl --strip-component=1
tar: --strip-component=1: Not found in archive
tar: Error exit delayed from previous errors.
make: *** [Makefile:146: /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/hack/libgit2/lib/libgit2.a] Error 1

@pjbgf
Copy link
Member Author

pjbgf commented Jan 27, 2022

This fails on Apple Silicon with:

@stefanprodan thank you for testing, the output was quite useful. Do you mind testing it again with the latest change?

@stefanprodan
Copy link
Member

Now it fails with:

Status: Downloaded newer image for ghcr.io/fluxcd/golang-with-libgit2:libgit2-1.1.1-4
+ id=c48e7a256f3261ed1baae1cf1a96493bd828b3c9274d0932eadf2e2f10da32b0
+ docker cp c48e7a256f3261ed1baae1cf1a96493bd828b3c9274d0932eadf2e2f10da32b0:/usr/local -
+ docker rm -v c48e7a256f3261ed1baae1cf1a96493bd828b3c9274d0932eadf2e2f10da32b0
c48e7a256f3261ed1baae1cf1a96493bd828b3c9274d0932eadf2e2f10da32b0
+ tar -xf output.tar.gz local/aarch64-alpine-linux-musl --strip-component=1
tar: --strip-component=1: Not found in archive
tar: Error exit delayed from previous errors.
make: *** [Makefile:146: /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/hack/libgit2/lib/libgit2.a] Error 1

@pjbgf pjbgf force-pushed the test-alpine branch 2 times, most recently from 990099e to 75dee8f Compare January 27, 2022 12:15
@pjbgf pjbgf marked this pull request as draft January 27, 2022 20:38
@stefanprodan
Copy link
Member

75dee8f fails with:

++ docker create --platform=linux/arm64 ghcr.io/fluxcd/golang-with-libgit2:libgit2-1.1.1-4
+ id=91f36e778f451431b3f3bc5289040f5f8d8dc9fbc83d4140f39d378654479207
+ docker cp 91f36e778f451431b3f3bc5289040f5f8d8dc9fbc83d4140f39d378654479207:/usr/local -
+ docker rm -v 91f36e778f451431b3f3bc5289040f5f8d8dc9fbc83d4140f39d378654479207
91f36e778f451431b3f3bc5289040f5f8d8dc9fbc83d4140f39d378654479207
+ tar -xf output.tar.gz local/aarch64-alpine-linux-musl
+ rm output.tar.gz
++ /bin/pwd
+ NEW_DIR=/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2
+ INSTALLED_DIR=/usr/local/aarch64-alpine-linux-musl
+ mkdir -p ./build
+ mv local/aarch64-alpine-linux-musl libgit2
+ rm -rf local
+ mv libgit2/ ./build
+ [[ darwin21.1.0 == \d\a\r\w\i\n* ]]
+ find /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2 -type f -name '*.pc'
+ xargs -I '{}' sed -i '' 's;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g' '{}'
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
sed: can't read s;/usr/local/aarch64-alpine-linux-musl;/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2;g: No such file or directory
make: *** [Makefile:151: /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2/lib/libgit2.a] Error 1

@pjbgf pjbgf force-pushed the test-alpine branch 9 times, most recently from eb5f1ae to a923945 Compare February 1, 2022 08:26
@pjbgf
Copy link
Member Author

pjbgf commented Feb 1, 2022

Running make test in the arm64 runners fail, it seems it cannot install envtest:

mkdir -p /home/ubuntu/source-controller/_work/source-controller/source-controller/testbin
/home/ubuntu/source-controller/_work/source-controller/source-controller/bin/setup-envtest use latest --arch=arm64 --bin-dir=/home/ubuntu/source-controller/_work/source-controller/source-controller/testbin
make: /home/ubuntu/source-controller/_work/source-controller/source-controller/bin/setup-envtest: Command not found
make: *** [Makefile:163: install-envtest] Error 127
Error: Process completed with exit code 2.

I initially thought the problem was the temporary folder was failing to be created, but then even after changing it to create the files at ./build/tmp/ it still errors. I removed the make test for the time being, but the previous ran can be seen here:
Source: https://github.com/fluxcd/source-controller/runs/5018039199?check_suite_focus=true

I executed the same command on a arm/v8 machine (rpi 4), and this worked fine (apart from some disk related tests):

root@rpi-k8s-apiserver:~/go/src/source-controller# make test
./hack/download-musl.sh
No package 'libgit2' found
+ MUSL_X86_64_FILENAME=x86_64-linux-musl-native.tgz
+ MUSL_X86_64_SHA512=44d441ad9aa11a06feddf3daa4c9f53ad7d9ca37af1f5a61379aca07793703d179410cea723c1b7fca94c4de19a321228bdb3656bc5cbdb5e3bea8e2d6dac6c7
+ MUSL_AARCH64_FILENAME=aarch64-linux-musl-native.tgz
+ MUSL_AARCH64_SHA512=16d544e09845c9dbba50f29e0cb04dd661e17eb63c56acad6a67fd2a78aa7596b792477c7177d3cd56d408a27dc291a90507df882f2b099c0f25511ce08fd3b5
+ MUSL_XX86_64_FILENAME=x86_64-linux-musl-cross.tgz
+ MUSL_XX86_64_SHA512=52abd1a56e670952116e35d1a62e048a9b6160471d988e16fa0e1611923dd108a581d2e00874af5eb04e4968b1ba32e0eb449a1f15c3e4d5240ebe09caf5a9f3
+ MUSL_XAARCH64_FILENAME=aarch64-linux-musl-cross.tgz
+ MUSL_XAARCH64_SHA512=8695ff86979cdf30fbbcd33061711f5b1ebc3c48a87822b9ca56cde6d3a22abd4dab30fdcd1789ac27c6febbaeb9e5bde59d79d66552fae53d54cc1377a19272
+ MUSL_XARMV7_FILENAME=armv7l-linux-musleabihf-cross.tgz
+ MUSL_XARMV7_SHA512=1bb399a61da425faac521df9b8d303e60ad101f6c7827469e0b4bc685ce1f3dedc606ac7b1e8e34d79f762a3bfe3e8ab479a97e97d9f36fbd9fc5dc9d7ed6fd1
++ uname -m
+ TARGET_ARCH=aarch64
+ ENV_FILE=false
+ MUSL_FILENAME=
+ MUSL_SHA512=
++ git rev-parse --show-toplevel
+ ROOT_DIR=/root/go/src/source-controller
+ MUSL_DIR=/root/go/src/source-controller/build/musl
++ uname -m
+ '[' aarch64 = aarch64 ']'
+ MUSL_FILENAME=x86_64-linux-musl-native.tgz
+ MUSL_SHA512=44d441ad9aa11a06feddf3daa4c9f53ad7d9ca37af1f5a61379aca07793703d179410cea723c1b7fca94c4de19a321228bdb3656bc5cbdb5e3bea8e2d6dac6c7
+ MUSL_PREFIX=aarch64-linux-musl-native/bin/aarch64-linux-musl
+ '[' aarch64 = arm64 ']'
+ '[' aarch64 = aarch64 ']'
+ MUSL_FILENAME=aarch64-linux-musl-native.tgz
+ MUSL_SHA512=16d544e09845c9dbba50f29e0cb04dd661e17eb63c56acad6a67fd2a78aa7596b792477c7177d3cd56d408a27dc291a90507df882f2b099c0f25511ce08fd3b5
+ mkdir -p /root/go/src/source-controller/build/musl
+ false
+ MUSL_AARCH64_URL=https://more.musl.cc/11.2.1/x86_64-linux-musl/aarch64-linux-musl-native.tgz
+ '[' '!' -f /root/go/src/source-controller/build/musl/bin ']'
+ TARGET_FILE=/root/go/src/source-controller/build/musl/aarch64-linux-musl-native.tgz
+ curl -o /root/go/src/source-controller/build/musl/aarch64-linux-musl-native.tgz -LO https://more.musl.cc/11.2.1/x86_64-linux-musl/aarch64-linux-musl-native.tgz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 81.3M  100 81.3M    0     0  3658k      0  0:00:22  0:00:22 --:--:--  9.9M
+ echo '16d544e09845c9dbba50f29e0cb04dd661e17eb63c56acad6a67fd2a78aa7596b792477c7177d3cd56d408a27dc291a90507df882f2b099c0f25511ce08fd3b5  /root/go/src/source-controller/build/musl/aarch64-linux-musl-native.tgz'
+ sha512sum
3af9db79f469e0e1755b583ecc0a0ea858ff481e0f536b1fe11f4ae0caec50ec93458e1bf8a4f6c4ff73f5310bb155a43ec8a109d8adf196c499f76d2f029bd3  -
+ tar xzf /root/go/src/source-controller/build/musl/aarch64-linux-musl-native.tgz -C /root/go/src/source-controller/build/musl
+ rm /root/go/src/source-controller/build/musl/aarch64-linux-musl-native.tgz
IMG=quay.io/paulinhu/golang-with-libgit2 TAG=libgit2-1.1.1-5 ./hack/install-libraries.sh
+ IMG=quay.io/paulinhu/golang-with-libgit2
+ TAG=libgit2-1.1.1-5
+ IMG_TAG=quay.io/paulinhu/golang-with-libgit2:libgit2-1.1.1-5
+ setup_current
+ '[' -d ./build/libgit2/libgit2-1.1.1-5 ']'
+ DIR=x86_64-alpine-linux-musl
+ PLATFORM=linux/amd64
++ uname -m
+ [[ aarch64 == armv7* ]]
++ uname -m
+ '[' aarch64 = arm64 ']'
++ uname -m
+ '[' aarch64 = aarch64 ']'
+ DIR=aarch64-alpine-linux-musl
+ PLATFORM=linux/arm64
+ setup linux/arm64 aarch64-alpine-linux-musl
+ PLATFORM=linux/arm64
+ DIR=aarch64-alpine-linux-musl
+ extract linux/arm64 aarch64-alpine-linux-musl
+ PLATFORM=linux/arm64
+ DIR=aarch64-alpine-linux-musl
++ docker create --platform=linux/arm64 quay.io/paulinhu/golang-with-libgit2:libgit2-1.1.1-5 sh
+ id=e9af4e2f10e06d502a3a348bf0dd999f8320f1ce32fe586dcb72216349c4cafb
+ docker cp e9af4e2f10e06d502a3a348bf0dd999f8320f1ce32fe586dcb72216349c4cafb:/usr/local -
+ docker rm -v e9af4e2f10e06d502a3a348bf0dd999f8320f1ce32fe586dcb72216349c4cafb
e9af4e2f10e06d502a3a348bf0dd999f8320f1ce32fe586dcb72216349c4cafb
+ tar -xf output.tar.gz local/aarch64-alpine-linux-musl
+ rm output.tar.gz
++ /bin/pwd
+ NEW_DIR=/root/go/src/source-controller/build/libgit2/libgit2-1.1.1-5
+ INSTALLED_DIR=/usr/local/aarch64-alpine-linux-musl
+ mkdir -p ./build/libgit2
+ mv local/aarch64-alpine-linux-musl libgit2-1.1.1-5
+ rm -rf local
+ mv libgit2-1.1.1-5/ ./build/libgit2
+ [[ linux-gnu == \d\a\r\w\i\n* ]]
+ find /root/go/src/source-controller/build/libgit2/libgit2-1.1.1-5 -type f -name '*.pc'
+ xargs -I '{}' sed -i 's;/usr/local/aarch64-alpine-linux-musl;/root/go/src/source-controller/build/libgit2/libgit2-1.1.1-5;g' '{}'
go: creating new go.mod: module tmp
Downloading sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
mkdir -p /root/go/src/source-controller/testbin
/root/go/src/source-controller/bin/setup-envtest use latest --arch=arm64 --bin-dir=/root/go/src/source-controller/testbin
Version: 1.23.3
OS/Arch: linux/arm64
md5: tdZ8ZNgMiHfnOSxBRCPDfg==
Path: /root/go/src/source-controller/testbin/k8s/1.23.3-linux-arm64
cd api; go test ./... -coverprofile cover.out
?   	github.com/fluxcd/source-controller/api/v1beta1	[no test files]
KUBEBUILDER_ASSETS="/root/go/src/source-controller/testbin/k8s/1.23.3-linux-arm64" \
go test -ldflags "-s -w -extldflags \"-static\"" -tags 'netgo,osusergo,static_build' ./... -coverprofile cover.out
?   	github.com/fluxcd/source-controller	[no test files]
ok  	github.com/fluxcd/source-controller/controllers	113.826s	coverage: 51.5% of statements
--- FAIL: TestCopyDirFail_SrcInaccessible (0.01s)
    fs_test.go:166: expected error for CopyDir(/tmp/dep3664845292/dir/src, /tmp/dep3177847000/dst), got none
--- FAIL: TestCopyDirFail_DstInaccessible (0.00s)
    fs_test.go:198: expected error for CopyDir(/tmp/dep2555729132/src, /tmp/dep632725807/dir/dst), got none
--- FAIL: TestCopyDirFailOpen (0.00s)
    fs_test.go:295: expected error for CopyDir(/tmp/dep119539386/src, /tmp/dep119539386/dst), got none
--- FAIL: TestCopyFileFail (0.00s)
    fs_test.go:469: expected error for /tmp/dep1308384666/dir/dir/file, got none
--- FAIL: TestIsDir (0.00s)
    fs_test.go:565: expected false for /tmp/dep3277770781/dir/dir, got true
--- FAIL: TestIsSymlink (0.00s)
    fs_test.go:642: expected false for /tmp/dep1946316438/dir/symlink, got true
FAIL
coverage: 40.3% of statements
FAIL	github.com/fluxcd/source-controller/internal/fs	0.082s
?   	github.com/fluxcd/source-controller/internal/helm	[no test files]
ok  	github.com/fluxcd/source-controller/internal/helm/chart	0.595s	coverage: 85.9% of statements
ok  	github.com/fluxcd/source-controller/internal/helm/getter	0.094s	coverage: 73.1% of statements
ok  	github.com/fluxcd/source-controller/internal/helm/repository	0.424s	coverage: 85.0% of statements
ok  	github.com/fluxcd/source-controller/pkg/gcp	0.113s	coverage: 76.5% of statements
ok  	github.com/fluxcd/source-controller/pkg/git	0.122s	coverage: 100.0% of statements
ok  	github.com/fluxcd/source-controller/pkg/git/gogit	1.793s	coverage: 76.0% of statements
ok  	github.com/fluxcd/source-controller/pkg/git/libgit2	0.778s	coverage: 75.8% of statements
ok  	github.com/fluxcd/source-controller/pkg/git/strategy	11.821s	coverage: 75.0% of statements
ok  	github.com/fluxcd/source-controller/pkg/git/strategy/proxy	1.988s	coverage: [no statements]
ok  	github.com/fluxcd/source-controller/pkg/sourceignore	0.032s	coverage: 90.7% of statements
FAIL
make: *** [Makefile:84: test] Error 1

Paulo Gomes added 3 commits February 7, 2022 13:00
Fix issues developing in amd64, arm64 and apple silicon

Signed-off-by: Paulo Gomes <[email protected]>
Signed-off-by: Paulo Gomes <[email protected]>
@pjbgf pjbgf force-pushed the test-alpine branch 9 times, most recently from 6a9afb5 to 598c350 Compare February 7, 2022 18:01
@pjbgf pjbgf marked this pull request as ready for review February 7, 2022 18:37
@pjbgf
Copy link
Member Author

pjbgf commented Feb 7, 2022

This has been tested on the physical environments:

  • linux-amd64
  • linux-arm64 (RPI4)
  • darwin-arm64 (Apple Silicon)

Pending the final test that @aryan9600 is going to run on darwin-arm64 (Apple Silicon).

@stefanprodan
Copy link
Member

92718e5 fails with:

+ DIR=libgit2-darwin
++ /bin/pwd
+ NEW_DIR=/Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2/libgit2-1.1.1-6-nostdio
+ INSTALLED_DIR=/Users/runner/work/golang-with-libgit2/golang-with-libgit2/build/libgit2-darwin-amd64
+ tar -xf output.tar.gz
+ rm output.tar.gz
+ mv libgit2-darwin libgit2-1.1.1-6-nostdio
+ mv libgit2-1.1.1-6-nostdio/ ./build/libgit2
++ /bin/pwd
+ sed -i '' 's;-L/Applications/Xcode_.* ;;g' /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2/libgit2-1.1.1-6-nostdio/lib/pkgconfig/libgit2.pc
sed: can't read s;-L/Applications/Xcode_.* ;;g: No such file or directory
make: *** [Makefile:186: /Users/stefanprodan/go/src/github.com/fluxcd/source-controller/build/libgit2/libgit2-1.1.1-6-nostdio/lib/libgit2.a] Error 2

Signed-off-by: Paulo Gomes <[email protected]>
@pjbgf pjbgf changed the title Update make test to use static libraries Upgrade to golang-with-libgit2:1.1.1.6 and use static libraries for in development Feb 8, 2022
Comment on lines +61 to +63
mkdir tmp-download; cd tmp-download; go mod init go-download;
GOBIN="${GITHUB_WORKSPACE}/build/gobin" go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
cd ..; rm -rf tmp-download
Copy link
Member Author

Choose a reason for hiding this comment

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

Our arm64 runners don't seem to be able to deal with Makefile's go-install-tool when installing setup-envtest. Although it works fine for all other targets.

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

This was quite something to review, but it looks all good to me!

Awesome job on fixing the wiring on the cross-compile to get the build times back to normal, and a stellar job on solving the build issues on MacOS 🙇 💯

@hiddeco hiddeco added area/ci CI related issues and pull requests area/git Git related issues and pull requests labels Feb 8, 2022
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Tested this on Apple Silicon and works great. Thanks @pjbgf 🏅

@hiddeco hiddeco merged commit e0d0344 into fluxcd:main Feb 8, 2022
@pjbgf pjbgf deleted the test-alpine branch February 8, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci CI related issues and pull requests area/git Git related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails on Apple Silicon
3 participants