You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by tn...@apache.org on 2014/10/29 00:56:15 UTC
[3/8] git commit: Fix docker flaky tests
Fix docker flaky tests
Conflicts:
src/slave/containerizer/docker.cpp
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e4e3ef13
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e4e3ef13
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e4e3ef13
Branch: refs/heads/docker_symlink
Commit: e4e3ef137a9a5be0404361b68dceed0ff0bc9c1c
Parents: ac38558
Author: Timothy Chen <tn...@gmail.com>
Authored: Tue Oct 14 17:31:25 2014 +0000
Committer: Timothy Chen <tn...@gmail.com>
Committed: Tue Oct 28 23:02:34 2014 +0000
----------------------------------------------------------------------
src/docker/docker.cpp | 15 +-
src/docker/docker.hpp | 24 +-
src/slave/containerizer/docker.cpp | 43 ++-
src/slave/containerizer/docker.hpp | 2 +-
src/tests/docker_containerizer_tests.cpp | 360 +++++++++++++++++++++++---
src/tests/docker_tests.cpp | 57 ++--
src/tests/environment.cpp | 2 +-
7 files changed, 391 insertions(+), 112 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 9973782..d423d44 100644
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -53,6 +53,8 @@ using std::string;
using std::vector;
+Nothing _nothing() { return Nothing(); }
+
template <typename T>
static Future<T> failure(
const string& cmd,
@@ -92,10 +94,10 @@ static Future<Nothing> checkError(const string& cmd, const Subprocess& s)
}
-Try<Docker> Docker::create(const string& path, bool validate)
+Try<Docker*> Docker::create(const string& path, bool validate)
{
if (!validate) {
- return Docker(path);
+ return new Docker(path);
}
#ifdef __linux__
@@ -165,7 +167,7 @@ Try<Docker> Docker::create(const string& path, bool validate)
} else if (major.get() < 1) {
break;
}
- return Docker(path);
+ return new Docker(path);
}
}
@@ -613,7 +615,7 @@ Future<Docker::Container> Docker::__inspect(const string& output)
Future<Nothing> Docker::logs(
const std::string& container,
- const std::string& directory)
+ const std::string& directory) const
{
// Redirect the logs into stdout/stderr.
//
@@ -638,7 +640,7 @@ Future<Nothing> Docker::logs(
" " + path + " logs --follow $1 &\n"
" pid=$!\n"
" " + path + " wait $1 >/dev/null 2>&1\n"
- " sleep 10" // Sleep 10 seconds to make sure the logs are flushed.
+ " sleep 10\n" // Sleep 10 seconds to make sure the logs are flushed.
" kill -TERM $pid >/dev/null 2>&1 &\n"
"}\n"
"logs " + container;
@@ -655,7 +657,8 @@ Future<Nothing> Docker::logs(
return Failure("Unable to launch docker logs: " + s.error());
}
- return Nothing();
+ return s.get().status()
+ .then(lambda::bind(&_nothing));
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 9656f15..b6b28f4 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -40,7 +40,10 @@ class Docker
{
public:
// Create Docker abstraction and optionally validate docker.
- static Try<Docker> create(const std::string& path, bool validate = true);
+ static Try<Docker*> create(
+ const std::string& path, bool validate = true);
+
+ virtual ~Docker() {}
class Container
{
@@ -79,7 +82,7 @@ public:
// Performs 'docker run IMAGE'.
- process::Future<Nothing> run(
+ virtual process::Future<Nothing> run(
const mesos::ContainerInfo& containerInfo,
const mesos::CommandInfo& commandInfo,
const std::string& name,
@@ -90,21 +93,21 @@ public:
// 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(
+ virtual process::Future<Nothing> kill(
const std::string& container,
bool remove = false) const;
// Performs 'docker rm (-f) CONTAINER'.
- process::Future<Nothing> rm(
+ virtual process::Future<Nothing> rm(
const std::string& container,
bool force = false) const;
// Performs 'docker inspect CONTAINER'.
- process::Future<Container> inspect(
+ virtual process::Future<Container> inspect(
const std::string& container) const;
// Performs 'docker ps (-a)'.
- process::Future<std::list<Container> > ps(
+ virtual process::Future<std::list<Container> > ps(
bool all = false,
const Option<std::string>& prefix = None()) const;
@@ -113,18 +116,19 @@ public:
//
// TODO(benh): Return the file descriptors, or some struct around
// them so others can do what they want with stdout/stderr.
- process::Future<Nothing> logs(
+ virtual process::Future<Nothing> logs(
const std::string& container,
- const std::string& directory);
+ const std::string& directory) const;
- process::Future<Image> pull(
+ virtual process::Future<Image> pull(
const std::string& directory,
const std::string& image) const;
-private:
+protected:
// Uses the specified path to the Docker CLI tool.
Docker(const std::string& _path) : path(_path) {};
+private:
static process::Future<Nothing> _kill(
const Docker& docker,
const std::string& container,
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index eb55ba9..12ca9f1 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -25,6 +25,7 @@
#include <process/defer.hpp>
#include <process/delay.hpp>
#include <process/io.hpp>
+#include <process/owned.hpp>
#include <process/reap.hpp>
#include <process/subprocess.hpp>
@@ -80,7 +81,7 @@ class DockerContainerizerProcess
public:
DockerContainerizerProcess(
const Flags& _flags,
- const Docker& _docker)
+ memory::shared_ptr<Docker> _docker)
: flags(_flags),
docker(_docker) {}
@@ -210,7 +211,7 @@ private:
const Flags flags;
- Docker docker;
+ memory::shared_ptr<Docker> docker;
struct Container
{
@@ -405,18 +406,19 @@ Option<ContainerID> parse(const Docker::Container& container)
Try<DockerContainerizer*> DockerContainerizer::create(const Flags& flags)
{
- Try<Docker> docker = Docker::create(flags.docker);
+ Try<Docker*> docker = Docker::create(flags.docker);
if (docker.isError()) {
return Error(docker.error());
}
- return new DockerContainerizer(flags, docker.get());
+ return new DockerContainerizer(
+ flags, memory::shared_ptr<Docker>(docker.get()));
}
DockerContainerizer::DockerContainerizer(
const Flags& flags,
- const Docker& docker)
+ memory::shared_ptr<Docker> docker)
{
process = new DockerContainerizerProcess(flags, docker);
spawn(process);
@@ -546,7 +548,7 @@ Future<Nothing> DockerContainerizerProcess::pull(
const string& directory,
const string& image)
{
- Future<Docker::Image> future = docker.pull(directory, image);
+ Future<Docker::Image> future = docker->pull(directory, image);
containers_[containerId]->pull = future;
return future.then(defer(self(), &Self::_pull, image));
}
@@ -793,7 +795,7 @@ Future<Nothing> DockerContainerizerProcess::recover(
// Get the list of all Docker containers (running and exited) in
// order to remove any orphans.
- return docker.ps(true, DOCKER_NAME_PREFIX)
+ return docker->ps(true, DOCKER_NAME_PREFIX)
.then(defer(self(), &Self::_recover, lambda::_1));
}
@@ -820,7 +822,7 @@ Future<Nothing> DockerContainerizerProcess::_recover(
if (!containers_.contains(id.get())) {
// TODO(benh): Retry 'docker rm -f' if it failed but the container
// still exists (asynchronously).
- docker.kill(container.id, true);
+ docker->kill(container.id, true);
}
}
@@ -914,7 +916,7 @@ Future<Nothing> DockerContainerizerProcess::__launch(
container->state = Container::RUNNING;
// Try and start the Docker container.
- return container->run = docker.run(
+ return container->run = docker->run(
container->container(),
container->command(),
container->name(),
@@ -1064,7 +1066,7 @@ Future<Docker::Container> DockerContainerizerProcess::____launch(
Container* container = containers_[containerId];
- return docker.inspect(container->name());
+ return docker->inspect(container->name());
}
@@ -1116,7 +1118,7 @@ Future<bool> DockerContainerizerProcess::______launch(
.onAny(defer(self(), &Self::reaped, containerId));
// TODO(benh): Check failure of Docker::logs.
- docker.logs(container->name(), container->directory);
+ docker->logs(container->name(), container->directory);
return true;
}
@@ -1154,7 +1156,7 @@ Future<Nothing> DockerContainerizerProcess::update(
return __update(containerId, _resources, container->pid.get());
}
- return docker.inspect(containers_[containerId]->name())
+ return docker->inspect(containers_[containerId]->name())
.then(defer(self(), &Self::_update, containerId, _resources, lambda::_1));
#else
return Nothing();
@@ -1330,7 +1332,7 @@ Future<ResourceStatistics> DockerContainerizerProcess::usage(
return Failure("Container is being removed: " + stringify(containerId));
}
- return docker.inspect(container->name())
+ return docker->inspect(container->name())
.then(defer(self(), &Self::_usage, containerId, lambda::_1));
#endif // __linux__
}
@@ -1525,17 +1527,8 @@ void DockerContainerizerProcess::_destroy(
// still exists (asynchronously).
LOG(INFO) << "Running docker kill on container '" << containerId << "'";
-
- if (killed) {
- docker.kill(container->name(), false)
- .onAny(defer(self(), &Self::__destroy, containerId, killed, lambda::_1));
- } else {
- // If the container exited normally, skip docker kill so logs can
- // still finish forwarding from the container. This is due to
- // a docker bug that is sometimes not writing out stdout output
- //if kill/stop is called on an already exited container.
- __destroy(containerId, killed, Nothing());
- }
+ docker->kill(container->name(), false)
+ .onAny(defer(self(), &Self::__destroy, containerId, killed, lambda::_1));
}
@@ -1625,7 +1618,7 @@ void DockerContainerizerProcess::reaped(const ContainerID& containerId)
void DockerContainerizerProcess::cleanup(const string& container)
{
- docker.rm(container, true);
+ docker->rm(container, true);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index fbbd45d..1aae84f 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -46,7 +46,7 @@ public:
// This is only public for tests.
DockerContainerizer(
const Flags& flags,
- const Docker& docker);
+ memory::shared_ptr<Docker> docker);
virtual ~DockerContainerizer();
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index d99e567..ba24307 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -21,6 +21,7 @@
#include <process/future.hpp>
#include <process/gmock.hpp>
+#include <process/owned.hpp>
#include <process/subprocess.hpp>
#include "linux/cgroups.hpp"
@@ -41,6 +42,8 @@ using namespace mesos::internal;
using namespace mesos::internal::slave::state;
using namespace mesos::internal::tests;
+using namespace process;
+
using mesos::internal::master::Master;
using mesos::internal::slave::Slave;
@@ -61,6 +64,43 @@ using testing::Eq;
using testing::Invoke;
using testing::Return;
+
+class MockDocker : public Docker
+{
+public:
+ MockDocker(const string& path) : Docker(path)
+ {
+ EXPECT_CALL(*this, logs(_, _))
+ .WillRepeatedly(Invoke(this, &MockDocker::_logs));
+
+ EXPECT_CALL(*this, kill(_, _))
+ .WillRepeatedly(Invoke(this, &MockDocker::_kill));
+ }
+
+ MOCK_CONST_METHOD2(
+ logs,
+ process::Future<Nothing>(
+ const string&,
+ const string&));
+
+ MOCK_CONST_METHOD2(kill, process::Future<Nothing>(const string&, bool));
+
+ process::Future<Nothing> _logs(
+ const string& container,
+ const string& directory) const
+ {
+ return Docker::logs(container, directory);
+ }
+
+ process::Future<Nothing> _kill(
+ const string& container,
+ bool remove) const
+ {
+ return Docker::kill(container, remove);
+ }
+};
+
+
class DockerContainerizerTest : public MesosTest
{
public:
@@ -79,6 +119,38 @@ public:
return false;
}
+
+
+ static bool running(
+ const list<Docker::Container>& containers,
+ const ContainerID& containerId)
+ {
+ 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)) {
+ return container.pid.isSome();
+ }
+ }
+
+ return false;
+ }
+
+
+ virtual void TearDown()
+ {
+ MockDocker docker(tests::flags.docker);
+ Future<list<Docker::Container>> containers =
+ docker.ps(true, slave::DOCKER_NAME_PREFIX);
+
+ AWAIT_READY(containers);
+
+ // Cleanup all mesos launched containers.
+ foreach (const Docker::Container& container, containers.get()) {
+ AWAIT_READY_FOR(docker.rm(container.id, true), Seconds(30));
+ }
+ }
};
@@ -86,9 +158,12 @@ class MockDockerContainerizer : public DockerContainerizer {
public:
MockDockerContainerizer(
const slave::Flags& flags,
- const Docker& docker)
+ memory::shared_ptr<Docker> docker)
: DockerContainerizer(flags, docker)
{
+ // NOTE: See TestContainerizer::setup for why we use
+ // 'EXPECT_CALL' and 'WillRepeatedly' here instead of
+ // 'ON_CALL' and 'WillByDefault'.
EXPECT_CALL(*this, launch(_, _, _, _, _, _, _))
.WillRepeatedly(Invoke(this, &MockDockerContainerizer::_launchExecutor));
@@ -190,9 +265,18 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor)
Try<PID<Master> > master = StartMaster();
ASSERT_SOME(master);
- slave::Flags flags = CreateSlaveFlags();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -270,8 +354,8 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
@@ -285,10 +369,14 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor)
AWAIT_READY(termination);
- containers = docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ containers = docker->ps(true, slave::DOCKER_NAME_PREFIX);
+
AWAIT_READY(containers);
- ASSERT_FALSE(exists(containers.get(), containerId.get()));
+ ASSERT_FALSE(running(containers.get(), containerId.get()));
+
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
Shutdown();
}
@@ -304,9 +392,18 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor_Bridged)
Try<PID<Master> > master = StartMaster();
ASSERT_SOME(master);
- slave::Flags flags = CreateSlaveFlags();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -385,8 +482,8 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor_Bridged)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
@@ -400,10 +497,13 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch_Executor_Bridged)
AWAIT_READY(termination);
- containers = docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ containers = docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
- ASSERT_FALSE(exists(containers.get(), containerId.get()));
+ ASSERT_FALSE(running(containers.get(), containerId.get()));
+
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
Shutdown();
}
@@ -415,9 +515,18 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch)
Try<PID<Master> > master = StartMaster();
ASSERT_SOME(master);
- slave::Flags flags = CreateSlaveFlags();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -485,8 +594,8 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch)
AWAIT_READY_FOR(statusRunning, Seconds(60));
EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
@@ -494,9 +603,21 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch)
ASSERT_TRUE(exists(containers.get(), containerId.get()));
+ Future<containerizer::Termination> termination =
+ dockerContainerizer.wait(containerId.get());
+
driver.stop();
driver.join();
+ AWAIT_READY(termination);
+
+ containers = docker->ps(true, slave::DOCKER_NAME_PREFIX);
+
+ ASSERT_FALSE(running(containers.get(), containerId.get()));
+
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Shutdown();
}
@@ -506,9 +627,18 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Kill)
Try<PID<Master> > master = StartMaster();
ASSERT_SOME(master);
- slave::Flags flags = CreateSlaveFlags();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -589,16 +719,19 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Kill)
AWAIT_READY(termination);
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
- ASSERT_FALSE(exists(containers.get(), containerId.get()));
+ ASSERT_FALSE(running(containers.get(), containerId.get()));
driver.stop();
driver.join();
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Shutdown();
}
@@ -612,7 +745,16 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Usage)
slave::Flags flags = CreateSlaveFlags();
flags.resources = Option<string>("cpus:2;mem:1024");
- Docker docker = Docker::create(tests::flags.docker).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -719,11 +861,15 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Usage)
// Usage() should fail again since the container is destroyed.
Future<ResourceStatistics> usage =
dockerContainerizer.usage(containerId.get());
+
AWAIT_FAILED(usage);
driver.stop();
driver.join();
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Shutdown();
}
@@ -736,7 +882,16 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Update)
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -806,7 +961,7 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Update)
EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
string containerName = slave::DOCKER_NAME_PREFIX + containerId.get().value();
- Future<Docker::Container> container = docker.inspect(containerName);
+ Future<Docker::Container> container = docker->inspect(containerName);
AWAIT_READY(container);
@@ -872,6 +1027,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Update)
driver.stop();
driver.join();
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Shutdown();
}
#endif //__linux__
@@ -888,7 +1046,8 @@ TEST_F(DockerContainerizerTest, DISABLED_ROOT_DOCKER_Recover)
{
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -910,7 +1069,7 @@ TEST_F(DockerContainerizerTest, DISABLED_ROOT_DOCKER_Recover)
commandInfo.set_value("sleep 1000");
Future<Nothing> d1 =
- docker.run(
+ docker->run(
containerInfo,
commandInfo,
slave::DOCKER_NAME_PREFIX + stringify(containerId),
@@ -919,7 +1078,7 @@ TEST_F(DockerContainerizerTest, DISABLED_ROOT_DOCKER_Recover)
resources);
Future<Nothing> d2 =
- docker.run(
+ docker->run(
containerInfo,
commandInfo,
slave::DOCKER_NAME_PREFIX + stringify(reapedContainerId),
@@ -983,6 +1142,8 @@ TEST_F(DockerContainerizerTest, DISABLED_ROOT_DOCKER_Recover)
AWAIT_READY(termination);
AWAIT_READY(reaped.get().status());
+
+ Shutdown();
}
@@ -993,7 +1154,22 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Logs)
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
+
+ // We skip killing the docker container because killing a container
+ // even when it terminated might not flush the logs and we end up
+ // not getting stdout/stderr in our tests.
+ EXPECT_CALL(*mockDocker, kill(_, _))
+ .WillRepeatedly(Return(Nothing()));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -1070,6 +1246,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Logs)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
// Now check that the proper output is in stderr and stdout (which
// might also contain other things, hence the use of a UUID).
Try<string> read = os::read(path::join(directory.get(), "stderr"));
@@ -1100,7 +1279,22 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD)
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
+
+ // We skip killing the docker container because killing a container
+ // even when it terminated might not flush the logs and we end up
+ // not getting stdout/stderr in our tests.
+ EXPECT_CALL(*mockDocker, kill(_, _))
+ .WillRepeatedly(Return(Nothing()));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -1179,6 +1373,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Try<string> read = os::read(path::join(directory.get(), "stdout"));
ASSERT_SOME(read);
@@ -1208,7 +1405,22 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Override)
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
+
+ // We skip killing the docker container because killing a container
+ // even when it terminated might not flush the logs and we end up
+ // not getting stdout/stderr in our tests.
+ EXPECT_CALL(*mockDocker, kill(_, _))
+ .WillRepeatedly(Return(Nothing()));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -1289,6 +1501,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Override)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
// Now check that the proper output is in stderr and stdout.
Try<string> read = os::read(path::join(directory.get(), "stdout"));
@@ -1321,7 +1536,22 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Args)
slave::Flags flags = CreateSlaveFlags();
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
+
+ // We skip killing the docker container because killing a container
+ // even when it terminated might not flush the logs and we end up
+ // not getting stdout/stderr in our tests.
+ EXPECT_CALL(*mockDocker, kill(_, _))
+ .WillRepeatedly(Return(Nothing()));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -1403,6 +1633,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Args)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
// Now check that the proper output is in stderr and stdout.
Try<string> read = os::read(path::join(directory.get(), "stdout"));
@@ -1441,7 +1674,16 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_SlaveRecoveryTaskContainer)
flags.recover = "reconnect";
flags.strict = true;
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
// We put the containerizer on the heap so we can more easily
// control it's lifetime, i.e., when we invoke the destructor.
@@ -1553,16 +1795,24 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_SlaveRecoveryTaskContainer)
ASSERT_EQ(TASK_RUNNING, status.get().state());
// Make sure the container is still running.
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
ASSERT_TRUE(exists(containers.get(), containerId.get()));
+ Future<containerizer::Termination> termination =
+ dockerContainerizer2->wait(containerId.get());
+
driver.stop();
driver.join();
+ AWAIT_READY(termination);
+
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
Shutdown();
delete dockerContainerizer2;
@@ -1594,7 +1844,16 @@ TEST_F(DockerContainerizerTest,
flags.recover = "reconnect";
flags.strict = true;
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
MockDockerContainerizer* dockerContainerizer1 =
new MockDockerContainerizer(flags, docker);
@@ -1731,8 +1990,8 @@ TEST_F(DockerContainerizerTest,
ASSERT_EQ(TASK_RUNNING, status.get().state());
// Make sure the container is still running.
- Future<list<Docker::Container> > containers =
- docker.ps(true, slave::DOCKER_NAME_PREFIX);
+ Future<list<Docker::Container>> containers =
+ docker->ps(true, slave::DOCKER_NAME_PREFIX);
AWAIT_READY(containers);
@@ -1741,7 +2000,8 @@ TEST_F(DockerContainerizerTest,
driver.stop();
driver.join();
- Shutdown();
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
delete dockerContainerizer2;
}
@@ -1760,7 +2020,22 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_PortMapping)
flags.resources = "cpus:1;mem:1024;ports:[10000-10000]";
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ memory::shared_ptr<Docker> docker(mockDocker);
+
+ // We need to capture and await on the logs process's future so that
+ // we can ensure there is no child process at the end of the test.
+ // The logs future is being awaited at teardown.
+ Future<Nothing> logs;
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(FutureResult(
+ &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs)));
+
+ // We skip killing the docker container because killing a container
+ // even when it terminated might not flush the logs and we end up
+ // not getting stdout/stderr in our tests.
+ EXPECT_CALL(*mockDocker, kill(_, _))
+ .WillRepeatedly(Return(Nothing()));
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -1856,6 +2131,9 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_PortMapping)
AWAIT_READY_FOR(statusFinished, Seconds(60));
EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+ // See above where we assign logs future for more comments.
+ AWAIT_READY_FOR(logs, Seconds(30));
+
// Now check that the proper output is in stdout.
Try<string> read = os::read(path::join(directory.get(), "stdout"));
@@ -1863,7 +2141,7 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_PortMapping)
// We expect the uuid that is sent to host port to be written
// to stdout by the docker container running nc -l.
- EXPECT_TRUE(strings::contains(read.get(), uuid));
+ ASSERT_TRUE(strings::contains(read.get(), uuid));
driver.stop();
driver.join();
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/tests/docker_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_tests.cpp b/src/tests/docker_tests.cpp
index 04139af..ff06a01 100644
--- a/src/tests/docker_tests.cpp
+++ b/src/tests/docker_tests.cpp
@@ -48,14 +48,15 @@ TEST(DockerTest, ROOT_DOCKER_interface)
{
string containerName = "mesos-docker-test";
Resources resources = Resources::parse("cpus:1;mem:512").get();
- Docker docker = Docker::create(tests::flags.docker, false).get();
+
+ Owned<Docker> docker(Docker::create(tests::flags.docker, false).get());
// Cleaning up the container first if it exists.
- Future<Nothing> status = docker.rm(containerName, true);
+ 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, containerName);
+ Future<list<Docker::Container> > containers = docker->ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
EXPECT_NE("/" + containerName, container.name);
@@ -75,7 +76,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
commandInfo.set_value("sleep 120");
// Start the container.
- status = docker.run(
+ status = docker->run(
containerInfo,
commandInfo,
containerName,
@@ -86,7 +87,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
AWAIT_READY(status);
// Should be able to see the container now.
- containers = docker.ps();
+ containers = docker->ps();
AWAIT_READY(containers);
bool found = false;
foreach (const Docker::Container& container, containers.get()) {
@@ -97,7 +98,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
}
EXPECT_TRUE(found);
- Future<Docker::Container> container = docker.inspect(containerName);
+ Future<Docker::Container> container = docker->inspect(containerName);
AWAIT_READY(container);
// Test some fields of the container.
@@ -106,18 +107,18 @@ TEST(DockerTest, ROOT_DOCKER_interface)
EXPECT_SOME(container.get().pid);
// Kill the container.
- status = docker.kill(containerName);
+ status = docker->kill(containerName);
AWAIT_READY(status);
// Now, the container should not appear in the result of ps().
// But it should appear in the result of ps(true).
- containers = docker.ps();
+ containers = docker->ps();
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
EXPECT_NE("/" + containerName, container.name);
}
- containers = docker.ps(true, containerName);
+ containers = docker->ps(true, containerName);
AWAIT_READY(containers);
found = false;
foreach (const Docker::Container& container, containers.get()) {
@@ -131,7 +132,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
// 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);
+ container = docker->inspect(containerName);
AWAIT_READY(container);
EXPECT_NE("", container.get().id);
@@ -139,16 +140,16 @@ TEST(DockerTest, ROOT_DOCKER_interface)
EXPECT_NONE(container.get().pid);
// Remove the container.
- status = docker.rm(containerName);
+ status = docker->rm(containerName);
AWAIT_READY(status);
// Should not be able to inspect the container.
- container = docker.inspect(containerName);
+ 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, containerName);
+ containers = docker->ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
EXPECT_NE("/" + containerName, container.name);
@@ -156,7 +157,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
// Start the container again, this time we will do a "rm -f"
// directly, instead of killing and rm.
- status = docker.run(
+ status = docker->run(
containerInfo,
commandInfo,
containerName,
@@ -167,7 +168,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
AWAIT_READY(status);
// Verify that the container is there.
- containers = docker.ps();
+ containers = docker->ps();
AWAIT_READY(containers);
found = false;
foreach (const Docker::Container& container, containers.get()) {
@@ -179,17 +180,17 @@ TEST(DockerTest, ROOT_DOCKER_interface)
EXPECT_TRUE(found);
// Then do a "rm -f".
- status = docker.rm(containerName, true);
+ status = docker->rm(containerName, true);
AWAIT_READY(status);
// Verify that the container is totally removed, that is we can't
// find it by ps() or ps(true).
- containers = docker.ps();
+ containers = docker->ps();
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
EXPECT_NE("/" + containerName, container.name);
}
- containers = docker.ps(true, containerName);
+ containers = docker->ps(true, containerName);
AWAIT_READY(containers);
foreach (const Docker::Container& container, containers.get()) {
EXPECT_NE("/" + containerName, container.name);
@@ -199,7 +200,7 @@ TEST(DockerTest, ROOT_DOCKER_interface)
TEST(DockerTest, ROOT_DOCKER_CheckCommandWithShell)
{
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ Owned<Docker> docker(Docker::create(tests::flags.docker, false).get());
ContainerInfo containerInfo;
containerInfo.set_type(ContainerInfo::DOCKER);
@@ -211,7 +212,7 @@ TEST(DockerTest, ROOT_DOCKER_CheckCommandWithShell)
CommandInfo commandInfo;
commandInfo.set_shell(true);
- Future<Nothing> run = docker.run(
+ Future<Nothing> run = docker->run(
containerInfo,
commandInfo,
"testContainer",
@@ -225,10 +226,10 @@ TEST(DockerTest, ROOT_DOCKER_CheckCommandWithShell)
TEST(DockerTest, ROOT_DOCKER_CheckPortResource)
{
string containerName = "mesos-docker-port-resource-test";
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ Owned<Docker> docker(Docker::create(tests::flags.docker, false).get());
// Make sure the container is removed.
- Future<Nothing> remove = docker.rm(containerName, true);
+ Future<Nothing> remove = docker->rm(containerName, true);
ASSERT_TRUE(process::internal::await(remove, Seconds(10)));
@@ -253,7 +254,7 @@ TEST(DockerTest, ROOT_DOCKER_CheckPortResource)
Resources resources =
Resources::parse("ports:[9998-9999];ports:[10001-11000]").get();
- Future<Nothing> run = docker.run(
+ Future<Nothing> run = docker->run(
containerInfo,
commandInfo,
containerName,
@@ -269,7 +270,7 @@ TEST(DockerTest, ROOT_DOCKER_CheckPortResource)
Try<string> directory = environment->mkdtemp();
CHECK_SOME(directory) << "Failed to create temporary directory";
- run = docker.run(
+ run = docker->run(
containerInfo,
commandInfo,
containerName,
@@ -279,8 +280,8 @@ TEST(DockerTest, ROOT_DOCKER_CheckPortResource)
AWAIT_READY(run);
- Future<Nothing> status = docker.rm(containerName, true);
- AWAIT_READY(status);
+ Future<Nothing> status = docker->rm(containerName, true);
+ ASSERT_TRUE(process::internal::await(status, Seconds(10)));
}
@@ -298,7 +299,7 @@ TEST(DockerTest, ROOT_DOCKER_CancelPull)
AWAIT_READY_FOR(s.get().status(), Seconds(30));
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ Owned<Docker> docker(Docker::create(tests::flags.docker, false).get());
Try<string> directory = environment->mkdtemp();
@@ -308,7 +309,7 @@ TEST(DockerTest, ROOT_DOCKER_CancelPull)
// sufficiently long that we can start it and discard (i.e., cancel
// it) right away and the future will indeed get discarded.
Future<Docker::Image> future =
- docker.pull(directory.get(), "lingmann/1gb");
+ docker->pull(directory.get(), "lingmann/1gb");
future.discard();
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4e3ef13/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 4dd78e7..7295301 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -174,7 +174,7 @@ public:
DockerFilter()
{
#ifdef __linux__
- Try<Docker> docker = Docker::create(flags.docker);
+ Try<Docker*> docker = Docker::create(flags.docker);
if (docker.isError()) {
dockerError = docker.error();
}