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/10/31 23:06:02 UTC
[03/12] git commit: Simplified launch continuations in
DockerContainerizerProcess.
Simplified launch continuations in DockerContainerizerProcess.
Combined redundant code when launching a Docker container with and
without a task.
Review: https://reviews.apache.org/r/26613
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5c3d6f1a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5c3d6f1a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5c3d6f1a
Branch: refs/heads/master
Commit: 5c3d6f1a99e5b249cfc0e05d0fef349b1a90a131
Parents: c3e7f2a
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 13:22:21 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:38 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 118 +++++++++++++-------------------
1 file changed, 47 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5c3d6f1a/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 8873022..c46de8d 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -142,22 +142,9 @@ private:
process::Future<Nothing> _recover(
const std::list<Docker::Container>& containers);
- process::Future<bool> _launch(
+ process::Future<Nothing> _launch(
const ContainerID& containerId,
- const TaskInfo& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint);
-
- process::Future<bool> _launch(
- const ContainerID& containerId,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint);
+ const std::string& directory);
process::Future<bool> __launch(
const ContainerID& containerId,
@@ -242,17 +229,36 @@ private:
{
static Try<Container*> create(
const ContainerID& id,
+ const Option<TaskInfo>& taskInfo,
+ const ExecutorInfo& executorInfo,
const std::string& directory,
const Option<std::string>& user);
Container(const ContainerID& id)
: state(FETCHING), id(id) {}
+ Container(const ContainerID& id,
+ const Option<TaskInfo>& taskInfo,
+ const ExecutorInfo& executorInfo)
+ : state(FETCHING),
+ id(id),
+ task(taskInfo),
+ executor(executorInfo) {}
+
std::string name()
{
return DOCKER_NAME_PREFIX + stringify(id);
}
+ std::string image() const
+ {
+ if (task.isSome()) {
+ return task.get().container().docker().image();
+ }
+
+ return executor.container().docker().image();
+ }
+
// The DockerContainerier needs to be able to properly clean up
// Docker containers, regardless of when they are destroyed. For
// example, if a container gets destroyed while we are fetching,
@@ -283,6 +289,8 @@ private:
} state;
ContainerID id;
+ Option<TaskInfo> task;
+ ExecutorInfo executor;
// Promise for future returned from wait().
Promise<containerizer::Termination> termination;
@@ -370,8 +378,11 @@ DockerContainerizer::~DockerContainerizer()
}
-Future<Nothing> DockerContainerizerProcess::Container::create(
- const ContainerID& containerId,
+Try<DockerContainerizerProcess::Container*>
+DockerContainerizerProcess::Container::create(
+ const ContainerID& id,
+ const Option<TaskInfo>& taskInfo,
+ const ExecutorInfo& executorInfo,
const string& directory,
const Option<string>& user)
{
@@ -380,24 +391,24 @@ Future<Nothing> DockerContainerizerProcess::Container::create(
Try<Nothing> touch = os::touch(path::join(directory, "stdout"));
if (touch.isError()) {
- return Failure("Failed to touch 'stdout': " + touch.error());
+ return Error("Failed to touch 'stdout': " + touch.error());
}
touch = os::touch(path::join(directory, "stderr"));
if (touch.isError()) {
- return Failure("Failed to touch 'stderr': " + touch.error());
+ return Error("Failed to touch 'stderr': " + touch.error());
}
if (user.isSome()) {
Try<Nothing> chown = os::chown(user.get(), directory);
if (chown.isError()) {
- return Failure("Failed to chown: " + chown.error());
+ return Error("Failed to chown: " + chown.error());
}
}
- return new Container(containerId);
+ return new Container(id, taskInfo, executorInfo);
}
@@ -751,7 +762,8 @@ Future<bool> DockerContainerizerProcess::launch(
return false;
}
- Try<Container*> container = Container::create(containerId, directory, user);
+ Try<Container*> container = Container::create(
+ containerId, taskInfo, executorInfo, directory, user);
if (container.isError()) {
return Failure("Failed to create container: " + container.error());
@@ -765,8 +777,9 @@ Future<bool> DockerContainerizerProcess::launch(
<< "') of framework '" << executorInfo.framework_id() << "'";
return fetch(containerId, taskInfo.command(), directory)
+ .then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(),
- &Self::_launch,
+ &Self::__launch,
containerId,
taskInfo,
executorInfo,
@@ -778,14 +791,9 @@ Future<bool> DockerContainerizerProcess::launch(
}
-Future<bool> DockerContainerizerProcess::_launch(
+Future<Nothing> DockerContainerizerProcess::_launch(
const ContainerID& containerId,
- const TaskInfo& taskInfo,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint)
+ const string& directory)
{
// Doing the fetch might have succeded but we were actually asked to
// destroy the container, which we did, so don't continue.
@@ -793,20 +801,14 @@ Future<bool> DockerContainerizerProcess::_launch(
return Failure("Container was destroyed while launching");
}
- containers_[containerId]->state = Container::PULLING;
+ Container* container = containers_[containerId];
- return pull(containerId, directory, taskInfo.container().docker().image())
- .then(defer(self(),
- &Self::__launch,
- containerId,
- taskInfo,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint));
+ container->state = Container::PULLING;
+
+ return pull(containerId, directory, container->image());
}
+
Future<bool> DockerContainerizerProcess::__launch(
const ContainerID& containerId,
const TaskInfo& taskInfo,
@@ -975,7 +977,8 @@ Future<bool> DockerContainerizerProcess::launch(
return false;
}
- Try<Container*> container = Container::create(containerId, directory, user);
+ Try<Container*> container = Container::create(
+ containerId, None(), executorInfo, directory, user);
if (container.isError()) {
return Failure("Failed to create container: " + container.error());
@@ -988,8 +991,9 @@ Future<bool> DockerContainerizerProcess::launch(
<< "' and framework '" << executorInfo.framework_id() << "'";
return fetch(containerId, executorInfo.command(), directory)
+ .then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(),
- &Self::_launch,
+ &Self::__launch,
containerId,
executorInfo,
directory,
@@ -1000,34 +1004,6 @@ Future<bool> DockerContainerizerProcess::launch(
}
-Future<bool> DockerContainerizerProcess::_launch(
- const ContainerID& containerId,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint)
-{
- // Doing the fetch might have succeded but we were actually asked to
- // destroy the container, which we did, so don't continue.
- if (!containers_.contains(containerId)) {
- return Failure("Container was destroyed while launching");
- }
-
- containers_[containerId]->state = Container::PULLING;
-
- return pull(containerId, directory, executorInfo.container().docker().image())
- .then(defer(self(),
- &Self::__launch,
- containerId,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint));
-}
-
-
Future<bool> DockerContainerizerProcess::__launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,