You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/11/23 21:59:04 UTC

[1/9] mesos git commit: Added flag for passing in a user to the logrotate module.

Repository: mesos
Updated Branches:
  refs/heads/master 212486a93 -> 1fc0551db


Added flag for passing in a user to the logrotate module.

This adds an optional field to the LogrotateContainerLogger's
companion binary.  When specified, the companion binary should switch
to the given user after being launched.

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


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

Branch: refs/heads/master
Commit: 120274ac51e5ae10e9530201ab67e56fa29edd6e
Parents: 212486a
Author: Sivaram Kannan <si...@gmail.com>
Authored: Wed Nov 23 11:14:36 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 11:39:05 2016 -0800

----------------------------------------------------------------------
 src/slave/container_loggers/logrotate.cpp | 11 +++++++++++
 src/slave/container_loggers/logrotate.hpp |  5 +++++
 2 files changed, 16 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/120274ac/src/slave/container_loggers/logrotate.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/logrotate.cpp b/src/slave/container_loggers/logrotate.cpp
index 431bc3c..61484b1 100644
--- a/src/slave/container_loggers/logrotate.cpp
+++ b/src/slave/container_loggers/logrotate.cpp
@@ -37,6 +37,7 @@
 
 #include <stout/os/pagesize.hpp>
 #include <stout/os/shell.hpp>
+#include <stout/os/su.hpp>
 #include <stout/os/write.hpp>
 
 #include "slave/container_loggers/logrotate.hpp"
@@ -242,6 +243,16 @@ int main(int argc, char** argv)
       << ErrnoError("Failed to put child in a new session").message;
   }
 
+  // If the `--user` flag is set, change the UID of this process to that user.
+  if (flags.user.isSome()) {
+    Try<Nothing> result = os::su(flags.user.get());
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE)
+        << ErrnoError("Failed to switch user for logrotate process").message;
+    }
+  }
+
   // Asynchronously control the flow and size of logs.
   LogrotateLoggerProcess process(flags);
   spawn(&process);

http://git-wip-us.apache.org/repos/asf/mesos/blob/120274ac/src/slave/container_loggers/logrotate.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/logrotate.hpp b/src/slave/container_loggers/logrotate.hpp
index d1db692..96dbd2d 100644
--- a/src/slave/container_loggers/logrotate.hpp
+++ b/src/slave/container_loggers/logrotate.hpp
@@ -112,12 +112,17 @@ struct Flags : public virtual flags::FlagsBase
 
           return None();
         });
+
+    add(&Flags::user,
+        "user",
+        "The user this command should run as.");
   }
 
   Bytes max_size;
   Option<std::string> logrotate_options;
   Option<std::string> log_filename;
   std::string logrotate_path;
+  Option<std::string> user;
 };
 
 } // namespace rotate {


[3/9] mesos git commit: Added test cases for the logrotate module with --switch_user.

Posted by jo...@apache.org.
Added test cases for the logrotate module with --switch_user.

This adds two regression tests for running a task under a different
user than the agent.  In both cases, the logrotate module should
still rotate logs, but the user that owns the log files will differ.

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


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

Branch: refs/heads/master
Commit: 1f93bdd9240d9d23456e4a5da1299dc4135512e7
Parents: d4ba90f
Author: Sivaram Kannan <si...@gmail.com>
Authored: Wed Nov 23 11:14:41 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 12:05:00 2016 -0800

----------------------------------------------------------------------
 src/tests/container_logger_tests.cpp | 192 ++++++++++++++++++++++++++++++
 1 file changed, 192 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1f93bdd9/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp
index 3c7626d..6f69c08 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -40,6 +40,7 @@
 #include <stout/os/pstree.hpp>
 #include <stout/os/read.hpp>
 #include <stout/os/stat.hpp>
+#include <stout/os/su.hpp>
 
 #include "master/master.hpp"
 
@@ -89,6 +90,7 @@ using std::vector;
 using testing::_;
 using testing::AtMost;
 using testing::Return;
