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

[Merged by Bors] - Multiarch docker GitHub actions #2065

Closed

Conversation

realbigsean
Copy link
Member

Issue Addressed

Resolves #1512

Proposed Changes

  • Adds a new docker Github Actions workflow
  • Removes the Dockerhub hook
  • Adds a new Dockerfile for use with pre-existing cross-compiled binaries
  • on pushes to unstable
    • builds an ARM64 image and tags it latest-arm64-unstable
    • builds an AMD64 image and tags it latest-amd64-unstable
    • builds an multiarch image by creating a manifest list referencing the prior two images and tags it latest-unstable
  • on pushes to stable
    • builds an ARM64 image and tags it latest-arm64
    • builds an AMD64 image and tags it latest-amd64
    • builds an multiarch image by creating a manifest list referencing the prior two images and tags it latest

Additional Info

  • for ARM64, first cross is used to cross compile the lighthouse and lcli binaries, then docker buildx is installed to actually build the docker image for the correct target platform. The image build pretty much just copies the binaries from local into the docker image (thanks @michaelsproul :) )
  • The AMD64 and ARM64 builds run in parallel, in total it's been taking around 45mins on a local runner
  • This PR does not cover version tags on docker images at the moment

@realbigsean realbigsean marked this pull request as ready for review December 7, 2020 21:39
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

This is awesome!

I'm inclined to merge soon, although I'd like to see how long it takes to run on Github's runners. Maybe we could temporarily tweak the build rules to run for this PR?

To help performance along I think removing lcli is a good idea, and we could maybe even try to merge my PR to reduce compilation time (#1989). I'll work on updating it today so it's ready to go.

.github/workflows/docker.yml Outdated Show resolved Hide resolved
#!/bin/bash

# Build hook to run on Docker Hub to ensure that the image is built with `PORTABLE=true`.
docker build --build-arg PORTABLE=true -f $DOCKERFILE_PATH -t $IMAGE_NAME .
Copy link
Member

Choose a reason for hiding this comment

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

I think as well as deleting this file we'll need one of the DockerHub admins to disable the DockerHub build (we can tag Paul/Age when the time is right).

@michaelsproul michaelsproul added ready-for-review The code is ready for review infra-ci labels Dec 7, 2020
@realbigsean
Copy link
Member Author

Removed the lcli cross-compilation. I'd also like to test on a Github server but I think the workflow has to be in the default branch before it can be triggered (unless there's some manual workaround?), so don't think I can from my PR here.

@sigp sigp deleted a comment from realbigsean Dec 8, 2020
@realbigsean
Copy link
Member Author

I disabled my local runner to test on a Github hosted runner (didn't realize you could use these for free) and kicked off a build on stable and unstable in my forked repo. These are the two runs:
https://github.com/realbigsean/lighthouse/actions/runs/409396423
https://github.com/realbigsean/lighthouse/actions/runs/409395631

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Happy to merge once the builds on your repo succeed. Hopefully #1989 will have merged by then too.

@realbigsean
Copy link
Member Author

The builds on my repo have succeeded, and I tested out the stable and unstable images on my pi and my laptop. All's well 👍

@michaelsproul
Copy link
Member

Awesome!

I think we can merge, and then disable the old docker build once it lands. We'll only update the latest-unstable tag, which is unused at the moment anyway.

bors r+

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Dec 9, 2020
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Nice! This looked like it would have been super fiddly.

Big benefit to users and devs in this one 🚀

bors bot pushed a commit that referenced this pull request Dec 9, 2020
## Issue Addressed

Resolves #1512

## Proposed Changes

- Adds a new docker Github Actions workflow  
- Removes the Dockerhub hook
- Adds a new Dockerfile for use with pre-existing cross-compiled binaries 
- on pushes to `unstable` 
  - builds an ARM64 image and tags it `latest-arm64-unstable`
  - builds an AMD64 image and tags it `latest-amd64-unstable`
  - builds an multiarch image by creating a manifest list referencing the prior two images and tags it `latest-unstable`
- on pushes to `stable` 
  - builds an ARM64 image and tags it `latest-arm64`
  - builds an AMD64 image and tags it `latest-amd64`
  - builds an multiarch image by creating a manifest list referencing the prior two images and tags it `latest`

## Additional Info
- for ARM64, first `cross` is used to cross compile the `lighthouse` and  `lcli` binaries, then `docker buildx` is installed to actually build the docker image for the correct target platform. The image build pretty much just copies the binaries from local into the docker image (thanks @michaelsproul :) )
- The AMD64 and ARM64 builds run in parallel, in total it's been taking around 45mins on a local runner
- This PR does **not** cover version tags on docker images at the moment

Co-authored-by: realbigsean <seananderson33@gmail.com>
@bors bors bot changed the title Multiarch docker GitHub actions [Merged by Bors] - Multiarch docker GitHub actions Dec 9, 2020
@bors bors bot closed this Dec 9, 2020
@michaelsproul
Copy link
Member

🎉 🎉 🎉

@realbigsean realbigsean deleted the multiarch-docker-github-actions branch November 21, 2023 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra-ci ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants