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

feat: build mirrorbits with Golang 1.23.0 in the image to get an arm64 binary #18

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

lemeurherve
Copy link
Member

@lemeurherve lemeurherve commented Oct 26, 2023

This PR fixes the arm64 image by building mirrorbits binary with the correct architecture as a workaround to the absence of arm64 binary release in https://github.com/etix/mirrorbits.

I wouldn't mind if this PR is refused as it's a bit convoluted, and not without potential flaws.
That would mean give up on mirrorbits migration to arm64 (not a big deal).

Testing done

$ docker build -t mirrorbits-arm --platform linux/arm64 .

$ docker run --platform=linux/arm64 -it mirrorbits-arm uname -a
Linux a1894fa37ce3 5.15.49-linuxkit-pr #1 SMP PREEMPT Thu May 25 07:27:39 UTC 2023 aarch64 GNU/Linux

$ docker run --platform=linux/arm64 -it mirrorbits-arm
 _______ __                        __     __ __
|   |   |__|.----.----.-----.----.|  |--.|__|  |_.-----.
|       |  ||   _|   _|  _  |   _||  _  ||  |   _|__ --|
|__|_|__|__||__| |__| |_____|__|  |_____||__|____|_____|  

2023/10/26 11:33:34.875 UTC open /usr/share/GeoIP/GeoLite2-City.mmdb: no such file or directory
2023/10/26 11:33:34.875 UTC open /usr/share/GeoIP/GeoLite2-ASN.mmdb: no such file or directory
2023/10/26 11:33:34.875 UTC Can't load the GeoIP databases, please set a valid path in the mirrorbits configuration

Follow-up of #17

Related to jenkins-infra/helpdesk#3837

Dockerfile Outdated Show resolved Hide resolved
@lemeurherve lemeurherve added the bug Something isn't working label Oct 26, 2023
@lemeurherve lemeurherve changed the title fix: build mirrorbits in the image to get an arm64 binary poc: build mirrorbits in the image to get an arm64 binary Oct 30, 2023
@lemeurherve lemeurherve changed the title poc: build mirrorbits in the image to get an arm64 binary PoC: build mirrorbits in the image to get an arm64 binary Oct 31, 2023
@lemeurherve lemeurherve removed the bug Something isn't working label Oct 31, 2023
Dockerfile Outdated Show resolved Hide resolved
@dduportal dduportal self-assigned this Aug 20, 2024
…ored deps + Go 1.23.0 + multi-stage fixes

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal dduportal marked this pull request as ready for review August 20, 2024 09:52
@dduportal dduportal requested review from a team as code owners August 20, 2024 09:52
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

As per the mirrorbits diff between v0.5.1 tag and the reference used in ths PR:

  • The missing Go code generated by protoc has been added by etix
  • The dependencies have been vendored (instead of requiring to run govendor)
  • Go modules support has been added (giving Go >= 1.11 compatibility)
  • No Golang or RPC proto definition changes

Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@dduportal dduportal changed the title PoC: build mirrorbits in the image to get an arm64 binary feat: build mirrorbits in the image to get an arm64 binary Aug 20, 2024
@dduportal dduportal changed the title feat: build mirrorbits in the image to get an arm64 binary feat: build mirrorbits with Golang 1.23.0 in the image to get an arm64 binary Aug 20, 2024
@dduportal
Copy link
Contributor

As per @timja review, it makes sense to switch to the Debian package.

We initially avoided it as we wanted to stay as close as current production (mirrorbits v0.5.1 binary) as possible.
This hypothesis changed as we must build a custom "dirty" version of mirrorbits in practice.

Advantages of using the Debian package:

  • Less maintenance (go build, go versions, etc.) for us
  • They already rebuilt it with CVE fixes (Golang but also go dependencies)
  • Out of the box support for arm64
  • APT packages dependencies already present

Let's update the PR

@dduportal dduportal marked this pull request as draft August 20, 2024 10:10
@dduportal
Copy link
Contributor

@timja could we get your advise on the following:

  • Both the "custom Golang build" and "Debian package" are working well (we tested on mirrors.updates.jenkins.io with success)
  • When using the Debian package, as per comments above, we saw the followings cons:
    • Need to use Debian Sid (unstable)
    • It also installs Redis packages (both client and server). Server is not started (Docker image) but it add unnecessary dependencies

I got mix feelings and not sure which one to select so a 3rd advice is interesting: Golang custom build (with maintenance pain but we can choose our own base image distribution) or Debian package (stick to unstable Debian distribution and more packages than needed)?

=> Good point from @lemeurherve: let's think about the upcoming official mirrorbits release (cf. etix/mirrorbits#179). We should expect them to provide binaries for both x86 and arm64. That could be an argument for using the Golang build here (instead of Debian package)

@timja
Copy link
Member

timja commented Aug 20, 2024

Hmm no great option, ideally would help them finish it off in some way, e.g. send a PR for arm64 support but the project doesn't even have CI (etix/mirrorbits#173) and isn't very responsive.

I've sent them feedback on the package here: etix/mirrorbits#150 (comment)
Might need to be sent to debian directly, unsure.

I would say go with the golang one for now as there's problems with the package (unless you want to rebuild the package with fixed dependencies)

@dduportal dduportal marked this pull request as ready for review August 20, 2024 14:24
@dduportal dduportal added the enhancement New feature or request label Aug 20, 2024
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

Given the arguments exposed above, let's go for the "custom golang build" then.

Cons:

  • We have to maintain the build (at least until a new release of mirrorbits with arm64 support is done)
  • We have to manage the dependencies (both build - such as Golang version- and runtime - packages)
  • We do not benefit from the Debian package go deps. security updates

Pros:

  • We use the latest Golang version to build it, removing CVEs
  • We do not increase the surface attack of the final image with unused packages (such as redis-server)
  • It's easy to change back to a normal mirrorbits binary

A big shoutout to @jlevesy whom helped us on this PR!

@dduportal dduportal merged commit c09f2d8 into jenkins-infra:main Aug 20, 2024
2 checks passed
@timja
Copy link
Member

timja commented Aug 22, 2024

The debian package is now fixed to not require redis anymore:
etix/mirrorbits#150 (comment)

in case you're interested in switching to it to simplify the build

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

Successfully merging this pull request may close these issues.

3 participants