Propagated executor sandbox creation errors. Rather than crashing if the agent fails to create the executor directory, propagate the error to the caller so that it can handle it appropriately.
Review: https://reviews.apache.org/r/66705/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/24773d48 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/24773d48 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/24773d48 Branch: refs/heads/master Commit: 24773d4842dee3d0a59ecb4b3ca65dc4ba02c94d Parents: b848b09 Author: James Peach <[email protected]> Authored: Fri Apr 20 08:56:49 2018 -0700 Committer: James Peach <[email protected]> Committed: Fri Apr 20 08:56:49 2018 -0700 ---------------------------------------------------------------------- src/slave/paths.cpp | 19 +++++++++++-------- src/slave/paths.hpp | 2 +- src/slave/slave.cpp | 13 +++++++++---- src/tests/paths_tests.cpp | 4 ++-- 4 files changed, 23 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/24773d48/src/slave/paths.cpp ---------------------------------------------------------------------- diff --git a/src/slave/paths.cpp b/src/slave/paths.cpp index 690bfe3..ed0b127 100644 --- a/src/slave/paths.cpp +++ b/src/slave/paths.cpp @@ -722,7 +722,7 @@ string getPersistentVolumePath( } -string createExecutorDirectory( +Try<string> createExecutorDirectory( const string& rootDir, const SlaveID& slaveId, const FrameworkID& frameworkId, @@ -749,9 +749,11 @@ string createExecutorDirectory( } Try<Nothing> mkdir = createSandboxDirectory(directory, user); - - CHECK_SOME(mkdir) - << "Failed to create executor directory '" << directory << "'"; + if (mkdir.isError()) { + return Error( + "Failed to create executor directory '" + directory + "': " + + mkdir.error()); + } // Remove the previous "latest" symlink. const string latest = @@ -764,10 +766,11 @@ string createExecutorDirectory( // Symlink the new executor directory to "latest". Try<Nothing> symlink = ::fs::symlink(directory, latest); - - CHECK_SOME(symlink) - << "Failed to symlink directory '" << directory - << "' to '" << latest << "'"; + if (symlink.isError()) { + return Error( + "Failed to symlink '" + directory + "' to '" + latest + "': " + + symlink.error()); + } return directory; } http://git-wip-us.apache.org/repos/asf/mesos/blob/24773d48/src/slave/paths.hpp ---------------------------------------------------------------------- diff --git a/src/slave/paths.hpp b/src/slave/paths.hpp index fe5ab9e..0158964 100644 --- a/src/slave/paths.hpp +++ b/src/slave/paths.hpp @@ -392,7 +392,7 @@ std::string getPersistentVolumePath( const Resource& resource); -std::string createExecutorDirectory( +Try<std::string> createExecutorDirectory( const std::string& rootDir, const SlaveID& slaveId, const FrameworkID& frameworkId, http://git-wip-us.apache.org/repos/asf/mesos/blob/24773d48/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 6885124..fab31fd 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -8872,6 +8872,7 @@ Executor* Framework::addExecutor(const ExecutorInfo& executorInfo) containerId.set_value(id::UUID::random().toString()); Option<string> user = None(); + #ifndef __WINDOWS__ if (slave->flags.switch_user) { // The command (either in form of task or executor command) can @@ -8891,7 +8892,7 @@ Executor* Framework::addExecutor(const ExecutorInfo& executorInfo) #endif // __WINDOWS__ // Create a directory for the executor. - const string directory = paths::createExecutorDirectory( + Try<string> directory = paths::createExecutorDirectory( slave->flags.work_dir, slave->info.id(), id(), @@ -8899,12 +8900,14 @@ Executor* Framework::addExecutor(const ExecutorInfo& executorInfo) containerId, user); + CHECK_SOME(directory); + Executor* executor = new Executor( slave, id(), executorInfo, containerId, - directory, + directory.get(), user, info.checkpoint()); @@ -8920,7 +8923,7 @@ Executor* Framework::addExecutor(const ExecutorInfo& executorInfo) LOG(INFO) << "Launching executor '" << executorInfo.executor_id() << "' of framework " << id() << " with resources " << executorInfo.resources() - << " in work directory '" << directory << "'"; + << " in work directory '" << directory.get() << "'"; const ExecutorID& executorId = executorInfo.executor_id(); FrameworkID frameworkId = id(); @@ -9610,8 +9613,10 @@ void Executor::checkpointExecutor() // Create the meta executor directory. // NOTE: This creates the 'latest' symlink in the meta directory. - paths::createExecutorDirectory( + Try<string> mkdir = paths::createExecutorDirectory( slave->metaDir, slave->info.id(), frameworkId, id, containerId); + + CHECK_SOME(mkdir); } http://git-wip-us.apache.org/repos/asf/mesos/blob/24773d48/src/tests/paths_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/paths_tests.cpp b/src/tests/paths_tests.cpp index dc765ed..3fa8f52 100644 --- a/src/tests/paths_tests.cpp +++ b/src/tests/paths_tests.cpp @@ -84,7 +84,7 @@ protected: TEST_F(PathsTest, CreateExecutorDirectory) { - const string& result = paths::createExecutorDirectory( + Try<string> result = paths::createExecutorDirectory( rootDir, slaveId, frameworkId, executorId, containerId); // Expected directory layout. @@ -99,7 +99,7 @@ TEST_F(PathsTest, CreateExecutorDirectory) "runs", containerId.value()); - ASSERT_EQ(dir, result); + ASSERT_SOME_EQ(dir, result); }
