svn commit: r1434967 - in /incubator/mesos/trunk: src/launcher/launcher.cpp third_party/libprocess/include/stout/os.hpp

[be...@apache.org]

Author: benh
Date: Thu Jan 17 23:39:28 2013
New Revision: 1434967

URL: http://svn.apache.org/viewvc?rev=1434967&view=rev
Log:
Recursively chown the executor work directory after extracting
resources.

From: Ben Mahler <be...@gmail.com>
Review: https://reviews.apache.org/r/8947

Modified:
    incubator/mesos/trunk/src/launcher/launcher.cpp
    incubator/mesos/trunk/third_party/libprocess/include/stout/os.hpp

Modified: incubator/mesos/trunk/src/launcher/launcher.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/launcher/launcher.cpp?rev=1434967&r1=1434966&r2=1434967&view=diff
==============================================================================
--- incubator/mesos/trunk/src/launcher/launcher.cpp (original)
+++ incubator/mesos/trunk/src/launcher/launcher.cpp Thu Jan 17 23:39:28 2013
@@ -32,6 +32,7 @@
 #include <stout/fatal.hpp>
 #include <stout/foreach.hpp>
 #include <stout/net.hpp>
+#include <stout/nothing.hpp>
 #include <stout/os.hpp>
 #include <stout/path.hpp>
 
@@ -82,15 +83,20 @@ int ExecutorLauncher::setup()
   const string& cwd = os::getcwd();
 
   // TODO(benh): Do this in the slave?
-  if (shouldSwitchUser && !os::chown(user, workDirectory)) {
-    cerr << "Failed to change ownership of framework's working directory "
-         << workDirectory << " to user " << user << endl;
-    return -1;
+  if (shouldSwitchUser) {
+    Try<Nothing> chown = os::chown(user, workDirectory);
+
+    if (chown.isError()) {
+      cerr << "Failed to change ownership of the executor work directory "
+           << workDirectory << " to user " << user << ": " << chown.error()
+           << endl;
+      return -1;
+    }
   }
 
   // Enter working directory.
   if (os::chdir(workDirectory) < 0) {
-    cerr << "Failed to chdir into framework working directory" << endl;
+    cerr << "Failed to chdir into executor work directory" << endl;
     return -1;
   }
 
@@ -113,7 +119,7 @@ int ExecutorLauncher::launch()
 {
   // Enter working directory.
   if (os::chdir(workDirectory) < 0) {
-    fatalerror("Failed to chdir into framework working directory");
+    fatalerror("Failed to chdir into the executor work directory");
   }
 
   if (shouldSwitchUser) {
@@ -276,7 +282,7 @@ int ExecutorLauncher::fetchExecutors()
 
       // Copy the resource to the current working directory.
       ostringstream command;
-      command << "cp " << resource << " .";
+      command << "cp '" << resource << "' .";
       cout << "Copying resource from " << resource << " to .";
 
       int ret = os::system(command.str());
@@ -294,9 +300,14 @@ int ExecutorLauncher::fetchExecutors()
       resource = path::join(".", base.get());
     }
 
-    if (shouldSwitchUser && !os::chown(user, resource)) {
-      cerr << "Failed to chown " << resource << endl;
-      return -1;
+    if (shouldSwitchUser) {
+      Try<Nothing> chown = os::chown(user, resource);
+
+      if (chown.isError()) {
+        cerr << "Failed to chown " << resource << " to user " << user << ": "
+             << chown.error() << endl;
+        return -1;
+      }
     }
 
     if (executable &&
@@ -325,6 +336,19 @@ int ExecutorLauncher::fetchExecutors()
       }
     }
   }
+
+  // Recursively chown the work directory, since extraction may have occurred.
+  if (shouldSwitchUser) {
+    Try<Nothing> chown = os::chown(user, ".");
+
+    if (chown.isError()) {
+      cerr << "Failed to recursively chown the work directory "
+           << workDirectory << " to user " << user << ": " << chown.error()
+           << endl;
+      return -1;
+    }
+  }
+
   return 0;
 }
 

Modified: incubator/mesos/trunk/third_party/libprocess/include/stout/os.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/third_party/libprocess/include/stout/os.hpp?rev=1434967&r1=1434966&r2=1434967&view=diff
==============================================================================
--- incubator/mesos/trunk/third_party/libprocess/include/stout/os.hpp (original)
+++ incubator/mesos/trunk/third_party/libprocess/include/stout/os.hpp Thu Jan 17 23:39:28 2013
@@ -501,25 +501,45 @@ inline Try<Nothing> rmdir(const std::str
 }
 
 
+inline int system(const std::string& command)
+{
+  return ::system(command.c_str());
+}
+
+
 // TODO(bmahler): Clean these bool functions to return Try<Nothing>.
 // Changes the specified path's user and group ownership to that of
 // the specified user..
-inline bool chown(const std::string& user, const std::string& path)
+inline Try<Nothing> chown(
+    const std::string& user,
+    const std::string& path,
+    bool recursive = true)
 {
   passwd* passwd;
   if ((passwd = ::getpwnam(user.c_str())) == NULL) {
-    PLOG(ERROR) << "Failed to get user information for '"
-                << user << "', getpwnam";
-    return false;
+    return Try<Nothing>::error(
+        "Failed to get user information for '" + user + "', getpwnam: "
+        + strerror(errno));
   }
 
-  if (::chown(path.c_str(), passwd->pw_uid, passwd->pw_gid) < 0) {
-    PLOG(ERROR) << "Failed to change user and group ownership of '"
-                << path << "', chown";
-    return false;
+  if (recursive) {
+    // TODO(bmahler): Consider walking the file tree instead. We would need
+    // to be careful to not miss dotfiles.
+    std::string command = "chown -R " + stringify(passwd->pw_uid) + ':' +
+        stringify(passwd->pw_gid) + " '" + path + "'";
+
+    int code = os::system(command);
+    if (code != 0) {
+      return Try<Nothing>::error(
+          "Failed to execute '" + command + "', exit code: " + stringify(code));
+    }
+  } else {
+    if (::chown(path.c_str(), passwd->pw_uid, passwd->pw_gid) < 0) {
+      return Try<Nothing>::error(strerror(errno));
+    }
   }
 
-  return true;
+  return Nothing();
 }
 
 
@@ -818,12 +838,6 @@ inline Try<std::list<std::string> > glob
 }
 
 
-inline int system(const std::string& command)
-{
-  return ::system(command.c_str());
-}
-
-
 // Returns the total number of cpus (cores).
 inline Try<long> cpus()
 {