Provided a factory for allocator in tests. The factory creates allocator instances in a way identical to how instances from modules are created. It allows us to use same typed tests for built-in and modularized allocators.
Review: https://reviews.apache.org/r/31265 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/54b5bdcc Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/54b5bdcc Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/54b5bdcc Branch: refs/heads/master Commit: 54b5bdccb43f724353fd3112115e980360e53c9a Parents: ab48d54 Author: Alexander Rukletsov <[email protected]> Authored: Tue Apr 21 12:11:58 2015 -0700 Committer: Niklas Q. Nielsen <[email protected]> Committed: Tue Apr 21 12:11:59 2015 -0700 ---------------------------------------------------------------------- src/examples/test_allocator_module.cpp | 7 ++++++- src/local/local.cpp | 14 ++++++++++++-- src/master/allocator/mesos/allocator.hpp | 13 ++++++++++++- src/master/main.cpp | 10 ++++++++-- src/tests/cluster.hpp | 10 +++++++++- src/tests/hierarchical_allocator_tests.cpp | 4 +++- src/tests/mesos.hpp | 13 +++++++++++-- 7 files changed, 61 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/examples/test_allocator_module.cpp ---------------------------------------------------------------------- diff --git a/src/examples/test_allocator_module.cpp b/src/examples/test_allocator_module.cpp index 5fe68e3..9679fd9 100644 --- a/src/examples/test_allocator_module.cpp +++ b/src/examples/test_allocator_module.cpp @@ -37,7 +37,12 @@ using mesos::internal::master::allocator::HierarchicalDRFAllocator; static Allocator* createDRFAllocator(const Parameters& parameters) { - return new HierarchicalDRFAllocator(); + Try<Allocator*> allocator = HierarchicalDRFAllocator::create(); + if (allocator.isError()) { + return NULL; + } + + return allocator.get(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/local/local.cpp ---------------------------------------------------------------------- diff --git a/src/local/local.cpp b/src/local/local.cpp index f448912..dff55d8 100644 --- a/src/local/local.cpp +++ b/src/local/local.cpp @@ -133,8 +133,18 @@ PID<Master> launch(const Flags& flags, Allocator* _allocator) } if (_allocator == NULL) { - // Create default allocator, save it for deleting later. - _allocator = allocator = new HierarchicalDRFAllocator(); + // Create a default allocator. + Try<Allocator*> defaultAllocator = HierarchicalDRFAllocator::create(); + if (defaultAllocator.isError()) { + EXIT(1) << "Failed to create an instance of HierarchicalDRFAllocator: " + << defaultAllocator.error(); + } + + // Update caller's instance. + _allocator = defaultAllocator.get(); + + // Save the instance for deleting later. + allocator = defaultAllocator.get(); } else { // TODO(benh): Figure out the behavior of allocator pointer and remove the // else block. http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/master/allocator/mesos/allocator.hpp ---------------------------------------------------------------------- diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp index 5360d0a..2681af5 100644 --- a/src/master/allocator/mesos/allocator.hpp +++ b/src/master/allocator/mesos/allocator.hpp @@ -24,6 +24,8 @@ #include <process/dispatch.hpp> #include <process/process.hpp> +#include <stout/try.hpp> + namespace mesos { namespace internal { namespace master { @@ -39,7 +41,8 @@ template <typename AllocatorProcess> class MesosAllocator : public mesos::master::allocator::Allocator { public: - MesosAllocator(); + // Factory to allow for typed tests. + static Try<mesos::master::allocator::Allocator*> create(); ~MesosAllocator(); @@ -101,6 +104,7 @@ public: const FrameworkID& frameworkId); private: + MesosAllocator(); MesosAllocator(const MesosAllocator&); // Not copyable. MesosAllocator& operator=(const MesosAllocator&); // Not assignable. @@ -180,6 +184,13 @@ public: template <typename AllocatorProcess> +Try<mesos::master::allocator::Allocator*> +MesosAllocator<AllocatorProcess>::create() +{ + return new MesosAllocator<AllocatorProcess>(); +} + +template <typename AllocatorProcess> MesosAllocator<AllocatorProcess>::MesosAllocator() { process = new AllocatorProcess(); http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/master/main.cpp ---------------------------------------------------------------------- diff --git a/src/master/main.cpp b/src/master/main.cpp index 5ec1825..88bb39d 100644 --- a/src/master/main.cpp +++ b/src/master/main.cpp @@ -200,8 +200,14 @@ int main(int argc, char** argv) LOG(INFO) << "Git SHA: " << build::GIT_SHA.get(); } - mesos::master::allocator::Allocator* allocator = - new allocator::HierarchicalDRFAllocator(); + // Create an instance of allocator. + Try<mesos::master::allocator::Allocator*> allocator_ = + allocator::HierarchicalDRFAllocator::create(); + if (allocator_.isError()) { + EXIT(1) << "Failed to create an instance of HierarchicalDRFAllocator: " + << allocator_.error(); + } + mesos::master::allocator::Allocator* allocator = allocator_.get(); state::Storage* storage = NULL; Log* log = NULL; http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/tests/cluster.hpp ---------------------------------------------------------------------- diff --git a/src/tests/cluster.hpp b/src/tests/cluster.hpp index fc4d5d1..8a176be 100644 --- a/src/tests/cluster.hpp +++ b/src/tests/cluster.hpp @@ -271,7 +271,15 @@ inline Try<process::PID<master::Master> > Cluster::Masters::start( } else { // If allocator is not provided, fall back to the default one, // managed by Cluster::Masters. - master.allocator = new master::allocator::HierarchicalDRFAllocator(); + Try<mesos::master::allocator::Allocator*> allocator_ = + master::allocator::HierarchicalDRFAllocator::create(); + if (allocator_.isError()) { + return Error( + "Failed to create an instance of HierarchicalDRFAllocator: " + + allocator_.error()); + } + + master.allocator = allocator_.get(); master.createdAllocator = true; } http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/tests/hierarchical_allocator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp index e25b99f..1a43dc7 100644 --- a/src/tests/hierarchical_allocator_tests.cpp +++ b/src/tests/hierarchical_allocator_tests.cpp @@ -40,6 +40,8 @@ #include "master/allocator/mesos/hierarchical.hpp" +#include "tests/mesos.hpp" + using mesos::internal::master::MIN_CPUS; using mesos::internal::master::MIN_MEM; @@ -71,7 +73,7 @@ class HierarchicalAllocatorTest : public ::testing::Test { protected: HierarchicalAllocatorTest() - : allocator(new HierarchicalDRFAllocator), + : allocator(createAllocator<HierarchicalDRFAllocator>()), nextSlaveId(1), nextFrameworkId(1) {} http://git-wip-us.apache.org/repos/asf/mesos/blob/54b5bdcc/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index f23fdcb..4294e28 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -859,13 +859,22 @@ ACTION_P(InvokeReviveOffers, allocator) template <typename T = master::allocator::HierarchicalDRFAllocator> +mesos::master::allocator::Allocator* createAllocator() +{ + // T represents the allocator type. It can be a default built-in + // allocator, or one provided by an allocator module. + Try<mesos::master::allocator::Allocator*> instance = T::create(); + CHECK_SOME(instance); + return CHECK_NOTNULL(instance.get()); +} + +template <typename T = master::allocator::HierarchicalDRFAllocator> class TestAllocator : public mesos::master::allocator::Allocator { public: // Actual allocation is done by an instance of real allocator, // which is specified by the template parameter. - TestAllocator() - : real(new master::allocator::HierarchicalDRFAllocator()) + TestAllocator() : real(createAllocator<T>()) { // We use 'ON_CALL' and 'WillByDefault' here to specify the // default actions (call in to the real allocator). This allows
