You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by me...@apache.org on 2015/02/27 01:33:33 UTC

mesos git commit: Reenabled hadoop_home and frameworks_home slave flags for mesos-fetcher.

Repository: mesos
Updated Branches:
  refs/heads/master 53e9b9aa5 -> d82e0abeb


Reenabled hadoop_home and frameworks_home slave flags for mesos-fetcher.

Now the containerizer/fetcher sets the HADOOP_HOME variable for
mesos-fetcher again if the slave flag hadoop_home is set. Added a test
that checks that HDFS fetching is not broken and also ensures that the
flag gets translated to the environment variable and then gets applied
in mesos-fetcher. Created a mock hadoop implementation script for this.
This script has the exact same side effects as a real haddop client in
the scope of our testing. Using this, Mesos testing has no extra
external dependencies (on Hadoop).

Slave flag frameworks_home does not need to be an evironment variable.
It is now part of FetcherInfo only, but it also gets tested now that it
works.

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


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

Branch: refs/heads/master
Commit: d82e0abeb56b586eb45d4ef04f136bba13322bd6
Parents: 53e9b9a
Author: Bernd Mathiske <be...@mesosphere.io>
Authored: Thu Feb 26 15:21:38 2015 -0800
Committer: Adam B <ad...@mesosphere.io>
Committed: Thu Feb 26 15:21:38 2015 -0800

----------------------------------------------------------------------
 include/mesos/fetcher/fetcher.proto |   1 -
 src/launcher/fetcher.cpp            |  34 +++---
 src/slave/containerizer/fetcher.cpp |   5 +-
 src/tests/fetcher_tests.cpp         | 191 ++++++++++++++++++++++++++++---
 4 files changed, 196 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d82e0abe/include/mesos/fetcher/fetcher.proto
