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(
