You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2019/07/19 22:06:20 UTC

[mesos] 01/03: Added two validations in `namespaces/ipc` isolator.

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 e58f4b97b5d13ccc18ad9b1632d7e6409bdd0c55
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Fri Jul 19 15:06:03 2019 -0700

    Added two validations in `namespaces/ipc` isolator.
    
    1. Do not support specifying the size of /dev/shm when the IPC mode
       is not `PRIVATE`.
    2. Do not support private IPC mode for debug containers.
    
    Review: https://reviews.apache.org/r/71120/
---
 docs/isolators/namespaces-ipc.md                   |  4 ++
 include/mesos/mesos.proto                          |  4 +-
 include/mesos/v1/mesos.proto                       |  4 +-
 .../mesos/isolators/namespaces/ipc.cpp             | 11 +++++
 src/tests/containerizer/isolator_tests.cpp         | 49 ++++++++++------------
 5 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/docs/isolators/namespaces-ipc.md b/docs/isolators/namespaces-ipc.md
index 4c978ec..4009660 100644
--- a/docs/isolators/namespaces-ipc.md
+++ b/docs/isolators/namespaces-ipc.md
@@ -55,3 +55,7 @@ flag, if that flag is not set too, the size of the /dev/shm will be half
 of the agent host RAM which is the default behavior of Linux. The
 `ContainerInfo.linux_info.shm_size` field will be ignored for the container which
 shares its parent's /dev/shm.
+
+Please note that we only support setting the `ContainerInfo.linux_info.shm_size` field
+when the `ContainerInfo.linux_info.ipc_mode` field is set to `PRIVATE`, otherwise the
+container launch will be rejected.
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 075c110..cb6d131 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -3324,7 +3324,9 @@ message LinuxInfo {
   // flag is not set too, the size of the /dev/shm will be half of the host RAM
   // which is the default behavior of Linux. This field will be ignored for the
   // container which shares /dev/shm from its parent and it will be also ignored
-  // for any containers if the `namespaces/ipc` isolator is not enabled.
+  // for any containers if the `namespaces/ipc` isolator is not enabled. Please
+  // note that we only support setting this field when the `ipc_mode` field is
+  // set to `PRIVATE` otherwise the container launch will be rejected.
   optional uint32 shm_size = 7;
 }
 
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 0dcaee6..438c3fe 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -3313,7 +3313,9 @@ message LinuxInfo {
   // flag is not set too, the size of the /dev/shm will be half of the host RAM
   // which is the default behavior of Linux. This field will be ignored for the
   // container which shares /dev/shm from its parent and it will be also ignored
-  // for any containers if the `namespaces/ipc` isolator is not enabled.
+  // for any containers if the `namespaces/ipc` isolator is not enabled. Please
+  // note that we only support setting this field when the `ipc_mode` field is
+  // set to `PRIVATE` otherwise the container launch will be rejected.
   optional uint32 shm_size = 7;
 }
 
