drv-daemon: ad-hoc Builds + queue-runner handoff - #1692
Conversation
There was a problem hiding this comment.
This should not be called this, dynamic derivations and import-from-derivation are completely separate features.
We want one test for "ad hoc jobs" and one test for a regular job (like with the project/jobset stuff) but using import from derivation.
There was a problem hiding this comment.
Also this test isn't using content-addressing (on purpose, because IFD is not experimental)
| my $ctx = test_context( | ||
| nix_config => qq| | ||
| experimental-features = ca-derivations dynamic-derivations | ||
| |, | ||
| ); | ||
|
|
There was a problem hiding this comment.
| my $ctx = test_context( | |
| nix_config => qq| | |
| experimental-features = ca-derivations dynamic-derivations | |
| |, | |
| ); |
| sub _spawn_builder { | ||
| my ($self) = @_; | ||
| my $ctx = $self->{ctx}; | ||
|
|
||
| $self->_spawn( | ||
| builder => "Builder", | ||
| [ | ||
| "hydra-builder", | ||
| "--gateway-endpoint", "http://[::1]:$self->{grpc_port}", | ||
| ], | ||
| env => { | ||
| NIX_REMOTE => $ctx->{builder}{nix_store_uri}, | ||
| NIX_CONF_DIR => $ctx->{builder}{nix_conf_dir}, | ||
| NIX_STATE_DIR => $ctx->{builder}{nix_state_dir}, | ||
| NIX_STORE_DIR => $ctx->{builder}{nix_store_dir}, | ||
| RUST_LOG => "hydra_builder=debug,info", | ||
| }, | ||
| ); | ||
|
|
||
| _wait_for($self->{ua}, "$self->{base_url}/status/machines", sub { | ||
| shift->decoded_content =~ /"hostname"/; | ||
| }) or die "Timed out waiting for builder to register\n"; | ||
| } |
There was a problem hiding this comment.
Very sus! we already have code to start the builder, we should not need new such code.
| sub _get_random_port { | ||
| my ($min, $max) = @_; | ||
| while (1) { | ||
| my $port = $min + int(rand($max - $min + 1)); | ||
| my $sock = IO::Socket::IP->new( | ||
| LocalAddr => '::', | ||
| LocalPort => $port, | ||
| Proto => 'tcp', | ||
| ReuseAddr => 0, | ||
| ); | ||
| if ($sock) { | ||
| close($sock); | ||
| return $port; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Should be shared with other htings
|
I think it would be best to be able to run the entire test suite (or at least the parts doing evaluation) with and without the new drv daemon. That means that the new IFD and ad hoc jobs tests would require the new daemon, but every other test would run twice in both modes. |
| // unix:// ignores NIX_STORE_DIR, so NIX_REMOTE must include the logical store dir. | ||
| auto drvDaemonSocket = config->getStrOption("drv_daemon_socket"); | ||
| auto nixStoreDir = config->getStrOption("nix_store_dir", "/nix/store"); | ||
|
|
||
| jobset.pid = startProcess([&]() { | ||
| if (!drvDaemonSocket.empty()) { | ||
| auto remote = "unix://" + urlEncodePath(drvDaemonSocket) | ||
| + "?store=" + urlEncodePath(nixStoreDir); | ||
| setenv("NIX_REMOTE", remote.c_str(), 1); | ||
| } |
There was a problem hiding this comment.
I am not sure, but this feels like the wrong layer of abstraction. I forgot that hydra-evaluator was its own separate demon, not one of the "scripts".
What that means is that:
- If the setting stays in the config, it should be holistic --- do we want evaluations to use the drv daemon or not? That is the actual policy choice here.
- If we instead mess around with the underlying mechanisms, we could just set
NIX_REMOTEfor the entire hydra-evaluator from the outside, not within it. I.e. it is the responsibility of however the daemons set up and run to wire things up correctly.
| /* Encode path/query separators before embedding operator paths in a unix:// store URL. */ | ||
| static std::string urlEncodePath(const std::string & s) | ||
| { | ||
| std::string out; | ||
| out.reserve(s.size()); | ||
| for (unsigned char c : s) { | ||
| bool safe = std::isalnum(c) | ||
| || c == '-' || c == '.' || c == '_' || c == '~' | ||
| || c == '/' || c == ':'; | ||
| if (safe) { | ||
| out.push_back(static_cast<char>(c)); | ||
| } else { | ||
| char buf[4]; | ||
| std::snprintf(buf, sizeof(buf), "%%%02X", c); | ||
| out.append(buf); | ||
| } | ||
| } | ||
| return out; | ||
| } | ||
|
|
There was a problem hiding this comment.
Per b5e42d4#r3171126384 we might not even need this (because the environment variables are set externally and not in this C++, option 2).
If we got with option 1, I think we can replace this with a Nix function, not hand-roll it.
| Logical store directory. Must match the store dir used by the | ||
| queue runner / builder; the value is also what the evaluator | ||
| appends as `?store=<dir>` when it points `NIX_REMOTE` at the | ||
| socket, since `unix://` connections ignore `NIX_STORE_DIR`. |
There was a problem hiding this comment.
I don't think that is true, I don't think they ignore NIX_STORE_DIR
There was a problem hiding this comment.
But also, we much rather not rely on NIX_STORE_DIR and just use NIX_REMOTE, so maybe this fine.
| routeEvaluatorIfds = lib.mkOption { | ||
| type = lib.types.bool; | ||
| default = true; | ||
| description = '' | ||
| When true, wire hydra-evaluator to this daemon and order it after hydra-drv-daemon.service. | ||
|
|
||
| Note: routing only takes effect when | ||
| `allowImportFromDerivation` is also enabled — otherwise | ||
| `nix-eval-jobs` rejects every IFD before the daemon can see it. | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
I don't understand why this is a setting for hydra-drv-daemon itself. That seems wrong --- why does it care who is using it and for what purpose?
| allowImportFromDerivation = lib.mkOption { | ||
| type = lib.types.bool; | ||
| default = false; | ||
| description = '' | ||
| When true, enable IFD evaluation in hydra.conf. This lets untrusted evals run builders, so it stays opt-in. | ||
| ''; | ||
| }; |
ad5b267 to
5f6e67f
Compare
Port hydra-evaluator from C++ to Rust, removing the last C++ executable from the project. The new implementation uses `sqlx` and `tokio`, matching the async patterns already established by hydra-queue-runner and hydra-builder. The scheduling logic is a fairly direct port; what gets explicit treatment is the error-recovery story, which did not survive translation on its own. The C++ evaluator leaned on pqxx throwing on a broken connection and the process exiting into systemd's `Restart=always`, which resynced all state; `sqlx`'s conveniences paper over connection loss instead. So the `db` crate's listener now disables `sqlx`'s eager reconnect so that connection loss surfaces as a stream error (see the comment there — silently dropped NOTIFYs would otherwise strand user-triggered evaluations), and both the main loop and the DB monitor exit for a supervisor restart on a broken connection. Other behaviors deliberately carried over from the C++: forgetting a disabled or deleted jobset kills its in-flight `hydra-eval-jobset` (the C++ `Pid` destructor did this), a failed spawn runs the same cleanup as a failed eval so `startTime` cannot stick forever, and the reaper holds the scheduler lock across its cleanup transaction so a freshly restarted eval cannot have its `startTime` clobbered. The SQL lives in `src/queries.rs` as compile-time-checked `sqlx::query!` macros on an extension trait over `db::Connection` — kept in this crate rather than the shared `db` crate because this slice of the schema only matters to the evaluator. The `db` crate contributes pooled acquisition (`Database::from_env`, reading `HYDRA_DATABASE_URL` via shared constants the queue runner now also uses) and a `raw()` escape hatch for exactly this pattern. Timestamps are `i32` to match the schema's `INT4` (see the comment on `JobsetSchedulingInfo`). Regenerating the `.sqlx` cache also uncovered a latent bug in the db crate: `get_drv_path_from_build_step` treated `drvPath` as nullable, but the schema declares it `not null`. Build/packaging: the C++ build is removed from `subprojects/hydra/meson.build`; the new `subprojects/hydra-evaluator/package.nix` follows the same per-binary pattern as `hydra-queue-runner` and `hydra-builder` (properly sharing the dep crates between these builds is left for later); the NixOS web-app module gains an `evaluatorExecutable` option matching how the queue runner and builder are discovered via their own modules; and stale references in `hydra-tests/meson.build` and `dev-shell.nix` are cleaned up. Co-authored-by: Jörg Thalheim <joerg@thalheim.io> Co-authored-by: Claude <noreply@anthropic.com>
Add hydra-drv-daemon, a nix-daemon protocol endpoint that turns build requests into ad-hoc Hydra Builds and waits for queue-runner completion. Read operations and store uploads are proxied to the upstream nix-daemon.
badcd81 to
07572d4
Compare
The daemon built in the previous commit has no way to be started yet. Add the NixOS service and the dev-shell Procfile entry so it runs as part of a normal Hydra deployment, listening on its socket. Nothing points at that socket yet — enabling the service on its own changes no behaviour. Routing evaluation through it is a separate, opt-in step.
An import-from-derivation builds on the evaluator host, which is the one machine in a Hydra deployment with no build capacity budgeted for it. Pointing the evaluator's `NIX_REMOTE` at the drv-daemon socket turns each IFD into an ordinary `Builds` row instead, so the queue runner and builders handle it. The wiring is environment-only — the evaluator itself learns nothing about the daemon. `routeIfdsThroughDaemon` in `nixos-modules/evaluator-drv-daemon.nix` sets the variable on the unit, and the dev Procfile does the same when the socket is present; see the comments there for why it stays opt-in and why it is inert without `allow_import_from_derivation`.
Add an end-to-end test stack for submitting a derivation through hydra-drv-daemon and building it through queue-runner/builder. The fixture is input-addressed because CA realisations are not surfaced yet.
07572d4 to
aa597ac
Compare
Motivation
Hydra currently builds import-from-derivation work on the evaluator host. Those builds bypass the queue runner, builder fleet, Hydra build history, and the normal logs/operators use to debug failures.
Solution
This PR adds hydra-drv-daemon, a Nix daemon protocol service that turns evaluator build requests into ad-hoc Hydra builds. When configured, hydra-evaluator points IFD-capable child processes at this daemon via NIX_REMOTE, so IFDs are built by the normal queue-runner / builder path instead of locally on the evaluator.
The daemon inserts an internal adhoc/adhoc build row, notifies the queue runner, waits for build_finished, and returns the completed build result to the Nix client. Read-side store operations and uploads are proxied to the upstream nix-daemon.