mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From y..@apache.org
Subject mesos git commit: Implemented a TODO to clean up host mounts irrelevant to the container in the container's mount namespace.
Date Thu, 17 Sep 2015 22:09:59 GMT
Repository: mesos
Updated Branches:
  refs/heads/master 9b3c8610b -> cf7e9261c


Implemented a TODO to clean up host mounts irrelevant to the container in the container's
mount namespace.

Review: https://reviews.apache.org/r/38447


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cf7e9261
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf7e9261
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf7e9261

Branch: refs/heads/master
Commit: cf7e9261c39fa2d956bb5560cb655cf62d0bddae
Parents: 9b3c861
Author: Jiang Yan Xu <yan@jxu.me>
Authored: Wed Sep 16 23:13:58 2015 -0700
Committer: Jiang Yan Xu <yan@jxu.me>
Committed: Thu Sep 17 15:09:22 2015 -0700

----------------------------------------------------------------------
 .../isolators/filesystem/linux.cpp              |  46 +++-
 .../isolators/filesystem/linux.hpp              |   1 +
 .../containerizer/filesystem_isolator_tests.cpp | 274 ++++++++++++++++---
 3 files changed, 262 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/cf7e9261/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 b674634..152acc3 100644
--- a/src/slave/containerizer/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/isolators/filesystem/linux.cpp
@@ -360,17 +360,14 @@ Future<Option<ContainerPrepareInfo>> LinuxFilesystemIsolatorProcess::__prepare(
   // namespace right after forking the executor process. We use these
   // commands to mount those volumes specified in the container info
   // so that they don't pollute the host mount namespace.
-  if (executorInfo.has_container() &&
-      executorInfo.container().volumes_size() > 0) {
-    Try<string> _script = script(executorInfo, directory, rootfs);
-    if (_script.isError()) {
-      return Failure("Failed to generate isolation script: " + _script.error());
-    }
-
-    CommandInfo* command = prepareInfo.add_commands();
-    command->set_value(_script.get());
+  Try<string> _script = script(containerId, executorInfo, directory, rootfs);
+  if (_script.isError()) {
+    return Failure("Failed to generate isolation script: " + _script.error());
   }
 
+  CommandInfo* command = prepareInfo.add_commands();
+  command->set_value(_script.get());
+
   return update(containerId, executorInfo.resources())
     .then([prepareInfo]() -> Future<Option<ContainerPrepareInfo>> {
       return prepareInfo;
@@ -379,12 +376,11 @@ Future<Option<ContainerPrepareInfo>> LinuxFilesystemIsolatorProcess::__prepare(
 
 
 Try<string> LinuxFilesystemIsolatorProcess::script(
+    const ContainerID& containerId,
     const ExecutorInfo& executorInfo,
     const string& directory,
     const Option<string>& rootfs)
 {
-  CHECK(executorInfo.has_container());
-
   ostringstream out;
   out << "#!/bin/sh\n";
   out << "set -x -e\n";
@@ -393,9 +389,31 @@ Try<string> LinuxFilesystemIsolatorProcess::script(
   // propagate back to the host mount namespace.
   out << "mount --make-rslave /\n";
 
-  // TODO(jieyu): Try to unmount work directory mounts and persistent
-  // volume mounts for other containers to release the extra
-  // references to those mounts.
+  // Try to unmount work directory mounts and persistent volume mounts
+  // for other containers to release the extra references to them.
+  // NOTE:
+  // 1) This doesn't completely eliminate the race condition between
+  //    this container copying mount table and other containers being
+  //    cleaned up. This is instead a best-effort attempt.
+  // 2) This script assumes that all the mounts the container needs
+  //    under the slave work directory have its container ID in the
+  //    path either for the mount source (e.g. sandbox self-bind mount)
+  //    or the mount target (e.g. mounting sandbox into new rootfs).
+  //
+  // TODO(xujyan): This command may fail if --work_dir is not specified
+  // with a real path as real paths are used in the mount table. It
+  // 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 | "
+      << "grep -v '" << containerId.value() << "' | "
+      << "cut -d' ' -f5 | " // '-f5' is the mount target. See MountInfoTable.
+      << "xargs --no-run-if-empty umount -l || "
+      << "true \n"; // We mask errors in this command.
+
+  if (!executorInfo.has_container()) {
+    return out.str();
+  }
 
   foreach (const Volume& volume, executorInfo.container().volumes()) {
     if (!volume.has_host_path()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/cf7e9261/src/slave/containerizer/isolators/filesystem/linux.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/filesystem/linux.hpp b/src/slave/containerizer/isolators/filesystem/linux.hpp
index 99f939f..041af7b 100644
--- a/src/slave/containerizer/isolators/filesystem/linux.hpp
+++ b/src/slave/containerizer/isolators/filesystem/linux.hpp
@@ -100,6 +100,7 @@ private:
       const Option<std::string>& rootfs);
 
   Try<std::string> script(
+      const ContainerID& containerId,
       const ExecutorInfo& executorInfo,
       const std::string& directory,
       const Option<std::string>& rootfs);

http://git-wip-us.apache.org/repos/asf/mesos/blob/cf7e9261/src/tests/containerizer/filesystem_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/filesystem_isolator_tests.cpp b/src/tests/containerizer/filesystem_isolator_tests.cpp
index ca9f423..e2eb049 100644
--- a/src/tests/containerizer/filesystem_isolator_tests.cpp
+++ b/src/tests/containerizer/filesystem_isolator_tests.cpp
@@ -26,6 +26,7 @@
 #include <stout/error.hpp>
 #include <stout/foreach.hpp>
 #include <stout/gtest.hpp>
+#include <stout/hashmap.hpp>
 #include <stout/os.hpp>
 #include <stout/path.hpp>
 #include <stout/uuid.hpp>
@@ -40,6 +41,8 @@
 
 #include "slave/containerizer/mesos/containerizer.hpp"
 
+#include "slave/containerizer/provisioner/paths.hpp"
+
 #include "tests/flags.hpp"
 #include "tests/mesos.hpp"
 
@@ -75,15 +78,17 @@ public:
   // LinuxFilesystemIsolator. The filesystem isolator takes a
   // TestAppcProvisioner which provisions APPC images by copying files
   // from the host filesystem.
+  // 'images' is a map of imageName -> rootfsPath.
+  // TODO(xujyan): The current assumption of one rootfs per image name
+  // is inconsistent with the real provisioner and we should fix it.
   Try<Owned<MesosContainerizer>> createContainerizer(
       const slave::Flags& flags,
-      const vector<string>& imageNames)
+      const hashmap<string, string>& images)
   {
     // Create the root filesystems.
     hashmap<string, Shared<Rootfs>> rootfses;
-    foreach (const string& imageName, imageNames) {
-      Try<Owned<Rootfs>> rootfs =
-        LinuxRootfs::create(path::join(os::getcwd(), "rootfses", imageName));
+    foreachpair (const string& imageName, const string& rootfsPath, images) {
+      Try<Owned<Rootfs>> rootfs = LinuxRootfs::create(rootfsPath);
 
       if (rootfs.isError()) {
         return Error("Failed to create LinuxRootfs: " + rootfs.error());
@@ -181,14 +186,19 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_ChangeRootFilesystem)
 {
   slave::Flags flags = CreateSlaveFlags();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
-
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "[ ! -d '" + os::getcwd() + "' ]");
@@ -229,14 +239,19 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_VolumeFromSandbox)
 {
   slave::Flags flags = CreateSlaveFlags();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
-
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "echo abc > /tmp/file");
@@ -281,14 +296,19 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_VolumeFromHost)
 {
   slave::Flags flags = CreateSlaveFlags();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
-
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "test -d /tmp/sandbox");
@@ -331,14 +351,19 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_VolumeFromHostSandboxMountPoint)
 {
   slave::Flags flags = CreateSlaveFlags();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
-
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "test -d mountpoint/sandbox");
@@ -379,16 +404,24 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_VolumeFromHostSandboxMountPoint)
 TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithRootFilesystem)
 {
   slave::Flags flags = CreateSlaveFlags();
-  flags.work_dir = os::getcwd();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
+  // Need this otherwise the persistent volumes are not created
+  // within the slave work_dir and thus not retrievable.
+  flags.work_dir = os::getcwd();
 
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "echo abc > volume/file");
@@ -441,16 +474,24 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithRootFilesystem)
 TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithoutRootFilesystem)
 {
   slave::Flags flags = CreateSlaveFlags();
-  flags.work_dir = os::getcwd();
-
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
 
-  ASSERT_SOME(containerizer);
+  // Need this otherwise the persistent volumes are not created
+  // within the slave work_dir and thus not retrievable.
+  flags.work_dir = os::getcwd();
 
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "echo abc > volume/file");
@@ -471,8 +512,14 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithoutRootFilesystem)
 
   ASSERT_SOME(os::mkdir(volume));
 
-  string directory = path::join(os::getcwd(), "sandbox");
-  ASSERT_SOME(os::mkdir(directory));
+  // To make sure the sandbox directory has the container ID in its
+  // path so it doesn't get unmounted by the launcher.
+  string directory = slave::paths::createExecutorDirectory(
+      flags.work_dir,
+      SlaveID(),
+      FrameworkID(),
+      executor.executor_id(),
+      containerId);
 
   Future<bool> launch = containerizer.get()->launch(
       containerId,
@@ -507,14 +554,19 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_ImageInVolumeWithoutRootFilesystem)
 {
   slave::Flags flags = CreateSlaveFlags();
 
-  Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image"});
-
-  ASSERT_SOME(containerizer);
-
   ContainerID containerId;
   containerId.set_value(UUID::random().toString());
 
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
+  Try<Owned<MesosContainerizer>> containerizer = createContainerizer(
+      flags,
+      {{"test_image", path::join(rootfsesDir, "test_image")}});
+
+  ASSERT_SOME(containerizer);
+
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "test -d rootfs/bin");
@@ -557,14 +609,21 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_ImageInVolumeWithRootFilesystem)
 {
   slave::Flags flags = CreateSlaveFlags();
 
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  string rootfsesDir = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId);
+
   Try<Owned<MesosContainerizer>> containerizer =
-    createContainerizer(flags, {"test_image_rootfs", "test_image_volume"});
+    createContainerizer(
+        flags,
+        {{"test_image_rootfs", path::join(rootfsesDir, "test_image_rootfs")},
+         {"test_image_volume", path::join(rootfsesDir, "test_image_volume")}});
 
   ASSERT_SOME(containerizer);
 
-  ContainerID containerId;
-  containerId.set_value(UUID::random().toString());
-
   ExecutorInfo executor = CREATE_EXECUTOR_INFO(
       "test_executor",
       "[ ! -d '" + os::getcwd() + "' ] && [ -d rootfs/bin ]");
@@ -598,6 +657,131 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_ImageInVolumeWithRootFilesystem)
   EXPECT_TRUE(wait.get().has_status());
   EXPECT_EQ(0, wait.get().status());
 }
