Repository: mesos Updated Branches: refs/heads/master c321283fc -> cf8ab5e45
Allowed Isolator::prepare to return a list of CommandInfos. Previously, the Isolators had to join all the preparation commands into a single command using `&&`. Now they can pass each command separately. Review: https://reviews.apache.org/r/36831 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cf8ab5e4 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf8ab5e4 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf8ab5e4 Branch: refs/heads/master Commit: cf8ab5e45863b632b6c129e6e58570fa5bed6214 Parents: c321283 Author: Kapil Arya <[email protected]> Authored: Tue Jul 28 10:55:17 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Jul 28 10:55:29 2015 -0700 ---------------------------------------------------------------------- include/mesos/slave/isolator.proto | 2 +- .../isolators/filesystem/shared.cpp | 11 +++-------- .../containerizer/isolators/namespaces/pid.cpp | 13 ++++++------- .../isolators/network/port_mapping.cpp | 2 +- src/slave/containerizer/mesos/containerizer.cpp | 6 ++++-- src/tests/containerizer/containerizer_tests.cpp | 10 +++++----- src/tests/containerizer/isolator_tests.cpp | 8 ++++---- src/tests/containerizer/port_mapping_tests.cpp | 19 ++++++++++++++++++- 8 files changed, 42 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/include/mesos/slave/isolator.proto ---------------------------------------------------------------------- diff --git a/include/mesos/slave/isolator.proto b/include/mesos/slave/isolator.proto index c04a4e6..3d9222b 100644 --- a/include/mesos/slave/isolator.proto +++ b/include/mesos/slave/isolator.proto @@ -67,6 +67,6 @@ message ContainerState */ message ContainerPrepareInfo { - optional CommandInfo command = 1; + repeated CommandInfo commands = 1; optional Environment environment = 2; } http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/slave/containerizer/isolators/filesystem/shared.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/filesystem/shared.cpp b/src/slave/containerizer/isolators/filesystem/shared.cpp index 0047945..f3c2916 100644 --- a/src/slave/containerizer/isolators/filesystem/shared.cpp +++ b/src/slave/containerizer/isolators/filesystem/shared.cpp @@ -108,7 +108,7 @@ Future<Option<ContainerPrepareInfo>> SharedFilesystemIsolatorProcess::prepare( set<string> containerPaths; containerPaths.insert(directory); - list<string> commands; + ContainerPrepareInfo prepareInfo; foreach (const Volume& volume, executorInfo.container().volumes()) { // Because the filesystem is shared we require the container path @@ -213,15 +213,10 @@ Future<Option<ContainerPrepareInfo>> SharedFilesystemIsolatorProcess::prepare( } } - commands.push_back("mount -n --bind " + - hostPath + - " " + - volume.container_path()); + prepareInfo.add_commands()->set_value( + "mount -n --bind " + hostPath + " " + volume.container_path()); } - ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->set_value(strings::join(" && ", commands)); - return prepareInfo; } http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/slave/containerizer/isolators/namespaces/pid.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/namespaces/pid.cpp b/src/slave/containerizer/isolators/namespaces/pid.cpp index 14871a9..8e643f4 100644 --- a/src/slave/containerizer/isolators/namespaces/pid.cpp +++ b/src/slave/containerizer/isolators/namespaces/pid.cpp @@ -166,12 +166,12 @@ Future<Option<ContainerPrepareInfo>> NamespacesPidIsolatorProcess::prepare( const Option<string>& rootfs, const Option<string>& user) { - list<string> commands; + ContainerPrepareInfo prepareInfo; // Mask the bind mount root directory in each container so // containers cannot see the namespace bind mount of other // containers. - commands.push_back( + prepareInfo.add_commands()->set_value( "mount -n --bind " + string(PID_NS_BIND_MOUNT_MASK_DIR) + " " + string(PID_NS_BIND_MOUNT_ROOT)); @@ -184,11 +184,10 @@ Future<Option<ContainerPrepareInfo>> NamespacesPidIsolatorProcess::prepare( // taken from unshare.c in utils-linux for --mount-proc. We use the // -n flag so the mount is not added to the mtab where it will not // be correctly removed with the namespace terminates. - commands.push_back("mount none /proc --make-private -o rec"); - commands.push_back("mount -n -t proc proc /proc -o nosuid,noexec,nodev"); - - ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->set_value(strings::join(" && ", commands)); + prepareInfo.add_commands()->set_value( + "mount none /proc --make-private -o rec"); + prepareInfo.add_commands()->set_value( + "mount -n -t proc proc /proc -o nosuid,noexec,nodev"); return prepareInfo; } http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/slave/containerizer/isolators/network/port_mapping.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp index 77e649c..3f6e9df 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/isolators/network/port_mapping.cpp @@ -2136,7 +2136,7 @@ Future<Option<ContainerPrepareInfo>> PortMappingIsolatorProcess::prepare( << executorInfo.executor_id(); ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->set_value(scripts(infos[containerId])); + prepareInfo.add_commands()->set_value(scripts(infos[containerId])); return prepareInfo; } http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/slave/containerizer/mesos/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp index b8045d7..6d07ff1 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -864,8 +864,10 @@ Future<bool> MesosContainerizerProcess::_launch( JSON::Object object; JSON::Array array; foreach (const Option<ContainerPrepareInfo>& prepareInfo, prepareInfos) { - if (prepareInfo.isSome() && prepareInfo.get().has_command()) { - array.values.push_back(JSON::Protobuf(prepareInfo.get().command())); + if (prepareInfo.isSome()) { + foreach (const CommandInfo& command, prepareInfo.get().commands()) { + array.values.push_back(JSON::Protobuf(command)); + } } } object.values["commands"] = array; http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/tests/containerizer/containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/containerizer_tests.cpp b/src/tests/containerizer/containerizer_tests.cpp index 18c478a..213fa4b 100644 --- a/src/tests/containerizer/containerizer_tests.cpp +++ b/src/tests/containerizer/containerizer_tests.cpp @@ -141,7 +141,7 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, ScriptSucceeds) Fetcher fetcher; ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->set_value("touch " + file); + prepareInfo.add_commands()->set_value("touch " + file); Try<MesosContainerizer*> containerizer = CreateContainerizer( &fetcher, @@ -192,7 +192,7 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, ScriptFails) Fetcher fetcher; ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->set_value("touch " + file + " && exit 1"); + prepareInfo.add_commands()->set_value("touch " + file + " && exit 1"); Try<MesosContainerizer*> containerizer = CreateContainerizer( &fetcher, @@ -248,12 +248,12 @@ TEST_F(MesosContainerizerIsolatorPreparationTest, MultipleScripts) // This isolator prepare command one will succeed if called first, otherwise // it won't get run. ContainerPrepareInfo prepare1; - prepare1.mutable_command()->set_value("touch " + file1 + " && exit 0"); + prepare1.add_commands()->set_value("touch " + file1 + " && exit 0"); prepares.push_back(prepare1); // This will fail, either first or after the successful command. ContainerPrepareInfo prepare2; - prepare2.mutable_command()->set_value("touch " + file2 + " && exit 1"); + prepare2.add_commands()->set_value("touch " + file2 + " && exit 1"); prepares.push_back(prepare2); Fetcher fetcher; @@ -647,7 +647,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhilePreparing) // Need to help the compiler to disambiguate between overloads. ContainerPrepareInfo prepareInfo; - prepareInfo.mutable_command()->CopyFrom(commandInfo); + prepareInfo.add_commands()->CopyFrom(commandInfo); Option<ContainerPrepareInfo> option = prepareInfo; promise.set(option); http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/tests/containerizer/isolator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp index 51a4db1..ff6e2b7 100644 --- a/src/tests/containerizer/isolator_tests.cpp +++ b/src/tests/containerizer/isolator_tests.cpp @@ -961,7 +961,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume) AWAIT_READY(prepare); ASSERT_SOME(prepare.get()); - ASSERT_TRUE(prepare.get().get().has_command()); + ASSERT_EQ(1, prepare.get().get().commands().size()); // The test will touch a file in container path. const string file = path::join(containerPath, UUID::random().toString()); @@ -973,7 +973,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume) args.push_back("/bin/sh"); args.push_back("-x"); args.push_back("-c"); - args.push_back(prepare.get().get().command().value() + " && touch " + file); + args.push_back(prepare.get().get().commands(0).value() + " && touch " + file); Try<pid_t> pid = launcher.get()->fork( containerId, @@ -1066,7 +1066,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume) AWAIT_READY(prepare); ASSERT_SOME(prepare.get()); - ASSERT_TRUE(prepare.get().get().has_command()); + ASSERT_EQ(1, prepare.get().get().commands().size()); // Test the volume mounting by touching a file in the container's // /tmp, which should then be in flags.work_dir. @@ -1077,7 +1077,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume) args.push_back("/bin/sh"); args.push_back("-x"); args.push_back("-c"); - args.push_back(prepare.get().get().command().value() + + args.push_back(prepare.get().get().commands(0).value() + " && touch " + path::join(containerPath, filename)); http://git-wip-us.apache.org/repos/asf/mesos/blob/cf8ab5e4/src/tests/containerizer/port_mapping_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp index 778dc8c..4bee74a 100644 --- a/src/tests/containerizer/port_mapping_tests.cpp +++ b/src/tests/containerizer/port_mapping_tests.cpp @@ -317,9 +317,13 @@ protected: launchFlags.pipe_read = pipes[0]; launchFlags.pipe_write = pipes[1]; + if (preparation.get().commands().size() != 1) { + return Error("No valid commands inside ContainerPrepareInfo."); + } + JSON::Object commands; JSON::Array array; - array.values.push_back(JSON::Protobuf(preparation.get().command())); + array.values.push_back(JSON::Protobuf(preparation.get().commands(0))); commands.values["commands"] = array; launchFlags.commands = commands; @@ -462,6 +466,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerTCP) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; @@ -529,6 +534,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerTCP) AWAIT_READY(preparation2); ASSERT_SOME(preparation2.get()); + ASSERT_EQ(1, preparation2.get().get().commands().size()); ostringstream command2; @@ -622,6 +628,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerUDP) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; @@ -689,6 +696,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_ContainerToContainerUDP) AWAIT_READY(preparation2); ASSERT_SOME(preparation2.get()); + ASSERT_EQ(1, preparation2.get().get().commands().size()); ostringstream command2; @@ -784,6 +792,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_HostToContainerUDP) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; @@ -901,6 +910,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_HostToContainerTCP) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; @@ -1026,6 +1036,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerICMPExternal) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; for (unsigned int i = 0; i < nameServers.size(); i++) { @@ -1112,6 +1123,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerICMPInternal) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; command1 << "ping -c1 127.0.0.1 && ping -c1 " << hostIP @@ -1201,6 +1213,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_ContainerARPExternal) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; for (unsigned int i = 0; i < nameServers.size(); i++) { @@ -1296,6 +1309,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_DNS) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; for (unsigned int i = 0; i < nameServers.size(); i++) { @@ -1387,6 +1401,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_TooManyContainers) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); ostringstream command1; command1 << "sleep 1000"; @@ -1504,6 +1519,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_SmallEgressLimit) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); // Fill 'size' bytes of data. The actual content does not matter. string data(size.bytes(), 'a'); @@ -1658,6 +1674,7 @@ TEST_F(PortMappingIsolatorTest, ROOT_NC_PortMappingStatistics) AWAIT_READY(preparation1); ASSERT_SOME(preparation1.get()); + ASSERT_EQ(1, preparation1.get().get().commands().size()); // Fill 'size' bytes of data. The actual content does not matter. string data(size.bytes(), 'a');
