search

NixOS / hydra

Hydra, the Nix-based continuous build system
http://nixos.org/hydra
GNU General Public License v3.0
1.17k stars 300 forks source link

Prepare for CA derivation support with lower impact changes #1316

Closed Ericson2314 closed 8 months ago

Ericson2314 commented 10 months ago

This is just C++ changes without any Perl / Frontend / SQL Schema changes.

The idea is that it should be possible to redeploy Hydra with these chnages with (a) no schema migration and also (b) no regressions. We should be able to much more safely deploy these to a staging server and then production hydra.nixos.org.

Extracted from #875

Ericson2314 commented 10 months ago

OK I tried to do a pretty through review of the code, originally by @thufschmitt above.

By @thufschmitt's own admission, a lot of this stuff duplicates code in Nix for CA derivations, but Hydra already duplicates Nix's scheduling logic quite badly. I don't think there is an easy way to "just not make that problem worse" --- I think the only way to not add more duplication would be to deduplicate the stuff that is already there. That, however, is a much larger project.

I therefore think the right thing to do is merge what we've got. It has been on a low simmer tested after all these years, after all, and with our new staging workflow, will be tested some more.

As the PR description says, I pulled out this change to avoid the scheme change, and misc web-app changes that went with it. While there is no real problem doing a schema change with the staging instance, it is a much larger headache with the prod instance, and I wouldn't want to race ahead with staging just to hit a brick wall with prod.

Without the scheme changes, and with the ca-derivations experimental feature disabled, I think we can quickly get this code onto master, hydra.ngi0.nixos.org, and then hydra.nixos.org. This would explicitly be not trying to enable any new functionality, but just making sure in a very-easy-to-roll-back way (again, no migrations) that there are no regressions.

If we can do that, then the rest of https://github.com/NixOS/hydra/pull/875 is a lot smaller, and we can do (a) performing the migration, (b) finally enabling ca-derivations as separate subsequent steps without the clutter of this first one.

check boxes for this plan added to https://github.com/NixOS/hydra/issues/838

thufschmitt commented 10 months ago

@Ericson2314 : Thanks a lot for that. That looks like a good plan.

I'm a bit stretched out, but I'll try to block enough time to review this and follow-ups (I'm the one who committed this in the first place after all :p )

Ericson2314 commented 10 months ago

@thufschmitt Thank you! Yeah I'd like to think this plan has enough fail-safes that is is OK if you don't review it, but you definitely have the most context having figured out what needs to happen.

I think I'll add some review comments where I'd like to write a code comment explaining why something, and if you could just provide the missing tidbit (if you remember it after all these years!) I think that would be plenty.

Ericson2314 commented 10 months ago

After discussing with @tomberek https://github.com/NixOS/nixos-org-configurations/pull/316/files this PR is now deployed to https://hydra.ngi0.nixos.org/.

I'll edit a CA derivations tracking issue for these things. Done: https://github.com/NixOS/hydra/issues/838

Ericson2314 commented 10 months ago

I think I want to fix the realisations issue first, now that I think I know how to.

nixos-discourse commented 9 months ago

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

https://discourse.nixos.org/t/content-addressed-nix-call-for-testers/12881/221

Ericson2314 commented 8 months ago

As discussed with @delroth, now that hydra.nixos.org is caught up with Nix 2.19, the next step is merging this!