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 2014/06/18 23:56:58 UTC
[1/2] git commit: Allowed launching subprocesses with flags.
Repository: mesos
Updated Branches:
refs/heads/master 83fcc8ca1 -> 02fa100ea
Allowed launching subprocesses with flags.
Review: https://reviews.apache.org/r/22749
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/82eb111a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/82eb111a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/82eb111a
Branch: refs/heads/master
Commit: 82eb111a536c0d5935ca4ff92ba6434a9bbae34b
Parents: 83fcc8c
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 18 11:43:44 2014 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jun 18 14:56:07 2014 -0700
----------------------------------------------------------------------
.../libprocess/include/process/subprocess.hpp | 12 +-
3rdparty/libprocess/src/subprocess.cpp | 25 +++-
.../libprocess/src/tests/subprocess_tests.cpp | 118 +++++++++++++++++++
3 files changed, 150 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/include/process/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index e93608a..d6e2c1f 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -10,6 +10,7 @@
#include <process/future.hpp>
+#include <stout/flags.hpp>
#include <stout/lambda.hpp>
#include <stout/memory.hpp>
#include <stout/option.hpp>
@@ -45,6 +46,7 @@ public:
const IO& in,
const IO& out,
const IO& err,
+ const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string> >& environment,
const Option<lambda::function<int()> >& setup);
@@ -94,9 +96,10 @@ public:
private:
friend Try<Subprocess> subprocess(
const std::string& command,
- const Subprocess::IO& in,
- const Subprocess::IO& out,
- const Subprocess::IO& err,
+ const IO& in,
+ const IO& out,
+ const IO& err,
+ const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string> >& environment,
const Option<lambda::function<int()> >& setup);
@@ -142,12 +145,14 @@ Try<Subprocess> subprocess(
const Subprocess::IO& in,
const Subprocess::IO& out,
const Subprocess::IO& err,
+ const Option<flags::FlagsBase>& flags = None(),
const Option<std::map<std::string, std::string> >& environment = None(),
const Option<lambda::function<int()> >& setup = None());
inline Try<Subprocess> subprocess(
const std::string& command,
+ const Option<flags::FlagsBase>& flags = None(),
const Option<std::map<std::string, std::string> >& environment = None(),
const Option<lambda::function<int()> >& setup = None())
{
@@ -156,6 +161,7 @@ inline Try<Subprocess> subprocess(
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
+ flags,
environment,
setup);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index 78fa1ec..6e2601d 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -16,6 +16,7 @@
#include <stout/foreach.hpp>
#include <stout/option.hpp>
#include <stout/os.hpp>
+#include <stout/strings.hpp>
#include <stout/try.hpp>
#include <stout/unreachable.hpp>
@@ -86,10 +87,11 @@ static Try<Nothing> cloexec(int stdinFd[2], int stdoutFd[2], int stderrFd[2])
// Runs the provided command in a subprocess.
Try<Subprocess> subprocess(
- const string& command,
+ const string& _command,
const Subprocess::IO& in,
const Subprocess::IO& out,
const Subprocess::IO& err,
+ const Option<flags::FlagsBase>& flags,
const Option<map<string, string> >& environment,
const Option<lambda::function<int()> >& setup)
{
@@ -217,6 +219,22 @@ Try<Subprocess> subprocess(
return Error("Failed to cloexec: " + cloexec.error());
}
+ // Prepare the command to execute. If the user specifies the
+ // 'flags', we will stringify it and append it to the command.
+ string command = _command;
+
+ if (flags.isSome()) {
+ foreachpair (const string& name, const flags::Flag& flag, flags.get()) {
+ Option<string> value = flag.stringify(flags.get());
+ if (value.isSome()) {
+ // TODO(jieyu): Need a better way to escape quotes. For
+ // example, what if 'value.get()' contains a single quote?
+ string argument = "--" + name + "='" + value.get() + "'";
+ command = strings::join(" ", command, argument);
+ }
+ }
+ }
+
// We need to do this construction before doing the fork as it
// might not be async-safe.
// TODO(tillt): Consider optimizing this to not pass an empty map
@@ -281,9 +299,12 @@ Try<Subprocess> subprocess(
}
}
+ // TODO(jieyu): Consider providing an optional way to launch the
+ // subprocess without using the shell (similar to 'shell=False'
+ // used in python subprocess.Popen).
execle("/bin/sh", "sh", "-c", command.c_str(), (char*) NULL, envp());
- ABORT("Failed to execle '/bin sh -c ", command.c_str(), "'\n");
+ ABORT("Failed to execle '/bin/sh -c ", command.c_str(), "'\n");
}
// Parent.
http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/src/tests/subprocess_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/subprocess_tests.cpp b/3rdparty/libprocess/src/tests/subprocess_tests.cpp
index 1cb1ce3..7dfa384 100644
--- a/3rdparty/libprocess/src/tests/subprocess_tests.cpp
+++ b/3rdparty/libprocess/src/tests/subprocess_tests.cpp
@@ -6,6 +6,7 @@
#include <map>
#include <string>
+#include <vector>
#include <process/gmock.hpp>
#include <process/gtest.hpp>
@@ -25,6 +26,7 @@ using namespace process;
using std::map;
using std::string;
+using std::vector;
class SubprocessTest: public TemporaryDirectoryTest {};
@@ -500,6 +502,114 @@ TEST_F(SubprocessTest, Default)
}
+struct Flags : public flags::FlagsBase
+{
+ Flags()
+ {
+ add(&b, "b", "bool");
+ add(&i, "i", "int");
+ add(&s, "s", "string");
+ add(&d, "d", "Duration");
+ add(&y, "y", "Bytes");
+ add(&j, "j", "JSON::Object");
+ }
+
+ Option<bool> b;
+ Option<int> i;
+ Option<string> s;
+ Option<Duration> d;
+ Option<Bytes> y;
+ Option<JSON::Object> j;
+};
+
+
+TEST_F(SubprocessTest, Flags)
+{
+ Clock::pause();
+
+ Flags flags;
+ flags.b = true;
+ flags.i = 42;
+ flags.s = "hello";
+ flags.d = Seconds(10);
+ flags.y = Bytes(100);
+
+ JSON::Object object;
+ object.values["strings"] = "string";
+ object.values["integer1"] = 1;
+ object.values["integer2"] = -1;
+ object.values["double1"] = 1;
+ object.values["double2"] = -1;
+ object.values["double3"] = -1.42;
+
+ JSON::Object nested;
+ nested.values["string"] = "string";
+ object.values["nested"] = nested;
+
+ JSON::Array array;
+ array.values.push_back(nested);
+ object.values["array"] = array;
+
+ flags.j = object;
+
+ string out = path::join(os::getcwd(), "stdout");
+
+ Try<Subprocess> s = subprocess(
+ "echo",
+ Subprocess::PIPE(),
+ Subprocess::PATH(out),
+ Subprocess::PIPE(),
+ flags);
+
+ ASSERT_SOME(s);
+
+ // Advance time until the internal reaper reaps the subprocess.
+ while (s.get().status().isPending()) {
+ Clock::advance(Seconds(1));
+ Clock::settle();
+ }
+
+ AWAIT_ASSERT_READY(s.get().status());
+ ASSERT_SOME(s.get().status().get());
+
+ int status = s.get().status().get().get();
+ EXPECT_TRUE(WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+
+ // Parse the output and make sure that it matches the flags we
+ // specified in the beginning.
+ Try<string> read = os::read(out);
+ ASSERT_SOME(read);
+
+ // TODO(jieyu): Consider testing the case where escaped spaces exist
+ // in the arguments.
+ vector<string> split = strings::split(read.get(), " ");
+ int argc = split.size() + 1;
+ char** argv = new char*[argc];
+ argv[0] = (char*) "command";
+ for (int i = 1; i < argc; i++) {
+ argv[i] = ::strdup(split[i - 1].c_str());
+ }
+
+ Flags flags2;
+ Try<Nothing> load = flags2.load(None(), argc, argv);
+ ASSERT_SOME(load);
+ EXPECT_EQ(flags.b, flags2.b);
+ EXPECT_EQ(flags.i, flags2.i);
+ EXPECT_EQ(flags.s, flags2.s);
+ EXPECT_EQ(flags.d, flags2.d);
+ EXPECT_EQ(flags.y, flags2.y);
+ EXPECT_EQ(flags.j, flags2.j);
+
+ for (int i = 1; i < argc; i++) {
+ ::free(argv[i]);
+ }
+ delete argv;
+
+ Clock::resume();
+}
+
+
TEST_F(SubprocessTest, Environment)
{
Clock::pause();
@@ -513,6 +623,7 @@ TEST_F(SubprocessTest, Environment)
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(s);
@@ -543,6 +654,7 @@ TEST_F(SubprocessTest, Environment)
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(s);
@@ -580,6 +692,7 @@ TEST_F(SubprocessTest, EnvironmentWithSpaces)
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(s);
@@ -617,6 +730,7 @@ TEST_F(SubprocessTest, EnvironmentWithSpacesAndQuotes)
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(s);
@@ -656,6 +770,7 @@ TEST_F(SubprocessTest, EnvironmentOverride)
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(s);
@@ -705,6 +820,7 @@ TEST_F(SubprocessTest, Setup)
Subprocess::PIPE(),
Subprocess::PIPE(),
None(),
+ None(),
lambda::bind(&setupChdir, directory.get()));
ASSERT_SOME(s);
@@ -746,6 +862,7 @@ TEST_F(SubprocessTest, SetupStatus)
Subprocess::PIPE(),
Subprocess::PIPE(),
None(),
+ None(),
lambda::bind(&setupStatus, 1));
ASSERT_SOME(s);
@@ -772,6 +889,7 @@ TEST_F(SubprocessTest, SetupStatus)
Subprocess::PIPE(),
Subprocess::PIPE(),
None(),
+ None(),
lambda::bind(&setupStatus, 0));
ASSERT_SOME(s);
[2/2] git commit: Updated mesos to use the new Subprocess interface
with flags.
Posted by ji...@apache.org.
Updated mesos to use the new Subprocess interface with flags.
Review: https://reviews.apache.org/r/22751
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/02fa100e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/02fa100e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/02fa100e
Branch: refs/heads/master
Commit: 02fa100eaf3d92261e39460b0aeb48c89306fc8a
Parents: 82eb111
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Jun 18 12:45:37 2014 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jun 18 14:56:22 2014 -0700
----------------------------------------------------------------------
src/launcher/launcher.cpp | 1 +
src/slave/containerizer/external_containerizer.cpp | 1 +
src/slave/containerizer/mesos_containerizer.cpp | 1 +
src/tests/slave_tests.cpp | 1 +
4 files changed, 4 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/02fa100e/src/launcher/launcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/launcher.cpp b/src/launcher/launcher.cpp
index 5585aad..872e2e8 100644
--- a/src/launcher/launcher.cpp
+++ b/src/launcher/launcher.cpp
@@ -211,6 +211,7 @@ process::Future<Option<int> > Operation::launch(
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
if (s.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/02fa100e/src/slave/containerizer/external_containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/external_containerizer.cpp b/src/slave/containerizer/external_containerizer.cpp
index 923046c..96c434b 100644
--- a/src/slave/containerizer/external_containerizer.cpp
+++ b/src/slave/containerizer/external_containerizer.cpp
@@ -1119,6 +1119,7 @@ Try<Subprocess> ExternalContainerizerProcess::invoke(
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment,
lambda::bind(&setup, sandbox.isSome() ? sandbox.get().directory
: string()));
http://git-wip-us.apache.org/repos/asf/mesos/blob/02fa100e/src/slave/containerizer/mesos_containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos_containerizer.cpp b/src/slave/containerizer/mesos_containerizer.cpp
index e5c159d..61c0a8d 100644
--- a/src/slave/containerizer/mesos_containerizer.cpp
+++ b/src/slave/containerizer/mesos_containerizer.cpp
@@ -663,6 +663,7 @@ Future<Nothing> MesosContainerizerProcess::fetch(
Subprocess::PIPE(),
Subprocess::PIPE(),
Subprocess::PIPE(),
+ None(),
environment);
if (fetcher.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/02fa100e/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 9dc4046..3a45191 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -351,6 +351,7 @@ TEST_F(SlaveTest, MesosExecutorWithOverride)
process::Subprocess::PIPE(),
process::Subprocess::PIPE(),
process::Subprocess::PIPE(),
+ None(),
environment);
ASSERT_SOME(executor);