You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by qi...@apache.org on 2019/04/29 08:26:49 UTC
[mesos] 01/02: Made nested contaienr can access its sandbox via
`MESOS_SANDBOX`.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 40beae143a24a35f85b047ef8ee243581f1c3c69
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Fri Apr 19 17:22:45 2019 +0800
Made nested contaienr can access its sandbox via `MESOS_SANDBOX`.
Previously in MESOS-8332 we narrowed task sandbox permissions from 0755
to 0750 which will cause nested container may not has permission to
access its sandbox via the environment variable `MESOS_SANDBOX`. Now in
this patch, for nested container which does not have its own rootfs, we
bind mount its sandbox to the directory specified via the agent flag
`--sandbox_directory` and set `MESOS_SANDBOX` to `--sandbox_directory`
as well, in this way such nested container will have the permission
to access its sandbox via `MESOS_SANDBOX`.
Review: https://reviews.apache.org/r/70514
---
src/slave/containerizer/mesos/containerizer.cpp | 24 +++++++++++++++-------
.../mesos/isolators/filesystem/linux.cpp | 23 +++++++++++++++++++++
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 0432448..c4a6827 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1837,15 +1837,25 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
if (container->containerClass() == ContainerClass::DEFAULT) {
// TODO(jieyu): Consider moving this to filesystem isolator.
//
- // NOTE: For the command executor case, although it uses the host
- // filesystem for itself, we still set 'MESOS_SANDBOX' according to
- // the root filesystem of the task (if specified). Command executor
- // itself does not use this environment variable.
+ // NOTE: For the command executor case, although it uses the host filesystem
+ // for itself, we still set `MESOS_SANDBOX` according to the root filesystem
+ // of the task (if specified). Command executor itself does not use this
+ // environment variable. For nested container which does not have its own
+ // rootfs, if the `filesystem/linux` isolator is enabled, we will also set
+ // `MESOS_SANDBOX` to `flags.sandbox_directory` since in `prepare` method
+ // of the `filesystem/linux` isolator we bind mount such nested container's
+ // sandbox to `flags.sandbox_directory`. Since such bind mount is only done
+ // by the `filesystem/linux` isolator, if another filesystem isolator (e.g.,
+ // `filesystem/posix`) is enabled instead, nested container may still have
+ // no permission to access its sandbox via `MESOS_SANDBOX`.
Environment::Variable* variable = containerEnvironment.add_variables();
variable->set_name("MESOS_SANDBOX");
- variable->set_value(container->config->has_rootfs()
- ? flags.sandbox_directory
- : container->config->directory());
+ variable->set_value(
+ (container->config->has_rootfs() ||
+ (strings::contains(flags.isolation, "filesystem/linux") &&
+ containerId.has_parent()))
+ ? flags.sandbox_directory
+ : container->config->directory());
}
// `launchInfo.environment` contains the environment returned by
diff --git a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
index 725754f..7b50258 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -502,6 +502,16 @@ Try<Isolator*> LinuxFilesystemIsolatorProcess::create(
containersRuntimeDir + "': " + mkdir.error());
}
+ // Create sandbox directory. We will bind mount the sandbox of nested
+ // container which does not have its own rootfs to this directory. See
+ // `prepare` for details.
+ mkdir = os::mkdir(flags.sandbox_directory);
+ if (mkdir.isError()) {
+ return Error(
+ "Failed to create sandbox directory at '" +
+ flags.sandbox_directory + "': " + mkdir.error());
+ }
+
Try<Nothing> containersDirMount = ensureAllowDevices(containersRuntimeDir);
if (containersDirMount.isError()) {
return Error(containersDirMount.error());
@@ -744,6 +754,19 @@ Future<Option<ContainerLaunchInfo>> LinuxFilesystemIsolatorProcess::prepare(
*launchInfo.add_mounts() = createContainerMount(
containerConfig.directory(), sandbox, MS_BIND | MS_REC);
+ } else if (containerId.has_parent()) {
+ // For nested container which does not have its own rootfs, bind mount its
+ // sandbox to the directory specified via `flags.sandbox_directory` (e.g.,
+ // `/mnt/mesos/sandbox`) in its own mount namespace and set the environment
+ // variable `MESOS_SANDBOX` to `flags.sandbox_directory` (see the `_launch`
+ // method of `MesosContainerizerProcess` for details). The reason that we do
+ // this is, in MESOS-8332 we narrowed task sandbox permissions from 0755 to
+ // 0750, since nested container's sandbox is subdirectory under its parent's
+ // sandbox, if we still set `MESOS_SANDBOX` to `containerConfig.directory()`
+ // for nested container, it will not have permission to access its sandbox
+ // via `MESOS_SANDBOX` if its user is different from its parent's user.
+ *launchInfo.add_mounts() = createContainerMount(
+ containerConfig.directory(), flags.sandbox_directory, MS_BIND | MS_REC);
}
// Currently, we only need to update resources for top level containers.