You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/07/04 17:01:22 UTC

[01/15] mesos git commit: Added an option to the launch helper binary to unshare mount namespace.

Repository: mesos
Updated Branches:
  refs/heads/master 97cc5b58c -> aecafbe0c


Added an option to the launch helper binary to unshare mount namespace.

This allows a custom executor to use this command to launch a command in
a new root filesystem without worrying about creating a new mount
namespace first. For example, the following command can be used to
launch a command (`ls -al /`) using a root filesystem (`/tmp/alpine`).

`mesos-containerizer launch                         \
    --unshare_namespace_mnt                         \
    --rootfs=/tmp/alpine                            \
    --command='{"shell":true,"value":"ls -al /"}'`

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


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

Branch: refs/heads/master
Commit: aecafbe0c94cc4c71a52550139db0cf3756d9079
Parents: 772cf40
Author: Jie Yu <yu...@gmail.com>
Authored: Sat Jul 2 22:43:43 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 19 +++++++++++++++++++
 src/slave/containerizer/mesos/launch.hpp |  3 +++
 2 files changed, 22 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/aecafbe0/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 80234aa..09733ab 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -15,6 +15,7 @@
 // limitations under the License.
 
 #include <errno.h>
+#include <sched.h>
 #include <string.h>
 
 #include <iostream>
@@ -26,6 +27,7 @@
 
 #ifdef __linux__
 #include "linux/fs.hpp"
+#include "linux/ns.hpp"
 #endif
 
 #include "mesos/mesos.hpp"
@@ -87,6 +89,13 @@ MesosContainerizerLaunch::Flags::Flags()
       "pre_exec_commands",
       "The additional preparation commands to execute before\n"
       "executing the command.");
+
+#ifdef __linux__
+  add(&unshare_namespace_mnt,
+      "unshare_namespace_mnt",
+      "Whether to launch the command in a new mount namespace.",
+      false);
+#endif // __linux__
 }
 
 
@@ -169,6 +178,16 @@ int MesosContainerizerLaunch::execute()
     }
   }
 
+#ifdef __linux__
+  if (flags.unshare_namespace_mnt) {
+    if (unshare(CLONE_NEWNS) != 0) {
+      cerr << "Failed to unshare mount namespace: "
+           << os::strerror(errno) << endl;
+      return 1;
+    }
+  }
+#endif // __linux__
+
   // Run additional preparation commands. These are run as the same
   // user and with the environment as the agent.
   if (flags.pre_exec_commands.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/aecafbe0/src/slave/containerizer/mesos/launch.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.hpp b/src/slave/containerizer/mesos/launch.hpp
index c6bba37..0e86da9 100644
--- a/src/slave/containerizer/mesos/launch.hpp
+++ b/src/slave/containerizer/mesos/launch.hpp
@@ -43,6 +43,9 @@ public:
     Option<int> pipe_read;
     Option<int> pipe_write;
     Option<JSON::Array> pre_exec_commands;
+#ifdef __linux__
+    bool unshare_namespace_mnt;
+#endif // __linux__
   };
 
   MesosContainerizerLaunch() : Subcommand(NAME) {}


[08/15] mesos git commit: Made control pipe to mesos-containerizer launch optional.

Posted by ji...@apache.org.
Made control pipe to mesos-containerizer launch optional.

This is the first step allowing the command executor to directly use
`mesos-containerizer launch` to launch user task. The control pipe is
used to synchronize with the parent process. In the command executor,
this synchronization is not needed. Therefore, we make it optional.

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


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

Branch: refs/heads/master
Commit: b3ba22eee450b98dae34520fb29868c6b8c92a08
Parents: 48ac0a5
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Jun 30 13:50:03 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 86 +++++++++++++++------------
 1 file changed, 47 insertions(+), 39 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b3ba22ee/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 83f4d7f..68f23b3 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -76,11 +76,19 @@ MesosContainerizerLaunch::Flags::Flags()
 
   add(&pipe_read,
       "pipe_read",
-      "The read end of the control pipe.");
+      "The read end of the control pipe. This is a file descriptor \n"
+      "on Posix, or a handle on Windows. It's caller's responsibility \n"
+      "to make sure the file descriptor or the handle is inherited \n"
+      "properly in the subprocess. It's used to synchronize with the \n"
+      "parent process. If not specified, no synchronization will happen.");
 
   add(&pipe_write,
       "pipe_write",
-      "The write end of the control pipe.");
+      "The write end of the control pipe. This is a file descriptor \n"
+      "on Posix, or a handle on Windows. It's caller's responsibility \n"
+      "to make sure the file descriptor or the handle is inherited \n"
+      "properly in the subprocess. It's used to synchronize with the \n"
+      "parent process. If not specified, no synchronization will happen.");
 
   add(&commands,
       "commands",
@@ -102,13 +110,13 @@ int MesosContainerizerLaunch::execute()
     return 1;
   }
 
-  if (flags.pipe_read.isNone()) {
-    cerr << "Flag --pipe_read is not specified" << endl;
-    return 1;
-  }
+  bool controlPipeSpecified =
+    flags.pipe_read.isSome() && flags.pipe_write.isSome();
 
-  if (flags.pipe_write.isNone()) {
-    cerr << "Flag --pipe_write is not specified" << endl;
+  if ((flags.pipe_read.isSome() && flags.pipe_write.isNone()) ||
+      (flags.pipe_read.isNone() && flags.pipe_write.isSome())) {
+    cerr << "Flag --pipe_read and --pipe_write should either be "
+         << "both set or both not set" << endl;
     return 1;
   }
 
@@ -134,43 +142,43 @@ int MesosContainerizerLaunch::execute()
     }
   }
 
