Repository: mesos Updated Branches: refs/heads/master 8a6e91227 -> 28998f718
Updated tests due to change of containerizer's `destroy()` return type. Review: https://reviews.apache.org/r/66511/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/28998f71 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/28998f71 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/28998f71 Branch: refs/heads/master Commit: 28998f7186682ba53b6e10b7f2116f4d0b23ae8d Parents: ff8999b Author: Andrei Budnik <abud...@mesosphere.com> Authored: Wed Apr 11 14:29:31 2018 -0700 Committer: Greg Mann <gregorywm...@gmail.com> Committed: Wed Apr 11 15:33:12 2018 -0700 ---------------------------------------------------------------------- src/tests/cluster.cpp | 3 +- src/tests/containerizer.cpp | 29 ++++++++------- src/tests/containerizer.hpp | 7 ++-- .../composing_containerizer_tests.cpp | 39 +++++++++++++------- .../docker_containerizer_tests.cpp | 6 ++- .../containerizer/io_switchboard_tests.cpp | 3 +- .../containerizer/mesos_containerizer_tests.cpp | 6 ++- src/tests/containerizer/mock_containerizer.hpp | 3 +- 8 files changed, 62 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/cluster.cpp ---------------------------------------------------------------------- diff --git a/src/tests/cluster.cpp b/src/tests/cluster.cpp index 97dde00..c071da6 100644 --- a/src/tests/cluster.cpp +++ b/src/tests/cluster.cpp @@ -655,7 +655,8 @@ Slave::~Slave() process::Future<Option<ContainerTermination>> wait = containerizer->wait(containerId); - process::Future<bool> destroy = containerizer->destroy(containerId); + process::Future<Option<ContainerTermination>> destroy = + containerizer->destroy(containerId); AWAIT(destroy); AWAIT(wait); http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer.cpp b/src/tests/containerizer.cpp index 13a290f..c4e18b7 100644 --- a/src/tests/containerizer.cpp +++ b/src/tests/containerizer.cpp @@ -242,11 +242,11 @@ public: .then(Option<ContainerTermination>::some); } - Future<bool> destroy( + Future<Option<mesos::slave::ContainerTermination>> destroy( const ContainerID& containerId) { if (!containers_.contains(containerId)) { - return false; + return None(); } const Owned<ContainerData>& containerData = containers_.at(containerId); @@ -274,12 +274,12 @@ public: containers_.erase(containerId); terminatedContainers[containerId] = termination; - return true; + return termination; } // Additional destroy method for testing because we won't know the // ContainerID created for each container. - Future<bool> destroy( + Future<Option<mesos::slave::ContainerTermination>> destroy( const FrameworkID& frameworkId, const ExecutorID& executorId) { @@ -298,7 +298,7 @@ public: LOG(WARNING) << "Ignoring destroy of unknown container" << " for executor '" << executorId << "'" << " of framework " << frameworkId; - return false; + return None(); } return destroy(containerId.get()); @@ -306,7 +306,8 @@ public: Future<bool> kill(const ContainerID& containerId, int /* signal */) { - return destroy(containerId); + return destroy(containerId) + .then([]() { return true; }); } Future<hashset<ContainerID>> containers() @@ -527,12 +528,13 @@ Future<Option<mesos::slave::ContainerTermination>> TestContainerizer::_wait( } -Future<bool> TestContainerizer::_destroy( +Future<Option<mesos::slave::ContainerTermination>> TestContainerizer::_destroy( const ContainerID& containerId) { // Need to disambiguate for the compiler. - Future<bool> (TestContainerizerProcess::*destroy)( - const ContainerID&) = &TestContainerizerProcess::destroy; + Future<Option<mesos::slave::ContainerTermination>> ( + TestContainerizerProcess::*destroy)(const ContainerID&) = + &TestContainerizerProcess::destroy; return process::dispatch( process.get(), @@ -553,14 +555,15 @@ Future<bool> TestContainerizer::_kill( } -Future<bool> TestContainerizer::destroy( +Future<Option<mesos::slave::ContainerTermination>> TestContainerizer::destroy( const FrameworkID& frameworkId, const ExecutorID& executorId) { // Need to disambiguate for the compiler. - Future<bool> (TestContainerizerProcess::*destroy)( - const FrameworkID&, - const ExecutorID&) = &TestContainerizerProcess::destroy; + Future<Option<mesos::slave::ContainerTermination>> ( + TestContainerizerProcess::*destroy)( + const FrameworkID&, const ExecutorID&) = + &TestContainerizerProcess::destroy; return process::dispatch( process.get(), http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer.hpp b/src/tests/containerizer.hpp index d6a69d6..6acb084 100644 --- a/src/tests/containerizer.hpp +++ b/src/tests/containerizer.hpp @@ -116,7 +116,8 @@ public: MOCK_METHOD1( destroy, - process::Future<bool>(const ContainerID&)); + process::Future<Option<mesos::slave::ContainerTermination>>( + const ContainerID&)); MOCK_METHOD2( kill, @@ -128,7 +129,7 @@ public: // Additional destroy method for testing because we won't know the // ContainerID created for each container. - process::Future<bool> destroy( + process::Future<Option<mesos::slave::ContainerTermination>> destroy( const FrameworkID& frameworkId, const ExecutorID& executorId); @@ -164,7 +165,7 @@ private: process::Future<Option<mesos::slave::ContainerTermination>> _wait( const ContainerID& containerId); - process::Future<bool> _destroy( + process::Future<Option<mesos::slave::ContainerTermination>> _destroy( const ContainerID& containerId); process::Future<bool> _kill( http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer/composing_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/composing_containerizer_tests.cpp b/src/tests/containerizer/composing_containerizer_tests.cpp index 09f7ea4..6964ac2 100644 --- a/src/tests/containerizer/composing_containerizer_tests.cpp +++ b/src/tests/containerizer/composing_containerizer_tests.cpp @@ -89,7 +89,7 @@ TEST_F(ComposingContainerizerTest, DestroyDuringUnsupportedLaunchLoop) .WillOnce(Return(launchPromise.future())); Future<Nothing> destroy; - Promise<bool> destroyPromise; + Promise<Option<ContainerTermination>> destroyPromise; EXPECT_CALL(*mockContainerizer1, destroy(_)) .WillOnce(DoAll(FutureSatisfy(&destroy), Return(destroyPromise.future()))); @@ -104,7 +104,8 @@ TEST_F(ComposingContainerizerTest, DestroyDuringUnsupportedLaunchLoop) EXPECT_TRUE(launched.isPending()); - Future<bool> destroyed = containerizer.destroy(containerId); + Future<Option<ContainerTermination>> destroyed = + containerizer.destroy(containerId); EXPECT_CALL(*mockContainerizer2, launch(_, _, _, _)) .Times(0); @@ -115,13 +116,15 @@ TEST_F(ComposingContainerizerTest, DestroyDuringUnsupportedLaunchLoop) AWAIT_READY(destroy); launchPromise.set(Containerizer::LaunchResult::NOT_SUPPORTED); - destroyPromise.set(false); + destroyPromise.set(Option<ContainerTermination>::none()); // `launched` should be a failure and `destroyed` should be true // because the launch was stopped from being tried on the 2nd // containerizer because of the destroy. AWAIT_FAILED(launched); - AWAIT_EXPECT_EQ(true, destroyed); + + AWAIT_READY(destroyed); + EXPECT_SOME(destroyed.get()); } @@ -155,7 +158,7 @@ TEST_F(ComposingContainerizerTest, DestroyDuringSupportedLaunchLoop) .WillOnce(Return(launchPromise.future())); Future<Nothing> destroy; - Promise<bool> destroyPromise; + Promise<Option<ContainerTermination>> destroyPromise; EXPECT_CALL(*mockContainerizer1, destroy(_)) .WillOnce(DoAll(FutureSatisfy(&destroy), Return(destroyPromise.future()))); @@ -170,7 +173,8 @@ TEST_F(ComposingContainerizerTest, DestroyDuringSupportedLaunchLoop) EXPECT_TRUE(launched.isPending()); - Future<bool> destroyed = containerizer.destroy(containerId); + Future<Option<ContainerTermination>> destroyed = + containerizer.destroy(containerId); EXPECT_CALL(*mockContainerizer2, launch(_, _, _, _)) .Times(0); @@ -181,12 +185,14 @@ TEST_F(ComposingContainerizerTest, DestroyDuringSupportedLaunchLoop) AWAIT_READY(destroy); launchPromise.set(Containerizer::LaunchResult::SUCCESS); - destroyPromise.set(false); + destroyPromise.set(Option<ContainerTermination>::none()); // `launched` should return true and `destroyed` should return false // because the launch succeeded and `destroyPromise` was set to false. AWAIT_EXPECT_EQ(Containerizer::LaunchResult::SUCCESS, launched); - AWAIT_EXPECT_EQ(false, destroyed); + + AWAIT_READY(destroyed); + EXPECT_NONE(destroyed.get()); } @@ -217,7 +223,7 @@ TEST_F(ComposingContainerizerTest, DestroyAfterLaunchLoop) .WillOnce(Return(launchPromise.future())); Future<Nothing> destroy; - Promise<bool> destroyPromise; + Promise<Option<ContainerTermination>> destroyPromise; EXPECT_CALL(*mockContainerizer1, destroy(_)) .WillOnce(DoAll(FutureSatisfy(&destroy), Return(destroyPromise.future()))); @@ -232,18 +238,21 @@ TEST_F(ComposingContainerizerTest, DestroyAfterLaunchLoop) EXPECT_TRUE(launched.isPending()); - Future<bool> destroyed = containerizer.destroy(containerId); + Future<Option<ContainerTermination>> destroyed = + containerizer.destroy(containerId); // We make sure the destroy is being called on the containerizer. AWAIT_READY(destroy); launchPromise.set(Containerizer::LaunchResult::NOT_SUPPORTED); - destroyPromise.set(false); + destroyPromise.set(Option<ContainerTermination>::none()); // `launch` should return false and `destroyed` should return false // because none of the containerizers support the launch. AWAIT_EXPECT_EQ(Containerizer::LaunchResult::NOT_SUPPORTED, launched); - AWAIT_EXPECT_EQ(false, destroyed); + + AWAIT_READY(destroyed); + EXPECT_NONE(destroyed.get()); } @@ -264,7 +273,11 @@ TEST_F(ComposingContainerizerTest, DestroyUnknownContainer) ContainerID containerId; containerId.set_value(id::UUID::random().toString()); - AWAIT_EXPECT_FALSE(containerizer.destroy(containerId)); + Future<Option<ContainerTermination>> destroyed = + containerizer.destroy(containerId); + + AWAIT_READY(destroyed); + EXPECT_NONE(destroyed.get()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer/docker_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp b/src/tests/containerizer/docker_containerizer_tests.cpp index c198ecb..847258d 100644 --- a/src/tests/containerizer/docker_containerizer_tests.cpp +++ b/src/tests/containerizer/docker_containerizer_tests.cpp @@ -3512,7 +3512,11 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_DestroyUnknownContainer) ContainerID containerId; containerId.set_value(id::UUID::random().toString()); - AWAIT_EXPECT_FALSE(containerizer->destroy(containerId)); + Future<Option<ContainerTermination>> destroyed = + containerizer->destroy(containerId); + + AWAIT_READY(destroyed); + EXPECT_NONE(destroyed.get()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer/io_switchboard_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/io_switchboard_tests.cpp b/src/tests/containerizer/io_switchboard_tests.cpp index 8fdaa7f..784386a 100644 --- a/src/tests/containerizer/io_switchboard_tests.cpp +++ b/src/tests/containerizer/io_switchboard_tests.cpp @@ -756,7 +756,8 @@ TEST_F(IOSwitchboardTest, ContainerAttach) Future<Option<ContainerTermination>> wait = containerizer->wait(containerId); - Future<bool> destroy = containerizer->destroy(containerId); + Future<Option<ContainerTermination>> destroy = + containerizer->destroy(containerId); AWAIT_READY(destroy); AWAIT_READY(wait); http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer/mesos_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp b/src/tests/containerizer/mesos_containerizer_tests.cpp index 4fce8af..01f2b38 100644 --- a/src/tests/containerizer/mesos_containerizer_tests.cpp +++ b/src/tests/containerizer/mesos_containerizer_tests.cpp @@ -885,7 +885,11 @@ TEST_F(MesosContainerizerDestroyTest, DestroyUnknownContainer) ContainerID containerId; containerId.set_value(id::UUID::random().toString()); - AWAIT_EXPECT_FALSE(containerizer->destroy(containerId)); + Future<Option<ContainerTermination>> destroyed = + containerizer->destroy(containerId); + + AWAIT_READY(destroyed); + EXPECT_NONE(destroyed.get()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/28998f71/src/tests/containerizer/mock_containerizer.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/mock_containerizer.hpp b/src/tests/containerizer/mock_containerizer.hpp index 01c617a..8700257 100644 --- a/src/tests/containerizer/mock_containerizer.hpp +++ b/src/tests/containerizer/mock_containerizer.hpp @@ -76,7 +76,8 @@ public: MOCK_METHOD1( destroy, - process::Future<bool>(const ContainerID&)); + process::Future<Option<mesos::slave::ContainerTermination>>( + const ContainerID&)); MOCK_METHOD0( containers,