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: add initial support for arm builds #40550

Merged
merged 2,177 commits into from
Mar 4, 2024
Merged

feat: add initial support for arm builds #40550

merged 2,177 commits into from
Mar 4, 2024

Conversation

johanneskoester
Copy link
Contributor

@johanneskoester johanneskoester commented Apr 21, 2023

Support for arm builds (macos-M1 and linux-graviton) via CircleCI.

TODOs

  • Merge aarch64 support in bioconda-utils: feat: add Linux aarch64/arm64 support for bioconda-utils bioconda-utils#866
  • Support for downloading artifacts from CircleCI. This needs to be done here, analogous to azure, and switchable between azure and circleci via a CLI flag here. Does anybody volunteer for this (thanks a lot, I am very busy with teaching and Snakemake 8 at the moment)?
  • Make base images multiarch bioconda-containers#55 needs to be fixed and merged (@martin-g is on it).
  • Decide how this should be enabled. Activate it for all, and let people skip failing stuff via a selector (# [arm64]) on build::skip as usual, or shall we not build by default and only enable it if something special is added to extra? Decision of bioconda/core: opt-in in the first round, via a special field in extra (see next item)
  • add support in bioconda-utils to run on linux-arm if the following is added to the recipe:
extra:
  additional-platforms:
    - linux-aarch64

@johanneskoester johanneskoester changed the title feat: add initial support for arm build [WIP] feat: add initial support for arm build Apr 21, 2023
@johanneskoester johanneskoester changed the title feat: add initial support for arm build feat: add initial support for arm builds Apr 21, 2023
@Yikun
Copy link
Contributor

Yikun commented Apr 23, 2023

Wow! Greate step! We will join and work together to make it happen!

Also cc @martin-g

@Yikun
Copy link
Contributor

Yikun commented Apr 25, 2023

I found some issue to make this PR green.

  1. Added multiarch for conda installation: Support multi-arch in bioconda-common bioconda-common#33

  2. Switch MINICONDA_VER to upper version, So, which version should we pick?

BTW, CIRCLE CI failed on osx-arm64 due to Resource class macos for macos.m1.large.gen1, image xcode:14.2.0 is not available for your project, or is not a valid resource class., there are some discussion on https://discuss.circleci.com/t/announcing-m1-large-now-available-on-performance-plans/47797/9 , I'm not sure CIRCLE CI offered osx-arm64 vm for free usage or not.

@Yikun
Copy link
Contributor

Yikun commented Apr 27, 2023

Upgrade miniconda version to py37_4.9.2 in bioconda-common bioconda/bioconda-common#34

Frankly, I'm not sure should we also upgrade to py38_4.10.1 to also support osx-arm64, looks like bump version from py37_4.8.3 to py37_4.9.2 is a small forward step which has less impact.

Copy link
Contributor

@Yikun Yikun left a comment

Choose a reason for hiding this comment

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

I test it locally in Linux aarch64 with latest bioconda-utils, it works! And the config.yaml should fix for latest bioconda-utils.

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@Yikun
Copy link
Contributor

Yikun commented May 9, 2023

FYI, @johanneskoester this also what we should be ready before we support aarch64: bioconda/bioconda-containers#55

resource_class: macos.m1.large.gen1
linux-aarch64:
machine:
image: ubuntu-2004:current
Copy link
Contributor

@martin-g martin-g Sep 7, 2023

Choose a reason for hiding this comment

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

It might be useful:

Since recently it is possible to use Docker executor with ARM64 resources - https://circleci.com/docs/using-docker/#arm
This way you could use a custom Docker image that has most of the dependencies preinstalled. It will save a lot of time for the runs/builds!

Here is a small example:

@ewels
Copy link
Member

ewels commented Sep 20, 2023

Hi all! Just wanted to pop my head up and say that we (@seqeralabs and @nextflow-io / @nf-core communities) would love to help out getting ARM bioconda builds rolling.

I'm seeing a few semi-private efforts starting to nudge forward (eg. Oxford Nanopore, some Seqera proof-of-concept testing) and I'm a bit anxious that without bioconda to unite behind, we will start to see some fragmentation & duplication of efforts.

Is there anything that we can do to move the bioconda infrastructure piece forward? I feel like we're coming to a place where we might be able to rally a fair amount of resources for this project, so if there's anywhere where that's needed then please say 🙏🏻

Once the bioconda back-end is in place, we could also try to help out with any community efforts to crowdsource arm recipe work via hackathons / events. That could be fun :)

@martin-g
Copy link
Contributor

@ewels Thank you for offering your help here!
I am not a member of the Bioconda team! I am also a user who tries to help with this task.

You might be interested in the progress so far. Please check:

It would be great if you have time to test the proposed changes and either propose improvements or approve/vote for them!

@Yikun
Copy link
Contributor

Yikun commented Sep 21, 2023

@ewels Great to hear about that!

BTW, we also have completed about 500+ bioconda (bioconductor & conda) packages validation based on latest bioconda-utils[1][2][3]. It really turns out that bioconda on ARM is possible.

We really need suggestion from Bioconda team about how we could move it forward in upstream.

[1] bioconda/bioconda-utils#866
[2] bioconda/bioconda-utils#867
[3] bioconda/bioconda-utils#906

@ewels
Copy link
Member

ewels commented Sep 21, 2023

Wow, lots of discussions going on all over the place - more than I realised. Thanks both! And impressive work @Yikun!

It seems to me that although there is still some work to do / some PRs to be merged to support this on the back-end, most things are about ready. Scanning over this, and chatting to @rpetit3, it sounds like the main thing missing is the CI infrastructure to run the automated builds. Is that approximately correct?

@dpryan79, over in this comment you mentioned discussions with AWS about credits. Is anyone in @bioconda/core aware of progress on this front, or more generally aware of the current state of discussions around build infrastructure + funding for ARM packages? It's possible that I might be in a position to help to un-stick some of this, depending on what's needed. Happy to help out if I can be of any use.

@johanneskoester
Copy link
Contributor Author

Hi @ewels. Indeed we have to offering to get AWS credits already, and that is still an option. This PR was on hold because I was reaching out to CircleCI whether it is possible to get a free M1 plan from them. We would prefer that because it would mean real and safe CI integration instead of having to manually set up something via AWS (which could as far as I am informed always lead to CI going mad and eating up all our credits, or even beyond). Unfortunately CircleCI did not answer me so far. However, if I am not wrong, at least the non-macos stuff is free to use for us already. So we could get started with that, and then, with Sequera behind us, we might be able to finally get an answer from CircleCI on the M1 question?

@ewels
Copy link
Member

ewels commented Oct 4, 2023

Sounds great! Yes AWS credits are always a little scary and do need keeping an eye on. We've been using them heavily for @nf_core stuff for several years now and it's been (mostly) smooth sailing. Just need billing alerts and a handful of people to share responsibility for regularly checking remaining credits. I've been looking into methods to better automate some of this - eg. automated slack messages with remaining credit levels. Nothing to report yet, but there may be some hope yet.

However, if I am not wrong, at least the non-macos stuff is free to use for us already. So we could get started with that, and then, with Seqera behind us, we might be able to finally get an answer from CircleCI on the M1 question?

Perfect - I'll see if I can get anything moving in CircleCI circles and in the mean time let's crack on! 🎉

@johanneskoester
Copy link
Contributor Author

johanneskoester commented Oct 4, 2023

