[mesos] 05/05: Revert "Made nested contaienr can access its sandbox via `MESOS_SANDBOX`."

[gi...@apache.org]

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

gilbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit eecb82c77117998af0c67a53c64e9b1e975acfa4
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon May 20 14:46:18 2019 -0700

    Revert "Made nested contaienr can access its sandbox via `MESOS_SANDBOX`."
    
    This reverts commit 40beae143a24a35f85b047ef8ee243581f1c3c69.
---
 src/slave/containerizer/mesos/containerizer.cpp    | 24 +++++++---------------
 .../mesos/isolators/filesystem/linux.cpp           | 23 ---------------------
 2 files changed, 7 insertions(+), 40 deletions(-)

diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index c4a6827..0432448 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1837,25 +1837,15 @@ 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. 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`.
+    // 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.
     Environment::Variable* variable = containerEnvironment.add_variables();
     variable->set_name("MESOS_SANDBOX");
-    variable->set_value(
-        (container->config->has_rootfs() ||
-         (strings::contains(flags.isolation, "filesystem/linux") &&
-          containerId.has_parent()))
-          ? flags.sandbox_directory
-          : container->config->directory());
+    variable->set_value(container->config->has_rootfs()
+      ? 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 7b50258..725754f 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -502,16 +502,6 @@ 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());
@@ -754,19 +744,6 @@ 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.