Repository: mesos
Updated Branches:
refs/heads/master 51da8c4bf -> 74c61ceba
Combined Mesos containerizer's launch methods.
This simplifies the container launch path by removing combining
the nested and non-nested container code paths into one.
The Mesos containerizer was originally translating the two
`containerizer->launch` entrypoints into a common method (also
called `launch`). The previous commits moved this translation
logic into the caller (i.e. the Agent).
The end result has some slight changes:
* It is now possible for the Agent to specify some more combinations
of `ContainerConfig`. For example, specifying a TaskInfo with
a DEBUG-class container. Or a nested container with Resources.
We may need to add extra validation around this.
* The `bool checkpoint` argument was replaced with a `Option<string>`
which optionally contains an absolute path. This allows us to
remove the `SlaveID` field.
Review: https://reviews.apache.org/r/58903
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/17ffb97a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/17ffb97a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/17ffb97a
Branch: refs/heads/master
Commit: 17ffb97ae7eda78edf85b204eba35bc59649a479
Parents: af21bb7
Author: Joseph Wu <josep...@apache.org>
Authored: Mon Apr 10 17:20:46 2017 -0700
Committer: Joseph Wu <josep...@apache.org>
Committed: Thu May 25 18:37:06 2017 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp | 354 ++++++-------------
src/slave/containerizer/mesos/containerizer.hpp | 45 +--
2 files changed, 111 insertions(+), 288 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/17ffb97a/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp
b/src/slave/containerizer/mesos/containerizer.cpp
index 199202a..f3e6210 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -493,63 +493,16 @@ Future<Nothing> MesosContainerizer::recover(
Future<bool> MesosContainerizer::launch(
const ContainerID& containerId,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const string& directory,
- const Option<string>& user,
- const SlaveID& slaveId,
+ const ContainerConfig& containerConfig,
const map<string, string>& environment,
- bool checkpoint)
+ const Option<std::string>& pidCheckpointPath)
{
- // Need to disambiguate for the compiler.
- Future<bool> (MesosContainerizerProcess::*launch)(
- const ContainerID&,
- const Option<TaskInfo>&,
- const ExecutorInfo&,
- const string&,
- const Option<string>&,
- const SlaveID&,
- const map<string, string>&,
- bool) = &MesosContainerizerProcess::launch;
-
return dispatch(process.get(),
- launch,
+ &MesosContainerizerProcess::launch,
containerId,
- taskInfo,
- executorInfo,
- directory,
- user,
- slaveId,
+ containerConfig,
environment,
- checkpoint);
-}
-
-
-Future<bool> MesosContainerizer::launch(
- const ContainerID& containerId,
- const CommandInfo& commandInfo,
- const Option<ContainerInfo>& containerInfo,
- const Option<string>& user,
- const SlaveID& slaveId,
- const Option<ContainerClass>& containerClass)
-{
- // Need to disambiguate for the compiler.
- Future<bool> (MesosContainerizerProcess::*launch)(
- const ContainerID&,
- const CommandInfo&,
- const Option<ContainerInfo>&,
- const Option<string>&,
- const SlaveID&,
- const Option<ContainerClass>&) = &MesosContainerizerProcess::launch;
-
- return dispatch(process.get(),
- launch,
- containerId,
- commandInfo,
- containerInfo,
- user,
- slaveId,
- containerClass);
+ pidCheckpointPath);
}
@@ -991,99 +944,117 @@ Future<Nothing> MesosContainerizerProcess::__recover(
}
-// Launching an executor involves the following steps:
+// Launching an container involves the following steps:
// 1. Call prepare on each isolator.
-// 2. Fork the executor. The forked child is blocked from exec'ing until it has
-// been isolated.
-// 3. Isolate the executor. Call isolate with the pid for each isolator.
-// 4. Fetch the executor.
-// 5. Exec the executor. The forked child is signalled to continue. It will
-// first execute any preparation commands from isolators and then exec the
-// executor.
+// 2. Fork a helper process. The forked helper is blocked from exec'ing
+// until it has been isolated.
+// 3. Isolate the helper's pid; e.g. call `isolate` for each isolator.
+// 4. Fetch any URIs.
+// 5. Signal the helper process to continue. It will first execute any
+// preparation commands from isolators and then exec the starting command.
Future<bool> MesosContainerizerProcess::launch(
const ContainerID& containerId,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& _executorInfo,
- const string& directory,
- const Option<string>& user,
- const SlaveID& slaveId,
+ const ContainerConfig& _containerConfig,
const map<string, string>& environment,
- bool checkpoint)
+ const Option<std::string>& pidCheckpointPath)
{
- CHECK(!containerId.has_parent());
-
if (containers_.contains(containerId)) {
- return Failure("Container already started");
+ return Failure(
+ (containerId.has_parent() ? "Nested container" : "Container") +
+ stringify(containerId) + " already started");
}
- if (taskInfo.isSome() &&
- taskInfo.get().has_container() &&
- taskInfo.get().container().type() != ContainerInfo::MESOS) {
+ if (_containerConfig.has_container_info() &&
+ _containerConfig.container_info().type() != ContainerInfo::MESOS) {
return false;
}
- // NOTE: We make a copy of the executor info because we may mutate
- // it with default container info.
- ExecutorInfo executorInfo = _executorInfo;
+ // NOTE: We make a copy of the ContainerConfig because we may need
+ // to modify it based on the parent container (for nested containers).
+ ContainerConfig containerConfig = _containerConfig;
- if (executorInfo.has_container() &&
- executorInfo.container().type() != ContainerInfo::MESOS) {
- return false;
- }
+ // For nested containers, we must perform some extra validation
+ // (i.e. does the parent exist?) and create the sandbox directory
+ // based on the parent's sandbox.
+ if (containerId.has_parent()) {
+ if (containerConfig.has_task_info() ||
+ containerConfig.has_executor_info()) {
+ return Failure(
+ "Nested containers may not supply a TaskInfo/ExecutorInfo");
+ }
- // Add the default container info to the executor info.
- // TODO(jieyu): Rename the flag to be default_mesos_container_info.
- if (!executorInfo.has_container() &&
- flags.default_container_info.isSome()) {
- executorInfo.mutable_container()->CopyFrom(
- flags.default_container_info.get());
- }
+ if (pidCheckpointPath.isSome()) {
+ return Failure("Nested containers may not be checkpointed");
+ }
- LOG(INFO) << "Starting container " << containerId
- << " for executor '" << executorInfo.executor_id()
- << "' of framework " << executorInfo.framework_id();
+ const ContainerID& parentContainerId = containerId.parent();
- ContainerConfig containerConfig;
- containerConfig.mutable_executor_info()->CopyFrom(executorInfo);
- containerConfig.mutable_command_info()->CopyFrom(executorInfo.command());
- containerConfig.mutable_resources()->CopyFrom(executorInfo.resources());
- containerConfig.set_directory(directory);
+ if (!containers_.contains(parentContainerId)) {
+ return Failure(
+ "Parent container " + stringify(parentContainerId) +
+ " does not exist");
+ }
- if (user.isSome()) {
- containerConfig.set_user(user.get());
- }
+ if (containers_[parentContainerId]->state == DESTROYING) {
+ return Failure(
+ "Parent container " + stringify(parentContainerId) +
+ " is in 'DESTROYING' state");
+ }
- if (taskInfo.isSome()) {
- // Command task case.
- containerConfig.mutable_task_info()->CopyFrom(taskInfo.get());
+ const ContainerID rootContainerId =
+ protobuf::getRootContainerId(containerId);
- if (taskInfo->has_container()) {
- ContainerInfo* containerInfo = containerConfig.mutable_container_info();
- containerInfo->CopyFrom(taskInfo->container());
+ CHECK(containers_.contains(rootContainerId));
+ if (containers_[rootContainerId]->directory.isNone()) {
+ return Failure(
+ "Unexpected empty sandbox directory for root container " +
+ stringify(rootContainerId));
}
- } else {
- // Other cases.
- if (executorInfo.has_container()) {
- ContainerInfo* containerInfo = containerConfig.mutable_container_info();
- containerInfo->CopyFrom(executorInfo.container());
+
+ const string directory = containerizer::paths::getSandboxPath(
+ containers_[rootContainerId]->directory.get(),
+ containerId);
+
+ Try<Nothing> mkdir = os::mkdir(directory);
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create nested sandbox directory '" +
+ directory + "': " + mkdir.error());
}
- }
- return launch(containerId,
- containerConfig,
- environment,
- slaveId,
- checkpoint);
-}
+#ifndef __WINDOWS__
+ if (containerConfig.has_user()) {
+ LOG(INFO) << "Trying to chown '" << directory << "' to user '"
+ << containerConfig.user() << "'";
+
+ Try<Nothing> chown = os::chown(containerConfig.user(), directory);
+ if (chown.isError()) {
+ LOG(WARNING)
+ << "Failed to chown sandbox directory '" << directory
+ << "'. This may be due to attempting to run the container "
+ << "as a nonexistent user on the agent; see the description"
+ << " for the `--switch_user` flag for more information: "
+ << chown.error();
+ }
+ }
+#endif // __WINDOWS__
+ // Modify the sandbox directory in the ContainerConfig.
+ // TODO(josephw): Should we validate that this value
+ // is not set for nested containers?
+ containerConfig.set_directory(directory);
+
+ // TODO(jieyu): This is currently best effort. After the agent fails
+ // over, 'executor_info' won't be set in root parent container's
+ // 'config'. Consider populating 'executor_info' in recover path.
+ if (containers_[rootContainerId]->config.has_executor_info()) {
+ containerConfig.mutable_executor_info()->CopyFrom(
+ containers_[rootContainerId]->config.executor_info());
+ }
+ }
+
+ LOG(INFO) << "Starting container " << containerId;
-Future<bool> MesosContainerizerProcess::launch(
- const ContainerID& containerId,
- const ContainerConfig& containerConfig,
- const map<string, string>& environment,
- const SlaveID& slaveId,
- bool checkpoint)
-{
// Before we launch the container, we first create the container
// runtime directory to hold internal checkpoint information about
// the container.
@@ -1146,8 +1117,7 @@ Future<bool> MesosContainerizerProcess::launch(
containerId,
lambda::_1,
environment,
- slaveId,
- checkpoint));
+ pidCheckpointPath));
}
container->provisioning = provisioner->provision(
@@ -1166,8 +1136,7 @@ Future<bool> MesosContainerizerProcess::launch(
containerId,
lambda::_1,
environment,
- slaveId,
- checkpoint));
+ pidCheckpointPath));
}));
}
@@ -1251,8 +1220,7 @@ Future<Nothing> MesosContainerizerProcess::prepare(
Future<Nothing> MesosContainerizerProcess::fetch(
- const ContainerID& containerId,
- const SlaveID& slaveId)
+ const ContainerID& containerId)
{
if (!containers_.contains(containerId)) {
return Failure("Container destroyed during isolating");
@@ -1270,18 +1238,13 @@ Future<Nothing> MesosContainerizerProcess::fetch(
const string directory = container->config.directory();
- Option<string> user;
- if (container->config.has_user()) {
- user = container->config.user();
- }
-
return fetcher->fetch(
containerId,
container->config.command_info(),
directory,
- user,
- slaveId,
- flags)
+ container->config.has_user()
+ ? container->config.user()
+ : Option<string>::none())
.then([=]() -> Future<Nothing> {
if (HookManager::hooksAvailable()) {
HookManager::slavePostFetchHook(containerId, directory);
@@ -1295,8 +1258,7 @@ Future<bool> MesosContainerizerProcess::_launch(
const ContainerID& containerId,
const Option<ContainerIO>& containerIO,
const map<string, string>& environment,
- const SlaveID& slaveId,
- bool checkpoint)
+ const Option<std::string>& pidCheckpointPath)
{
if (!containers_.contains(containerId)) {
return Failure("Container destroyed during preparing");
@@ -1756,23 +1718,16 @@ Future<bool> MesosContainerizerProcess::_launch(
container->pid = pid;
// Checkpoint the forked pid if requested by the agent.
- if (checkpoint) {
- const string& path = slave::paths::getForkedPidPath(
- slave::paths::getMetaRootDir(flags.work_dir),
- slaveId,
- container->config.executor_info().framework_id(),
- container->config.executor_info().executor_id(),
- containerId);
-
+ if (pidCheckpointPath.isSome()) {
LOG(INFO) << "Checkpointing container's forked pid " << pid
- << " to '" << path << "'";
+ << " to '" << pidCheckpointPath.get() << "'";
Try<Nothing> checkpointed =
- slave::state::checkpoint(path, stringify(pid));
+ slave::state::checkpoint(pidCheckpointPath.get(), stringify(pid));
if (checkpointed.isError()) {
LOG(ERROR) << "Failed to checkpoint container's forked pid to '"
- << path << "': " << checkpointed.error();
+ << pidCheckpointPath.get() << "': " << checkpointed.error();
return Failure("Could not checkpoint container's pid");
}
@@ -1809,8 +1764,7 @@ Future<bool> MesosContainerizerProcess::_launch(
return isolate(containerId, pid)
.then(defer(self(),
&Self::fetch,
- containerId,
- slaveId))
+ containerId))
.then(defer(self(), &Self::exec, containerId, pipes[1]))
.onAny([pipes]() { os::close(pipes[0]); })
.onAny([pipes]() { os::close(pipes[1]); });
@@ -1903,104 +1857,6 @@ Future<bool> MesosContainerizerProcess::exec(
}
-Future<bool> MesosContainerizerProcess::launch(
- const ContainerID& containerId,
- const CommandInfo& commandInfo,
- const Option<ContainerInfo>& containerInfo,
- const Option<string>& user,
- const SlaveID& slaveId,
- const Option<ContainerClass>& containerClass)
-{
- CHECK(containerId.has_parent());
-
- if (containers_.contains(containerId)) {
- return Failure(
- "Nested container " + stringify(containerId) + " already started");
- }
-
- const ContainerID& parentContainerId = containerId.parent();
- if (!containers_.contains(parentContainerId)) {
- return Failure(
- "Parent container " + stringify(parentContainerId) +
- " does not exist");
- }
-
- if (containers_[parentContainerId]->state == DESTROYING) {
- return Failure(
- "Parent container " + stringify(parentContainerId) +
- " is in 'DESTROYING' state");
- }
-
- LOG(INFO) << "Starting nested container " << containerId;
-
- const ContainerID rootContainerId =
protobuf::getRootContainerId(containerId);
-
- CHECK(containers_.contains(rootContainerId));
- if (containers_[rootContainerId]->directory.isNone()) {
- return Failure(
- "Unexpected empty sandbox directory for root container " +
- stringify(rootContainerId));
- }
-
- const string directory = containerizer::paths::getSandboxPath(
- containers_[rootContainerId]->directory.get(),
- containerId);
-
- Try<Nothing> mkdir = os::mkdir(directory);
- if (mkdir.isError()) {
- return Failure(
- "Failed to create nested sandbox directory '" +
- directory + "': " + mkdir.error());
- }
-
-#ifndef __WINDOWS__
- if (user.isSome()) {
- LOG(INFO) << "Trying to chown '" << directory << "' to user '"
- << user.get() << "'";
-
- Try<Nothing> chown = os::chown(user.get(), directory);
- if (chown.isError()) {
- LOG(WARNING) << "Failed to chown sandbox directory '" << directory
- << "'. This may be due to attempting to run the container "
- << "as a nonexistent user on the agent; see the description"
- << " for the `--switch_user` flag for more information: "
- << chown.error();
- }
- }
-#endif // __WINDOWS__
-
- ContainerConfig containerConfig;
- containerConfig.mutable_command_info()->CopyFrom(commandInfo);
- containerConfig.set_directory(directory);
-
- if (user.isSome()) {
- containerConfig.set_user(user.get());
- }
-
- if (containerInfo.isSome()) {
- containerConfig.mutable_container_info()->CopyFrom(containerInfo.get());
- }
-
- if (containerClass.isSome()) {
- containerConfig.set_container_class(containerClass.get());
- }
-
- // TODO(jieyu): This is currently best effort. After the agent fails
- // over, 'executor_info' won't be set in root parent container's
- // 'config'. Consider populating 'executor_info' in recover path.
- if (containers_[rootContainerId]->config.has_executor_info()) {
- containerConfig.mutable_executor_info()->CopyFrom(
- containers_[rootContainerId]->config.executor_info());
- }
-
- return launch(containerId,
- containerConfig,
- map<string, string>(),
- slaveId,
- false);
-}
-
-
Future<Connection> MesosContainerizerProcess::attach(
const ContainerID& containerId)
{
http://git-wip-us.apache.org/repos/asf/mesos/blob/17ffb97a/src/slave/containerizer/mesos/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.hpp
b/src/slave/containerizer/mesos/containerizer.hpp
index d767031..ea06919 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -80,21 +80,9 @@ public:
virtual process::Future<bool> launch(
const ContainerID& containerId,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& directory,
- const Option<std::string>& user,
- const SlaveID& slaveId,
+ const mesos::slave::ContainerConfig& containerConfig,
const std::map<std::string, std::string>& environment,
- bool checkpoint);
-
- virtual process::Future<bool> launch(
- const ContainerID& containerId,
- const CommandInfo& commandInfo,
- const Option<ContainerInfo>& containerInfo,
- const Option<std::string>& user,
- const SlaveID& slaveId,
- const Option<mesos::slave::ContainerClass>& containerClass = None());
+ const Option<std::string>& pidCheckpointPath);
virtual process::Future<process::http::Connection> attach(
const ContainerID& containerId);
@@ -153,21 +141,9 @@ public:
virtual process::Future<bool> launch(
const ContainerID& containerId,
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& directory,
- const Option<std::string>& user,
- const SlaveID& slaveId,
+ const mesos::slave::ContainerConfig& containerConfig,
const std::map<std::string, std::string>& environment,
- bool checkpoint);
-
- virtual process::Future<bool> launch(
- const ContainerID& containerId,
- const CommandInfo& commandInfo,
- const Option<ContainerInfo>& containerInfo,
- const Option<std::string>& user,
- const SlaveID& slaveId,
- const Option<mesos::slave::ContainerClass>& containerClass);
+ const Option<std::string>& pidCheckpointPath);
virtual process::Future<process::http::Connection> attach(
const ContainerID& containerId);
@@ -230,22 +206,13 @@ private:
const Option<ProvisionInfo>& provisionInfo);
process::Future<Nothing> fetch(
- const ContainerID& containerId,
- const SlaveID& slaveId);
-
- process::Future<bool> launch(
- const ContainerID& containerId,
- const mesos::slave::ContainerConfig& containerConfig,
- const std::map<std::string, std::string>& environment,
- const SlaveID& slaveId,
- bool checkpoint);
+ const ContainerID& containerId);
process::Future<bool> _launch(
const ContainerID& containerId,
const Option<mesos::slave::ContainerIO>& containerIO,
const std::map<std::string, std::string>& environment,
- const SlaveID& slaveId,
- bool checkpoint);
+ const Option<std::string>& pidCheckpointPath);
process::Future<bool> isolate(
const ContainerID& containerId,
