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:00 UTC
[01/12] git commit: More simplifications of Docker container
launching.
Repository: mesos
Updated Branches:
refs/heads/master 3073bd4e6 -> bd7561e8e
More simplifications of Docker container launching.
Review: https://reviews.apache.org/r/26614
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4cbcac4a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4cbcac4a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4cbcac4a
Branch: refs/heads/master
Commit: 4cbcac4ab94d7a25eaf879ee87b5098b542e417e
Parents: 5c3d6f1
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 13:54:15 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:38 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 224 ++++++++++++++++++--------------
1 file changed, 126 insertions(+), 98 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4cbcac4a/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index c46de8d..b211db0 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -146,22 +146,9 @@ private:
const ContainerID& containerId,
const std::string& directory);
- 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,
@@ -232,18 +219,34 @@ private:
const Option<TaskInfo>& taskInfo,
const ExecutorInfo& executorInfo,
const std::string& directory,
- const Option<std::string>& user);
+ const Option<std::string>& user,
+ const SlaveID& slaveId,
+ const PID<Slave>& slavePid,
+ bool checkpoint,
+ const Flags& flags);
Container(const ContainerID& id)
: state(FETCHING), id(id) {}
Container(const ContainerID& id,
const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo)
+ const ExecutorInfo& executorInfo,
+ const std::string& directory,
+ const Option<std::string>& user,
+ const SlaveID& slaveId,
+ const PID<Slave>& slavePid,
+ bool checkpoint,
+ const Flags& flags)
: state(FETCHING),
id(id),
task(taskInfo),
- executor(executorInfo) {}
+ executor(executorInfo),
+ directory(directory),
+ user(user),
+ slaveId(slaveId),
+ slavePid(slavePid),
+ checkpoint(checkpoint),
+ flags(flags) {}
std::string name()
{
@@ -259,6 +262,49 @@ private:
return executor.container().docker().image();
}
+ ContainerInfo container() const
+ {
+ if (task.isSome()) {
+ return task.get().container();
+ }
+
+ return executor.container();
+ }
+
+ CommandInfo command() const
+ {
+ if (task.isSome()) {
+ return task.get().command();
+ }
+
+ return executor.command();
+ }
+
+ // Returns the environment to use when launching the Docker container.
+ std::map<std::string, std::string> environment() const
+ {
+ if (task.isSome()) {
+ // TODO(benh): Is this really correct!?
+ return std::map<std::string, std::string>();
+ }
+
+ std::map<std::string, std::string> environment = executorEnvironment(
+ executor,
+ directory,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags.recovery_timeout);
+
+ // Include any environment variables from CommandInfo.
+ foreach (const Environment::Variable& variable,
+ executor.command().environment().variables()) {
+ environment[variable.name()] = variable.value();
+ }
+
+ return environment;
+ }
+
// 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,
@@ -291,6 +337,12 @@ private:
ContainerID id;
Option<TaskInfo> task;
ExecutorInfo executor;
+ std::string directory;
+ Option<std::string> user;
+ SlaveID slaveId;
+ PID<Slave> slavePid;
+ bool checkpoint;
+ Flags flags;
// Promise for future returned from wait().
Promise<containerizer::Termination> termination;
@@ -384,7 +436,11 @@ DockerContainerizerProcess::Container::create(
const Option<TaskInfo>& taskInfo,
const ExecutorInfo& executorInfo,
const string& directory,
- const Option<string>& user)
+ const Option<string>& user,
+ const SlaveID& slaveId,
+ const PID<Slave>& slavePid,
+ bool checkpoint,
+ const Flags& flags)
{
// Before we do anything else we first make sure the stdout/stderr
// files exist and have the right file ownership.
@@ -408,7 +464,16 @@ DockerContainerizerProcess::Container::create(
}
}
- return new Container(id, taskInfo, executorInfo);
+ return new Container(
+ id,
+ taskInfo,
+ executorInfo,
+ directory,
+ user,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags);
}
@@ -763,7 +828,15 @@ Future<bool> DockerContainerizerProcess::launch(
}
Try<Container*> container = Container::create(
- containerId, taskInfo, executorInfo, directory, user);
+ containerId,
+ taskInfo,
+ executorInfo,
+ directory,
+ user,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags);
if (container.isError()) {
return Failure("Failed to create container: " + container.error());
@@ -778,8 +851,9 @@ Future<bool> DockerContainerizerProcess::launch(
return fetch(containerId, taskInfo.command(), directory)
.then(defer(self(), &Self::_launch, containerId, directory))
+ .then(defer(self(), &Self::__launch, containerId, directory))
.then(defer(self(),
- &Self::__launch,
+ &Self::___launch,
containerId,
taskInfo,
executorInfo,
@@ -809,40 +883,35 @@ Future<Nothing> 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)
{
if (!containers_.contains(containerId)) {
return Failure("Container was destroyed while pulling image");
}
- containers_[containerId]->state = Container::RUNNING;
+ Container* container = containers_[containerId];
+
+ container->state = Container::RUNNING;
// Try and start the Docker container.
- containers_[containerId]->run = docker.run(
- taskInfo.container(),
- taskInfo.command(),
- containers_[containerId]->name(),
+ return container->run = docker.run(
+ container->container(),
+ container->command(),
+ container->name(),
directory,
flags.docker_sandbox_directory,
- taskInfo.resources());
-
- return containers_[containerId]->run
- .then(defer(self(),
- &Self::___launch,
- containerId,
- taskInfo,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint));
+ // TODO(benh): Figure out the right resources to pass here.
+ // Apparently when running a container for a task we pass the
+ // resources (taskInfo.resources()) but not when running a
+ // container for an executor!? Either way when we do an
+ // 'update' later we should properly set the resources but I
+ // don't know why in the past we were sometimes not passing
+ // resources (and I can't seem to find any documentation that
+ // addresses this issue).
+ None(),
+ container->environment());
}
@@ -978,7 +1047,15 @@ Future<bool> DockerContainerizerProcess::launch(
}
Try<Container*> container = Container::create(
- containerId, None(), executorInfo, directory, user);
+ containerId,
+ None(),
+ executorInfo,
+ directory,
+ user,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags);
if (container.isError()) {
return Failure("Failed to create container: " + container.error());
@@ -992,8 +1069,9 @@ Future<bool> DockerContainerizerProcess::launch(
return fetch(containerId, executorInfo.command(), directory)
.then(defer(self(), &Self::_launch, containerId, directory))
+ .then(defer(self(), &Self::__launch, containerId, directory))
.then(defer(self(),
- &Self::__launch,
+ &Self::___launch,
containerId,
executorInfo,
directory,
@@ -1004,56 +1082,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)
-{
- if (!containers_.contains(containerId)) {
- return Failure("Container was destroyed while pulling image");
- }
-
- containers_[containerId]->state = Container::RUNNING;
-
- map<string, string> env = executorEnvironment(
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint,
- flags.recovery_timeout);
-
- // Include any environment variables from CommandInfo.
- foreach (const Environment::Variable& variable,
- executorInfo.command().environment().variables()) {
- env[variable.name()] = variable.value();
- }
-
- // Try and start the Docker container.
- containers_[containerId]->run = docker.run(
- executorInfo.container(),
- executorInfo.command(),
- containers_[containerId]->name(),
- directory,
- flags.docker_sandbox_directory,
- None(),
- env);
-
- return containers_[containerId]->run
- .then(defer(self(),
- &Self::___launch,
- containerId,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint));
-}
-
-
Future<bool> DockerContainerizerProcess::___launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
[05/12] git commit: Minor cleanup when creating Docker containers.
Posted by be...@apache.org.
Minor cleanup when creating Docker containers.
Review: https://reviews.apache.org/r/26616
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e6fb81c6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e6fb81c6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e6fb81c6
Branch: refs/heads/master
Commit: e6fb81c69a82bbdecf67d1800955f409be018c22
Parents: 9ce1fc5
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 14:44:31 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e6fb81c6/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 73e3c40..4bf5be4 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -234,7 +234,14 @@ private:
slaveId(slaveId),
slavePid(slavePid),
checkpoint(checkpoint),
- flags(flags) {}
+ flags(flags)
+ {
+ if (task.isSome()) {
+ resources = task.get().resources();
+ } else {
+ resources = executor.resources();
+ }
+ }
std::string name()
{
@@ -993,10 +1000,6 @@ Future<bool> DockerContainerizerProcess::___launch(
return Failure("Failed to synchronize with child process: " + error);
}
- // Store the resources for usage().
- CHECK_SOME(container->task);
- container->resources = container->task.get().resources();
-
// And finally watch for when the executor gets reaped.
container->status.set(process::reap(s.get().pid()));
@@ -1102,9 +1105,6 @@ Future<bool> DockerContainerizerProcess::_____launch(
"Failed to checkpoint executor's pid: " + checkpointed.error());
}
- // Store the resources for usage().
- containers_[containerId]->resources = containers_[containerId]->executor.resources();
-
// And finally watch for when the container gets reaped.
containers_[containerId]->status.set(process::reap(pid.get()));
[11/12] git commit: Removed unnecessary parameter.
Posted by be...@apache.org.
Removed unnecessary parameter.
Review: https://reviews.apache.org/r/26618
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9c3cd48c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9c3cd48c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9c3cd48c
Branch: refs/heads/master
Commit: 9c3cd48c5c957b170a1de8c82c3fb89de1c366ca
Parents: 4e2daac
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 14:59:55 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9c3cd48c/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 8b412a6..57a6066 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -147,12 +147,10 @@ private:
const std::list<Docker::Container>& containers);
process::Future<Nothing> _launch(
- const ContainerID& containerId,
- const std::string& directory);
+ const ContainerID& containerId);
process::Future<Nothing> __launch(
- const ContainerID& containerId,
- const std::string& directory);
+ const ContainerID& containerId);
// NOTE: This continuation is only applicable when launching a
// container for a task.
@@ -883,8 +881,8 @@ 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, containerId, directory))
+ .then(defer(self(), &Self::_launch, containerId))
+ .then(defer(self(), &Self::__launch, containerId))
.then(defer(self(), &Self::___launch, containerId))
.then(defer(self(), &Self::______launch, containerId, lambda::_1))
.onFailed(defer(self(), &Self::destroy, containerId, true));
@@ -892,8 +890,7 @@ Future<bool> DockerContainerizerProcess::launch(
Future<Nothing> DockerContainerizerProcess::_launch(
- const ContainerID& containerId,
- const string& directory)
+ const ContainerID& containerId)
{
// Doing the fetch might have succeded but we were actually asked to
// destroy the container, which we did, so don't continue.
@@ -905,13 +902,12 @@ Future<Nothing> DockerContainerizerProcess::_launch(
container->state = Container::PULLING;
- return pull(containerId, directory, container->image());
+ return pull(containerId, container->directory, container->image());
}
Future<Nothing> DockerContainerizerProcess::__launch(
- const ContainerID& containerId,
- const string& directory)
+ const ContainerID& containerId)
{
if (!containers_.contains(containerId)) {
return Failure("Container was destroyed while pulling image");
@@ -926,7 +922,7 @@ Future<Nothing> DockerContainerizerProcess::__launch(
container->container(),
container->command(),
container->name(),
- directory,
+ container->directory,
flags.docker_sandbox_directory,
// TODO(benh): Figure out the right resources to pass here.
// Apparently when running a container for a task we pass the
@@ -1062,8 +1058,8 @@ 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, containerId, directory))
+ .then(defer(self(), &Self::_launch, containerId))
+ .then(defer(self(), &Self::__launch, containerId))
.then(defer(self(), &Self::____launch, containerId))
.then(defer(self(), &Self::_____launch, containerId, lambda::_1))
.then(defer(self(), &Self::______launch, containerId, lambda::_1))
[09/12] git commit: Even more simplification of launch paths in
Docker containerizer.
Posted by be...@apache.org.
Even more simplification of launch paths in Docker containerizer.
Review: https://reviews.apache.org/r/26615
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9ce1fc54
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9ce1fc54
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9ce1fc54
Branch: refs/heads/master
Commit: 9ce1fc5486b341782b01698486b3a3d527c57c3c
Parents: 4cbcac4
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 14:42:20 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 218 +++++++++++++-------------------
1 file changed, 85 insertions(+), 133 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9ce1fc54/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index b211db0..73e3c40 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -139,6 +139,10 @@ private:
process::Future<Nothing> _pull(const std::string& image);
+ Try<Nothing> checkpoint(
+ const ContainerID& containerId,
+ pid_t pid);
+
process::Future<Nothing> _recover(
const std::list<Docker::Container>& containers);
@@ -151,29 +155,13 @@ private:
const std::string& directory);
process::Future<bool> ___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 ContainerID& containerId);
process::Future<bool> ____launch(
+ const ContainerID& containerId);
+
+ process::Future<bool> _____launch(
const ContainerID& containerId,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint,
const Docker::Container& container);
void _destroy(
@@ -357,7 +345,9 @@ private:
Future<Nothing> run;
// We keep track of the resources for each container so we can set
- // the ResourceStatistics limits in usage().
+ // the ResourceStatistics limits in usage(). Note that this is
+ // different than just what we might get from TaskInfo::resources
+ // or ExecutorInfo::resources because they can change dynamically.
Resources resources;
// The mesos-fetcher subprocess, kept around so that we can do a
@@ -558,6 +548,32 @@ Future<Nothing> DockerContainerizerProcess::_pull(const string& image)
}
+Try<Nothing> DockerContainerizerProcess::checkpoint(
+ const ContainerID& containerId,
+ pid_t pid)
+{
+ CHECK(containers_.contains(containerId));
+
+ Container* container = containers_[containerId];
+
+ if (container->checkpoint) {
+ const string& path =
+ slave::paths::getForkedPidPath(
+ slave::paths::getMetaRootDir(flags.work_dir),
+ container->slaveId,
+ container->executor.framework_id(),
+ container->executor.executor_id(),
+ containerId);
+
+ LOG(INFO) << "Checkpointing pid " << pid << " to '" << path << "'";
+
+ return slave::state::checkpoint(path, stringify(pid));
+ }
+
+ return Nothing();
+}
+
+
Future<Nothing> DockerContainerizer::recover(
const Option<SlaveState>& state)
{
@@ -852,15 +868,7 @@ Future<bool> DockerContainerizerProcess::launch(
return fetch(containerId, taskInfo.command(), directory)
.then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(), &Self::__launch, containerId, directory))
- .then(defer(self(),
- &Self::___launch,
- containerId,
- taskInfo,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint))
+ .then(defer(self(), &Self::___launch, containerId))
.onFailed(defer(self(), &Self::destroy, containerId, true));
}
@@ -916,31 +924,27 @@ Future<Nothing> DockerContainerizerProcess::__launch(
Future<bool> DockerContainerizerProcess::___launch(
- const ContainerID& containerId,
- const TaskInfo& taskInfo,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint)
+ const ContainerID& containerId)
{
// After we do Docker::run we shouldn't remove a container until
// after we set 'status', which we do in this function.
CHECK(containers_.contains(containerId));
+ Container* container = containers_[containerId];
+
// Prepare environment variables for the executor.
- map<string, string> env = executorEnvironment(
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint,
+ map<string, string> environment = executorEnvironment(
+ container->executor,
+ container->directory,
+ container->slaveId,
+ container->slavePid,
+ container->checkpoint,
flags.recovery_timeout);
// Include any enviroment variables from ExecutorInfo.
foreach (const Environment::Variable& variable,
- executorInfo.command().environment().variables()) {
- env[variable.name()] = variable.value();
+ container->executor.command().environment().variables()) {
+ environment[variable.name()] = variable.value();
}
// Construct the mesos-executor "override" to do a 'docker wait'
@@ -949,46 +953,30 @@ Future<bool> DockerContainerizerProcess::___launch(
// don't want the exit status from 'docker wait' but rather the exit
// status from the container, hence the use of /bin/bash.
string override =
- "/bin/sh -c 'exit `" +
- flags.docker + " wait " + containers_[containerId]->name() + "`'";
+ "/bin/sh -c 'exit `" + flags.docker + " wait " + container->name() + "`'";
Try<Subprocess> s = subprocess(
- executorInfo.command().value() + " --override " + override,
+ container->executor.command().value() + " --override " + override,
Subprocess::PIPE(),
- Subprocess::PATH(path::join(directory, "stdout")),
- Subprocess::PATH(path::join(directory, "stderr")),
- env,
- lambda::bind(&setup, directory));
+ Subprocess::PATH(path::join(container->directory, "stdout")),
+ Subprocess::PATH(path::join(container->directory, "stderr")),
+ environment,
+ lambda::bind(&setup, container->directory));
if (s.isError()) {
return Failure("Failed to fork executor: " + s.error());
}
- // Checkpoint the executor's pid if requested.
- if (checkpoint) {
- const string& path = slave::paths::getForkedPidPath(
- slave::paths::getMetaRootDir(flags.work_dir),
- slaveId,
- executorInfo.framework_id(),
- executorInfo.executor_id(),
- containerId);
-
- LOG(INFO) << "Checkpointing executor's forked pid "
- << s.get().pid() << " to '" << path << "'";
+ // Checkpoint the executor's pid (if necessary).
+ Try<Nothing> checkpointed = checkpoint(containerId, s.get().pid());
- Try<Nothing> checkpointed =
- slave::state::checkpoint(path, stringify(s.get().pid()));
-
- if (checkpointed.isError()) {
- LOG(ERROR) << "Failed to checkpoint executor's forked pid to '"
- << path << "': " << checkpointed.error();
-
- // Close the subprocess's stdin so that it aborts.
- CHECK_SOME(s.get().in());
- os::close(s.get().in().get());
+ if (checkpointed.isError()) {
+ // Close the subprocess's stdin so that it aborts.
+ CHECK_SOME(s.get().in());
+ os::close(s.get().in().get());
- return Failure("Could not checkpoint executor's pid");
- }
+ return Failure(
+ "Failed to checkpoint executor's pid: " + checkpointed.error());
}
// Checkpoing complete, now synchronize with the process so that it
@@ -1006,16 +994,17 @@ Future<bool> DockerContainerizerProcess::___launch(
}
// Store the resources for usage().
- containers_[containerId]->resources = taskInfo.resources();
+ CHECK_SOME(container->task);
+ container->resources = container->task.get().resources();
// And finally watch for when the executor gets reaped.
- containers_[containerId]->status.set(process::reap(s.get().pid()));
+ container->status.set(process::reap(s.get().pid()));
- containers_[containerId]->status.future().get()
+ container->status.future().get()
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(containers_[containerId]->name(), directory);
+ docker.logs(container->name(), container->directory);
return true;
}
@@ -1070,48 +1059,24 @@ Future<bool> DockerContainerizerProcess::launch(
return fetch(containerId, executorInfo.command(), directory)
.then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(), &Self::__launch, containerId, directory))
- .then(defer(self(),
- &Self::___launch,
- containerId,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint))
+ .then(defer(self(), &Self::____launch, containerId))
.onFailed(defer(self(), &Self::destroy, containerId, true));
}
-Future<bool> DockerContainerizerProcess::___launch(
- const ContainerID& containerId,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint)
+Future<bool> DockerContainerizerProcess::____launch(
+ const ContainerID& containerId)
{
// We shouldn't remove container until we set 'status'.
CHECK(containers_.contains(containerId));
+
return docker.inspect(containers_[containerId]->name())
- .then(defer(self(),
- &Self::____launch,
- containerId,
- executorInfo,
- directory,
- slaveId,
- slavePid,
- checkpoint,
- lambda::_1));
+ .then(defer(self(), &Self::_____launch, containerId, lambda::_1));
}
-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,
const Docker::Container& container)
{
// After we do Docker::run we shouldn't remove a container until
@@ -1124,34 +1089,21 @@ Future<bool> DockerContainerizerProcess::____launch(
return Failure("Unable to get executor pid after launch");
}
- if (checkpoint) {
- // TODO(tnachen): We might not be able to checkpoint if the slave
- // dies before it can checkpoint while the executor is still
- // running. Optinally we can consider recording the slave id and
- // executor id as part of the docker container name so we can
- // recover from this.
- const string& path =
- slave::paths::getForkedPidPath(
- slave::paths::getMetaRootDir(flags.work_dir),
- slaveId,
- executorInfo.framework_id(),
- executorInfo.executor_id(),
- containerId);
+ // TODO(tnachen): We might not be able to checkpoint if the slave
+ // dies before it can checkpoint while the executor is still
+ // running. Optinally we can consider recording the slave id and
+ // executor id as part of the docker container name so we can
+ // recover from this.
- LOG(INFO) << "Checkpointing executor's forked pid "
- << pid.get() << " to '" << path << "'";
+ Try<Nothing> checkpointed = checkpoint(containerId, pid.get());
- Try<Nothing> checkpointed =
- slave::state::checkpoint(path, stringify(pid.get()));
-
- if (checkpointed.isError()) {
- return Failure("Failed to checkpoint executor's forked pid to '"
- + path + "': " + checkpointed.error());
- }
+ if (checkpointed.isError()) {
+ return Failure(
+ "Failed to checkpoint executor's pid: " + checkpointed.error());
}
// Store the resources for usage().
- containers_[containerId]->resources = executorInfo.resources();
+ containers_[containerId]->resources = containers_[containerId]->executor.resources();
// And finally watch for when the container gets reaped.
containers_[containerId]->status.set(process::reap(pid.get()));
@@ -1160,7 +1112,7 @@ Future<bool> DockerContainerizerProcess::____launch(
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(containers_[containerId]->name(), directory);
+ docker.logs(containers_[containerId]->name(), containers_[containerId]->directory);
return true;
}
[12/12] git commit: Replaced all tabs with spaces.
Posted by be...@apache.org.
Replaced all tabs with spaces.
Review: https://reviews.apache.org/r/26621
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ed4f96f9
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ed4f96f9
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ed4f96f9
Branch: refs/heads/master
Commit: ed4f96f99cfae1b526eadb9880e9e247c0fdbf4b
Parents: f12a633
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 19:15:36 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 90 ++++++++++++++++-----------------
1 file changed, 45 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ed4f96f9/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 07502e1..d6617a9 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -211,43 +211,43 @@ private:
struct Container
{
static Try<Container*> create(
- const ContainerID& id,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& directory,
- const Option<std::string>& user,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint,
- const Flags& flags);
+ const ContainerID& id,
+ const Option<TaskInfo>& taskInfo,
+ const ExecutorInfo& executorInfo,
+ const std::string& directory,
+ const Option<std::string>& user,
+ const SlaveID& slaveId,
+ const PID<Slave>& slavePid,
+ bool checkpoint,
+ const Flags& flags);
Container(const ContainerID& id)
: state(FETCHING), id(id) {}
Container(const ContainerID& id,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& directory,
- const Option<std::string>& user,
- const SlaveID& slaveId,
- const PID<Slave>& slavePid,
- bool checkpoint,
- const Flags& flags)
+ const Option<TaskInfo>& taskInfo,
+ const ExecutorInfo& executorInfo,
+ const std::string& directory,
+ const Option<std::string>& user,
+ const SlaveID& slaveId,
+ const PID<Slave>& slavePid,
+ bool checkpoint,
+ const Flags& flags)
: state(FETCHING),
- id(id),
- task(taskInfo),
- executor(executorInfo),
- directory(directory),
- user(user),
- slaveId(slaveId),
- slavePid(slavePid),
- checkpoint(checkpoint),
- flags(flags)
+ id(id),
+ task(taskInfo),
+ executor(executorInfo),
+ directory(directory),
+ user(user),
+ slaveId(slaveId),
+ slavePid(slavePid),
+ checkpoint(checkpoint),
+ flags(flags)
{
if (task.isSome()) {
- resources = task.get().resources();
+ resources = task.get().resources();
} else {
- resources = executor.resources();
+ resources = executor.resources();
}
}
@@ -259,7 +259,7 @@ private:
std::string image() const
{
if (task.isSome()) {
- return task.get().container().docker().image();
+ return task.get().container().docker().image();
}
return executor.container().docker().image();
@@ -268,7 +268,7 @@ private:
ContainerInfo container() const
{
if (task.isSome()) {
- return task.get().container();
+ return task.get().container();
}
return executor.container();
@@ -277,7 +277,7 @@ private:
CommandInfo command() const
{
if (task.isSome()) {
- return task.get().command();
+ return task.get().command();
}
return executor.command();
@@ -288,13 +288,13 @@ private:
std::map<std::string, std::string> environment() const
{
if (task.isNone()) {
- return executorEnvironment(
- executor,
- directory,
- slaveId,
- slavePid,
- checkpoint,
- flags.recovery_timeout);
+ return executorEnvironment(
+ executor,
+ directory,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags.recovery_timeout);
}
return std::map<std::string, std::string>();
@@ -566,11 +566,11 @@ Try<Nothing> DockerContainerizerProcess::checkpoint(
if (container->checkpoint) {
const string& path =
slave::paths::getForkedPidPath(
- slave::paths::getMetaRootDir(flags.work_dir),
- container->slaveId,
- container->executor.framework_id(),
- container->executor.executor_id(),
- containerId);
+ slave::paths::getMetaRootDir(flags.work_dir),
+ container->slaveId,
+ container->executor.framework_id(),
+ container->executor.executor_id(),
+ containerId);
LOG(INFO) << "Checkpointing pid " << pid << " to '" << path << "'";
@@ -974,7 +974,7 @@ Future<pid_t> DockerContainerizerProcess::___launch(
os::close(s.get().in().get());
return Failure(
- "Failed to checkpoint executor's pid: " + checkpointed.error());
+ "Failed to checkpoint executor's pid: " + checkpointed.error());
}
// Checkpoing complete, now synchronize with the process so that it
@@ -1088,7 +1088,7 @@ Future<pid_t> DockerContainerizerProcess::_____launch(
if (checkpointed.isError()) {
return Failure(
- "Failed to checkpoint executor's pid: " + checkpointed.error());
+ "Failed to checkpoint executor's pid: " + checkpointed.error());
}
return pid.get();
[02/12] git commit: Introduced
DockerContainerizerProcess::Container::name.
Posted by be...@apache.org.
Introduced DockerContainerizerProcess::Container::name.
Review: https://reviews.apache.org/r/26611
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/05211c53
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/05211c53
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/05211c53
Branch: refs/heads/master
Commit: 05211c532d20d6802f49b3f21407c94935a88879
Parents: 3073bd4
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 12:38:14 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:38 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/05211c53/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 9a29489..312c8d8 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -234,8 +234,6 @@ private:
// container destroy.
void reaped(const ContainerID& containerId);
- static std::string containerName(const ContainerID& containerId);
-
const Flags flags;
Docker docker;
@@ -245,6 +243,11 @@ private:
Container(const ContainerID& id)
: state(FETCHING), id(id) {}
+ std::string name()
+ {
+ return DOCKER_NAME_PREFIX + stringify(id);
+ }
+
// 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,
@@ -565,12 +568,6 @@ static int setup(const string& directory)
}
-string DockerContainerizerProcess::containerName(const ContainerID& containerId)
-{
- return DOCKER_NAME_PREFIX + stringify(containerId);
-}
-
-
Future<Nothing> DockerContainerizerProcess::recover(
const Option<SlaveState>& state)
{
@@ -810,7 +807,7 @@ Future<bool> DockerContainerizerProcess::__launch(
containers_[containerId]->run = docker.run(
taskInfo.container(),
taskInfo.command(),
- containerName(containerId),
+ containers_[containerId]->name(),
directory,
flags.docker_sandbox_directory,
taskInfo.resources());
@@ -863,7 +860,7 @@ Future<bool> DockerContainerizerProcess::___launch(
// status from the container, hence the use of /bin/bash.
string override =
"/bin/sh -c 'exit `" +
- flags.docker + " wait " + containerName(containerId) + "`'";
+ flags.docker + " wait " + containers_[containerId]->name() + "`'";
Try<Subprocess> s = subprocess(
executorInfo.command().value() + " --override " + override,
@@ -928,7 +925,7 @@ Future<bool> DockerContainerizerProcess::___launch(
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(containerName(containerId), directory);
+ docker.logs(containers_[containerId]->name(), directory);
return true;
}
@@ -1061,7 +1058,7 @@ Future<bool> DockerContainerizerProcess::__launch(
containers_[containerId]->run = docker.run(
executorInfo.container(),
executorInfo.command(),
- containerName(containerId),
+ containers_[containerId]->name(),
directory,
flags.docker_sandbox_directory,
None(),
@@ -1089,7 +1086,7 @@ Future<bool> DockerContainerizerProcess::___launch(
{
// We shouldn't remove container until we set 'status'.
CHECK(containers_.contains(containerId));
- return docker.inspect(containerName(containerId))
+ return docker.inspect(containers_[containerId]->name())
.then(defer(self(),
&Self::____launch,
containerId,
@@ -1157,7 +1154,7 @@ Future<bool> DockerContainerizerProcess::____launch(
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(containerName(containerId), directory);
+ docker.logs(containers_[containerId]->name(), directory);
return true;
}
@@ -1195,7 +1192,7 @@ Future<Nothing> DockerContainerizerProcess::update(
return __update(containerId, _resources, container->pid.get());
}
- return docker.inspect(containerName(containerId))
+ return docker.inspect(container->name())
.then(defer(self(), &Self::_update, containerId, _resources, lambda::_1));
#else
return Nothing();
@@ -1371,7 +1368,7 @@ Future<ResourceStatistics> DockerContainerizerProcess::usage(
return Failure("Container is being removed: " + stringify(containerId));
}
- return docker.inspect(containerName(containerId))
+ return docker.inspect(container->name())
.then(defer(self(), &Self::_usage, containerId, lambda::_1));
#endif // __linux__
}
@@ -1567,7 +1564,7 @@ void DockerContainerizerProcess::_destroy(
LOG(INFO) << "Running docker kill on container '" << containerId << "'";
- docker.kill(containerName(containerId), true)
+ docker.kill(container->name(), true)
.onAny(defer(self(), &Self::__destroy, containerId, killed, lambda::_1));
}
[06/12] git commit: And more refactoring in the Docker containerizer
to simplify.
Posted by be...@apache.org.
And more refactoring in the Docker containerizer to simplify.
Review: https://reviews.apache.org/r/26617
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4e2daacc
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4e2daacc
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4e2daacc
Branch: refs/heads/master
Commit: 4e2daacc459bca7fb995af235b6bdb87f1b86881
Parents: e6fb81c
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 14:56:35 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 68 +++++++++++++++++++++------------
1 file changed, 44 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4e2daacc/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 4bf5be4..8b412a6 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -154,16 +154,26 @@ private:
const ContainerID& containerId,
const std::string& directory);
- process::Future<bool> ___launch(
+ // NOTE: This continuation is only applicable when launching a
+ // container for a task.
+ process::Future<pid_t> ___launch(
const ContainerID& containerId);
- process::Future<bool> ____launch(
+ // NOTE: This continuation is only applicable when launching a
+ // container for an executor.
+ process::Future<Docker::Container> ____launch(
const ContainerID& containerId);
- process::Future<bool> _____launch(
+ // NOTE: This continuation is only applicable when launching a
+ // container for an executor.
+ process::Future<pid_t> _____launch(
const ContainerID& containerId,
const Docker::Container& container);
+ process::Future<bool> ______launch(
+ const ContainerID& containerId,
+ pid_t pid);
+
void _destroy(
const ContainerID& containerId,
bool killed);
@@ -876,6 +886,7 @@ Future<bool> DockerContainerizerProcess::launch(
.then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(), &Self::__launch, containerId, directory))
.then(defer(self(), &Self::___launch, containerId))
+ .then(defer(self(), &Self::______launch, containerId, lambda::_1))
.onFailed(defer(self(), &Self::destroy, containerId, true));
}
@@ -930,11 +941,11 @@ Future<Nothing> DockerContainerizerProcess::__launch(
}
-Future<bool> DockerContainerizerProcess::___launch(
+Future<pid_t> DockerContainerizerProcess::___launch(
const ContainerID& containerId)
{
// After we do Docker::run we shouldn't remove a container until
- // after we set 'status', which we do in this function.
+ // after we set Container::status.
CHECK(containers_.contains(containerId));
Container* container = containers_[containerId];
@@ -1000,16 +1011,7 @@ Future<bool> DockerContainerizerProcess::___launch(
return Failure("Failed to synchronize with child process: " + error);
}
- // And finally watch for when the executor gets reaped.
- container->status.set(process::reap(s.get().pid()));
-
- container->status.future().get()
- .onAny(defer(self(), &Self::reaped, containerId));
-
- // TODO(benh): Check failure of Docker::logs.
- docker.logs(container->name(), container->directory);
-
- return true;
+ return s.get().pid();
}
@@ -1063,27 +1065,31 @@ Future<bool> DockerContainerizerProcess::launch(
.then(defer(self(), &Self::_launch, containerId, directory))
.then(defer(self(), &Self::__launch, containerId, directory))
.then(defer(self(), &Self::____launch, containerId))
+ .then(defer(self(), &Self::_____launch, containerId, lambda::_1))
+ .then(defer(self(), &Self::______launch, containerId, lambda::_1))
.onFailed(defer(self(), &Self::destroy, containerId, true));
}
-Future<bool> DockerContainerizerProcess::____launch(
+Future<Docker::Container> DockerContainerizerProcess::____launch(
const ContainerID& containerId)
{
- // We shouldn't remove container until we set 'status'.
+ // After we do Docker::run we shouldn't remove a container until
+ // after we set Container::status.
CHECK(containers_.contains(containerId));
- return docker.inspect(containers_[containerId]->name())
- .then(defer(self(), &Self::_____launch, containerId, lambda::_1));
+ Container* container = containers_[containerId];
+
+ return docker.inspect(container->name());
}
-Future<bool> DockerContainerizerProcess::_____launch(
+Future<pid_t> DockerContainerizerProcess::_____launch(
const ContainerID& containerId,
const Docker::Container& container)
{
// After we do Docker::run we shouldn't remove a container until
- // after we set 'status', which we do in this function.
+ // after we set Container::status.
CHECK(containers_.contains(containerId));
Option<int> pid = container.pid;
@@ -1105,14 +1111,28 @@ Future<bool> DockerContainerizerProcess::_____launch(
"Failed to checkpoint executor's pid: " + checkpointed.error());
}
+ return pid.get();
+}
+
+
+Future<bool> DockerContainerizerProcess::______launch(
+ const ContainerID& containerId,
+ pid_t pid)
+{
+ // After we do Docker::run we shouldn't remove a container until
+ // after we set 'status', which we do in this function.
+ CHECK(containers_.contains(containerId));
+
+ Container* container = containers_[containerId];
+
// And finally watch for when the container gets reaped.
- containers_[containerId]->status.set(process::reap(pid.get()));
+ container->status.set(process::reap(pid));
- containers_[containerId]->status.future().get()
+ container->status.future().get()
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(containers_[containerId]->name(), containers_[containerId]->directory);
+ docker.logs(container->name(), container->directory);
return true;
}
[08/12] git commit: Removed redundant environment for Docker
containers.
Posted by be...@apache.org.
Removed redundant environment for Docker containers.
Review: https://reviews.apache.org/r/26620
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f12a6336
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f12a6336
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f12a6336
Branch: refs/heads/master
Commit: f12a63360b95353db5a3bc937c2219e78bf29011
Parents: fdedd0b
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 19:12:14 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f12a6336/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 86b5ba9..07502e1 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -283,29 +283,21 @@ private:
return executor.command();
}
- // Returns the environment to use when launching the Docker container.
+ // Returns any extra environment varaibles to set when launching
+ // the Docker container (beyond the those found in CommandInfo).
std::map<std::string, std::string> environment() const
{
- if (task.isSome()) {
- // TODO(benh): Is this really correct!?
- return std::map<std::string, std::string>();
- }
-
- std::map<std::string, std::string> environment = executorEnvironment(
- executor,
- directory,
- slaveId,
- slavePid,
- checkpoint,
- flags.recovery_timeout);
-
- // Include any environment variables from CommandInfo.
- foreach (const Environment::Variable& variable,
- executor.command().environment().variables()) {
- environment[variable.name()] = variable.value();
+ if (task.isNone()) {
+ return executorEnvironment(
+ executor,
+ directory,
+ slaveId,
+ slavePid,
+ checkpoint,
+ flags.recovery_timeout);
}
- return environment;
+ return std::map<std::string, std::string>();
}
// The DockerContainerier needs to be able to properly clean up
[03/12] git commit: Simplified launch continuations in
DockerContainerizerProcess.
Posted by be...@apache.org.
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,
[10/12] git commit: Fixed non-deterministic test.
Posted by be...@apache.org.
Fixed non-deterministic test.
The DockerContainerizerTest.ROOT_DOCKER_PortMapping test was doing a
Docker::kill on the container which should be finishing properly which
was causing the call to Docker::kill to fail when the container had
already finished properly and been cleaned up, thus causing the test
to fail.
Review: https://reviews.apache.org/r/26630
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bd7561e8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bd7561e8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bd7561e8
Branch: refs/heads/master
Commit: bd7561e8e6e65d94830d36e2276cd937ac6964b0
Parents: ed4f96f
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Oct 12 14:56:12 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/tests/docker_containerizer_tests.cpp | 4 ----
1 file changed, 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/bd7561e8/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index 67d60a8..d99e567 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -1853,10 +1853,6 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_PortMapping)
ASSERT_SOME(s);
AWAIT_READY_FOR(s.get().status(), Seconds(60));
- string container = slave::DOCKER_NAME_PREFIX + stringify(containerId.get());
-
- AWAIT_READY_FOR(docker.kill(container, false), Seconds(30));
-
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
[04/12] git commit: Introduced
DockerContainerizerProcess::Container::create.
Posted by be...@apache.org.
Introduced DockerContainerizerProcess::Container::create.
Simplifies the generic setup of a new container that currently
consists of making sure stdout/stderr exists and the directory is
properly chown'ed.
Review: https://reviews.apache.org/r/26612
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c3e7f2a8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c3e7f2a8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c3e7f2a8
Branch: refs/heads/master
Commit: c3e7f2a8176f7ae333986fb226dc6bf8952f4d84
Parents: 05211c5
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 12:51:04 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:38 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 86 +++++++++++++++++----------------
1 file changed, 44 insertions(+), 42 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3e7f2a8/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 312c8d8..8873022 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -240,6 +240,11 @@ private:
struct Container
{
+ static Try<Container*> create(
+ const ContainerID& id,
+ const std::string& directory,
+ const Option<std::string>& user);
+
Container(const ContainerID& id)
: state(FETCHING), id(id) {}
@@ -365,6 +370,37 @@ DockerContainerizer::~DockerContainerizer()
}
+Future<Nothing> DockerContainerizerProcess::Container::create(
+ const ContainerID& containerId,
+ const string& directory,
+ const Option<string>& user)
+{
+ // Before we do anything else we first make sure the stdout/stderr
+ // files exist and have the right file ownership.
+ Try<Nothing> touch = os::touch(path::join(directory, "stdout"));
+
+ if (touch.isError()) {
+ return Failure("Failed to touch 'stdout': " + touch.error());
+ }
+
+ touch = os::touch(path::join(directory, "stderr"));
+
+ if (touch.isError()) {
+ return Failure("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 new Container(containerId);
+}
+
+
Future<Nothing> DockerContainerizerProcess::fetch(
const ContainerID& containerId,
const CommandInfo& commandInfo,
@@ -715,36 +751,19 @@ Future<bool> DockerContainerizerProcess::launch(
return false;
}
- // Before we do anything else we first make sure the stdout/stderr
- // files exist and have the right file ownership.
- Try<Nothing> touch = os::touch(path::join(directory, "stdout"));
+ Try<Container*> container = Container::create(containerId, directory, user);
- if (touch.isError()) {
- return Failure("Failed to touch 'stdout': " + touch.error());
+ if (container.isError()) {
+ return Failure("Failed to create container: " + container.error());
}
- touch = os::touch(path::join(directory, "stderr"));
-
- if (touch.isError()) {
- return Failure("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());
- }
- }
+ containers_[containerId] = container.get();
LOG(INFO) << "Starting container '" << containerId
<< "' for task '" << taskInfo.task_id()
<< "' (and executor '" << executorInfo.executor_id()
<< "') of framework '" << executorInfo.framework_id() << "'";
- Container* container = new Container(containerId);
- containers_[containerId] = container;
-
return fetch(containerId, taskInfo.command(), directory)
.then(defer(self(),
&Self::_launch,
@@ -956,35 +975,18 @@ Future<bool> DockerContainerizerProcess::launch(
return false;
}
- // Before we do anything else we first make sure the stdout/stderr
- // files exist and have the right file ownership.
- Try<Nothing> touch = os::touch(path::join(directory, "stdout"));
+ Try<Container*> container = Container::create(containerId, directory, user);
- if (touch.isError()) {
- return Failure("Failed to touch 'stdout': " + touch.error());
- }
-
- touch = os::touch(path::join(directory, "stderr"));
-
- if (touch.isError()) {
- return Failure("Failed to touch 'stderr': " + touch.error());
+ if (container.isError()) {
+ return Failure("Failed to create container: " + container.error());
}
- if (user.isSome()) {
- Try<Nothing> chown = os::chown(user.get(), directory);
-
- if (chown.isError()) {
- return Failure("Failed to chown: " + chown.error());
- }
- }
+ containers_[containerId] = container.get();
LOG(INFO) << "Starting container '" << containerId
<< "' for executor '" << executorInfo.executor_id()
<< "' and framework '" << executorInfo.framework_id() << "'";
- Container* container = new Container(containerId);
- containers_[containerId] = container;
-
return fetch(containerId, executorInfo.command(), directory)
.then(defer(self(),
&Self::_launch,
[07/12] git commit: Use the right resources when starting Docker
container.
Posted by be...@apache.org.
Use the right resources when starting Docker container.
Review: https://reviews.apache.org/r/26619
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fdedd0b5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fdedd0b5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fdedd0b5
Branch: refs/heads/master
Commit: fdedd0b5dd7e6875aed2fa0cd35f4ce85c593326
Parents: 9c3cd48
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sat Oct 11 19:05:28 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Oct 31 15:05:39 2014 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fdedd0b5/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 57a6066..86b5ba9 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -924,15 +924,7 @@ Future<Nothing> DockerContainerizerProcess::__launch(
container->name(),
container->directory,
flags.docker_sandbox_directory,
- // TODO(benh): Figure out the right resources to pass here.
- // Apparently when running a container for a task we pass the
- // resources (taskInfo.resources()) but not when running a
- // container for an executor!? Either way when we do an
- // 'update' later we should properly set the resources but I
- // don't know why in the past we were sometimes not passing
- // resources (and I can't seem to find any documentation that
- // addresses this issue).
- None(),
+ container->resources,
container->environment());
}