Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Recursively chown the executor work directory after extracting

resources.

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

git-svn-id: https://svn.apache.org/repos/asf/incubator/mesos/trunk@1434967 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information...
commit ecc5a9bc7f0595f64a8f3ba58345ef2856f59a87 1 parent 7c2511f
@benh benh authored
Showing with 63 additions and 25 deletions.
  1. +34 −10 src/launcher/launcher.cpp
  2. +29 −15 third_party/libprocess/include/stout/os.hpp
View
44 src/launcher/launcher.cpp
@@ -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;
}
View
44 third_party/libprocess/include/stout/os.hpp
@@ -501,25 +501,45 @@ inline Try<Nothing> rmdir(const std::string& directory, bool recursive = true)
}
+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(const std::string& pattern)
}
-inline int system(const std::string& command)
-{
- return ::system(command.c_str());
-}
-
-
// Returns the total number of cpus (cores).
inline Try<long> cpus()
{
Please sign in to comment.
Something went wrong with that request. Please try again.