+using testing::WithParamInterface;
 
 namespace mesos {
 namespace internal {
@@ -579,6 +581,196 @@ TEST_F(ContainerLoggerTest, LOGROTATE_ModuleFDOwnership)
   driver.join();
 }
 
+
+// These tests are parameterized by the boolean `--switch-user` agent flag.
+class UserContainerLoggerTest
+  : public ContainerLoggerTest, public WithParamInterface<bool> {};
+
+INSTANTIATE_TEST_CASE_P(
+    bool,
+    UserContainerLoggerTest,
+    ::testing::Values(true, false));
+
+
+// Tests that the packaged logrotate container logger will rotate files when
+// the agent is root, but the executor is launched as a non-root user.
+//
+// 1. When `--switch_user` is true on the agent, the logger module should
+//    launch subprocesses with the same user as the executor.
+// 2. When `--switch_user` is false on the agent, the logger module should
+//    inherit the user of the agent.
+TEST_P(UserContainerLoggerTest, ROOT_LOGROTATE_RotateWithSwitchUserTrueOrFalse)
+{
+  // Create a master, agent, and framework.
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+  // We'll need access to these flags later.
+  slave::Flags flags = CreateSlaveFlags();
+
+  // Use the non-default container logger that rotates logs.
+  flags.container_logger = LOGROTATE_CONTAINER_LOGGER_NAME;
+
+  // Parameterize the `--switch_user` flag to test both options.
+  flags.switch_user = GetParam();
+
+  // In order for the unprivileged user to successfully chdir, the
+  // agent's work directory needs to have execute permissions.
+  Try<Nothing> chmod = os::chmod(
+      flags.work_dir, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH);
+  ASSERT_SOME(chmod);
+
+  Fetcher fetcher;
+
+  // We use an actual containerizer + executor since we want something to run.
+  Try<MesosContainerizer*> _containerizer =
+    MesosContainerizer::create(flags, false, &fetcher);
+
+  CHECK_SOME(_containerizer);
+  Owned<MesosContainerizer> containerizer(_containerizer.get());
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  Try<Owned<cluster::Slave>> slave =
+    StartSlave(detector.get(), containerizer.get(), flags);
+  ASSERT_SOME(slave);
+
+  AWAIT_READY(slaveRegisteredMessage);
+  SlaveID slaveId = slaveRegisteredMessage.get().slave_id();
+
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  Future<FrameworkID> frameworkId;
+  EXPECT_CALL(sched, registered(&driver, _, _))
+    .WillOnce(FutureArg<1>(&frameworkId));
+
+  // Wait for an offer, and start a task.
+  Future<vector<Offer>> offers;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureArg<1>(&offers))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+  driver.start();
+  AWAIT_READY(frameworkId);
+
+  AWAIT_READY(offers);
+  EXPECT_NE(0u, offers.get().size());
+
+  // Start a task that spams stdout with 3 MB of (mostly blank) output.
+  // The logrotate container logger module is loaded with parameters that limit
+  // the log size to five files of 2 MB each.  After the task completes, there
+  // should be two files with a total size of 3 MB.  The "stdout" file should
+  // be 1 MB large.
+  TaskInfo task = createTask(
+      offers.get()[0],
+      "i=0; while [ $i -lt 3072 ]; "
+      "do printf '%-1024d\\n' $i; i=$((i+1)); done");
+
+  // Start the task as a non-root user.
+  task.mutable_command()->set_user("nobody");
+
+  Future<TaskStatus> statusRunning;
+  Future<TaskStatus> statusFinished;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusRunning))
+    .WillOnce(FutureArg<1>(&statusFinished))
+    .WillRepeatedly(Return());       // Ignore subsequent updates.
+
+  driver.launchTasks(offers.get()[0].id(), {task});
+
+  AWAIT_READY(statusRunning);
+  EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
+
+  AWAIT_READY(statusFinished);
+  EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+
+  driver.stop();
+  driver.join();
+
+  // The `LogrotateContainerLogger` spawns some `mesos-logrotate-logger`
+  // processes above, which continue running briefly after the container exits.
+  // Once they finish reading the container's pipe, they should exit.
+  Try<os::ProcessTree> pstrees = os::pstree(0);
+  ASSERT_SOME(pstrees);
+  foreach (const os::ProcessTree& pstree, pstrees.get().children) {
+    // Wait for the logger subprocesses to exit, for up to 5 seconds each.
+    Duration waited = Duration::zero();
+    do {
+      if (!os::exists(pstree.process.pid)) {
+        break;
+      }
+
+      // Push the clock ahead to speed up the reaping of subprocesses.
+      Clock::pause();
+      Clock::settle();
+      Clock::advance(Seconds(1));
+      Clock::resume();
+
+      os::sleep(Milliseconds(100));
+      waited += Milliseconds(100);
+    } while (waited < Seconds(5));
+
+    EXPECT_LE(waited, Seconds(5));
+  }
+
+  // Check for the expected log rotation.
+  string sandboxDirectory = path::join(
+      slave::paths::getExecutorPath(
+          flags.work_dir,
+          slaveId,
+          frameworkId.get(),
+          statusRunning->executor_id()),
+      "runs",
+      "latest");
+
+  ASSERT_TRUE(os::exists(sandboxDirectory));
+
+  // The leading log file should be owned by the logrotate module's
+  // companion binary's user.
+  string stdoutPath = path::join(sandboxDirectory, "stdout");
+  ASSERT_TRUE(os::exists(stdoutPath));
+
+  struct stat stdoutStat;
+  ASSERT_GE(::stat(stdoutPath.c_str(), &stdoutStat), 0);
+
+  // Depending on the `--switch_user`, the expected user is either
+  // "nobody" or "root".
+  Result<string> stdoutUser = os::user(stdoutStat.st_uid);
+  if (GetParam()) {
+    ASSERT_SOME_EQ("nobody", stdoutUser);
+  } else {
+    ASSERT_SOME_EQ("root", stdoutUser);
+  }
+
+  // The leading log file should be about half full (1 MB).
+  // NOTE: We don't expect the size of the leading log file to be precisely
+  // one MB since there is also the executor's output besides the task's stdout.
+  Try<Bytes> stdoutSize = os::stat::size(stdoutPath);
+  ASSERT_SOME(stdoutSize);
+  EXPECT_LE(1024u, stdoutSize->kilobytes());
+  EXPECT_GE(1050u, stdoutSize->kilobytes());
+
+  // We should only have files up to "stdout.1".
+  stdoutPath = path::join(sandboxDirectory, "stdout.2");
+  EXPECT_FALSE(os::exists(stdoutPath));
+
+  // The only rotated log file (2 MB each) should be present.
+  stdoutPath = path::join(sandboxDirectory, "stdout.1");
+  ASSERT_TRUE(os::exists(stdoutPath));
+
+  // NOTE: The rotated files are written in contiguous blocks, meaning that
+  // each file may be less than the maximum allowed size.
+  stdoutSize = os::stat::size(stdoutPath);
+  ASSERT_SOME(stdoutSize);
+  EXPECT_LE(2040u, stdoutSize->kilobytes());
+  EXPECT_GE(2048u, stdoutSize->kilobytes());
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {


[2/9] mesos git commit: Added new parameter for user to ContainerLogger's prepare function.

Posted by jo...@apache.org.
Added new parameter for user to ContainerLogger's prepare function.

This new parameter allows the containerizer to pass along the
user the container will eventually run as.  The ContainerLogger
module may choose to launch subprocesses under that user if applicable.

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


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

Branch: refs/heads/master
Commit: d4ba90fac3b8d0507d8d952c413730b78c8be4ee
Parents: 120274a
Author: Sivaram Kannan <si...@gmail.com>
Authored: Wed Nov 23 11:14:39 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 11:47:52 2016 -0800

----------------------------------------------------------------------
 include/mesos/slave/container_logger.hpp        |  3 ++-
 src/slave/container_loggers/lib_logrotate.cpp   | 11 ++++++++---
 src/slave/container_loggers/lib_logrotate.hpp   |  3 ++-
 src/slave/container_loggers/sandbox.cpp         |  9 ++++++---
 src/slave/container_loggers/sandbox.hpp         |  3 ++-
 src/slave/containerizer/docker.cpp              | 10 ++++++++--
 src/slave/containerizer/mesos/containerizer.cpp |  8 +++++++-
 src/tests/container_logger_tests.cpp            |  6 +++---
 8 files changed, 38 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/include/mesos/slave/container_logger.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/slave/container_logger.hpp b/include/mesos/slave/container_logger.hpp
index 9399747..43bd4b6 100644
--- a/include/mesos/slave/container_logger.hpp
+++ b/include/mesos/slave/container_logger.hpp
@@ -150,7 +150,8 @@ public:
    */
   virtual process::Future<SubprocessInfo> prepare(
       const ExecutorInfo& executorInfo,
-      const std::string& sandboxDirectory) = 0;
+      const std::string& sandboxDirectory,
+      const Option<std::string>& user) = 0;
 };
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/container_loggers/lib_logrotate.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index 53698d3..27726ab 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -73,7 +73,8 @@ public:
   // the files according to the configured maximum size and number of files.
   Future<SubprocessInfo> prepare(
       const ExecutorInfo& executorInfo,
-      const std::string& sandboxDirectory)
+      const std::string& sandboxDirectory,
+      const Option<std::string>& user)
   {
     // Inherit most, but not all of the agent's environment.
     // Since the subprocess links to libmesos, it will need some of the
@@ -163,6 +164,7 @@ public:
     outFlags.logrotate_options = overridenFlags.logrotate_stdout_options;
     outFlags.log_filename = path::join(sandboxDirectory, "stdout");
     outFlags.logrotate_path = flags.logrotate_path;
+    outFlags.user = user;
 
     // If we are on systemd, then extend the life of the process as we
     // do with the executor. Any grandchildren's lives will also be
@@ -220,6 +222,7 @@ public:
     errFlags.logrotate_options = overridenFlags.logrotate_stderr_options;
     errFlags.log_filename = path::join(sandboxDirectory, "stderr");
     errFlags.logrotate_path = flags.logrotate_path;
+    errFlags.user = user;
 
     Try<Subprocess> errProcess = subprocess(
         path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
@@ -275,13 +278,15 @@ Try<Nothing> LogrotateContainerLogger::initialize()
 
 Future<SubprocessInfo> LogrotateContainerLogger::prepare(
     const ExecutorInfo& executorInfo,
-    const std::string& sandboxDirectory)
+    const std::string& sandboxDirectory,
+    const Option<std::string>& user)
 {
   return dispatch(
       process.get(),
       &LogrotateContainerLoggerProcess::prepare,
       executorInfo,
-      sandboxDirectory);
+      sandboxDirectory,
+      user);
 }
 
 } // namespace logger {

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/container_loggers/lib_logrotate.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/lib_logrotate.hpp b/src/slave/container_loggers/lib_logrotate.hpp
index 28fdf3b..f2abfe7 100644
--- a/src/slave/container_loggers/lib_logrotate.hpp
+++ b/src/slave/container_loggers/lib_logrotate.hpp
@@ -198,7 +198,8 @@ public:
   virtual process::Future<mesos::slave::ContainerLogger::SubprocessInfo>
   prepare(
       const ExecutorInfo& executorInfo,
-      const std::string& sandboxDirectory);
+      const std::string& sandboxDirectory,
+      const Option<std::string>& user);
 
 protected:
   Flags flags;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/container_loggers/sandbox.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.cpp b/src/slave/container_loggers/sandbox.cpp
