diff options
author | Maximilian Bosch <maximilian@mbosch.me> | 2024-10-12 23:42:59 +0200 |
---|---|---|
committer | Maximilian Bosch <maximilian@mbosch.me> | 2024-10-14 06:01:18 +0000 |
commit | 4682e40183b86972e5a1ef8f17e5366b9b3a8b2c (patch) | |
tree | ff9fd4814236884808699f2cc422c9bca75d58f3 /src/libstore | |
parent | 326cbecb61d42ce73caa391cdcb21acb2581bf7c (diff) |
ssh-ng: better way to keep SSH errors visible
A better fix than in 104448e75d87d03d2fb0b4ac96d4da72e1dae50d, hence a
revert + the fix.
It turns out that this commit has the side-effect that when having e.g.
`StrictHostKeyChecking=accept-new` for a remote builder, the warnings à la
Warning: Permanently added 'builder' (ED25519) to the list of known hosts.
actually end up in the derivation's log whereas hostkey verification
errors don't, but only in the stderr of the `nix-build` invocation
(which was the motivation for the patch).
This change writes the stderr from the build-hook to
* the daemon's stderr, so that the SSH errors appear in the journal
(which was the case before 104448e75d87d03d2fb0b4ac96d4da72e1dae50d)
* the client's stderr, as a log message
* NOT to the drv log (this is handled via `handleJSONLogMessage`)
I tried to fix the issue for legacy-ssh as well, but failed and
ultimately decided to not bother.
I know that we'll sooner or later replace the entire component, however
this is the part of the patch I have working for a while, so I figured I
might still submit it for the time being.
Change-Id: I21ca1aa0d8ae281d2eacddf26e0aa825272707e5
Diffstat (limited to 'src/libstore')
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 1 | ||||
-rw-r--r-- | src/libstore/machines.cc | 2 | ||||
-rw-r--r-- | src/libstore/ssh-store.cc | 8 |
3 files changed, 3 insertions, 8 deletions
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7f72efa6a..96140e10b 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1211,6 +1211,7 @@ HookReply DerivationGoal::tryBuildHook() else { s += "\n"; writeLogsToStderr(s); + logger->log(lvlInfo, s); } } diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index d0897b81f..7314e3177 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -68,11 +68,11 @@ ref<Store> Machine::openStore() const { Store::Params storeParams; if (storeUri.starts_with("ssh://")) { + storeParams["log-fd"] = "4"; storeParams["max-connections"] = "1"; } if (storeUri.starts_with("ssh://") || storeUri.starts_with("ssh-ng://")) { - storeParams["log-fd"] = "4"; if (sshKey != "") storeParams["ssh-key"] = sshKey; if (sshPublicHostKey != "") diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index fb60326c1..5c1fc0c1f 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -30,11 +30,6 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore { public: - // Hack for getting remote build log output. - // Intentionally not in `SSHStoreConfig` so that it doesn't appear in - // the documentation - const Setting<int> logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; - SSHStore(const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) , RemoteStoreConfig(params) @@ -49,8 +44,7 @@ public: sshPublicHostKey, // Use SSH master only if using more than 1 connection. connections->capacity() > 1, - compress, - logFD) + compress) { } |