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/15 22:25:30 UTC

[4/4] mesos git commit: Made `ExecutorInfo.command` optional.

Made `ExecutorInfo.command` optional.

This is done to allow schedulers to use `ExecutorInfo` with just
`executor_id` set. This is a requirement for the upcoming support
for `TaskGroup` semantics.

Note that in this patch, TASK_ERROR is returned when
`ExecutorInfo.command` is unset to keep the existing semantics for
`LAUNCH` offer operation. This might change when we add `LAUNCH_GROUP`
operation.

Review: https://reviews.apache.org/r/51059


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c7f5d870
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c7f5d870
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c7f5d870

Branch: refs/heads/master
Commit: c7f5d870fb61f63c5ec1eb5cf0cd95022c49dee3
Parents: fc5f86a
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Aug 12 12:59:17 2016 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Mon Aug 15 15:25:05 2016 -0700

----------------------------------------------------------------------
 include/mesos/mesos.proto             | 37 ++++++++++++++++++++++++---
 include/mesos/v1/mesos.proto          | 37 ++++++++++++++++++++++++---
 src/master/validation.cpp             | 21 +++++++++++++---
 src/tests/master_validation_tests.cpp | 40 ++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c7f5d870/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index cd75038..90518a2 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -501,13 +501,43 @@ message CommandInfo {
 
 
 /**
- * Describes information about an executor. The 'data' field can be
- * used to pass arbitrary bytes to an executor.
+ * Describes information about an executor.
  */
 message ExecutorInfo {
+  enum Type {
+    UNKNOWN = 0;
+
+    // Mesos provides a simple built-in default executor that frameworks can
+    // leverage to run shell commands and containers.
+    //
+    // NOTES:
+    //
+    // 1) `command` must not be set when using a default executor.
+    //
+    // 2) If "cpu" or "mem" resources are not set, default values of 0.1 cpu
+    //    and 32 MB memory will be used; schedulers should account for these
+    //    resources when accepting offers.
+    //
+    // 3) Default executor only accepts a *single* launch operation.
+    DEFAULT = 1;
+
+    // For frameworks that need custom functionality to run tasks, a `CUSTOM`
+    // executor can be used. Note that `command` must be set when using a
+    // `CUSTOM` executor.
+    CUSTOM = 2;
+  }
+
+  // For backwards compatibility, if this field is not set when using `LAUNCH`
+  // offer operation, Mesos will infer the type by checking if `command` is
+  // set (`CUSTOM`) or unset (`DEFAULT`).
+  //
+  // TODO(vinod): Add support for explicitly setting `type` to `DEFAULT `
+  // in `LAUNCH` offer operation.
+  optional Type type = 15;
+
   required ExecutorID executor_id = 1;
   optional FrameworkID framework_id = 8; // TODO(benh): Make this required.
-  required CommandInfo command = 7;
+  optional CommandInfo command = 7;
 
   // Executor provided with a container will launch the container
   // with the executor's CommandInfo and we expect the container to
@@ -529,6 +559,7 @@ message ExecutorInfo {
   // free-form metadata instead.
   optional string source = 10 [deprecated = true]; // Since 1.0.
 
+  // This field can be used to pass arbitrary bytes to an executor.
   optional bytes data = 4;
 
   // Service discovery information for the executor. It is not

http://git-wip-us.apache.org/repos/asf/mesos/blob/c7f5d870/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index a30f877..93a4bce 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -501,13 +501,43 @@ message CommandInfo {
 
 
 /**
- * Describes information about an executor. The 'data' field can be
- * used to pass arbitrary bytes to an executor.
+ * Describes information about an executor.
  */
 message ExecutorInfo {
+  enum Type {
+    UNKNOWN = 0;
+
+    // Mesos provides a simple built-in default executor that frameworks can
+    // leverage to run shell commands and containers.
+    //
+    // NOTES:
+    //
+    // 1) `command` must not be set when using a default executor.
+    //
+    // 2) If "cpu" or "mem" resources are not set, default values of 0.1 cpu
+    //    and 32 MB memory will be used; schedulers should account for these
+    //    resources when accepting offers.
+    //
+    // 3) Default executor only accepts a *single* launch operation.
+    DEFAULT = 1;
+
+    // For frameworks that need custom functionality to run tasks, a `CUSTOM`
+    // executor can be used. Note that `command` must be set when using a
+    // `CUSTOM` executor.
+    CUSTOM = 2;
+  }
+
+  // For backwards compatibility, if this field is not set when using `LAUNCH`
+  // offer operation, Mesos will infer the type by checking if `command` is
+  // set (`CUSTOM`) or unset (`DEFAULT`).
+  //
+  // TODO(vinod): Add support for explicitly setting `type` to `DEFAULT `
+  // in `LAUNCH` offer operation.
+  optional Type type = 15;
+
   required ExecutorID executor_id = 1;
   optional FrameworkID framework_id = 8; // TODO(benh): Make this required.
-  required CommandInfo command = 7;
+  optional CommandInfo command = 7;
 
   // Executor provided with a container will launch the container
   // with the executor's CommandInfo and we expect the container to
@@ -529,6 +559,7 @@ message ExecutorInfo {
   // free-form metadata instead.
   optional string source = 10 [deprecated = true]; // Since 1.0.
 
+  // This field can be used to pass arbitrary bytes to an executor.
   optional bytes data = 4;
 
   // Service discovery information for the executor. It is not

http://git-wip-us.apache.org/repos/asf/mesos/blob/c7f5d870/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index 52d9137..ddc7ac3 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -572,10 +572,8 @@ Option<Error> validateExecutorInfo(
   }
 
   if (task.has_executor()) {
-    // The master currently expects ExecutorInfo.framework_id to be
-    // set even though it is an optional field. Currently, the
-    // scheduler driver ensures that the field is set. For schedulers
-    // not using the driver, we need to do the validation here.
+    // Master ensures `ExecutorInfo.framework_id` is set before calling
+    // this method.
     CHECK(task.executor().has_framework_id());
 
     if (task.executor().framework_id() != framework->id()) {
@@ -585,6 +583,21 @@ Option<Error> validateExecutorInfo(
           " vs Expected: " + stringify(framework->id()) + ")");
     }
 
+
+    // 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'");
+    }
+
+    // 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");
+    }
+
     const ExecutorID& executorId = task.executor().executor_id();
     Option<ExecutorInfo> executorInfo = None();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/c7f5d870/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index 9eb82a5..ad89812 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -702,6 +702,46 @@ TEST_F(TaskValidationTest, TaskUsesCommandInfoAndExecutorInfo)
 }
 
 
+// TODO(vinod): Revisit this test after `TaskGroup` validation is implemented.
+TEST_F(TaskValidationTest, TaskUsesExecutorInfoWithoutCommandInfo)
+{
+  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 an executor without command info.
+  ExecutorInfo executor;
+  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.command' must be set"));
+
+  driver.stop();
+  driver.join();
+}
+
+
 TEST_F(TaskValidationTest, TaskUsesNoResources)
 {
   Try<Owned<cluster::Master>> master = StartMaster();