You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2018/02/14 08:34:21 UTC
[2/4] mesos git commit: Attached/detached volume directory for task
which has volume specified.
Attached/detached volume directory for task which has volume specified.
Review: https://reviews.apache.org/r/65570/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/345a9621
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/345a9621
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/345a9621
Branch: refs/heads/1.5.x
Commit: 345a9621f1021834d067ee591fb54b53753621c3
Parents: 8ae1e89
Author: Qian Zhang <zh...@gmail.com>
Authored: Wed Feb 14 00:17:37 2018 -0800
Committer: Gilbert Song <so...@gmail.com>
Committed: Wed Feb 14 00:33:51 2018 -0800
----------------------------------------------------------------------
src/slave/slave.cpp | 144 ++++++++++++++++++++++++++++++++++++++++++++---
src/slave/slave.hpp | 50 ++++++++++------
2 files changed, 168 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/345a9621/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 065b255..151c5eb 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1031,6 +1031,7 @@ void Slave::attachTaskVolumeDirectory(
CHECK_EQ(task.executor_id(), executorInfo.executor_id());
+ // This is the case that the task has disk resources specified.
foreach (const Resource& resource, task.resources()) {
// Ignore if there are no disk resources or if the
// disk resources did not specify a volume mapping.
@@ -1040,15 +1041,15 @@ void Slave::attachTaskVolumeDirectory(
const Volume& volume = resource.disk().volume();
- const string executorDirectory = paths::getExecutorRunPath(
+ const string executorRunPath = paths::getExecutorRunPath(
flags.work_dir,
info.id(),
task.framework_id(),
task.executor_id(),
executorContainerId);
- const string executorVolumePath =
- path::join(executorDirectory, volume.container_path());
+ const string executorDirectoryPath =
+ path::join(executorRunPath, volume.container_path());
const string taskPath = paths::getTaskPath(
flags.work_dir,
@@ -1058,16 +1059,86 @@ void Slave::attachTaskVolumeDirectory(
executorContainerId,
task.task_id());
- const string taskVolumePath =
+ const string taskDirectoryPath =
path::join(taskPath, volume.container_path());
- files->attach(executorVolumePath, taskVolumePath)
+ files->attach(executorDirectoryPath, taskDirectoryPath)
.onAny(defer(
self(),
&Self::fileAttached,
lambda::_1,
- executorVolumePath,
- taskVolumePath));
+ executorDirectoryPath,
+ taskDirectoryPath));
+ }
+
+ // This is the case that the executor has disk resources specified
+ // and the task's ContainerInfo has a `SANDBOX_PATH` volume with type
+ // `PARENT` to share the executor's disk volume.
+ hashset<string> executorContainerPaths;
+ foreach (const Resource& resource, executorInfo.resources()) {
+ // Ignore if there are no disk resources or if the
+ // disk resources did not specify a volume mapping.
+ if (!resource.has_disk() || !resource.disk().has_volume()) {
+ continue;
+ }
+
+ const Volume& volume = resource.disk().volume();
+ executorContainerPaths.insert(volume.container_path());
+ }
+
+ if (executorContainerPaths.empty()) {
+ return;
+ }
+
+ if (task.has_container()) {
+ foreach (const Volume& volume, task.container().volumes()) {
+ if (!volume.has_source() ||
+ volume.source().type() != Volume::Source::SANDBOX_PATH) {
+ continue;
+ }
+
+ CHECK(volume.source().has_sandbox_path());
+
+ const Volume::Source::SandboxPath& sandboxPath =
+ volume.source().sandbox_path();
+
+ if (sandboxPath.type() != Volume::Source::SandboxPath::PARENT) {
+ continue;
+ }
+
+ if (!executorContainerPaths.contains(sandboxPath.path())) {
+ continue;
+ }
+
+ const string executorRunPath = paths::getExecutorRunPath(
+ flags.work_dir,
+ info.id(),
+ task.framework_id(),
+ task.executor_id(),
+ executorContainerId);
+
+ const string executorDirectoryPath =
+ path::join(executorRunPath, sandboxPath.path());
+
+ const string taskPath = paths::getTaskPath(
+ flags.work_dir,
+ info.id(),
+ task.framework_id(),
+ task.executor_id(),
+ executorContainerId,
+ task.task_id());
+
+ const string taskDirectoryPath =
+ path::join(taskPath, volume.container_path());
+
+ files->attach(executorDirectoryPath, taskDirectoryPath)
+ .onAny(defer(
+ self(),
+ &Self::fileAttached,
+ lambda::_1,
+ executorDirectoryPath,
+ taskDirectoryPath));
+ }
}
}
@@ -1083,9 +1154,22 @@ void Slave::detachTaskVolumeDirectories(
(executorInfo.has_type() &&
executorInfo.type() == ExecutorInfo::DEFAULT));
+ hashset<string> executorContainerPaths;
+ foreach (const Resource& resource, executorInfo.resources()) {
+ // Ignore if there are no disk resources or if the
+ // disk resources did not specify a volume mapping.
+ if (!resource.has_disk() || !resource.disk().has_volume()) {
+ continue;
+ }
+
+ const Volume& volume = resource.disk().volume();
+ executorContainerPaths.insert(volume.container_path());
+ }
+
foreach (const Task& task, tasks) {
CHECK_EQ(task.executor_id(), executorInfo.executor_id());
+ // This is the case that the task has disk resources specified.
foreach (const Resource& resource, task.resources()) {
// Ignore if there are no disk resources or if the
// disk resources did not specify a volume mapping.
@@ -1103,10 +1187,52 @@ void Slave::detachTaskVolumeDirectories(
executorContainerId,
task.task_id());
- const string taskVolumePath =
+ const string taskDirectoryPath =
path::join(taskPath, volume.container_path());
- files->detach(taskVolumePath);
+ files->detach(taskDirectoryPath);
+ }
+
+ if (executorContainerPaths.empty()) {
+ continue;
+ }
+
+ // This is the case that the executor has disk resources specified
+ // and the task's ContainerInfo has a `SANDBOX_PATH` volume with type
+ // `PARENT` to share the executor's disk volume.
+ if (task.has_container()) {
+ foreach (const Volume& volume, task.container().volumes()) {
+ if (!volume.has_source() ||
+ volume.source().type() != Volume::Source::SANDBOX_PATH) {
+ continue;
+ }
+
+ CHECK(volume.source().has_sandbox_path());
+
+ const Volume::Source::SandboxPath& sandboxPath =
+ volume.source().sandbox_path();
+
+ if (sandboxPath.type() != Volume::Source::SandboxPath::PARENT) {
+ continue;
+ }
+
+ if (!executorContainerPaths.contains(sandboxPath.path())) {
+ continue;
+ }
+
+ const string taskPath = paths::getTaskPath(
+ flags.work_dir,
+ info.id(),
+ task.framework_id(),
+ task.executor_id(),
+ executorContainerId,
+ task.task_id());
+
+ const string taskDirectoryPath =
+ path::join(taskPath, volume.container_path());
+
+ files->detach(taskDirectoryPath);
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/345a9621/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 38808cb..45cea0d 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -409,30 +409,46 @@ public:
Nothing detachFile(const std::string& path);
- // TODO(qianzhang): This is a workaround to make the default executor
- // task's volume directory visible in MESOS UI. In MESOS-7225, we made
- // sure a task can access any volumes specified in its disk resources
- // from its sandbox by introducing a workaround to the default executor,
- // i.e., adding a `SANDBOX_PATH` volume with type `PARENT` to the
- // corresponding nested container. This volume gets translated into a
- // bind mount in the nested container's mount namespace, which is is not
- // visible in Mesos UI because it operates in the host namespace. See
- // Mesos-8279 for details.
+ // TODO(qianzhang): This is a workaround to make the default executor task's
+ // volume directory visible in MESOS UI. It handles two cases:
+ // 1. The task has disk resources specified. In this case any disk resources
+ // specified for the task are mounted on the top level container since
+ // currently all resources of nested containers are merged in the top
+ // level executor container. To make sure the task can access any volumes
+ // specified in its disk resources from its sandbox, a workaround was
+ // introduced to the default executor in MESOS-7225, i.e., adding a
+ // `SANDBOX_PATH` volume with type `PARENT` to the corresponding nested
+ // container. This volume gets translated into a bind mount in the nested
+ // container's mount namespace, which is not visible in Mesos UI because
+ // it operates in the host namespace. See MESOS-8279 for details.
+ // 2. The executor has disk resources specified and the task's ContainerInfo
+ // has a `SANDBOX_PATH` volume with type `PARENT` specified to share the
+ // executor's disk volume. Similar to the first case, this `SANDBOX_PATH`
+ // volume gets translated into a bind mount which is not visible in Mesos
+ // UI. See MESOS-8565 for details.
//
- // To make the task's volume directory visible in Mesos UI, here we
- // attach the executor's volume directory to it, so when users browse
- // task's volume directory in Mesos UI, what they actually browse is the
- // executor's volume directory. Note when calling `Files::attach()`, the
- // third argument `authorized` is not specified because it is already
- // specified when we do the attach for the executor's sandbox and it also
- // applies to the executor's tasks.
+ // To make the task's volume directory visible in Mesos UI, here we attach the
+ // executor's volume directory to it so that it can be accessed via the /files
+ // endpoint. So when users browse task's volume directory in Mesos UI, what
+ // they actually browse is the executor's volume directory. Note when calling
+ // `Files::attach()`, the third argument `authorized` is not specified because
+ // it is already specified when we do the attach for the executor's sandbox
+ // and it also applies to the executor's tasks. Note that for the second case
+ // we can not do the attach when the task's ContainerInfo has a `SANDBOX_PATH`
+ // volume with type `PARENT` but the executor has NO disk resources, because
+ // in such case the attach will fail due to the executor's volume directory
+ // not existing which will actually be created by the `volume/sandbox_path`
+ // isolator when launching the nested container. But if the executor has disk
+ // resources, then we will not have this issue since the executor's volume
+ // directory will be created by the `filesystem/linux` isolator when launching
+ // the executor before we do the attach.
void attachTaskVolumeDirectory(
const ExecutorInfo& executorInfo,
const ContainerID& executorContainerId,
const Task& task);
// TODO(qianzhang): Remove the task's volume directory from the /files
- // endpoint. This is a workaround for MESOS-8279.
+ // endpoint. This is a workaround for MESOS-8279 and MESOS-8565.
void detachTaskVolumeDirectories(
const ExecutorInfo& executorInfo,
const ContainerID& executorContainerId,