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

Multiarch enablement on Dockerfile and Dockerfile.build #538

Merged
merged 10 commits into from
Apr 18, 2022

Conversation

aleskandro
Copy link
Contributor

An alternative that allows installing golang-ci-lint as before also on ARM64 is by bumping to golang:1.18. See the issue above.

cc @yselkowitz

Refers ARMOCP-293

@codeclimate
Copy link

codeclimate bot commented Mar 29, 2022

Code Climate has analyzed commit 3f0fba6 and detected 0 issues on this pull request.

View more on Code Climate.

@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #538 (3f0fba6) into master (1219538) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #538   +/-   ##
======================================
  Coverage    86.7%   86.7%           
======================================
  Files          26      26           
  Lines        3556    3556           
======================================
  Hits         3082    3082           
  Misses        328     328           
  Partials      146     146           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1219538...3f0fba6. Read the comment docs.

@ldemailly
Copy link
Member

ldemailly commented Mar 31, 2022

Thanks for this!

Changes golangci-lint installation due to golangci/golangci-lint#2374

I scanned the issue and it seem to relate to go 1.18 not 1.17?

Are you sure https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh doesn't work?
(do you have a sample ci/example of error from your fork?)

In general I prefer ubuntu to debian/alpine/... but I guess it doesn't matter too much if in the end the from scratch images perform and are still small

I don't see this plugged in into the actual https://github.com/fortio/fortio/blob/master/release/release.sh though or
https://github.com/fortio/fortio/blob/master/.github/workflows/main.yml#L38-L43

