You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2018/04/20 16:22:11 UTC
[2/5] mesos git commit: Propagated executor sandbox creation errors.
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 <jp...@apache.org>
Authored: Fri Apr 20 08:56:49 2018 -0700
Committer: James Peach <jp...@apache.org>
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);
}