You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2014/06/19 02:38:11 UTC
[1/2] git commit: Refactored os::user to return a Result.
Repository: mesos
Updated Branches:
refs/heads/master eb1b7c56d -> ef2106e6d
Refactored os::user to return a Result.
Review: https://reviews.apache.org/r/22722/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/926ed809
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/926ed809
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/926ed809
Branch: refs/heads/master
Commit: 926ed809c5a0a448a4cbc7baec33013b6574ea13
Parents: eb1b7c5
Author: Alexandra Sava <al...@gmail.com>
Authored: Wed Jun 18 17:05:36 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Wed Jun 18 17:05:36 2014 -0700
----------------------------------------------------------------------
.../libprocess/3rdparty/stout/include/stout/os.hpp | 16 ++++++++--------
.../3rdparty/stout/tests/os/setns_tests.cpp | 5 ++++-
.../libprocess/3rdparty/stout/tests/os_tests.cpp | 10 ++++++----
3 files changed, 18 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/926ed809/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
index 72f3e70..0529f88 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
@@ -813,10 +813,12 @@ inline Try<std::list<std::string> > find(
}
-// TODO(idownes): Refactor to return a Result<string>, returning
-// None() and ErrnoError as appropriate rather than LOG(FATAL).
-inline std::string user()
+inline Result<std::string> user(Option<uid_t> uid = None())
{
+ if (uid.isNone()) {
+ uid = ::getuid();
+ }
+
int size = sysconf(_SC_GETPW_R_SIZE_MAX);
if (size == -1) {
// Initial value for buffer size.
@@ -829,12 +831,12 @@ inline std::string user()
while (true) {
char* buffer = new char[size];
- if (getpwuid_r(::getuid(), &passwd, buffer, size, &result) == 0) {
+ if (getpwuid_r(uid.get(), &passwd, buffer, size, &result) == 0) {
// getpwuid_r will return 0 but set result == NULL if the uid is
// not found.
if (result == NULL) {
delete[] buffer;
- LOG(FATAL) << "Failed to find username for uid " << ::getuid();
+ return None();
}
std::string user(passwd.pw_name);
@@ -843,7 +845,7 @@ inline std::string user()
} else {
if (errno != ERANGE) {
delete[] buffer;
- PLOG(FATAL) << "Failed to get username information";
+ return ErrnoError();
}
// getpwuid_r set ERANGE so try again with a larger buffer.
@@ -851,8 +853,6 @@ inline std::string user()
delete[] buffer;
}
}
-
- return UNREACHABLE();
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/926ed809/3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp
index f6d79b1..ff9f10b 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp
@@ -24,7 +24,10 @@ static void* child(void* arg)
TEST(OsSetnsTest, setns)
{
- if (os::user() != "root") {
+ Result<string> user = os::user();
+ ASSERT_SOME(user);
+
+ if (user.get() != "root") {
return;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/926ed809/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
index 1a6ce0b..7fa7346 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -701,23 +701,25 @@ TEST_F(OsTest, user)
{
std::ostringstream user_;
EXPECT_SOME_EQ(0, os::shell(&user_ , "id -un"));
- EXPECT_EQ(strings::trim(user_.str()), os::user());
+
+ Result<string> user = os::user();
+ ASSERT_SOME_EQ(strings::trim(user_.str()), user);
std::ostringstream uid_;
EXPECT_SOME_EQ(0, os::shell(&uid_, "id -u"));
Try<uid_t> uid = numify<uid_t>(strings::trim(uid_.str()));
ASSERT_SOME(uid);
- EXPECT_SOME_EQ(uid.get(), os::getuid(os::user()));
+ EXPECT_SOME_EQ(uid.get(), os::getuid(user.get()));
std::ostringstream gid_;
EXPECT_SOME_EQ(0, os::shell(&gid_, "id -g"));
Try<gid_t> gid = numify<gid_t>(strings::trim(gid_.str()));
ASSERT_SOME(gid);
- EXPECT_SOME_EQ(gid.get(), os::getgid(os::user()));
+ EXPECT_SOME_EQ(gid.get(), os::getgid(user.get()));
EXPECT_NONE(os::getuid(UUID::random().toString()));
EXPECT_NONE(os::getgid(UUID::random().toString()));
- EXPECT_TRUE(os::su(os::user()));
+ EXPECT_TRUE(os::su(user.get()));
EXPECT_FALSE(os::su(UUID::random().toString()));
}
[2/2] git commit: Updated mesos to use refactored os::user call.
Posted by bm...@apache.org.
Updated mesos to use refactored os::user call.
Review: https://reviews.apache.org/r/22722/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ef2106e6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ef2106e6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ef2106e6
Branch: refs/heads/master
Commit: ef2106e6d71b69cf803748214f5fcb7e88f6f3d9
Parents: 926ed80
Author: Alexandra Sava <al...@gmail.com>
Authored: Wed Jun 18 17:15:33 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Wed Jun 18 17:15:33 2014 -0700
----------------------------------------------------------------------
src/cli/execute.cpp | 14 ++++++++++++--
src/sched/sched.cpp | 5 ++++-
src/scheduler/scheduler.cpp | 5 ++++-
src/tests/environment.cpp | 9 ++++++---
src/tests/mesos.cpp | 25 ++++++++++++++++++++-----
src/tests/script.cpp | 5 ++++-
src/tests/slave_tests.cpp | 8 +++++---
7 files changed, 55 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/cli/execute.cpp
----------------------------------------------------------------------
diff --git a/src/cli/execute.cpp b/src/cli/execute.cpp
index e881b64..7e20f4c 100644
--- a/src/cli/execute.cpp
+++ b/src/cli/execute.cpp
@@ -283,6 +283,16 @@ int main(int argc, char** argv)
return -1;
}
+ Result<string> user = os::user();
+ if (!user.isSome()) {
+ if (user.isError()) {
+ cerr << "Failed to get username: " << user.error() << endl;
+ } else {
+ cerr << "No username for uid " << ::getuid() << endl;
+ }
+ return -1;
+ }
+
// Copy the package to HDFS if requested save it's location as a URI
// for passing to the command (in CommandInfo).
Option<string> uri = None();
@@ -298,7 +308,7 @@ int main(int argc, char** argv)
// already been uploaded before ...).
// Store the file at '/user/package'.
- string path = path::join("/", os::user(), flags.package.get());
+ string path = path::join("/", user.get(), flags.package.get());
// Check if the file exists and remove it if we're overwriting.
Try<bool> exists = hdfs.exists(path);
@@ -333,7 +343,7 @@ int main(int argc, char** argv)
uri);
FrameworkInfo framework;
- framework.set_user(os::user());
+ framework.set_user(user.get());
framework.set_name("");
framework.set_checkpoint(flags.checkpoint);
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/sched/sched.cpp
----------------------------------------------------------------------
diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp
index 003e685..68782d4 100644
--- a/src/sched/sched.cpp
+++ b/src/sched/sched.cpp
@@ -1117,7 +1117,10 @@ void MesosSchedulerDriver::initialize() {
// See FrameWorkInfo in include/mesos/mesos.proto:
if (framework.user().empty()) {
- framework.set_user(os::user());
+ Result<string> user = os::user();
+ CHECK_SOME(user);
+
+ framework.set_user(user.get());
}
if (framework.hostname().empty()) {
framework.set_hostname(os::hostname().get());
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/scheduler/scheduler.cpp
----------------------------------------------------------------------
diff --git a/src/scheduler/scheduler.cpp b/src/scheduler/scheduler.cpp
index 4ae188e..498d6aa 100644
--- a/src/scheduler/scheduler.cpp
+++ b/src/scheduler/scheduler.cpp
@@ -177,7 +177,10 @@ public:
// TODO(benh): Make FrameworkInfo.user be optional and add a
// 'user' to either TaskInfo or CommandInfo.
if (call.framework_info().user() == "") {
- call.mutable_framework_info()->set_user(os::user());
+ Result<string> user = os::user();
+ CHECK_SOME(user);
+
+ call.mutable_framework_info()->set_user(user.get());
}
// Only a REGISTER should not have set the framework ID.
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 005fc54..21b9d1d 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -78,8 +78,11 @@ static bool enable(const ::testing::TestInfo& test)
names.push_back(test.test_case_name());
names.push_back(test.name());
+ Result<string> user = os::user();
+ CHECK_SOME(user);
+
foreach (const string& name, names) {
- if (strings::contains(name, "ROOT_") && os::user() != "root") {
+ if (strings::contains(name, "ROOT_") && user.get() != "root") {
return false;
}
@@ -111,7 +114,7 @@ static bool enable(const ::testing::TestInfo& test)
// On Linux non-privileged users are limited to 64k of locked memory so we
// cannot run the MemIsolatorTest.Usage.
- if (strings::contains(name, "MemIsolatorTest") && os::user() != "root") {
+ if (strings::contains(name, "MemIsolatorTest") && user.get() != "root") {
return false;
}
#endif
@@ -136,7 +139,7 @@ static bool enable(const ::testing::TestInfo& test)
const string& type = test.type_param();
if (strings::contains(type, "Cgroups")) {
#ifdef __linux__
- return os::user() == "root" && cgroups::enabled();
+ return user.get() == "root" && cgroups::enabled();
#else
return false;
#endif
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index f197da6..1037420 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -349,9 +349,12 @@ slave::Flags ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
slave::Flags flags = MesosTest::CreateSlaveFlags();
#ifdef __linux__
+ Result<string> user = os::user();
+ EXPECT_SOME(user);
+
// Use cgroup isolators if they're available and we're root.
// TODO(idownes): Refactor the cgroups/non-cgroups code.
- if (cgroups::enabled() && os::user() == "root") {
+ if (cgroups::enabled() && user.get() == "root") {
flags.isolation = "cgroups/cpu,cgroups/mem";
flags.cgroups_hierarchy = baseHierarchy;
flags.cgroups_root = TEST_CGROUPS_ROOT + "_" + UUID::random().toString();
@@ -372,7 +375,10 @@ slave::Flags ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
#ifdef __linux__
void ContainerizerTest<slave::MesosContainerizer>::SetUpTestCase()
{
- if (cgroups::enabled() && os::user() == "root") {
+ Result<string> user = os::user();
+ EXPECT_SOME(user);
+
+ if (cgroups::enabled() && user.get() == "root") {
// Clean up any testing hierarchies.
Try<std::set<string> > hierarchies = cgroups::hierarchies();
ASSERT_SOME(hierarchies);
@@ -387,7 +393,10 @@ void ContainerizerTest<slave::MesosContainerizer>::SetUpTestCase()
void ContainerizerTest<slave::MesosContainerizer>::TearDownTestCase()
{
- if (cgroups::enabled() && os::user() == "root") {
+ Result<string> user = os::user();
+ EXPECT_SOME(user);
+
+ if (cgroups::enabled() && user.get() == "root") {
// Clean up any testing hierarchies.
Try<std::set<string> > hierarchies = cgroups::hierarchies();
ASSERT_SOME(hierarchies);
@@ -410,7 +419,10 @@ void ContainerizerTest<slave::MesosContainerizer>::SetUp()
subsystems.insert("freezer");
subsystems.insert("perf_event");
- if (cgroups::enabled() && os::user() == "root") {
+ Result<string> user = os::user();
+ EXPECT_SOME(user);
+
+ if (cgroups::enabled() && user.get() == "root") {
foreach (const string& subsystem, subsystems) {
// Establish the base hierarchy if this is the first subsystem checked.
if (baseHierarchy.empty()) {
@@ -456,7 +468,10 @@ void ContainerizerTest<slave::MesosContainerizer>::TearDown()
{
MesosTest::TearDown();
- if (cgroups::enabled() && os::user() == "root") {
+ Result<string> user = os::user();
+ EXPECT_SOME(user);
+
+ if (cgroups::enabled() && user.get() == "root") {
foreach (const string& subsystem, subsystems) {
string hierarchy = path::join(baseHierarchy, subsystem);
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/tests/script.cpp
----------------------------------------------------------------------
diff --git a/src/tests/script.cpp b/src/tests/script.cpp
index 9f1be63..d57fc7d 100644
--- a/src/tests/script.cpp
+++ b/src/tests/script.cpp
@@ -139,7 +139,10 @@ void execute(const string& script)
mesos::ACL::RunTasks* run = acls.add_run_tasks();
run->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
- run->mutable_users()->add_values(os::user());
+
+ Result<string> user = os::user();
+ CHECK_SOME(user) << "Failed to get current user name";
+ run->mutable_users()->add_values(user.get());
mesos::ACL::ReceiveOffers* offer = acls.add_receive_offers();
offer->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
http://git-wip-us.apache.org/repos/asf/mesos/blob/ef2106e6/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 3a45191..21fe685 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -429,11 +429,13 @@ TEST_F(SlaveTest, ROOT_RunTaskWithCommandInfoWithoutUser)
task.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id());
task.mutable_resources()->MergeFrom(offers.get()[0].resources());
- // Command executor will run as user running test.
- string user = os::user();
+ Result<string> user = os::user();
+ CHECK_SOME(user) << "Failed to get current user name"
+ << (user.isError() ? ": " + user.error() : "");
+ // Command executor will run as user running test.
CommandInfo command;
- command.set_value("test `whoami` = " + user);
+ command.set_value("test `whoami` = " + user.get());
task.mutable_command()->MergeFrom(command);