guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libstore: stop using ssh connection sharing

Browse Source 
our implementation is mostly unused, completely untested, and simply
breaks when users configure connection sharing independently. we can
safely delete this "feature" and inherit user configuration instead.

also have the remote build test check that connection sharing works.

fixes #304, fixes #644

Change-Id: Iea44cc0f8e51a1d231ad186a95c7e310bbfeb303
This commit is contained in:
eldritch horrors
2025-04-21 13:43:44 +00:00
parent 2238f7a8ab
commit f92235e1d2
6 changed files with 42 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/manual/rl-next/ssh-connection-sharing-removed.md
+14
View File
@@ -0,0 +1,14 @@
---
synopsis: "Remove lix-initiated ssh connection sharing"
issues: [fj#304, fj#644]
cls: [3005]
category: Fixes
credits: [horrors]
---
Lix no longer explicitly requests ssh connection sharing when connecting to remote stores.
This may impact command latency when `NIX_REMOTE` is set to a `ssh://` or `ssh-ng://` url,
or if `--store` is specified. Remote build connections did not use ssh connection sharing.
Connection sharing configuration is now inherited from user configuration at all times. It
is now advisable to configure connection sharing for remote builders for improved latency.
lix/libstore/legacy-ssh-store.cc
-2
View File
@@ -118,8 +118,6 @@ struct LegacySSHStore final : public Store
config_.port,
config_.sshKey,
config_.sshPublicHostKey,
// Use SSH master only if using more than 1 connection.
connections->capacity() > 1,
config_.compress,
config_.logFD)
{
lix/libstore/ssh-store.cc
-2
View File
@@ -42,8 +42,6 @@ public:
config_.port,
config_.sshKey,
config_.sshPublicHostKey,
// Use SSH master only if using more than 1 connection.
connections->capacity() > 1,
config_.compress)
{
}
lix/libstore/ssh.cc
+4 -72
View File
@@ -8,13 +8,12 @@
namespace nix {
SSHMaster::SSHMaster(const std::string & host, const std::optional<uint16_t> port, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD)
SSHMaster::SSHMaster(const std::string & host, const std::optional<uint16_t> port, const std::string & keyFile, const std::string & sshPublicHostKey, bool compress, int logFD)
: host(host)
, port(port)
, fakeSSH(host == "localhost")
, keyFile(keyFile)
, sshPublicHostKey(sshPublicHostKey)
, useMaster(useMaster && !fakeSSH)
, compress(compress)
, logFD(logFD)
{
@@ -44,23 +43,10 @@ void SSHMaster::addCommonSSHOpts(Strings & args)
}
if (compress)
args.push_back("-C");
args.push_back("-oPermitLocalCommand=yes");
args.push_back("-oLocalCommand=echo started");
}
bool SSHMaster::isMasterRunning() {
Strings args = {"-O", "check", host};
addCommonSSHOpts(args);
auto res = runProgram(RunOptions {.program = "ssh", .args = args, .mergeStderrToStdout = true});
return res.first == 0;
}
std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string & command)
{
Path socketPath = startMaster();
Pipe in, out;
in.create();
out.create();
@@ -70,7 +56,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
options.dieWithParent = false;
std::optional<Finally<std::function<void()>>> resumeLoggerDefer;
if (!fakeSSH && !useMaster) {
if (!fakeSSH) {
logger->pause();
resumeLoggerDefer.emplace([&]() { logger->resume(); });
}
@@ -95,11 +81,9 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
} else {
args = { "ssh", host.c_str(), "-x" };
addCommonSSHOpts(args);
if (socketPath != "")
args.insert(args.end(), {"-S", socketPath});
}
args.push_back(command);
args.push_back(fmt("echo started; %s", command));
execvp(args.begin()->c_str(), stringsToCharPtrs(args).data());
// could not exec ssh/bash
@@ -112,7 +96,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
// Wait for the SSH connection to be established,
// So that we don't overwrite the password prompt with our progress bar.
if (!fakeSSH && !useMaster && !isMasterRunning()) {
{
std::string reply;
try {
reply = readLine(out.readSide.get());
@@ -130,56 +114,4 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
return conn;
}
Path SSHMaster::startMaster()
{
if (!useMaster) return "";
auto state(state_.lock());
if (state->sshMaster) return state->socketPath;
state->socketPath = (Path) *state->tmpDir + "/ssh.sock";
Pipe out;
out.create();
ProcessOptions options;
options.dieWithParent = false;
logger->pause();
Finally cleanup = [&]() { logger->resume(); };
if (isMasterRunning())
return state->socketPath;
state->sshMaster = startProcess([&]() {
restoreProcessContext();
close(out.readSide.get());
if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1)
throw SysError("duping over stdout");
Strings args = { "ssh", host.c_str(), "-M", "-N", "-S", state->socketPath };
addCommonSSHOpts(args);
execvp(args.begin()->c_str(), stringsToCharPtrs(args).data());
throw SysError("unable to execute '%s'", args.front());
}, options);
out.writeSide.reset();
std::string reply;
try {
reply = readLine(out.readSide.get());
} catch (EndOfFile & e) { }
if (reply != "started") {
printTalkative("SSH master stdout first line: %s", reply);
throw Error("failed to start SSH master connection to '%s'", host);
}
return state->socketPath;
}
}
lix/libstore/ssh.hh
+1 -7
View File
@@ -17,25 +17,21 @@ private:
bool fakeSSH;
const std::string keyFile;
const std::string sshPublicHostKey;
const bool useMaster;
const bool compress;
const int logFD;
struct State
{
Pid sshMaster;
std::unique_ptr<AutoDelete> tmpDir;
Path socketPath;
};
Sync<State> state_;
void addCommonSSHOpts(Strings & args);
bool isMasterRunning();
public:
SSHMaster(const std::string & host, const std::optional<uint16_t> port, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD = -1);
SSHMaster(const std::string & host, const std::optional<uint16_t> port, const std::string & keyFile, const std::string & sshPublicHostKey, bool compress, int logFD = -1);
struct Connection
{
@@ -44,8 +40,6 @@ public:
};
std::unique_ptr<Connection> startCommand(const std::string & command);
Path startMaster();
};
}
tests/nixos/remote-builds.nix
+23 -1
View File
@@ -75,7 +75,23 @@ in
virtualisation.writableStore = true;
virtualisation.additionalPaths = [ config.system.build.extraUtils ];
nix.settings.substituters = lib.mkForce [ ];
programs.ssh.extraConfig = "ConnectTimeout 30";
programs.ssh.extraConfig = ''
ConnectTimeout 30
Host builder2-cs
HostName builder2
ControlMaster auto
ControlPersist yes
ControlPath ~/.ssh/builder2-cs.sock
'';
specialisation.with-sharing.configuration.nix.buildMachines = lib.mkForce [
{
hostName = "builder2-cs";
sshUser = "root";
sshKey = "/root/.ssh/id_ed25519";
system = "i686-linux";
maxJobs = 1;
}
];
};
};
@@ -126,6 +142,12 @@ in
# Test whether the build hook automatically skips unavailable builders.
builder1.block()
client.succeed("nix-build ${expr nodes.client 4}")
# test that connection sharing doesn't break anything
client.succeed("/run/current-system/specialisation/with-sharing/bin/switch-to-configuration test")
client.succeed("ssh builder2-cs true")
client.succeed("ssh -O check builder2-cs")
client.succeed("nix-build ${expr nodes.client 6}")
'';
};
}