diff --git a/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp b/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
index 743d48d..aaaec6b 100644
--- a/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
+++ b/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
@@ -119,6 +119,12 @@ Future<Option<ContainerLaunchInfo>> NamespacesIPCIsolatorProcess::prepare(
     }
 
     if (containerConfig.container_info().linux_info().has_shm_size()) {
+      if (ipcMode != LinuxInfo::PRIVATE) {
+        return Failure(
+            "Only support specifying the size of /dev/shm "
+            "when the IPC mode is `PRIVATE`");
+      }
+
       shmSize =
         Megabytes(containerConfig.container_info().linux_info().shm_size());
     } else if (flags.default_container_shm_size.isSome()) {
@@ -133,6 +139,11 @@ Future<Option<ContainerLaunchInfo>> NamespacesIPCIsolatorProcess::prepare(
     // so it will share its parent container's /dev/shm.
     if (containerConfig.has_container_class() &&
         containerConfig.container_class() == ContainerClass::DEBUG) {
+      if (ipcMode == LinuxInfo::PRIVATE) {
+        return Failure(
+            "Private IPC mode is not supported for DEBUG containers");
+      }
+
       launchInfo.add_enter_namespaces(CLONE_NEWIPC);
       return launchInfo;
     }
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 14feaed..3d79f23 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -523,8 +523,8 @@ TEST_F(NamespacesIsolatorTest, ROOT_IPCNamespaceWithIPCIsolatorDisabled)
 
 // This test verifies that a top-level container with private IPC mode will
 // have its own IPC namespace and /dev/shm, and it can share IPC namespace
-// and /dev/shm with its child container, grandchild container and debug
-// container.
+// and /dev/shm with its child containers, grandchild containers and debug
+// containers.
 TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
 {
   Try<Owned<MesosContainerizer>> containerizer =
@@ -580,8 +580,7 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
   // its own /dev/shm rather than in agent's /dev/shm.
   ASSERT_FALSE(os::exists("/dev/shm/root"));
 
-  // Now launch two child containers with `SHARE_PARENT` ipc mode and
-  // 256MB /dev/shm.
+  // Now launch two child containers with `SHARE_PARENT` ipc mode.
   ContainerID childContainerId1, childContainerId2;
 
   childContainerId1.mutable_parent()->CopyFrom(containerId);
@@ -593,12 +592,11 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
   containerInfo.mutable_linux_info()->set_ipc_mode(LinuxInfo::SHARE_PARENT);
-  containerInfo.mutable_linux_info()->set_shm_size(256);
 
-  // Launch the first child container, check its /dev/shm size is 128MB
-  // rather than 256MB, it can see the file created by its parent container
-  // in /dev/shm and it is in the same IPC namespace with its parent container,
-  // and then write its IPC namespace inode to a file under /dev/shm.
+  // Launch the first child container, check its /dev/shm size is 128MB, it
+  // can see the file created by its parent container in /dev/shm and it is
+  // in the same IPC namespace with its parent container, and then write its
+  // IPC namespace inode to a file under /dev/shm.
   launch = containerizer.get()->launch(
       childContainerId1,
       createContainerConfig(
@@ -630,10 +628,10 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
   EXPECT_LT(waited, process::TEST_AWAIT_TIMEOUT);
 
   // Launch the second child container with its own rootfs, check its /dev/shm
-  // size is 128MB rather than 256MB, it can see the files created by its parent
-  // container and the first child container in /dev/shm and it is in the same
-  // IPC namespace with its parent container and the first child container. and
-  // then write its IPC namespace inode to a file under /dev/shm.
+  // size is 128MB, it can see the files created by its parent container and the
+  // first child container in /dev/shm and it is in the same IPC namespace with
+  // its parent container and the first child container, and then write its IPC
+  // namespace inode to a file under /dev/shm.
   mesos::Image image;
   image.set_type(mesos::Image::DOCKER);
   image.mutable_docker()->set_name("alpine");
@@ -671,10 +669,9 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
 
   EXPECT_LT(waited, process::TEST_AWAIT_TIMEOUT);
 
-  // Launch a grandchild container with `SHARE_PARENT` ipc mode and
-  // 256MB /dev/shm under the first child container, check its /dev/shm
-  // size is 128MB rather than 256MB, it can see the files created by
-  // its parent and grandparent containers and it is in the same IPC
+  // Launch a grandchild container with `SHARE_PARENT` ipc mode under the first
+  // child container, check its /dev/shm size is 128MB, it can see the files
+  // created by its parent and grandparent containers and it is in the same IPC
   // namespace with its parent and grandparent containers.
   ContainerID grandchildContainerId;
   grandchildContainerId.mutable_parent()->CopyFrom(childContainerId1);
@@ -701,16 +698,15 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
   ASSERT_TRUE(wait.get()->has_status());
   EXPECT_WEXITSTATUS_EQ(0, wait.get()->status());
 
-  // Launch a debug container with `PRIVATE` ipc mode and 256MB /dev/shm
-  // under the first child container, check its /dev/shm size is 128MB
-  // rather than 256MB and it is in the same IPC namespace with its parent
-  // container even its ipc mode is `PRIVATE`.
+  // Launch a debug container under the first child container, check its
+  // /dev/shm size is 128MB and it is in the same IPC namespace with its
+  // parent container.
   ContainerID debugContainerId1;
   debugContainerId1.mutable_parent()->CopyFrom(childContainerId1);
   debugContainerId1.set_value(id::UUID::random().toString());
 
   containerInfo.clear_mesos();
-  containerInfo.mutable_linux_info()->set_ipc_mode(LinuxInfo::PRIVATE);
+  containerInfo.clear_linux_info();
 
   launch = containerizer.get()->launch(
       debugContainerId1,
@@ -731,16 +727,13 @@ TEST_F(NamespacesIsolatorTest, ROOT_ShareIPCNamespace)
   ASSERT_TRUE(wait.get()->has_status());
   EXPECT_WEXITSTATUS_EQ(0, wait.get()->status());
 
-  // Launch a debug container with `PRIVATE` ipc mode and 256MB /dev/shm
-  // under the second child container, check its /dev/shm size is 128MB
-  // rather than 256MB and it is in the same IPC namespace with its parent
-  // container even its ipc mode is `PRIVATE`.
+  // Launch another debug container under the second child container, check its
+  // /dev/shm size is 128MB and it is in the same IPC namespace with its parent
+  // container.
   ContainerID debugContainerId2;
   debugContainerId2.mutable_parent()->CopyFrom(childContainerId2);
   debugContainerId2.set_value(id::UUID::random().toString());
 
-  containerInfo.mutable_linux_info()->set_ipc_mode(LinuxInfo::PRIVATE);
-
   launch = containerizer.get()->launch(
       debugContainerId2,
       createContainerConfig(