This is an automated email from the ASF dual-hosted git repository. qianzhang pushed a commit to branch 1.8.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d6ac03b3374c1f674c1229e9a6e68df2ea54c0a9 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 | 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.