index cc263eb..b55e089 100644
--- a/src/slave/container_loggers/sandbox.cpp
+++ b/src/slave/container_loggers/sandbox.cpp
@@ -57,7 +57,8 @@ public:
 
   process::Future<ContainerLogger::SubprocessInfo> prepare(
       const ExecutorInfo& executorInfo,
-      const std::string& sandboxDirectory)
+      const std::string& sandboxDirectory,
+      const Option<std::string>& user)
   {
     ContainerLogger::SubprocessInfo info;
 
@@ -92,13 +93,15 @@ Try<Nothing> SandboxContainerLogger::initialize()
 Future<ContainerLogger::SubprocessInfo>
 SandboxContainerLogger::prepare(
     const ExecutorInfo& executorInfo,
-    const std::string& sandboxDirectory)
+    const std::string& sandboxDirectory,
+    const Option<std::string>& user)
 {
   return dispatch(
       process.get(),
       &SandboxContainerLoggerProcess::prepare,
       executorInfo,
-      sandboxDirectory);
+      sandboxDirectory,
+      user);
 }
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/container_loggers/sandbox.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.hpp b/src/slave/container_loggers/sandbox.hpp
index e0aeb32..4ec090c 100644
--- a/src/slave/container_loggers/sandbox.hpp
+++ b/src/slave/container_loggers/sandbox.hpp
@@ -61,7 +61,8 @@ public:
   virtual process::Future<mesos::slave::ContainerLogger::SubprocessInfo>
   prepare(
       const ExecutorInfo& executorInfo,
-      const std::string& sandboxDirectory);
+      const std::string& sandboxDirectory,
+      const Option<std::string>& user);
 
 protected:
   process::Owned<SandboxContainerLoggerProcess> process;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index ccabf99..a8e522f 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1289,7 +1289,10 @@ Future<Docker::Container> DockerContainerizerProcess::launchExecutorContainer(
   Container* container = containers_.at(containerId);
   container->state = Container::RUNNING;
 
-  return logger->prepare(container->executor, container->directory)
+  return logger->prepare(
+      container->executor,
+      container->directory,
+      container->user)
     .then(defer(
         self(),
         [=](const ContainerLogger::SubprocessInfo& subprocessInfo)
@@ -1398,7 +1401,10 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
 
   return allocateGpus
     .then(defer(self(), [=]() {
-      return logger->prepare(container->executor, container->directory);
+      return logger->prepare(
+          container->executor,
+          container->directory,
+          container->user);
     }))
     .then(defer(
         self(),

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index e47a120..d9e6bb4 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1393,9 +1393,15 @@ Future<bool> MesosContainerizerProcess::_launch(
     executorInfo = containers_[rootContainerId]->config.executor_info();
   }
 
+  Option<string> user;
+  if (container->config.has_user()) {
+    user = container->config.user();
+  }
+
   return logger->prepare(
       executorInfo,
-      container->config.directory())
+      container->config.directory(),
+      user)
     .then(defer(
         self(),
         [=](const ContainerLogger::SubprocessInfo& subprocessInfo)

http://git-wip-us.apache.org/repos/asf/mesos/blob/d4ba90fa/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp
index 1bb94a8..3c7626d 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -109,7 +109,7 @@ public:
       .WillRepeatedly(Return(Nothing()));
 
     // All output is redirected to STDOUT_FILENO and STDERR_FILENO.
-    EXPECT_CALL(*this, prepare(_, _))
+    EXPECT_CALL(*this, prepare(_, _, _))
       .WillRepeatedly(Return(mesos::slave::ContainerLogger::SubprocessInfo()));
   }
 
@@ -117,10 +117,10 @@ public:
 
   MOCK_METHOD0(initialize, Try<Nothing>(void));
 
-  MOCK_METHOD2(
+  MOCK_METHOD3(
       prepare,
       Future<mesos::slave::ContainerLogger::SubprocessInfo>(
-          const ExecutorInfo&, const string&));
+          const ExecutorInfo&, const string&, const Option<string>&));
 };
 
 


[6/9] mesos git commit: Modified a test to use the updated Bytes operators.

Posted by jo...@apache.org.
Modified a test to use the updated Bytes operators.

This is the only test that multiplies/divides Bytes by a decimal value.

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


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

Branch: refs/heads/master
Commit: c1218239da746a8db7a79beffb6982d466a018d1
Parents: 4c79b39
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Nov 23 13:08:43 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 13:08:43 2016 -0800

----------------------------------------------------------------------
 src/tests/hierarchical_allocator_tests.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c1218239/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index edd5cea..3b0eb75 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1061,15 +1061,15 @@ TEST_F(HierarchicalAllocatorTest, Allocatable)
 
   // Not enough memory or cpu to be considered allocatable.
   SlaveInfo slave1 = createSlaveInfo(
-      "cpus:" + stringify(MIN_CPUS / 2) + ";"
-      "mem:" + stringify((MIN_MEM / 2).megabytes()) + ";"
+      "cpus:" + stringify(MIN_CPUS / 2u) + ";"
+      "mem:" + stringify((MIN_MEM / 2u).megabytes()) + ";"
       "disk:128");
   allocator->addSlave(slave1.id(), slave1, None(), slave1.resources(), {});
 
   // Enough cpus to be considered allocatable.
   SlaveInfo slave2 = createSlaveInfo(
       "cpus:" + stringify(MIN_CPUS) + ";"
-      "mem:" + stringify((MIN_MEM / 2).megabytes()) + ";"
+      "mem:" + stringify((MIN_MEM / 2u).megabytes()) + ";"
       "disk:128");
   allocator->addSlave(slave2.id(), slave2, None(), slave2.resources(), {});
 
@@ -1082,7 +1082,7 @@ TEST_F(HierarchicalAllocatorTest, Allocatable)
 
   // Enough memory to be considered allocatable.
   SlaveInfo slave3 = createSlaveInfo(
-      "cpus:" + stringify(MIN_CPUS / 2) + ";"
+      "cpus:" + stringify(MIN_CPUS / 2u) + ";"
       "mem:" + stringify((MIN_MEM).megabytes()) + ";"
       "disk:128");
   allocator->addSlave(slave3.id(), slave3, None(), slave3.resources(), {});
@@ -1097,10 +1097,10 @@ TEST_F(HierarchicalAllocatorTest, Allocatable)
   // slave4 has enough cpu and memory to be considered allocatable,
   // but it lies across unreserved and reserved resources!
   SlaveInfo slave4 = createSlaveInfo(
-      "cpus:" + stringify(MIN_CPUS / 1.5) + ";"
-      "mem:" + stringify((MIN_MEM / 2).megabytes()) + ";"
-      "cpus(role1):" + stringify(MIN_CPUS / 1.5) + ";"
-      "mem(role1):" + stringify((MIN_MEM / 2).megabytes()) + ";"
+      "cpus:" + stringify(MIN_CPUS * 3u / 2u) + ";"
+      "mem:" + stringify((MIN_MEM / 2u).megabytes()) + ";"
+      "cpus(role1):" + stringify(MIN_CPUS * 3u / 2u) + ";"
+      "mem(role1):" + stringify((MIN_MEM / 2u).megabytes()) + ";"
       "disk:128");
   allocator->addSlave(slave4.id(), slave4, None(), slave4.resources(), {});
 


[9/9] mesos git commit: Transformed a fatal log message into a simple exit.

Posted by jo...@apache.org.
Transformed a fatal log message into a simple exit.

The failures we see while validating flags in libprocess does not
warrant printing a stack trace on validation error.  The stack trace
gives a false impression that something went wrong in libprocess,
versus the user specifying an incorrect environment variable.

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


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

Branch: refs/heads/master
Commit: 1fc0551db54cd4492c3772d68212b9d7c6571728
Parents: 1d33559
Author: Armand Grillet <ar...@gmail.com>
Authored: Wed Nov 23 13:21:34 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 13:44:57 2016 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1fc0551d/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 7a7586b..e9a4bbb 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -1077,7 +1077,7 @@ bool initialize(
   Try<flags::Warnings> load = flags.load("LIBPROCESS_");
 
   if (load.isError()) {
-    LOG(FATAL) << flags.usage(load.error());
+    EXIT(EXIT_FAILURE) << flags.usage(load.error());
   }
 
   // Log any flag warnings.


[4/9] mesos git commit: Added note about MESOS-5856 to the upgrade doc.

Posted by jo...@apache.org.
Added note about MESOS-5856 to the upgrade doc.

The 1.2 release contains a breaking change to the ContainerLogger
interface.  The change is an additional argument in one of the
module's methods.


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

Branch: refs/heads/master
Commit: 842c4cf42766473764fe58d78bad430745abc043
Parents: 1f93bdd
Author: Joseph Wu <jo...@apache.org>
Authored: Wed Nov 23 11:32:03 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 12:29:55 2016 -0800

----------------------------------------------------------------------
 docs/upgrades.md | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/842c4cf4/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index 8e45fdb..74f4367 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -53,6 +53,24 @@ We categorize the changes as follows:
   </td>
   <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Module API-->
     <ul style="padding-left:10px;">
+      <li>C <a href="#1-2-x-container-logger-interface">Container Logger prepare method</a></li>
+    </ul>
+  </td>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Endpoints-->
+  </td>
+</tr>
+<tr>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Version-->
+  1.1.x
+  </td>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Mesos Core-->
+  </td>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Flags-->
+  </td>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Framework API-->
+  </td>
+  <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Module API-->
+    <ul style="padding-left:10px;">
       <li>R <a href="#1-1-x-container-logger-interface">Container Logger recovery method</a></li>
     </ul>
     <ul style="padding-left:10px;">
@@ -205,6 +223,12 @@ We categorize the changes as follows:
 </table>
 
 
+## Upgrading from 1.1.x to 1.2.x ##
+
+<a name="1-2-x-container-logger-interface"></a>
+
+* Mesos 1.2 modifies the `ContainerLogger`'s `prepare()` method.  The method now takes an additional argument for the `user` the logger should run a subprocess as.  Please see [MESOS-5856](https://issues.apache.org/jira/browse/MESOS-5856) for more information.
+
 ## Upgrading from 1.0.x to 1.1.x ##
 
 <a name="1-1-x-container-logger-interface"></a>


[7/9] mesos git commit: Added net::IP parsing template to the flags parsers.

Posted by jo...@apache.org.
Added net::IP parsing template to the flags parsers.

This will allow us to specify the `net::IP` type
as a field inside flags.

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


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

Branch: refs/heads/master
Commit: d022300acc907bfe507a7cb8f1c5767a2bd6d7bb
Parents: c121823
Author: Armand Grillet <ar...@gmail.com>
Authored: Wed Nov 23 13:21:31 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 13:37:44 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/flags/parse.hpp | 8 ++++++++
 1 file changed, 8 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d022300a/3rdparty/stout/include/stout/flags/parse.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/flags/parse.hpp b/3rdparty/stout/include/stout/flags/parse.hpp
index 67a89c9..65edd86 100644
--- a/3rdparty/stout/include/stout/flags/parse.hpp
+++ b/3rdparty/stout/include/stout/flags/parse.hpp
@@ -19,6 +19,7 @@
 #include <stout/bytes.hpp>
 #include <stout/duration.hpp>
 #include <stout/error.hpp>
+#include <stout/ip.hpp>
 #include <stout/json.hpp>
 #include <stout/path.hpp>
 #include <stout/strings.hpp>
@@ -77,6 +78,13 @@ inline Try<Bytes> parse(const std::string& value)
 
 
 template <>
+inline Try<net::IP> parse(const std::string& value)
+{
+  return net::IP::parse(value, AF_INET);
+}
+
+
+template <>
 inline Try<JSON::Object> parse(const std::string& value)
 {
   // A value that already starts with 'file://' will properly be


[8/9] mesos git commit: Transformed env variable parsing into Flags in libprocess.

Posted by jo...@apache.org.
Transformed env variable parsing into Flags in libprocess.

This retains existing environment variables read by libprocess
(LIBPROCESS + IP, ADVERTISE_IP, PORT, ADVERTISE_PORT) but parsing
is done via a Flags object.  This also documents the behavior and
expectations of the flags, and prints a more helpful error message
if the environment variables are set incorrectly.

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


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

Branch: refs/heads/master
Commit: 1d33559fad35a4ca84f275b1c1544aa5afa48e28
Parents: d022300
Author: Armand Grillet <ar...@gmail.com>
Authored: Wed Nov 23 13:21:32 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 13:42:13 2016 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 125 ++++++++++++++++++++++---------
 1 file changed, 90 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1d33559f/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index a2382ef..7a7586b 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -91,6 +91,7 @@
 #include <process/ssl/flags.hpp>
 
 #include <stout/duration.hpp>
+#include <stout/flags.hpp>
 #include <stout/foreach.hpp>
 #include <stout/lambda.hpp>
 #include <stout/net.hpp>
@@ -99,6 +100,7 @@
 #include <stout/os.hpp>
 #include <stout/os/strerror.hpp>
 #include <stout/path.hpp>
+#include <stout/stringify.hpp>
 #include <stout/strings.hpp>
 #include <stout/synchronized.hpp>
 #include <stout/thread_local.hpp>
@@ -150,6 +152,72 @@ using std::vector;
 
 namespace process {
 
+namespace internal {
+
+// These are environment variables expected in `process::initialize`.
+// All these flags should be loaded with the prefix "LIBPROCESS_".
+struct Flags : public virtual flags::FlagsBase
+{
+  Flags()
+  {
+    add(&Flags::ip,
+        "ip",
+        "The IP address for communication to and from libprocess.\n"
+        "If not specified, libprocess will attempt to reverse-DNS lookup\n"
+        "the hostname and use that IP instead.");
+
+    add(&Flags::advertise_ip,
+        "advertise_ip",
+        "The IP address that will be advertised to the outside world\n"
+        "for communication to and from libprocess.  This is useful,\n"
+        "for example, for containerized tasks in which communication\n"
+        "is bound locally to a non-public IP that will be inaccessible\n"
+        "to the master.");
+
+    add(&Flags::port,
+        "port",
+        "The port for communication to and from libprocess.\n"
+        "If not specified or set to 0, libprocess will bind it to a random\n"
+        "available port.",
+        [](const Option<int>& value) -> Option<Error> {
+          if (value.isSome()) {
+            if (value.get() < 0 || value.get() > USHRT_MAX) {
+              return Error(
+                  "LIBPROCESS_PORT=" + stringify(value.get()) +
+                  " is not a valid port");
+            }
+          }
+
+          return None();
+        });
+
+    add(&Flags::advertise_port,
+        "advertise_port",
+        "The port that will be advertised to the outside world\n"
+        "for communication to and from libprocess.  NOTE: This port\n"
+        "will not actually be bound (only the local '--port' will be), so\n"
+        "redirection to the local IP and port must be provided separately.",
+        [](const Option<int>& value) -> Option<Error> {
+          if (value.isSome()) {
+            if (value.get() <= 0 || value.get() > USHRT_MAX) {
+              return Error(
+                  "LIBPROCESS_ADVERTISE_PORT=" + stringify(value.get()) +
+                  " is not a valid port");
+            }
+          }
+
+          return None();
+        });
+  }
+
+  Option<net::IP> ip;
+  Option<net::IP> advertise_ip;
+  Option<int> port;
+  Option<int> advertise_port;
+};
+
+} // namespace internal {
+
 namespace ID {
 
 string generate(const string& prefix)
@@ -1001,28 +1069,28 @@ bool initialize(
 
   Clock::initialize(lambda::bind(&timedout, lambda::_1));
 
+  // Fill in the local IP and port for inter-libprocess communication.
   __address__ = Address::LOCALHOST_ANY();
 
-  // Check environment for ip.
-  Option<string> value = os::getenv("LIBPROCESS_IP");
-  if (value.isSome()) {
-    Try<net::IP> ip = net::IP::parse(value.get(), AF_INET);
-    if (ip.isError()) {
-      LOG(FATAL) << "Parsing LIBPROCESS_IP=" << value.get()
-                 << " failed: " << ip.error();
-    }
-    __address__.ip = ip.get();
+  // Fetch and parse the libprocess environment variables.
+  internal::Flags flags;
+  Try<flags::Warnings> load = flags.load("LIBPROCESS_");
+
+  if (load.isError()) {
+    LOG(FATAL) << flags.usage(load.error());
   }
 
-  // Check environment for port.
-  value = os::getenv("LIBPROCESS_PORT");
-  if (value.isSome()) {
-    Try<int> result = numify<int>(value.get().c_str());
-    if (result.isSome() && result.get() >=0 && result.get() <= USHRT_MAX) {
-      __address__.port = result.get();
-    } else {
-      LOG(FATAL) << "LIBPROCESS_PORT=" << value.get() << " is not a valid port";
-    }
+  // Log any flag warnings.
+  foreach (const flags::Warning& warning, load->warnings) {
+    LOG(WARNING) << warning.message;
+  }
+
+  if (flags.ip.isSome()) {
+    __address__.ip = flags.ip.get();
+  }
+
+  if (flags.port.isSome()) {
+    __address__.port = flags.port.get();
   }
 
   // Create a "server" socket for communicating.
@@ -1053,25 +1121,12 @@ bool initialize(
   __address__ = bind.get();
 
   // If advertised IP and port are present, use them instead.
-  value = os::getenv("LIBPROCESS_ADVERTISE_IP");
-  if (value.isSome()) {
-    Try<net::IP> ip = net::IP::parse(value.get(), AF_INET);
-    if (ip.isError()) {
-      LOG(FATAL) << "Parsing LIBPROCESS_ADVERTISE_IP=" << value.get()
-                 << " failed: " << ip.error();
-    }
-    __address__.ip = ip.get();
+  if (flags.advertise_ip.isSome()) {
+    __address__.ip = flags.advertise_ip.get();
   }
 
-  value = os::getenv("LIBPROCESS_ADVERTISE_PORT");
-  if (value.isSome()) {
-    Try<int> result = numify<int>(value.get().c_str());
-    if (result.isSome() && result.get() >=0 && result.get() <= USHRT_MAX) {
-      __address__.port = result.get();
-    } else {
-      LOG(FATAL) << "LIBPROCESS_ADVERTISE_PORT=" << value.get()
-                 << " is not a valid port";
-    }
+  if (flags.advertise_port.isSome()) {
+    __address__.port = flags.advertise_port.get();
   }
 
   // Lookup hostname if missing ip or if ip is 0.0.0.0 in case we


[5/9] mesos git commit: Changed Bytes operators to take unsigned integers.

Posted by jo...@apache.org.
Changed Bytes operators to take unsigned integers.

The multiplication and division operators for `Bytes` take a `double`
argument.  Changing this to an unsigned integer removes the implicit
`double` to `uint64_t` casting.  These operators are currently only
used in tests.

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


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

Branch: refs/heads/master
Commit: 4c79b39330e38c865c5e9812e5911f598c1710fd
Parents: 842c4cf
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Nov 23 13:08:42 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Nov 23 13:08:42 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/bytes.hpp | 28 ++++------------------------
 3rdparty/stout/tests/bytes_tests.cpp   |  4 ++--
 2 files changed, 6 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/4c79b393/3rdparty/stout/include/stout/bytes.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/bytes.hpp b/3rdparty/stout/include/stout/bytes.hpp
index 9debe2f..98223db 100644
--- a/3rdparty/stout/include/stout/bytes.hpp
+++ b/3rdparty/stout/include/stout/bytes.hpp
@@ -97,24 +97,14 @@ public:
     return *this;
   }
 
-  Bytes& operator*=(double multiplier)
+  Bytes& operator*=(uint64_t multiplier)
   {
-    if (multiplier < 0) {
-      ABORT("Multiplying Bytes by negative multiplier "
-            "'" + stringify(multiplier) + "'");
-    }
-
     value *= multiplier;
     return *this;
   }
 
-  Bytes& operator/=(double divisor)
+  Bytes& operator/=(uint64_t divisor)
   {
-    if (divisor < 0) {
-      ABORT("Dividing Bytes by negative divisor "
-            "'" + stringify(divisor) + "'");
-    }
-
     value /= divisor;
     return *this;
   }
@@ -194,26 +184,16 @@ inline Bytes operator-(const Bytes& lhs, const Bytes& rhs)
 }
 
 
-inline Bytes operator*(const Bytes& lhs, double multiplier)
+inline Bytes operator*(const Bytes& lhs, uint64_t multiplier)
 {
-  if (multiplier < 0) {
-    ABORT("Multiplying Bytes by negative multiplier "
-          "'" + stringify(multiplier) + "'");
-  }
-
   Bytes result = lhs;
   result *= multiplier;
   return result;
 }
 
 
-inline Bytes operator/(const Bytes& lhs, double divisor)
+inline Bytes operator/(const Bytes& lhs, uint64_t divisor)
 {
-  if (divisor < 0) {
-    ABORT("Dividing Bytes by negative divisor "
-          "'" + stringify(divisor) + "'");
-  }
-
   Bytes result = lhs;
   result /= divisor;
   return result;

http://git-wip-us.apache.org/repos/asf/mesos/blob/4c79b393/3rdparty/stout/tests/bytes_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/bytes_tests.cpp b/3rdparty/stout/tests/bytes_tests.cpp
index 9d32ea1..1e175d7 100644
--- a/3rdparty/stout/tests/bytes_tests.cpp
+++ b/3rdparty/stout/tests/bytes_tests.cpp
@@ -44,9 +44,9 @@ TEST(BytesTest, Arithmetic)
   EXPECT_EQ(Terabytes(1), Gigabytes(512) + Gigabytes(512));
   EXPECT_EQ(Terabytes(1), Terabytes(2) - Terabytes(1));
 
-  EXPECT_EQ(Terabytes(1), Gigabytes(1) * 1024);
+  EXPECT_EQ(Terabytes(1), Gigabytes(1) * 1024u);
 
-  EXPECT_EQ(Gigabytes(1), Terabytes(1) / 1024);
+  EXPECT_EQ(Gigabytes(1), Terabytes(1) / 1024u);
 }