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/17 18:27:28 UTC
[mesos] branch 1.4.x 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 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/1.4.x by this push:
new 2820975 Updated Docker library to avoid 'os::killtree()' when discarding.
2820975 is described below
commit 2820975b776be90573bc34fee4a78d9adaca3145
Author: Greg Mann <gr...@gmail.com>
AuthorDate: Wed Oct 3 18:10:23 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 | 99 ++++++++++++++++++++++++++++++++++++++-------------
src/docker/docker.hpp | 6 ++--
2 files changed, 78 insertions(+), 27 deletions(-)
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 1dd8233..8411c95 100755
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -155,7 +155,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);
}
}
@@ -1126,13 +1126,22 @@ 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());
@@ -1180,14 +1189,21 @@ 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());
@@ -1205,14 +1221,26 @@ 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());
@@ -1235,8 +1263,14 @@ 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 " + containerName;
- _inspect(cmd, promise, retryInterval, callback);
+ vector<string> argv;
+ argv.push_back(path);
+ argv.push_back("-H");
+ argv.push_back(socket);
+ argv.push_back("inspect");
+ argv.push_back(containerName);
+
+ _inspect(argv, promise, retryInterval, callback);
return promise->future()
.onDiscard([callback]() {
@@ -1248,7 +1282,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)
@@ -1257,10 +1291,13 @@ 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());
@@ -1294,13 +1331,13 @@ void Docker::_inspect(
s.get().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,
@@ -1316,6 +1353,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) {
@@ -1325,7 +1364,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;
}
@@ -1347,13 +1386,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,
@@ -1376,11 +1415,13 @@ void Docker::___inspect(
return;
}
- if (retryInterval.isSome() && !container.get().started) {
+ 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;
}
@@ -1392,7 +1433,17 @@ Future<list<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 81c1154..02ca495 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -334,14 +334,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,
@@ -350,7 +350,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,