Updated test mocks per interface changes. This updates the mocks for the containerizers and fetcher after the interface changes. For the containerizers, the two launch methods have been combined into a single method.
The fetcher now has an argument in its constructor and no longer takes some arguments (SlaveID and Flags) in its methods. Launching nested containers via the test containerizer was originally a no-op, so similar behavior is implemented in the combined method. Review: https://reviews.apache.org/r/58908 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/46da722e Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/46da722e Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/46da722e Branch: refs/heads/master Commit: 46da722e72fa06133fd0a5130abc34a6edcbbf57 Parents: 66070eb Author: Joseph Wu <josep...@apache.org> Authored: Mon May 1 13:59:32 2017 -0700 Committer: Joseph Wu <josep...@apache.org> Committed: Thu May 25 18:37:07 2017 -0700 ---------------------------------------------------------------------- src/tests/cluster.cpp | 2 +- src/tests/containerizer.cpp | 154 +++++--------------- src/tests/containerizer.hpp | 36 +---- src/tests/containerizer/mock_containerizer.hpp | 20 +-- src/tests/mesos.cpp | 19 +-- src/tests/mesos.hpp | 18 +-- src/tests/mock_docker.cpp | 2 +- src/tests/mock_docker.hpp | 42 ++---- 8 files changed, 72 insertions(+), 221 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/cluster.cpp ---------------------------------------------------------------------- diff --git a/src/tests/cluster.cpp b/src/tests/cluster.cpp index a4f57e0..d657da6 100644 --- a/src/tests/cluster.cpp +++ b/src/tests/cluster.cpp @@ -413,7 +413,7 @@ Try<process::Owned<Slave>> Slave::start( slave->containerizer = containerizer.get(); } else { // Create a new fetcher. - slave->fetcher.reset(new slave::Fetcher()); + slave->fetcher.reset(new slave::Fetcher(flags)); Try<slave::Containerizer*> _containerizer = slave::Containerizer::create(flags, true, slave->fetcher.get()); http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer.cpp b/src/tests/containerizer.cpp index 548da3a..1d2b639 100644 --- a/src/tests/containerizer.cpp +++ b/src/tests/containerizer.cpp @@ -37,6 +37,7 @@ using testing::Invoke; using testing::Return; using mesos::slave::ContainerClass; +using mesos::slave::ContainerConfig; using mesos::slave::ContainerTermination; using mesos::v1::executor::Mesos; @@ -87,35 +88,42 @@ public: Future<bool> launch( const ContainerID& containerId, - const Option<TaskInfo>& taskInfo, - const ExecutorInfo& executorInfo, - const string& directory, - const Option<string>& user, - const SlaveID& slaveId, + const ContainerConfig& containerConfig, const map<string, string>& environment, - bool checkpoint) + const Option<string>& pidCheckpointPath) { CHECK(!terminatedContainers.contains(containerId)) << "Failed to launch nested container " << containerId - << " for executor '" << executorInfo.executor_id() << "'" - << " of framework " << executorInfo.framework_id() + << " for executor '" << containerConfig.executor_info().executor_id() + << "' of framework " << containerConfig.executor_info().framework_id() << " because this ContainerID is being re-used with" << " a previously terminated container"; CHECK(!containers_.contains(containerId)) << "Failed to launch container " << containerId - << " for executor '" << executorInfo.executor_id() << "'" - << " of framework " << executorInfo.framework_id() + << " for executor '" << containerConfig.executor_info().executor_id() + << "' of framework " << containerConfig.executor_info().framework_id() << " because it is already launched"; - CHECK(executors.contains(executorInfo.executor_id())) - << "Failed to launch executor '" << executorInfo.executor_id() << "'" - << " of framework " << executorInfo.framework_id() + containers_[containerId] = Owned<ContainerData>(new ContainerData()); + + if (containerId.has_parent()) { + // Launching a nested container via the test containerizer is a + // no-op for now. + return true; + } + + CHECK(executors.contains(containerConfig.executor_info().executor_id())) + << "Failed to launch executor '" + << containerConfig.executor_info().executor_id() + << "' of framework " << containerConfig.executor_info().framework_id() << " because it is unknown to the containerizer"; - containers_[containerId] = Owned<ContainerData>(new ContainerData()); - containers_.at(containerId)->executorId = executorInfo.executor_id(); - containers_.at(containerId)->frameworkId = executorInfo.framework_id(); + containers_.at(containerId)->executorId = + containerConfig.executor_info().executor_id(); + + containers_.at(containerId)->frameworkId = + containerConfig.executor_info().framework_id(); // We need to synchronize all reads and writes to the environment // as this is global state. @@ -151,14 +159,15 @@ public: // code where we do this as well and it's likely we can do this once // in 'executorEnvironment()'. foreach (const Environment::Variable& variable, - executorInfo.command().environment().variables()) { + containerConfig.executor_info() + .command().environment().variables()) { os::setenv(variable.name(), variable.value()); } os::setenv("MESOS_LOCAL", "1"); const Owned<ExecutorData>& executorData = - executors.at(executorInfo.executor_id()); + executors.at(containerConfig.executor_info().executor_id()); if (executorData->executor != nullptr) { executorData->driver = Owned<MesosExecutorDriver>( @@ -190,29 +199,6 @@ public: return true; } - Future<bool> launch( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const Option<ContainerInfo>& containerInfo, - const Option<string>& user, - const SlaveID& slaveId, - const Option<ContainerClass>& containerClass) - { - CHECK(!terminatedContainers.contains(containerId)) - << "Failed to launch nested container " << containerId - << " because this ContainerID is being re-used with" - << " a previously terminated container"; - - CHECK(!containers_.contains(containerId)) - << "Failed to launch nested container " << containerId - << " because it is already launched"; - - containers_[containerId] = Owned<ContainerData>(new ContainerData()); - - // No-op for now. - return true; - } - Future<Nothing> update( const ContainerID& containerId, const Resources& resources) @@ -220,7 +206,6 @@ public: return Nothing(); } - Future<Connection> attach( const ContainerID& containerId) { @@ -433,30 +418,8 @@ void TestContainerizer::setup() EXPECT_CALL(*this, update(_, _)) .WillRepeatedly(Invoke(this, &TestContainerizer::_update)); - Future<bool> (TestContainerizer::*_launch)( - const ContainerID& containerId, - const Option<TaskInfo>& taskInfo, - const ExecutorInfo& executorInfo, - const string& directory, - const Option<string>& user, - const SlaveID& slaveId, - const map<string, string>& environment, - bool checkpoint) = - &TestContainerizer::_launch; - - EXPECT_CALL(*this, launch(_, _, _, _, _, _, _, _)) - .WillRepeatedly(Invoke(this, _launch)); - - Future<bool> (TestContainerizer::*_launchNested)( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const Option<ContainerInfo>& containerInfo, - const Option<string>& user, - const SlaveID& slaveId, - const Option<ContainerClass>&) = &TestContainerizer::_launch; - - EXPECT_CALL(*this, launch(_, _, _, _, _, _)) - .WillRepeatedly(Invoke(this, _launchNested)); + EXPECT_CALL(*this, launch(_, _, _, _)) + .WillRepeatedly(Invoke(this, &TestContainerizer::_launch)); EXPECT_CALL(*this, attach(_)) .WillRepeatedly(Invoke(this, &TestContainerizer::_attach)); @@ -481,66 +444,17 @@ Future<Nothing> TestContainerizer::_recover( Future<bool> TestContainerizer::_launch( const ContainerID& containerId, - const Option<TaskInfo>& taskInfo, - const ExecutorInfo& executorInfo, - const string& directory, - const Option<string>& user, - const SlaveID& slaveId, + const ContainerConfig& containerConfig, const map<string, string>& environment, - bool checkpoint) + const Option<string>& pidCheckpointPath) { - // Need to disambiguate for the compiler. - Future<bool> (TestContainerizerProcess::*launch)( - const ContainerID&, - const Option<TaskInfo>&, - const ExecutorInfo&, - const string&, - const Option<string>&, - const SlaveID&, - const map<string, string>&, - bool) = &TestContainerizerProcess::launch; - return process::dispatch( process.get(), - launch, + &TestContainerizerProcess::launch, containerId, - taskInfo, - executorInfo, - directory, - user, - slaveId, + containerConfig, environment, - checkpoint); -} - - -Future<bool> TestContainerizer::_launch( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const Option<ContainerInfo>& containerInfo, - const Option<string>& user, - const SlaveID& slaveId, - const Option<ContainerClass>& containerClass) -{ - // Need to disambiguate for the compiler. - Future<bool> (TestContainerizerProcess::*launch)( - const ContainerID&, - const CommandInfo&, - const Option<ContainerInfo>&, - const Option<string>&, - const SlaveID&, - const Option<ContainerClass>& containerClass) = - &TestContainerizerProcess::launch; - - return process::dispatch( - process.get(), - launch, - containerId, - commandInfo, - containerInfo, - user, - slaveId, - containerClass); + pidCheckpointPath); } http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/containerizer.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer.hpp b/src/tests/containerizer.hpp index 63fe236..4bd40c3 100644 --- a/src/tests/containerizer.hpp +++ b/src/tests/containerizer.hpp @@ -84,27 +84,13 @@ public: recover, process::Future<Nothing>(const Option<slave::state::SlaveState>&)); - MOCK_METHOD8( + MOCK_METHOD4( launch, process::Future<bool>( const ContainerID&, - const Option<TaskInfo>&, - const ExecutorInfo&, - const std::string&, - const Option<std::string>&, - const SlaveID&, + const mesos::slave::ContainerConfig&, const std::map<std::string, std::string>&, - bool checkpoint)); - - MOCK_METHOD6( - launch, - process::Future<bool>( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const Option<ContainerInfo>& containerInfo, - const Option<std::string>& user, - const SlaveID& slaveId, - const Option<mesos::slave::ContainerClass>& containerClass)); + const Option<std::string>&)); MOCK_METHOD1( attach, @@ -150,21 +136,9 @@ private: process::Future<bool> _launch( const ContainerID& containerId, - const Option<TaskInfo>& taskInfo, - const ExecutorInfo& executorInfo, - const std::string& directory, - const Option<std::string>& user, - const SlaveID& slaveId, + const mesos::slave::ContainerConfig& containerConfig, const std::map<std::string, std::string>& environment, - bool checkpoint); - - process::Future<bool> _launch( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const Option<ContainerInfo>& containerInfo, - const Option<std::string>& user, - const SlaveID& slaveId, - const Option<mesos::slave::ContainerClass>& containerClass = None()); + const Option<std::string>& pidCheckpointPath); process::Future<process::http::Connection> _attach( const ContainerID& containerId); http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/containerizer/mock_containerizer.hpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/mock_containerizer.hpp b/src/tests/containerizer/mock_containerizer.hpp index ca0ae05..0adcb01 100644 --- a/src/tests/containerizer/mock_containerizer.hpp +++ b/src/tests/containerizer/mock_containerizer.hpp @@ -46,27 +46,13 @@ public: process::Future<Nothing>( const Option<slave::state::SlaveState>&)); - MOCK_METHOD8( + MOCK_METHOD4( launch, process::Future<bool>( const ContainerID&, - const Option<TaskInfo>&, - const ExecutorInfo&, - const std::string&, - const Option<std::string>&, - const SlaveID&, + const mesos::slave::ContainerConfig&, const std::map<std::string, std::string>&, - bool)); - - MOCK_METHOD6( - launch, - process::Future<bool>( - const ContainerID&, - const CommandInfo&, - const Option<ContainerInfo>&, - const Option<std::string>&, - const SlaveID&, - const Option<mesos::slave::ContainerClass>&)); + const Option<std::string>&)); MOCK_METHOD1( attach, http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/mesos.cpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 714a520..a6ddb45 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -506,12 +506,13 @@ MockExecutor::MockExecutor(const ExecutorID& _id) : id(_id) {} MockExecutor::~MockExecutor() {} -MockFetcherProcess::MockFetcherProcess() +MockFetcherProcess::MockFetcherProcess(const slave::Flags& flags) + : slave::FetcherProcess(flags) { // Set up default behaviors, calling the original methods. - EXPECT_CALL(*this, _fetch(_, _, _, _, _, _)) + EXPECT_CALL(*this, _fetch(_, _, _, _, _)) .WillRepeatedly(Invoke(this, &MockFetcherProcess::unmocked__fetch)); - EXPECT_CALL(*this, run(_, _, _, _, _)) + EXPECT_CALL(*this, run(_, _, _, _)) .WillRepeatedly(Invoke(this, &MockFetcherProcess::unmocked_run)); } @@ -552,16 +553,14 @@ Future<Nothing> MockFetcherProcess::unmocked__fetch( const ContainerID& containerId, const string& sandboxDirectory, const string& cacheDirectory, - const Option<string>& user, - const slave::Flags& flags) + const Option<string>& user) { return slave::FetcherProcess::_fetch( entries, containerId, sandboxDirectory, cacheDirectory, - user, - flags); + user); } @@ -569,15 +568,13 @@ Future<Nothing> MockFetcherProcess::unmocked_run( const ContainerID& containerId, const string& sandboxDirectory, const Option<string>& user, - const FetcherInfo& info, - const slave::Flags& flags) + const FetcherInfo& info) { return slave::FetcherProcess::run( containerId, sandboxDirectory, user, - info, - flags); + info); } http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index 3c57f25..9b04a40 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -2098,10 +2098,10 @@ using MockHTTPExecutor = tests::executor::MockHTTPExecutor< class MockFetcherProcess : public slave::FetcherProcess { public: - MockFetcherProcess(); + MockFetcherProcess(const slave::Flags& flags); virtual ~MockFetcherProcess(); - MOCK_METHOD6(_fetch, process::Future<Nothing>( + MOCK_METHOD5(_fetch, process::Future<Nothing>( const hashmap< CommandInfo::URI, Option<process::Future<std::shared_ptr<Cache::Entry>>>>& @@ -2109,8 +2109,7 @@ public: const ContainerID& containerId, const std::string& sandboxDirectory, const std::string& cacheDirectory, - const Option<std::string>& user, - const slave::Flags& flags)); + const Option<std::string>& user)); process::Future<Nothing> unmocked__fetch( const hashmap< @@ -2120,22 +2119,19 @@ public: const ContainerID& containerId, const std::string& sandboxDirectory, const std::string& cacheDirectory, - const Option<std::string>& user, - const slave::Flags& flags); + const Option<std::string>& user); - MOCK_METHOD5(run, process::Future<Nothing>( + MOCK_METHOD4(run, process::Future<Nothing>( const ContainerID& containerId, const std::string& sandboxDirectory, const Option<std::string>& user, - const mesos::fetcher::FetcherInfo& info, - const slave::Flags& flags)); + const mesos::fetcher::FetcherInfo& info)); process::Future<Nothing> unmocked_run( const ContainerID& containerId, const std::string& sandboxDirectory, const Option<std::string>& user, - const mesos::fetcher::FetcherInfo& info, - const slave::Flags& flags); + const mesos::fetcher::FetcherInfo& info); }; http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/mock_docker.cpp ---------------------------------------------------------------------- diff --git a/src/tests/mock_docker.cpp b/src/tests/mock_docker.cpp index 81a14ca..0ed6386 100644 --- a/src/tests/mock_docker.cpp +++ b/src/tests/mock_docker.cpp @@ -98,7 +98,7 @@ MockDockerContainerizerProcess::MockDockerContainerizerProcess( : slave::DockerContainerizerProcess( flags, fetcher, logger, docker, nvidia) { - EXPECT_CALL(*this, fetch(_, _)) + EXPECT_CALL(*this, fetch(_)) .WillRepeatedly(Invoke(this, &MockDockerContainerizerProcess::_fetch)); EXPECT_CALL(*this, pull(_)) http://git-wip-us.apache.org/repos/asf/mesos/blob/46da722e/src/tests/mock_docker.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mock_docker.hpp b/src/tests/mock_docker.hpp index f58211d..5987364 100644 --- a/src/tests/mock_docker.hpp +++ b/src/tests/mock_docker.hpp @@ -153,24 +153,20 @@ public: // NOTE: See TestContainerizer::setup for why we use // 'EXPECT_CALL' and 'WillRepeatedly' here instead of // 'ON_CALL' and 'WillByDefault'. - EXPECT_CALL(*this, launch(_, _, _, _, _, _, _, _)) + EXPECT_CALL(*this, launch(_, _, _, _)) .WillRepeatedly(Invoke(this, &MockDockerContainerizer::_launch)); EXPECT_CALL(*this, update(_, _)) .WillRepeatedly(Invoke(this, &MockDockerContainerizer::_update)); } - MOCK_METHOD8( + MOCK_METHOD4( launch, process::Future<bool>( const ContainerID&, - const Option<TaskInfo>&, - const ExecutorInfo&, - const std::string&, - const Option<std::string>&, - const SlaveID&, + const mesos::slave::ContainerConfig&, const std::map<std::string, std::string>&, - bool checkpoint)); + const Option<std::string>&)); MOCK_METHOD2( update, @@ -182,23 +178,15 @@ public: // use &slave::DockerContainerizer::launch with 'Invoke'). process::Future<bool> _launch( const ContainerID& containerId, - const Option<TaskInfo>& taskInfo, - const ExecutorInfo& executorInfo, - const std::string& directory, - const Option<std::string>& user, - const SlaveID& slaveId, + const mesos::slave::ContainerConfig& containerConfig, const std::map<std::string, std::string>& environment, - bool checkpoint) + const Option<std::string>& pidCheckpointPath) { return slave::DockerContainerizer::launch( containerId, - taskInfo, - executorInfo, - directory, - user, - slaveId, + containerConfig, environment, - checkpoint); + pidCheckpointPath); } process::Future<Nothing> _update( @@ -226,21 +214,17 @@ public: virtual ~MockDockerContainerizerProcess(); - MOCK_METHOD2( + MOCK_METHOD1( fetch, - process::Future<Nothing>( - const ContainerID& containerId, - const SlaveID& slaveId)); + process::Future<Nothing>(const ContainerID&)); MOCK_METHOD1( pull, - process::Future<Nothing>(const ContainerID& containerId)); + process::Future<Nothing>(const ContainerID&)); - process::Future<Nothing> _fetch( - const ContainerID& containerId, - const SlaveID& slaveId) + process::Future<Nothing> _fetch(const ContainerID& containerId) { - return slave::DockerContainerizerProcess::fetch(containerId, slaveId); + return slave::DockerContainerizerProcess::fetch(containerId); } process::Future<Nothing> _pull(const ContainerID& containerId)