-  int pipe[2] = { flags.pipe_read.get(), flags.pipe_write.get() };
+  if (controlPipeSpecified) {
+    int pipe[2] = { flags.pipe_read.get(), flags.pipe_write.get() };
 
-// NOTE: On windows we need to pass `HANDLE`s between processes, as
-// file descriptors are not unique across processes. Here we convert
-// back from from the `HANDLE`s we receive to fds that can be used in
-// os-agnostic code.
+    // NOTE: On windows we need to pass `HANDLE`s between processes,
+    // as file descriptors are not unique across processes. Here we
+    // convert back from from the `HANDLE`s we receive to fds that can
+    // be used in os-agnostic code.
 #ifdef __WINDOWS__
-  pipe[0] = os::handle_to_fd(pipe[0], _O_RDONLY | _O_TEXT);
-  pipe[1] = os::handle_to_fd(pipe[1], _O_TEXT);
+    pipe[0] = os::handle_to_fd(pipe[0], _O_RDONLY | _O_TEXT);
+    pipe[1] = os::handle_to_fd(pipe[1], _O_TEXT);
 #endif // __WINDOWS__
 
-  Try<Nothing> close = os::close(pipe[1]);
-  if (close.isError()) {
-    cerr << "Failed to close pipe[1]: " << close.error() << endl;
-    return 1;
-  }
+    Try<Nothing> close = os::close(pipe[1]);
+    if (close.isError()) {
+      cerr << "Failed to close pipe[1]: " << close.error() << endl;
+      return 1;
+    }
 
-  // Do a blocking read on the pipe until the parent signals us to continue.
-  char dummy;
-  ssize_t length;
-  while ((length = os::read(
-              pipe[0],
-              &dummy,
-              sizeof(dummy))) == -1 &&
-          errno == EINTR);
-
-  if (length != sizeof(dummy)) {
-     // There's a reasonable probability this will occur during agent
-     // restarts across a large/busy cluster.
-     cerr << "Failed to synchronize with agent (it's probably exited)" << endl;
-     return 1;
-  }
+    // Do a blocking read on the pipe until the parent signals us to continue.
+    char dummy;
+    ssize_t length;
+    while ((length = os::read(pipe[0], &dummy, sizeof(dummy))) == -1 &&
+           errno == EINTR);
+
+    if (length != sizeof(dummy)) {
+       // There's a reasonable probability this will occur during
+       // agent restarts across a large/busy cluster.
+       cerr << "Failed to synchronize with agent "
+            << "(it's probably exited)" << endl;
+       return 1;
+    }
 
-  close = os::close(pipe[0]);
-  if (close.isError()) {
-    cerr << "Failed to close pipe[0]: " << close.error() << endl;
-    return 1;
+    close = os::close(pipe[0]);
+    if (close.isError()) {
+      cerr << "Failed to close pipe[0]: " << close.error() << endl;
+      return 1;
+    }
   }
 
   // Run additional preparation commands. These are run as the same


[11/15] mesos git commit: Renamed healthCheckDir to launcherDir in command executor.

Posted by ji...@apache.org.
Renamed healthCheckDir to launcherDir in command executor.

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


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

Branch: refs/heads/master
Commit: 40ec90707edc7cd2eecf5df9fd2ca2c0ef5eb26f
Parents: 96f8498
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 16:29:18 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/executor.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/40ec9070/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 85de2a1..f78f0bf 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -134,7 +134,7 @@ class CommandExecutor: public ProtobufProcess<CommandExecutor>
 {
 public:
   CommandExecutor(
-      const string& _healthCheckDir,
+      const string& _launcherDir,
       const Option<string>& _rootfs,
       const Option<string>& _sandboxDirectory,
       const Option<string>& _workingDirectory,
@@ -152,7 +152,7 @@ public:
       shutdownGracePeriod(_shutdownGracePeriod),
       frameworkInfo(None()),
       taskId(None()),
-      healthCheckDir(_healthCheckDir),
+      launcherDir(_launcherDir),
       rootfs(_rootfs),
       sandboxDirectory(_sandboxDirectory),
       workingDirectory(_workingDirectory),
@@ -739,7 +739,7 @@ private:
   Option<KillPolicy> killPolicy;
   Option<FrameworkInfo> frameworkInfo;
   Option<TaskID> taskId;
-  string healthCheckDir;
+  string launcherDir;
   Option<string> rootfs;
   Option<string> sandboxDirectory;
   Option<string> workingDirectory;


[04/15] mesos git commit: Added an abstraction os::raw::Argv in stout.

Posted by ji...@apache.org.
Added an abstraction os::raw::Argv in stout.

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


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

Branch: refs/heads/master
Commit: 7e63b80804d0d1d2156f28eacc2f6ccbd6ff6c31
Parents: 72a4752
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 29 21:47:10 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/Makefile.am           |  1 +
 3rdparty/stout/include/stout/os.hpp          |  3 +
 3rdparty/stout/include/stout/os/raw/argv.hpp | 78 +++++++++++++++++++++++
 3rdparty/stout/tests/os_tests.cpp            | 12 ++++
 4 files changed, 94 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7e63b808/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/Makefile.am b/3rdparty/stout/include/Makefile.am
index f10c836..4768064 100644
--- a/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/stout/include/Makefile.am
@@ -129,6 +129,7 @@ nobase_include_HEADERS =			\
   stout/os/posix/stat.hpp			\
   stout/os/posix/su.hpp				\
   stout/os/posix/write.hpp			\
+  stout/os/raw/argv.hpp				\
   stout/os/raw/environment.hpp			\
   stout/os/windows/bootid.hpp			\
   stout/os/windows/chroot.hpp			\

http://git-wip-us.apache.org/repos/asf/mesos/blob/7e63b808/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os.hpp b/3rdparty/stout/include/stout/os.hpp
index 53b0093..facf183 100644
--- a/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/stout/include/stout/os.hpp
@@ -64,6 +64,9 @@
 #include <stout/os/touch.hpp>
 #include <stout/os/utime.hpp>
 
+#include <stout/os/raw/argv.hpp>
+#include <stout/os/raw/environment.hpp>
+
 // For readability, we minimize the number of #ifdef blocks in the code by
 // splitting platform specifc system calls into separate directories.
 #ifdef __WINDOWS__

http://git-wip-us.apache.org/repos/asf/mesos/blob/7e63b808/3rdparty/stout/include/stout/os/raw/argv.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/raw/argv.hpp b/3rdparty/stout/include/stout/os/raw/argv.hpp
new file mode 100644
index 0000000..3ac6216
--- /dev/null
+++ b/3rdparty/stout/include/stout/os/raw/argv.hpp
@@ -0,0 +1,78 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//  http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __STOUT_OS_RAW_ARGV_HPP__
+#define __STOUT_OS_RAW_ARGV_HPP__
+
+#include <string>
+#include <vector>
+
+#include <stout/foreach.hpp>
+
+namespace os {
+namespace raw {
+
+/**
+ * Represent the argument list expected by `execv` routines. The
+ * argument list is an array of pointers that point to null-terminated
+ * strings. The array of pointers must be terminated by a nullptr. To
+ * use this abstraction, see the following example:
+ *
+ *   vector<string> args = {"arg0", "arg1"};
+ *   os::raw::Argv argv(args);
+ *   execvp("my_binary", argv);
+ */
+class Argv
+{
+public:
+  template <typename Iterable>
+  explicit Argv(const Iterable& iterable)
+  {
+    std::vector<char*> _argv;
+    foreach (const std::string& arg, iterable) {
+      char* _arg = new char[arg.size() + 1];
+
+      // NOTE: strcpy will fill the terminating `\0' character.
+      strcpy(_arg, arg.c_str());
+      _argv.emplace_back(_arg);
+    }
+
+    size = _argv.size();
+    argv = new char*[size + 1];
+    for (size_t i = 0; i < size; i++) {
+      argv[i] = _argv[i];
+    }
+    argv[size] = nullptr;
+  }
+
+  ~Argv()
+  {
+    for (size_t i = 0; i < size; i++) {
+      delete[] argv[i];
+    }
+    delete[] argv;
+  }
+
+  operator char**()
+  {
+    return argv;
+  }
+
+private:
+  char** argv;
+  size_t size;
+};
+
+} // namespace raw {
+} // namespace os {
+
+#endif // __STOUT_OS_RAW_ARGV_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/7e63b808/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp
index 1dedff2..e674440 100644
--- a/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/stout/tests/os_tests.cpp
@@ -102,6 +102,18 @@ TEST_F(OsTest, Environment)
 }
 
 
+TEST_F(OsTest, Argv)
+{
+  vector<string> args = {"arg0", "arg1", "arg2"};
+
+  os::raw::Argv _argv(args);
+  char** argv = _argv;
+  for (size_t i = 0; i < args.size(); i++) {
+    EXPECT_EQ(args[i], argv[i]);
+  }
+}
+
+
 TEST_F(OsTest, System)
 {
   EXPECT_EQ(0, os::system("exit 0"));


[14/15] mesos git commit: Simplified a flag in the launch helper binary.

Posted by ji...@apache.org.
Simplified a flag in the launch helper binary.

Using JSON::Object is not necessary. Also, the current name is quite
confusing as we have another flag called '--command'. This patch
simplified this flag.

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


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

Branch: refs/heads/master
Commit: 772cf4007745680a39c4a7bbf820397b194995d2
Parents: 9062dbc
Author: Jie Yu <yu...@gmail.com>
Authored: Sat Jul 2 21:49:22 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 10 +++-------
 src/slave/containerizer/mesos/launch.cpp        | 19 ++++---------------
 src/slave/containerizer/mesos/launch.hpp        |  2 +-
 src/tests/containerizer/port_mapping_tests.cpp  |  8 +++-----
 4 files changed, 11 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/772cf400/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 4fa10a9..856dd82 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1163,7 +1163,7 @@ Future<bool> MesosContainerizerProcess::__launch(
     environment[variable.name()] = variable.value();
   }
 
-  JSON::Array commandArray;
+  JSON::Array preExecCommands;
 
   // TODO(jieyu): We should use Option here. If no namespace is
   // required, we should pass None() to 'launcher->fork'.
@@ -1177,7 +1177,7 @@ Future<bool> MesosContainerizerProcess::__launch(
     // Populate the list of additional commands to be run inside the container
     // context.
     foreach (const CommandInfo& command, launchInfo->commands()) {
-      commandArray.values.emplace_back(JSON::protobuf(command));
+      preExecCommands.values.emplace_back(JSON::protobuf(command));
     }
 
     // Process additional environment variables returned by isolators.
@@ -1193,10 +1193,6 @@ Future<bool> MesosContainerizerProcess::__launch(
     }
   }
 
-  // TODO(jieyu): Use JSON::Array once we have generic parse support.
-  JSON::Object commands;
-  commands.values["commands"] = commandArray;
-
   if (executorLaunchCommand.isNone()) {
     executorLaunchCommand = executorInfo.command();
   }
@@ -1269,7 +1265,7 @@ Future<bool> MesosContainerizerProcess::__launch(
     launchFlags.pipe_read = os::fd_to_handle(pipes[0]);
     launchFlags.pipe_write = os::fd_to_handle(pipes[1]);
 #endif // __WINDOWS
-    launchFlags.commands = commands;
+    launchFlags.pre_exec_commands = preExecCommands;
 
     VLOG(1) << "Launching '" << MESOS_CONTAINERIZER << "' with flags '"
             << launchFlags << "'";

http://git-wip-us.apache.org/repos/asf/mesos/blob/772cf400/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 4e52807..80234aa 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -83,8 +83,8 @@ MesosContainerizerLaunch::Flags::Flags()
       "properly in the subprocess. It's used to synchronize with the \n"
       "parent process. If not specified, no synchronization will happen.");
 
-  add(&commands,
-      "commands",
+  add(&pre_exec_commands,
+      "pre_exec_commands",
       "The additional preparation commands to execute before\n"
       "executing the command.");
 }
@@ -171,20 +171,9 @@ int MesosContainerizerLaunch::execute()
 
   // Run additional preparation commands. These are run as the same
   // user and with the environment as the agent.
-  if (flags.commands.isSome()) {
+  if (flags.pre_exec_commands.isSome()) {
     // TODO(jieyu): Use JSON::Array if we have generic parse support.
-    JSON::Object object = flags.commands.get();
-    if (object.values.count("commands") == 0) {
-      cerr << "Invalid JSON format for flag --commands" << endl;
-      return 1;
-    }
-
-    if (!object.values["commands"].is<JSON::Array>()) {
-      cerr << "Invalid JSON format for flag --commands" << endl;
-      return 1;
-    }
-
-    JSON::Array array = object.values["commands"].as<JSON::Array>();
+    JSON::Array array = flags.pre_exec_commands.get();
     foreach (const JSON::Value& value, array.values) {
       if (!value.is<JSON::Object>()) {
         cerr << "Invalid JSON format for flag --commands" << endl;

http://git-wip-us.apache.org/repos/asf/mesos/blob/772cf400/src/slave/containerizer/mesos/launch.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.hpp b/src/slave/containerizer/mesos/launch.hpp
index 72b99ad..c6bba37 100644
--- a/src/slave/containerizer/mesos/launch.hpp
+++ b/src/slave/containerizer/mesos/launch.hpp
@@ -42,7 +42,7 @@ public:
 #endif // __WINDOWS__
     Option<int> pipe_read;
     Option<int> pipe_write;
-    Option<JSON::Object> commands; // Additional preparation commands.
+    Option<JSON::Array> pre_exec_commands;
   };
 
   MesosContainerizerLaunch() : Subcommand(NAME) {}

http://git-wip-us.apache.org/repos/asf/mesos/blob/772cf400/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index b34d5b0..eeb8938 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -327,12 +327,10 @@ protected:
       return Error("No valid commands inside ContainerLaunchInfo.");
     }
 
-    JSON::Object commands;
-    JSON::Array array;
-    array.values.push_back(JSON::protobuf(launchInfo->commands(0)));
-    commands.values["commands"] = array;
+    JSON::Array preExecCommands;
+    preExecCommands.values.push_back(JSON::protobuf(launchInfo->commands(0)));
 
-    launchFlags.commands = commands;
+    launchFlags.pre_exec_commands = preExecCommands;
 
     vector<string> argv(2);
     argv[0] = MESOS_CONTAINERIZER;


[12/15] mesos git commit: Moved MESOS_CONTAINERIZER to a separate constants file.

Posted by ji...@apache.org.
Moved MESOS_CONTAINERIZER to a separate constants file.

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


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

Branch: refs/heads/master
Commit: dc5e7c4bcfaeecded1376a3295ef8ec8506b3ec6
Parents: 0e009a6
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 11:00:16 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/Makefile.am                                 |  1 +
 src/slave/containerizer/mesos/constants.hpp     | 34 ++++++++++++++++++++
 src/slave/containerizer/mesos/containerizer.cpp |  6 +---
 src/slave/containerizer/mesos/containerizer.hpp |  2 --
 src/tests/containerizer/port_mapping_tests.cpp  |  1 +
 5 files changed, 37 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/dc5e7c4b/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 3522b77..28dd151 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -969,6 +969,7 @@ libmesos_no_3rdparty_la_SOURCES +=					\
   slave/containerizer/docker.hpp					\
   slave/containerizer/external_containerizer.hpp			\
   slave/containerizer/fetcher.hpp					\
+  slave/containerizer/mesos/constants.hpp				\
   slave/containerizer/mesos/containerizer.hpp				\
   slave/containerizer/mesos/isolator.hpp				\
   slave/containerizer/mesos/launch.hpp					\

http://git-wip-us.apache.org/repos/asf/mesos/blob/dc5e7c4b/src/slave/containerizer/mesos/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/constants.hpp b/src/slave/containerizer/mesos/constants.hpp
new file mode 100644
index 0000000..5be3950
--- /dev/null
+++ b/src/slave/containerizer/mesos/constants.hpp
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __MESOS_CONTAINERIZER_CONSTANTS_HPP__
+#define __MESOS_CONTAINERIZER_CONSTANTS_HPP__
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+#ifndef __WINDOWS__
+constexpr char MESOS_CONTAINERIZER[] = "mesos-containerizer";
+#else
+constexpr char MESOS_CONTAINERIZER[] = "mesos-containerizer.exe";
+#endif // __WINDOWS__
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
+
+#endif  // __MESOS_CONTAINERIZER_CONSTANTS_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/dc5e7c4b/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 04daddd..4fa10a9 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -105,6 +105,7 @@
 #include "slave/containerizer/mesos/isolators/network/port_mapping.hpp"
 #endif
 
+#include "slave/containerizer/mesos/constants.hpp"
 #include "slave/containerizer/mesos/containerizer.hpp"
 #include "slave/containerizer/mesos/launch.hpp"
 #include "slave/containerizer/mesos/provisioner/provisioner.hpp"
@@ -135,11 +136,6 @@ using state::FrameworkState;
 using state::ExecutorState;
 using state::RunState;
 
-#ifndef __WINDOWS__
-const char MESOS_CONTAINERIZER[] = "mesos-containerizer";
-#else
-const char MESOS_CONTAINERIZER[] = "mesos-containerizer.exe";
-#endif // __WINDOWS__
 
 Try<MesosContainerizer*> MesosContainerizer::create(
     const Flags& flags,

http://git-wip-us.apache.org/repos/asf/mesos/blob/dc5e7c4b/src/slave/containerizer/mesos/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.hpp b/src/slave/containerizer/mesos/containerizer.hpp
index 70f6d35..3274e09 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -44,8 +44,6 @@ namespace mesos {
 namespace internal {
 namespace slave {
 
-extern const char MESOS_CONTAINERIZER[];
-
 // Forward declaration.
 class MesosContainerizerProcess;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/dc5e7c4b/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index 2fbb9ec..b34d5b0 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -57,6 +57,7 @@
 
 #include "slave/containerizer/fetcher.hpp"
 
+#include "slave/containerizer/mesos/constants.hpp"
 #include "slave/containerizer/mesos/containerizer.hpp"
 #include "slave/containerizer/mesos/launch.hpp"
 #include "slave/containerizer/mesos/launcher.hpp"


[03/15] mesos git commit: Used the argv version for command that launches the command executor.

Posted by ji...@apache.org.
Used the argv version for command that launches the command executor.

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


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

Branch: refs/heads/master
Commit: 96f8498140c36440c342adf66a2a5965669e797f
Parents: dc5e7c4
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 16:23:04 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp       | 18 +++++++-----------
 src/tests/slave_tests.cpp |  6 +++---
 2 files changed, 10 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/96f84981/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index da643e6..6857c87 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3949,18 +3949,15 @@ ExecutorInfo Slave::getExecutorInfo(
       executor.mutable_command()->set_user(task.command().user());
     }
 
-    Result<string> path =
-      os::realpath(path::join(flags.launcher_dir, "mesos-executor"));
-
-    // Explicitly set 'shell' to true since we want to use the shell
-    // for running the mesos-executor (and even though this is the
-    // default we want to be explicit).
-    executor.mutable_command()->set_shell(true);
+    Result<string> path = os::realpath(
+        path::join(flags.launcher_dir, "mesos-executor"));
 
     if (path.isSome()) {
+      executor.mutable_command()->set_shell(false);
+      executor.mutable_command()->set_value(path.get());
+      executor.mutable_command()->add_arguments("mesos-executor");
+
       if (hasRootfs) {
-        executor.mutable_command()->set_shell(false);
-        executor.mutable_command()->add_arguments("mesos-executor");
         executor.mutable_command()->add_arguments(
             "--sandbox_directory=" + flags.sandbox_directory);
 
@@ -3981,9 +3978,8 @@ ExecutorInfo Slave::getExecutorInfo(
         }
 #endif // __WINDOWS__
       }
-
-      executor.mutable_command()->set_value(path.get());
     } else {
+      executor.mutable_command()->set_shell(true);
       executor.mutable_command()->set_value(
           "echo '" +
           (path.isError() ? path.error() : "No such file or directory") +

http://git-wip-us.apache.org/repos/asf/mesos/blob/96f84981/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index d800cb8..868e521 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -543,8 +543,8 @@ TEST_F(SlaveTest, GetExecutorInfo)
 
   // Now assert that it actually is running mesos-executor without any
   // bleedover from the command we intend on running.
-  EXPECT_TRUE(executor.command().shell());
-  EXPECT_EQ(0, executor.command().arguments_size());
+  EXPECT_FALSE(executor.command().shell());
+  EXPECT_EQ(1, executor.command().arguments_size());
   ASSERT_TRUE(executor.has_labels());
   EXPECT_EQ(2, executor.labels().labels_size());
   ASSERT_TRUE(executor.has_discovery());
@@ -595,7 +595,7 @@ TEST_F(SlaveTest, GetExecutorInfoForTaskWithContainer)
   const ExecutorInfo& executor = slave.getExecutorInfo(frameworkInfo, task);
 
   // Now assert that the executor has both the command and ContainerInfo
-  EXPECT_TRUE(executor.command().shell());
+  EXPECT_FALSE(executor.command().shell());
   // CommandInfo.container is not included. In this test the ContainerInfo
   // must be included in Executor.container (copied from TaskInfo.container).
   EXPECT_TRUE(executor.has_container());


[10/15] mesos git commit: Fixed a const ref issue in command executor.

Posted by ji...@apache.org.
Fixed a const ref issue in command executor.

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


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

Branch: refs/heads/master
Commit: 0e009a688b80b1908253b397263a41126e8cd4c1
Parents: d5247fb
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Jun 30 17:05:03 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/posix/executor.cpp | 6 +++---
 src/launcher/posix/executor.hpp | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0e009a68/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index 3ced30f..7dd47ea 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -44,9 +44,9 @@ namespace internal {
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
     const Option<string>& user,
-    Option<string>& rootfs,
-    Option<string>& sandboxDirectory,
-    Option<string>& workingDirectory)
+    const Option<string>& rootfs,
+    const Option<string>& sandboxDirectory,
+    const Option<string>& workingDirectory)
 {
   if (rootfs.isSome()) {
     // The command executor is responsible for chrooting into the

http://git-wip-us.apache.org/repos/asf/mesos/blob/0e009a68/src/launcher/posix/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.hpp b/src/launcher/posix/executor.hpp
index 1ff2437..8121bdb 100644
--- a/src/launcher/posix/executor.hpp
+++ b/src/launcher/posix/executor.hpp
@@ -30,9 +30,9 @@ namespace internal {
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
     const Option<std::string>& user,
-    Option<std::string>& rootfs,
-    Option<std::string>& sandboxDirectory,
-    Option<std::string>& workingDirectory);
+    const Option<std::string>& rootfs,
+    const Option<std::string>& sandboxDirectory,
+    const Option<std::string>& workingDirectory);
 
 } // namespace internal {
 } // namespace v1 {


[05/15] mesos git commit: Removed the argv parameter in command executor helper.

Posted by ji...@apache.org.
Removed the argv parameter in command executor helper.

It's confusing to pass in 'argv' because it's already contained in
'command'. This patch removed this parameter.

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


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

Branch: refs/heads/master
Commit: 72a475251403e530c3279f435c17cbf8baa9cc31
Parents: 97cc5b5
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 29 17:40:19 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/executor.cpp         | 11 -----------
 src/launcher/posix/executor.cpp   | 10 +++++++++-
 src/launcher/posix/executor.hpp   |  1 -
 src/launcher/windows/executor.cpp | 12 +++++++++++-
 src/launcher/windows/executor.hpp |  1 -
 5 files changed, 20 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 6301025..85de2a1 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -390,18 +390,10 @@ protected:
 
     cout << "Starting task " << task->task_id() << endl;
 
-    // Prepare the argv before fork as it's not async signal safe.
-    char **argv = new char*[command.arguments().size() + 1];
-    for (int i = 0; i < command.arguments().size(); i++) {
-      argv[i] = (char*) command.arguments(i).c_str();
-    }
-    argv[command.arguments().size()] = nullptr;
-
 #ifndef __WINDOWS__
     pid = launchTaskPosix(
         command,
         user,
-        argv,
         rootfs,
         sandboxDirectory,
         workingDirectory);
@@ -411,7 +403,6 @@ protected:
     // open the reap function will work.
     PROCESS_INFORMATION processInformation = launchTaskWindows(
         command,
-        argv,
         rootfs);
 
     pid = processInformation.dwProcessId;
@@ -420,8 +411,6 @@ protected:
     processHandle = processInformation.hProcess;
 #endif
 
-    delete[] argv;
-
     cout << "Forked command at " << pid << endl;
 
     if (task->has_health_check()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index ab1dd93..805280e 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -42,7 +42,6 @@ namespace internal {
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
     const Option<string>& user,
-    char** argv,
     Option<string>& rootfs,
     Option<string>& sandboxDirectory,
     Option<string>& workingDirectory)
@@ -81,6 +80,13 @@ pid_t launchTaskPosix(
         strings::join(", ", command.arguments())).get();
   }
 
+  // Prepare the argv before fork as it's not async signal safe.
+  char **argv = new char*[command.arguments().size() + 1];
+  for (int i = 0; i < command.arguments().size(); i++) {
+    argv[i] = (char*) command.arguments(i).c_str();
+  }
+  argv[command.arguments().size()] = nullptr;
+
   pid_t pid;
   if ((pid = fork()) == -1) {
     ABORT("Failed to fork to run '" + commandString + "'"
@@ -208,6 +214,8 @@ pid_t launchTaskPosix(
     ABORT("Failed to exec: " + os::strerror(errno));
   }
 
+  delete[] argv;
+
   return pid;
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/posix/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.hpp b/src/launcher/posix/executor.hpp
index e5f3c0a..1ff2437 100644
--- a/src/launcher/posix/executor.hpp
+++ b/src/launcher/posix/executor.hpp
@@ -30,7 +30,6 @@ namespace internal {
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
     const Option<std::string>& user,
-    char** argv,
     Option<std::string>& rootfs,
     Option<std::string>& sandboxDirectory,
     Option<std::string>& workingDirectory);

http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/windows/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/windows/executor.cpp b/src/launcher/windows/executor.cpp
index f6da398..b008052 100644
--- a/src/launcher/windows/executor.cpp
+++ b/src/launcher/windows/executor.cpp
@@ -35,7 +35,6 @@ namespace internal {
 
 PROCESS_INFORMATION launchTaskWindows(
     const CommandInfo& command,
-    char** argv,
     Option<string>& rootfs)
 {
   PROCESS_INFORMATION processInfo;
@@ -67,7 +66,18 @@ PROCESS_INFORMATION launchTaskWindows(
   } else {
     // Not a shell command, execute as-is.
     executable = command.value();
+
+    // TODO(jieyu): Consider allowing os::stringify_args to take
+    // `command.arguments()` directly.
+    char **argv = new char*[command.arguments().size() + 1];
+    for (int i = 0; i < command.arguments().size(); i++) {
+      argv[i] = (char*) command.arguments(i).c_str();
+    }
+    argv[command.arguments().size()] = nullptr;
+
     commandLine = os::stringify_args(argv);
+
+    delete[] argv;
   }
 
   cout << commandLine << endl;

http://git-wip-us.apache.org/repos/asf/mesos/blob/72a47525/src/launcher/windows/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/windows/executor.hpp b/src/launcher/windows/executor.hpp
index 04484b6..bae44f5 100644
--- a/src/launcher/windows/executor.hpp
+++ b/src/launcher/windows/executor.hpp
@@ -29,7 +29,6 @@ namespace internal {
 
 PROCESS_INFORMATION launchTaskWindows(
     const CommandInfo& command,
-    char** argv,
     Option<std::string>& rootfs);
 
 } // namespace internal {


[02/15] mesos git commit: Used the launch helper binary to launch user task in command executor.

Posted by ji...@apache.org.
Used the launch helper binary to launch user task in command executor.

Currently, command executor and 'mesos-containerizer launch' share a
lot of the logic. Command executor should in fact, just use
`mesos-containerizer launch` to launch the user task. Potentially,
'mesos-containerizer launch' can be also used by custom executor to
launch user tasks.

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


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

Branch: refs/heads/master
Commit: 9062dbccd6135802cb6925eaad9807e936235b5a
Parents: 7ded2aa
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 17:57:04 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/executor.cpp                |   1 +
 src/launcher/posix/executor.cpp          | 197 +++++++-------------------
 src/launcher/posix/executor.hpp          |   1 +
 src/slave/containerizer/mesos/launch.cpp |  18 ++-
 4 files changed, 64 insertions(+), 153 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9062dbcc/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 78c4182..5a5f95f 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -393,6 +393,7 @@ protected:
 #ifndef __WINDOWS__
     pid = launchTaskPosix(
         command,
+        launcherDir,
         user,
         rootfs,
         sandboxDirectory,

http://git-wip-us.apache.org/repos/asf/mesos/blob/9062dbcc/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index 7dd47ea..6814b9f 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -14,35 +14,51 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "launcher/posix/executor.hpp"
-
 #include <iostream>
 
+#include <process/subprocess.hpp>
+
 #include <stout/os.hpp>
+#include <stout/protobuf.hpp>
 #include <stout/strings.hpp>
 
 #include <stout/os/raw/argv.hpp>
 
+#include "internal/devolve.hpp"
+
+#include "launcher/posix/executor.hpp"
+
 #ifdef __linux__
 #include "linux/fs.hpp"
 #endif
 
+#include "slave/containerizer/mesos/constants.hpp"
+#include "slave/containerizer/mesos/launch.hpp"
+
 #ifdef __linux__
 namespace fs = mesos::internal::fs;
 #endif
 
+using process::SETSID;
+using process::Subprocess;
+
 using std::cout;
 using std::cerr;
 using std::endl;
 using std::string;
 using std::vector;
 
+using mesos::internal::devolve;
+using mesos::internal::slave::MESOS_CONTAINERIZER;
+using mesos::internal::slave::MesosContainerizerLaunch;
+
 namespace mesos {
 namespace v1 {
 namespace internal {
 
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
+    const string& launcherDir,
     const Option<string>& user,
     const Option<string>& rootfs,
     const Option<string>& sandboxDirectory,
@@ -66,153 +82,48 @@ pid_t launchTaskPosix(
 #endif // __linux__
   }
 
-  // Prepare the command log message.
-  string commandString;
-
-  if (command.shell()) {
-    commandString = strings::format(
-        "%s %s '%s'",
-        os::Shell::arg0,
-        os::Shell::arg1,
-        command.value()).get();
-  } else {
-    commandString = strings::format(
-        "[%s, %s]",
-        command.value(),
-        strings::join(", ", command.arguments())).get();
-  }
+  // Prepare the flags to pass to the launch process.
+  MesosContainerizerLaunch::Flags launchFlags;
 
-  // Prepare the argv before fork as it's not async signal safe.
-  os::raw::Argv argv(command.arguments());
+  launchFlags.command = JSON::protobuf(devolve(command));
 
-  pid_t pid;
-  if ((pid = fork()) == -1) {
-    ABORT("Failed to fork to run '" + commandString + "'"
-          ": " + os::strerror(errno));
+  if (rootfs.isSome()) {
+    CHECK_SOME(sandboxDirectory);
+    launchFlags.working_directory = workingDirectory.isSome()
+      ? workingDirectory
+      : sandboxDirectory;
   }
 
-  // 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.
-    // NOTE: POSIX guarantees a forked child's pid does not match any
-    // existing process group id so only a single `setsid()` is
-    // required and the session id will be the pid.
-    if (::setsid() == -1) {
-      ABORT("Failed to put child in a new session: " + os::strerror(errno));
-    }
-
-    if (rootfs.isSome()) {
-      // NOTE: we need to put change user, chdir logics in command
-      // executor because these depend on the new root filesystem.
-      // If the command task does not change root fiesystem, these
-      // will be handled in the containerizer.
-#ifdef __linux__
-      // NOTE: If 'user' is set, we will get the uid, gid, and the
-      // supplementary group ids associated with the specified user
-      // before changing the filesystem root. This is because after
-      // changing the filesystem root, the current process might no
-      // longer have access to /etc/passwd and /etc/group on the
-      // host.
-      Option<uid_t> uid;
-      Option<gid_t> gid;
-      vector<gid_t> gids;
-
-      // TODO(gilbert): For the case container user exists, support
-      // framework/task/default user -> container user mapping once
-      // user namespace and container capabilities is available for
-      // mesos container.
-
-      if (user.isSome()) {
-        Result<uid_t> _uid = os::getuid(user.get());
-        if (!_uid.isSome()) {
-          ABORT("Failed to get the uid of user '" + user.get() + "': " +
-                (_uid.isError() ? _uid.error() : "not found"));
-        }
-
-        // No need to change user/groups if the specified user is
-        // the same as that of the current process.
-        if (_uid.get() != os::getuid().get()) {
-          Result<gid_t> _gid = os::getgid(user.get());
-          if (!_gid.isSome()) {
-            ABORT("Failed to get the gid of user '" + user.get() + "': " +
-                  (_gid.isError() ? _gid.error() : "not found"));
-          }
-
-          Try<vector<gid_t>> _gids = os::getgrouplist(user.get());
-          if (_gids.isError()) {
-            ABORT("Failed to get the supplementary gids of "
-                  "user '" + user.get() + "': " +
-                  (_gids.isError() ? _gids.error() : "not found"));
-          }
-
-          uid = _uid.get();
-          gid = _gid.get();
-          gids = _gids.get();
-        }
-      }
-
-      Try<Nothing> chroot = fs::chroot::enter(rootfs.get());
-      if (chroot.isError()) {
-        ABORT("Failed to enter chroot '" + rootfs.get() + "'"
-              ": " + chroot.error());
-      }
-
-      if (uid.isSome()) {
-        Try<Nothing> setgid = os::setgid(gid.get());
-        if (setgid.isError()) {
-          ABORT("Failed to set gid to " + stringify(gid.get()) +
-                ": " + setgid.error());
-        }
-
-        Try<Nothing> setgroups = os::setgroups(gids, uid);
-        if (setgroups.isError()) {
-          ABORT("Failed to set supplementary gids: " + setgroups.error());
-        }
-
-        Try<Nothing> setuid = os::setuid(uid.get());
-        if (setuid.isError()) {
-          ABORT("Failed to set uid to " + stringify(uid.get()) +
-                ": " + setuid.error());
-        }
-      }
-
-      // Determine the current working directory for the executor.
-      string cwd;
-      if (workingDirectory.isSome()) {
-        cwd = workingDirectory.get();
-      } else {
-        CHECK_SOME(sandboxDirectory);
-        cwd = sandboxDirectory.get();
-      }
-
-      Try<Nothing> chdir = os::chdir(cwd);
-      if (chdir.isError()) {
-        ABORT("Failed to chdir into current working directory "
-              "'" + cwd + "': " + chdir.error());
-      }
-#else
-      ABORT("Rootfs is only supported on Linux");
-#endif // __linux__
-    }
-
-    cout << commandString << endl;
-
-    // The child has successfully setsid, now run the command.
-    if (command.shell()) {
-      execlp(os::Shell::name,
-             os::Shell::arg0,
-             os::Shell::arg1,
-             command.value().c_str(),
-             (char*) nullptr);
-    } else {
-      execvp(command.value().c_str(), argv);
-    }
-
-    ABORT("Failed to exec: " + os::strerror(errno));
+  launchFlags.rootfs = rootfs;
+  launchFlags.user = user;
+
+  string commandString = strings::format(
+      "%s %s %s",
+      path::join(launcherDir, MESOS_CONTAINERIZER),
+      MesosContainerizerLaunch::NAME,
+      stringify(launchFlags)).get();
+
+  // Fork the child using launcher.
+  vector<string> argv(2);
+  argv[0] = MESOS_CONTAINERIZER;
+  argv[1] = MesosContainerizerLaunch::NAME;
+
+  Try<Subprocess> s = subprocess(
+      path::join(launcherDir, MESOS_CONTAINERIZER),
+      argv,
+      Subprocess::FD(STDIN_FILENO),
+      Subprocess::FD(STDOUT_FILENO),
+      Subprocess::FD(STDERR_FILENO),
+      SETSID,
+      launchFlags);
+
+  if (s.isError()) {
+    ABORT("Failed to launch '" + commandString + "': " + s.error());
   }
 
-  return pid;
+  cout << commandString << endl;
+
+  return s->pid();
 }
 
 } // namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9062dbcc/src/launcher/posix/executor.hpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.hpp b/src/launcher/posix/executor.hpp
index 8121bdb..a508089 100644
--- a/src/launcher/posix/executor.hpp
+++ b/src/launcher/posix/executor.hpp
@@ -29,6 +29,7 @@ namespace internal {
 
 pid_t launchTaskPosix(
     const mesos::v1::CommandInfo& command,
+    const std::string& launcherDir,
     const Option<std::string>& user,
     const Option<std::string>& rootfs,
     const Option<std::string>& sandboxDirectory,

http://git-wip-us.apache.org/repos/asf/mesos/blob/9062dbcc/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index a08a206..4e52807 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -347,19 +347,17 @@ int MesosContainerizerLaunch::execute()
   // Relay the environment variables.
   // TODO(jieyu): Consider using a clean environment.
 
-  if (command.get().shell()) {
+  if (command->shell()) {
     // Execute the command using shell.
-    os::execlp(os::Shell::name, os::Shell::arg0,
-               os::Shell::arg1, command.get().value().c_str(), (char*) nullptr);
+    os::execlp(os::Shell::name,
+               os::Shell::arg0,
+               os::Shell::arg1,
+               command->value().c_str(),
+               (char*) nullptr);
   } else {
     // Use execvp to launch the command.
-    char** argv = new char*[command.get().arguments().size() + 1];
-    for (int i = 0; i < command.get().arguments().size(); i++) {
-      argv[i] = strdup(command.get().arguments(i).c_str());
-    }
-    argv[command.get().arguments().size()] = nullptr;
-
-    execvp(command.get().value().c_str(), argv);
+    execvp(command->value().c_str(),
+           os::raw::Argv(command->arguments()));
   }
 
   // If we get here, the execle call failed.


[09/15] mesos git commit: Removed --sandbox flag from mesos-containerizer launch command.

Posted by ji...@apache.org.
Removed --sandbox flag from mesos-containerizer launch command.

This flag is unnecessary. The caller should calculate the current
working directory for the command, instead of doing the calculation in
this helper binary. This patch moved the calculation to the caller and
simplified this helper binary.

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


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

Branch: refs/heads/master
Commit: d5247fb34b17faef2c14c6edcbedd1ab8ecfb307
Parents: b3ba22e
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Jun 30 16:12:43 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 28 ++++++-------
 src/slave/containerizer/mesos/launch.cpp        | 41 ++++++--------------
 src/slave/containerizer/mesos/launch.hpp        |  1 -
 src/tests/containerizer/launch_tests.cpp        |  2 +-
 src/tests/containerizer/port_mapping_tests.cpp  |  2 +-
 5 files changed, 29 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index a523799..04daddd 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1229,20 +1229,22 @@ Future<bool> MesosContainerizerProcess::__launch(
 
     launchFlags.command = JSON::protobuf(executorLaunchCommand.get());
 
-    launchFlags.sandbox = executorRootfs.isSome()
-      ? flags.sandbox_directory
-      : directory;
-
-    // NOTE: If the executor shares the host filesystem, we should not
-    // allow them to 'cd' into an arbitrary directory because that'll
-    // create security issues.
-    if (executorRootfs.isNone() && workingDirectory.isSome()) {
-      LOG(WARNING) << "Ignore working directory '" << workingDirectory.get()
-                   << "' specified in container launch info for container "
-                   << containerId << " since the executor is using the "
-                   << "host filesystem";
+    if (executorRootfs.isNone()) {
+      // NOTE: If the executor shares the host filesystem, we should
+      // not allow them to 'cd' into an arbitrary directory because
+      // that'll create security issues.
+      if (workingDirectory.isSome()) {
+        LOG(WARNING) << "Ignore working directory '" << workingDirectory.get()
+                     << "' specified in container launch info for container "
+                     << containerId << " since the executor is using the "
+                     << "host filesystem";
+      }
+
+      launchFlags.working_directory = directory;
     } else {
-      launchFlags.working_directory = workingDirectory;
+      launchFlags.working_directory = workingDirectory.isSome()
+        ? workingDirectory
+        : flags.sandbox_directory;
     }
 
 #ifdef __WINDOWS__

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 68f23b3..a08a206 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -51,23 +51,16 @@ MesosContainerizerLaunch::Flags::Flags()
       "command",
       "The command to execute.");
 
-  add(&sandbox,
-      "sandbox",
-      "The sandbox for the executor. If rootfs is specified this must\n"
-      "be relative to the new root.");
-
   add(&working_directory,
       "working_directory",
-      "The working directory for the executor. It will be ignored if\n"
-      "container root filesystem is not specified.");
+      "The working directory for the command. It has to be an absolute path \n"
+      "w.r.t. the root filesystem used for the command.");
 
 #ifndef __WINDOWS__
   add(&rootfs,
       "rootfs",
-      "Absolute path to the container root filesystem.\n"
-      "The command and sandbox flags are interpreted relative\n"
-      "to rootfs\n"
-      "Different platforms may implement 'chroot' differently.");
+      "Absolute path to the container root filesystem. The command will be \n"
+      "interpreted relative to this path");
 
   add(&user,
       "user",
@@ -105,11 +98,6 @@ int MesosContainerizerLaunch::execute()
     return 1;
   }
 
-  if (flags.sandbox.isNone()) {
-    cerr << "Flag --sandbox is not specified" << endl;
-    return 1;
-  }
-
   bool controlPipeSpecified =
     flags.pipe_read.isSome() && flags.pipe_write.isSome();
 
@@ -346,19 +334,14 @@ int MesosContainerizerLaunch::execute()
   }
 #endif // __WINDOWS__
 
-  // Determine the current working directory for the executor.
-  string cwd;
-  if (rootfs.isSome() && flags.working_directory.isSome()) {
-    cwd = flags.working_directory.get();
-  } else {
-    cwd = flags.sandbox.get();
-  }
-
-  Try<Nothing> chdir = os::chdir(cwd);
-  if (chdir.isError()) {
-    cerr << "Failed to chdir into current working directory '"
-         << cwd << "': " << chdir.error() << endl;
-    return 1;
+  if (flags.working_directory.isSome()) {
+    Try<Nothing> chdir = os::chdir(flags.working_directory.get());
+    if (chdir.isError()) {
+      cerr << "Failed to chdir into current working directory "
+           << "'" << flags.working_directory.get() << "': "
+           << chdir.error() << endl;
+      return 1;
+    }
   }
 
   // Relay the environment variables.

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/launch.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.hpp b/src/slave/containerizer/mesos/launch.hpp
index c716e03..72b99ad 100644
--- a/src/slave/containerizer/mesos/launch.hpp
+++ b/src/slave/containerizer/mesos/launch.hpp
@@ -35,7 +35,6 @@ public:
     Flags();
 
     Option<JSON::Object> command;
-    Option<std::string> sandbox;
     Option<std::string> working_directory;
 #ifndef __WINDOWS__
     Option<std::string> rootfs;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/tests/containerizer/launch_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp
index 3e36f2f..ef0c87c 100644
--- a/src/tests/containerizer/launch_tests.cpp
+++ b/src/tests/containerizer/launch_tests.cpp
@@ -63,7 +63,7 @@ public:
     command.set_value(_command);
 
     launchFlags.command = JSON::protobuf(command);
-    launchFlags.sandbox = "/tmp";
+    launchFlags.working_directory = "/tmp";
     launchFlags.pipe_read = open("/dev/zero", O_RDONLY);
     launchFlags.pipe_write = open("/dev/null", O_WRONLY);
     launchFlags.rootfs = rootfs;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index 1c50cf5..2fbb9ec 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -314,7 +314,7 @@ protected:
     MesosContainerizerLaunch::Flags launchFlags;
 
     launchFlags.command = JSON::protobuf(commandInfo);
-    launchFlags.sandbox = os::getcwd();
+    launchFlags.working_directory = os::getcwd();
 
     CHECK_SOME(os::user());
     launchFlags.user = os::user().get();


[13/15] mesos git commit: Explicitly passed in launcher dir to command executor.

Posted by ji...@apache.org.
Explicitly passed in launcher dir to command executor.

Relying on dirname is not quite reliable, especially in tests where
launchable scripts are under 'build/src', but actual binaries are
under 'build/src/.libs'. This patch passed in launcher_dir explicitly
through executor flags.

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


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

Branch: refs/heads/master
Commit: 293d9eaa9c008af426df47bed3b29ecda8f70a72
Parents: 40ec907
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 16:40:32 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/executor.cpp        | 14 +++++++-------
 src/slave/slave.cpp              |  2 ++
 src/tests/health_check_tests.cpp |  8 --------
 src/tests/slave_tests.cpp        |  2 +-
 4 files changed, 10 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/293d9eaa/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index f78f0bf..78c4182 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -787,6 +787,11 @@ public:
         "If specified, this is the overrided command for launching the\n"
         "task (instead of the command from TaskInfo).");
 
+    add(&launcher_dir,
+        "launcher_dir",
+        "Directory path of Mesos binaries.",
+        PKGLIBEXECDIR);
+
     // TODO(nnielsen): Add 'prefix' option to enable replacing
     // 'sh -c' with user specified wrapper.
   }
@@ -796,6 +801,7 @@ public:
   Option<string> working_directory;
   Option<string> user;
   Option<string> task_command;
+  string launcher_dir;
 };
 
 
@@ -827,12 +833,6 @@ int main(int argc, char** argv)
     LOG(WARNING) << warning.message;
   }
 
-  const Option<string> envPath = os::getenv("MESOS_LAUNCHER_DIR");
-
-  string path = envPath.isSome()
-    ? envPath.get()
-    : os::realpath(Path(argv[0]).dirname()).get();
-
   Option<string> value = os::getenv("MESOS_FRAMEWORK_ID");
   if (value.isNone()) {
     EXIT(EXIT_FAILURE)
@@ -868,7 +868,7 @@ int main(int argc, char** argv)
 
   Owned<mesos::v1::internal::CommandExecutor> executor(
       new mesos::v1::internal::CommandExecutor(
-          path,
+          flags.launcher_dir,
           flags.rootfs,
           flags.sandbox_directory,
           flags.working_directory,

http://git-wip-us.apache.org/repos/asf/mesos/blob/293d9eaa/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 6857c87..de4cb5a 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3956,6 +3956,8 @@ ExecutorInfo Slave::getExecutorInfo(
       executor.mutable_command()->set_shell(false);
       executor.mutable_command()->set_value(path.get());
       executor.mutable_command()->add_arguments("mesos-executor");
+      executor.mutable_command()->add_arguments(
+          "--launcher_dir=" + flags.launcher_dir);
 
       if (hasRootfs) {
         executor.mutable_command()->add_arguments(

http://git-wip-us.apache.org/repos/asf/mesos/blob/293d9eaa/src/tests/health_check_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index 3e8e41b..47c35d1 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -118,14 +118,6 @@ public:
     CommandInfo command;
     command.set_value(cmd);
 
-    Environment::Variable* variable =
-      command.mutable_environment()->add_variables();
-
-    // We need to set the correct directory to launch health check process
-    // instead of the default for tests.
-    variable->set_name("MESOS_LAUNCHER_DIR");
-    variable->set_value(getLauncherDir());
-
     task.mutable_command()->CopyFrom(command);
 
     if (containerInfo.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/293d9eaa/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 868e521..60f9e16 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -544,7 +544,7 @@ TEST_F(SlaveTest, GetExecutorInfo)
   // Now assert that it actually is running mesos-executor without any
   // bleedover from the command we intend on running.
   EXPECT_FALSE(executor.command().shell());
-  EXPECT_EQ(1, executor.command().arguments_size());
+  EXPECT_EQ(2, executor.command().arguments_size());
   ASSERT_TRUE(executor.has_labels());
   EXPECT_EQ(2, executor.labels().labels_size());
   ASSERT_TRUE(executor.has_discovery());


[15/15] mesos git commit: Added devolve function for CommandInfo.

Posted by ji...@apache.org.
Added devolve function for CommandInfo.

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


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

Branch: refs/heads/master
Commit: 7ded2aaa8fa13b6b1184132a4a9451b11454ca54
Parents: 293d9ea
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Jul 1 17:11:34 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/internal/devolve.cpp | 6 ++++++
 src/internal/devolve.hpp | 1 +
 2 files changed, 7 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7ded2aaa/src/internal/devolve.cpp
----------------------------------------------------------------------
diff --git a/src/internal/devolve.cpp b/src/internal/devolve.cpp
index 04fac99..a2ad464 100644
--- a/src/internal/devolve.cpp
+++ b/src/internal/devolve.cpp
@@ -127,6 +127,12 @@ TaskStatus devolve(const v1::TaskStatus& status)
 }
 
 
+CommandInfo devolve(const v1::CommandInfo& command)
+{
+  return devolve<CommandInfo>(command);
+}
+
+
 executor::Call devolve(const v1::executor::Call& call)
 {
   return devolve<executor::Call>(call);

http://git-wip-us.apache.org/repos/asf/mesos/blob/7ded2aaa/src/internal/devolve.hpp
----------------------------------------------------------------------
diff --git a/src/internal/devolve.hpp b/src/internal/devolve.hpp
index 7c242d3..3812fd6 100644
--- a/src/internal/devolve.hpp
+++ b/src/internal/devolve.hpp
@@ -56,6 +56,7 @@ InverseOffer devolve(const v1::InverseOffer& inverseOffer);
 Credential devolve(const v1::Credential& credential);
 TaskID devolve(const v1::TaskID& taskId);
 TaskStatus devolve(const v1::TaskStatus& status);
+CommandInfo devolve(const v1::CommandInfo& command);
 
 scheduler::Call devolve(const v1::scheduler::Call& call);
 scheduler::Event devolve(const v1::scheduler::Event& event);


[06/15] mesos git commit: Used os::raw::Argv in command executor.

Posted by ji...@apache.org.
Used os::raw::Argv in command executor.

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


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

Branch: refs/heads/master
Commit: e9e7cf00e6f5cc71efdfe1d7439484b28bc07f86
Parents: 7e63b80
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 29 21:48:28 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/launcher/posix/executor.cpp   | 10 +++-------
 src/launcher/windows/executor.cpp | 10 +---------
 2 files changed, 4 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e7cf00/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index 805280e..3ced30f 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -21,6 +21,8 @@
 #include <stout/os.hpp>
 #include <stout/strings.hpp>
 
+#include <stout/os/raw/argv.hpp>
+
 #ifdef __linux__
 #include "linux/fs.hpp"
 #endif
@@ -81,11 +83,7 @@ pid_t launchTaskPosix(
   }
 
   // Prepare the argv before fork as it's not async signal safe.
-  char **argv = new char*[command.arguments().size() + 1];
-  for (int i = 0; i < command.arguments().size(); i++) {
-    argv[i] = (char*) command.arguments(i).c_str();
-  }
-  argv[command.arguments().size()] = nullptr;
+  os::raw::Argv argv(command.arguments());
 
   pid_t pid;
   if ((pid = fork()) == -1) {
@@ -214,8 +212,6 @@ pid_t launchTaskPosix(
     ABORT("Failed to exec: " + os::strerror(errno));
   }
 
-  delete[] argv;
-
   return pid;
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e7cf00/src/launcher/windows/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/windows/executor.cpp b/src/launcher/windows/executor.cpp
index b008052..a7b8125 100644
--- a/src/launcher/windows/executor.cpp
+++ b/src/launcher/windows/executor.cpp
@@ -69,15 +69,7 @@ PROCESS_INFORMATION launchTaskWindows(
 
     // TODO(jieyu): Consider allowing os::stringify_args to take
     // `command.arguments()` directly.
-    char **argv = new char*[command.arguments().size() + 1];
-    for (int i = 0; i < command.arguments().size(); i++) {
-      argv[i] = (char*) command.arguments(i).c_str();
-    }
-    argv[command.arguments().size()] = nullptr;
-
-    commandLine = os::stringify_args(argv);
-
-    delete[] argv;
+    commandLine = os::stringify_args(os::raw::Argv(command.arguments()));
   }
 
   cout << commandLine << endl;


[07/15] mesos git commit: Fixed some style issue in stout Makefile.

Posted by ji...@apache.org.
Fixed some style issue in stout Makefile.

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


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

Branch: refs/heads/master
Commit: 48ac0a5a76d707a1a36ec66270e79c743284f03b
Parents: e9e7cf0
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Jun 30 09:19:43 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/48ac0a5a/3rdparty/stout/include/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/Makefile.am b/3rdparty/stout/include/Makefile.am
index 4768064..1f2ee85 100644
--- a/3rdparty/stout/include/Makefile.am
+++ b/3rdparty/stout/include/Makefile.am
@@ -84,7 +84,7 @@ nobase_include_HEADERS =			\
   stout/os/open.hpp				\
   stout/os/os.hpp				\
   stout/os/osx.hpp				\
-  stout/os/pagesize.hpp			\
+  stout/os/pagesize.hpp				\
   stout/os/permissions.hpp			\
   stout/os/process.hpp				\
   stout/os/pstree.hpp				\
@@ -143,7 +143,7 @@ nobase_include_HEADERS =			\
   stout/os/windows/kill.hpp			\
   stout/os/windows/killtree.hpp			\
   stout/os/windows/mkdtemp.hpp			\
-  stout/os/windows/pagesize.hpp		\
+  stout/os/windows/pagesize.hpp			\
   stout/os/windows/read.hpp			\
   stout/os/windows/rename.hpp			\
   stout/os/windows/rm.hpp			\