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 <[email protected]> Authored: Fri Oct 31 16:42:03 2014 -0700 Committer: Timothy Chen <[email protected]> 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");
