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

enable nixos tests for fetchGit #9808

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DavHau
Copy link
Member

@DavHau DavHau commented Jan 19, 2024

Motivation

Support fetchTree stabilization by running the relevant nixos tests for each PR

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@thufschmitt
Copy link
Member

thufschmitt commented Jan 19, 2024

Discussed during the Nix maintainers meeting today. We'll close that one because it's imposing a too big overhead on the CI. @roberth will look at improving the CI situation (implement #7674 )

  • Not as-it-is because taking too long on GH actions
  • @roberth will look for better solutions as part of his CI work
  • @thufschmitt: old talks about just paying for a beefier CI runner. But was abandonned in favor of getting a hydra deployment for that (which never happened)
    • Something to think about if the rest doesn't work
  • Close

@edolstra
Copy link
Member

edolstra commented Jan 23, 2024

We'll close that one because it's imposing a too big overhead on the CI.

@thufschmitt By "that one", do you mean this one? Looks like it adds about 4 minutes to the ubuntu-latest job. Running some VM test would be useful, but I don't know if the fetchGit one is really worth it. tests/nixos/remote-builds.nix would be my preference because it tests building in a "real" environment and remote building via SSH.

@DavHau
Copy link
Member Author

DavHau commented Jan 23, 2024

We can probably minimize the added time by making the attributes available to 'nix flake check', then we don't have to go through an additional evaluation and fetching cycle

@thufschmitt
Copy link
Member

@DavHau do you want to give this a stab? I might indeed help.

Also, looking at the CI logs, I see this:

Fri, 19 Jan 2024 06:30:22 GMT building '/nix/store/95m7r3ql5j62yjfd5sy4hcbzx6v59jqn-gitea-1.19.4.drv'...
[...] Stuff happening
Fri, 19 Jan 2024 06:30:34 GMT building '/nix/store/a0qgw55zivr312nsga0c622hy84hxdwa-nixos-vm.drv'...
Fri, 19 Jan 2024 06:32:03 GMT gitea> buildPhase completed in 1 minutes 38 seconds
Fri, 19 Jan 2024 06:32:03 GMT gitea> running tests
Fri, 19 Jan 2024 06:33:35 GMT gitea> ?        code.gitea.io/gitea     [no test files]
Fri, 19 Jan 2024 06:33:35 GMT gitea> checkPhase completed in 1 minutes 32 seconds
Fri, 19 Jan 2024 06:33:35 GMT gitea> installing
Fri, 19 Jan 2024 06:33:35 GMT gitea> post-installation fixup
Fri, 19 Jan 2024 06:33:35 GMT gitea> shrinking RPATHs of ELF executables and libraries in /nix/store/qbrfbpckbrzkbwx64pmx4as03jc6clbq-gitea-1.19.4
Fri, 19 Jan 2024 06:33:35 GMT gitea> shrinking /nix/store/qbrfbpckbrzkbwx64pmx4as03jc6clbq-gitea-1.19.4/bin/.gitea-wrapped
Fri, 19 Jan 2024 06:33:36 GMT gitea> checking for references to /build/ in /nix/store/qbrfbpckbrzkbwx64pmx4as03jc6clbq-gitea-1.19.4...
Fri, 19 Jan 2024 06:33:36 GMT gitea> patching script interpreter paths in /nix/store/qbrfbpckbrzkbwx64pmx4as03jc6clbq-gitea-1.19.4
Fri, 19 Jan 2024 06:33:36 GMT gitea> stripping (with command strip and flags -S -p) in  /nix/store/qbrfbpckbrzkbwx64pmx4as03jc6clbq-gitea-1.19.4/bin
Fri, 19 Jan 2024 06:33:36 GMT gitea> shrinking RPATHs of ELF executables and libraries in /nix/store/lgkijl0g0a0z8qncpshzycli9yxx8by0-gitea-1.19.4-data
Fri, 19 Jan 2024 06:33:36 GMT gitea> checking for references to /build/ in /nix/store/lgkijl0g0a0z8qncpshzycli9yxx8by0-gitea-1.19.4-data...
Fri, 19 Jan 2024 06:33:37 GMT gitea> patching script interpreter paths in /nix/store/lgkijl0g0a0z8qncpshzycli9yxx8by0-gitea-1.19.4-data
Fri, 19 Jan 2024 06:33:38 GMT building '/nix/store/1f4la8ggxsx0i2n794by9h24wz2lr07v-app.ini.drv'...

It looks like

  1. Gitea gets rebuild in the CI
  2. It takes three minutes (out of the four required for the whole command to run)
  3. Most of these are in the hot path (at least the logs don't show anything else happening)

I've restarted the workflow. If that's indeed what's taking most of the time, the new run should be noticeably faster since Gitea will now be in the CI's cachix cache.

@DavHau
Copy link
Member Author

DavHau commented Jan 24, 2024

Yes, gitea on that nixpkgs release has been marked insecure, therefore it's not cached on hydra.
Now the test is already down to 1 min,
I'll refactor the this to nix flake check tomorrow, then we'll hopefully see even better results.
I'll also enable the remote-building test mentioned by @edolstra

@DavHau DavHau force-pushed the nixos-tests-git branch 2 times, most recently from 03207dd to bbb99a8 Compare January 25, 2024 06:22
@DavHau
Copy link
Member Author

DavHau commented Jan 25, 2024

Alright, I made this a flake check and also added the remote builder test. The total overhead introduced by this PR now seems to be between 2-3 minutes.

You could also set up larger github runners which would reduce the time of all checks to a fraction. Currently the runners have only 2 cores, but you could get up to 64.

@thufschmitt
Copy link
Member

Now the test is already down to 1 min,
[...]
The total overhead introduced by this PR now seems to be between 2-3 minutes.

I'm confused here. Are you saying that merging everything into checks made it run slower overall?

You could also set up larger github runners which would reduce the time of all checks to a fraction. Currently the runners have only 2 cores, but you could get up to 64.

Yes, I'd very much like us to do that. This was abandoned in the past because of a proposal to directly get Hydra as the CI (but that never happens), but we should certainly revisit it. @roberth also offered to set up Hercules CI here, but we can have beefier GH runners as a stop-gap measure in the meantime any way.

@DavHau
Copy link
Member Author

DavHau commented Jan 25, 2024

It might have increased from 1 min to 2-3 min because I also enabled the remote builder test.

@DavHau
Copy link
Member Author

DavHau commented Jan 26, 2024

It seems like the nixos tests don't terminate properly some times and then the runner times out. Not sure why.

@DavHau
Copy link
Member Author

DavHau commented Jan 26, 2024

Oh the remote builder job failed on hydra as well, so it doesn't seem to be a problem with the github runners. I now remove the remote builders job again, as it should not be the purpose of this PR to fix it.

Copy link

@krisu-pl krisu-pl left a comment

Choose a reason for hiding this comment

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

LGTM

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-01-19-nix-team-meeting-minutes-116/38837/1

@DavHau
Copy link
Member Author

DavHau commented Jan 26, 2024

The timeout issue still occurs. Not sure how to deal with it.

@DavHau
Copy link
Member Author

DavHau commented Feb 28, 2024

All tests are green now

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.

6 participants