----------------------------------------------------------------------
diff --git a/include/mesos/fetcher/fetcher.proto b/include/mesos/fetcher/fetcher.proto
index facb87b..311af9a 100644
--- a/include/mesos/fetcher/fetcher.proto
+++ b/include/mesos/fetcher/fetcher.proto
@@ -33,5 +33,4 @@ message FetcherInfo {
   required string work_directory = 2;
   optional string user = 3;
   optional string frameworks_home = 4;
-  optional string hadoop_home = 5;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/d82e0abe/src/launcher/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index fed0105..796526f 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -149,7 +149,8 @@ Try<string> fetchWithNet(
 
 Try<string> fetchWithLocalCopy(
     const string& uri,
-    const string& directory)
+    const string& directory,
+    const Option<std::string>& frameworksHome)
 {
     string local = uri;
     bool fileUri = false;
@@ -167,16 +168,15 @@ Try<string> fetchWithLocalCopy(
 
     if (local.find_first_of("/") != 0) {
         // We got a non-Hadoop and non-absolute path.
-        if (os::hasenv("MESOS_FRAMEWORKS_HOME")) {
-            local = path::join(os::getenv("MESOS_FRAMEWORKS_HOME"), local);
-            LOG(INFO) << "Prepended environment variable "
-            << "MESOS_FRAMEWORKS_HOME to relative path, "
-            << "making it: '" << local << "'";
+        if (frameworksHome.isSome()) {
+            local = path::join(frameworksHome.get(), local);
+            LOG(INFO) << "Prepended the slave's frameworks_home flag value "
+                      << " to relative path, making it: '" << local << "'";
         } else {
             LOG(ERROR) << "A relative path was passed for the resource but the "
-            << "environment variable MESOS_FRAMEWORKS_HOME is not set. "
-            << "Please either specify this config option "
-            << "or avoid using a relative path";
+                       << "slave's frameworks_home flag is not set; "
+                       << "please either specify this slave configuration "
+                       << "option or avoid using a relative path";
             return Error("Could not resolve relative URI");
         }
     }
@@ -192,12 +192,12 @@ Try<string> fetchWithLocalCopy(
     std::ostringstream command;
     command << "cp '" << local << "' '" << path << "'";
     LOG(INFO) << "Copying resource from '" << local
-    << "' to '" << directory << "'";
+              << "' to '" << directory << "'";
 
     int status = os::system(command.str());
     if (status != 0) {
         LOG(ERROR) << "Failed to copy '" << local
-        << "' : Exit status " << status;
+                   << "' : Exit status " << status;
         return Error("Local copy failed");
     }
 
@@ -208,7 +208,8 @@ Try<string> fetchWithLocalCopy(
 // Fetch URI into directory.
 Try<string> fetch(
     const string& uri,
-    const string& directory)
+    const string& directory,
+    const Option<std::string>& frameworksHome)
 {
     LOG(INFO) << "Fetching URI '" << uri << "'";
     // Some checks to make sure using the URI value in shell commands
@@ -225,7 +226,7 @@ Try<string> fetch(
     // We consider file:// or no scheme uri are considered local uri.
     if (strings::startsWith(uri, "file://") ||
         uri.find("://") == string::npos) {
-      return fetchWithLocalCopy(uri, directory);
+      return fetchWithLocalCopy(uri, directory, frameworksHome);
     }
 
     // 2. Try to fetch URI using os::net / libcurl implementation.
@@ -295,10 +296,15 @@ int main(int argc, char* argv[])
     user = fetcherInfo.get().user();
   }
 
+  Option<std::string> frameworksHome = None();
+  if (fetcherInfo.get().has_frameworks_home()) {
+    frameworksHome = fetcherInfo.get().frameworks_home();
+  }
+
   // Fetch each URI to a local file, chmod, then chown if a user is provided.
   foreach (const CommandInfo::URI& uri, commandInfo.uris()) {
     // Fetch the URI to a local file.
-    Try<string> fetched = fetch(uri.value(), directory);
+    Try<string> fetched = fetch(uri.value(), directory, frameworksHome);
     if (fetched.isError()) {
       EXIT(1) << "Failed to fetch: " << uri.value();
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/d82e0abe/src/slave/containerizer/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.cpp b/src/slave/containerizer/fetcher.cpp
index d290f95..9e9e9d0 100644
--- a/src/slave/containerizer/fetcher.cpp
+++ b/src/slave/containerizer/fetcher.cpp
@@ -71,11 +71,12 @@ map<string, string> Fetcher::environment(
     fetcherInfo.set_frameworks_home(flags.frameworks_home);
   }
 
+  map<string, string> result;
+
   if (!flags.hadoop_home.empty()) {
-    fetcherInfo.set_hadoop_home(flags.hadoop_home);
+    result["HADOOP_HOME"] = flags.hadoop_home;
   }
 
-  map<string, string> result;
   result["MESOS_FETCHER_INFO"] = stringify(JSON::Protobuf(fetcherInfo));
 
   return result;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d82e0abe/src/tests/fetcher_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fetcher_tests.cpp b/src/tests/fetcher_tests.cpp
index 2438620..0ae0a10 100644
--- a/src/tests/fetcher_tests.cpp
+++ b/src/tests/fetcher_tests.cpp
@@ -21,6 +21,8 @@
 #include <map>
 #include <string>
 
+#include <hdfs/hdfs.hpp>
+
 #include <process/future.hpp>
 #include <process/gmock.hpp>
 #include <process/gtest.hpp>
@@ -82,7 +84,9 @@ TEST_F(FetcherEnvironmentTest, Simple)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
-  EXPECT_EQ(1u, environment.size());
+  EXPECT_EQ(2u, environment.size());
+
+  EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]);
 
   Try<JSON::Object> parse =
     JSON::parse<JSON::Object>(environment["MESOS_FETCHER_INFO"]);
@@ -96,7 +100,6 @@ TEST_F(FetcherEnvironmentTest, Simple)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -121,7 +124,9 @@ TEST_F(FetcherEnvironmentTest, MultipleURIs)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
-  EXPECT_EQ(1u, environment.size());
+  EXPECT_EQ(2u, environment.size());
+
+  EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]);
 
   Try<JSON::Object> parse =
     JSON::parse<JSON::Object>(environment["MESOS_FETCHER_INFO"]);
@@ -135,7 +140,6 @@ TEST_F(FetcherEnvironmentTest, MultipleURIs)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -155,7 +159,9 @@ TEST_F(FetcherEnvironmentTest, NoUser)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, None(), flags);
 
-  EXPECT_EQ(1u, environment.size());
+  EXPECT_EQ(2u, environment.size());
+
+  EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]);
 
   Try<JSON::Object> parse =
     JSON::parse<JSON::Object>(environment["MESOS_FETCHER_INFO"]);
@@ -169,7 +175,6 @@ TEST_F(FetcherEnvironmentTest, NoUser)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_FALSE(fetcherInfo.get().has_user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -190,6 +195,7 @@ TEST_F(FetcherEnvironmentTest, EmptyHadoop)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
+  EXPECT_EQ(0, environment.count("HADOOP_HOME"));
   EXPECT_EQ(1u, environment.size());
 
   Try<JSON::Object> parse =
@@ -204,7 +210,6 @@ TEST_F(FetcherEnvironmentTest, EmptyHadoop)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -224,6 +229,7 @@ TEST_F(FetcherEnvironmentTest, NoHadoop)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
+  EXPECT_EQ(0, environment.count("HADOOP_HOME"));
   EXPECT_EQ(1u, environment.size());
 
   Try<JSON::Object> parse =
@@ -238,7 +244,6 @@ TEST_F(FetcherEnvironmentTest, NoHadoop)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_FALSE(fetcherInfo.get().has_hadoop_home());
 }
 
 
@@ -260,7 +265,9 @@ TEST_F(FetcherEnvironmentTest, NoExtractNoExecutable)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
-  EXPECT_EQ(1u, environment.size());
+  EXPECT_EQ(2u, environment.size());
+
+  EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]);
 
   Try<JSON::Object> parse =
     JSON::parse<JSON::Object>(environment["MESOS_FETCHER_INFO"]);
