You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/08/23 03:06:28 UTC
[2/3] mesos git commit: Refactored task validations in master.
Refactored task validations in master.
Refactored in such a way that most of the helper functions can be
reused for doing task group validation.
Note that there are no functional changes here only code movement.
Review: https://reviews.apache.org/r/51248
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fb58d38d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fb58d38d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fb58d38d
Branch: refs/heads/master
Commit: fb58d38d86b63804448455bcf2e356d75554e5e1
Parents: 47f2c21
Author: Vinod Kone <vi...@gmail.com>
Authored: Tue Aug 16 19:22:34 2016 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Mon Aug 22 20:04:04 2016 -0700
----------------------------------------------------------------------
src/master/validation.cpp | 475 +++++++++++++++++++----------
src/master/validation.hpp | 17 +-
src/tests/master_validation_tests.cpp | 109 ++++++-
3 files changed, 438 insertions(+), 163 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fb58d38d/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index 19a63ea..803aa48 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -470,6 +470,25 @@ Option<Error> validateUniquePersistenceID(
}
+// Validates that revocable and non-revocable
+// resources of the same name do not exist.
+//
+// TODO(vinod): Is this the right place to do this?
+Option<Error> validateRevocableAndNonRevocableResources(
+ const Resources& _resources)
+{
+ foreach (const string& name, _resources.names()) {
+ Resources resources = _resources.get(name);
+ if (!resources.revocable().empty() && resources != resources.revocable()) {
+ return Error("Cannot use both revocable and non-revocable '" + name +
+ "' at the same time");
+ }
+ }
+
+ return None();
+}
+
+
// Validates that all the given resources are persistent volumes.
Option<Error> validatePersistentVolume(
const RepeatedPtrField<Resource>& volumes)
@@ -514,6 +533,161 @@ Option<Error> validate(const RepeatedPtrField<Resource>& resources)
} // namespace resource {
+namespace executor {
+namespace internal {
+
+Option<Error> validateType(const ExecutorInfo& executor)
+{
+ switch (executor.type()) {
+ case ExecutorInfo::DEFAULT:
+ if (executor.has_command()) {
+ return Error(
+ "'ExecutorInfo.command' must not be set for 'DEFAULT' executor");
+ }
+ break;
+
+ case ExecutorInfo::CUSTOM:
+ if (!executor.has_command()) {
+ return Error(
+ "'ExecutorInfo.command' must be set for 'CUSTOM' executor");
+ }
+ break;
+
+ case ExecutorInfo::UNKNOWN:
+ // This could happen if a new executor type is introduced in the
+ // protos but the master doesn't know about it yet (e.g., new
+ // scheduler launches new type of executor on an old master).
+ return None();
+ }
+
+ return None();
+}
+
+
+Option<Error> validateCompatibleExecutorInfo(
+ const ExecutorInfo& executor,
+ Framework* framework,
+ Slave* slave)
+{
+ CHECK_NOTNULL(framework);
+ CHECK_NOTNULL(slave);
+
+ const ExecutorID& executorId = executor.executor_id();
+ Option<ExecutorInfo> executorInfo = None();
+
+ if (slave->hasExecutor(framework->id(), executorId)) {
+ executorInfo =
+ slave->executors.at(framework->id()).at(executorId);
+ }
+
+ if (executorInfo.isSome() && !(executor == executorInfo.get())) {
+ return Error(
+ "ExecutorInfo is not compatible with existing ExecutorInfo"
+ " with same ExecutorID).\n"
+ "------------------------------------------------------------\n"
+ "Existing ExecutorInfo:\n" +
+ stringify(executorInfo.get()) + "\n"
+ "------------------------------------------------------------\n"
+ "ExecutorInfo:\n" +
+ stringify(executor) + "\n"
+ "------------------------------------------------------------\n");
+ }
+
+ return None();
+}
+
+
+Option<Error> validateFrameworkID(
+ const ExecutorInfo& executor,
+ Framework* framework)
+{
+ CHECK_NOTNULL(framework);
+
+ // Master ensures `ExecutorInfo.framework_id`
+ // is set before calling this method.
+ CHECK(executor.has_framework_id());
+
+ if (executor.framework_id() != framework->id()) {
+ return Error(
+ "ExecutorInfo has an invalid FrameworkID"
+ " (Actual: " + stringify(executor.framework_id()) +
+ " vs Expected: " + stringify(framework->id()) + ")");
+ }
+
+ return None();
+}
+
+
+Option<Error> validateShutdownGracePeriod(const ExecutorInfo& executor)
+{
+ // Make sure provided duration is non-negative.
+ if (executor.has_shutdown_grace_period() &&
+ Nanoseconds(executor.shutdown_grace_period().nanoseconds()) <
+ Duration::zero()) {
+ return Error(
+ "ExecutorInfo's 'shutdown_grace_period' must be non-negative");
+ }
+
+ return None();
+}
+
+
+Option<Error> validateResources(const ExecutorInfo& executor)
+{
+ Option<Error> error = resource::validate(executor.resources());
+ if (error.isSome()) {
+ return Error("Executor uses invalid resources: " + error->message);
+ }
+
+ const Resources& resources = executor.resources();
+
+ error = resource::validateUniquePersistenceID(resources);
+ if (error.isSome()) {
+ return Error(
+ "Executor uses duplicate persistence ID: " + error->message);
+ }
+
+ error = resource::validateRevocableAndNonRevocableResources(resources);
+ if (error.isSome()) {
+ return Error("Executor mixes revocable and non-revocable resources: " +
+ error->message);
+ }
+
+ return None();
+}
+
+
+Option<Error> validate(
+ const ExecutorInfo& executor,
+ Framework* framework,
+ Slave* slave)
+{
+ CHECK_NOTNULL(framework);
+ CHECK_NOTNULL(slave);
+
+ vector<lambda::function<Option<Error>()>> validators = {
+ lambda::bind(internal::validateType, executor),
+ lambda::bind(internal::validateFrameworkID, executor, framework),
+ lambda::bind(internal::validateShutdownGracePeriod, executor),
+ lambda::bind(internal::validateResources, executor),
+ lambda::bind(
+ internal::validateCompatibleExecutorInfo, executor, framework, slave)
+ };
+
+ foreach (const lambda::function<Option<Error>()>& validator, validators) {
+ Option<Error> error = validator();
+ if (error.isSome()) {
+ return error;
+ }
+ }
+
+ return None();
+}
+
+} // namespace internal {
+} // namespace executor {
+
+
namespace task {
namespace internal {
@@ -559,73 +733,115 @@ Option<Error> validateSlaveID(const TaskInfo& task, Slave* slave)
}
-// Validates that tasks that use the "same" executor (i.e., same
-// ExecutorID) have an identical ExecutorInfo.
-Option<Error> validateExecutorInfo(
- const TaskInfo& task,
- Framework* framework,
- Slave* slave)
+Option<Error> validateKillPolicy(const TaskInfo& task)
{
- if (task.has_executor() == task.has_command()) {
- return Error(
- "Task should have at least one (but not both) of CommandInfo or "
- "ExecutorInfo present");
+ if (task.has_kill_policy() &&
+ task.kill_policy().has_grace_period() &&
+ Nanoseconds(task.kill_policy().grace_period().nanoseconds()) <
+ Duration::zero()) {
+ return Error("Task's 'kill_policy.grace_period' must be non-negative");
}
- if (task.has_executor()) {
- // Master ensures `ExecutorInfo.framework_id` is set before calling
- // this method.
- CHECK(task.executor().has_framework_id());
+ return None();
+}
- if (task.executor().framework_id() != framework->id()) {
- return Error(
- "ExecutorInfo has an invalid FrameworkID"
- " (Actual: " + stringify(task.executor().framework_id()) +
- " vs Expected: " + stringify(framework->id()) + ")");
+
+Option<Error> validateHealthCheck(const TaskInfo& task)
+{
+ if (task.has_health_check()) {
+ Option<Error> error = health::validation::healthCheck(task.health_check());
+ if (error.isSome()) {
+ return Error("Task uses invalid health check: " + error->message);
}
+ }
+ return None();
+}
- // TODO(vinod): Revisit these when `TaskGroup` validation is added
- // (MESOS-6042).
- if (task.executor().has_type() &&
- task.executor().type() != ExecutorInfo::CUSTOM) {
- return Error("'ExecutorInfo.type' must be 'CUSTOM'");
- }
+Option<Error> validateResources(const TaskInfo& task)
+{
+ if (task.resources().empty()) {
+ return Error("Task uses no resources");
+ }
- // While `ExecutorInfo.command` is optional in the protobuf,
- // semantically it is still required for backwards compatibility.
- if (!task.executor().has_command()) {
- return Error("'ExecutorInfo.command' must be set");
- }
+ Option<Error> error = resource::validate(task.resources());
+ if (error.isSome()) {
+ return Error("Task uses invalid resources: " + error->message);
+ }
- const ExecutorID& executorId = task.executor().executor_id();
- Option<ExecutorInfo> executorInfo = None();
+ const Resources& resources = task.resources();
- if (slave->hasExecutor(framework->id(), executorId)) {
- executorInfo =
- slave->executors.get(framework->id()).get().get(executorId);
- }
+ error = resource::validateUniquePersistenceID(resources);
+ if (error.isSome()) {
+ return Error("Task uses duplicate persistence ID: " + error->message);
+ }
- if (executorInfo.isSome() && !(task.executor() == executorInfo.get())) {
- return Error(
- "Task has invalid ExecutorInfo (existing ExecutorInfo"
- " with same ExecutorID is not compatible).\n"
- "------------------------------------------------------------\n"
- "Existing ExecutorInfo:\n" +
- stringify(executorInfo.get()) + "\n"
- "------------------------------------------------------------\n"
- "Task's ExecutorInfo:\n" +
- stringify(task.executor()) + "\n"
- "------------------------------------------------------------\n");
- }
+ error = resource::validateRevocableAndNonRevocableResources(resources);
+ if (error.isSome()) {
+ return Error("Task mixes revocable and non-revocable resources: " +
+ error->message);
+ }
- // Make sure provided duration is non-negative.
- if (task.executor().has_shutdown_grace_period() &&
- Nanoseconds(task.executor().shutdown_grace_period().nanoseconds()) <
- Duration::zero()) {
- return Error(
- "ExecutorInfo's 'shutdown_grace_period' must be non-negative");
+ return None();
+}
+
+
+Option<Error> validateTaskAndExecutorResources(const TaskInfo& task)
+{
+ Resources total = task.resources();
+ if (task.has_executor()) {
+ total += task.executor().resources();
+ }
+
+ Option<Error> error = resource::validate(total);
+ if (error.isSome()) {
+ return Error(
+ "Task and its executor use invalid resources: " + error->message);
+ }
+
+ error = resource::validateUniquePersistenceID(total);
+ if (error.isSome()) {
+ return Error("Task and its executor use duplicate persistence ID: " +
+ error->message);
+ }
+
+ error = resource::validateRevocableAndNonRevocableResources(total);
+ if (error.isSome()) {
+ return Error("Task and its executor mix revocable and non-revocable"
+ " resources: " + error->message);
+ }
+
+ return None();
+}
+
+
+// Validates task specific fields except its executor (if it exists).
+Option<Error> validateTask(
+ const TaskInfo& task,
+ Framework* framework,
+ Slave* slave)
+{
+ CHECK_NOTNULL(framework);
+ CHECK_NOTNULL(slave);
+
+ // NOTE: The order in which the following validate functions are
+ // executed does matter!
+ vector<lambda::function<Option<Error>()>> validators = {
+ lambda::bind(internal::validateTaskID, task),
+ lambda::bind(internal::validateUniqueTaskID, task, framework),
+ lambda::bind(internal::validateSlaveID, task, slave),
+ lambda::bind(internal::validateKillPolicy, task),
+ lambda::bind(internal::validateHealthCheck, task),
+ lambda::bind(internal::validateResources, task)
+ };
+
+ // TODO(jieyu): Add a validateCommandInfo function.
+
+ foreach (const lambda::function<Option<Error>()>& validator, validators) {
+ Option<Error> error = validator();
+ if (error.isSome()) {
+ return error;
}
}
@@ -633,31 +849,54 @@ Option<Error> validateExecutorInfo(
}
-// Validates that the task and the executor are using proper amount of
-// resources. For instance, the used resources by a task on a slave
-// should not exceed the total resources offered on that slave.
-Option<Error> validateResourceUsage(
+// Validates `Task.executor` if it exists.
+Option<Error> validateExecutor(
const TaskInfo& task,
Framework* framework,
Slave* slave,
const Resources& offered)
{
- Resources taskResources = task.resources();
+ CHECK_NOTNULL(framework);
+ CHECK_NOTNULL(slave);
- if (taskResources.empty()) {
- return Error("Task uses no resources");
+ if (task.has_executor() == task.has_command()) {
+ return Error(
+ "Task should have at least one (but not both) of CommandInfo or "
+ "ExecutorInfo present");
}
- Resources executorResources;
- if (task.has_executor()) {
- executorResources = task.executor().resources();
- }
+ Resources total = task.resources();
+
+ Option<Error> error = None();
- // Validate minimal cpus and memory resources of executor and log
- // warnings if not set.
if (task.has_executor()) {
+ const ExecutorInfo& executor = task.executor();
+
+ // Do the general validation first.
+ error = executor::internal::validate(executor, framework, slave);
+ if (error.isSome()) {
+ return error;
+ }
+
+ // Now do specific validation when an executor is specified on `Task`.
+
+ // TODO(vinod): Revisit this when we allow schedulers to explicitly
+ // specify 'DEFAULT' executor in the `LAUNCH` operation.
+
+ if (executor.has_type() && executor.type() != ExecutorInfo::CUSTOM) {
+ return Error("'ExecutorInfo.type' must be 'CUSTOM'");
+ }
+
+ // While `ExecutorInfo.command` is optional in the protobuf,
+ // semantically it is still required for backwards compatibility.
+ if (!executor.has_command()) {
+ return Error("'ExecutorInfo.command' must be set");
+ }
+
// TODO(martin): MESOS-1807. Return Error instead of logging a
- // warning in 0.22.0.
+ // warning.
+ const Resources& executorResources = executor.resources();
+
Option<double> cpus = executorResources.cpus();
if (cpus.isNone() || cpus.get() < MIN_CPUS) {
LOG(WARNING)
@@ -681,85 +920,25 @@ Option<Error> validateResourceUsage(
<< "). Please update your executor, as this will be mandatory "
<< "in future releases.";
}
- }
-
- // Validate if resources needed by the task (and its executor in
- // case the executor is new) are available.
- Resources total = taskResources;
- if (!slave->hasExecutor(framework->id(), task.executor().executor_id())) {
- total += executorResources;
- }
-
- if (!offered.contains(total)) {
- return Error(
- "Task uses more resources " + stringify(total) +
- " than available " + stringify(offered));
- }
-
- return None();
-}
-
-
-// Validates that the resources specified by the task are valid.
-Option<Error> validateResources(const TaskInfo& task)
-{
- Option<Error> error = resource::validate(task.resources());
- if (error.isSome()) {
- return Error("Task uses invalid resources: " + error.get().message);
- }
-
- Resources total = task.resources();
- if (task.has_executor()) {
- error = resource::validate(task.executor().resources());
- if (error.isSome()) {
- return Error("Executor uses invalid resources: " + error.get().message);
+ if (!slave->hasExecutor(framework->id(), task.executor().executor_id())) {
+ total += executorResources;
}
-
- total += task.executor().resources();
}
- // A task and its executor can either use non-revocable resources
- // or revocable resources of a given name but not both.
- foreach (const string& name, total.names()) {
- Resources resources = total.get(name);
- if (!resources.revocable().empty() &&
- resources != resources.revocable()) {
- return Error("Task (and its executor, if exists) uses both revocable and"
- " non-revocable " + name);
- }
- }
+ // Now validate combined resources of task and executor.
- error = resource::validateUniquePersistenceID(total);
+ // NOTE: This is refactored into a separate function
+ // so that it can be easily unit tested.
+ error = task::internal::validateTaskAndExecutorResources(task);
if (error.isSome()) {
return error;
}
- return None();
-}
-
-
-Option<Error> validateKillPolicy(const TaskInfo& task)
-{
- if (task.has_kill_policy() &&
- task.kill_policy().has_grace_period() &&
- Nanoseconds(task.kill_policy().grace_period().nanoseconds()) <
- Duration::zero()) {
- return Error("Task's 'kill_policy.grace_period' must be non-negative");
- }
-
- return None();
-}
-
-
-Option<Error> validateHealthCheck(const TaskInfo& task)
-{
- if (task.has_health_check()) {
- Option<Error> error = health::validation::healthCheck(task.health_check());
-
- if (error.isSome()) {
- return Error("Task uses invalid health check: " + error.get().message);
- }
+ if (!offered.contains(total)) {
+ return Error(
+ "Total resources " + stringify(total) + " required by task and its"
+ " executor is more than available " + stringify(offered));
}
return None();
@@ -768,6 +947,7 @@ Option<Error> validateHealthCheck(const TaskInfo& task)
} // namespace internal {
+// Validate task and its executor (if it exists).
Option<Error> validate(
const TaskInfo& task,
Framework* framework,
@@ -777,26 +957,11 @@ Option<Error> validate(
CHECK_NOTNULL(framework);
CHECK_NOTNULL(slave);
- // NOTE: The order in which the following validate functions are
- // executed does matter! For example, 'validateResourceUsage'
- // assumes that ExecutorInfo is valid which is verified by
- // 'validateExecutorInfo'.
vector<lambda::function<Option<Error>()>> validators = {
- lambda::bind(internal::validateTaskID, task),
- lambda::bind(internal::validateUniqueTaskID, task, framework),
- lambda::bind(internal::validateSlaveID, task, slave),
- lambda::bind(internal::validateExecutorInfo, task, framework, slave),
- lambda::bind(internal::validateResources, task),
- lambda::bind(internal::validateKillPolicy, task),
- lambda::bind(internal::validateHealthCheck, task),
- lambda::bind(
- internal::validateResourceUsage, task, framework, slave, offered)
+ lambda::bind(internal::validateTask, task, framework, slave),
+ lambda::bind(internal::validateExecutor, task, framework, slave, offered)
};
- // TODO(benh): Add a validateHealthCheck function.
-
- // TODO(jieyu): Add a validateCommandInfo function.
-
foreach (const lambda::function<Option<Error>()>& validator, validators) {
Option<Error> error = validator();
if (error.isSome()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/fb58d38d/src/master/validation.hpp
----------------------------------------------------------------------
diff --git a/src/master/validation.hpp b/src/master/validation.hpp
index fd00609..f1df645 100644
--- a/src/master/validation.hpp
+++ b/src/master/validation.hpp
@@ -75,6 +75,18 @@ Option<Error> validate(
} // namespace resource {
+namespace executor {
+
+// Functions in this namespace are only exposed for testing.
+namespace internal {
+
+// Validates that fields are properly set depending on the type of the executor.
+Option<Error> validateType(const ExecutorInfo& executor);
+
+} // namespace internal {
+} // namespace executor {
+
+
namespace task {
// Validates a task that a framework attempts to launch within the
@@ -92,9 +104,12 @@ Option<Error> validate(
// Functions in this namespace are only exposed for testing.
namespace internal {
-// Validates resources of the task and executor (if present).
+// Validates resources of the task.
Option<Error> validateResources(const TaskInfo& task);
+// Validates resources of the task and its executor.
+Option<Error> validateTaskAndExecutorResources(const TaskInfo& task);
+
// Validates the kill policy of the task.
Option<Error> validateKillPolicy(const TaskInfo& task);
http://git-wip-us.apache.org/repos/asf/mesos/blob/fb58d38d/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index ad89812..4800df1 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -702,7 +702,6 @@ TEST_F(TaskValidationTest, TaskUsesCommandInfoAndExecutorInfo)
}
-// TODO(vinod): Revisit this test after `TaskGroup` validation is implemented.
TEST_F(TaskValidationTest, TaskUsesExecutorInfoWithoutCommandInfo)
{
Try<Owned<cluster::Master>> master = StartMaster();
@@ -719,6 +718,8 @@ TEST_F(TaskValidationTest, TaskUsesExecutorInfoWithoutCommandInfo)
EXPECT_CALL(sched, registered(&driver, _, _));
// Create an executor without command info.
+ // Note that we don't set type as 'CUSTOM' because it is not
+ // required for `LAUNCH` operation.
ExecutorInfo executor;
executor.mutable_executor_id()->set_value("default");
@@ -742,6 +743,49 @@ TEST_F(TaskValidationTest, TaskUsesExecutorInfoWithoutCommandInfo)
}
+// This test verifies that a scheduler cannot explicitly specify
+// a 'DEFAULT' executor when using `LAUNCH` operation.
+// TODO(vinod): Revisit this when the above is allowed.
+TEST_F(TaskValidationTest, TaskUsesDefaultExecutor)
+{
+ Try<Owned<cluster::Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _));
+
+ // Create a 'DEFAULT' executor.
+ ExecutorInfo executor;
+ executor.set_type(ExecutorInfo::DEFAULT);
+ executor.mutable_executor_id()->set_value("default");
+
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(LaunchTasks(executor, 1, 1, 16, "*"))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ Future<TaskStatus> status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&status));
+
+ driver.start();
+
+ AWAIT_READY(status);
+ EXPECT_EQ(TASK_ERROR, status->state());
+ EXPECT_TRUE(strings::startsWith(
+ status->message(), "'ExecutorInfo.type' must be 'CUSTOM'"));
+
+ driver.stop();
+ driver.join();
+}
+
+
TEST_F(TaskValidationTest, TaskUsesNoResources)
{
Try<Owned<cluster::Master>> master = StartMaster();
@@ -842,7 +886,7 @@ TEST_F(TaskValidationTest, TaskUsesMoreResourcesThanOffered)
EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason());
EXPECT_TRUE(status.get().has_message());
EXPECT_TRUE(strings::contains(
- status.get().message(), "Task uses more resources"));
+ status.get().message(), "more than available"));
driver.stop();
driver.join();
@@ -1013,7 +1057,7 @@ TEST_F(TaskValidationTest, ExecutorInfoDiffersOnSameSlave)
EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason());
EXPECT_TRUE(status.get().has_message());
EXPECT_TRUE(strings::contains(
- status.get().message(), "Task has invalid ExecutorInfo"));
+ status.get().message(), "ExecutorInfo is not compatible"));
EXPECT_CALL(exec, shutdown(_))
.Times(AtMost(1));
@@ -1190,7 +1234,7 @@ TEST_F(TaskValidationTest, TaskAndExecutorUseRevocableResources)
task.add_resources()->CopyFrom(cpus);
executor.add_resources()->CopyFrom(cpus);
task.mutable_executor()->CopyFrom(executor);
- EXPECT_NONE(task::internal::validateResources(task));
+ EXPECT_NONE(task::internal::validateTaskAndExecutorResources(task));
// Revocable cpus.
Resource revocableCpus = cpus;
@@ -1202,7 +1246,7 @@ TEST_F(TaskValidationTest, TaskAndExecutorUseRevocableResources)
executor.clear_resources();
executor.add_resources()->CopyFrom(revocableCpus);
task.mutable_executor()->CopyFrom(executor);
- EXPECT_NONE(task::internal::validateResources(task));
+ EXPECT_NONE(task::internal::validateTaskAndExecutorResources(task));
// A task with revocable cpus and its executor with non-revocable
// cpus is invalid.
@@ -1211,7 +1255,7 @@ TEST_F(TaskValidationTest, TaskAndExecutorUseRevocableResources)
executor.clear_resources();
executor.add_resources()->CopyFrom(cpus);
task.mutable_executor()->CopyFrom(executor);
- EXPECT_SOME(task::internal::validateResources(task));
+ EXPECT_SOME(task::internal::validateTaskAndExecutorResources(task));
// A task with non-revocable cpus and its executor with
// non-revocable cpus is invalid.
@@ -1220,7 +1264,7 @@ TEST_F(TaskValidationTest, TaskAndExecutorUseRevocableResources)
executor.clear_resources();
executor.add_resources()->CopyFrom(revocableCpus);
task.mutable_executor()->CopyFrom(executor);
- EXPECT_SOME(task::internal::validateResources(task));
+ EXPECT_SOME(task::internal::validateTaskAndExecutorResources(task));
}
@@ -1355,6 +1399,57 @@ TEST_F(TaskValidationTest, KillPolicyGracePeriodIsNonNegative)
// TODO(benh): Add tests which launch multiple tasks and check for
// aggregate resource usage.
+
+class ExecutorValidationTest : public MesosTest {};
+
+
+TEST_F(ExecutorValidationTest, ExecutorType)
+{
+ ExecutorInfo executorInfo;
+ executorInfo = DEFAULT_EXECUTOR_INFO;
+ executorInfo.mutable_framework_id()->set_value(UUID::random().toString());
+
+ {
+ // 'CUSTOM' executor with `CommandInfo` set is valid.
+ executorInfo.set_type(ExecutorInfo::CUSTOM);
+ executorInfo.mutable_command();
+
+ EXPECT_NONE(::executor::internal::validateType(executorInfo));
+ }
+
+ {
+ // 'CUSTOM' executor without `CommandInfo` set is invalid.
+ executorInfo.set_type(ExecutorInfo::CUSTOM);
+ executorInfo.clear_command();
+
+ Option<Error> error = ::executor::internal::validateType(executorInfo);
+ EXPECT_SOME(error);
+ EXPECT_TRUE(strings::contains(
+ error->message,
+ "'ExecutorInfo.command' must be set for 'CUSTOM' executor"));
+ }
+
+ {
+ // 'DEFAULT' executor without `CommandInfo` set is valid.
+ executorInfo.set_type(ExecutorInfo::DEFAULT);
+ executorInfo.clear_command();
+
+ EXPECT_NONE(::executor::internal::validateType(executorInfo));
+ }
+
+ {
+ // 'DEFAULT' executor with `CommandInfo` set is invalid.
+ executorInfo.set_type(ExecutorInfo::DEFAULT);
+ executorInfo.mutable_command();
+
+ Option<Error> error = ::executor::internal::validateType(executorInfo);
+ EXPECT_SOME(error);
+ EXPECT_TRUE(strings::contains(
+ error->message,
+ "'ExecutorInfo.command' must not be set for 'DEFAULT' executor"));
+ }
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {