You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2014/08/05 00:10:01 UTC
[40/43] git commit: Addressing Docker review comments
Addressing Docker review comments
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/48e7d4a4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/48e7d4a4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/48e7d4a4
Branch: refs/heads/master
Commit: 48e7d4a4438331129d5604315788c0a421542093
Parents: 233e2d4
Author: Timothy Chen <tn...@gmail.com>
Authored: Thu Jul 24 10:57:34 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Mon Aug 4 15:08:17 2014 -0700
----------------------------------------------------------------------
src/Makefile.am | 4 -
src/docker/docker.cpp | 447 ++++++++++++---------
src/docker/docker.hpp | 73 ++--
src/examples/docker_no_executor_framework.cpp | 2 +-
src/slave/containerizer/containerizer.cpp | 2 +-
src/slave/containerizer/docker.cpp | 240 +++++------
src/slave/containerizer/docker.hpp | 8 +-
src/slave/flags.hpp | 2 +-
src/tests/cgroups_tests.cpp | 13 +
src/tests/docker_containerizer_tests.cpp | 56 +--
src/tests/docker_tests.cpp | 69 ++--
src/tests/environment.cpp | 30 +-
12 files changed, 495 insertions(+), 451 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 850fad3..04be4e0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1154,10 +1154,6 @@ EXTRA_DIST += examples/python/test_containerizer.py \
examples/python/test_framework.py
-# Docker test executor image files.
-EXTRA_DIST += tests/mesos_test_executor_docker_image/Dockerfile \
- tests/mesos_test_executor_docker_image/install.sh
-
dist_check_SCRIPTS += \
tests/balloon_framework_test.sh \
tests/low_level_scheduler_libprocess_test.sh \
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 4842cee..ee9c882 100644
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -20,14 +20,17 @@
#include <vector>
#include <stout/lambda.hpp>
-#include <stout/strings.hpp>
-
+#include <stout/os.hpp>
#include <stout/result.hpp>
+#include <stout/strings.hpp>
#include <stout/os/read.hpp>
#include <process/check.hpp>
#include <process/collect.hpp>
+#include <process/io.hpp>
+
+#include "common/status_utils.hpp"
#include "docker/docker.hpp"
@@ -46,8 +49,66 @@ using std::string;
using std::vector;
-Try<Nothing> Docker::validate(const Docker &docker)
+template<class T>
+static Future<T> failure(
+ const string& cmd,
+ int status,
+ const string& err)
+{
+ return Failure(
+ "Failed to '" + cmd + "': exit status = " +
+ WSTRINGIFY(status) + " stderr = " + err);
+}
+
+
+// Asynchronously read stderr from subprocess.
+static Future<string> err(const Subprocess& s)
+{
+ CHECK_SOME(s.err());
+
+ Try<Nothing> nonblock = os::nonblock(s.err().get());
+ if (nonblock.isError()) {
+ return Failure("Cannot set nonblock for stderr: " + nonblock.error());
+ }
+
+ // TODO(tnachen): Although unlikely, it's possible to not capture
+ // the caller's failure message if io::read stderr fails. Can
+ // chain a callback to at least log.
+ return io::read(s.err().get());
+}
+
+
+static Future<Nothing> _checkError(const string& cmd, const Subprocess& s)
+{
+ Option<int> status = s.status().get();
+ if (status.isNone()) {
+ return Failure("No status found for '" + cmd + "'");
+ }
+
+ if (status.get() != 0) {
+ // TODO(tnachen): Consider returning stdout as well.
+ return err(s).then(
+ lambda::bind(failure<Nothing>, cmd, status.get(), lambda::_1));
+ }
+
+ return Nothing();
+}
+
+
+// Returns a failure if no status or non-zero status returned from
+// subprocess.
+static Future<Nothing> checkError(const string& cmd, const Subprocess& s)
+{
+ return s.status().then(lambda::bind(_checkError, cmd, s));
+}
+
+
+Try<Docker> Docker::create(const string& path, bool validate)
{
+ if (!validate) {
+ return Docker(path);
+ }
+
// Make sure that cgroups are mounted, and at least the 'cpu'
// subsystem is attached.
Result<string> hierarchy = cgroups::hierarchy("cpu");
@@ -58,76 +119,109 @@ Try<Nothing> Docker::validate(const Docker &docker)
"to mount cgroups manually!");
}
- Future<std::string> info = docker.info();
+ std::string cmd = path + " info";
+
+ Try<Subprocess> s = subprocess(
+ cmd,
+ Subprocess::PATH("/dev/null"),
+ Subprocess::PIPE(),
+ Subprocess::PATH("/dev/null"));
- if (!info.await(Seconds(3))) {
- return Error("Failed to use Docker: Timed out");
- } else if (info.isFailed()) {
- return Error("Failed to use Docker: " + info.failure());
+ if (s.isError()) {
+ return Error(s.error());
}
- return Nothing();
+ Try<Nothing> nonblock = os::nonblock(s.get().out().get());
+ if (nonblock.isError()) {
+ return Error("Failed to accept nonblock stdout:" + nonblock.error());
+ }
+
+ Future<string> output = io::read(s.get().out().get());
+
+ if (!output.await(Seconds(5))) {
+ return Error("Docker info failed with time out");
+ } else if (output.isFailed()) {
+ return Error("Docker info failed: " + output.failure());
+ }
+
+ return Docker(path);
}
-string Docker::Container::id() const
+Try<Docker::Container> Docker::Container::create(const JSON::Object& json)
{
map<string, JSON::Value>::const_iterator entry =
json.values.find("Id");
- CHECK(entry != json.values.end());
- JSON::Value value = entry->second;
- CHECK(value.is<JSON::String>());
- return value.as<JSON::String>().value;
-}
+ if (entry == json.values.end()) {
+ return Error("Unable to find Id in container");
+ }
-string Docker::Container::name() const
-{
- map<string, JSON::Value>::const_iterator entry =
- json.values.find("Name");
- CHECK(entry != json.values.end());
- JSON::Value value = entry->second;
- CHECK(value.is<JSON::String>());
- return value.as<JSON::String>().value;
-}
+ JSON::Value idValue = entry->second;
+ if (!idValue.is<JSON::String>()) {
+ return Error("Id in container is not a string type");
+ }
-Option<pid_t> Docker::Container::pid() const
-{
- map<string, JSON::Value>::const_iterator state =
- json.values.find("State");
- CHECK(state != json.values.end());
- JSON::Value value = state->second;
- CHECK(value.is<JSON::Object>());
+ string id = idValue.as<JSON::String>().value;
- map<string, JSON::Value>::const_iterator entry =
- value.as<JSON::Object>().values.find("Pid");
- CHECK(entry != json.values.end());
- // TODO(yifan) reload operator '=' to reuse the value variable above.
+ entry = json.values.find("Name");
+ if (entry == json.values.end()) {
+ return Error("Unable to find Name in container");
+ }
+
+ JSON::Value nameValue = entry->second;
+ if (!nameValue.is<JSON::String>()) {
+ return Error("Name in container is not string type");
+ }
+
+ string name = nameValue.as<JSON::String>().value;
+
+ entry = json.values.find("State");
+ if (entry == json.values.end()) {
+ return Error("Unable to find State in container");
+ }
+
+ JSON::Value stateValue = entry->second;
+ if (!stateValue.is<JSON::Object>()) {
+ return Error("State in container is not object type");
+ }
+
+ entry = stateValue.as<JSON::Object>().values.find("Pid");
+ if (entry == json.values.end()) {
+ return Error("Unable to find Pid in State");
+ }
+
+ // TODO(yifan): Reload operator '=' to reuse the value variable above.
JSON::Value pidValue = entry->second;
- CHECK(pidValue.is<JSON::Number>());
+ if (!pidValue.is<JSON::Number>()) {
+ return Error("Pid in State is not number type");
+ }
pid_t pid = pid_t(pidValue.as<JSON::Number>().value);
- if (pid == 0) {
- return None();
+
+ Option<pid_t> optionalPid;
+ if (pid != 0) {
+ optionalPid = pid;
}
- return pid;
+
+ return Docker::Container(id, name, optionalPid);
}
-Future<Option<int> > Docker::run(
+
+Future<Nothing> Docker::run(
const string& image,
const string& command,
const string& name,
const Option<mesos::Resources>& resources,
const Option<map<string, string> >& env) const
{
-
- string cmd = " run -d";
+ string cmd = path + " run -d";
if (resources.isSome()) {
// TODO(yifan): Support other resources (e.g. disk, ports).
Option<double> cpus = resources.get().cpus();
if (cpus.isSome()) {
uint64_t cpuShare =
- std::max((uint64_t) (CPU_SHARES_PER_CPU * cpus.get()), MIN_CPU_SHARES);
+ std::max((uint64_t) (CPU_SHARES_PER_CPU * cpus.get()), MIN_CPU_SHARES);
cmd += " -c " + stringify(cpuShare);
}
@@ -139,6 +233,8 @@ Future<Option<int> > Docker::run(
}
if (env.isSome()) {
+ // TODO(tnachen): Use subprocess with args instead once we can
+ // handle splitting command string into args.
foreachpair (string key, string value, env.get()) {
key = strings::replace(key, "\"", "\\\"");
value = strings::replace(value, "\"", "\\\"");
@@ -148,88 +244,96 @@ Future<Option<int> > Docker::run(
cmd += " --net=host --name=" + name + " " + image + " " + command;
- VLOG(1) << "Running " << path << cmd;
+ VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- path + cmd,
- Subprocess::PIPE(),
- Subprocess::PIPE(),
+ cmd,
+ Subprocess::PATH("/dev/null"),
+ Subprocess::PATH("/dev/null"),
Subprocess::PIPE());
if (s.isError()) {
return Failure(s.error());
}
- return s.get().status();
+
+ return checkError(cmd, s.get());
}
-Future<Option<int> > Docker::kill(const string& container) const
+Future<Nothing> Docker::kill(const string& container, bool remove) const
{
- VLOG(1) << "Running " << path << " kill " << container;
+ const string cmd = path + " kill " + container;
+
+ VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- path + " kill " + container,
- Subprocess::PIPE(),
- Subprocess::PIPE(),
+ cmd,
+ Subprocess::PATH("/dev/null"),
+ Subprocess::PATH("/dev/null"),
Subprocess::PIPE());
if (s.isError()) {
return Failure(s.error());
}
- return s.get().status();
+ return s.get().status()
+ .then(lambda::bind(
+ &Docker::_kill,
+ *this,
+ container,
+ cmd,
+ s.get(),
+ remove));
}
-
-Future<Option<int> > Docker::rm(
+Future<Nothing> Docker::_kill(
+ const Docker& docker,
const string& container,
- const bool force) const
+ const string& cmd,
+ const Subprocess& s,
+ bool remove)
{
- string cmd = force ? " rm -f " : " rm ";
-
- VLOG(1) << "Running " << path << cmd << container;
-
- Try<Subprocess> s = subprocess(
- path + cmd + container,
- Subprocess::PIPE(),
- Subprocess::PIPE(),
- Subprocess::PIPE());
+ Option<int> status = s.status().get();
- if (s.isError()) {
- return Failure(s.error());
+ if (remove) {
+ bool force = !status.isSome() || status.get() != 0;
+ return docker.rm(container, force);
}
- return s.get().status();
+ return checkError(cmd, s);
}
-Future<Option<int> > Docker::killAndRm(const string& container) const
+Future<Nothing> Docker::rm(
+ const string& container,
+ bool force) const
{
- return kill(container)
- .then(lambda::bind(Docker::_killAndRm, *this, container, lambda::_1));
-}
+ const string cmd = path + (force ? " rm -f " : " rm ") + container;
+ VLOG(1) << "Running " << cmd;
-Future<Option<int> > Docker::_killAndRm(
- const Docker& docker,
- const string& container,
- const Option<int>& status)
-{
- // If 'kill' fails, then do a 'rm -f'.
- if (status.isNone()) {
- return docker.rm(container, true);
+ Try<Subprocess> s = subprocess(
+ cmd,
+ Subprocess::PATH("/dev/null"),
+ Subprocess::PATH("/dev/null"),
+ Subprocess::PIPE());
+
+ if (s.isError()) {
+ return Failure(s.error());
}
- return docker.rm(container);
+
+ return checkError(cmd, s.get());
}
Future<Docker::Container> Docker::inspect(const string& container) const
{
- VLOG(1) << "Running " << path << " inspect " << container;
+ const string cmd = path + " inspect " + container;
+ VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- path + " inspect " + container,
- Subprocess::PIPE(),
+ cmd,
+ Subprocess::PATH("/dev/null"),
Subprocess::PIPE(),
Subprocess::PIPE());
@@ -238,88 +342,61 @@ Future<Docker::Container> Docker::inspect(const string& container) const
}
return s.get().status()
- .then(lambda::bind(&Docker::_inspect, s.get()));
+ .then(lambda::bind(&Docker::_inspect, cmd, s.get()));
}
-namespace os {
-
-inline Result<std::string> read(
- int fd,
- Option<size_t> size = None(),
- size_t chunk = 16 * 4096)
-{
- std::string result;
-
- while (size.isNone() || result.size() < size.get()) {
- char buffer[chunk];
- ssize_t length = ::read(fd, buffer, chunk);
-
- if (length < 0) {
- // TODO(bmahler): Handle a non-blocking fd? (EAGAIN, EWOULDBLOCK)
- if (errno == EINTR) {
- continue;
- }
- return ErrnoError();
- } else if (length == 0) {
- // Reached EOF before expected! Only return as much data as
- // available or None if we haven't read anything yet.
- if (result.size() > 0) {
- return result;
- }
- return None();
- }
-
- result.append(buffer, length);
- }
-
- return result;
-}
-
-} // namespace os {
-
-
-Future<Docker::Container> Docker::_inspect(const Subprocess& s)
+Future<Docker::Container> Docker::_inspect(
+ const string& cmd,
+ const Subprocess& s)
{
// Check the exit status of 'docker inspect'.
CHECK_READY(s.status());
Option<int> status = s.status().get();
- if (status.isSome() && status.get() != 0) {
- // TODO(benh): Include stderr in error message.
- Result<string> read = os::read(s.err().get());
- return Failure("Failed to do 'docker inspect': " +
- (read.isSome()
- ? read.get()
- : " exited with status " + stringify(status.get())));
+ if (!status.isSome()) {
+ return Failure("No status found from '" + cmd + "'");
+ } else if (status.get() != 0) {
+ return err(s).then(
+ lambda::bind(
+ failure<Docker::Container>,
+ cmd,
+ status.get(),
+ lambda::_1));
}
// Read to EOF.
- // TODO(benh): Read output asynchronously.
CHECK_SOME(s.out());
- Result<string> output = os::read(s.out().get());
-
- if (output.isError()) {
- // TODO(benh): Include stderr in error message.
- return Failure("Failed to read output: " + output.error());
- } else if (output.isNone()) {
- // TODO(benh): Include stderr in error message.
- return Failure("No output available");
+ Try<Nothing> nonblock = os::nonblock(s.out().get());
+ if (nonblock.isError()) {
+ return Failure("Failed to accept nonblock stdout:" + nonblock.error());
}
+ Future<string> output = io::read(s.out().get());
+ return output.then(lambda::bind(&Docker::__inspect, lambda::_1));
+}
- Try<JSON::Array> parse = JSON::parse<JSON::Array>(output.get());
+
+Future<Docker::Container> Docker::__inspect(const string& output)
+{
+ Try<JSON::Array> parse = JSON::parse<JSON::Array>(output);
if (parse.isError()) {
return Failure("Failed to parse JSON: " + parse.error());
}
JSON::Array array = parse.get();
-
- // Skip the container if it no longer exists.
+ // Only return if only one container identified with name.
if (array.values.size() == 1) {
CHECK(array.values.front().is<JSON::Object>());
- return Docker::Container(array.values.front().as<JSON::Object>());
+ Try<Docker::Container> container =
+ Docker::Container::create(array.values.front().as<JSON::Object>());
+
+ if (container.isError()) {
+ return Failure("Unable to create container: " + container.error());
+ }
+
+ return container.get();
}
// TODO(benh): Handle the case where the short container ID was
@@ -330,16 +407,16 @@ Future<Docker::Container> Docker::_inspect(const Subprocess& s)
Future<list<Docker::Container> > Docker::ps(
- const bool all,
+ bool all,
const Option<string>& prefix) const
{
- string cmd = all ? " ps -a" : " ps";
+ string cmd = path + (all ? " ps -a" : " ps");
- VLOG(1) << "Running " << path << cmd;
+ VLOG(1) << "Running " << cmd;
Try<Subprocess> s = subprocess(
- path + cmd,
- Subprocess::PIPE(),
+ cmd,
+ Subprocess::PATH("/dev/null"),
Subprocess::PIPE(),
Subprocess::PIPE());
@@ -348,39 +425,46 @@ Future<list<Docker::Container> > Docker::ps(
}
return s.get().status()
- .then(lambda::bind(&Docker::_ps, *this, s.get(), prefix));
+ .then(lambda::bind(&Docker::_ps, *this, cmd, s.get(), prefix));
}
Future<list<Docker::Container> > Docker::_ps(
const Docker& docker,
+ const string& cmd,
const Subprocess& s,
const Option<string>& prefix)
{
- // Check the exit status of 'docker ps'.
- CHECK_READY(s.status());
-
Option<int> status = s.status().get();
- if (status.isSome() && status.get() != 0) {
- // TODO(benh): Include stderr in error message.
- return Failure("Failed to do 'docker ps'");
+ if (!status.isSome()) {
+ return Failure("No status found from '" + cmd + "'");
+ } else if (status.get() != 0) {
+ return err(s).then(
+ lambda::bind(
+ failure<list<Docker::Container> >,
+ cmd,
+ status.get(),
+ lambda::_1));
}
// Read to EOF.
- // TODO(benh): Read output asynchronously.
CHECK_SOME(s.out());
- Result<string> output = os::read(s.out().get());
-
- if (output.isError()) {
- // TODO(benh): Include stderr in error message.
- return Failure("Failed to read output: " + output.error());
- } else if (output.isNone()) {
- // TODO(benh): Include stderr in error message.
- return Failure("No output available");
+ Try<Nothing> nonblock = os::nonblock(s.out().get());
+ if (nonblock.isError()) {
+ return Failure("Failed to accept nonblock stdout:" + nonblock.error());
}
+ Future<string> output = io::read(s.out().get());
+ return output.then(lambda::bind(&Docker::__ps, docker, prefix, lambda::_1));
+}
- vector<string> lines = strings::tokenize(output.get(), "\n");
+
+Future<list<Docker::Container> > Docker::__ps(
+ const Docker& docker,
+ const Option<string>& prefix,
+ const string& output)
+{
+ vector<string> lines = strings::tokenize(output, "\n");
// Skip the header.
CHECK(!lines.empty());
@@ -392,6 +476,7 @@ Future<list<Docker::Container> > Docker::_ps(
// Inspect the containers that we are interested in depending on
// whether or not a 'prefix' was specified.
vector<string> columns = strings::split(strings::trim(line), " ");
+ // We expect the name column to be the last column from ps.
string name = columns[columns.size() - 1];
if (prefix.isNone()) {
futures.push_back(docker.inspect(name));
@@ -402,33 +487,3 @@ Future<list<Docker::Container> > Docker::_ps(
return collect(futures);
}
-
-
-Future<std::string> Docker::info() const
-{
- std::string cmd = path + " info";
-
- VLOG(1) << "Running " << cmd;
-
- Try<Subprocess> s = subprocess(
- cmd,
- Subprocess::PIPE(),
- Subprocess::PIPE(),
- Subprocess::PIPE());
-
- if (s.isError()) {
- return Failure(s.error());
- }
-
- Result<string> output = os::read(s.get().out().get());
-
- if (output.isError()) {
- // TODO(benh): Include stderr in error message.
- return Failure("Failed to read output: " + output.error());
- } else if (output.isNone()) {
- // TODO(benh): Include stderr in error message.
- return Failure("No output available");
- }
-
- return output.get();
-}
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index c4724de..98b2d60 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -37,55 +37,50 @@
class Docker
{
public:
- // Validate Docker support
- static Try<Nothing> validate(const Docker& docker);
+ // Create Docker abstraction and optionally validate docker.
+ static Try<Docker> create(const std::string& path, bool validate = true);
class Container
{
public:
- Container(const JSON::Object& json) : json(json) {}
+ static Try<Container> create(const JSON::Object& json);
// Returns the ID of the container.
- std::string id() const;
+ std::string id;
// Returns the name of the container.
- std::string name() const;
+ std::string name;
- // Returns the Pid of the container, or None if the container is
+ // Returns the pid of the container, or None if the container is
// not running.
- Option<pid_t> pid() const;
+ Option<pid_t> pid;
private:
- JSON::Object json; // JSON returned from 'docker inspect'.
+ Container(
+ const std::string& _id,
+ const std::string& _name,
+ const Option<pid_t>& _pid)
+ : id(_id), name(_name), pid(_pid) {}
};
- // Uses the specified path to the Docker CLI tool.
- Docker(const std::string& path) : path(path) {}
-
// Performs 'docker run IMAGE'.
- process::Future<Option<int> > run(
+ process::Future<Nothing> run(
const std::string& image,
const std::string& command,
const std::string& name,
const Option<mesos::Resources>& resources = None(),
const Option<std::map<std::string, std::string> >& env = None()) const;
- // Performs 'docker kill CONTAINER'.
- process::Future<Option<int> > kill(
- const std::string& container) const;
+ // Performs 'docker kill CONTAINER'. If remove is true then a rm -f
+ // will be called when kill failed, otherwise a failure is returned.
+ process::Future<Nothing> kill(
+ const std::string& container,
+ bool remove = false) const;
// Performs 'docker rm (-f) CONTAINER'.
- process::Future<Option<int> > rm(
+ process::Future<Nothing> rm(
const std::string& container,
- const bool force = false) const;
-
- // Performs 'docker kill && docker rm'
- // if 'docker kill' fails, then will do a 'docker rm -f'.
- //
- // TODO(yifan): Depreciate this when the docker provides
- // something like 'docker rm --kill'.
- process::Future<Option<int> > killAndRm(
- const std::string& container) const;
+ bool force = false) const;
// Performs 'docker inspect CONTAINER'.
process::Future<Container> inspect(
@@ -93,23 +88,37 @@ public:
// Performs 'docker ps (-a)'.
process::Future<std::list<Container> > ps(
- const bool all = false,
+ bool all = false,
const Option<std::string>& prefix = None()) const;
- process::Future<std::string> info() const;
-
private:
- // Continuations.
+ // Uses the specified path to the Docker CLI tool.
+ Docker(const std::string& _path) : path(_path) {};
+
+ static process::Future<Nothing> _kill(
+ const Docker& docker,
+ const std::string& container,
+ const std::string& cmd,
+ const process::Subprocess& s,
+ bool remove);
+
static process::Future<Container> _inspect(
+ const std::string& cmd,
const process::Subprocess& s);
+
+ static process::Future<Container> __inspect(
+ const std::string& output);
+
static process::Future<std::list<Container> > _ps(
const Docker& docker,
+ const std::string& cmd,
const process::Subprocess& s,
const Option<std::string>& prefix);
- static process::Future<Option<int> > _killAndRm(
+
+ static process::Future<std::list<Container> > __ps(
const Docker& docker,
- const std::string& container,
- const Option<int>& status);
+ const Option<std::string>& prefix,
+ const std::string& output);
const std::string path;
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/examples/docker_no_executor_framework.cpp
----------------------------------------------------------------------
diff --git a/src/examples/docker_no_executor_framework.cpp b/src/examples/docker_no_executor_framework.cpp
index 3619405..d5385d9 100644
--- a/src/examples/docker_no_executor_framework.cpp
+++ b/src/examples/docker_no_executor_framework.cpp
@@ -176,7 +176,7 @@ int main(int argc, char** argv)
FrameworkInfo framework;
framework.set_user(""); // Have Mesos fill in the current user.
- framework.set_name("No Executor Framework (C++)");
+ framework.set_name("Docker No Executor Framework (C++)");
// TODO(vinod): Make checkpointing the default when it is default
// on the slave.
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/slave/containerizer/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/containerizer.cpp b/src/slave/containerizer/containerizer.cpp
index 003775b..c91ba38 100644
--- a/src/slave/containerizer/containerizer.cpp
+++ b/src/slave/containerizer/containerizer.cpp
@@ -173,7 +173,7 @@ Try<Containerizer*> Containerizer::create(const Flags& flags, bool local)
}
} else if (type == "docker") {
Try<DockerContainerizer*> containerizer =
- DockerContainerizer::create(flags, local);
+ DockerContainerizer::create(flags);
if (containerizer.isError()) {
return Error("Could not create DockerContainerizer: " +
containerizer.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 294b4c2..904cdd3 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -28,8 +28,6 @@
#include <stout/hashset.hpp>
#include <stout/os.hpp>
-#include "common/status_utils.hpp"
-
#include "docker/docker.hpp"
#ifdef __linux__
@@ -75,11 +73,10 @@ class DockerContainerizerProcess
{
public:
DockerContainerizerProcess(
- const Flags& flags,
- bool local,
- const Docker& docker)
- : flags(flags),
- docker(docker) {}
+ const Flags& _flags,
+ const Docker& _docker)
+ : flags(_flags),
+ docker(_docker) {}
virtual process::Future<Nothing> recover(
const Option<state::SlaveState>& state);
@@ -115,7 +112,7 @@ public:
virtual void destroy(
const ContainerID& containerId,
- const bool& killed = true);
+ bool killed = true); // process is either killed or reaped.
virtual process::Future<hashset<ContainerID> > containers();
@@ -131,16 +128,14 @@ private:
const std::string& directory,
const SlaveID& slaveId,
const PID<Slave>& slavePid,
- bool checkpoint,
- const Option<int>& status);
+ bool checkpoint);
process::Future<bool> _launch(
const ContainerID& containerId,
const ExecutorInfo& executorInfo,
const SlaveID& slaveId,
const PID<Slave>& slavePid,
- bool checkpoint,
- const Option<int>& status);
+ bool checkpoint);
process::Future<bool> __launch(
const ContainerID& containerId,
@@ -150,16 +145,15 @@ private:
bool checkpoint,
const Docker::Container& container);
-
void _destroy(
const ContainerID& containerId,
- const bool& killed,
- const Future<Option<int> >& future);
+ bool killed,
+ const Future<Nothing>& future);
void __destroy(
const ContainerID& containerId,
- const bool& killed,
- const Future<Option<int > >& status);
+ bool killed,
+ const Future<Option<int> >& status);
process::Future<Nothing> _update(
const ContainerID& containerId,
@@ -174,9 +168,7 @@ private:
// container destroy.
void reaped(const ContainerID& containerId);
- // Parse the ContainerID from a Docker container and return None if
- // the container was not launched from Mesos.
- Option<ContainerID> parse(const Docker::Container& container);
+ static std::string containerName(const ContainerID& containerId);
const Flags flags;
@@ -201,26 +193,48 @@ private:
};
+// Parse the ContainerID from a Docker container and return None if
+// the container was not launched from Mesos.
+Option<ContainerID> parse(
+ const Docker::Container& container)
+{
+ Option<string> name = None();
+
+ if (strings::startsWith(container.name, DOCKER_NAME_PREFIX)) {
+ name = strings::remove(
+ container.name, DOCKER_NAME_PREFIX, strings::PREFIX);
+ } else if (strings::startsWith(container.name, "/" + DOCKER_NAME_PREFIX)) {
+ name = strings::remove(
+ container.name, "/" + DOCKER_NAME_PREFIX, strings::PREFIX);
+ }
+
+ if (name.isSome()) {
+ ContainerID id;
+ id.set_value(name.get());
+ return id;
+ }
+
+ return None();
+}
+
+
Try<DockerContainerizer*> DockerContainerizer::create(
- const Flags& flags,
- bool local)
+ const Flags& flags)
{
- Docker docker(flags.docker);
- Try<Nothing> validation = Docker::validate(docker);
- if (validation.isError()) {
- return Error(validation.error());
+ Try<Docker> docker = Docker::create(flags.docker);
+ if (docker.isError()) {
+ return Error(docker.error());
}
- return new DockerContainerizer(flags, local, docker);
+ return new DockerContainerizer(flags, docker.get());
}
DockerContainerizer::DockerContainerizer(
const Flags& flags,
- bool local,
const Docker& docker)
{
- process = new DockerContainerizerProcess(flags, local, docker);
+ process = new DockerContainerizerProcess(flags, docker);
spawn(process);
}
@@ -355,6 +369,12 @@ static int setup(const string& directory)
}
+string DockerContainerizerProcess::containerName(const ContainerID& containerId)
+{
+ return DOCKER_NAME_PREFIX + stringify(containerId);
+}
+
+
Future<Nothing> DockerContainerizerProcess::recover(
const Option<SlaveState>& state)
{
@@ -415,7 +435,6 @@ Future<Nothing> DockerContainerizerProcess::recover(
promises.put(containerId, promise);
- CHECK_SOME(run.get().forkedPid);
pid_t pid = run.get().forkedPid.get();
statuses[containerId] = process::reap(pid);
@@ -429,8 +448,7 @@ Future<Nothing> DockerContainerizerProcess::recover(
// pid as one that just exited (highly unlikely) and the
// slave dies after the new executor is launched but before
// it hears about the termination of the earlier executor
- // (also unlikely). Regardless, the launcher can't do
- // anything sensible so this is considered an error.
+ // (also unlikely).
return Failure(
"Detected duplicate pid " + stringify(pid) +
" for container " + stringify(containerId));
@@ -453,7 +471,7 @@ Future<Nothing> DockerContainerizerProcess::_recover(
{
foreach (const Docker::Container& container, containers) {
VLOG(1) << "Checking if Docker container named '"
- << container.name() << "' was started by Mesos";
+ << container.name << "' was started by Mesos";
Option<ContainerID> id = parse(container);
@@ -470,7 +488,7 @@ Future<Nothing> DockerContainerizerProcess::_recover(
if (!statuses.keys().contains(id.get())) {
// TODO(benh): Retry 'docker rm -f' if it failed but the container
// still exists (asynchronously).
- docker.killAndRm(container.id());
+ docker.kill(container.id, true);
}
}
@@ -488,14 +506,13 @@ Future<bool> DockerContainerizerProcess::launch(
bool checkpoint)
{
if (promises.contains(containerId)) {
- LOG(ERROR) << "Cannot start already running container '"
- << containerId << "'";
return Failure("Container already started");
}
CommandInfo command = executorInfo.command();
if (!command.has_container()) {
+ LOG(INFO) << "No container info found, skipping launch";
return false;
}
@@ -503,6 +520,7 @@ Future<bool> DockerContainerizerProcess::launch(
// Check if we should try and launch this command.
if (!strings::startsWith(image, "docker:///")) {
+ LOG(INFO) << "No docker image found, skipping launch";
return false;
}
@@ -519,7 +537,7 @@ Future<bool> DockerContainerizerProcess::launch(
image = strings::remove(image, "docker:///", strings::PREFIX);
// Construct the Docker container name.
- string name = DOCKER_NAME_PREFIX + stringify(containerId);
+ string name = containerName(containerId);
map<string, string> env = executorEnvironment(
executorInfo,
@@ -546,8 +564,7 @@ Future<bool> DockerContainerizerProcess::launch(
executorInfo,
slaveId,
slavePid,
- checkpoint,
- lambda::_1))
+ checkpoint))
.onFailed(defer(self(), &Self::destroy, containerId, false));
}
@@ -563,12 +580,11 @@ Future<bool> DockerContainerizerProcess::launch(
bool checkpoint)
{
if (promises.contains(containerId)) {
- LOG(ERROR) << "Cannot start already running container '"
- << containerId << "'";
return Failure("Container already started");
}
if (!taskInfo.has_command()) {
+ LOG(WARNING) << "Not expecting call without command info";
return false;
}
@@ -577,6 +593,8 @@ Future<bool> DockerContainerizerProcess::launch(
// Check if we should try and launch this command.
if (!command.has_container() ||
!strings::startsWith(command.container().image(), "docker:///")) {
+ LOG(INFO) << "No container info or container image is not docker image, "
+ << "skipping launch";
return false;
}
@@ -598,7 +616,7 @@ Future<bool> DockerContainerizerProcess::launch(
image = strings::remove(image, "docker:///", strings::PREFIX);
// Construct the Docker container name.
- string name = DOCKER_NAME_PREFIX + stringify(containerId);
+ string name = containerName(containerId);
// Start a docker container then launch the executor (but destroy
// the Docker container if launching the executor failed).
@@ -611,8 +629,7 @@ Future<bool> DockerContainerizerProcess::launch(
directory,
slaveId,
slavePid,
- checkpoint,
- lambda::_1))
+ checkpoint))
.onFailed(defer(self(), &Self::destroy, containerId, false));
}
@@ -624,17 +641,8 @@ Future<bool> DockerContainerizerProcess::_launch(
const string& directory,
const SlaveID& slaveId,
const PID<Slave>& slavePid,
- bool checkpoint,
- const Option<int>& status)
+ bool checkpoint)
{
- // Try and see if the run failed.
- if (status.isSome() && status.get() != 0) {
- // Best effort kill and remove the container just in case.
- docker.killAndRm(DOCKER_NAME_PREFIX + stringify(containerId));
- return Failure("Failed to run the container (" +
- WSTRINGIFY(status.get()) + ")");
- }
-
// Prepare environment variables for the executor.
map<string, string> env = executorEnvironment(
executorInfo,
@@ -656,9 +664,8 @@ Future<bool> DockerContainerizerProcess::_launch(
// don't want the exit status from 'docker wait' but rather the exit
// status from the container, hence the use of /bin/bash.
string override =
- "/bin/bash -c 'exit `" +
- flags.docker + " wait " + DOCKER_NAME_PREFIX + stringify(containerId) +
- "`'";
+ "/bin/sh -c 'exit `" +
+ flags.docker + " wait " + containerName(containerId) + "`'";
Try<Subprocess> s = subprocess(
executorInfo.command().value() + " --override " + override,
@@ -708,9 +715,9 @@ Future<bool> DockerContainerizerProcess::_launch(
errno == EINTR);
if (length != sizeof(c)) {
+ string error = string(strerror(errno));
os::close(s.get().in().get());
- return Failure("Failed to synchronize with child process: " +
- string(strerror(errno)));
+ return Failure("Failed to synchronize with child process: " + error);
}
// And finally watch for when the executor gets reaped.
@@ -728,17 +735,9 @@ Future<bool> DockerContainerizerProcess::_launch(
const ExecutorInfo& executorInfo,
const SlaveID& slaveId,
const PID<Slave>& slavePid,
- bool checkpoint,
- const Option<int>& status)
+ bool checkpoint)
{
- if (status.isSome() && status.get() != 0) {
- // Best effort kill and remove the container just in case.
- docker.killAndRm(DOCKER_NAME_PREFIX + stringify(containerId));
- return Failure("Failed to run the container (" +
- WSTRINGIFY(status.get()) + ")");
- }
-
- return docker.inspect(DOCKER_NAME_PREFIX + stringify(containerId))
+ return docker.inspect(containerName(containerId))
.then(defer(self(),
&Self::__launch,
containerId,
@@ -758,18 +757,18 @@ Future<bool> DockerContainerizerProcess::__launch(
bool checkpoint,
const Docker::Container& container)
{
- Option<int> pid = container.pid();
+ Option<int> pid = container.pid;
if (!pid.isSome()) {
return Failure("Unable to get executor pid after launch");
}
if (checkpoint) {
- // TODO(tnachen): We might not be able to checkpoint
- // if the slave dies before it can checkpoint while
- // the executor is still running. Optinally we can consider
- // recording the slave id and executor id as part of the
- // docker container name so we can recover from this.
+ // TODO(tnachen): We might not be able to checkpoint if the slave
+ // dies before it can checkpoint while the executor is still
+ // running. Optinally we can consider recording the slave id and
+ // executor id as part of the docker container name so we can
+ // recover from this.
const string& path =
slave::paths::getForkedPidPath(
slave::paths::getMetaRootDir(flags.work_dir),
@@ -806,22 +805,21 @@ Future<Nothing> DockerContainerizerProcess::update(
const Resources& _resources)
{
if (!promises.contains(containerId)) {
- LOG(WARNING)
- << "Ignoring updating unknown container: "
- << containerId.value();
+ LOG(WARNING) << "Ignoring updating unknown container: "
+ << containerId;
return Nothing();
}
+ // Store the resources for usage()
+ resources.put(containerId, _resources);
+
#ifdef __linux__
if (!_resources.cpus().isSome() && !_resources.mem().isSome()) {
LOG(WARNING) << "Ignoring update as no supported resources are present";
return Nothing();
}
- // Store the resources for usage()
- resources.put(containerId, _resources);
-
- return docker.inspect(DOCKER_NAME_PREFIX + stringify(containerId))
+ return docker.inspect(containerName(containerId))
.then(defer(self(), &Self::_update, containerId, _resources, lambda::_1));
#else
return Nothing();
@@ -860,7 +858,7 @@ Future<Nothing> DockerContainerizerProcess::_update(
// update the proper cgroup control files.
// First check that this container still appears to be running.
- Option<pid_t> pid = container.pid();
+ Option<pid_t> pid = container.pid;
if (pid.isNone()) {
return Nothing();
}
@@ -873,10 +871,9 @@ Future<Nothing> DockerContainerizerProcess::_update(
return Failure("Failed to determine cgroup for the 'cpu' subsystem: " +
cpuCgroup.error());
} else if (cpuCgroup.isNone()) {
- LOG(WARNING)
- << "Container " << containerId
- << " does not appear to be a member of a cgroup "
- << "where the 'cpu' subsystem is mounted";
+ LOG(WARNING) << "Container " << containerId
+ << " does not appear to be a member of a cgroup "
+ << "where the 'cpu' subsystem is mounted";
}
// And update the CPU shares (if applicable).
@@ -895,10 +892,9 @@ Future<Nothing> DockerContainerizerProcess::_update(
return Failure("Failed to update 'cpu.shares': " + write.error());
}
- LOG(INFO)
- << "Updated 'cpu.shares' to " << shares
- << " at " << path::join(cpuHierarchy.get(), cpuCgroup.get())
- << " for container " << containerId;
+ LOG(INFO) << "Updated 'cpu.shares' to " << shares
+ << " at " << path::join(cpuHierarchy.get(), cpuCgroup.get())
+ << " for container " << containerId;
}
// Now determine the cgroup for the 'memory' subsystem.
@@ -908,16 +904,16 @@ Future<Nothing> DockerContainerizerProcess::_update(
return Failure("Failed to determine cgroup for the 'memory' subsystem: " +
memoryCgroup.error());
} else if (memoryCgroup.isNone()) {
- LOG(WARNING)
- << "Container " << containerId
- << " does not appear to be a member of a cgroup "
- << "where the 'memory' subsystem is mounted";
+ LOG(WARNING) << "Container " << containerId
+ << " does not appear to be a member of a cgroup "
+ << "where the 'memory' subsystem is mounted";
}
// And update the memory limits (if applicable).
if (memoryHierarchy.isSome() &&
memoryCgroup.isSome() &&
_resources.mem().isSome()) {
+ // TODO(tnachen): investigate and handle OOM with docker.
Bytes mem = _resources.mem().get();
Bytes limit = std::max(mem, MIN_MEMORY);
@@ -931,9 +927,8 @@ Future<Nothing> DockerContainerizerProcess::_update(
write.error());
}
- LOG(INFO)
- << "Updated 'memory.soft_limit_in_bytes' to " << limit
- << " for container " << containerId;
+ LOG(INFO) << "Updated 'memory.soft_limit_in_bytes' to " << limit
+ << " for container " << containerId;
// Read the existing limit.
Try<Bytes> currentLimit =
@@ -946,6 +941,9 @@ Future<Nothing> DockerContainerizerProcess::_update(
}
// Only update if new limit is higher.
+ // TODO(benh): Introduce a MemoryWatcherProcess which monitors the
+ // discrepancy between usage and soft limit and introduces a
+ // "manual oom" if necessary.
if (limit > currentLimit.get()) {
write = cgroups::memory::limit_in_bytes(
memoryHierarchy.get(), memoryCgroup.get(), limit);
@@ -955,10 +953,9 @@ Future<Nothing> DockerContainerizerProcess::_update(
write.error());
}
- LOG(INFO)
- << "Updated 'memory.limit_in_bytes' to " << limit
- << " at " << path::join(memoryHierarchy.get(), memoryCgroup.get())
- << " for container " << containerId;
+ LOG(INFO) << "Updated 'memory.limit_in_bytes' to " << limit << " at "
+ << path::join(memoryHierarchy.get(), memoryCgroup.get())
+ << " for container " << containerId;
}
}
#endif // __linux__
@@ -982,7 +979,7 @@ Future<ResourceStatistics> DockerContainerizerProcess::usage(
}
// Construct the Docker container name.
- string name = DOCKER_NAME_PREFIX + stringify(containerId);
+ string name = containerName(containerId);
return docker.inspect(name)
.then(defer(self(), &Self::_usage, containerId, lambda::_1));
#endif // __linux__
@@ -993,7 +990,7 @@ Future<ResourceStatistics> DockerContainerizerProcess::_usage(
const ContainerID& containerId,
const Docker::Container& container)
{
- Option<pid_t> pid = container.pid();
+ Option<pid_t> pid = container.pid;
if (pid.isNone()) {
return Failure("Container is not running");
}
@@ -1038,7 +1035,7 @@ Future<containerizer::Termination> DockerContainerizerProcess::wait(
void DockerContainerizerProcess::destroy(
const ContainerID& containerId,
- const bool& killed)
+ bool killed)
{
if (!promises.contains(containerId)) {
LOG(WARNING) << "Ignoring destroy of unknown container: " << containerId;
@@ -1071,15 +1068,15 @@ void DockerContainerizerProcess::destroy(
// TODO(benh): Retry 'docker rm -f' if it failed but the container
// still exists (asynchronously).
- docker.killAndRm(DOCKER_NAME_PREFIX + stringify(containerId))
+ docker.kill(containerName(containerId), true)
.onAny(defer(self(), &Self::_destroy, containerId, killed, lambda::_1));
}
void DockerContainerizerProcess::_destroy(
const ContainerID& containerId,
- const bool& killed,
- const Future<Option<int> >& future)
+ bool killed,
+ const Future<Nothing>& future)
{
if (!future.isReady()) {
promises[containerId]->fail(
@@ -1108,7 +1105,7 @@ void DockerContainerizerProcess::_destroy(
void DockerContainerizerProcess::__destroy(
const ContainerID& containerId,
- const bool& killed,
+ bool killed,
const Future<Option<int> >& status)
{
containerizer::Termination termination;
@@ -1116,6 +1113,8 @@ void DockerContainerizerProcess::__destroy(
if (status.isReady() && status.get().isSome()) {
termination.set_status(status.get().get());
}
+ termination.set_message(killed ?
+ "Docker task killed" : "Docker process terminated");
promises[containerId]->set(termination);
@@ -1144,29 +1143,6 @@ void DockerContainerizerProcess::reaped(const ContainerID& containerId)
}
-Option<ContainerID> DockerContainerizerProcess::parse(
- const Docker::Container& container)
-{
- Option<string> name = None();
-
- if (strings::startsWith(container.name(), DOCKER_NAME_PREFIX)) {
- name = strings::remove(
- container.name(), DOCKER_NAME_PREFIX, strings::PREFIX);
- } else if (strings::startsWith(container.name(), "/" + DOCKER_NAME_PREFIX)) {
- name = strings::remove(
- container.name(), "/" + DOCKER_NAME_PREFIX, strings::PREFIX);
- }
-
- if (name.isSome()) {
- ContainerID id;
- id.set_value(name.get());
- return id;
- }
-
- return None();
-}
-
-
} // namespace slave {
} // namespace internal {
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index f4eb0ff..fbbd45d 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -41,15 +41,11 @@ class DockerContainerizerProcess;
class DockerContainerizer : public Containerizer
{
public:
- static Try<DockerContainerizer*> create(
- const Flags& flags,
- bool local);
-
- static Try<Nothing> prepareCgroups(const Flags& flags);
+ static Try<DockerContainerizer*> create(const Flags& flags);
+ // This is only public for tests.
DockerContainerizer(
const Flags& flags,
- bool local,
const Docker& docker);
virtual ~DockerContainerizer();
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 66bba7c..c348109 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -279,7 +279,7 @@ public:
add(&Flags::docker,
"docker",
- "The path to the docker executable for docker containerizer.",
+ "The absolute path to the docker executable for docker containerizer.",
"docker");
#ifdef WITH_NETWORK_ISOLATOR
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/tests/cgroups_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cgroups_tests.cpp b/src/tests/cgroups_tests.cpp
index 01cf498..1d2fc36 100644
--- a/src/tests/cgroups_tests.cpp
+++ b/src/tests/cgroups_tests.cpp
@@ -288,6 +288,19 @@ TEST_F(CgroupsAnyHierarchyWithCpuMemoryTest, ROOT_CGROUPS_SubsystemsHierarchy)
}
+TEST_F(CgroupsAnyHierarchyWithCpuMemoryTest, ROOT_CGROUPS_FindCgroupSubsystems)
+{
+ pid_t pid = ::getpid();
+ Result<std::string> cpuHierarchy = cgroups::cpu::cgroup(pid);
+ EXPECT_FALSE(cpuHierarchy.isError());
+ EXPECT_SOME(cpuHierarchy);
+
+ Result<std::string> memHierarchy = cgroups::memory::cgroup(pid);
+ EXPECT_FALSE(memHierarchy.isError());
+ EXPECT_SOME(memHierarchy);
+}
+
+
TEST_F(CgroupsNoHierarchyTest, ROOT_CGROUPS_NOHIERARCHY_MountUnmountHierarchy)
{
EXPECT_ERROR(cgroups::mount("/tmp", "cpu"));
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index 84324b0..e936e7e 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -57,15 +57,15 @@ using testing::Return;
class DockerContainerizerTest : public MesosTest
{
public:
- static bool containerExists(
+ static bool exists(
const list<Docker::Container>& containers,
const ContainerID& containerId)
{
- string expectedName = slave::DOCKER_NAME_PREFIX + containerId.value();
+ string expectedName = slave::DOCKER_NAME_PREFIX + stringify(containerId);
foreach (const Docker::Container& container, containers) {
// Docker inspect name contains an extra slash in the beginning.
- if (strings::contains(container.name(), expectedName)) {
+ if (strings::contains(container.name, expectedName)) {
return true;
}
}
@@ -79,9 +79,8 @@ class MockDockerContainerizer : public DockerContainerizer {
public:
MockDockerContainerizer(
const slave::Flags& flags,
- bool local,
const Docker& docker)
- : DockerContainerizer(flags, local, docker)
+ : DockerContainerizer(flags, docker)
{
EXPECT_CALL(*this, launch(_, _, _, _, _, _, _))
.WillRepeatedly(Invoke(this, &MockDockerContainerizer::_launchExecutor));
@@ -188,9 +187,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch_Executor)
slave::Flags flags = CreateSlaveFlags();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker, false).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
Try<PID<Slave> > slave = StartSlave(&dockerContainerizer);
ASSERT_SOME(slave);
@@ -262,7 +261,7 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch_Executor)
AWAIT_READY(containers);
- ASSERT_TRUE(containerExists(containers.get(), containerId.get()));
+ ASSERT_TRUE(exists(containers.get(), containerId.get()));
Future<containerizer::Termination> termination =
dockerContainerizer.wait(containerId.get());
@@ -275,7 +274,7 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch_Executor)
containers = docker.ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
- ASSERT_FALSE(containerExists(containers.get(), containerId.get()));
+ ASSERT_FALSE(exists(containers.get(), containerId.get()));
Shutdown();
}
@@ -289,9 +288,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch)
slave::Flags flags = CreateSlaveFlags();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker, false).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
Try<PID<Slave> > slave = StartSlave(&dockerContainerizer);
ASSERT_SOME(slave);
@@ -358,7 +357,7 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch)
ASSERT_TRUE(containers.get().size() > 0);
- ASSERT_TRUE(containerExists(containers.get(), containerId.get()));
+ ASSERT_TRUE(exists(containers.get(), containerId.get()));
dockerContainerizer.destroy(containerId.get());
@@ -376,9 +375,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Kill)
slave::Flags flags = CreateSlaveFlags();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker, false).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
Try<PID<Slave> > slave = StartSlave(&dockerContainerizer);
ASSERT_SOME(slave);
@@ -456,7 +455,7 @@ TEST_F(DockerContainerizerTest, DOCKER_Kill)
AWAIT_READY(containers);
- ASSERT_FALSE(containerExists(containers.get(), containerId.get()));
+ ASSERT_FALSE(exists(containers.get(), containerId.get()));
driver.stop();
driver.join();
@@ -474,9 +473,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Usage)
slave::Flags flags = CreateSlaveFlags();
flags.resources = Option<string>("cpus:2;mem:1024");
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
Try<PID<Slave> > slave = StartSlave(&dockerContainerizer, flags);
ASSERT_SOME(slave);
@@ -593,9 +592,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Update)
slave::Flags flags = CreateSlaveFlags();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
Try<PID<Slave> > slave = StartSlave(&dockerContainerizer);
ASSERT_SOME(slave);
@@ -676,7 +675,7 @@ TEST_F(DockerContainerizerTest, DOCKER_Update)
ASSERT_SOME(cpuHierarchy);
ASSERT_SOME(memoryHierarchy);
- Option<pid_t> pid = container.get().pid();
+ Option<pid_t> pid = container.get().pid;
ASSERT_SOME(pid);
Result<string> cpuCgroup = cgroups::cpu::cgroup(pid.get());
@@ -715,13 +714,20 @@ TEST_F(DockerContainerizerTest, DOCKER_Update)
#endif //__linux__
-TEST_F(DockerContainerizerTest, DOCKER_Recover)
+// Disabling recover test as the docker rm in recover is async.
+// Even though we wait for the container to finish, when the wait
+// returns docker rm might still be in progress.
+// TODO(tnachen): Re-enable test when we wait for the async kill
+// to finish. One way to do this is to mock the Docker interface
+// and let the mocked docker collect all the remove futures and
+// at the end of the test wait for all of them before the test exits.
+TEST_F(DockerContainerizerTest, DISABLED_DOCKER_Recover)
{
slave::Flags flags = CreateSlaveFlags();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker).get();
- MockDockerContainerizer dockerContainerizer(flags, true, docker);
+ MockDockerContainerizer dockerContainerizer(flags, docker);
ContainerID containerId;
containerId.set_value("c1");
@@ -730,14 +736,14 @@ TEST_F(DockerContainerizerTest, DOCKER_Recover)
Resources resources = Resources::parse("cpus:1;mem:512").get();
- Future<Option<int> > d1 =
+ Future<Nothing> d1 =
docker.run(
"busybox",
"sleep 360",
slave::DOCKER_NAME_PREFIX + stringify(containerId),
resources);
- Future<Option<int> > d2 =
+ Future<Nothing> d2 =
docker.run(
"busybox",
"sleep 360",
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/tests/docker_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_tests.cpp b/src/tests/docker_tests.cpp
index b7a7b6f..1951d9a 100644
--- a/src/tests/docker_tests.cpp
+++ b/src/tests/docker_tests.cpp
@@ -33,43 +33,40 @@
using namespace mesos;
using namespace mesos::internal;
-using process::Future;
+using namespace process;
using std::list;
using std::string;
-// This test tests the functionality of the
-// docker's interfaces.
+// This test tests the functionality of the docker's interfaces.
TEST(DockerTest, DOCKER_interface)
{
string containerName = "mesos-docker-test";
Resources resources = Resources::parse("cpus:1;mem:512").get();
- Docker docker(tests::flags.docker);
+ Docker docker = Docker::create(tests::flags.docker, false).get();
- // Cleaning up the container first.
- Future<Option<int> > status = docker.rm(containerName, true);
- AWAIT_READY(status);
- ASSERT_SOME(status.get());
+ // Cleaning up the container first if it exists.
+ Future<Nothing> status = docker.rm(containerName, true);
+ ASSERT_TRUE(status.await(Seconds(10)));
// Verify that we do not see the container.
- Future<list<Docker::Container> > containers = docker.ps(true);
+ Future<list<Docker::Container> > containers = docker.ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
- EXPECT_NE("/" + containerName, container.name());
+ EXPECT_NE("/" + containerName, container.name);
}
// Start the container.
status = docker.run("busybox", "sleep 120", containerName, resources);
AWAIT_READY(status);
- ASSERT_SOME(status.get());
// Should be able to see the container now.
containers = docker.ps();
AWAIT_READY(containers);
bool found = false;
foreach (const Docker::Container& container, containers.get()) {
- if ("/" + containerName == container.name()) {
+ if ("/" + containerName == container.name) {
found = true;
break;
}
@@ -80,75 +77,70 @@ TEST(DockerTest, DOCKER_interface)
AWAIT_READY(container);
// Test some fields of the container.
- EXPECT_NE("", container.get().id());
- EXPECT_EQ("/" + containerName, container.get().name());
- EXPECT_SOME(container.get().pid());
+ EXPECT_NE("", container.get().id);
+ EXPECT_EQ("/" + containerName, container.get().name);
+ EXPECT_SOME(container.get().pid);
// Kill the container.
status = docker.kill(containerName);
AWAIT_READY(status);
- ASSERT_SOME(status.get());
// Now, the container should not appear in the result of ps().
// But it should appear in the result of ps(true).
containers = docker.ps();
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
- EXPECT_NE("/" + containerName, container.name());
+ EXPECT_NE("/" + containerName, container.name);
}
- containers = docker.ps(true);
+ containers = docker.ps(true, containerName);
AWAIT_READY(containers);
found = false;
foreach (const Docker::Container& container, containers.get()) {
- if ("/" + containerName == container.name()) {
+ if ("/" + containerName == container.name) {
found = true;
break;
}
}
EXPECT_TRUE(found);
- // Check the container's info, both id and name should remain
- // the same since we haven't removed it, but the pid should be none
+ // Check the container's info, both id and name should remain the
+ // same since we haven't removed it, but the pid should be none
// since it's not running.
container = docker.inspect(containerName);
AWAIT_READY(container);
- EXPECT_NE("", container.get().id());
- EXPECT_EQ("/" + containerName, container.get().name());
- EXPECT_NONE(container.get().pid());
+ EXPECT_NE("", container.get().id);
+ EXPECT_EQ("/" + containerName, container.get().name);
+ EXPECT_NONE(container.get().pid);
// Remove the container.
status = docker.rm(containerName);
AWAIT_READY(status);
- ASSERT_SOME(status.get());
// Should not be able to inspect the container.
container = docker.inspect(containerName);
AWAIT_FAILED(container);
- // Also, now we should not be able to see the container
- // by invoking ps(true).
- containers = docker.ps(true);
+ // Also, now we should not be able to see the container by invoking
+ // ps(true).
+ containers = docker.ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
- EXPECT_NE("/" + containerName, container.name());
+ EXPECT_NE("/" + containerName, container.name);
}
// Start the container again, this time we will do a "rm -f"
// directly, instead of killing and rm.
- //
- // First, Invoke docker.run()
status = docker.run("busybox", "sleep 120", containerName, resources);
AWAIT_READY(status);
- ASSERT_SOME(status.get());
// Verify that the container is there.
containers = docker.ps();
AWAIT_READY(containers);
found = false;
foreach (const Docker::Container& container, containers.get()) {
- if ("/" + containerName == container.name()) {
+ if ("/" + containerName == container.name) {
found = true;
break;
}
@@ -158,18 +150,17 @@ TEST(DockerTest, DOCKER_interface)
// Then do a "rm -f".
status = docker.rm(containerName, true);
AWAIT_READY(status);
- ASSERT_SOME(status.get());
- // Verify that the container is totally removed,
- // that is we can't find it by ps() or ps(true).
+ // Verify that the container is totally removed, that is we can't
+ // find it by ps() or ps(true).
containers = docker.ps();
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
- EXPECT_NE("/" + containerName, container.name());
+ EXPECT_NE("/" + containerName, container.name);
}
- containers = docker.ps(true);
+ containers = docker.ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
- EXPECT_NE("/" + containerName, container.name());
+ EXPECT_NE("/" + containerName, container.name);
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/48e7d4a4/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 6c80fa3..eec7d3e 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -29,7 +29,6 @@
#include <process/gmock.hpp>
#include <process/gtest.hpp>
-#include <process/subprocess.hpp>
#include <stout/check.hpp>
#include <stout/error.hpp>
@@ -131,29 +130,30 @@ static bool enable(const ::testing::TestInfo& test)
}
#endif
+ // Filter out benchmark tests when we run 'make check'.
+ if (strings::contains(name, "BENCHMARK_") && !flags.benchmark) {
+ return false;
+ }
+
if (strings::contains(name, "DOCKER_")) {
- Docker docker(flags.docker);
- Try<Nothing> validate = Docker::validate(docker);
- if (validate.isError()) {
+ Try<Docker> docker = Docker::create(flags.docker);
+ if (docker.isError()) {
std::cerr
<< "-------------------------------------------------------------\n"
<< "Skipping Docker tests because validation failed\n"
- << "[Error] " + validate.error() + "\n"
+ << "[Error] " + docker.error() + "\n"
<< "-------------------------------------------------------------"
<< std::endl;
+
+ return false;
}
#ifdef __linux__
- return user.get() == "root" && !validate.isError();
-#else
- return !validate.isError();
+ if (user.get() != "root") {
+ return false;
+ }
#endif
}
-
- // Filter out benchmark tests when we run 'make check'.
- if (strings::contains(name, "BENCHMARK_") && !flags.benchmark) {
- return false;
- }
}
// Filter out regular tests when we run 'make bench', which
@@ -170,7 +170,9 @@ static bool enable(const ::testing::TestInfo& test)
const string& type = test.type_param();
if (strings::contains(type, "Cgroups")) {
#ifdef __linux__
- return user.get() == "root" && cgroups::enabled();
+ if (user.get() != "root" || !cgroups::enabled()) {
+ return false;
+ }
#else
return false;
#endif