ssh-ng: also store build logs to make them accessible by nix log
#6029
Conversation
Ma27
changed the title
nix lognix log
|
@edolstra @Ericson2314 if we want to avoid the double-parsing of JSON messages, we should IMHO add another signature to Anything else to be fixed before this can be merged? %) |
|
cc @Ericson2314 updated accordingly :) |
|
As I mentioned in chat, this looks good to me now except that it would be great to have a test. Later, I am hoping we can make do this logging stuff a bit more abstractly / separate it from the rest of the scheduler, but I don't want that refactor to cause a regression! |
Yup, I started hacking on that one yesterday, but got too tired at some point %) |
|
@Ericson2314 updated accordingly and added a test that failed for me on master and passes on this branch :) Details are in the commit messages %) |
|
If I read the code correctly, this stores the log locally as the remote sends it during build. So I am correct that contrary to the http store used for cache.nixos.org, if I build remotely with |
|
That's correct. But I interpreted #5079 to be exactly what my problem (i.e. |
|
Quite ironically my actual use case when I opened this issue was "fetch logs from remote CI with nix log --option substituters ssh-ng://host". I wrote it in the issue differently as it looked easier to describe. |
|
This definitely fixes a bug; should we take the discussion of @symphorien actual issue back to #5079? I don't yet follow. |
|
You are right, I opened another issue #6146 |
|
Rebased onto latest master to resolve a merge conflict. ping @Ericson2314 @edolstra |
|
@edolstra done! |
|
@Ma27 Please rebase again? MacOS issue is fixed now. |
Right now when building a derivation remotely via
$ nix build -j0 -f . hello -L --builders 'ssh://builder'
it's possible later to read through the entire build-log by running
`nix log -f . hello`. This isn't possible however when using `ssh-ng`
rather than `ssh`.
The reason for that is that there are two different ways to transfer
logs in Nix through e.g. an SSH tunnel (that are used by `ssh`/`ssh-ng`
respectively):
* `ssh://` receives its logs from the fd pointing to `builderOut`. This
is directly passed to the "log-sink" (and to the logger on each `\n`),
hence `nix log` works here.
* `ssh-ng://` however expects JSON-like messages (i.e. `@nix {log data
in here}`) and passes it directly to the logger without doing anything
with the `logSink`. However it's certainly possible to extract
log-lines from this format as these have their own message-type in the
JSON payload (i.e. `resBuildLogLine`).
This is basically what I changed in this patch: if the code-path for
`builderOut` is not reached and a `logSink` is initialized, the
message was successfully processed by the JSON logger (i.e. it's in
the expected format) and the line is of the expected type (i.e.
`resBuildLogLine`), the line will be written to the log-sink as well.
Closes NixOS#5079
To avoid that JSON messages are parsed twice in case of
remote builds with `ssh-ng://`, I split up the original
`handleJSONLogMessage` into three parts:
* `parseJSONMessage(const std::string&)` checks if it's a message in the
form of `@nix {...}` and tries to parse it (and prints an error if the
parsing fails).
* `handleJSONLogMessage(nlohmann::json&, ...)` reads the fields from the
message and passes them to the logger.
* `handleJSONLogMessage(const std::string&, ...)` behaves as before, but
uses the two functions mentioned above as implementation.
In case of `ssh-ng://`-logs the first two methods are invoked manually.
A few notes: * The `echo hi` is needed to make sure that a file that can be read by `nix log` is properly created (i.e. some output is needed). This is known and to be fixed in NixOS#6051. * We explicitly ignore the floating-CA case here: the `$out` of `input3` depends on `$out` of `input2`. This means that there are actually two derivations - I assume that this is because at eval time (i.e. `nix-instantiate -A`) the hash of `input2` isn't known yet and the other .drv is created as soon as `input2` was built. This is another issue on its own, so we ignore the case here explicitly.
|
@Ericson2314 done! %) |
|
Thanks! |
|
@edolstra After a few round of review, I have approved this. I hope you have time to take a look soon! |
|
ping @edolstra @thufschmitt |
|
... and the CI is green, let’s merge. Thanks! |
Right now when building a derivation remotely via
it's possible later to read through the entire build-log by running
nix log -f . hello. This isn't possible however when usingssh-ngrather than
ssh.The reason for that is that there are two different ways to transfer
logs in Nix through e.g. an SSH tunnel (that are used by
ssh/ssh-ngrespectively):
ssh://receives its logs from the fd pointing tobuilderOut. Thisis directly passed to the "log-sink" (and to the logger on each
\n),hence
nix logworks here.ssh-ng://however expects JSON-like messages (i.e.@nix {log data in here}) and passes it directly to the logger without doing anythingwith the
logSink. However it's certainly possible to extractlog-lines from this format as these have their own message-type in the
JSON payload (i.e.
resBuildLogLine).This is basically what I changed in this patch: if the code-path for
builderOutis not reached and alogSinkis initialized, themessage was successfully processed by the JSON logger (i.e. it's in
the expected format) and the line is of the expected type (i.e.
resBuildLogLine), the line will be written to the log-sink as well.Closes #5079
cc @edolstra @thufschmitt @Ericson2314 @symphorien