You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2017/05/26 01:40:59 UTC

[08/16] mesos git commit: Updated test mocks per interface changes.

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 <jo...@apache.org>
Authored: Mon May 1 13:59:32 2017 -0700
Committer: Joseph Wu <jo...@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)