+
+
+// This test verifies that multiple containers with images can be
+// launched simultaneously with no interference.
+TEST_F(LinuxFilesystemIsolatorTest, ROOT_MultipleContainers)
+{
+  slave::Flags flags = CreateSlaveFlags();
+
+  // Need this otherwise the persistent volumes are not created
+  // within the slave work_dir and thus not retrievable.
+  flags.work_dir = os::getcwd();
+
+  ContainerID containerId1;
+  containerId1.set_value(UUID::random().toString());
+
+  ContainerID containerId2;
+  containerId2.set_value(UUID::random().toString());
+
+  string rootfsesDir1 = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId1);
+
+  string rootfsesDir2 = slave::provisioner::paths::getContainerDir(
+      slave::paths::getProvisionerDir(flags.work_dir),
+      containerId2);
+
+  Try<Owned<MesosContainerizer>> containerizer =
+    createContainerizer(
+        flags,
+        {{"test_image1", path::join(rootfsesDir1, "test_image1")},
+         {"test_image2", path::join(rootfsesDir2, "test_image2")}});
+
+  ASSERT_SOME(containerizer);
+
+  SlaveID slaveId;
+  slaveId.set_value("test_slave");
+
+  // First launch container 1 which has a long running task which
+  // guarantees that its work directory mount is in the host mount
+  // table when container 2 is launched.
+  ExecutorInfo executor1 = CREATE_EXECUTOR_INFO(
+      "test_executor1",
+      "sleep 1000"); // Long running task.
+
+  executor1.mutable_container()->CopyFrom(createContainerInfo("test_image1"));
+
+  // Create a persistent volume for container 1. We do this because
+  // we want to test container 2 cleaning up multiple mounts.
+  executor1.add_resources()->CopyFrom(createPersistentVolume(
+      Megabytes(32),
+      "test_role",
+      "persistent_volume_id",
+      "volume"));
+
+  string volume = slave::paths::getPersistentVolumePath(
+      os::getcwd(),
+      "test_role",
+      "persistent_volume_id");
+
+  ASSERT_SOME(os::mkdir(volume));
+
+  string directory1 = slave::paths::createExecutorDirectory(
+      flags.work_dir,
+      slaveId,
+      DEFAULT_FRAMEWORK_INFO.id(),
+      executor1.executor_id(),
+      containerId1);
+
+  Future<bool> launch1 = containerizer.get()->launch(
+      containerId1,
+      executor1,
+      directory1,
+      None(),
+      slaveId,
+      PID<Slave>(),
+      false);
+
+  // Wait for the launch to complete.
+  AWAIT_READY(launch1);
+
+  // Now launch container 2 which will copy the host mount table with
+  // container 1's work directory mount in it.
+  ExecutorInfo executor2 = CREATE_EXECUTOR_INFO(
+      "test_executor2",
+      "[ ! -d '" + os::getcwd() + "' ]");
+
+  executor2.mutable_container()->CopyFrom(createContainerInfo("test_image2"));
+
+  string directory2 = slave::paths::createExecutorDirectory(
+      flags.work_dir,
+      slaveId,
+      DEFAULT_FRAMEWORK_INFO.id(),
+      executor2.executor_id(),
+      containerId2);
+
+  Future<bool> launch2 = containerizer.get()->launch(
+      containerId2,
+      executor2,
+      directory2,
+      None(),
+      slaveId,
+      PID<Slave>(),
+      false);
+
+  AWAIT_READY(launch2);
+
+  containerizer.get()->destroy(containerId1);
+
+  // Wait on the containers.
+  Future<containerizer::Termination> wait1 =
+    containerizer.get()->wait(containerId1);
+  Future<containerizer::Termination> wait2 =
+    containerizer.get()->wait(containerId2);
+
+  AWAIT_READY(wait1);
+  AWAIT_READY(wait2);
+
+  // Executor 1 was forcefully killed.
+  EXPECT_TRUE(wait1.get().has_status());
+  EXPECT_EQ(9, wait1.get().status());
+
+  // Executor 2 exited normally.
+  EXPECT_TRUE(wait2.get().has_status());
+  EXPECT_EQ(0, wait2.get().status());
+}
 #endif // __linux__
 
 } // namespace tests {


Mime
View raw message