Skip to content

Commit

Permalink
Use the new StoreReference in Machine
Browse files Browse the repository at this point in the history 
This makes the remote builder abstract syntax more robust.
  • Loading branch information
@Ericson2314
Ericson2314 committed May 22, 2024
1 parent d27dd63 commit a63c888
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 48 deletions.
22 changes: 11 additions & 11 deletions src/build-remote/build-remote.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static std::string currentLoad;

static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot)
{
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true);
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri.render()), slot), true);
}

static bool allSupportedLocally(Store & store, const std::set<std::string>& requiredFeatures) {
Expand Down Expand Up @@ -99,7 +99,7 @@ static int main_build_remote(int argc, char * * argv)
}

std::optional<StorePath> drvPath;
std::string storeUri;
StoreReference storeUri;

while (true) {

Expand Down Expand Up @@ -135,7 +135,7 @@ static int main_build_remote(int argc, char * * argv)
Machine * bestMachine = nullptr;
uint64_t bestLoad = 0;
for (auto & m : machines) {
debug("considering building on remote machine '%s'", m.storeUri);
debug("considering building on remote machine '%s'", m.storeUri.render());

if (m.enabled &&
m.systemSupported(neededSystem) &&
Expand Down Expand Up @@ -233,7 +233,7 @@ static int main_build_remote(int argc, char * * argv)

try {

Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri.render()));

sshStore = bestMachine->openStore();
sshStore->connect();
Expand All @@ -242,7 +242,7 @@ static int main_build_remote(int argc, char * * argv)
} catch (std::exception & e) {
auto msg = chomp(drainFD(5, false));
printError("cannot build on '%s': %s%s",
bestMachine->storeUri, e.what(),
bestMachine->storeUri.render(), e.what(),
msg.empty() ? "" : ": " + msg);
bestMachine->enabled = false;
continue;
Expand All @@ -257,15 +257,15 @@ static int main_build_remote(int argc, char * * argv)

assert(sshStore);

std::cerr << "# accept\n" << storeUri << "\n";
std::cerr << "# accept\n" << storeUri.render() << "\n";

auto inputs = readStrings<PathSet>(source);
auto wantedOutputs = readStrings<StringSet>(source);

AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + escapeUri(storeUri) + ".upload-lock", true);
AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + escapeUri(storeUri.render()) + ".upload-lock", true);

{
Activity act(*logger, lvlTalkative, actUnknown, fmt("waiting for the upload lock to '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("waiting for the upload lock to '%s'", storeUri.render()));

auto old = signal(SIGALRM, handleAlarm);
alarm(15 * 60);
Expand All @@ -278,7 +278,7 @@ static int main_build_remote(int argc, char * * argv)
auto substitute = settings.buildersUseSubstitutes ? Substitute : NoSubstitute;

{
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri.render()));
copyPaths(*store, *sshStore, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute);
}

Expand Down Expand Up @@ -316,7 +316,7 @@ static int main_build_remote(int argc, char * * argv)
optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv);
auto & result = *optResult;
if (!result.success())
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri.render(), result.errorMsg);
} else {
copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute);
auto res = sshStore->buildPathsWithResults({
Expand Down Expand Up @@ -359,7 +359,7 @@ static int main_build_remote(int argc, char * * argv)
}

if (!missingPaths.empty()) {
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri.render()));
if (auto localStore = store.dynamic_pointer_cast<LocalStore>())
for (auto & path : missingPaths)
localStore->locksHeld.insert(store->printStorePath(path)); /* FIXME: ugly */
Expand Down
35 changes: 22 additions & 13 deletions src/libstore/machines.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@

namespace nix {

Machine::Machine(decltype(storeUri) storeUri,
Machine::Machine(
const std::string & storeUri,
decltype(systemTypes) systemTypes,
decltype(sshKey) sshKey,
decltype(maxJobs) maxJobs,
decltype(speedFactor) speedFactor,
decltype(supportedFeatures) supportedFeatures,
decltype(mandatoryFeatures) mandatoryFeatures,
decltype(sshPublicHostKey) sshPublicHostKey) :
storeUri(
storeUri(StoreReference::parse(
// Backwards compatibility: if the URI is schemeless, is not a path,
// and is not one of the special store connection words, prepend
// ssh://.
Expand All @@ -28,7 +29,7 @@ Machine::Machine(decltype(storeUri) storeUri,
|| hasPrefix(storeUri, "local?")
|| hasPrefix(storeUri, "?")
? storeUri
: "ssh://" + storeUri),
: "ssh://" + storeUri)),
systemTypes(systemTypes),
sshKey(sshKey),
maxJobs(maxJobs),
Expand Down Expand Up @@ -63,23 +64,26 @@ bool Machine::mandatoryMet(const std::set<std::string> & features) const
});
}

