You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/09/22 02:18:41 UTC
[1/2] mesos git commit: Made slave's work_dir a shared mount in
LinuxFilesystemIsolator.
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 <yu...@gmail.com>
Authored: Mon Sep 21 14:21:21 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
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()));
}
}
[2/2] mesos git commit: Removed the unneeded container work directory
mounts.
Posted by ji...@apache.org.
Removed the unneeded container work directory mounts.
Review: https://reviews.apache.org/r/38582
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/81f61414
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/81f61414
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/81f61414
Branch: refs/heads/master
Commit: 81f61414c7e26e7317eb6f14d8aa2b45e72f4396
Parents: 2871cbb
Author: Jie Yu <yu...@gmail.com>
Authored: Mon Sep 21 16:47:10 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Sep 21 17:18:22 2015 -0700
----------------------------------------------------------------------
.../isolators/filesystem/linux.cpp | 58 ++------------------
1 file changed, 6 insertions(+), 52 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/81f61414/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 57615ec..e821b27 100644
--- a/src/slave/containerizer/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/isolators/filesystem/linux.cpp
@@ -331,54 +331,7 @@ Future<Option<ContainerPrepareInfo>> LinuxFilesystemIsolatorProcess::__prepare(
ContainerPrepareInfo prepareInfo;
prepareInfo.set_namespaces(CLONE_NEWNS);
- if (rootfs.isNone()) {
- // It the container does not change its root filesystem, we need
- // to do a self bind mount of the container's work directory and
- // mark it as a shared mount. This is necessary for any mounts
- // underneath it to be propagated into the container's mount
- // namespace. This is how we can update persistent volumes.
- LOG(INFO) << "Bind mounting work directory '" << directory
- << "' for container " << containerId;
-
- Try<Nothing> mount = fs::mount(
- directory,
- directory,
- None(),
- MS_BIND,
- NULL);
-
- if (mount.isError()) {
- return Failure(
- "Failed to self bind mount work directory '" +
- directory + "': " + mount.error());
- }
-
- mount = fs::mount(
- None(),
- directory,
- None(),
- MS_SLAVE,
- NULL);
-
- if (mount.isError()) {
- return Failure(
- "Failed to mark work directory '" + directory +
- "' as a slave mount: " + mount.error());
- }
-
- mount = fs::mount(
- None(),
- directory,
- None(),
- MS_SHARED,
- NULL);
-
- if (mount.isError()) {
- return Failure(
- "Failed to mark work directory '" + directory +
- "' as a shared mount: " + mount.error());
- }
- } else {
+ if (rootfs.isSome()) {
// If the container changes its root filesystem, we need to mount
// the container's work directory into its root filesystem
// (creating it if needed) so that the executor and the task can
@@ -854,10 +807,11 @@ Future<Nothing> LinuxFilesystemIsolatorProcess::cleanup(
}
if (!sandboxMountExists) {
- // This could happen if the container is not launched by this
- // isolator (e.g., slaves prior to 0.25.0).
- LOG(WARNING) << "Ignoring unmounting sandbox/work directory"
- << " for container " << containerId;
+ // This could happen if the container was not launched by this
+ // isolator (e.g., slaves prior to 0.25.0), or the container did
+ // not specify a root filesystem.
+ LOG(INFO) << "Ignoring unmounting sandbox/work directory"
+ << " for container " << containerId;
} else {
LOG(INFO) << "Unmounting sandbox/work directory '" << sandbox
<< "' for container " << containerId;