This is an automated email from the ASF dual-hosted git repository. abudnik pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push: new 6c2a94c Added `SlaveOptions` for wrapping all parameters of `StartSlave`. 6c2a94c is described below commit 6c2a94ca0eca90e6d3517e4400f4529ddce0b84c Author: Andrei Budnik <abud...@apache.org> AuthorDate: Mon Sep 2 17:15:52 2019 +0200 Added `SlaveOptions` for wrapping all parameters of `StartSlave`. This patch introduces a `SlaveOptions` struct which holds optional parameters accepted by `cluster::Slave::create`. Added an overload of `StartSlave` that accepts `SlaveOptions`. It's a preferred way of creating and starting an instance of `cluster::Slave` in tests, since underlying `cluster::Slave::create` accepts a long list of optional arguments, which might be extended in the future. Review: https://reviews.apache.org/r/71424 --- src/tests/mesos.cpp | 281 +++++++++++++--------------------------------------- src/tests/mesos.hpp | 104 +++++++++++++++---- 2 files changed, 153 insertions(+), 232 deletions(-) diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 0396ce7..e77db22 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -334,25 +334,22 @@ Try<Owned<cluster::Master>> MesosTest::StartMaster( } -Try<Owned<cluster::Slave>> MesosTest::StartSlave( - MasterDetector* detector, - const Option<slave::Flags>& flags, - bool mock) +Try<Owned<cluster::Slave>> MesosTest::StartSlave(const SlaveOptions& options) { Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - None(), - None(), - None(), - None(), - None(), - None(), - mock); - - if (slave.isSome() && !mock) { + options.detector, + options.flags.isNone() ? CreateSlaveFlags() : options.flags.get(), + options.id, + options.containerizer, + options.gc, + options.taskStatusUpdateManager, + options.resourceEstimator, + options.qosController, + options.secretGenerator, + options.authorizer, + options.mock); + + if (slave.isSome() && !options.mock) { slave.get()->start(); } @@ -362,28 +359,23 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( Try<Owned<cluster::Slave>> MesosTest::StartSlave( MasterDetector* detector, - slave::Containerizer* containerizer, const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - containerizer, - None(), - None(), - None(), - None(), - None(), - None(), - mock); + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags)); +} - if (slave.isSome() && !mock) { - slave.get()->start(); - } - return slave; +Try<Owned<cluster::Slave>> MesosTest::StartSlave( + MasterDetector* detector, + slave::Containerizer* containerizer, + const Option<slave::Flags>& flags, + bool mock) +{ + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withContainerizer(containerizer)); } @@ -393,24 +385,9 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - id, - None(), - None(), - None(), - None(), - None(), - None(), - None(), - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withId(id)); } @@ -420,17 +397,10 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const string& id, const Option<slave::Flags>& flags) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - id, - containerizer); - - if (slave.isSome()) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector) + .withFlags(flags) + .withId(id) + .withContainerizer(containerizer)); } @@ -440,24 +410,9 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - gc, - None(), - None(), - None(), - None(), - None(), - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withGc(gc)); } @@ -466,20 +421,9 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( mesos::slave::ResourceEstimator* resourceEstimator, const Option<slave::Flags>& flags) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - None(), - None(), - resourceEstimator); - - if (slave.isSome()) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector) + .withFlags(flags) + .withResourceEstimator(resourceEstimator)); } @@ -489,71 +433,35 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( mesos::slave::ResourceEstimator* resourceEstimator, const Option<slave::Flags>& flags) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - containerizer, - None(), - None(), - resourceEstimator); - - if (slave.isSome()) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector) + .withFlags(flags) + .withContainerizer(containerizer) + .withResourceEstimator(resourceEstimator)); } Try<Owned<cluster::Slave>> MesosTest::StartSlave( MasterDetector* detector, - mesos::slave::QoSController* qoSController, + mesos::slave::QoSController* qosController, const Option<slave::Flags>& flags) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - None(), - None(), - None(), - qoSController); - - if (slave.isSome()) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector) + .withFlags(flags) + .withQosController(qosController)); } Try<Owned<cluster::Slave>> MesosTest::StartSlave( MasterDetector* detector, slave::Containerizer* containerizer, - mesos::slave::QoSController* qoSController, + mesos::slave::QoSController* qosController, const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - containerizer, - None(), - None(), - None(), - qoSController, - None(), - None(), - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withContainerizer(containerizer) + .withQosController(qosController)); } @@ -563,24 +471,9 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - None(), - None(), - None(), - None(), - None(), - authorizer, - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withAuthorizer(authorizer)); } @@ -591,24 +484,10 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - containerizer, - None(), - None(), - None(), - None(), - None(), - authorizer, - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withContainerizer(containerizer) + .withAuthorizer(authorizer)); } @@ -620,24 +499,11 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( const Option<slave::Flags>& flags, bool mock) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - containerizer, - None(), - None(), - None(), - None(), - secretGenerator, - authorizer, - mock); - - if (slave.isSome() && !mock) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector, mock) + .withFlags(flags) + .withContainerizer(containerizer) + .withSecretGenerator(secretGenerator) + .withAuthorizer(authorizer)); } @@ -646,22 +512,9 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( mesos::SecretGenerator* secretGenerator, const Option<slave::Flags>& flags) { - Try<Owned<cluster::Slave>> slave = cluster::Slave::create( - detector, - flags.isNone() ? CreateSlaveFlags() : flags.get(), - None(), - None(), - None(), - None(), - None(), - None(), - secretGenerator); - - if (slave.isSome()) { - slave.get()->start(); - } - - return slave; + return StartSlave(SlaveOptions(detector) + .withFlags(flags) + .withSecretGenerator(secretGenerator)); } diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index 25359a2..ecde518 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -119,6 +119,87 @@ constexpr char DEFAULT_JWT_SECRET_KEY[] = class MockExecutor; +struct SlaveOptions +{ + SlaveOptions( + mesos::master::detector::MasterDetector* detector, + bool mock = false) + : detector(detector), mock(mock) + {} + + SlaveOptions& withFlags(const Option<slave::Flags>& flags) + { + this->flags = flags; + return *this; + } + + SlaveOptions& withId(const Option<std::string>& id) + { + this->id = id; + return *this; + } + + SlaveOptions& withContainerizer( + const Option<slave::Containerizer*>& containerizer) + { + this->containerizer = containerizer; + return *this; + } + + SlaveOptions& withGc(const Option<slave::GarbageCollector*>& gc) + { + this->gc = gc; + return *this; + } + + SlaveOptions& withTaskStatusUpdateManager( + const Option<slave::TaskStatusUpdateManager*>& taskStatusUpdateManager) + { + this->taskStatusUpdateManager = taskStatusUpdateManager; + return *this; + } + + SlaveOptions& withResourceEstimator( + const Option<mesos::slave::ResourceEstimator*>& resourceEstimator) + { + this->resourceEstimator = resourceEstimator; + return *this; + } + + SlaveOptions& withQosController( + const Option<mesos::slave::QoSController*>& qosController) + { + this->qosController = qosController; + return *this; + } + + SlaveOptions& withSecretGenerator( + const Option<mesos::SecretGenerator*>& secretGenerator) + { + this->secretGenerator = secretGenerator; + return *this; + } + + SlaveOptions& withAuthorizer(const Option<Authorizer*>& authorizer) + { + this->authorizer = authorizer; + return *this; + } + + mesos::master::detector::MasterDetector* detector; + bool mock; + Option<slave::Flags> flags; + Option<std::string> id; + Option<slave::Containerizer*> containerizer; + Option<slave::GarbageCollector*> gc; + Option<slave::TaskStatusUpdateManager*> taskStatusUpdateManager; + Option<mesos::slave::ResourceEstimator*> resourceEstimator; + Option<mesos::slave::QoSController*> qosController; + Option<mesos::SecretGenerator*> secretGenerator; + Option<Authorizer*> authorizer; +}; + + // NOTE: `SSLTemporaryDirectoryTest` exists even when SSL is not compiled into // Mesos. In this case, the class is an alias of `TemporaryDirectoryTest`. class MesosTest : public SSLTemporaryDirectoryTest @@ -157,24 +238,11 @@ protected: const std::shared_ptr<MockRateLimiter>& slaveRemovalLimiter, const Option<master::Flags>& flags = None()); - // TODO(bmahler): Consider adding a builder style interface, e.g. - // - // Try<PID<Slave>> slave = - // Slave().With(flags) - // .With(executor) - // .With(containerizer) - // .With(detector) - // .With(gc) - // .Start(); - // - // Or options: - // - // Injections injections; - // injections.executor = executor; - // injections.containerizer = containerizer; - // injections.detector = detector; - // injections.gc = gc; - // Try<PID<Slave>> slave = StartSlave(injections); + // Starts a slave with the specified options. + // NOTE: This is a preferred method to start a slave. + // The other overloads of `StartSlave` are DEPRECATED! + virtual Try<process::Owned<cluster::Slave>> StartSlave( + const SlaveOptions& options); // Starts a slave with the specified detector and flags. virtual Try<process::Owned<cluster::Slave>> StartSlave(