This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit beaae8df702e51102069b2b0502e924697ae36a2
Author: Qian Zhang <zhq527...@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           | 25 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index e8a4ab3..1867f3b 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1779,15 +1779,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 a47899c..93a88a0 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -202,6 +202,16 @@ Try<Isolator*> 
LinuxFilesystemIsolatorProcess::create(const Flags& flags)
     }
   }
 
+  // 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.
+  Try<Nothing> mkdir = os::mkdir(flags.sandbox_directory);
+  if (mkdir.isError()) {
+    return Error(
+        "Failed to create sandbox directory at '" +
+        flags.sandbox_directory + "': " + mkdir.error());
+  }
+
   Owned<MesosIsolatorProcess> process(
       new LinuxFilesystemIsolatorProcess(flags));
 
@@ -395,6 +405,21 @@ Future<Option<ContainerLaunchInfo>> 
LinuxFilesystemIsolatorProcess::prepare(
     mount->set_source(containerConfig.directory());
     mount->set_target(sandbox);
     mount->set_flags(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.
+    ContainerMountInfo* mount = launchInfo.add_mounts();
+    mount->set_source(containerConfig.directory());
+    mount->set_target(flags.sandbox_directory);
+    mount->set_flags(MS_BIND | MS_REC);
   }
 
   // Currently, we only need to update resources for top level containers.

Reply via email to