You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2018/10/05 00:07:16 UTC
[mesos] branch master updated: Updated Docker library to avoid
'os::killtree()' when discarding.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new 6654b26 Updated Docker library to avoid 'os::killtree()' when discarding.
6654b26 is described below
commit 6654b26918b464764ae0329cba3669b4566a8d60
Author: Greg Mann <gr...@mesosphere.io>
AuthorDate: Thu Oct 4 16:43:57 2018 -0700
Updated Docker library to avoid 'os::killtree()' when discarding.
This patch updates the Docker library to consistently use the
overload of `subprocess()` which runs its binary via `execvp()`
rather than through a shell. This makes it safe to use
`os::kill()` rather than `os::killtree()` when discarding
these commands, which this patch also accomplishes.
Review: https://reviews.apache.org/r/68923/
---
src/docker/docker.cpp | 102 ++++++++++++++++++++++++++++++++++++++------------
src/docker/docker.hpp | 6 +--
2 files changed, 81 insertions(+), 27 deletions(-)
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index fb39f74..9ebb5f2 100644
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -162,7 +162,7 @@ void commandDiscarded(const Subprocess& s, const string& cmd)
{
if (s.status().isPending()) {
VLOG(1) << "'" << cmd << "' is being discarded";
- os::killtree(s.pid(), SIGKILL);
+ os::kill(s.pid(), SIGKILL);
}
}
@@ -1204,16 +1204,26 @@ Future<Nothing> Docker::stop(
stringify(timeoutSecs));
}
- string cmd = path + " -H " + socket + " stop -t " + stringify(timeoutSecs) +
- " " + containerName;
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("stop");
+ argv.push_back("-t");
+ argv.push_back(stringify(timeoutSecs));
+ argv.push_back(containerName);
+
+ const string cmd = strings::join(" ", argv);
VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- cmd,
+ path,
+ argv,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
+ nullptr,
None(),
None(),
createParentHooks());
@@ -1261,17 +1271,25 @@ Future<Nothing> Docker::kill(
const string& containerName,
int signal) const
{
- const string cmd =
- path + " -H " + socket +
- " kill --signal=" + stringify(signal) + " " + containerName;
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("kill");
+ argv.push_back("--signal=" + stringify(signal));
+ argv.push_back(containerName);
+
+ const string cmd = strings::join(" ", argv);
VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- cmd,
+ path,
+ argv,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
+ nullptr,
None(),
None(),
createParentHooks());
@@ -1289,17 +1307,30 @@ Future<Nothing> Docker::rm(
bool force) const
{
// The `-v` flag removes Docker volumes that may be present.
- const string cmd =
- path + " -H " + socket +
- (force ? " rm -f -v " : " rm -v ") + containerName;
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("rm");
+
+ if (force) {
+ argv.push_back("-f");
+ }
+
+ argv.push_back("-v");
+ argv.push_back(containerName);
+
+ const string cmd = strings::join(" ", argv);
VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- cmd,
+ path,
+ argv,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
+ nullptr,
None(),
None(),
createParentHooks());
@@ -1322,10 +1353,15 @@ Future<Docker::Container> Docker::inspect(
// discarded, and a mutex to control access to the callback.
auto callback = std::make_shared<pair<lambda::function<void()>, mutex>>();
- const string cmd =
- path + " -H " + socket + " inspect --type=container " + containerName;
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("inspect");
+ argv.push_back("--type=container");
+ argv.push_back(containerName);
- _inspect(cmd, promise, retryInterval, callback);
+ _inspect(argv, promise, retryInterval, callback);
return promise->future()
.onDiscard([callback]() {
@@ -1337,7 +1373,7 @@ Future<Docker::Container> Docker::inspect(
void Docker::_inspect(
- const string& cmd,
+ const vector<string>& argv,
const Owned<Promise<Docker::Container>>& promise,
const Option<Duration>& retryInterval,
shared_ptr<pair<lambda::function<void()>, mutex>> callback)
@@ -1346,13 +1382,17 @@ void Docker::_inspect(
return;
}
+ const string cmd = strings::join(" ", argv);
+
VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- cmd,
+ argv[0],
+ argv,
Subprocess::PATH(os::DEV_NULL),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ nullptr,
None(),
None(),
createParentHooks());
@@ -1386,13 +1426,13 @@ void Docker::_inspect(
s->status()
.onAny([=]() {
- __inspect(cmd, promise, retryInterval, output, s.get(), callback);
+ __inspect(argv, promise, retryInterval, output, s.get(), callback);
});
}
void Docker::__inspect(
- const string& cmd,
+ const vector<string>& argv,
const Owned<Promise<Docker::Container>>& promise,
const Option<Duration>& retryInterval,
Future<string> output,
@@ -1408,6 +1448,8 @@ void Docker::__inspect(
Option<int> status = s.status().get();
+ const string cmd = strings::join(" ", argv);
+
if (!status.isSome()) {
promise->fail("No status found from '" + cmd + "'");
} else if (status.get() != 0) {
@@ -1417,7 +1459,7 @@ void Docker::__inspect(
VLOG(1) << "Retrying inspect with non-zero status code. cmd: '"
<< cmd << "', interval: " << stringify(retryInterval.get());
Clock::timer(retryInterval.get(),
- [=]() { _inspect(cmd, promise, retryInterval, callback); });
+ [=]() { _inspect(argv, promise, retryInterval, callback); });
return;
}
@@ -1439,13 +1481,13 @@ void Docker::__inspect(
CHECK_SOME(s.out());
output
.onAny([=](const Future<string>& output) {
- ___inspect(cmd, promise, retryInterval, output, callback);
+ ___inspect(argv, promise, retryInterval, output, callback);
});
}
void Docker::___inspect(
- const string& cmd,
+ const vector<string>& argv,
const Owned<Promise<Docker::Container>>& promise,
const Option<Duration>& retryInterval,
const Future<string>& output,
@@ -1468,11 +1510,13 @@ void Docker::___inspect(
return;
}
+ const string cmd = strings::join(" ", argv);
+
if (retryInterval.isSome() && !container->started) {
VLOG(1) << "Retrying inspect since container not yet started. cmd: '"
<< cmd << "', interval: " << stringify(retryInterval.get());
Clock::timer(retryInterval.get(),
- [=]() { _inspect(cmd, promise, retryInterval, callback); } );
+ [=]() { _inspect(argv, promise, retryInterval, callback); });
return;
}
@@ -1484,7 +1528,17 @@ Future<vector<Docker::Container>> Docker::ps(
bool all,
const Option<string>& prefix) const
{
- string cmd = path + " -H " + socket + (all ? " ps -a" : " ps");
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("ps");
+
+ if (all) {
+ argv.push_back("-a");
+ }
+
+ const string cmd = strings::join(" ", argv);
VLOG(1) << "Running " << cmd;
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 25d9ca6..b48d894 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -345,14 +345,14 @@ private:
bool remove);
static void _inspect(
- const std::string& cmd,
+ const std::vector<std::string>& argv,
const process::Owned<process::Promise<Container>>& promise,
const Option<Duration>& retryInterval,
std::shared_ptr<std::pair<lambda::function<void()>, std::mutex>>
callback);
static void __inspect(
- const std::string& cmd,
+ const std::vector<std::string>& argv,
const process::Owned<process::Promise<Container>>& promise,
const Option<Duration>& retryInterval,
process::Future<std::string> output,
@@ -361,7 +361,7 @@ private:
callback);
static void ___inspect(
- const std::string& cmd,
+ const std::vector<std::string>& argv,
const process::Owned<process::Promise<Container>>& promise,
const Option<Duration>& retryInterval,
const process::Future<std::string>& output,