Repository: mesos Updated Branches: refs/heads/master 4085b8607 -> 79ac35626
Updated the fetcher subprocess to pass in the environment explicitly. Review: https://reviews.apache.org/r/19164 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/79ac3562 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/79ac3562 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/79ac3562 Branch: refs/heads/master Commit: 79ac3562681707da9cfff26793d0cee8c404a2aa Parents: 4085b86 Author: Dominic Hamon <dha...@twopensource.com> Authored: Tue Apr 8 14:37:08 2014 -0700 Committer: Benjamin Mahler <bmah...@twitter.com> Committed: Tue Apr 8 14:37:08 2014 -0700 ---------------------------------------------------------------------- src/slave/containerizer/mesos_containerizer.cpp | 28 +++-- src/tests/containerizer_tests.cpp | 105 ++++++++++--------- 2 files changed, 67 insertions(+), 66 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/79ac3562/src/slave/containerizer/mesos_containerizer.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos_containerizer.cpp b/src/slave/containerizer/mesos_containerizer.cpp index c819c97..1ce41d7 100644 --- a/src/slave/containerizer/mesos_containerizer.cpp +++ b/src/slave/containerizer/mesos_containerizer.cpp @@ -53,8 +53,8 @@ using state::RunState; Future<Nothing> _nothing() { return Nothing(); } -// Helper method to build the command sent to the fetcher. -std::string buildCommand( +// Helper method to build the environment map used to launch fetcher. +map<string, string> fetcherEnvironment( const CommandInfo& commandInfo, const std::string& directory, const Option<std::string>& user, @@ -70,23 +70,20 @@ std::string buildCommand( // Remove extra space at the end. uris = strings::trim(uris); - // Use /usr/bin/env to set the environment variables for the fetcher - // subprocess because we cannot pollute the slave's environment. - // TODO(idownes): Remove this once Subprocess accepts environment variables. - string command = "/usr/bin/env"; - command += " MESOS_EXECUTOR_URIS=\"" + uris + "\""; - command += " MESOS_WORK_DIRECTORY=" + directory; + map<string, string> environment; + environment["MESOS_EXECUTOR_URIS"] = uris; + environment["MESOS_WORK_DIRECTORY"] = directory; if (user.isSome()) { - command += " MESOS_USER=" + user.get(); + environment["MESOS_USER"] = user.get(); } if (!flags.frameworks_home.empty()) { - command += " MESOS_FRAMEWORKS_HOME=" + flags.frameworks_home; + environment["MESOS_FRAMEWORKS_HOME"] = flags.frameworks_home; } if (!flags.hadoop_home.empty()) { - command += " HADOOP_HOME=" + flags.hadoop_home; + environment["HADOOP_HOME"] = flags.hadoop_home; } - return command; + return environment; } @@ -501,15 +498,16 @@ Future<Nothing> MesosContainerizerProcess::fetch( return Failure("Could not fetch URIs: failed to find mesos-fetcher"); } - string command = buildCommand(commandInfo, directory, user, flags); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, user, flags); // Now the actual mesos-fetcher command. - command += " " + realpath.get(); + string command = realpath.get(); LOG(INFO) << "Fetching URIs for container '" << containerId << "' using command '" << command << "'"; - Try<Subprocess> fetcher = subprocess(command); + Try<Subprocess> fetcher = subprocess(command, environment); if (fetcher.isError()) { return Failure("Failed to execute mesos-fetcher: " + fetcher.error()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/79ac3562/src/tests/containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer_tests.cpp b/src/tests/containerizer_tests.cpp index b539070..6c48146 100644 --- a/src/tests/containerizer_tests.cpp +++ b/src/tests/containerizer_tests.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ +#include <map> #include <string> #include <vector> @@ -23,13 +24,15 @@ #include <mesos/mesos.hpp> -#include <slave/containerizer/mesos_containerizer.hpp> -#include <slave/flags.hpp> +#include "slave/flags.hpp" + +#include "slave/containerizer/mesos_containerizer.hpp" using namespace mesos; using namespace mesos::internal; using namespace mesos::internal::slave; +using std::map; using std::string; using std::vector; @@ -38,7 +41,7 @@ namespace internal { namespace slave { // Forward declaration. -string buildCommand( +map<string, string> fetcherEnvironment( const CommandInfo& commandInfo, const string& directory, const Option<string>& user, @@ -51,7 +54,8 @@ string buildCommand( class MesosContainerizerProcessTest : public ::testing::Test {}; -TEST_F(MesosContainerizerProcessTest, Simple) { +TEST_F(MesosContainerizerProcessTest, Simple) +{ CommandInfo commandInfo; CommandInfo::URI uri; uri.set_value("hdfs:///uri"); @@ -65,20 +69,19 @@ TEST_F(MesosContainerizerProcessTest, Simple) { flags.frameworks_home = "/tmp/frameworks"; flags.hadoop_home = "/tmp/hadoop"; - string command = buildCommand(commandInfo, directory, user, flags); - - EXPECT_STREQ( - "/usr/bin/env " - "MESOS_EXECUTOR_URIS=\"hdfs:///uri+0\" " - "MESOS_WORK_DIRECTORY=/tmp/directory " - "MESOS_USER=user " - "MESOS_FRAMEWORKS_HOME=/tmp/frameworks " - "HADOOP_HOME=/tmp/hadoop", - command.c_str()); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, user, flags); + EXPECT_EQ(5u, environment.size()); + EXPECT_EQ("hdfs:///uri+0", environment["MESOS_EXECUTOR_URIS"]); + EXPECT_EQ(directory, environment["MESOS_WORK_DIRECTORY"]); + EXPECT_EQ(user.get(), environment["MESOS_USER"]); + EXPECT_EQ(flags.frameworks_home, environment["MESOS_FRAMEWORKS_HOME"]); + EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]); } -TEST_F(MesosContainerizerProcessTest, MultipleURIs) { +TEST_F(MesosContainerizerProcessTest, MultipleURIs) +{ CommandInfo commandInfo; CommandInfo::URI uri; uri.set_value("hdfs:///uri1"); @@ -95,20 +98,21 @@ TEST_F(MesosContainerizerProcessTest, MultipleURIs) { flags.frameworks_home = "/tmp/frameworks"; flags.hadoop_home = "/tmp/hadoop"; - string command = buildCommand(commandInfo, directory, user, flags); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, user, flags); - EXPECT_STREQ( - "/usr/bin/env " - "MESOS_EXECUTOR_URIS=\"hdfs:///uri1+0 hdfs:///uri2+1\" " - "MESOS_WORK_DIRECTORY=/tmp/directory " - "MESOS_USER=user " - "MESOS_FRAMEWORKS_HOME=/tmp/frameworks " - "HADOOP_HOME=/tmp/hadoop", - command.c_str()); + EXPECT_EQ(5u, environment.size()); + EXPECT_EQ( + "hdfs:///uri1+0 hdfs:///uri2+1", environment["MESOS_EXECUTOR_URIS"]); + EXPECT_EQ(directory, environment["MESOS_WORK_DIRECTORY"]); + EXPECT_EQ(user.get(), environment["MESOS_USER"]); + EXPECT_EQ(flags.frameworks_home, environment["MESOS_FRAMEWORKS_HOME"]); + EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]); } -TEST_F(MesosContainerizerProcessTest, NoUser) { +TEST_F(MesosContainerizerProcessTest, NoUser) +{ CommandInfo commandInfo; CommandInfo::URI uri; uri.set_value("hdfs:///uri"); @@ -121,19 +125,19 @@ TEST_F(MesosContainerizerProcessTest, NoUser) { flags.frameworks_home = "/tmp/frameworks"; flags.hadoop_home = "/tmp/hadoop"; - string command = buildCommand(commandInfo, directory, None(), flags); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, None(), flags); - EXPECT_STREQ( - "/usr/bin/env " - "MESOS_EXECUTOR_URIS=\"hdfs:///uri+0\" " - "MESOS_WORK_DIRECTORY=/tmp/directory " - "MESOS_FRAMEWORKS_HOME=/tmp/frameworks " - "HADOOP_HOME=/tmp/hadoop", - command.c_str()); + EXPECT_EQ(4u, environment.size()); + EXPECT_EQ("hdfs:///uri+0", environment["MESOS_EXECUTOR_URIS"]); + EXPECT_EQ(directory, environment["MESOS_WORK_DIRECTORY"]); + EXPECT_EQ(flags.frameworks_home, environment["MESOS_FRAMEWORKS_HOME"]); + EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]); } -TEST_F(MesosContainerizerProcessTest, EmptyHadoop) { +TEST_F(MesosContainerizerProcessTest, EmptyHadoop) +{ CommandInfo commandInfo; CommandInfo::URI uri; uri.set_value("hdfs:///uri"); @@ -147,19 +151,19 @@ TEST_F(MesosContainerizerProcessTest, EmptyHadoop) { flags.frameworks_home = "/tmp/frameworks"; flags.hadoop_home = ""; - string command = buildCommand(commandInfo, directory, user, flags); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, user, flags); - EXPECT_STREQ( - "/usr/bin/env " - "MESOS_EXECUTOR_URIS=\"hdfs:///uri+0\" " - "MESOS_WORK_DIRECTORY=/tmp/directory " - "MESOS_USER=user " - "MESOS_FRAMEWORKS_HOME=/tmp/frameworks", - command.c_str()); + EXPECT_EQ(4u, environment.size()); + EXPECT_EQ("hdfs:///uri+0", environment["MESOS_EXECUTOR_URIS"]); + EXPECT_EQ(directory, environment["MESOS_WORK_DIRECTORY"]); + EXPECT_EQ(user.get(), environment["MESOS_USER"]); + EXPECT_EQ(flags.frameworks_home, environment["MESOS_FRAMEWORKS_HOME"]); } -TEST_F(MesosContainerizerProcessTest, NoHadoop) { +TEST_F(MesosContainerizerProcessTest, NoHadoop) +{ CommandInfo commandInfo; CommandInfo::URI uri; uri.set_value("hdfs:///uri"); @@ -172,13 +176,12 @@ TEST_F(MesosContainerizerProcessTest, NoHadoop) { Flags flags; flags.frameworks_home = "/tmp/frameworks"; - string command = buildCommand(commandInfo, directory, user, flags); + map<string, string> environment = + fetcherEnvironment(commandInfo, directory, user, flags); - EXPECT_STREQ( - "/usr/bin/env " - "MESOS_EXECUTOR_URIS=\"hdfs:///uri+0\" " - "MESOS_WORK_DIRECTORY=/tmp/directory " - "MESOS_USER=user " - "MESOS_FRAMEWORKS_HOME=/tmp/frameworks", - command.c_str()); + EXPECT_EQ(4u, environment.size()); + EXPECT_EQ("hdfs:///uri+0", environment["MESOS_EXECUTOR_URIS"]); + EXPECT_EQ(directory, environment["MESOS_WORK_DIRECTORY"]); + EXPECT_EQ(user.get(), environment["MESOS_USER"]); + EXPECT_EQ(flags.frameworks_home, environment["MESOS_FRAMEWORKS_HOME"]); }
