From a5c6347ff06ba09530fdf0e01828aaec89f6ceb6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Jul 2014 16:02:19 +0200 Subject: [PATCH] =?UTF-8?q?build-remote.pl:=20Use=20=E2=80=98nix-store=20-?= =?UTF-8?q?-serve=E2=80=99=20on=20the=20remote=20side?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes things more efficient (we don't need to use an SSH master connection, and we only start a single remote process) and gets rid of locking issues (the remote nix-store process will keep inputs and outputs locked as long as they're needed). It also makes it more or less secure to connect directly to the root account on the build machine, using a forced command (e.g. ‘command="nix-store --serve --write"’). This bypasses the Nix daemon and is therefore more efficient. Also, don't call nix-store to import the output paths. --- perl/lib/Nix/CopyClosure.pm | 96 +++++++++++++-------------------- perl/lib/Nix/SSH.pm | 81 +++++++++++++++++++++++++++- perl/lib/Nix/Store.pm | 2 +- perl/lib/Nix/Store.xs | 11 ++++ perl/local.mk | 2 +- scripts/build-remote.pl.in | 68 ++++++++--------------- src/libstore/remote-store.cc | 1 + src/nix-store/nix-store.cc | 37 +++++++++++-- src/nix-store/serve-protocol.hh | 2 + 9 files changed, 185 insertions(+), 115 deletions(-) diff --git a/perl/lib/Nix/CopyClosure.pm b/perl/lib/Nix/CopyClosure.pm index 131f0b5a4..f701a7c8a 100644 --- a/perl/lib/Nix/CopyClosure.pm +++ b/perl/lib/Nix/CopyClosure.pm @@ -3,76 +3,27 @@ package Nix::CopyClosure; use strict; use Nix::Config; use Nix::Store; +use Nix::SSH; use List::Util qw(sum); use IPC::Open2; -sub readN { - my ($bytes, $from) = @_; - my $res = ""; - while ($bytes > 0) { - my $s; - my $n = sysread($from, $s, $bytes); - die "I/O error reading from remote side\n" if !defined $n; - die "got EOF while expecting $bytes bytes from remote side\n" if !$n; - $bytes -= $n; - $res .= $s; - } - return $res; -} - - -sub readInt { - my ($from) = @_; - return unpack("L= 0x300; - }; - if ($@) { - chomp $@; - warn "$@; falling back to old closure copying method\n"; - return oldCopyTo(\@closure, @_); - } - # Send the "query valid paths" command with the "lock" option # enabled. This prevents a race where the remote host # garbage-collect paths that are already there. Optionally, ask # the remote host to substitute missing paths. - syswrite($to, pack("L 0) { - my @ps = splice(@$closure, 0, 1500); + while (scalar(@closure) > 0) { + my @ps = splice(@closure, 0, 1500); open(READ, "set -f; ssh $sshHost @{$sshOpts} nix-store --check-validity --print-invalid @ps|"); while () { chomp; diff --git a/perl/lib/Nix/SSH.pm b/perl/lib/Nix/SSH.pm index 584c44500..c8792043c 100644 --- a/perl/lib/Nix/SSH.pm +++ b/perl/lib/Nix/SSH.pm @@ -1,5 +1,16 @@ +package Nix::SSH; + use strict; use File::Temp qw(tempdir); +use IPC::Open2; + +our @ISA = qw(Exporter); +our @EXPORT = qw( + sshOpts openSSHConnection closeSSHConnection + readN readInt writeInt writeString writeStrings + connectToRemoteNix +); + our @sshOpts = split ' ', ($ENV{"NIX_SSHOPTS"} or ""); @@ -8,6 +19,7 @@ push @sshOpts, "-x"; my $sshStarted = 0; my $sshHost; + # Open a master SSH connection to `host', unless there already is a # running master connection (as determined by `-O check'). sub openSSHConnection { @@ -18,7 +30,7 @@ sub openSSHConnection { my $tmpDir = tempdir("nix-ssh.XXXXXX", CLEANUP => 1, TMPDIR => 1) or die "cannot create a temporary directory"; - + push @sshOpts, "-S", "$tmpDir/control"; # Start the master. We can't use the `-f' flag (fork into @@ -39,6 +51,7 @@ sub openSSHConnection { return 0; } + # Tell the master SSH client to exit. sub closeSSHConnection { if ($sshStarted) { @@ -48,6 +61,70 @@ sub closeSSHConnection { } } + +sub readN { + my ($bytes, $from) = @_; + my $res = ""; + while ($bytes > 0) { + my $s; + my $n = sysread($from, $s, $bytes); + die "I/O error reading from remote side\n" if !defined $n; + die "got EOF while expecting $bytes bytes from remote side\n" if !$n; + $bytes -= $n; + $res .= $s; + } + return $res; +} + + +sub readInt { + my ($from) = @_; + return unpack("L= 0x300; + + return ($from, $to, $pid); +} + + END { my $saved = $?; closeSSHConnection; $? = $saved; } -return 1; +1; diff --git a/perl/lib/Nix/Store.pm b/perl/lib/Nix/Store.pm index 191116ee5..89cfaefa5 100644 --- a/perl/lib/Nix/Store.pm +++ b/perl/lib/Nix/Store.pm @@ -15,7 +15,7 @@ our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } ); our @EXPORT = qw( isValidPath queryReferences queryPathInfo queryDeriver queryPathHash queryPathFromHashPart - topoSortPaths computeFSClosure followLinksToStorePath exportPaths + topoSortPaths computeFSClosure followLinksToStorePath exportPaths importPaths hashPath hashFile hashString addToStore makeFixedOutputPath derivationFromPath diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 07ccebe62..ff90616d3 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -179,6 +179,17 @@ void exportPaths(int fd, int sign, ...) } +void importPaths(int fd) + PPCODE: + try { + doInit(); + FdSource source(fd); + store->importPaths(false, source); + } catch (Error & e) { + croak(e.what()); + } + + SV * hashPath(char * algo, int base32, char * path) PPCODE: try { diff --git a/perl/local.mk b/perl/local.mk index 74c054e71..564683dff 100644 --- a/perl/local.mk +++ b/perl/local.mk @@ -27,7 +27,7 @@ ifeq ($(perlbindings), yes) Store_CXXFLAGS = \ -I$(shell $(perl) -e 'use Config; print $$Config{archlibexp};')/CORE \ - -D_FILE_OFFSET_BITS=64 + -D_FILE_OFFSET_BITS=64 -Wno-unused-variable -Wno-literal-suffix Store_ALLOW_UNDEFINED = 1 diff --git a/scripts/build-remote.pl.in b/scripts/build-remote.pl.in index 6dfa16d5c..687b0e131 100755 --- a/scripts/build-remote.pl.in +++ b/scripts/build-remote.pl.in @@ -4,7 +4,7 @@ use Fcntl qw(:DEFAULT :flock); use English '-no_match_vars'; use IO::Handle; use Nix::Config; -use Nix::SSH qw/sshOpts openSSHConnection/; +use Nix::SSH; use Nix::CopyClosure; use Nix::Store; no warnings('once'); @@ -90,6 +90,7 @@ if (defined $conf && -e $conf) { # Wait for the calling process to ask us whether we can build some derivation. my ($drvPath, $hostName, $slotLock); +my ($from, $to); REQ: while (1) { $_ = || exit 0; @@ -195,13 +196,15 @@ REQ: while (1) { # Connect to the selected machine. @sshOpts = ("-i", $machine->{sshKeys}, "-x"); $hostName = $machine->{hostName}; - if (openSSHConnection($hostName)) { - last REQ if system("ssh $hostName @sshOpts nix-builds-inhibited < /dev/null > /dev/null 2>&1") != 0; - warn "machine `$hostName' is refusing builds, trying other available machines...\n"; - closeSSHConnection; - } else { - warn "unable to open SSH connection to `$hostName', trying other available machines...\n"; - } + eval { + ($from, $to) = connectToRemoteNix($hostName, \@sshOpts); + # FIXME: check if builds are inhibited. + }; + last REQ unless $@; + print STDERR "$@"; + warn "unable to open SSH connection to `$hostName', trying other available machines...\n"; + $from = undef; + $to = undef; $machine->{enabled} = 0; } } @@ -220,18 +223,6 @@ my $maybeSign = ""; $maybeSign = "--sign" if -e "$Nix::Config::confDir/signing-key.sec"; -# Register the derivation as a temporary GC root. Note that $PPID is -# the PID of the remote SSH process, which, due to the use of a -# persistant SSH connection, should be the same across all remote -# command invocations for this session. -my $rootsDir = "@localstatedir@/nix/gcroots/tmp"; -system("ssh $hostName @sshOpts 'mkdir -m 1777 -p $rootsDir; ln -sfn $drvPath $rootsDir/\$PPID.drv'"); - -sub removeRoots { - system("ssh $hostName @sshOpts 'rm -f $rootsDir/\$PPID.drv $rootsDir/\$PPID.out'"); -} - - # Copy the derivation and its dependencies to the build machine. This # is guarded by an exclusive lock per machine to prevent multiple # build-remote instances from copying to a machine simultaneously. @@ -255,48 +246,33 @@ if ($@) { print STDERR "somebody is hogging $uploadLock, continuing...\n"; unlink $uploadLock; } -Nix::CopyClosure::copyTo($hostName, [ @sshOpts ], [ $drvPath, @inputs ], "", "", 0, 0, $maybeSign ne "", ""); +Nix::CopyClosure::copyToOpen($from, $to, $hostName, [ $drvPath, @inputs ], "", "", 0, 0, $maybeSign ne "", ""); close UPLOADLOCK; # Perform the build. -my $buildFlags = - "--max-silent-time $maxSilentTime --option build-timeout $buildTimeout" - . " --fallback --add-root $rootsDir/\$PPID.out --quiet" - . " --option build-keep-log false --option build-use-substitutes false"; - -# We let the remote side kill its process group when the connection is -# closed unexpectedly. This is necessary to ensure that no processes -# are left running on the remote system if the local Nix process is -# killed. (SSH itself doesn't kill child processes if the connection -# is interrupted unless the `-tt' flag is used to force a pseudo-tty, -# in which case every child receives SIGHUP; however, `-tt' doesn't -# work on some platforms when connection sharing is used.) print STDERR "building `$drvPath' on `$hostName'\n"; -pipe STDIN, DUMMY; # make sure we have a readable STDIN -if (system("exec ssh $hostName @sshOpts '(read; kill -INT -\$\$) <&0 & exec nix-store -r $drvPath $buildFlags > /dev/null' 2>&4") != 0) { +writeInt(6, $to) or die; # == cmdBuildPaths +writeStrings([$drvPath], $to); +writeInt($maxSilentTime, $to); +writeInt($buildTimeout, $to); +my $res = readInt($from); +if ($res != 0) { # Note that if we get exit code 100 from `nix-store -r', it # denotes a permanent build failure (as opposed to an SSH problem # or a temporary Nix problem). We propagate this to the caller to # allow it to distinguish between transient and permanent # failures. - my $res = $? >> 8; print STDERR "build of `$drvPath' on `$hostName' failed with exit code $res\n"; - removeRoots; exit $res; } -#print "build of `$drvPath' on `$hostName' succeeded\n"; - # Copy the output from the build machine. my @outputs2 = grep { !isValidPath($_) } @outputs; if (scalar @outputs2 > 0) { - system("exec ssh $hostName @sshOpts 'nix-store --export @outputs2'" . - "| NIX_HELD_LOCKS='@outputs2' @bindir@/nix-store --import > /dev/null") == 0 - or die("cannot copy paths " . join(", ", @outputs) . " from `$hostName': $?"); + writeInt(5, $to) or die; # == cmdExportPaths + writeStrings(\@outputs2, $to); + $ENV{'NIX_HELD_LOCKS'} = "@outputs2"; # FIXME: ugly + importPaths(fileno($from)); } - - -# Get rid of the temporary GC roots. -removeRoots; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 3b021bb2a..f566ccf53 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -35,6 +35,7 @@ template T readStorePaths(Source & from) } template PathSet readStorePaths(Source & from); +template Paths readStorePaths(Source & from); RemoteStore::RemoteStore() diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index f31eb0e29..0196c2fc1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -928,7 +928,6 @@ static void opServe(Strings opFlags, Strings opArgs) } writeStrings(store->queryValidPaths(paths), out); - out.flush(); break; } @@ -947,17 +946,15 @@ static void opServe(Strings opFlags, Strings opArgs) writeLongLong(info.narSize, out); } writeString("", out); - out.flush(); break; } case cmdDumpStorePath: dumpPath(readStorePath(in), out); - out.flush(); break; case cmdImportPaths: { - if (!writeAllowed) throw Error("importing paths not allowed"); + if (!writeAllowed) throw Error("importing paths is not allowed"); string compression = readString(in); if (compression != "") { @@ -986,7 +983,6 @@ static void opServe(Strings opFlags, Strings opArgs) store->importPaths(false, in); writeInt(1, out); // indicate success - out.flush(); /* The decompressor will have left stdin in an undefined state, so we can't continue. */ @@ -995,9 +991,40 @@ static void opServe(Strings opFlags, Strings opArgs) break; } + case cmdExportPaths: { + exportPaths(*store, readStorePaths(in), false, out); + break; + } + + case cmdBuildPaths: { + /* Used by build-remote.pl. */ + if (!writeAllowed) throw Error("building paths is not allowed"); + PathSet paths = readStorePaths(in); + + // FIXME: changing options here doesn't work if we're + // building through the daemon. + verbosity = lvlError; + settings.keepLog = false; + settings.useSubstitutes = false; + settings.maxSilentTime = readInt(in); + settings.buildTimeout = readInt(in); + + int res = 0; + try { + store->buildPaths(paths); + } catch (Error & e) { + printMsg(lvlError, format("error: %1%") % e.msg()); + res = e.status; + } + writeInt(res, out); + break; + } + default: throw Error(format("unknown serve command %1%") % cmd); } + + out.flush(); } } diff --git a/src/nix-store/serve-protocol.hh b/src/nix-store/serve-protocol.hh index 07ff4f7a7..eb13b46e5 100644 --- a/src/nix-store/serve-protocol.hh +++ b/src/nix-store/serve-protocol.hh @@ -14,6 +14,8 @@ typedef enum { cmdQueryPathInfos = 2, cmdDumpStorePath = 3, cmdImportPaths = 4, + cmdExportPaths = 5, + cmdBuildPaths = 6, } ServeCommand; }