Adds ARM arch to API docker build #1248

dngrhm · 2022-07-27T13:40:30Z

Description

As a developer, I want to run the Stacks blockchain API on a Raspberry Pi or Apple Mx processor. This is needed as the amd64 docker builds run slowly. This pull request adds an ARM build to the API docker image that is pushed to the Docker hub. This should have no impact on developers of the API. The CI build will take ~~a little~~ longer.

Type of Change

New feature
Bug fix
API reference/documentation update
Other

Does this introduce a breaking change?

No breaking changes

Are documentation updates required?

Documentation updates are not required.

Testing information

To test, force a CI build.
Ensure that the build works and pushes an ARM build to Docker hub

Checklist

Code is commented where needed
Unit test coverage for new or modified code paths
npm run test passes
Changelog is updated
Tag 1 of @rafaelcr or @zone117x for review

@rafaelcr @wileyj Please review 🙏

adds qemu setup

rafaelcr

LGTM, thank you!
I'd just like to see what @CharlieC3 thinks of adding this to our CI

CharlieC3 · 2022-07-27T14:03:27Z

@rafaelcr Let's see the new build time first. It can lengthen it quite a bit since ARM has to be built inside an emulator (qemu).

coveralls · 2022-07-27T14:08:15Z

Coverage decreased (-18.5%) to 57.873% when pulling b0e9c54 on dngrhm:master into edab62a on hirosystems:master.

rafaelcr · 2022-07-27T14:20:10Z

@CharlieC3 the build_publish step failed because this PR is from an external contributor 🤔 is there any other way we could test this besides recreating the changes into a new PR from us?

CharlieC3 · 2022-07-27T14:24:19Z

@rafaelcr The workflow will need to be modified so it can run from a fork, which would require a separate PR. We should have time to take a look at this next week.

dngrhm · 2022-07-27T15:34:59Z

The snapshots show the time increase for building with qemu. I'll look into how to shorten this up a bit. @wileyj has done some pretty amazing stuff to optimize the build for the blockchain docker images. I'll see what I can use from that.

CI build on my fork

Previous CI build on upstream

wileyj · 2022-07-27T20:54:24Z

I’m afk for a bit, but if this is still open when I return I’m happy to look

…

On Wed, Jul 27, 2022 at 17:35 dngrhm ***@***.***> wrote: The snapshot show the time increase for building with qemu. I'll look into how to shorten this up a bit. @wileyj <https://github.com/wileyj> has done some pretty amazing stuff to optimize the build for the blockchain docker images. I'll see what I can use from that. CI build on my fork [image: image] <https://user-images.githubusercontent.com/1048802/181287838-6e32643e-e9d0-460d-a78d-072f98ca4726.png> Previous CI build on upstream [image: image] <https://user-images.githubusercontent.com/1048802/181287993-f6751764-c5e6-49de-9126-a2d98ee884d5.png> — Reply to this email directly, view it on GitHub <#1248 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVXIHGUB4HSS5ZLRXHXEZ3VWFJK7ANCNFSM54ZUZNKA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

codecov-commenter · 2022-08-04T04:39:27Z

Codecov Report

Merging #1248 (db1c05a) into master (299705f) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1248   +/-   ##
=======================================
  Coverage   78.32%   78.33%           
=======================================
  Files          77       77           
  Lines       11034    11034           
  Branches     2463     2463           
=======================================
+ Hits         8642     8643    +1     
+ Misses       2279     2278    -1     
  Partials      113      113

see 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

zone117x

This is great! Could we make it so that multiarch building is only performed on merges into master? An extra 30 minutes to CI for each PR is hard to swallow.

CLAassistant · 2022-08-05T18:39:18Z

All committers have signed the CLA.

wileyj · 2022-08-08T22:01:40Z

This is great! Could we make it so that multiarch building is only performed on merges into master? An extra 30 minutes to CI for each PR is hard to swallow.

with QEMU, it's not that simple 😢

However, you can target which arches to build with docker buildx - for example, only build the arm64 arch when it's a release tag. else, build x86_64 as a default.

https://github.com/dngrhm/stacks-blockchain-api/blob/master/.github/workflows/ci.yml#L555-L563

I would argue this should be changed based on how the ci is triggered - setting a default env. var of linux/amd64, and if the run is a release tag, then add the arm arch.
psuedo code:

if <release tag is provided>; then
  docker_platforms="linux/arm64, linux/amd64, linux/amd64/v2, linux/amd64/v3"
else
  docker_platforms="linux/arm64"
fi

