Repository: mesos Updated Branches: refs/heads/master 40a0c3e85 -> 81f61414c
Made slave's work_dir a shared mount in LinuxFilesystemIsolator. Review: https://reviews.apache.org/r/38569 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2871cbb5 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2871cbb5 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2871cbb5 Branch: refs/heads/master Commit: 2871cbb55b5b8ff08d21d527b26bde7275a8d46c Parents: 40a0c3e Author: Jie Yu <[email protected]> Authored: Mon Sep 21 14:21:21 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Mon Sep 21 17:18:21 2015 -0700 ---------------------------------------------------------------------- .../isolators/filesystem/linux.cpp | 69 +++++++++++++++++++- src/tests/environment.cpp | 20 ++++++ src/tests/utils.cpp | 16 +++++ 3 files changed, 104 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/2871cbb5/src/slave/containerizer/isolators/filesystem/linux.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/linux.cpp b/src/slave/containerizer/isolators/filesystem/linux.cpp index f1e6f75..57615ec 100644 --- a/src/slave/containerizer/isolators/filesystem/linux.cpp +++ b/src/slave/containerizer/isolators/filesystem/linux.cpp @@ -31,6 +31,8 @@ #include <stout/stringify.hpp> #include <stout/strings.hpp> +#include <stout/os/shell.hpp> + #include "linux/fs.hpp" #include "linux/ns.hpp" @@ -67,6 +69,71 @@ Try<Isolator*> LinuxFilesystemIsolatorProcess::create( return Error("LinuxFilesystemIsolator requires root privileges"); } + // Make slave's work_dir a shared mount so that when forking a child + // process (with a new mount namespace), the child process does not + // hold extra references to container's work directory mounts and + // provisioner mounts (e.g., when using the bind backend) because + // cleanup operations within work_dir can be propagted to all + // container namespaces. See MESOS-3483 for more details. + LOG(INFO) << "Making '" << flags.work_dir << "' a shared mount"; + + Try<fs::MountInfoTable> table = fs::MountInfoTable::read(); + if (table.isError()) { + return Error("Failed to get mount table: " + table.error()); + } + + Option<fs::MountInfoTable::Entry> workDirMount; + foreach (const fs::MountInfoTable::Entry& entry, table.get().entries) { + // TODO(jieyu): Make sure 'flags.work_dir' is a canonical path. + if (entry.target == flags.work_dir) { + workDirMount = entry; + break; + } + } + + // Do a self bind mount if needed. + if (workDirMount.isNone()) { + // NOTE: Instead of using fs::mount to perform the bind mount, we + // use the shell command here because the syscall 'mount' does not + // update the mount table (i.e., /etc/mtab). In other words, the + // mount will not be visible if the operator types command + // 'mount'. Since this mount will still be presented after all + // containers and the slave are stopped, it's better to make it + // visible. It's OK to use the blocking os::shell here because + // 'create' will only be invoked during initialization. + Try<string> mount = os::shell( + "mount --bind %s %s", + flags.work_dir.c_str(), + flags.work_dir.c_str()); + + if (mount.isError()) { + return Error( + "Failed to self bind mount '" + flags.work_dir + + "': " + mount.error()); + } + } + + // Mark the mount as a shared+slave mount. + Try<string> mount = os::shell( + "mount --make-slave %s", + flags.work_dir.c_str()); + + if (mount.isError()) { + return Error( + "Failed to mark '" + flags.work_dir + + "' as a slave mount: " + mount.error()); + } + + mount = os::shell( + "mount --make-shared %s", + flags.work_dir.c_str()); + + if (mount.isError()) { + return Error( + "Failed to mark '" + flags.work_dir + + "' as a shared mount: " + mount.error()); + } + Owned<MesosIsolatorProcess> process( new LinuxFilesystemIsolatorProcess(flags, provisioner)); @@ -431,7 +498,7 @@ Try<string> LinuxFilesystemIsolatorProcess::script( // doesn't work when the paths contain reserved characters such as // spaces either because such characters in mount info are encoded // in the escaped form (i.e. '\0xx'). - out << "grep '" << flags.work_dir << "' /proc/self/mountinfo | " + out << "grep -E '" << flags.work_dir << "/.+' /proc/self/mountinfo | " << "grep -v '" << containerId.value() << "' | " << "cut -d' ' -f5 | " // '-f5' is the mount target. See MountInfoTable. << "xargs --no-run-if-empty umount -l || " http://git-wip-us.apache.org/repos/asf/mesos/blob/2871cbb5/src/tests/environment.cpp ---------------------------------------------------------------------- diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp index e40cde2..3c586ec 100644 --- a/src/tests/environment.cpp +++ b/src/tests/environment.cpp @@ -21,6 +21,7 @@ #include <sys/wait.h> #include <string.h> +#include <unistd.h> #include <list> #include <set> @@ -39,8 +40,11 @@ #include <stout/os.hpp> #include <stout/strings.hpp> +#include <stout/os/shell.hpp> + #ifdef __linux__ #include "linux/cgroups.hpp" +#include "linux/fs.hpp" #endif #ifdef WITH_NETWORK_ISOLATOR @@ -441,6 +445,22 @@ void Environment::SetUp() void Environment::TearDown() { foreach (const string& directory, directories) { +#ifdef __linux__ + // Try to remove any mounts under 'directory'. + if (::geteuid() == 0) { + Try<string> umount = os::shell( + "grep '%s' /proc/mounts | " + "cut -d' ' -f2 | " + "xargs --no-run-if-empty umount -l", + directory.c_str()); + + if (umount.isError()) { + LOG(ERROR) << "Failed to umount for directory '" << directory + << "': " << umount.error(); + } + } +#endif + Try<Nothing> rmdir = os::rmdir(directory); if (rmdir.isError()) { LOG(ERROR) << "Failed to remove '" << directory http://git-wip-us.apache.org/repos/asf/mesos/blob/2871cbb5/src/tests/utils.cpp ---------------------------------------------------------------------- diff --git a/src/tests/utils.cpp b/src/tests/utils.cpp index 498c9aa..cc3e9e9 100644 --- a/src/tests/utils.cpp +++ b/src/tests/utils.cpp @@ -72,6 +72,22 @@ void TemporaryDirectoryTest::TearDown() ASSERT_SOME(os::chdir(cwd)); if (sandbox.isSome()) { +#ifdef __linux__ + // Try to remove any mounts under sandbox. + if (::geteuid() == 0) { + Try<string> umount = os::shell( + "grep '%s' /proc/mounts | " + "cut -d' ' -f2 | " + "xargs --no-run-if-empty umount -l", + sandbox.get().c_str()); + + if (umount.isError()) { + LOG(ERROR) << "Failed to umount for sandbox '" << sandbox.get() + << "': " << umount.error(); + } + } +#endif + ASSERT_SOME(os::rmdir(sandbox.get())); } }
