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:09:28 UTC
[07/43] git commit: Added Docker unit test,
Docker flag and fix issues found.
Added Docker unit test, Docker flag and fix issues found.
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/286e78bf
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/286e78bf
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/286e78bf
Branch: refs/heads/master
Commit: 286e78bfb607a3ce712deaa74881e0ed3a4c155b
Parents: 9bd535e
Author: Timothy Chen <tn...@gmail.com>
Authored: Wed Jun 25 18:25:51 2014 +0000
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Mon Aug 4 15:08:15 2014 -0700
----------------------------------------------------------------------
src/docker/docker.cpp | 6 +-
src/docker/docker.hpp | 2 +-
src/slave/containerizer/containerizer.cpp | 5 +-
src/slave/containerizer/docker.cpp | 75 ++++++++++++++++++++----
src/slave/containerizer/docker.hpp | 3 +-
src/slave/flags.hpp | 6 ++
src/tests/docker_containerizer_tests.cpp | 81 +++++++++++++++++++++-----
src/tests/environment.cpp | 13 +++--
src/tests/flags.hpp | 7 +++
9 files changed, 160 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 1e3ed71..070279e 100644
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -21,7 +21,7 @@ using std::string;
using std::vector;
-Try<Nothing> Docker::validateDocker(const Docker &docker)
+Try<Nothing> Docker::validate(const Docker &docker)
{
Future<std::string> info = docker.info();
@@ -76,11 +76,11 @@ Future<Option<int> > Docker::run(
const string& command,
const string& name) const
{
- VLOG(1) << "Running " << path << " run --name=" << name << " "
+ VLOG(1) << "Running " << path << " run -d --name=" << name << " "
<< image << " " << command;
Try<Subprocess> s = subprocess(
- path + " run --name=" + name + " " + image + " " + command,
+ path + " run -d --name=" + name + " " + image + " " + command,
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE());
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 080e341..56b6c61 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -34,7 +34,7 @@ class Docker
{
public:
// Validate Docker support
- static Try<Nothing> validateDocker(const Docker& docker);
+ static Try<Nothing> validate(const Docker& docker);
class Container
{
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/slave/containerizer/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/containerizer.cpp b/src/slave/containerizer/containerizer.cpp
index 0978dc2..003775b 100644
--- a/src/slave/containerizer/containerizer.cpp
+++ b/src/slave/containerizer/containerizer.cpp
@@ -171,10 +171,9 @@ Try<Containerizer*> Containerizer::create(const Flags& flags, bool local)
} else {
containerizers.push_back(containerizer.get());
}
- } else if (type == "docker") {
- Docker docker("docker");
+ } else if (type == "docker") {
Try<DockerContainerizer*> containerizer =
- DockerContainerizer::create(flags, local, docker);
+ DockerContainerizer::create(flags, local);
if (containerizer.isError()) {
return Error("Could not create DockerContainerizer: " +
containerizer.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index f9cfa9f..851b663 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -100,7 +100,9 @@ public:
virtual Future<containerizer::Termination> wait(
const ContainerID& containerId);
- virtual void destroy(const ContainerID& containerId);
+ virtual void destroy(
+ const ContainerID& containerId,
+ const bool& killed = true);
virtual process::Future<hashset<ContainerID> > containers();
@@ -118,6 +120,16 @@ private:
const PID<Slave>& slavePid,
bool checkpoint);
+ void _destroy(
+ const ContainerID& containerId,
+ const bool& killed,
+ const Future<Option<int> >& future);
+
+ void __destroy(
+ const ContainerID& containerId,
+ const bool& killed,
+ const Future<Option<int > >& status);
+
// Call back for when the executor exits. This will trigger
// container destroy.
void reaped(const ContainerID& containerId);
@@ -151,10 +163,10 @@ private:
Try<DockerContainerizer*> DockerContainerizer::create(
const Flags& flags,
- bool local,
- const Docker& docker)
+ bool local)
{
- Try<Nothing> validation = Docker::validateDocker(docker);
+ Docker docker(flags.docker);
+ Try<Nothing> validation = Docker::validate(docker);
if (validation.isError()) {
return Error(validation.error());
}
@@ -259,7 +271,7 @@ Future<containerizer::Termination> DockerContainerizer::wait(
void DockerContainerizer::destroy(const ContainerID& containerId)
{
- dispatch(process, &DockerContainerizerProcess::destroy, containerId);
+ dispatch(process, &DockerContainerizerProcess::destroy, containerId, true);
}
@@ -499,7 +511,7 @@ Future<bool> DockerContainerizerProcess::launch(
slaveId,
slavePid,
checkpoint))
- .onFailed(defer(self(), &Self::destroy, containerId));
+ .onFailed(defer(self(), &Self::destroy, containerId, false));
}
@@ -535,11 +547,10 @@ Future<bool> DockerContainerizerProcess::_launch(
"docker wait " + DOCKER_NAME_PREFIX + stringify(containerId);
Try<Subprocess> s = subprocess(
- executorInfo.command().value() + "--override " + override,
+ executorInfo.command().value() + " --override " + override,
Subprocess::PIPE(),
Subprocess::PATH(path::join(directory, "stdout")),
Subprocess::PATH(path::join(directory, "stderr")),
- None(),
env,
lambda::bind(&setup, directory));
@@ -629,7 +640,9 @@ Future<containerizer::Termination> DockerContainerizerProcess::wait(
}
-void DockerContainerizerProcess::destroy(const ContainerID& containerId)
+void DockerContainerizerProcess::destroy(
+ const ContainerID& containerId,
+ const bool& killed)
{
if (!promises.contains(containerId)) {
LOG(WARNING) << "Ignoring destroy of unknown container: " << containerId;
@@ -658,7 +671,47 @@ void DockerContainerizerProcess::destroy(const ContainerID& containerId)
// TODO(benh): Retry 'docker kill' if it failed but the container
// still exists (asynchronously).
- docker.kill(DOCKER_NAME_PREFIX + stringify(containerId));
+ docker.kill(DOCKER_NAME_PREFIX + stringify(containerId))
+ .onAny(defer(self(), &Self::_destroy, containerId, killed, lambda::_1));
+}
+
+
+void DockerContainerizerProcess::_destroy(
+ const ContainerID& containerId,
+ const bool& killed,
+ const Future<Option<int> >& future)
+{
+ if (!future.isReady()) {
+ promises[containerId]->fail(
+ "Failed to destroy container: " +
+ (future.isFailed() ? future.failure() : "discarded future"));
+
+ destroying.erase(containerId);
+
+ return;
+ }
+
+ statuses.get(containerId).get()
+ .onAny(defer(self(), &Self::__destroy, containerId, killed, lambda::_1));
+}
+
+
+void DockerContainerizerProcess::__destroy(
+ const ContainerID& containerId,
+ const bool& killed,
+ const Future<Option<int> >& status)
+{
+ containerizer::Termination termination;
+ termination.set_killed(killed);
+ if (status.isReady() && status.get().isSome()) {
+ termination.set_status(status.get().get());
+ }
+
+ promises[containerId]->set(termination);
+
+ destroying.erase(containerId);
+ promises.erase(containerId);
+ statuses.erase(containerId);
}
@@ -677,7 +730,7 @@ void DockerContainerizerProcess::reaped(const ContainerID& containerId)
LOG(INFO) << "Executor for container '" << containerId << "' has exited";
// The executor has exited so destroy the container.
- destroy(containerId);
+ destroy(containerId, false);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index 0ecbc88..a6411e8 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -37,8 +37,7 @@ class DockerContainerizer : public Containerizer
public:
static Try<DockerContainerizer*> create(
const Flags& flags,
- bool local,
- const Docker& docker);
+ bool local);
DockerContainerizer(
const Flags& flags,
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 5ea2f38..66bba7c 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -277,6 +277,11 @@ public:
"The default container image to use if not specified by a task,\n"
"when using external containerizer");
+ add(&Flags::docker,
+ "docker",
+ "The path to the docker executable for docker containerizer.",
+ "docker");
+
#ifdef WITH_NETWORK_ISOLATOR
add(&Flags::ephemeral_ports_per_container,
"ephemeral_ports_per_container",
@@ -342,6 +347,7 @@ public:
Option<std::string> containerizer_path;
std::string containerizers;
Option<std::string> default_container_image;
+ std::string docker;
#ifdef WITH_NETWORK_ISOLATOR
uint16_t ephemeral_ports_per_container;
Option<std::string> private_resources;
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index 8cbde0b..636187f 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -33,28 +33,68 @@ using namespace mesos::internal::tests;
using mesos::internal::master::Master;
using mesos::internal::slave::Slave;
+using mesos::internal::slave::DockerContainerizer;
using process::Future;
using process::PID;
using std::vector;
using std::list;
+using std::string;
using testing::_;
+using testing::DoDefault;
using testing::Eq;
using testing::Return;
class DockerContainerizerTest : public MesosTest {};
-TEST_F(DockerContainerizerTest, DOCKER_Launch) {
+class MockDockerContainerizer : public slave::DockerContainerizer {
+public:
+ MockDockerContainerizer(
+ const slave::Flags& flags,
+ bool local,
+ const Docker& docker) : DockerContainerizer(flags, local, docker) {}
+
+ process::Future<bool> launch(
+ const ContainerID& containerId,
+ const TaskInfo& taskInfo,
+ const ExecutorInfo& executorInfo,
+ const std::string& directory,
+ const Option<std::string>& user,
+ const SlaveID& slaveId,
+ const process::PID<Slave>& slavePid,
+ bool checkpoint)
+ {
+ // Keeping the last launched container id
+ lastContainerId = containerId;
+ return slave::DockerContainerizer::launch(
+ containerId,
+ taskInfo,
+ executorInfo,
+ directory,
+ user,
+ slaveId,
+ slavePid,
+ checkpoint);
+ }
+
+ ContainerID lastContainerId;
+};
+
+
+TEST_F(DockerContainerizerTest, DOCKER_Launch)
+{
Try<PID<Master> > master = StartMaster();
ASSERT_SOME(master);
slave::Flags flags = CreateSlaveFlags();
- flags.isolation.clear();
- flags.containerizers = "docker";
- Try<PID<Slave> > slave = StartSlave(flags);
+ Docker docker("docker");
+
+ MockDockerContainerizer dockerContainer(flags, true, docker);
+
+ Try<PID<Slave> > slave = StartSlave((slave::Containerizer*) &dockerContainer);
ASSERT_SOME(slave);
MockScheduler sched;
@@ -72,6 +112,8 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch) {
driver.start();
+ AWAIT_READY(frameworkId);
+
AWAIT_READY(offers);
EXPECT_NE(0u, offers.get().size());
@@ -84,13 +126,9 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch) {
task.mutable_resources()->CopyFrom(offer.resources());
CommandInfo command;
-
- CommandInfo::ContainerInfo* containerInfo =
- task.mutable_command()->mutable_container();
-
+ CommandInfo::ContainerInfo* containerInfo = command.mutable_container();
containerInfo->set_image("docker://busybox");
-
- command.set_value("sleep 30");
+ command.set_value("sleep 120");
task.mutable_command()->CopyFrom(command);
@@ -100,20 +138,35 @@ TEST_F(DockerContainerizerTest, DOCKER_Launch) {
tasks.push_back(task);
EXPECT_CALL(sched, statusUpdate(&driver, _))
- .WillOnce(FutureArg<1>(&statusRunning));
+ .WillOnce(FutureArg<1>(&statusRunning))
+ .WillRepeatedly(DoDefault());
driver.launchTasks(offers.get()[0].id(), tasks);
- AWAIT_READY(statusRunning);
- ASSERT_EQ(TASK_RUNNING, statusRunning.get().state());
+ AWAIT_READY_FOR(statusRunning, Seconds(60));
+ EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
- Docker docker("docker");
Future<list<Docker::Container> > containers = docker.ps();
AWAIT_READY(containers);
ASSERT_TRUE(containers.get().size() > 0);
+ bool foundContainer = false;
+ string expectedName = "mesos-" + dockerContainer.lastContainerId.value();
+
+ foreach(const Docker::Container& container, containers.get()) {
+ // Docker inspect name contains an extra slash in the beginning
+ if (strings::contains(container.name(), expectedName)) {
+ foundContainer = true;
+ break;
+ }
+ }
+
+ ASSERT_TRUE(foundContainer);
+
+ AWAIT_READY(docker.kill(expectedName));
+
driver.stop();
driver.join();
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 11a08e3..b75c485 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -131,17 +131,22 @@ static bool enable(const ::testing::TestInfo& test)
#endif
if (strings::contains(name, "DOCKER_")) {
- Docker docker("docker");
- Try<Nothing> validate = Docker::validateDocker(docker);
+ Docker docker(flags.docker);
+ Try<Nothing> validate = Docker::validate(docker);
if (validate.isError()) {
- std::cerr
+ std::cerr
<< "-------------------------------------------------------------\n"
<< "Skipping Docker tests because validation failed\n"
- << "[Error] " + validate.error() + "\n"
+ << "[Error] " + validate.error() + "\n"
<< "-------------------------------------------------------------"
<< std::endl;
}
+
+#ifdef __linux__
+ return user.get() == "root" && !validate.isError();
+#else
return !validate.isError();
+#endif
}
// Filter out benchmark tests when we run 'make check'.
http://git-wip-us.apache.org/repos/asf/mesos/blob/286e78bf/src/tests/flags.hpp
----------------------------------------------------------------------
diff --git a/src/tests/flags.hpp b/src/tests/flags.hpp
index a003e7f..189fad9 100644
--- a/src/tests/flags.hpp
+++ b/src/tests/flags.hpp
@@ -61,16 +61,23 @@ public:
path = os::realpath(BUILD_DIR);
CHECK_SOME(path);
+
add(&Flags::build_dir,
"build_dir",
"Where to find the build directory",
path.get());
+
+ add(&Flags::docker,
+ "docker",
+ "Where to find docker executable",
+ "docker");
}
bool verbose;
bool benchmark;
std::string source_dir;
std::string build_dir;
+ std::string docker;
};
// Global flags for running the tests.