if arm builds are only performed on release tag (or some other non-common event), the build time shouldn't be a big problem since the additional time will only be added when something non-common happens.
for example you can see how i'm working this methodology into the stacks-blockchain CI:
stacks-network/stacks-core#3199
https://github.com/wileyj/stacks-blockchain/blob/feat/update-ci/.github/workflows/ci.yml

it's a little different than how the API workflow is setup, but it gives an idea of what i'm suggesting.
outside of that, i don't think it's possible to speed it up since QEMU is really slow.

I'm also skeptical of using a cache here @dngrhm - since this is a single build of a dockerfile, i'm not sure it adds anything to the workflow (unless i'm missing something). i.e. no other steps are using that cache, so it's not adding anything.

The only other thing that might possibly speed things up (but this is dubious at best, and has it's own issues) is using ramdisk to store the build files. But, there is a limit to the amount of storage available and it could result in bad workflow runs without any apparent reason as to why. There is also the fact that it may not speed things up enough to warrant using this method. I wouldn't do this, but it's an option.

dngrhm · 2022-08-09T03:52:46Z

Thanks for the feedback @wileyj!! I was skeptical of the cache as well. I added it as a test and haven't seen improvement from it. I removed the cache and updated the platform logic as you suggest.

wileyj · 2023-04-06T22:39:52Z

Thanks for the feedback @wileyj!! I was skeptical of the cache as well. I added it as a test and haven't seen improvement from it. I removed the cache and updated the platform logic as you suggest.

oh snap, i think we have competing PR's: #1578

zone117x · 2023-04-20T12:37:21Z

Closing in favor of #1578

feat: adds arm64 arch to build

d7ba89c

adds qemu setup

rafaelcr approved these changes Jul 27, 2022

View reviewed changes

rafaelcr requested a review from CharlieC3 July 27, 2022 14:01

feat: Updates Dockerfile to use staged build to remove build deps

44c1edc

fix: move QEMU before buildx setup updated action versions

9bd934d

zone117x reviewed Aug 4, 2022

View reviewed changes

dngrhm added 2 commits August 4, 2022 22:11

feat: Adds cache settings to docker multi-platform build

d5df572

feat: Includes ARM platform builds only for new release versions

75e336a

feat: Uses env vars to set selected docker platforms for build

b0e9c54

Merge branch 'hirosystems:master' into master

db1c05a

zone117x closed this Apr 20, 2023

pradel mentioned this pull request Apr 16, 2024

feat: add docker build for arm #1947

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds ARM arch to API docker build #1248

Adds ARM arch to API docker build #1248

dngrhm commented Jul 27, 2022 •

edited

Loading

rafaelcr left a comment

CharlieC3 commented Jul 27, 2022

coveralls commented Jul 27, 2022 •

edited

Loading

rafaelcr commented Jul 27, 2022

CharlieC3 commented Jul 27, 2022

dngrhm commented Jul 27, 2022 •

edited

Loading

wileyj commented Jul 27, 2022 via email

codecov-commenter commented Aug 4, 2022 •

edited by codecov bot

Loading

zone117x left a comment

CLAassistant commented Aug 5, 2022 •

edited

Loading

wileyj commented Aug 8, 2022

dngrhm commented Aug 9, 2022

wileyj commented Apr 6, 2023

zone117x commented Apr 20, 2023

Adds ARM arch to API docker build #1248

Adds ARM arch to API docker build #1248

Conversation

dngrhm commented Jul 27, 2022 • edited Loading

Description

Type of Change

Does this introduce a breaking change?

Are documentation updates required?

Testing information

Checklist

rafaelcr left a comment

Choose a reason for hiding this comment

CharlieC3 commented Jul 27, 2022

coveralls commented Jul 27, 2022 • edited Loading

rafaelcr commented Jul 27, 2022

CharlieC3 commented Jul 27, 2022

dngrhm commented Jul 27, 2022 • edited Loading

wileyj commented Jul 27, 2022 via email

codecov-commenter commented Aug 4, 2022 • edited by codecov bot Loading

Codecov Report

zone117x left a comment

Choose a reason for hiding this comment

CLAassistant commented Aug 5, 2022 • edited Loading

wileyj commented Aug 8, 2022

dngrhm commented Aug 9, 2022

wileyj commented Apr 6, 2023

zone117x commented Apr 20, 2023

dngrhm commented Jul 27, 2022 •

edited

Loading

coveralls commented Jul 27, 2022 •

edited

Loading

dngrhm commented Jul 27, 2022 •

edited

Loading

codecov-commenter commented Aug 4, 2022 •

edited by codecov bot

Loading

CLAassistant commented Aug 5, 2022 •

edited

Loading