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

Parallel nix evaluation in github actions #947

Closed
wants to merge 6 commits into from

Conversation

henrirosten
Copy link
Collaborator

@henrirosten henrirosten commented Jan 13, 2025

Description of changes

Do not merge this, see: #947 (comment)

Various updates to Ghaf github actions, most importantly:

  • Move the nix evaluation step to its own workflow eval.yml, which runs the nix evaluation in parallel, splitting the evaluation to 8 subsets, each of which are executed on a separate github-hosted runner. In testing, this improves the eval action runtime from ~35 minutes down to ~7 minutes.
  • eval.sh allows filtering the evaluated targets via the -t option: the initial version in this PR will evaluate all Ghaf flake outputs that match the default filter 'packages.*'.
  • Start running the following actions on push or merge to main (and not only in PR): eval, build, and check to make it more apparent if one of those workflows would start failing after merge to main.

Checklist for things done

  • Summary of the proposed changes in the PR description
  • More detailed description in the commit message(s)
  • Commits are squashed into relevant entities - avoid a lot of minimal dev time commits in the PR
  • Contribution guidelines followed
  • Ghaf documentation updated with the commit - https://tiiuae.github.io/ghaf/
  • PR linked to architecture documentation and requirement(s) (ticket id)
  • Test procedure described (or includes tests). Select one or more:
    • Tested on Lenovo X1 x86_64
    • Tested on Jetson Orin NX or AGX aarch64
    • Tested on Polarfire riscv64
  • Author has run make-checks and it passes
  • All automatic Github Action checks pass - see actions
  • Author has added reviewers and removed PR draft status
  • Change requires full re-installation
  • Change can be updated with nixos-rebuild ... switch

Instructions for Testing

Tested in fork: https://github.com/henrirosten/ghaf/actions/workflows/eval.yml

  • List all targets that this applies to:
  • Is this a new feature
    • List the test steps to verify:
  • If it is an improvement how does it impact existing functionality?

@henrirosten henrirosten temporarily deployed to internal-build-workflow January 13, 2025 12:23 — with GitHub Actions Inactive
@henrirosten henrirosten changed the title Gh parallel nix eval Parallel nix evaluation in github actions Jan 13, 2025
@henrirosten henrirosten marked this pull request as ready for review January 13, 2025 12:36
Copy link
Collaborator

@joinemm joinemm left a comment

Choose a reason for hiding this comment

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

LGTM

@henrirosten henrirosten marked this pull request as draft January 14, 2025 07:56
@henrirosten
Copy link
Collaborator Author

Back to draft: let's not merge this yet, I'll add one more change to this PR in a moment.

@henrirosten henrirosten temporarily deployed to internal-build-workflow January 14, 2025 10:55 — with GitHub Actions Inactive
@henrirosten
Copy link
Collaborator Author

  • Kept the manual rebase step in build.yml, which we need to do since github.event.pull_request.merge isn't available for pull_request_target triggered events
  • Simplify the build-yml-check step

@henrirosten henrirosten marked this pull request as ready for review January 14, 2025 11:00
.github/eval.sh Show resolved Hide resolved
.github/eval.sh Show resolved Hide resolved
@Mic92
Copy link
Collaborator

Mic92 commented Jan 14, 2025

If you want to try out something quickly you can also go for:

https://github.com/nix-community/nix-github-actions

Don't know how this would scale for this project because we don't quite a few nixos closures here. https://github.com/DeterminateSystems/magic-nix-cache may work but don't know if we would run into too many rate limits here.

filter="$3"
echo "[+] Using filter: '$filter'"
# Output all flake output names
nix flake show --all-systems --json |\
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is just about evaluation, this will already evaluate everything. You shouldn't need nix-eval-jobs afterwards.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be also faster because nix-eval-jobs has to serialize drv files to disk.

@henrirosten
Copy link
Collaborator Author

henrirosten commented Jan 14, 2025

Based on #947 (comment), we should not merge this.
Instead, I'll push a series of PRs to implement the changes from this PR, keeping @Mic92's comments in mind.

@henrirosten henrirosten marked this pull request as draft January 14, 2025 13:58
@henrirosten
Copy link
Collaborator Author

Closing in favor of #948 and #949

@henrirosten henrirosten deleted the gh-parallel-nix-eval branch January 14, 2025 16:09
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.

3 participants