to create multiple images or is that somehow automatic and will create multiple images selectable by arch?
(I'm actually not familiar with multi arch docker if you have a faq/doc pointer for me)

@ldemailly ldemailly self-requested a review March 31, 2022 22:59
@aleskandro
Copy link
Contributor Author

aleskandro commented Apr 4, 2022

Hello @ldemailly,

I scanned the issue and it seem to relate to go 1.18 not 1.17?

Are you sure https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh doesn't work?
(do you have a sample ci/example of error from your fork?)

building with go 1.17.z and using the previous script fails, at least, on ARM. We could either bump to go1.18 and use the script to install golang-ci-lint, or we can keep on go1.17 but using the go install way for that. I tried to be minimal in changes. What do you think? See error below.

In general I prefer ubuntu to debian/alpine/... but I guess it doesn't matter too much if in the end the from scratch images perform and are still small.

I did it to not delegate here the responsibility to keep these lines in sync for each architecture.

I don't see this plugged in into the actual https://github.com/fortio/fortio/blob/master/release/release.sh though or
https://github.com/fortio/fortio/blob/master/.github/workflows/main.yml#L38-L43

to create multiple images or is that somehow automatic and will create multiple images selectable by arch?
(I'm actually not familiar with multi arch docker if you have a faq/doc pointer for me)

I'm pushing right now another commit that also enables the CI for multi-arch by using Github actions as you are doing already. Note that Github actions only provide x86_64 servers to run the jobs. Hence, the only way I got to make it architecture-aware is by using docker buildx, and cross-building capabilities from the golang compiler and qemu.

Here is the summary of the changes:

- Updated Dockerfiles for cross-building release artifacts
- fpm packages by arch
- fixes Dockerfile.build due to Docker-ce not being available for s390x
- Makefile actions now use docker buildx
- Removing pulling of ubuntu:focal in Makefile: docker pull would not cache other architecture's images
- Windows and MacOS release artifacts are now built in release/Dockerfile.

A successful run of the CI is available at https://github.com/aleskandro/fortio/runs/5818686964?check_suite_focus=true

I needed to update the .build image manually before. Also, the release took a lot time more to build for each arch.

The error from golangci-lint

 => [auth] library/golang:pull token for registry-1.docker.io                                                                                        0.0s
 => [linux/arm64  2/11] RUN apt-get -y update &&   apt-get --no-install-recommends -y upgrade &&   DEBIAN_FRONTEND=noninteractive apt-get --no-in  143.0s
 => [linux/arm64  3/11] RUN gem install --no-document fpm                                                                                           20.8s
 => [linux/arm64  4/11] RUN go version # check it's indeed the version we expect                                                                     0.4s
 => [linux/arm64  5/11] RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b /go/bin             22.4s
 => ERROR [linux/arm64  6/11] RUN golangci-lint version                                                                                             56.2s
 => CANCELED [linux/amd64  2/11] RUN apt-get -y update &&   apt-get --no-install-recommends -y upgrade &&   DEBIAN_FRONTEND=noninteractive apt-get   0.0s
------
 > [linux/arm64  6/11] RUN golangci-lint version:
#0 55.68 panic: load embedded ruleguard rules: rules/rules.go:13: can't load fmt
#0 55.68 
#0 55.68 goroutine 1 [running]:
#0 55.68 github.com/go-critic/go-critic/checkers.init.22()
#0 55.68        github.com/go-critic/go-critic@v0.6.2/checkers/embedded_rules.go:46 +0x488
------
Dockerfile.build:17
--------------------
  15 |     # RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.44.2
  16 |     RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $GOPATH/bin
  17 | >>> RUN golangci-lint version
  18 |     
  19 |     # Docker:
--------------------
error: failed to solve: process "/bin/sh -c golangci-lint version" did not complete successfully: exit code: 2

@aleskandro aleskandro force-pushed the multiarch branch 7 times, most recently from e573d66 to 742edae Compare April 4, 2022 16:32
- Switching build image to golang:1.17.8
- Changes golangci-lint installation due to golangci/golangci-lint#2374
- Building windows/mac binaries only when architecture is amd64 in fortio Dockerfile
- Docker repo is now architecture-aware
- Updated Dockerfiles for cross-building release artifacts
- fpm packages by arch
- fixes Dockerfile.build due to Docker-ce not available for s390x
- Makefile actions now uses docker buildx
- Removing pulling of ubuntu:focal in Makefile: docker pull would not cache other architecture's images
- Windows and MacOS release artifacts are now built in release/Dockerfile.in
@odidev
Copy link

odidev commented Apr 11, 2022

I am working on adding arm64 jobs in CircleCI in the consul repo, which uses Fortio images for envoy tests. These changes are working perfectly for building the arm64 image for Fortio and successfully tested it as well.

It will be helpful if Fortio images are released.

@ldemailly
Copy link
Member

I haven't forgotten, it's just been very busy

@ldemailly
Copy link
Member

I got your branch locally and was trying to make v39 of the build image:

$ make -n update-build-image
/Applications/Xcode.app/Contents/Developer/usr/bin/make docker-push-internal IMAGE=.build TAG=v39
./cert-gen
echo "### Now building docker.io/fortio/fortio.build:v39"
docker buildx build --platform linux/amd64,linux/arm64,linux/ppc64le,linux/s390x -f Dockerfile.build --load -t docker.io/fortio/fortio.build:v39 .
echo "### Now pushing docker.io/fortio/fortio.build:v39"
docker buildx build --push --platform linux/amd64,linux/arm64,linux/ppc64le,linux/s390x -f Dockerfile.build -t docker.io/fortio/fortio.build:v39 .

so I tried

$ docker buildx build --platform linux/amd64,linux/arm64,linux/ppc64le,linux/s390x -f Dockerfile.build --load -t docker.io/fortio/fortio.build:v39 .
[+] Building 0.0s (0/0)                                                                                                                                                                                                    
error: docker exporter does not currently support exporting manifest lists

reading around seems --load doesn't quite work in multi arch so I tried

$ docker buildx build --platform linux/amd64,linux/arm64,linux/ppc64le,linux/s390x -f Dockerfile.build -t docker.io/fortio/fortio.build:v39 .
[+] Building 0.0s (0/0)                                                                                                                                                                                                    
error: multiple platforms feature is currently not supported for docker driver. Please switch to a different driver (eg. "docker buildx create --use")

@ldemailly
Copy link
Member

ldemailly commented Apr 17, 2022

Hello @ldemailly,

I scanned the issue and it seem to relate to go 1.18 not 1.17?
Are you sure https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh doesn't work?
(do you have a sample ci/example of error from your fork?)

building with go 1.17.z and using the previous script fails, at least, on ARM. We could either bump to go1.18 and use the script to install golang-ci-lint, or we can keep on go1.17 but using the go install way for that. I tried to be minimal in changes. What do you think? See error below.

I reproed indeed the arm failure and found what I think is a more direct issue golangci/golangci-lint#2673 (not related to 1.18) - I updated in 18a6c2c
(can still use the binary install that they recommend)

…--load doesn't work for me... had to split build and push and call just push
Makefile Outdated Show resolved Hide resolved
@ldemailly
Copy link
Member

@ldemailly
Copy link
Member

it's missing the rpm, the deb and orig.tar.gz

fixed that part

@ldemailly
Copy link
Member

https://github.com/fortio/fortio/releases/tag/v1.27.0-pre2 has a lot of artifacts now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants