Support self hosted Github workers on forks #29259

pull Sjors wants to merge 4 commits into bitcoin:master from Sjors:2024/01/github-self-hosted-ci changing 3 files +208 −13
  1. Sjors commented at 10:47 am on January 17, 2024: member

    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:

    • deduplicate yaml (can’t use anchors https://github.com/actions/runner/issues/1182)
    • add macOS native worker
    • add Windows native worker (I can’t easily test this myself, so might leave it followup)
    • prevent self hosted jobs from cluttering UI on the main
    • check behaviour against the gui repo
    • make resource consumption configurable (some function of nproc? or configured locally on the workers?)
    • documentation

    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.

  2. ci: refer to origin/master in linter script
    When this script is used as part of a Github Action, there is no master branch, only origin/master.
    51b9268d0b
  3. ci: vary /tmp/env 3144fb3e87
  4. ci: skip Github Actions on forks
    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.
    a3b5b87a6a
  5. ci: add self-hosted runners for GitHub 06878f08c7
  6. DrahtBot commented at 10:48 am on January 17, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

  7. maflcko commented at 11:21 am on January 17, 2024: member

    splurge on a Cirrus account, and since Github free minutes are limited

    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?

    Related: I wrote a gist for how to run CI on Ubuntu desktop with one runner per window.

    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:

    • Write code, format code.
    • Compile and run tests (manual, unit, functional, fuzz, …), depending on what was changed.
    • Make a pull request and wait for the CI result. (lint should be running and be complete within a minute or two)
    • In case there is a CI failure, the fix is either trivial and obvious, in which case a quick force push can be done
    • If not, the CI failure should be reproduced locally by using sanitizers (valigrind, asan, …) or libFuzzer, or whatever config flag is used in the CI task that caused the failure.
    • Only if the failure can not be reproduced in the normal workflow, the CI task can be debugged via the CI runner. (Depending on how many pushes are needed, people are free to use the public runners in this repo, I’d say)
  8. in ci/lint/06_script.sh:14 in 51b9268d0b outdated
    10@@ -11,7 +11,7 @@ set -ex
    11 if [ -n "$LOCAL_BRANCH" ]; then
    12   # To faithfully recreate CI linting locally, specify all commits on the current
    13   # branch.
    14-  COMMIT_RANGE="$(git merge-base HEAD master)..HEAD"
    15+  COMMIT_RANGE="$(git merge-base HEAD origin/master)..HEAD"
    


    maflcko commented at 11:25 am on January 17, 2024:
    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.
  9. in ci/test/02_run_container.sh:55 in 3144fb3e87 outdated
    51@@ -52,7 +52,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
    52                   --mount "type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" \
    53                   --mount "type=volume,src=${CONTAINER_NAME}_depends_SDKs_android,dst=$DEPENDS_DIR/SDKs/android" \
    54                   --mount "type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" \
    55-                  --env-file /tmp/env \
    56+                  --env-file /tmp/env-$USER \
    


    maflcko commented at 11:25 am on January 17, 2024:
    May be good to explain this change?

    Sjors commented at 1:19 pm on January 17, 2024:

    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.


    maflcko commented at 1:29 pm on January 17, 2024:

    Ah. May be better to use $CONTAINER_NAME then?

    Could be a separate pull request, because this is a bug fix?

  10. Sjors commented at 1:28 pm on January 17, 2024: member

    A Cirrus account is free for public open source projects, as well as persistent workers connected to it.

    Oh that’s nice.

    GitHub “free” minutes should also be unlimited for public open source, no?

    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 a3b5b87a6a366579d6aa286956462dfadc6a13d9.


    Related: I wrote a gist for how to run CI on Ubuntu desktop with one runner per window.

    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.

    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.

    If not, the CI failure should be reproduced locally by using sanitizers (valigrind, asan, …) or libFuzzer, or whatever config flag is used in the CI task that caused the failure.

  11. maflcko commented at 1:32 pm on January 17, 2024: member

    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?

  12. maflcko commented at 1:36 pm on January 17, 2024: member

    A Cirrus account is free for public open source projects, as well as persistent workers connected to it.

    Oh that’s nice.

    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.

  13. Sjors commented at 1:42 pm on January 17, 2024: member
    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
  14. Sjors commented at 1:47 pm on January 17, 2024: member

    So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus.

    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?

  15. maflcko commented at 1:47 pm on January 17, 2024: member

    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

    Yes, see https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42

  16. maflcko commented at 1:48 pm on January 17, 2024: member

    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?

    The workers currently use Cirrus self-hosted ones, so no changes are needed. This is documented in the cirrus yaml

  17. Sjors commented at 2:05 pm on January 17, 2024: member

    A Cirrus account is free for public open source projects, as well as persistent workers connected to it.

    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.

  18. maflcko commented at 2:12 pm on January 17, 2024: member
    The assumptions for the runners are documented where they are configured: https://github.com/bitcoin/bitcoin/blob/master/.cirrus.yml#L16
  19. Sjors commented at 2:17 pm on January 17, 2024: member
    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.
  20. Sjors commented at 2:49 pm on January 18, 2024: member
    Closing in favour of #29274
  21. Sjors closed this on Jan 18, 2024


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-24 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me