@@ -274,7 +281,6 @@ TEST_F(FetcherEnvironmentTest, NoExtractNoExecutable)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -296,7 +302,9 @@ TEST_F(FetcherEnvironmentTest, NoExtractExecutable)
   map<string, string> environment =
     Fetcher::environment(commandInfo, directory, user, flags);
 
-  EXPECT_EQ(1u, environment.size());
+  EXPECT_EQ(2u, environment.size());
+
+  EXPECT_EQ(flags.hadoop_home, environment["HADOOP_HOME"]);
 
   Try<JSON::Object> parse =
     JSON::parse<JSON::Object>(environment["MESOS_FETCHER_INFO"]);
@@ -310,7 +318,6 @@ TEST_F(FetcherEnvironmentTest, NoExtractExecutable)
   EXPECT_EQ(directory, fetcherInfo.get().work_directory());
   EXPECT_EQ(user.get(), fetcherInfo.get().user());
   EXPECT_EQ(flags.frameworks_home, fetcherInfo.get().frameworks_home());
-  EXPECT_EQ(flags.hadoop_home, fetcherInfo.get().hadoop_home());
 }
 
 
@@ -328,7 +335,6 @@ TEST_F(FetcherTest, FileURI)
   EXPECT_FALSE(os::exists(localFile));
 
   slave::Flags flags;
-  flags.frameworks_home = "/tmp/frameworks";
 
   CommandInfo commandInfo;
   CommandInfo::URI* uri = commandInfo.add_uris();
@@ -353,22 +359,21 @@ TEST_F(FetcherTest, FileURI)
 }
 
 
-TEST_F(FetcherTest, FilePath)
+TEST_F(FetcherTest, AbsoluteFilePath)
 {
   string fromDir = path::join(os::getcwd(), "from");
   ASSERT_SOME(os::mkdir(fromDir));
-  string testFile = path::join(fromDir, "test");
-  EXPECT_FALSE(os::write(testFile, "data").isError());
+  string testPath = path::join(fromDir, "test");
+  EXPECT_FALSE(os::write(testPath, "data").isError());
 
   string localFile = path::join(os::getcwd(), "test");
   EXPECT_FALSE(os::exists(localFile));
 
   slave::Flags flags;
-  flags.frameworks_home = "/tmp/frameworks";
 
   CommandInfo commandInfo;
   CommandInfo::URI* uri = commandInfo.add_uris();
-  uri->set_value(testFile);
+  uri->set_value(testPath);
 
   map<string, string> environment =
     Fetcher::environment(commandInfo, os::getcwd(), None(), flags);
@@ -389,6 +394,65 @@ TEST_F(FetcherTest, FilePath)
 }
 
 
+TEST_F(FetcherTest, RelativeFilePath)
+{
+  string fromDir = path::join(os::getcwd(), "from");
+  ASSERT_SOME(os::mkdir(fromDir));
+  string testPath = path::join(fromDir, "test");
+  EXPECT_FALSE(os::write(testPath, "data").isError());
+
+  string localFile = path::join(os::getcwd(), "test");
+  EXPECT_FALSE(os::exists(localFile));
+
+  slave::Flags flags;
+
+  CommandInfo commandInfo;
+  CommandInfo::URI* uri = commandInfo.add_uris();
+  uri->set_value("test");
+
+  // The first run must fail, because we have not set frameworks_home yet.
+
+  map<string, string> environment1 =
+    Fetcher::environment(commandInfo, os::getcwd(), None(), flags);
+
+  Try<Subprocess> fetcherSubprocess1 =
+    process::subprocess(
+      path::join(mesos::internal::tests::flags.build_dir, "src/mesos-fetcher"),
+      environment1);
+
+  ASSERT_SOME(fetcherSubprocess1);
+  Future<Option<int>> status1 = fetcherSubprocess1.get().status();
+
+  AWAIT_READY(status1);
+  ASSERT_SOME(status1.get());
+
+  // mesos-fetcher always exits with EXIT(1) on failure.
+  EXPECT_EQ(1, WIFEXITED(status1.get().get()));
+
+  EXPECT_FALSE(os::exists(localFile));
+
+  // The next run must succeed due to this flag.
+  flags.frameworks_home = fromDir;
+
+  map<string, string> environment2 =
+    Fetcher::environment(commandInfo, os::getcwd(), None(), flags);
+
+  Try<Subprocess> fetcherSubprocess2 =
+    process::subprocess(
+      path::join(mesos::internal::tests::flags.build_dir, "src/mesos-fetcher"),
+      environment2);
+
+  ASSERT_SOME(fetcherSubprocess2);
+  Future<Option<int>> status2 = fetcherSubprocess2.get().status();
+
+  AWAIT_READY(status2);
+  ASSERT_SOME(status2.get());
+  EXPECT_EQ(0, status2.get().get());
+
+  EXPECT_TRUE(os::exists(localFile));
+}
+
+
 class HttpProcess : public Process<HttpProcess>
 {
 public:
@@ -636,6 +700,97 @@ TEST_F(FetcherTest, ExtractNotExecutable)
   ASSERT_SOME(os::rm(path.get()));
 }
 