Ok, seems to work!
There are six remaining blockers for this PR now:

  • Support for downloading artifacts from CircleCI. This needs to be done here, analogous to azure, and switchable between azure and circleci via a CLI flag here. Does anybody volunteer for this (thanks a lot, I am very busy with teaching and Snakemake 8 at the moment)?
  • Make base images multiarch bioconda-containers#55 needs to be fixed and merged (@martin-g is on it).
  • Decide how this should be enabled. Activate it for all, and let people skip failing stuff via a selector (# [arm64]) on build::skip as usual, or shall we not build by default and only enable it if something special is added to extra? Decision of bioconda/core: opt-in in the first round, via a special field in extra (see next item)
  • add support in bioconda-utils to run on linux-arm if the following is added to the recipe:
extra:
  additional-platforms:
    - linux-aarch64
  • Update the documentation (again, happy to take a PR on https://github.com/bioconda/bioconda-docs for the same reason as item 1).
  • Implement bulk updates on circle CI for arm. This requires porting .github/workflows/bulk.yml to circleci.

@aliciaaevans
Copy link
Contributor

@johanneskoester I can start working on item 1 (artifacts from CircleCI).

@johanneskoester
Copy link
Contributor Author

@johanneskoester I can start working on item 1 (artifacts from CircleCI).

Thanks a lot @aliciaaevans, and welcome to the team!

.circleci/config.yml Outdated Show resolved Hide resolved
johanneskoester pushed a commit to bioconda/bioconda-utils that referenced this pull request Oct 8, 2023
- Support for downloading artifacts from CircleCI instead of Azure.
- Added option `--artifact-source` with `choices=['azure', 'circleci']`
to `handle_merged_pr`
- Check architecture when returning platform
- See PR for the ARM builds
bioconda/bioconda-recipes#40550
@Yikun
Copy link
Contributor

Yikun commented Oct 9, 2023

add support in bioconda-utils to run on linux-arm if the following is added to the recipe:

bioconda/bioconda-utils#923 I drafted an initial PR for this task, feel free to review! Thanks

@aliciaaevans
Copy link
Contributor

For branches where there's no additional-platforms enabled, it seems like a bit of a waste to do all the initial setup steps in CircleCI and then not end up building anything. I'm not sure if there's a clean way to skip or abort the job, though, and it would require checking the meta.yaml file(s) without using bioconda-utils (since it would not yet be installed). I played around a bit with using yg but it can't parse jinja templates.

Any thoughts on if that is worth pursuing?

@martin-g
Copy link
Contributor

I think it is a good optimization!
Maybe you can use sed first to remove the jinja placeholders and then pipe it to yq ?
Something like: cat $recipe/meta.yaml | sed -i 's/^(.*){{(.*)}}(.*)$/$1$2$3/g' | yq ... (I am not sure about the syntax, it is Perl-ish !)

@Yikun
Copy link
Contributor

Yikun commented Oct 11, 2023

$ grep -A 2 "additional-platforms:" recipes/abra2/meta.yaml
  additional-platforms:
    - linux-aarch64

$ grep -A 2 "additional-platforms:" recipes/abra2/meta.yaml | grep -q linux-aarch64
$ echo $?
0

$ grep -A 2 "additional-platforms:" recipes/abra2/meta.yaml | grep -q linux-x86
$ echo $?
1

A simple way without bioconda-utils install came to my mind might like above, even though it's not a perfect way, seems a possible alternative workaround.

johanneskoester pushed a commit to bioconda/bioconda-utils that referenced this pull request Oct 14, 2023
…nal-platforms (#923)

This patches add support in bioconda-utils to run on linux-arm if the
following is added to the recipe:
```yaml
extra:
  additional-platforms:
    - linux-aarch64
```

- Add a `check_native_platform_skippable` to checkout should we skip the
recipes in current nativei platform
- Add `extra_additional_platforms` property to recipe object
- Add UT to test above function
```
py.test --durations=0 test/test_utils.py::test_native_platform_skipping
py.test --durations=0 test/test_recipe.py::test_recipe_extra_additional_platforms
```
- E2E test
```
bioconda-utils build recipes config.yml --git-range origin/master HEAD
bioconda-utils build recipes config.yml --git-range origin/master HEAD --force
```
- Fix test trigger by adding `fetch-depth`

related: bioconda/bioconda-recipes#40550

---------

Signed-off-by: Yikun Jiang <yikunkero@gmail.com>
@aliciaaevans
Copy link
Contributor

See #43708 for doing a check for any additional-platforms before installing bioconda-utils. (PR would merge to this branch, but if this gets merged to master first, I can update it.)

I'm going to start working on porting .github/workflows/bulk.yml to CircleCI next, assuming no one else has that in progress.

@daler
Copy link
Member

daler commented Feb 25, 2024

(Ugh, should have done the merge from master in the GH interface, not the local rebase and push. Sorry for the commit noise)

Anyway, to report back on all of this: CircleCI tokens are fixed and packages can be successfully uploaded to Anaconda. Images can also be uploaded to quay.io/biocontainers too, so artifacts are all working, BUT handling the images remains challenging and may need more discussion. Here's why (see details):

LOTS OF DETAILS...

We had initially discussed putting a -aarch64 suffix on tags, but thought that using manifests would be much better (Recall that a manifest is a list of images, each on different arch, that is pulled automatically for the right arch when using a single tag). However, manifests will be problematic, because the images need to be collected from two different CI platforms and put into a single manifest for uploading. We had previously discussed checking for the existence of a manifest, downloading it (and image) if it exists, and add the local image to the manifest before pushing both images and manifest back. But this still has a race condition: if no manifest yet exists, both CI platforms may try pushing their single-image manifests. I verified that pushing a manifest completely replaces the existing, so in a race condition the latest one wins and we don't get both archs.

Another approach would be to communicate across CI platforms with workflow dispatch:

  • once CircleCI creates artifacts, trigger a dummy GH Actions workflow from CircleCI
  • once Azure creates artifacts, trigger another dummy GH Actions workflow from Azure.
  • a downstream GH Action "upload containers" job would depend on those two GH Actions jobs (see e.g. this SO). So it would wait until all artifacts are created, collect artifacts across CircleCI and Azure, build a single manifest with both images, and push to quay.io.

This latter approach will still break when a PR is merged but artifacts are no longer available. bioconda-utils does have a --fallback build option, but that would need to be somehow changed to trigger new jobs on CircleCI and Azure, and trigger the above steps again. I haven't thought this part through carefully yet. But a cross-CI-platform DAG of workflow runs seems complex and brittle and makes me a bit nervous...

Another idea: maybe we could do a scheduled nightly job that, upon seeing multiple archs for a package in the conda channel, builds multi-arch images out of those packages. That is, normal CI jobs only upload conda packages; a separate, asynchronous job builds multi-arch images out of them. I think with e.g. --platform=linux/arm64 args to docker/buildah we could do this on a single platform, like GH Actions. Then upload the manifest and images. This could be a way of "patching" missing containers. This will need some more bioconda-utils and mulled work.

If we go with -arch suffixes, I think it would be straightforward to "patch" images later as well. In that case we'd just need to run jobs on CircleCI.

TL;DR: I propose:

  1. Merge this PR to move forward with the linux-aarch64 conda package builds.
  2. Revisit adding arch suffixes on biocontainer tags.
  3. Discuss other ideas for collecting images from multiple CI platforms into a single manifest that can be pushed.
  4. Patch missing arm biocontainer images later (since we'll need a solution for that situation no matter what)

@martin-g
Copy link
Contributor

martin-g commented Feb 26, 2024

  1. Merge this PR to move forward with the linux-aarch64 conda package builds.

+1

2. Revisit adding arch suffixes on biocontainer tags.

This is for the images, right ? This way the user has a "workaround" in case there is a problem with the manifest! +1

4. Patch missing arm biocontainer images later (since we'll need a solution for that situation no matter what)

This would be a matter of adding extra-platforms: linux-aarch64 and bumping the build number of a recipe, no ?

@daler
Copy link
Member

daler commented Feb 26, 2024

For item 2, yes, this is for the package images pushed to quay.io/biocontainers. Since manifests are a part of the OCI spec it's unlikely users would have trouble with it; in fact it's how many base docker images e.g. alpine are already provided. It's nice because you can use the same pull command independent of architecture.

For item 4, the "patching" I had in mind was for cases where we had an arm package but not an arm container. So extra-platforms: linux-aarch64 would already be there in the recipes. Once package containers are working, yep, a build number bump should then work, but it would still leave an "orphaned" older build number with no container. We'd need to decide whether that matters.

@martin-g
Copy link
Contributor

martin-g commented Feb 29, 2024

3. Discuss other ideas for collecting images from multiple CI platforms into a single manifest that can be pushed.

Would https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#check_suite work here ?
The idea is to be notified when all checks (including the Azure and CircleCI ones) finish and then pull the new images and create a manifest for them.

@martin-g
Copy link
Contributor

I played with this idea and created a Github App https://github.com/martin-g/my-first-github-app/blob/main/index.js that listens on check_suite.completed event and just logs the JSON payload.
Unfortunately the payload returns

...
"status": "completed",
"conclusion": "success",
...

even when a CircleCI check fails - martin-g/circleci-arm64-test#5

If a Github Action check (Pull Request / pull_request ) fails then the payload contains:

"status": "completed",
"conclusion": "failure",

@martin-g
Copy link
Contributor

@martin-g
Copy link
Contributor

martin-g commented Feb 29, 2024

It does work ! And the Github App is not really needed.
Now if my CircleCI workflow fails then the check_suite.completed event says "conclusion": "failure",!
I guess something similar could be done for the Azure checks too (it seems to be a paid service and I cannot test it).

If the payload has "conclusion": "success" then the Github App could trigger another Github Actions workflow that will create the manifest!

@Yikun
Copy link
Contributor

Yikun commented Mar 2, 2024

1.Merge this PR to move forward with the linux-aarch64 conda package builds.

+1, very exciting to to see this happening!

  1. Patch missing arm biocontainer images later (since we'll need a solution for that situation no matter what)

+1, I think it is reasonable, we can improve it step by step!

@markjens
Copy link

markjens commented Mar 4, 2024

TL;DR: I propose:

  1. Merge this PR to move forward with the linux-aarch64 conda package builds.

Oh, we are that close!!!
+1 !

@daler daler merged commit 36731b7 into master Mar 4, 2024
6 checks passed
@daler daler deleted the arm-builds branch March 4, 2024 21:42
@jmarshall
Copy link
Member

I see with this merge a linux-aarch64 build has now appeared for the test package abra2. 🎉

Does this mean it is time for interested maintainers to start adding additional-platforms:/- linux-aarch64 to individual recipes in ordinary PRs? Or is there more to do first? (Probably an update at #33333 would be useful.)

@daler
Copy link
Member

daler commented Mar 4, 2024

Yes!! Please add additional platforms: [linux-aarch64] for recipes as you see fit 🚀

I'll update at #33333, need to get the docs fully deployed first though (that is in progress).

pysam, samtools, htslib are among the highest out-degree centrality of the entire bioconda channel DAG, so if you want to get those working that would be a huge help.

@jmarshall
Copy link
Member

You will be unsurprised to hear those are precisely the packages I have in mind. 😄
They are already built for ARM in their upstream CI, so I would not expect it to take much to get them working… 🤞

@daler daler removed the WIP label Mar 4, 2024
@rpetit3
Copy link
Member

rpetit3 commented Mar 5, 2024

Awesome work everyone! Thank you @daler for taking the lead, as well as everyone else for providing tremendous feedback and all the behind the scenes PRs!

If you update recipes to opt-in to ARM builds, please feel free to request me as a reviewer!

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.