Support self hosted Github workers on forks #29259
Conversation
When this script is used as part of a Github Action, there is no master branch, only origin/master.
Due to the minute multipliers for Windows and macOS forks on free account will run out of free credit very quickly. This may come at the expense other projects they're working on.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. |
A Cirrus account is free for public open source projects, as well as persistent workers connected to it. GitHub "free" minutes should also be unlimited for public open source, no?
Nice, but generally I don't think there is a need to run all CI tasks locally, because it is faster and easier to just use your normal workflow to compile and run the Bitcoin Core tests. Using the normal workflow has the benefit that modifications are faster picked up, without having to start the CI task from scratch. Generally, I expect the workflow to be something like:
|
| @@ -11,7 +11,7 @@ set -ex | |||
| if [ -n "$LOCAL_BRANCH" ]; then | |||
| # To faithfully recreate CI linting locally, specify all commits on the current | |||
| # branch. | |||
| COMMIT_RANGE="$(git merge-base HEAD master)..HEAD" | |||
| COMMIT_RANGE="$(git merge-base HEAD origin/master)..HEAD" | |||
There was a problem hiding this comment.
unrelated: Generally I'd find it better if we get rid of the commit range completely. For example, when running the whitespace linter, it seems easier to just lint all code at HEAD, instead of trying to guess which code was modified and only lint that.
| @@ -52,7 +52,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then | |||
| --mount "type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" \ | |||
| --mount "type=volume,src=${CONTAINER_NAME}_depends_SDKs_android,dst=$DEPENDS_DIR/SDKs/android" \ | |||
| --mount "type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" \ | |||
| --env-file /tmp/env \ | |||
| --env-file /tmp/env-$USER \ | |||
There was a problem hiding this comment.
May be good to explain this change?
There was a problem hiding this comment.
Hours and hours of headache :-)
Because I run four workers as different users, all builds would silently fail for the other users because they didn't have write permission on /tmp/env.
There was a problem hiding this comment.
Ah. May be better to use $CONTAINER_NAME then?
Could be a separate pull request, because this is a bug fix?
Oh that's nice.
Ah, that could explain why my minutes aren't decreasing in Github settings despite having done a few CI runs. If that's documented somewhere then I'll drop a3b5b87.
In the general case I agree. I usually run only a few tests related to what I'm working on, and then the whole test suite before pushing (well, not always...). But I've occasionally managed to trip up one or two CI instances on e.g. memory leaks. Sometimes I can reproduce them locally with a configure change, but other configurations are trickier. For those cases I can further improve the gist to only run a subset of the jobs.
|
In that case it seems easier to just run the "one or two" CI tasks on your machine that would become the "self-hosted runner", instead of going through the extra step to set up GitHub CI and the self-hosted runner with GHA? |
So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus. Regardless, given that GHA minutes are "free", feel free to move the Cirrus self-hosted runners over to GHA-hosted runners. Not sure if this is trivially possible for all tasks, but if it is, it would make it easier to run the CI on forks with less hardware to setup. |
|
It looks like Github's "test each commit" makes an assumption that doesn't hold for forks: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28 |
I haven't looked into how self-hosted runners work with Cirrus. Adding another SAAS company into the loop seems suboptimal. And if Cirrus also reads all it configuration out of a yaml file in the repo, would it actually reduce the amount of code changes in the main repo? |
Yes, see https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42 |
The workers currently use Cirrus self-hosted ones, so no changes are needed. This is documented in the cirrus yaml |
This is a bit confusing. It's free but we use self hosting instead? Is that for performance, security some configuration issue? I just gave Cirrus permission to run on my fork, but it complains "No public persistent worker pools found!" for all jobs. So I guess self-hosting is not optional for forks? I'll see if I can figure out how to get this to work. It seems the first step is to add a (private?) worker in https://cirrus-ci.com/settings/github/Sjors and then install cirrus-cli in a way similar to how it works for Github? https://github.com/cirruslabs/cirrus-cli/blob/master/PERSISTENT-WORKERS.md In the long run a (slight) dependency on Cirrus for CI may be better than Github, since the deeper we integrate with Github, the more difficult it is to move code hosting / coordination elsewhere. |
|
The assumptions for the runners are documented where they are configured: https://github.com/bitcoin/bitcoin/blob/master/.cirrus.yml#L16 |
|
I'll try to get that to work. At first glance it doesn't seem more complicated than the Github approach. If so then I'll close this and salvage the first two commits. |
|
Closing in favour of #29274 |
576828e ci: test-each-commit merge base optional (Sjors Provoost) e9bfbb5 ci: forks can opt-out of CI branch push (Cirrus only) (Sjors Provoost) Pull request description: Maintainer note: `SKIP_BRANCH_PUSH=true` must be set in Cirrus for `bitcoin-core/gui` before merging this. See `https://cirrus-ci.com/github/bitcoin-core/gui` -> Settings. --- I find myself making pull requests against my fork (mostly on top of #28983, or asking others to do so. Currently only the Github actions are run on forks, because we use self-hosted runners for the Cirrus tasks. While setting up my own self-hosted runners for my fork, I ran into a number of issues. Some of those were addressed by #29441, but remaining issues are: 1. When PRs are opened in the fork, cirrus CI jobs are run twice because PRs and branches reside in the same repository, rather than a main repository and a fork repository, as is the case with bitcoin/bitcoin PRs. Fix this by adding a `SKIP_BRANCH_PUSH` configuration option that allows skipping CI runs not directly associated with a PR. The fix is a generalization of [#20328](#20328), which fixed a similar problem for the bitcoin-core/gui mirror repository, and it allows removing a hardcoded reference to that repository. Github actions jobs will still run twice despite this change, see [#29274 (comment)](#29274 (comment)). Initially this PR tried to prevent that with b9fdd0d, but this had some potentially negative side effects, see [#29274 (comment)](#29274 (comment)), so that commit was dropped for now. 2. When PRs are opened in the fork, the "test-each-commit" github action can fail due to not being able to find a recent merge commit. This problem doesn't happen in the bitcoin/bitcoin repository because branches in this repository used as the base for pull requests always point at merge commits. This PR replaces #29259 using the self hosted workers via Cirrus instead of Github. You can see this PR in action on this pull request to my fork: Sjors#30 To test it yourself: 1. spin up at least two [self hosted runners](https://github.com/cirruslabs/cirrus-cli/blob/master/PERSISTENT-WORKERS.md). Either use a seperate VM for each, or give them their own user. 3. Install Podman and other CI dependencies (see .cirrus.yml) 4. Give Cirrus access to your fork at https://cirrus-ci.com/settings/github/YOU 5. Get a token from Cirrus and use it to start your worker(s) 6. Optionally set SKIP_BRANCH_PUSH=true ~and NO_ARM=true~ env variables (see .cirrus.yml) make a pull request to your own fork, with this PR as the base branch Security wise: when dealing with code from strangers on the internet, review it first before running the CI. There's a Cirrus check-box that requires approval for people without write access to trigger CI. ACKs for top commit: maflcko: ACK 576828e ryanofsky: Code review ACK 576828e. Tree-SHA512: fb6be2f228aa62f45a65ce5c613c979b6f387df396f9601ce4622b27aa317a66f198e7d7a194592b0bb397b32a2f50f8be47065834d74af4ea09407c5c8d306d
I find myself making pull requests against my fork (mostly on top of #28983), or asking others to do so. Since I don't want to splurge on a Cirrus account, and since Github free minutes are limited, I looked into how self hosted runners work on Github.
You can see this PR in action on this pull request to my fork: https://github.com/Sjors/bitcoin/actions/runs/7554260507
To test it yourself, spin up one or more self hosted runners (each as its own user), install Docker (maybe in single user mode), and then make a pull request to your own fork, with this PR as the base branch.
Security wise: when dealing with code from strangers on the internet, review it first before running the CI.
This PR also disables Github Actions on forks in order to prevent forks from running out of free minutes (that their owners may need for other projects with much shorter CI runs). This could be made into a separate PR, or dropped altogether.
The first two commits could be split out as well.
TODO:
Related: I wrote a gist for how to run CI on Ubuntu desktop with one runner per window. The windows close if the run is successful so you can inspect the ones that fail. But I find using Github's CI web interface a bit more convenient, hence this PR.