Windows: Combined Posix/Windows-Launcher into SubprocessLauncher. This commit renames the `PosixLauncher` into the SubprocessLauncher` and deletes the trivially derived class `WindowsLauncher`. With the improved job object support in stout/libprocess, the same launcher is now suitable for both POSIX systems and Windows. Thus, the previous name became a misnomer (PosixLauncher).
Review: https://reviews.apache.org/r/57974/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cb236056 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cb236056 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cb236056 Branch: refs/heads/master Commit: cb23605674027cd4c7ee0a877b8af31ca01c85e9 Parents: c94041b Author: Andrew Schwartzmeyer <[email protected]> Authored: Mon Apr 3 13:38:17 2017 -0700 Committer: Joseph Wu <[email protected]> Committed: Tue Apr 4 16:45:16 2017 -0700 ---------------------------------------------------------------------- src/slave/containerizer/mesos/containerizer.cpp | 10 +++--- .../containerizer/mesos/isolators/posix.hpp | 2 +- src/slave/containerizer/mesos/launcher.cpp | 33 ++++++++++---------- src/slave/containerizer/mesos/launcher.hpp | 21 +++---------- src/tests/container_logger_tests.cpp | 2 +- .../containerizer/mesos_containerizer_tests.cpp | 22 ++++++------- 6 files changed, 40 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/slave/containerizer/mesos/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp index 527c96d..bc611a5 100644 --- a/src/slave/containerizer/mesos/containerizer.cpp +++ b/src/slave/containerizer/mesos/containerizer.cpp @@ -226,7 +226,7 @@ Try<MesosContainerizer*> MesosContainerizer::create( if (flags_.launcher == "linux") { return LinuxLauncher::create(flags_); } else if (flags_.launcher == "posix") { - return PosixLauncher::create(flags_); + return SubprocessLauncher::create(flags_); } else { return Error("Unknown or unsupported launcher: " + flags_.launcher); } @@ -235,13 +235,13 @@ Try<MesosContainerizer*> MesosContainerizer::create( return Error("Unsupported launcher: " + flags_.launcher); } - return WindowsLauncher::create(flags_); + return SubprocessLauncher::create(flags_); #else if (flags_.launcher != "posix") { return Error("Unsupported launcher: " + flags_.launcher); } - return PosixLauncher::create(flags_); + return SubprocessLauncher::create(flags_); #endif // __linux__ }(); @@ -1574,9 +1574,9 @@ Future<bool> MesosContainerizerProcess::_launch( containerIO->err, nullptr, launchEnvironment, - // 'enterNamespaces' will be ignored by PosixLauncher. + // 'enterNamespaces' will be ignored by SubprocessLauncher. _enterNamespaces, - // 'cloneNamespaces' will be ignored by PosixLauncher. + // 'cloneNamespaces' will be ignored by SubprocessLauncher. _cloneNamespaces); if (forked.isError()) { http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/slave/containerizer/mesos/isolators/posix.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/isolators/posix.hpp b/src/slave/containerizer/mesos/isolators/posix.hpp index 6270046..caa282c 100644 --- a/src/slave/containerizer/mesos/isolators/posix.hpp +++ b/src/slave/containerizer/mesos/isolators/posix.hpp @@ -47,7 +47,7 @@ public: { foreach (const mesos::slave::ContainerState& run, state) { // This should (almost) never occur: see comment in - // PosixLauncher::recover(). + // SubprocessLauncher::recover(). if (pids.contains(run.container_id())) { return process::Failure("Container already recovered"); } http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/slave/containerizer/mesos/launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp index 5114c13..ec31fa2 100644 --- a/src/slave/containerizer/mesos/launcher.cpp +++ b/src/slave/containerizer/mesos/launcher.cpp @@ -20,6 +20,9 @@ #include <process/delay.hpp> #include <process/process.hpp> #include <process/reap.hpp> +#ifdef __WINDOWS__ +#include <process/windows/jobobject.hpp> +#endif // __WINDOWS__ #include <stout/unreachable.hpp> @@ -46,13 +49,13 @@ namespace mesos { namespace internal { namespace slave { -Try<Launcher*> PosixLauncher::create(const Flags& flags) +Try<Launcher*> SubprocessLauncher::create(const Flags& flags) { - return new PosixLauncher(); + return new SubprocessLauncher(); } -Future<hashset<ContainerID>> PosixLauncher::recover( +Future<hashset<ContainerID>> SubprocessLauncher::recover( const list<ContainerState>& states) { foreach (const ContainerState& state, states) { @@ -78,7 +81,7 @@ Future<hashset<ContainerID>> PosixLauncher::recover( } -Try<pid_t> PosixLauncher::fork( +Try<pid_t> SubprocessLauncher::fork( const ContainerID& containerId, const string& path, const vector<string>& argv, @@ -91,11 +94,11 @@ Try<pid_t> PosixLauncher::fork( const Option<int>& cloneNamespaces) { if (enterNamespaces.isSome() && enterNamespaces.get() != 0) { - return Error("Posix launcher does not support entering namespaces"); + return Error("Subprocess launcher does not support entering namespaces"); } if (cloneNamespaces.isSome() && cloneNamespaces.get() != 0) { - return Error("Posix launcher does not support cloning namespaces"); + return Error("Subprocess launcher does not support cloning namespaces"); } if (pids.contains(containerId)) { @@ -103,16 +106,18 @@ Try<pid_t> PosixLauncher::fork( stringify(containerId)); } - // If we are on systemd, then extend the life of the child. Any - // grandchildren's lives will also be extended. vector<process::Subprocess::ParentHook> parentHooks; #ifdef __linux__ + // If we are on systemd, then extend the life of the child. Any + // grandchildren's lives will also be extended. if (systemd::enabled()) { parentHooks.emplace_back(Subprocess::ParentHook( &systemd::mesos::extendLifetime)); } #elif defined(__WINDOWS__) + // If we are on Windows, then ensure the child is placed inside a + // new job object. parentHooks.emplace_back(Subprocess::ParentHook::CREATE_JOB()); #endif // __linux__ @@ -146,7 +151,7 @@ Try<pid_t> PosixLauncher::fork( Future<Nothing> _destroy(const Future<Option<int>>& future); -Future<Nothing> PosixLauncher::destroy(const ContainerID& containerId) +Future<Nothing> SubprocessLauncher::destroy(const ContainerID& containerId) { LOG(INFO) << "Asked to destroy container " << containerId; @@ -158,7 +163,7 @@ Future<Nothing> PosixLauncher::destroy(const ContainerID& containerId) pid_t pid = pids.get(containerId).get(); // Kill all processes in the session and process group. - Try<list<os::ProcessTree>> trees = os::killtree(pid, SIGKILL, true, true); + os::killtree(pid, SIGKILL, true, true); pids.erase(containerId); @@ -180,7 +185,8 @@ Future<Nothing> _destroy(const Future<Option<int>>& future) } -Future<ContainerStatus> PosixLauncher::status(const ContainerID& containerId) +Future<ContainerStatus> SubprocessLauncher::status( + const ContainerID& containerId) { if (!pids.contains(containerId)) { return Failure("Container does not exist!"); @@ -193,11 +199,6 @@ Future<ContainerStatus> PosixLauncher::status(const ContainerID& containerId) } -Try<Launcher*> WindowsLauncher::create(const Flags& flags) -{ - return new WindowsLauncher(); -} - } // namespace slave { } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/slave/containerizer/mesos/launcher.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/launcher.hpp b/src/slave/containerizer/mesos/launcher.hpp index 79f6eea..f69d934 100644 --- a/src/slave/containerizer/mesos/launcher.hpp +++ b/src/slave/containerizer/mesos/launcher.hpp @@ -86,12 +86,14 @@ public: // groups and sessions to track processes in a container. POSIX states // that process groups cannot migrate between sessions so all // processes for a container will be contained in a session. -class PosixLauncher : public Launcher +// Also suitable for Windows, which uses job objects to obtain the +// same functionality. Everything is coordinated through `Subprocess`. +class SubprocessLauncher : public Launcher { public: static Try<Launcher*> create(const Flags& flags); - virtual ~PosixLauncher() {} + virtual ~SubprocessLauncher() {} virtual process::Future<hashset<ContainerID>> recover( const std::list<mesos::slave::ContainerState>& states); @@ -114,7 +116,7 @@ public: const ContainerID& containerId); protected: - PosixLauncher() {} + SubprocessLauncher() {} // The 'pid' is the process id of the first process and also the // process group id and session id. @@ -122,19 +124,6 @@ protected: }; -// Minimal implementation of a `Launcher` for the Windows platform. Does not -// take into account process groups (jobs) or sessions. -class WindowsLauncher : public PosixLauncher -{ -public: - static Try<Launcher*> create(const Flags& flags); - - virtual ~WindowsLauncher() {} - -private: - WindowsLauncher() {} -}; - } // namespace slave { } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/tests/container_logger_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp index 4ccb2e4..28436b6 100644 --- a/src/tests/container_logger_tests.cpp +++ b/src/tests/container_logger_tests.cpp @@ -71,7 +71,7 @@ using mesos::internal::master::Master; using mesos::internal::slave::Fetcher; using mesos::internal::slave::Launcher; using mesos::internal::slave::MesosContainerizer; -using mesos::internal::slave::PosixLauncher; +using mesos::internal::slave::SubprocessLauncher; using mesos::internal::slave::Provisioner; using mesos::internal::slave::Slave; http://git-wip-us.apache.org/repos/asf/mesos/blob/cb236056/src/tests/containerizer/mesos_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp b/src/tests/containerizer/mesos_containerizer_tests.cpp index 9a5cfe4..13e0f7e 100644 --- a/src/tests/containerizer/mesos_containerizer_tests.cpp +++ b/src/tests/containerizer/mesos_containerizer_tests.cpp @@ -64,7 +64,7 @@ using mesos::internal::slave::Launcher; using mesos::internal::slave::MesosContainerizer; using mesos::internal::slave::MesosContainerizerProcess; using mesos::internal::slave::MESOS_CONTAINERIZER; -using mesos::internal::slave::PosixLauncher; +using mesos::internal::slave::SubprocessLauncher; using mesos::internal::slave::Provisioner; using mesos::internal::slave::ProvisionInfo; using mesos::internal::slave::Slave; @@ -281,7 +281,7 @@ public: slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); if (launcher_.isError()) { return Error(launcher_.error()); } @@ -715,7 +715,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhileFetching) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); Owned<Launcher> launcher(launcher_.get()); @@ -784,7 +784,7 @@ TEST_F(MesosContainerizerDestroyTest, DestroyWhilePreparing) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); Owned<Launcher> launcher(launcher_.get()); @@ -908,7 +908,7 @@ TEST_F(MesosContainerizerProvisionerTest, ProvisionFailed) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); Owned<Launcher> launcher(new TestLauncher(Owned<Launcher>(launcher_.get()))); @@ -996,7 +996,7 @@ TEST_F(MesosContainerizerProvisionerTest, DestroyWhileProvisioning) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); Owned<Launcher> launcher(new TestLauncher(Owned<Launcher>(launcher_.get()))); @@ -1086,7 +1086,7 @@ TEST_F(MesosContainerizerProvisionerTest, IsolatorCleanupBeforePrepare) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); Owned<Launcher> launcher(new TestLauncher(Owned<Launcher>(launcher_.get()))); @@ -1186,11 +1186,11 @@ ACTION_P(InvokeDestroyAndWait, launcher) // 'container_destroy_errors' metric is updated. TEST_F(MesosContainerizerDestroyTest, LauncherDestroyFailure) { - // Create a TestLauncher backed by PosixLauncher. + // Create a TestLauncher backed by SubprocessLauncher. slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher_ = PosixLauncher::create(flags); + Try<Launcher*> launcher_ = SubprocessLauncher::create(flags); ASSERT_SOME(launcher_); TestLauncher* testLauncher = @@ -1220,7 +1220,7 @@ TEST_F(MesosContainerizerDestroyTest, LauncherDestroyFailure) CommandInfo commandInfo; taskInfo.mutable_command()->MergeFrom(commandInfo); - // Destroy the container using the PosixLauncher but return a failed + // Destroy the container using the SubprocessLauncher but return a failed // future to the containerizer. EXPECT_CALL(*testLauncher, destroy(_)) .WillOnce(DoAll(InvokeDestroyAndWait(testLauncher), @@ -1326,7 +1326,7 @@ TEST_F(MesosLauncherStatusTest, ExecutorPIDTest) slave::Flags flags = CreateSlaveFlags(); flags.launcher = "posix"; - Try<Launcher*> launcher = PosixLauncher::create(flags); + Try<Launcher*> launcher = SubprocessLauncher::create(flags); ASSERT_SOME(launcher); ContainerID containerId;
