Skip to content

Use Cubit in GitHub hosted runners #393

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Use Cubit in GitHub hosted runners #393

wants to merge 3 commits into from

Conversation

isteinbrecher
Copy link
Collaborator

This PR allows to use GitHub hosted runners with Cubit. This is done by actually installing cubit from an official source and activating a testing version with GitHub secrets.

The download, unpacking and license setup of Cubit takes between 60 and 80 seconds, so it is in the same range as pulling the 4C image - I think this is a reasonable increase in runtime for the pipelines.

This brings a number of advantages:

  • We now have one test which tests MeshPy with all dependencies (ArborX, CubitPy, 4C)
  • Everything is done in GitHub, no need to run any testing machines
  • In the future we can now also easily test the ArborX performance

The one super tricky part was using GitHub secrets in standard PR workflows. Secrets are needed to activate CubitPy. I am still not 100% sure if everything works as expected, but we will have to see.

One problem is that we enforce forks, however PRs from forks are not allowed do use secrets. Thus we have to trigger the pipeline with pull_request_target instead of pull_request this will (hopefully) allow to use the secrets in every PR - BUT it will use the workflow definition from main, so users can not try to figure out the secrets.

This should work fine, but leaves one question: how to test changes to the testing workflow. This is achieved by having a second branch ci-testing in the main MeshPy repository. Branches on the main repo are allowed to use secrets. Pushes to ci-testing now trigger the pipeline and allow full secret access. So the idea is to have all testing related work on this branch in the future. Leaves one final issue, when we merge from ci-testing to main we would theoretically trigger two pipelines (one for the PR (but with the testing setup from main, and one for a push to ci-testing) this should be avoided by if guarding all tests.

Depends on #392

Follow up PRs: Think about performance testing ArborX

@isteinbrecher
Copy link
Collaborator Author

@davidrudlstorfer I am curious about your opinion, and there is no need in rushing this.

Copy link
Collaborator

@davidrudlstorfer davidrudlstorfer left a comment

Choose a reason for hiding this comment

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

I am going to write a little bit more on the entire procedure in just a minute

@@ -1,17 +1,12 @@
name: setup_virtual_python_environment
description: Configure the python virtual environment
inputs:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the clean-up, hopefully sooner than later I will update the virtual env process in our workflows. We should also use a conda env everywhere. This will drastically simplify everything and also make it more stable. I used the same approach in the workflows in FourCIPP

type: choice

env:
CUBIT_DOWNLOAD_URL: https://f002.backblazeb2.com/file/cubit-downloads/Coreform-Cubit/Releases/Linux/Coreform-Cubit-2025.3%2B58709-Lin64.tar.gz
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any process to update this link? Does Cubit somewhere provide a link to the latest release?

@davidrudlstorfer
Copy link
Collaborator

First of all, kudos to you for setting-up Cubit to work on Github runners. I am impressed that this works and I know the pain and endless testing for setting-up new Github workflows :D

I am fully fine with the Cubit setup procedure and we can merge this

Regarding the workflow I am not 100% sure I understand it in its full form, maybe it would make sense that we briefly discuss this in person.

First of all, I think you already researched a lot but just two things I stumbled over.

  1. https://stackoverflow.com/a/77010688 I think we can never mitigate the risk of leaking the secrets, some workaround is always available. Therefore, we need to ensure to trust the people who can open a pull request. Currently (and probably in the near future) it's only people from our organisation. So I would not worry too much about the security risks - because we have to approve new people anyways.

  2. Why do we need to handle changes to the testing workflow separately? This is the point I do not fully understand. Is it because we can only run workflows which are already merged in the main branch? I would not worry too much about that because, 1. we almost never change/will change anything in our base setup, 2. if we change something it fails most of the times and we redo it anyways (looking at you Github workflows), 3. I would only add the complexity of the ci-testing branch if we really need it in the future

  3. https://github.com/imjohnbo/ok-to-test would something like this help us? Seems similar to what dealii and trilinos is doing as far as I can tell.

All in all, it is super important that we have the necessary measurements to ensure the github secrets are not leaked. But I think because we are in a "safe" environment where only trusted people can execute a workflow we do not need to worry too much about leaking them.

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.

2 participants