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);