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 <[email protected]> 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(