ref<Store> Machine::openStore() const
StoreReference Machine::completeStoreReference() const
{
Store::Params storeParams;
if (hasPrefix(storeUri, "ssh://")) {
storeParams["max-connections"] = "1";
storeParams["log-fd"] = "4";
auto storeUri = this->storeUri;

auto * generic = std::get_if<StoreReference::Specified>(&storeUri.variant);

if (generic && generic->scheme == "ssh") {
storeUri.params["max-connections"] = "1";
storeUri.params["log-fd"] = "4";
}

if (hasPrefix(storeUri, "ssh://") || hasPrefix(storeUri, "ssh-ng://")) {
if (generic && (generic->scheme == "ssh" || generic->scheme == "ssh-ng")) {
if (sshKey != "")
storeParams["ssh-key"] = sshKey;
storeUri.params["ssh-key"] = sshKey;
if (sshPublicHostKey != "")
storeParams["base64-ssh-public-host-key"] = sshPublicHostKey;
storeUri.params["base64-ssh-public-host-key"] = sshPublicHostKey;
}

{
auto & fs = storeParams["system-features"];
auto & fs = storeUri.params["system-features"];
auto append = [&](auto feats) {
for (auto & f : feats) {
if (fs.size() > 0) fs += ' ';
Expand All @@ -90,7 +94,12 @@ ref<Store> Machine::openStore() const
append(mandatoryFeatures);
}

return nix::openStore(storeUri, storeParams);
return storeUri;
}

ref<Store> Machine::openStore() const
{
return nix::openStore(completeStoreReference());
}

static std::vector<std::string> expandBuilderLines(const std::string & builders)
Expand Down
21 changes: 19 additions & 2 deletions src/libstore/machines.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
///@file

#include "types.hh"
#include "store-reference.hh"

namespace nix {

class Store;

struct Machine {

const std::string storeUri;
const StoreReference storeUri;
const std::set<std::string> systemTypes;
const std::string sshKey;
const unsigned int maxJobs;
Expand All @@ -36,7 +37,8 @@ struct Machine {
*/
bool mandatoryMet(const std::set<std::string> & features) const;

Machine(decltype(storeUri) storeUri,
Machine(
const std::string & storeUri,
decltype(systemTypes) systemTypes,
decltype(sshKey) sshKey,
decltype(maxJobs) maxJobs,
Expand All @@ -45,6 +47,21 @@ struct Machine {
decltype(mandatoryFeatures) mandatoryFeatures,
decltype(sshPublicHostKey) sshPublicHostKey);

/**
* Elaborate `storeUri` into a complete store reference,
* incorporating information from the other fields of the `Machine`
* as applicable.
*/
StoreReference completeStoreReference() const;

/**
* Open a `Store` for this machine.
*
* Just a simple function composition:
* ```c++
* nix::openStore(completeStoreReference())
* ```
*/
ref<Store> openStore() const;
};

Expand Down
47 changes: 25 additions & 22 deletions tests/unit/libstore/machines.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,16 @@
#include "file-system.hh"
#include "util.hh"

#include <gtest/gtest.h>
#include <gmock/gmock-matchers.h>

using testing::Contains;
using testing::ElementsAre;
using testing::EndsWith;
using testing::Eq;
using testing::Field;
using testing::SizeIs;

using nix::absPath;
using nix::FormatError;
using nix::UsageError;
using nix::getMachines;
using nix::Machine;
using nix::Machines;
using nix::pathExists;
using nix::Settings;
using nix::settings;
using namespace nix;

class Environment : public ::testing::Environment {
public:
Expand All @@ -40,7 +32,7 @@ TEST(machines, getMachinesUriOnly) {
settings.builders = "[email protected]";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(1));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq(StoreReference::parse("ssh://[email protected]"))));
EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
EXPECT_THAT(actual[0], Field(&Machine::sshKey, SizeIs(0)));
EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(1)));
Expand All @@ -54,7 +46,7 @@ TEST(machines, getMachinesDefaults) {
settings.builders = "[email protected] - - - - - - -";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(1));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq(StoreReference::parse("ssh://[email protected]"))));
EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
EXPECT_THAT(actual[0], Field(&Machine::sshKey, SizeIs(0)));
EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(1)));
Expand All @@ -64,20 +56,31 @@ TEST(machines, getMachinesDefaults) {
EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, SizeIs(0)));
}

MATCHER_P(AuthorityMatches, authority, "") {
*result_listener
<< "where the authority of "
<< arg.render()
<< " is "
<< authority;
auto * generic = std::get_if<StoreReference::Specified>(&arg.variant);
if (!generic) return false;
return generic->authority == authority;
}

TEST(machines, getMachinesWithNewLineSeparator) {
settings.builders = "[email protected]\n[email protected]";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(2));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
}

TEST(machines, getMachinesWithSemicolonSeparator) {
settings.builders = "[email protected] ; [email protected]";
Machines actual = getMachines();
EXPECT_THAT(actual, SizeIs(2));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
}

TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
Expand All @@ -86,7 +89,7 @@ TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
"benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(1));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
EXPECT_THAT(actual[0], Field(&Machine::sshKey, Eq("/home/nix/.ssh/id_scratchy_auto")));
EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(8)));
Expand All @@ -104,7 +107,7 @@ TEST(machines,
"KEY+BASE64+ENCODED==";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(1));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
EXPECT_THAT(actual[0], Field(&Machine::sshKey, Eq("/home/nix/.ssh/id_scratchy_auto")));
EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(8)));
Expand All @@ -120,7 +123,7 @@ TEST(machines, getMachinesWithMultiOptions) {
"MandatoryFeature1,MandatoryFeature2";
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(1));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("[email protected]")));
EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("Arch1", "Arch2")));
EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, ElementsAre("SupportedFeature1", "SupportedFeature2")));
EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, ElementsAre("MandatoryFeature1", "MandatoryFeature2")));
Expand All @@ -146,9 +149,9 @@ TEST(machines, getMachinesWithCorrectFileReference) {
settings.builders = std::string("@") + path;
Machines actual = getMachines();
ASSERT_THAT(actual, SizeIs(3));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("[email protected]"))));
}

TEST(machines, getMachinesWithCorrectFileReferenceToEmptyFile) {
Expand Down

0 comments on commit a63c888

Please sign in to comment.