You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/12/22 20:36:58 UTC
[1/2] mesos git commit: Cleaned up ContainerInfo generation logic in
the slave.
Repository: mesos
Updated Branches:
refs/heads/master 223aad37e -> 7773ce827
Cleaned up ContainerInfo generation logic in the slave.
After this patch, the invariant becomes that for command tasks, we
always set the ContainerInfo of the generated ExecutorInfo to be the
same as that in TaskInfo (if exists). This simplifies the logic when we
actually generate ContainerInfo for containerizer during launching
phase.
This also made the logic introduced in the following patch more readable
and maintainable: https://reviews.apache.org/r/63598
Review: https://reviews.apache.org/r/64811
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7773ce82
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7773ce82
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7773ce82
Branch: refs/heads/master
Commit: 7773ce827edeb4431182284aaba0eda9401d0ac6
Parents: 141173a
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Dec 22 11:42:07 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Dec 22 12:36:22 2017 -0800
----------------------------------------------------------------------
src/slave/slave.cpp | 56 +++++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/7773ce82/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index b3ce5a1..dde3dac 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3006,20 +3006,28 @@ void Slave::launchExecutor(
containerConfig.set_user(executor->user.get());
}
+ // For both of the following cases, `ExecutorInfo.container` is what
+ // we want to tell the containerizer about the container to be
+ // launched:
+ // (1) If this is a command task case (i.e., the framework specifies
+ // the `TaskInfo` but not `ExecutorInfo`), the
+ // `ExecutorInfo.container` is already copied from
+ // `TaskInfo.container` in `Slave::getExecutorInfo`. As a
+ // result, we should just inform the containerizer about
+ // `ExecutorInfo.container`.
+ // (2) If this is a non command task (e.g., default executor, custom
+ // executor), the `ExecutorInfo.container` is what we want to
+ // tell the containerizer anyway.
if (executorInfo_.has_container()) {
- containerConfig.mutable_container_info()
- ->CopyFrom(executorInfo_.container());
+ containerConfig.mutable_container_info()
+ ->CopyFrom(executorInfo_.container());
}
if (executor->isCommandExecutor()) {
- if (taskInfo.isSome()) {
- containerConfig.mutable_task_info()->CopyFrom(taskInfo.get());
+ CHECK_SOME(taskInfo)
+ << "Command (or Docker) executor does not support task group";
- if (taskInfo.get().has_container()) {
- containerConfig.mutable_container_info()
- ->CopyFrom(taskInfo.get().container());
- }
- }
+ containerConfig.mutable_task_info()->CopyFrom(taskInfo.get());
}
// Prepare environment variables for the executor.
@@ -5460,26 +5468,6 @@ ExecutorInfo Slave::getExecutorInfo(
executor.mutable_container()->CopyFrom(task.container());
}
- // TODO(jieyu): We should move those Mesos containerizer specific
- // logic (e.g., 'hasRootfs') to Mesos containerizer.
- bool hasRootfs = task.has_container() &&
- task.container().type() == ContainerInfo::MESOS &&
- task.container().mesos().has_image();
-
- if (hasRootfs) {
- ContainerInfo* container = executor.mutable_container();
-
- // For command tasks, we are now copying the entire
- // `task.container` into the `executorInfo`. Thus,
- // `executor.container` now has the image if `task.container`
- // had one. However, in the case of Mesos container with rootfs,
- // we want to run the command executor in the host filesystem
- // and let the command executor pivot_root to the rootfs for its
- // task. For this reason, we need to strip the image in
- // `executor.container.mesos`.
- container->mutable_mesos()->clear_image();
- }
-
// Prepare an executor name which includes information on the
// command being launched.
string name = "(Task: " + task.task_id().value() + ") ";
@@ -5570,6 +5558,12 @@ ExecutorInfo Slave::getExecutorInfo(
executor.mutable_command()->add_arguments(
"--launcher_dir=" + flags.launcher_dir);
+ // TODO(jieyu): We should move those Mesos containerizer specific
+ // logic (e.g., 'hasRootfs') to Mesos containerizer.
+ bool hasRootfs = task.has_container() &&
+ task.container().type() == ContainerInfo::MESOS &&
+ task.container().mesos().has_image();
+
if (hasRootfs) {
executor.mutable_command()->add_arguments(
"--sandbox_directory=" + flags.sandbox_directory);
@@ -8868,6 +8862,10 @@ Executor::Executor(
{
CHECK_NOTNULL(slave);
+ // TODO(jieyu): The way we determine if an executor is command
+ // executor (including docker executor) is really hacky. We rely on
+ // the fact that docker executor launch command is set in the docker
+ // containerizer so that this check is still valid in the slave.
Result<string> executorPath =
os::realpath(path::join(slave->flags.launcher_dir, MESOS_EXECUTOR));
[2/2] mesos git commit: Set container info from executor by default
if available.
Posted by ji...@apache.org.
Set container info from executor by default if available.
The current implementation only works for non-command executor
instances. We still need to get the container info from the executor
(if none has been defined in the task) in command mode to properly use
some features (volumes for example).
Review: https://reviews.apache.org/r/63598/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/141173a7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/141173a7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/141173a7
Branch: refs/heads/master
Commit: 141173a72c0ef6b209b38ef382700e3b94b463cc
Parents: 223aad3
Author: Julien Pepy <j....@criteo.com>
Authored: Fri Dec 22 09:33:29 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Dec 22 12:36:22 2017 -0800
----------------------------------------------------------------------
src/slave/slave.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/141173a7/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 47d5632..b3ce5a1 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3006,6 +3006,11 @@ void Slave::launchExecutor(
containerConfig.set_user(executor->user.get());
}
+ if (executorInfo_.has_container()) {
+ containerConfig.mutable_container_info()
+ ->CopyFrom(executorInfo_.container());
+ }
+
if (executor->isCommandExecutor()) {
if (taskInfo.isSome()) {
containerConfig.mutable_task_info()->CopyFrom(taskInfo.get());
@@ -3015,11 +3020,6 @@ void Slave::launchExecutor(
->CopyFrom(taskInfo.get().container());
}
}
- } else {
- if (executorInfo_.has_container()) {
- containerConfig.mutable_container_info()
- ->CopyFrom(executorInfo_.container());
- }
}
// Prepare environment variables for the executor.