You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2014/08/04 18:28:18 UTC
[2/4] git commit: Added 'bool' return value to Containerizer::launch.
Added 'bool' return value to Containerizer::launch.
Review: https://reviews.apache.org/r/23461
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1ae6ade3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1ae6ade3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1ae6ade3
Branch: refs/heads/master
Commit: 1ae6ade3d4c5c52794be11422f731959a581ac9d
Parents: 6ddca90
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Jun 22 17:53:22 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Mon Aug 4 09:15:50 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/containerizer.hpp | 12 +++++---
.../containerizer/external_containerizer.cpp | 12 ++++----
.../containerizer/external_containerizer.hpp | 10 +++----
src/slave/containerizer/mesos/containerizer.cpp | 30 ++++++++++++--------
src/slave/containerizer/mesos/containerizer.hpp | 14 ++++-----
src/slave/slave.cpp | 10 +++++--
src/slave/slave.hpp | 2 +-
src/tests/containerizer.cpp | 6 ++--
src/tests/containerizer.hpp | 6 ++--
src/tests/containerizer_tests.cpp | 8 +++---
src/tests/external_containerizer_test.cpp | 4 +--
src/tests/slave_recovery_tests.cpp | 2 +-
src/tests/slave_tests.cpp | 2 +-
13 files changed, 67 insertions(+), 51 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/containerizer/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/containerizer.hpp b/src/slave/containerizer/containerizer.hpp
index a9f89fc..02754cd 100644
--- a/src/slave/containerizer/containerizer.hpp
+++ b/src/slave/containerizer/containerizer.hpp
@@ -72,8 +72,10 @@ public:
virtual process::Future<Nothing> recover(
const Option<state::SlaveState>& state) = 0;
- // Launch a containerized executor.
- virtual process::Future<Nothing> launch(
+ // Launch a containerized executor. Returns true if launching this
+ // ExecutorInfo is supported and it has been launched, otherwise
+ // false or a failure is something went wrong.
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
@@ -82,10 +84,12 @@ public:
const process::PID<Slave>& slavePid,
bool checkpoint) = 0;
- // Launch a containerized task.
+ // Launch a containerized task. Returns true if launching this
+ // TaskInfo/ExecutorInfo is supported and it has been launched,
+ // otherwise false or a failure is something went wrong.
// TODO(nnielsen): Obsolete the executorInfo argument when the slave
// doesn't require executors to run standalone tasks.
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/containerizer/external_containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/external_containerizer.cpp b/src/slave/containerizer/external_containerizer.cpp
index 3f28d85..03de1fc 100644
--- a/src/slave/containerizer/external_containerizer.cpp
+++ b/src/slave/containerizer/external_containerizer.cpp
@@ -153,7 +153,7 @@ Future<Nothing> ExternalContainerizer::recover(
}
-Future<Nothing> ExternalContainerizer::launch(
+Future<bool> ExternalContainerizer::launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const string& directory,
@@ -175,7 +175,7 @@ Future<Nothing> ExternalContainerizer::launch(
}
-Future<Nothing> ExternalContainerizer::launch(
+Future<bool> ExternalContainerizer::launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
@@ -418,7 +418,7 @@ Future<Nothing> ExternalContainerizerProcess::___recover()
}
-Future<Nothing> ExternalContainerizerProcess::launch(
+Future<bool> ExternalContainerizerProcess::launch(
const ContainerID& containerId,
const Option<TaskInfo>& taskInfo,
const ExecutorInfo& executor,
@@ -527,7 +527,7 @@ Future<Nothing> ExternalContainerizerProcess::launch(
}
-Future<Nothing> ExternalContainerizerProcess::_launch(
+Future<bool> ExternalContainerizerProcess::_launch(
const ContainerID& containerId,
const Future<Option<int> >& future)
{
@@ -546,13 +546,13 @@ Future<Nothing> ExternalContainerizerProcess::_launch(
// have gotten chained up.
actives[containerId]->launched.set(Nothing());
- return Nothing();
+ return true;
}
void ExternalContainerizerProcess::__launch(
const ContainerID& containerId,
- const Future<Nothing>& future)
+ const Future<bool>& future)
{
VLOG(1) << "Launch confirmation callback triggered on container '"
<< containerId << "'";
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/containerizer/external_containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/external_containerizer.hpp b/src/slave/containerizer/external_containerizer.hpp
index 94dffbb..b32efdd 100644
--- a/src/slave/containerizer/external_containerizer.hpp
+++ b/src/slave/containerizer/external_containerizer.hpp
@@ -81,7 +81,7 @@ public:
virtual process::Future<Nothing> recover(
const Option<state::SlaveState>& state);
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
@@ -90,7 +90,7 @@ public:
const process::PID<Slave>& slavePid,
bool checkpoint);
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const TaskInfo& task,
const ExecutorInfo& executorInfo,
@@ -130,7 +130,7 @@ public:
process::Future<Nothing> recover(const Option<state::SlaveState>& state);
// Start the containerized executor.
- process::Future<Nothing> launch(
+ process::Future<bool> launch(
const ContainerID& containerId,
const Option<TaskInfo>& taskInfo,
const ExecutorInfo& executorInfo,
@@ -219,13 +219,13 @@ private:
process::Future<Nothing> ___recover();
- process::Future<Nothing> _launch(
+ process::Future<bool> _launch(
const ContainerID& containerId,
const process::Future<Option<int> >& future);
void __launch(
const ContainerID& containerId,
- const process::Future<Nothing>& future);
+ const process::Future<bool>& future);
process::Future<containerizer::Termination> _wait(
const ContainerID& containerId);
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 2c394e2..694c9d1 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -124,7 +124,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
LOG(INFO) << "Using isolation: " << isolation;
// Create a MesosContainerizerProcess using isolators and a launcher.
- hashmap<std::string, Try<Isolator*> (*)(const Flags&)> creators;
+ hashmap<string, Try<Isolator*> (*)(const Flags&)> creators;
creators["posix/cpu"] = &PosixCpuIsolatorProcess::create;
creators["posix/mem"] = &PosixMemIsolatorProcess::create;
@@ -200,7 +200,7 @@ Future<Nothing> MesosContainerizer::recover(
}
-Future<Nothing> MesosContainerizer::launch(
+Future<bool> MesosContainerizer::launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const string& directory,
@@ -221,7 +221,7 @@ Future<Nothing> MesosContainerizer::launch(
}
-Future<Nothing> MesosContainerizer::launch(
+Future<bool> MesosContainerizer::launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
@@ -391,7 +391,7 @@ Future<Nothing> MesosContainerizerProcess::__recover(
// 4. Exec the executor. The forked child is signalled to continue. It will
// first execute any preparation commands from isolators and then exec the
// executor.
-Future<Nothing> MesosContainerizerProcess::launch(
+Future<bool> MesosContainerizerProcess::launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const string& directory,
@@ -411,10 +411,10 @@ Future<Nothing> MesosContainerizerProcess::launch(
// run.
const CommandInfo& command = executorInfo.command();
if (command.has_container()) {
- // We return a Failure as this containerizer does not support
+ // We return false as this containerizer does not support
// handling ContainerInfo. Users have to be made aware of this
// lack of support to prevent confusion in the task configuration.
- return Failure("ContainerInfo is not supported");
+ return false;
}
Owned<Promise<containerizer::Termination> > promise(
@@ -445,7 +445,7 @@ Future<Nothing> MesosContainerizerProcess::launch(
}
-Future<Nothing> MesosContainerizerProcess::launch(
+Future<bool> MesosContainerizerProcess::launch(
const ContainerID& containerId,
const TaskInfo&,
const ExecutorInfo& executorInfo,
@@ -616,7 +616,7 @@ Future<Nothing> MesosContainerizerProcess::fetch(
}
-Future<Nothing> MesosContainerizerProcess::_launch(
+Future<bool> MesosContainerizerProcess::_launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const string& directory,
@@ -735,7 +735,13 @@ Future<Nothing> MesosContainerizerProcess::_launch(
}
-Future<Nothing> MesosContainerizerProcess::isolate(
+static Future<bool> _isolate()
+{
+ return true;
+}
+
+
+Future<bool> MesosContainerizerProcess::isolate(
const ContainerID& containerId,
pid_t _pid)
{
@@ -753,11 +759,11 @@ Future<Nothing> MesosContainerizerProcess::isolate(
// Wait for all isolators to complete.
return collect(futures)
- .then(lambda::bind(&_nothing));
+ .then(lambda::bind(&_isolate));
}
-Future<Nothing> MesosContainerizerProcess::exec(
+Future<bool> MesosContainerizerProcess::exec(
const ContainerID& containerId,
int pipeWrite)
{
@@ -779,7 +785,7 @@ Future<Nothing> MesosContainerizerProcess::exec(
string(strerror(errno)));
}
- return Nothing();
+ return true;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/containerizer/mesos/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.hpp b/src/slave/containerizer/mesos/containerizer.hpp
index 8746968..bf246ca 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -52,7 +52,7 @@ public:
virtual process::Future<Nothing> recover(
const Option<state::SlaveState>& state);
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
@@ -61,7 +61,7 @@ public:
const process::PID<Slave>& slavePid,
bool checkpoint);
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
@@ -109,7 +109,7 @@ public:
process::Future<Nothing> recover(
const Option<state::SlaveState>& state);
- process::Future<Nothing> launch(
+ process::Future<bool> launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
@@ -118,7 +118,7 @@ public:
const process::PID<Slave>& slavePid,
bool checkpoint);
- process::Future<Nothing> launch(
+ process::Future<bool> launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
@@ -161,7 +161,7 @@ private:
const std::string& directory,
const Option<std::string>& user);
- process::Future<Nothing> _launch(
+ process::Future<bool> _launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
@@ -171,11 +171,11 @@ private:
bool checkpoint,
const std::list<Option<CommandInfo> >& scripts);
- process::Future<Nothing> isolate(
+ process::Future<bool> isolate(
const ContainerID& containerId,
pid_t _pid);
- process::Future<Nothing> exec(
+ process::Future<bool> exec(
const ContainerID& containerId,
int pipeWrite);
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index b4a5a45..df69b75 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2418,7 +2418,7 @@ void Slave::executorLaunched(
const FrameworkID& frameworkId,
const ExecutorID& executorId,
const ContainerID& containerId,
- const Future<Nothing>& future)
+ const Future<bool>& future)
{
// Set up callback for executor termination. Note that we do this
// regardless of whether or not we have successfully launched the
@@ -2444,6 +2444,12 @@ void Slave::executorLaunched(
<< "' failed to start: "
<< (future.isFailed() ? future.failure() : " future discarded");
return;
+ } else if (future.get()) {
+ LOG(ERROR) << "Container '" << containerId
+ << "' for executor '" << executorId
+ << "' of framework '" << frameworkId
+ << "' failed to start: TaskInfo/ExecutorInfo not supported";
+ return;
}
Framework* framework = getFramework(frameworkId);
@@ -3490,7 +3496,7 @@ Executor* Framework::launchExecutor(
}
// Launch the container.
- Future<Nothing> launch;
+ Future<bool> launch;
if (!executor->commandExecutor) {
// If the executor is _not_ a command executor, this means that
// the task will include the executor to run. The actual task to
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index a896bb6..9ef597e 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -199,7 +199,7 @@ public:
const FrameworkID& frameworkId,
const ExecutorID& executorId,
const ContainerID& containerId,
- const process::Future<Nothing>& future);
+ const process::Future<bool>& future);
void executorTerminated(
const FrameworkID& frameworkId,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer.cpp b/src/tests/containerizer.cpp
index 3f11d35..a17e1e0 100644
--- a/src/tests/containerizer.cpp
+++ b/src/tests/containerizer.cpp
@@ -73,7 +73,7 @@ TestContainerizer::~TestContainerizer()
}
-Future<Nothing> TestContainerizer::_launch(
+Future<bool> TestContainerizer::_launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const string& directory,
@@ -137,11 +137,11 @@ Future<Nothing> TestContainerizer::_launch(
new Promise<containerizer::Termination>());
promises[containerId] = promise;
- return Nothing();
+ return true;
}
-Future<Nothing> TestContainerizer::launch(
+Future<bool> TestContainerizer::launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer.hpp b/src/tests/containerizer.hpp
index 9325864..24b014f 100644
--- a/src/tests/containerizer.hpp
+++ b/src/tests/containerizer.hpp
@@ -66,7 +66,7 @@ public:
MOCK_METHOD7(
launch,
- process::Future<Nothing>(
+ process::Future<bool>(
const ContainerID&,
const ExecutorInfo&,
const std::string&,
@@ -75,7 +75,7 @@ public:
const process::PID<slave::Slave>&,
bool checkpoint));
- virtual process::Future<Nothing> launch(
+ virtual process::Future<bool> launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
@@ -113,7 +113,7 @@ private:
void setup();
// Default 'launch' implementation.
- process::Future<Nothing> _launch(
+ process::Future<bool> _launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const std::string& directory,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer_tests.cpp b/src/tests/containerizer_tests.cpp
index 70e1245..2c90d2f 100644
--- a/src/tests/containerizer_tests.cpp
+++ b/src/tests/containerizer_tests.cpp
@@ -312,7 +312,7 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, ScriptSucceeds)
ContainerID containerId;
containerId.set_value("test_container");
- process::Future<Nothing> launch = containerizer.get()->launch(
+ process::Future<bool> launch = containerizer.get()->launch(
containerId,
CREATE_EXECUTOR_INFO("executor", "exit 0"),
directory,
@@ -356,7 +356,7 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, ScriptFails)
ContainerID containerId;
containerId.set_value("test_container");
- Future<Nothing> launch = containerizer.get()->launch(
+ Future<bool> launch = containerizer.get()->launch(
containerId,
CREATE_EXECUTOR_INFO("executor", "exit 0"),
directory,
@@ -409,7 +409,7 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, MultipleScripts)
ContainerID containerId;
containerId.set_value("test_container");
- Future<Nothing> launch = containerizer.get()->launch(
+ Future<bool> launch = containerizer.get()->launch(
containerId,
CREATE_EXECUTOR_INFO("executor", "exit 0"),
directory,
@@ -462,7 +462,7 @@ TEST_F(MesosContainerizerExecuteTest, IoRedirection)
string command =
"(echo '" + errMsg + "' 1>&2) && echo '" + outMsg + "'";
- process::Future<Nothing> launch = containerizer.get()->launch(
+ process::Future<bool> launch = containerizer.get()->launch(
containerId,
CREATE_EXECUTOR_INFO("executor", command),
directory,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/external_containerizer_test.cpp
----------------------------------------------------------------------
diff --git a/src/tests/external_containerizer_test.cpp b/src/tests/external_containerizer_test.cpp
index c26f3c2..2f6aa50 100644
--- a/src/tests/external_containerizer_test.cpp
+++ b/src/tests/external_containerizer_test.cpp
@@ -77,7 +77,7 @@ class MockExternalContainerizer : public slave::ExternalContainerizer
public:
MOCK_METHOD8(
launch,
- process::Future<Nothing>(
+ process::Future<bool>(
const ContainerID&,
const TaskInfo&,
const ExecutorInfo&,
@@ -98,7 +98,7 @@ public:
.WillRepeatedly(Invoke(this, &MockExternalContainerizer::_launch));
}
- process::Future<Nothing> _launch(
+ process::Future<bool> _launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp
index 0b17a8a..9a14b4c 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -3209,7 +3209,7 @@ TYPED_TEST(SlaveRecoveryTest, RestartBeforeContainerizerLaunch)
Future<Nothing> launch;
EXPECT_CALL(*containerizer1, launch(_, _, _, _, _, _, _))
.WillOnce(DoAll(FutureSatisfy(&launch),
- Return(Future<Nothing>())));
+ Return(Future<bool>())));
// Ensure that wait doesn't complete so that containerizer doesn't
// return a failure on 'wait' due to the pending launch.
http://git-wip-us.apache.org/repos/asf/mesos/blob/1ae6ade3/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index e45255a..b4f9f30 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -303,7 +303,7 @@ TEST_F(SlaveTest, MesosExecutorWithOverride)
Future<Nothing> launch;
EXPECT_CALL(containerizer, launch(_, _, _, _, _, _, _))
.WillOnce(DoAll(FutureSatisfy(&launch),
- Return(Future<Nothing>())));
+ Return(Future<bool>())));
// Expect wait after launch is called. wait() will fail if not
// intercepted here as the container will never be registered within