Repository: mesos
Updated Branches:
  refs/heads/master 13d846034 -> 4aa3ec22c


Updated command executor to use the new CommandInfo.

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


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

Branch: refs/heads/master
Commit: 4aa3ec22cfde53bb6aaf078c7c24ea351e3b41ed
Parents: 955c64a
Author: Jie Yu <yujie....@gmail.com>
Authored: Tue Aug 12 22:13:03 2014 -0700
Committer: Jie Yu <yujie....@gmail.com>
Committed: Tue Aug 12 23:28:29 2014 -0700

----------------------------------------------------------------------
 src/launcher/executor.cpp | 55 ++++++++++++++++++++++++++++++++++++++----
 src/master/master.cpp     |  2 ++
 src/slave/slave.cpp       | 30 +++++++++++++++++++----
 3 files changed, 77 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/4aa3ec22/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index b096b68..64a4175 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -24,6 +24,7 @@
 #include <iostream>
 #include <list>
 #include <string>
+#include <vector>
 
 #include <mesos/executor.hpp>
 
@@ -117,9 +118,25 @@ public:
       return;
     }
 
+    // Sanity checks.
     CHECK(task.has_command()) << "Expecting task " << task.task_id()
                               << " to have a command!";
 
+    // TODO(jieyu): For now, we just fail the executor if the task's
+    // CommandInfo is not valid. The framework will receive
+    // TASK_FAILED for the task, and will most likely find out the
+    // cause with some debugging. This is a temporary solution. A more
+    // correct solution is to perform this validation at master side.
+    if (task.command().shell()) {
+      CHECK(task.command().has_value())
+        << "Shell command of task " << task.task_id()
+        << " is not specified!";
+    } else {
+      CHECK(task.command().has_value())
+        << "Executable of task " << task.task_id()
+        << " is not specified!";
+    }
+
     cout << "Starting task " << task.task_id() << endl;
 
     // TODO(benh): Clean this up with the new 'Fork' abstraction.
@@ -145,12 +162,30 @@ public:
       abort();
     }
 
+    // Prepare the argv before fork as it's not async signal safe.
+    char **argv = new char*[task.command().argv_size() + 1];
+    for (int i = 0; i < task.command().argv_size(); i++) {
+      argv[i] = (char*) task.command().argv(i).c_str();
+    }
+    argv[task.command().argv_size()] = NULL;
+
+    // Prepare the messages before fork as it's not async signal safe.
+    string command;
+    if (task.command().shell()) {
+      command = "sh -c '" + task.command().value() + "'";
+    } else {
+      command =
+        "[" + task.command().value() + ", " +
+        strings::join(", ", task.command().argv()) + "]";
+    }
+
     if ((pid = fork()) == -1) {
-      cerr << "Failed to fork to run '" << task.command().value() << "': "
+      cerr << "Failed to fork to run " << command << ": "
            << strerror(errno) << endl;
       abort();
     }
 
+    // TODO(jieyu): Make the child process async signal safe.
     if (pid == 0) {
       // In child process, we make cleanup easier by putting process
       // into it's own session.
@@ -183,11 +218,19 @@ public:
       os::close(pipes[1]);
 
       // The child has successfully setsid, now run the command.
-
       if (override.isNone()) {
-        cout << "sh -c '" << task.command().value() << "'" << endl;
-        execl("/bin/sh", "sh", "-c",
-              task.command().value().c_str(), (char*) NULL);
+        cout << command << endl;
+
+        if (task.command().shell()) {
+          execl(
+              "/bin/sh",
+              "sh",
+              "-c",
+              task.command().value().c_str(),
+              (char*) NULL);
+        } else {
+          execvp(task.command().value().c_str(), argv);
+        }
       } else {
         char** argv = override.get();
 
@@ -205,6 +248,8 @@ public:
       abort();
     }
 
+    delete[] argv;
+
     // In parent process.
     os::close(pipes[1]);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/4aa3ec22/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index d53d6c2..a8cf9ba 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2249,6 +2249,8 @@ Future<Option<Error> > Master::validateTask(
 
   // TODO(benh): Add a HealthCheckChecker visitor.
 
+  // TODO(jieyu): Add a CommandInfoCheck visitor.
+
   // Invoke each visitor.
   Option<Error> error = None();
   foreach (TaskInfoVisitor* visitor, taskVisitors) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/4aa3ec22/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 787bd05..59477d5 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2391,13 +2391,33 @@ ExecutorInfo Slave::getExecutorInfo(
 
     // Prepare an executor name which includes information on the
     // command being launched.
-    string name =
-      "(Task: " + task.task_id().value() + ") " + "(Command: sh -c '";
+    string name = "(Task: " + task.task_id().value() + ") ";
 
-    if (task.command().value().length() > 15) {
-      name += task.command().value().substr(0, 12) + "...')";
+    if (task.command().shell()) {
+      if (!task.command().has_value()) {
+        name += "(Command: NO COMMAND)";
+      } else {
+        name += "(Command: sh -c '";
+        if (task.command().value().length() > 15) {
+          name += task.command().value().substr(0, 12) + "...')";
+        } else {
+          name += task.command().value() + "')";
+        }
+      }
     } else {
-      name += task.command().value() + "')";
+      if (!task.command().has_value()) {
+        name += "(Command: NO EXECUTABLE)";
+      } else {
+        string args =
+          task.command().value() + ", " +
+          strings::join(", ", task.command().argv());
+
+        if (args.length() > 15) {
+          name += "(Command: [" + args.substr(0, 12) + "...])";
+        } else {
+          name += "(Command: [" + args + "])";
+        }
+      }
     }
 
     executor.set_name("Command Executor " + name);

Reply via email to