+
+// Tests fetching via the local HDFS client. Since we cannot rely on
+// Hadoop being installed, we use our own mock version that works on
+// the local file system only, but this lets us exercise the exact
+// same C++ code paths as if there were real Hadoop at the other end.
+// Specifying a source URI that begins with "hdfs://" makes control
+// flow go there. To use this method of fetching, the slave flag
+// hadoop_home gets set to a path where we create a fake hadoop
+// executable. This also tests whether the HADOOP_HOME environment
+// variable gets set for mesos-fetcher (MESOS-2390).
+TEST_F(FetcherTest, HdfsURI)
+{
+  string hadoopPath = os::getcwd();
+  ASSERT_TRUE(os::exists(hadoopPath));
+
+  string hadoopBinPath = path::join(hadoopPath, "bin");
+  ASSERT_SOME(os::mkdir(hadoopBinPath));
+  ASSERT_SOME(os::chmod(hadoopBinPath, S_IRWXU | S_IRWXG | S_IRWXO));
+
+  const string& proof = path::join(hadoopPath, "proof");
+
+  // This acts exactly as "hadoop" for testing purposes.
+  string mockHadoopScript =
+    "#!/usr/bin/env bash\n"
+    "\n"
+    "touch " + proof + "\n"
+    "\n"
+    "if [[ 'version' == $1 ]]; then\n"
+    "  echo $0 'for Mesos testing'\n"
+    "fi\n"
+    "\n"
+    "# hadoop fs -copyToLocal $3 $4\n"
+    "if [[ 'fs' == $1 && '-copyToLocal' == $2 ]]; then\n"
+    "  if [[ $3 == 'hdfs://'* ]]; then\n"
+    "    # Remove 'hdfs://<host>/' and use just the (absolute) path.\n"
+    "    withoutProtocol=${3/'hdfs:'\\/\\//}\n"
+    "    withoutHost=${withoutProtocol#*\\/}\n"
+    "    absolutePath='/'$withoutHost\n"
+    "   cp $absolutePath $4\n"
+    "  else\n"
+    "    cp #3 $4\n"
+    "  fi\n"
+    "fi\n";
+
+  string hadoopCommand = path::join(hadoopBinPath, "hadoop");
+  ASSERT_SOME(os::write(hadoopCommand, mockHadoopScript));
+  ASSERT_SOME(os::chmod(hadoopCommand,
+                        S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH));
+
+  string fromDir = path::join(os::getcwd(), "from");
+  ASSERT_SOME(os::mkdir(fromDir));
+  string testFile = path::join(fromDir, "test");
+  EXPECT_SOME(os::write(testFile, "data"));
+
+  string localFile = path::join(os::getcwd(), "test");
+  EXPECT_FALSE(os::exists(localFile));
+
+  slave::Flags flags;
+  flags.launcher_dir = path::join(tests::flags.build_dir, "src");
+  flags.hadoop_home = hadoopPath;
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  CommandInfo commandInfo;
+  CommandInfo::URI* uri = commandInfo.add_uris();
+  uri->set_value(path::join("hdfs://localhost", testFile));
+
+  Option<int> stdout = None();
+  Option<int> stderr = None();
+
+  // Redirect mesos-fetcher output if running the tests verbosely.
+  if (tests::flags.verbose) {
+    stdout = STDOUT_FILENO;
+    stderr = STDERR_FILENO;
+  }
+
+  Fetcher fetcher;
+
+  Future<Nothing> fetch = fetcher.fetch(
+      containerId, commandInfo, os::getcwd(), None(), flags, stdout, stderr);
+
+  AWAIT_READY(fetch);
+
+  // Proof that we used our own mock version of Hadoop.
+  ASSERT_TRUE(os::exists(proof));
+
+  // Proof that hdfs fetching worked.
+  EXPECT_TRUE(os::exists(localFile));
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {