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