You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2018/06/28 18:22:29 UTC
[15/16] mesos git commit: Windows: Wrapped docker executor code in
job object.
Windows: Wrapped docker executor code in job object.
One of the docker tests revealed that the docker executor code could
leak some of the docker processes if it gets killed. To avoid this, we
run the docker executor and all processes launched by it in a job
object, so that `os::killtree` will properly kill the process tree.
Review: https://reviews.apache.org/r/67456/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1f4ead9c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1f4ead9c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1f4ead9c
Branch: refs/heads/master
Commit: 1f4ead9c1bba859ad1498d737a176b359268575d
Parents: 11f8b6d
Author: Akash Gupta <ak...@hotmail.com>
Authored: Wed Jun 27 14:30:19 2018 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Jun 27 15:06:10 2018 -0700
----------------------------------------------------------------------
src/docker/docker.cpp | 65 ++++++++++++++++++++++++++++-----
src/docker/executor.cpp | 32 ++++++++++++++++
src/slave/containerizer/docker.cpp | 15 ++++++++
3 files changed, 103 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f4ead9c/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index c6db244..baac70f 100644
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -34,10 +34,18 @@
#include <stout/os/read.hpp>
#include <stout/os/write.hpp>
+#ifdef __WINDOWS__
+#include <stout/os/windows/jobobject.hpp>
+#endif // __WINDOWS__
+
#include <process/check.hpp>
#include <process/collect.hpp>
#include <process/io.hpp>
+#ifdef __WINDOWS__
+#include <process/windows/jobobject.hpp>
+#endif // __WINDOWS__
+
#include "common/status_utils.hpp"
#include "docker/docker.hpp"
@@ -159,6 +167,19 @@ void commandDiscarded(const Subprocess& s, const string& cmd)
}
+vector<Subprocess::ParentHook> createParentHooks()
+{
+ return {
+#ifdef __WINDOWS__
+ // To correctly discard the docker cli process tree in `commandDiscarded`,
+ // we need to wrap the process in a job object.
+ Subprocess::ParentHook::CREATE_JOB(),
+ Subprocess::ParentHook(&os::set_job_kill_on_close_limit),
+#endif // __WINDOWS__
+ };
+}
+
+
Future<Version> Docker::version() const
{
string cmd = path + " -H " + socket + " --version";
@@ -167,7 +188,10 @@ Future<Version> Docker::version() const
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1143,7 +1167,10 @@ Future<Option<int>> Docker::run(
Subprocess::PATH(os::DEV_NULL),
_stdout,
_stderr,
- nullptr);
+ nullptr,
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + path + "': " + s.error());
@@ -1186,7 +1213,10 @@ Future<Nothing> Docker::stop(
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1241,7 +1271,10 @@ Future<Nothing> Docker::kill(
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1266,7 +1299,10 @@ Future<Nothing> Docker::rm(
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1314,7 +1350,10 @@ void Docker::_inspect(
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
promise->fail("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1451,7 +1490,10 @@ Future<vector<Docker::Container>> Docker::ps(
cmd,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
- Subprocess::PIPE());
+ Subprocess::PIPE(),
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1622,7 +1664,10 @@ Future<Docker::Image> Docker::pull(
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
Subprocess::PIPE(),
- nullptr);
+ nullptr,
+ None(),
+ None(),
+ createParentHooks());
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1769,7 +1814,9 @@ Future<Docker::Image> Docker::__pull(
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
nullptr,
- environment);
+ environment,
+ None(),
+ createParentHooks());
if (s_.isError()) {
return Failure("Failed to execute '" + cmd + "': " + s_.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f4ead9c/src/docker/executor.cpp
----------------------------------------------------------------------
diff --git a/src/docker/executor.cpp b/src/docker/executor.cpp
index 572594a..d9f5a73 100644
--- a/src/docker/executor.cpp
+++ b/src/docker/executor.cpp
@@ -39,9 +39,16 @@
#include <stout/os.hpp>
#include <stout/protobuf.hpp>
#include <stout/try.hpp>
+#ifdef __WINDOWS__
+#include <stout/windows.hpp>
+#endif // __WINDOWS__
#include <stout/os/killtree.hpp>
+#ifdef __WINDOWS__
+#include <stout/os/windows/jobobject.hpp>
+#endif // __WINDOWS__
+
#include "checks/checks_runtime.hpp"
#include "checks/health_checker.hpp"
@@ -968,6 +975,31 @@ int main(int argc, char** argv)
{
GOOGLE_PROTOBUF_VERIFY_VERSION;
+#ifdef __WINDOWS__
+ // We need a handle to the job object which this container is associated with.
+ // Without this handle, the job object would be destroyed by the OS when the
+ // agent exits (or crashes), making recovery impossible. By holding a handle,
+ // we tie the lifetime of the job object to the container itself. In this way,
+ // a recovering agent can reattach to the container by opening a new handle to
+ // the job object.
+ const pid_t pid = ::GetCurrentProcessId();
+ const Try<std::wstring> name = os::name_job(pid);
+ if (name.isError()) {
+ cerr << "Failed to create job object name from pid: " << name.error()
+ << endl;
+ return EXIT_FAILURE;
+ }
+
+ // NOTE: This handle will not be destructed, even though it is a
+ // `SharedHandle`, because it will (purposefully) never go out of scope.
+ Try<SharedHandle> handle = os::open_job(JOB_OBJECT_QUERY, false, name.get());
+ if (handle.isError()) {
+ cerr << "Failed to open job object '" << stringify(name.get())
+ << "' for the current container: " << handle.error() << endl;
+ return EXIT_FAILURE;
+ }
+#endif // __WINDOWS__
+
mesos::internal::docker::Flags flags;
// Load flags from environment and command line.
http://git-wip-us.apache.org/repos/asf/mesos/blob/1f4ead9c/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 7737107..277a155 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -33,6 +33,10 @@
#include <process/reap.hpp>
#include <process/subprocess.hpp>
+#ifdef __WINDOWS__
+#include <process/windows/jobobject.hpp>
+#endif // __WINDOWS__
+
#include <stout/adaptor.hpp>
#include <stout/fs.hpp>
#include <stout/hashmap.hpp>
@@ -46,6 +50,10 @@
#include <stout/os/killtree.hpp>
#include <stout/os/which.hpp>
+#ifdef __WINDOWS__
+#include <stout/os/windows/jobobject.hpp>
+#endif // __WINDOWS__
+
#include "common/status_utils.hpp"
#include "hook/manager.hpp"
@@ -1588,6 +1596,13 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
parentHooks.emplace_back(Subprocess::ParentHook(
&systemd::mesos::extendLifetime));
}
+#elif __WINDOWS__
+ parentHooks.emplace_back(Subprocess::ParentHook::CREATE_JOB());
+ // Setting the "kill on close" job object limit ties the lifetime of the
+ // docker processes to that of the executor. This ensures that if the
+ // executor exits, the docker processes aren't leaked.
+ parentHooks.emplace_back(Subprocess::ParentHook(
+ [](pid_t pid) { return os::set_job_kill_on_close_limit(pid); }));
#endif // __linux__
// Prepare the flags to pass to the mesos docker executor process.