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/11/01 00:47:44 UTC
[1/2] git commit: Fixed docker flaky tests.
Repository: mesos
Updated Branches:
refs/heads/master e8554e511 -> 2fbb2fb4d
Fixed docker flaky tests.
Docker tests are flaky, mostly around getting expected output from the
docker container forwarded to stdout/stderr.
This is due to Docker not always have the stdout/stderr output
available for docker logs if kill/rm is called.
Review: https://reviews.apache.org/r/26862
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2fbb2fb4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2fbb2fb4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2fbb2fb4
Branch: refs/heads/master
Commit: 2fbb2fb4d8e19aa4eb055b14a11268b6bf9ce4c4
Parents: 20b0225
Author: Timothy Chen <tn...@apache.org>
Authored: Fri Oct 31 16:42:03 2014 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Oct 31 16:48:41 2014 -0700
----------------------------------------------------------------------
src/docker/docker.cpp | 15 +-
src/docker/docker.hpp | 23 +-
src/slave/containerizer/docker.cpp | 41 ++-
src/slave/containerizer/docker.hpp | 4 +-
src/tests/docker_containerizer_tests.cpp | 373 +++++++++++++++++++++++---
src/tests/docker_tests.cpp | 57 ++--
src/tests/environment.cpp | 4 +-
7 files changed, 406 insertions(+), 111 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2fbb2fb4/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/2fbb2fb4/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 9656f15..2dc692c 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -40,7 +40,9 @@ 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 +81,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 +92,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 +115,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/2fbb2fb4/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 7e5ff37..c3850dd 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -80,7 +80,7 @@ class DockerContainerizerProcess
public:
DockerContainerizerProcess(
const Flags& _flags,
- const Docker& _docker)
+ Shared<Docker> _docker)
: flags(_flags),
docker(_docker) {}
@@ -210,7 +210,7 @@ private:
const Flags flags;
- Docker docker;
+ Shared<Docker> docker;
struct Container
{
@@ -405,18 +405,18 @@ 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, Shared<Docker>(docker.get()));
}
DockerContainerizer::DockerContainerizer(
const Flags& flags,
- const Docker& docker)
+ Shared<Docker> docker)
{
process = new DockerContainerizerProcess(flags, docker);
spawn(process);
@@ -546,7 +546,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 +793,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 +820,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 +914,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 +1064,7 @@ Future<Docker::Container> DockerContainerizerProcess::____launch(
Container* container = containers_[containerId];
- return docker.inspect(container->name());
+ return docker->inspect(container->name());
}
@@ -1116,7 +1116,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 +1154,7 @@ Future<Nothing> DockerContainerizerProcess::update(
return __update(containerId, _resources, container->pid.get());
}
- return docker.inspect(container->name())
+ return docker->inspect(containers_[containerId]->name())
.then(defer(self(), &Self::_update, containerId, _resources, lambda::_1));
#else
return Nothing();
@@ -1330,7 +1330,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 +1525,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 +1616,7 @@ void DockerContainerizerProcess::reaped(const ContainerID& containerId)
void DockerContainerizerProcess::remove(const string& container)
{
- docker.rm(container, true);
+ docker->rm(container, true);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/2fbb2fb4/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index fbbd45d..7365a84 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -19,6 +19,8 @@
#ifndef __DOCKER_CONTAINERIZER_HPP__
#define __DOCKER_CONTAINERIZER_HPP__
+#include <process/shared.hpp>
+
#include <stout/hashset.hpp>
#include "docker/docker.hpp"
@@ -46,7 +48,7 @@ public:
// This is only public for tests.
DockerContainerizer(
const Flags& flags,
- const Docker& docker);
+ process::Shared<Docker> docker);
virtual ~DockerContainerizer();
http://git-wip-us.apache.org/repos/asf/mesos/blob/2fbb2fb4/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index d99e567..4309509 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -41,6 +41,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 +63,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 +118,41 @@ 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()
+ {
+ Try<Docker*> docker = Docker::create(tests::flags.docker, false);
+ ASSERT_SOME(docker);
+ Future<list<Docker::Container>> containers =
+ docker.get()->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.get()->rm(container.id, true), Seconds(30));
+ }
+
+ delete docker.get();
+ }
};
@@ -86,9 +160,12 @@ class MockDockerContainerizer : public DockerContainerizer {
public:
MockDockerContainerizer(
const slave::Flags& flags,
- const Docker& docker)
+ Shared<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 +267,19 @@ 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);
+ Shared<Docker> docker(mockDocker);
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ // 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)));
+
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -270,8 +357,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 +372,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 +395,19 @@ 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);
+ Shared<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 +486,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 +501,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 +519,19 @@ 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);
+ Shared<Docker> docker(mockDocker);
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ // 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)));
+
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -485,8 +599,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 +608,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 +632,19 @@ 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);
+ Shared<Docker> docker(mockDocker);
- Docker docker = Docker::create(tests::flags.docker, false).get();
+ // 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)));
+
+ slave::Flags flags = CreateSlaveFlags();
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -589,16 +725,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 +751,17 @@ 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);
+ Shared<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 +868,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 +889,17 @@ 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);
+ Shared<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 +969,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 +1035,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 +1054,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);
+ Shared<Docker> docker(mockDocker);
MockDockerContainerizer dockerContainerizer(flags, docker);
@@ -910,7 +1077,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 +1086,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 +1150,8 @@ TEST_F(DockerContainerizerTest, DISABLED_ROOT_DOCKER_Recover)
AWAIT_READY(termination);
AWAIT_READY(reaped.get().status());
+
+ Shutdown();
}
@@ -993,7 +1162,23 @@ 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);
+ Shared<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 +1255,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 +1288,23 @@ 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);
+ Shared<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 +1383,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 +1415,23 @@ 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);
+ Shared<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 +1512,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 +1547,23 @@ 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);
+ Shared<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 +1645,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 +1686,17 @@ 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);
+ Shared<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 +1808,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 +1857,17 @@ 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);
+ Shared<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 +2004,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 +2014,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 +2034,23 @@ 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);
+ Shared<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 +2146,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"));
http://git-wip-us.apache.org/repos/asf/mesos/blob/2fbb2fb4/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/2fbb2fb4/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 4dd78e7..bc2fc90 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -174,9 +174,11 @@ public:
DockerFilter()
{
#ifdef __linux__
- Try<Docker> docker = Docker::create(flags.docker);
+ Try<Docker*> docker = Docker::create(flags.docker);
if (docker.isError()) {
dockerError = docker.error();
+ } else {
+ delete docker.get();
}
#else
dockerError = Error("Docker tests not supported on non-Linux systems");
[2/2] git commit: Schedule docker containers for removal.
Posted by tn...@apache.org.
Schedule docker containers for removal.
Instead of removing docker containers right after reap, schedule it to
be removed later.
Review: https://reviews.apache.org/r/26861
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/20b0225f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/20b0225f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/20b0225f
Branch: refs/heads/master
Commit: 20b0225fcf1ed84c0f518ae51b856f43c044782f
Parents: e8554e5
Author: Timothy Chen <tn...@apache.org>
Authored: Fri Oct 31 16:41:07 2014 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Oct 31 16:48:41 2014 -0700
----------------------------------------------------------------------
src/slave/constants.cpp | 1 +
src/slave/constants.hpp | 3 +++
src/slave/containerizer/docker.cpp | 28 ++++++++++++++++++++++++++--
src/slave/flags.hpp | 8 ++++++++
4 files changed, 38 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/20b0225f/src/slave/constants.cpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.cpp b/src/slave/constants.cpp
index e1da5c0..d6ad78c 100644
--- a/src/slave/constants.cpp
+++ b/src/slave/constants.cpp
@@ -49,6 +49,7 @@ const std::string DEFAULT_PORTS = "[31000-32000]";
#ifdef WITH_NETWORK_ISOLATOR
const uint16_t DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER = 1024;
#endif
+const Duration DOCKER_REMOVE_DELAY = Hours(6);
Duration MASTER_PING_TIMEOUT()
{
http://git-wip-us.apache.org/repos/asf/mesos/blob/20b0225f/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index 9030871..701dd89 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -94,6 +94,9 @@ const Bytes DEFAULT_EXECUTOR_MEM = Megabytes(32);
extern const uint16_t DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER;
#endif
+// Default duration that docker containers will be removed after exit.
+extern const Duration DOCKER_REMOVE_DELAY;
+
// If no pings received within this timeout, then the slave will
// trigger a re-detection of the master to cause a re-registration.
Duration MASTER_PING_TIMEOUT();
http://git-wip-us.apache.org/repos/asf/mesos/blob/20b0225f/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index d6617a9..7e5ff37 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -23,6 +23,7 @@
#include <process/check.hpp>
#include <process/defer.hpp>
+#include <process/delay.hpp>
#include <process/io.hpp>
#include <process/reap.hpp>
#include <process/subprocess.hpp>
@@ -204,6 +205,9 @@ private:
// container destroy.
void reaped(const ContainerID& containerId);
+ // Removes the docker container.
+ void remove(const std::string& container);
+
const Flags flags;
Docker docker;
@@ -1522,8 +1526,16 @@ void DockerContainerizerProcess::_destroy(
LOG(INFO) << "Running docker kill on container '" << containerId << "'";
- docker.kill(container->name(), true)
- .onAny(defer(self(), &Self::__destroy, containerId, killed, lambda::_1));
+ 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());
+ }
}
@@ -1547,6 +1559,9 @@ void DockerContainerizerProcess::__destroy(
(kill.isFailed() ? kill.failure() : "discarded future"));
containers_.erase(containerId);
+
+ delay(flags.docker_remove_delay, self(), &Self::remove, container->name());
+
delete container;
return;
@@ -1582,6 +1597,9 @@ void DockerContainerizerProcess::___destroy(
container->termination.set(termination);
containers_.erase(containerId);
+
+ delay(flags.docker_remove_delay, self(), &Self::remove, container->name());
+
delete container;
}
@@ -1605,6 +1623,12 @@ void DockerContainerizerProcess::reaped(const ContainerID& containerId)
}
+void DockerContainerizerProcess::remove(const string& container)
+{
+ docker.rm(container, true);
+}
+
+
} // namespace slave {
} // namespace internal {
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/20b0225f/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index f7a8cde..319c002 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -291,6 +291,7 @@ public:
"The default container image to use if not specified by a task,\n"
"when using external containerizer.\n");
+ // Docker containerizer flags.
add(&Flags::docker,
"docker",
"The absolute path to the docker executable for docker\n"
@@ -303,6 +304,12 @@ public:
"sandbox is mapped to.\n",
"/mnt/mesos/sandbox");
+ add(&Flags::docker_remove_delay,
+ "docker_remove_delay",
+ "The amount of time to wait before removing docker containers\n"
+ "(e.g., 3days, 2weeks, etc).\n",
+ DOCKER_REMOVE_DELAY);
+
add(&Flags::default_container_info,
"default_container_info",
"JSON formatted ContainerInfo that will be included into\n"
@@ -437,6 +444,7 @@ public:
Option<std::string> default_container_image;
std::string docker;
std::string docker_sandbox_directory;
+ Duration docker_remove_delay;
Option<ContainerInfo> default_container_info;
#ifdef WITH_NETWORK_ISOLATOR
uint16_t ephemeral_ports_per_container;