Repository: mesos Updated Branches: refs/heads/master 0e4019fff -> 4b12df29c
Refactored container Launcher to accept namespaces dynamically. This prepares MesosContainerizer to allow isolators to specify namespaces for each individual containerizer. Thus, allowing isolators to decide whether or not to enable isolation for a particular container. Review: https://reviews.apache.org/r/38363 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e047f7d6 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e047f7d6 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e047f7d6 Branch: refs/heads/master Commit: e047f7d69b5297cc787487b6093119a3be517e48 Parents: 0e4019f Author: Kapil Arya <[email protected]> Authored: Wed Sep 16 17:00:39 2015 -0700 Committer: Niklas Q. Nielsen <[email protected]> Committed: Wed Sep 16 17:00:40 2015 -0700 ---------------------------------------------------------------------- src/slave/containerizer/launcher.cpp | 3 +- src/slave/containerizer/launcher.hpp | 6 ++-- src/slave/containerizer/linux_launcher.cpp | 21 +++++------ src/slave/containerizer/linux_launcher.hpp | 9 ++--- src/slave/containerizer/mesos/containerizer.cpp | 26 ++++++++------ .../containerizer/filesystem_isolator_tests.cpp | 3 +- src/tests/containerizer/isolator_tests.cpp | 37 +++++++++++--------- src/tests/containerizer/launcher.hpp | 11 +++--- 8 files changed, 62 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/launcher.cpp b/src/slave/containerizer/launcher.cpp index 5267eec..6649d35 100644 --- a/src/slave/containerizer/launcher.cpp +++ b/src/slave/containerizer/launcher.cpp @@ -107,7 +107,8 @@ Try<pid_t> PosixLauncher::fork( const Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<map<string, string>>& environment, - const Option<lambda::function<int()>>& setup) + const Option<lambda::function<int()>>& setup, + const Option<int>& namespaces) { if (pids.contains(containerId)) { return Error("Process has already been forked for container " + http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/launcher.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/launcher.hpp b/src/slave/containerizer/launcher.hpp index 8cc832e..8945776 100644 --- a/src/slave/containerizer/launcher.hpp +++ b/src/slave/containerizer/launcher.hpp @@ -72,7 +72,8 @@ public: const process::Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string>>& environment, - const Option<lambda::function<int()>>& setup) = 0; + const Option<lambda::function<int()>>& setup, + const Option<int>& namespaces) = 0; // Kill all processes in the containerized context. virtual process::Future<Nothing> destroy(const ContainerID& containerId) = 0; @@ -102,7 +103,8 @@ public: const process::Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string>>& environment, - const Option<lambda::function<int()>>& setup); + const Option<lambda::function<int()>>& setup, + const Option<int>& namespaces); virtual process::Future<Nothing> destroy(const ContainerID& containerId); http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/linux_launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/linux_launcher.cpp b/src/slave/containerizer/linux_launcher.cpp index 12acf4d..dde552f 100644 --- a/src/slave/containerizer/linux_launcher.cpp +++ b/src/slave/containerizer/linux_launcher.cpp @@ -67,16 +67,12 @@ static ContainerID container(const string& cgroup) LinuxLauncher::LinuxLauncher( const Flags& _flags, - int _namespaces, const string& _hierarchy) : flags(_flags), - namespaces(_namespaces), hierarchy(_hierarchy) {} -Try<Launcher*> LinuxLauncher::create( - const Flags& flags, - const Option<int>& namespaces) +Try<Launcher*> LinuxLauncher::create(const Flags& flags) { Try<string> hierarchy = cgroups::prepare( flags.cgroups_hierarchy, @@ -104,7 +100,6 @@ Try<Launcher*> LinuxLauncher::create( return new LinuxLauncher( flags, - namespaces.isSome() ? namespaces.get() : 0, hierarchy.get()); } @@ -178,7 +173,9 @@ static int childMain(void* _func) // The customized clone function which will be used by 'subprocess()'. -static pid_t clone(const lambda::function<int()>& func, int namespaces) +static pid_t clone( + const lambda::function<int()>& func, + const Option<int>& namespaces) { // Stack for the child. // - unsigned long long used for best alignment. @@ -186,13 +183,16 @@ static pid_t clone(const lambda::function<int()>& func, int namespaces) // - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux. static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)]; + int flags = namespaces.isSome() ? namespaces.get() : 0; + flags |= SIGCHLD; // Specify SIGCHLD as child termination signal. + LOG(INFO) << "Cloning child process with flags = " - << ns::stringify(namespaces); + << ns::stringify(flags); return ::clone( childMain, &stack[sizeof(stack)/sizeof(stack[0]) - 1], // stack grows down. - namespaces | SIGCHLD, // Specify SIGCHLD as child termination signal. + flags, (void*) &func); } @@ -246,7 +246,8 @@ Try<pid_t> LinuxLauncher::fork( const process::Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<map<string, string>>& environment, - const Option<lambda::function<int()>>& setup) + const Option<lambda::function<int()>>& setup, + const Option<int>& namespaces) { // Create a freezer cgroup for this container if necessary. Try<bool> exists = cgroups::exists(hierarchy, cgroup(containerId)); http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/linux_launcher.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/linux_launcher.hpp b/src/slave/containerizer/linux_launcher.hpp index bf6bf3f..f6112c9 100644 --- a/src/slave/containerizer/linux_launcher.hpp +++ b/src/slave/containerizer/linux_launcher.hpp @@ -30,9 +30,7 @@ namespace slave { class LinuxLauncher : public Launcher { public: - static Try<Launcher*> create( - const Flags& flags, - const Option<int>& namespaces); + static Try<Launcher*> create(const Flags& flags); virtual ~LinuxLauncher() {} @@ -48,19 +46,18 @@ public: const process::Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string>>& environment, - const Option<lambda::function<int()>>& setup); + const Option<lambda::function<int()>>& setup, + const Option<int>& namespaces); virtual process::Future<Nothing> destroy(const ContainerID& containerId); private: LinuxLauncher( const Flags& flags, - int namespaces, const std::string& hierarchy); static const std::string subsystem; const Flags flags; - const int namespaces; const std::string hierarchy; std::string cgroup(const ContainerID& containerId); http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/mesos/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp index 0023f1d..71f16cc 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -233,17 +233,11 @@ Try<MesosContainerizer*> MesosContainerizer::create( } #ifdef __linux__ - int namespaces = 0; - foreach (const Owned<Isolator>& isolator, isolators) { - if (isolator->namespaces().get().isSome()) { - namespaces |= isolator->namespaces().get().get(); - } - } - - // Determine which launcher to use based on the isolation flag. + // Always use LinuxLauncher if running as root since we will be picking + // namespaces during the fork() call. Try<Launcher*> launcher = - (strings::contains(isolation, "cgroups") || namespaces != 0) - ? LinuxLauncher::create(flags_, namespaces) + (geteuid() == 0) + ? LinuxLauncher::create(flags_) : PosixLauncher::create(flags_); #else Try<Launcher*> launcher = PosixLauncher::create(flags_); @@ -836,6 +830,15 @@ Future<bool> MesosContainerizerProcess::_launch( launchFlags.commands = object; + // TODO(karya): Create ContainerPrepareInfo.namespaces and use that instead of + // Isolator::namespaces(). + int namespaces = 0; + foreach (const Owned<Isolator>& isolator, isolators) { + if (isolator->namespaces().get().isSome()) { + namespaces |= isolator->namespaces().get().get(); + } + } + // Fork the child using launcher. vector<string> argv(2); argv[0] = MESOS_CONTAINERIZER; @@ -852,7 +855,8 @@ Future<bool> MesosContainerizerProcess::_launch( : Subprocess::PATH(path::join(directory, "stderr"))), launchFlags, environment, - None()); + None(), + namespaces); // 'namespaces' will be ignored by PosixLauncher. if (forked.isError()) { return Failure("Failed to fork executor: " + forked.error()); http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/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 4dbfeac..ca9f423 100644 --- a/src/tests/containerizer/filesystem_isolator_tests.cpp +++ b/src/tests/containerizer/filesystem_isolator_tests.cpp @@ -105,8 +105,7 @@ public: Owned<Isolator> isolator(_isolator.get()); - Try<Launcher*> _launcher = - LinuxLauncher::create(flags, isolator->namespaces().get().get()); + Try<Launcher*> _launcher = LinuxLauncher::create(flags); if (_launcher.isError()) { return Error("Failed to create LinuxLauncher: " + _launcher.error()); http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/tests/containerizer/isolator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp index d34c82a..1d0d41b 100644 --- a/src/tests/containerizer/isolator_tests.cpp +++ b/src/tests/containerizer/isolator_tests.cpp @@ -198,7 +198,8 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage) Subprocess::FD(STDERR_FILENO), None(), None(), - lambda::bind(&childSetup, pipes)); + lambda::bind(&childSetup, pipes), + None()); ASSERT_SOME(pid); @@ -308,7 +309,8 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage) Subprocess::FD(STDERR_FILENO), None(), None(), - lambda::bind(&childSetup, pipes)); + lambda::bind(&childSetup, pipes), + None()); ASSERT_SOME(pid); @@ -405,6 +407,7 @@ TEST_F(RevocableCpuIsolatorTest, ROOT_CGROUPS_RevocableCpu) Subprocess::PATH("/dev/null"), None(), None(), + None(), None()); ASSERT_SOME(pid); @@ -451,8 +454,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs) Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags); CHECK_SOME(isolator); - Try<Launcher*> launcher = - LinuxLauncher::create(flags, isolator.get()->namespaces().get()); + Try<Launcher*> launcher = LinuxLauncher::create(flags); CHECK_SOME(launcher); // Set the executor's resources to 0.5 cpu. @@ -500,7 +502,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs) Subprocess::FD(STDERR_FILENO), None(), None(), - lambda::bind(&childSetup, pipes)); + lambda::bind(&childSetup, pipes), + isolator.get()->namespaces().get()); ASSERT_SOME(pid); @@ -562,8 +565,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota) Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags); CHECK_SOME(isolator); - Try<Launcher*> launcher = - LinuxLauncher::create(flags, isolator.get()->namespaces().get()); + Try<Launcher*> launcher = LinuxLauncher::create(flags); CHECK_SOME(launcher); // Set the executor's resources to 100.5 cpu. @@ -602,7 +604,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota) Subprocess::FD(STDERR_FILENO), None(), None(), - lambda::bind(&childSetup, pipes)); + lambda::bind(&childSetup, pipes), + isolator.get()->namespaces().get()); ASSERT_SOME(pid); @@ -646,8 +649,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids) Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags); CHECK_SOME(isolator); - Try<Launcher*> launcher = - LinuxLauncher::create(flags, isolator.get()->namespaces().get()); + Try<Launcher*> launcher = LinuxLauncher::create(flags); CHECK_SOME(launcher); ExecutorInfo executorInfo; @@ -692,7 +694,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids) Subprocess::FD(STDERR_FILENO), None(), None(), - lambda::bind(&childSetup, pipes)); + lambda::bind(&childSetup, pipes), + isolator.get()->namespaces().get()); ASSERT_SOME(pid); @@ -921,8 +924,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume) Try<Isolator*> isolator = SharedFilesystemIsolatorProcess::create(flags); CHECK_SOME(isolator); - Try<Launcher*> launcher = - LinuxLauncher::create(flags, isolator.get()->namespaces().get()); + Try<Launcher*> launcher = LinuxLauncher::create(flags); CHECK_SOME(launcher); // Use /var/tmp so we don't mask the work directory (under /tmp). @@ -975,7 +977,8 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume) Subprocess::FD(STDERR_FILENO), None(), None(), - None()); + None(), + isolator.get()->namespaces().get()); ASSERT_SOME(pid); // Set up the reaper to wait on the forked child. @@ -1027,8 +1030,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume) Try<Isolator*> isolator = SharedFilesystemIsolatorProcess::create(flags); CHECK_SOME(isolator); - Try<Launcher*> launcher = - LinuxLauncher::create(flags, isolator.get()->namespaces().get()); + Try<Launcher*> launcher = LinuxLauncher::create(flags); CHECK_SOME(launcher); // We'll mount the absolute test work directory as /var/tmp in the @@ -1080,7 +1082,8 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume) Subprocess::FD(STDERR_FILENO), None(), None(), - None()); + None(), + isolator.get()->namespaces().get()); ASSERT_SOME(pid); // Set up the reaper to wait on the forked child. http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/tests/containerizer/launcher.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/launcher.hpp b/src/tests/containerizer/launcher.hpp index 6f020f9..5d34bab 100644 --- a/src/tests/containerizer/launcher.hpp +++ b/src/tests/containerizer/launcher.hpp @@ -55,7 +55,7 @@ ACTION_P(InvokeRecover, launcher) ACTION_P(InvokeFork, launcher) { return launcher->real->fork( - arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8); + arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9); } @@ -79,9 +79,9 @@ public: EXPECT_CALL(*this, recover(_)) .WillRepeatedly(DoDefault()); - ON_CALL(*this, fork(_, _, _, _, _, _, _, _, _)) + ON_CALL(*this, fork(_, _, _, _, _, _, _, _, _, _)) .WillByDefault(InvokeFork(this)); - EXPECT_CALL(*this, fork(_, _, _, _, _, _, _, _, _)) + EXPECT_CALL(*this, fork(_, _, _, _, _, _, _, _, _, _)) .WillRepeatedly(DoDefault()); ON_CALL(*this, destroy(_)) @@ -97,7 +97,7 @@ public: process::Future<hashset<ContainerID>>( const std::list<mesos::slave::ContainerState>& states)); - MOCK_METHOD9( + MOCK_METHOD10( fork, Try<pid_t>( const ContainerID& containerId, @@ -108,7 +108,8 @@ public: const process::Subprocess::IO& err, const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string> >& env, - const Option<lambda::function<int()> >& setup)); + const Option<lambda::function<int()> >& setup, + const Option<int>& namespaces)); MOCK_METHOD1( destroy,
