You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2016/01/25 05:13:43 UTC

[01/11] mesos git commit: Refactored Subprocess::IO to improve readability.

Repository: mesos
Updated Branches:
  refs/heads/master f3ae12f8e -> 4543cdf97


Refactored Subprocess::IO to improve readability.

Replaced `int pipefd[2]` with the structs `InputFileDescriptors` and
`OutputFileDescriptors`.

Replaced the `switch` statements inside `process::subprocess` with
lambdas defined in the static helpers (`PIPE`, `PATH`, `FD`).

Slight cleanup of Subprocess code.

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


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

Branch: refs/heads/master
Commit: 3673887d941b7dfea8fced8769a71856621c5674
Parents: f3ae12f
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:45:10 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:21 2016 -0800

----------------------------------------------------------------------
 .../libprocess/include/process/subprocess.hpp   | 102 +++---
 3rdparty/libprocess/src/subprocess.cpp          | 338 +++++++++----------
 2 files changed, 222 insertions(+), 218 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/3673887d/3rdparty/libprocess/include/process/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index 8704cd0..5b2489e 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -28,6 +28,7 @@
 
 #include <stout/flags.hpp>
 #include <stout/lambda.hpp>
+#include <stout/none.hpp>
 #include <stout/option.hpp>
 #include <stout/try.hpp>
 
@@ -47,19 +48,46 @@ public:
   /**
    * Describes how the I/O is redirected for stdin/stdout/stderr.
    * One of the following three modes are supported:
-   *   1. PIPE: Redirect to a pipe. The pipe will be created
-   *      automatically and the user can read/write the parent side of
-   *      the pipe from in()/out()/err().
-   *   2. PATH: Redirect to a file. The file will be created if it
-   *      does not exist. If the file exists, it will be appended.
+   *   1. PIPE: Redirect to a pipe.  The pipe will be created when
+   *      launching a subprocess and the the user can read/write the
+   *      parent side of the pipe using `Subprocess::in/out/err`.
+   *   2. PATH: Redirect to a file.  For stdout/stderr, the file will
+   *      be created if it does not exist.  If the file exists, it
+   *      will be appended.
    *   3. FD: Redirect to an open file descriptor.
    */
   class IO
   {
   public:
-    bool isPipe() const { return mode == PIPE; }
-    bool isPath() const { return mode == PATH; }
-    bool isFd() const { return mode == FD; }
+    /**
+     * For input file descriptors a child reads from the `read` file
+     * descriptor and a parent may write to the `write` file
+     * descriptor if one is present.
+     *
+     * NOTE: We initialize `read` to -1 so that we do not close an
+     * arbitrary file descriptor,in case we encounter an error
+     * while starting a subprocess (closing -1 is always a no-op).
+     */
+    struct InputFileDescriptors
+    {
+      int read = -1;
+      Option<int> write = None();
+    };
+
+    /**
+     * For output file descriptors a child write to the `write` file
+     * descriptor and a parent may read from the `read` file
+     * descriptor if one is present.
+     *
+     * NOTE: We initialize `write` to -1 so that we do not close an
+     * arbitrary file descriptor,in case we encounter an error
+     * while starting a subprocess (closing -1 is always a no-op).
+     */
+    struct OutputFileDescriptors
+    {
+      Option<int> read = None();
+      int write = -1;
+    };
 
   private:
     friend class Subprocess;
@@ -76,50 +104,26 @@ public:
         const Option<lambda::function<
             pid_t(const lambda::function<int()>&)>>& clone);
 
-    enum Mode
-    {
-      PIPE, // Redirect I/O to a pipe.
-      PATH, // Redirect I/O to a file.
-      FD,   // Redirect I/O to an open file descriptor.
-    };
+    IO(const lambda::function<Try<InputFileDescriptors>()>& _input,
+       const lambda::function<Try<OutputFileDescriptors>()>& _output)
+      : input(_input),
+        output(_output) {}
 
-    IO(Mode _mode, const Option<int>& _fd, const Option<std::string>& _path)
-      : mode(_mode), fd(_fd), path(_path) {}
+    /**
+     * Prepares a set of file descriptors for stdin of a subprocess.
+     */
+    lambda::function<Try<InputFileDescriptors>()> input;
 
-    Mode mode;
-    Option<int> fd;
-    Option<std::string> path;
+    /**
+     * Prepares a set of file descriptors for stdout/stderr of a subprocess.
+     */
+    lambda::function<Try<OutputFileDescriptors>()> output;
   };
 
-  /**
-   * Provides some syntactic sugar to create an IO::PIPE redirector.
-   *
-   * @return An IO::PIPE redirector.
-   */
-  static IO PIPE()
-  {
-    return IO(IO::PIPE, None(), None());
-  }
-
-  /**
-   * Provides some syntactic sugar to create an IO::PATH redirector.
-   *
-   * @return An IO::PATH redirector.
-   */
-  static IO PATH(const std::string& path)
-  {
-    return IO(IO::PATH, None(), path);
-  }
-
-  /**
-   * Provides some syntactic sugar to create an IO::FD redirector.
-   *
-   * @return An IO::FD redirector.
-   */
-  static IO FD(int fd)
-  {
-    return IO(IO::FD, fd, None());
-  }
+  // Some syntactic sugar to create an IO::PIPE redirector.
+  static IO PIPE();
+  static IO PATH(const std::string& path);
+  static IO FD(int fd);
 
   /**
    * @return The operating system PID for this subprocess.
@@ -186,7 +190,7 @@ private:
     pid_t pid;
 
     // The parent side of the pipe for stdin/stdout/stderr. If the
-    // mode is not PIPE, None will be stored.
+    // IO mode is not a pipe, `None` will be stored.
     // NOTE: stdin, stdout, stderr are macros on some systems, hence
     // these names instead.
     Option<int> in;

http://git-wip-us.apache.org/repos/asf/mesos/blob/3673887d/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index 8635234..63cdcc4 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -38,6 +38,10 @@ using std::string;
 using std::vector;
 
 namespace process {
+
+using InputFileDescriptors = Subprocess::IO::InputFileDescriptors;
+using OutputFileDescriptors = Subprocess::IO::OutputFileDescriptors;
+
 namespace internal {
 
 // See the comment below as to why subprocess is passed to cleanup.
@@ -59,12 +63,17 @@ static void cleanup(
 }
 
 
-static void close(int stdinFd[2], int stdoutFd[2], int stderrFd[2])
+// This function will invoke `os::close` on all specified file
+// descriptors that are valid (i.e., not `None` and >= 0).
+static void close(
+    const InputFileDescriptors& stdinfds,
+    const OutputFileDescriptors& stdoutfds,
+    const OutputFileDescriptors& stderrfds)
 {
   int fds[6] = {
-    stdinFd[0], stdinFd[1],
-    stdoutFd[0], stdoutFd[1],
-    stderrFd[0], stderrFd[1]
+    stdinfds.read, stdinfds.write.getOrElse(-1),
+    stdoutfds.read.getOrElse(-1), stdoutfds.write,
+    stderrfds.read.getOrElse(-1), stderrfds.write
   };
 
   foreach (int fd, fds) {
@@ -74,14 +83,17 @@ static void close(int stdinFd[2], int stdoutFd[2], int stderrFd[2])
   }
 }
 
-// This function will invoke os::cloexec on all file descriptors in
-// these pairs that are valid (i.e., >= 0).
-static Try<Nothing> cloexec(int stdinFd[2], int stdoutFd[2], int stderrFd[2])
+// This function will invoke `os::cloexec` on all specified file
+// descriptors that are valid (i.e., not `None` and >= 0).
+static Try<Nothing> cloexec(
+    const InputFileDescriptors& stdinfds,
+    const OutputFileDescriptors& stdoutfds,
+    const OutputFileDescriptors& stderrfds)
 {
   int fds[6] = {
-    stdinFd[0], stdinFd[1],
-    stdoutFd[0], stdoutFd[1],
-    stderrFd[0], stderrFd[1]
+    stdinfds.read, stdinfds.write.getOrElse(-1),
+    stdoutfds.read.getOrElse(-1), stdoutfds.write,
+    stderrfds.read.getOrElse(-1), stderrfds.write
   };
 
   foreach (int fd, fds) {
@@ -99,6 +111,90 @@ static Try<Nothing> cloexec(int stdinFd[2], int stdoutFd[2], int stderrFd[2])
 }  // namespace internal {
 
 
+Subprocess::IO Subprocess::PIPE()
+{
+  return Subprocess::IO(
+      []() -> Try<InputFileDescriptors> {
+        int pipefd[2];
+        if (::pipe(pipefd) == -1) {
+          return ErrnoError("Failed to create pipe");
+        }
+
+        InputFileDescriptors fds;
+        fds.read = pipefd[0];
+        fds.write = pipefd[1];
+        return fds;
+      },
+      []() -> Try<OutputFileDescriptors> {
+        int pipefd[2];
+        if (::pipe(pipefd) == -1) {
+          return ErrnoError("Failed to create pipe");
+        }
+
+        OutputFileDescriptors fds;
+        fds.read = pipefd[0];
+        fds.write = pipefd[1];
+        return fds;
+      });
+}
+
+
+Subprocess::IO Subprocess::PATH(const string& path)
+{
+  return Subprocess::IO(
+      [path]() -> Try<InputFileDescriptors> {
+        Try<int> open = os::open(path, O_RDONLY | O_CLOEXEC);
+        if (open.isError()) {
+          return Error("Failed to open '" + path + "': " + open.error());
+        }
+
+        InputFileDescriptors fds;
+        fds.read = open.get();
+        return fds;
+      },
+      [path]() -> Try<OutputFileDescriptors> {
+        Try<int> open = os::open(
+            path,
+            O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC,
+            S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+        if (open.isError()) {
+          return Error("Failed to open '" + path + "': " + open.error());
+        }
+
+        OutputFileDescriptors fds;
+        fds.write = open.get();
+        return fds;
+      });
+}
+
+
+Subprocess::IO Subprocess::FD(int fd)
+{
+  return Subprocess::IO(
+      [fd]() -> Try<InputFileDescriptors> {
+        int prepared_fd = ::dup(fd);
+        if (prepared_fd == -1) {
+          return ErrnoError("Failed to dup");
+        }
+
+        InputFileDescriptors fds;
+        fds.read = prepared_fd;
+        return fds;
+      },
+      [fd]() -> Try<OutputFileDescriptors> {
+        int prepared_fd = ::dup(fd);
+        if (prepared_fd == -1) {
+          return ErrnoError("Failed to dup");
+        }
+
+        OutputFileDescriptors fds;
+        fds.write = prepared_fd;
+        return fds;
+      });
+}
+
+
 static pid_t defaultClone(const lambda::function<int()>& func)
 {
   pid_t pid = ::fork();
@@ -120,50 +216,47 @@ static pid_t defaultClone(const lambda::function<int()>& func)
 static int childMain(
     const string& path,
     char** argv,
-    const Subprocess::IO& in,
-    const Subprocess::IO& out,
-    const Subprocess::IO& err,
     char** envp,
     const Option<lambda::function<int()>>& setup,
-    int stdinFd[2],
-    int stdoutFd[2],
-    int stderrFd[2])
+    const InputFileDescriptors& stdinfds,
+    const OutputFileDescriptors& stdoutfds,
+    const OutputFileDescriptors& stderrfds)
 {
   // Close parent's end of the pipes.
-  if (in.isPipe()) {
-    ::close(stdinFd[1]);
+  if (stdinfds.write.isSome()) {
+    ::close(stdinfds.write.get());
   }
-  if (out.isPipe()) {
-    ::close(stdoutFd[0]);
+  if (stdoutfds.read.isSome()) {
+    ::close(stdoutfds.read.get());
   }
-  if (err.isPipe()) {
-    ::close(stderrFd[0]);
+  if (stderrfds.read.isSome()) {
+    ::close(stderrfds.read.get());
   }
 
   // Redirect I/O for stdin/stdout/stderr.
-  while (::dup2(stdinFd[0], STDIN_FILENO) == -1 && errno == EINTR);
-  while (::dup2(stdoutFd[1], STDOUT_FILENO) == -1 && errno == EINTR);
-  while (::dup2(stderrFd[1], STDERR_FILENO) == -1 && errno == EINTR);
+  while (::dup2(stdinfds.read, STDIN_FILENO) == -1 && errno == EINTR);
+  while (::dup2(stdoutfds.write, STDOUT_FILENO) == -1 && errno == EINTR);
+  while (::dup2(stderrfds.write, STDERR_FILENO) == -1 && errno == EINTR);
 
   // Close the copies. We need to make sure that we do not close the
   // file descriptor assigned to stdin/stdout/stderr in case the
   // parent has closed stdin/stdout/stderr when calling this
   // function (in that case, a dup'ed file descriptor may have the
   // same file descriptor number as stdin/stdout/stderr).
-  if (stdinFd[0] != STDIN_FILENO &&
-      stdinFd[0] != STDOUT_FILENO &&
-      stdinFd[0] != STDERR_FILENO) {
-    ::close(stdinFd[0]);
+  if (stdinfds.read != STDIN_FILENO &&
+      stdinfds.read != STDOUT_FILENO &&
+      stdinfds.read != STDERR_FILENO) {
+    ::close(stdinfds.read);
   }
-  if (stdoutFd[1] != STDIN_FILENO &&
-      stdoutFd[1] != STDOUT_FILENO &&
-      stdoutFd[1] != STDERR_FILENO) {
-    ::close(stdoutFd[1]);
+  if (stdoutfds.write != STDIN_FILENO &&
+      stdoutfds.write != STDOUT_FILENO &&
+      stdoutfds.write != STDERR_FILENO) {
+    ::close(stdoutfds.write);
   }
-  if (stderrFd[1] != STDIN_FILENO &&
-      stderrFd[1] != STDOUT_FILENO &&
-      stderrFd[1] != STDERR_FILENO) {
-    ::close(stderrFd[1]);
+  if (stderrfds.write != STDIN_FILENO &&
+      stderrfds.write != STDOUT_FILENO &&
+      stderrfds.write != STDERR_FILENO) {
+    ::close(stderrfds.write);
   }
 
   if (setup.isSome()) {
@@ -191,127 +284,44 @@ Try<Subprocess> subprocess(
     const Option<lambda::function<
         pid_t(const lambda::function<int()>&)>>& _clone)
 {
-  // File descriptors for redirecting stdin/stdout/stderr. These file
-  // descriptors are used for different purposes depending on the
-  // specified I/O modes. If the mode is PIPE, the two file
-  // descriptors represent two ends of a pipe. If the mode is PATH or
-  // FD, only one of the two file descriptors is used. Our protocol
-  // here is that index 0 is always for reading, and index 1 is always
-  // for writing (similar to the pipe semantics).
-  int stdinFd[2] = { -1, -1 };
-  int stdoutFd[2] = { -1, -1 };
-  int stderrFd[2] = { -1, -1 };
+  // File descriptors for redirecting stdin/stdout/stderr.
+  // These file descriptors are used for different purposes depending
+  // on the specified I/O modes.
+  // See `Subprocess::PIPE`, `Subprocess::PATH`, and `Subprocess::FD`.
+  InputFileDescriptors stdinfds;
+  OutputFileDescriptors stdoutfds;
+  OutputFileDescriptors stderrfds;
 
   // Prepare the file descriptor(s) for stdin.
-  switch (in.mode) {
-    case Subprocess::IO::FD: {
-      stdinFd[0] = ::dup(in.fd.get());
-      if (stdinFd[0] == -1) {
-        return ErrnoError("Failed to dup");
-      }
-      break;
-    }
-    case Subprocess::IO::PIPE: {
-      if (::pipe(stdinFd) == -1) {
-        return ErrnoError("Failed to create pipe");
-      }
-      break;
-    }
-    case Subprocess::IO::PATH: {
-      Try<int> open = os::open(in.path.get(), O_RDONLY | O_CLOEXEC);
-      if (open.isError()) {
-        return Error(
-            "Failed to open '" + in.path.get() + "': " + open.error());
-      }
-      stdinFd[0] = open.get();
-      break;
-    }
-    default:
-      UNREACHABLE();
+  Try<InputFileDescriptors> input = in.input();
+  if (input.isError()) {
+    return Error(input.error());
   }
 
+  stdinfds = input.get();
+
   // Prepare the file descriptor(s) for stdout.
-  switch (out.mode) {
-    case Subprocess::IO::FD: {
-      stdoutFd[1] = ::dup(out.fd.get());
-      if (stdoutFd[1] == -1) {
-        // Save the errno as 'close' below might overwrite it.
-        ErrnoError error("Failed to dup");
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return error;
-      }
-      break;
-    }
-    case Subprocess::IO::PIPE: {
-      if (::pipe(stdoutFd) == -1) {
-        // Save the errno as 'close' below might overwrite it.
-        ErrnoError error("Failed to create pipe");
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return error;
-      }
-      break;
-    }
-    case Subprocess::IO::PATH: {
-      Try<int> open = os::open(
-          out.path.get(),
-          O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC,
-          S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-
-      if (open.isError()) {
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return Error(
-            "Failed to open '" + out.path.get() + "': " + open.error());
-      }
-      stdoutFd[1] = open.get();
-      break;
-    }
-    default:
-      UNREACHABLE();
+  Try<OutputFileDescriptors> output = out.output();
+  if (output.isError()) {
+    internal::close(stdinfds, stdoutfds, stderrfds);
+    return Error(output.error());
   }
 
+  stdoutfds = output.get();
+
   // Prepare the file descriptor(s) for stderr.
-  switch (err.mode) {
-    case Subprocess::IO::FD: {
-      stderrFd[1] = ::dup(err.fd.get());
-      if (stderrFd[1] == -1) {
-        // Save the errno as 'close' below might overwrite it.
-        ErrnoError error("Failed to dup");
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return error;
-      }
-      break;
-    }
-    case Subprocess::IO::PIPE: {
-      if (::pipe(stderrFd) == -1) {
-        // Save the errno as 'close' below might overwrite it.
-        ErrnoError error("Failed to create pipe");
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return error;
-      }
-      break;
-    }
-    case Subprocess::IO::PATH: {
-      Try<int> open = os::open(
-          err.path.get(),
-          O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC,
-          S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-
-      if (open.isError()) {
-        internal::close(stdinFd, stdoutFd, stderrFd);
-        return Error(
-            "Failed to open '" + err.path.get() + "': " + open.error());
-      }
-      stderrFd[1] = open.get();
-      break;
-    }
-    default:
-      UNREACHABLE();
+  output = err.output();
+  if (output.isError()) {
+    internal::close(stdinfds, stdoutfds, stderrfds);
+    return Error(output.error());
   }
 
+  stderrfds = output.get();
+
   // TODO(jieyu): Consider using O_CLOEXEC for atomic close-on-exec.
-  Try<Nothing> cloexec = internal::cloexec(stdinFd, stdoutFd, stderrFd);
+  Try<Nothing> cloexec = internal::cloexec(stdinfds, stdoutfds, stderrfds);
   if (cloexec.isError()) {
-    internal::close(stdinFd, stdoutFd, stderrFd);
+    internal::close(stdinfds, stdoutfds, stderrfds);
     return Error("Failed to cloexec: " + cloexec.error());
   }
 
@@ -365,14 +375,11 @@ Try<Subprocess> subprocess(
       &childMain,
       path,
       _argv,
-      in,
-      out,
-      err,
       envp,
       setup,
-      stdinFd,
-      stdoutFd,
-      stderrFd));
+      stdinfds,
+      stdoutfds,
+      stderrfds));
 
   delete[] _argv;
 
@@ -386,7 +393,7 @@ Try<Subprocess> subprocess(
   if (pid == -1) {
     // Save the errno as 'close' below might overwrite it.
     ErrnoError error("Failed to clone");
-    internal::close(stdinFd, stdoutFd, stderrFd);
+    internal::close(stdinfds, stdoutfds, stderrfds);
     return error;
   }
 
@@ -394,24 +401,17 @@ Try<Subprocess> subprocess(
   Subprocess process;
   process.data->pid = pid;
 
-  // Close the file descriptors that are created by this function. For
-  // pipes, we close the child ends and store the parent ends (see the
-  // code below).
-  os::close(stdinFd[0]);
-  os::close(stdoutFd[1]);
-  os::close(stderrFd[1]);
+  // Close the child-ends of the file descriptors that are created
+  // by this function.
+  os::close(stdinfds.read);
+  os::close(stdoutfds.write);
+  os::close(stderrfds.write);
 
-  // If the mode is PIPE, store the parent side of the pipe so that
+  // For any pipes, store the parent side of the pipe so that
   // the user can communicate with the subprocess.
-  if (in.mode == Subprocess::IO::PIPE) {
-    process.data->in = stdinFd[1];
-  }
-  if (out.mode == Subprocess::IO::PIPE) {
-    process.data->out = stdoutFd[0];
-  }
-  if (err.mode == Subprocess::IO::PIPE) {
-    process.data->err = stderrFd[0];
-  }
+  process.data->in = stdinfds.write;
+  process.data->out = stdoutfds.read;
+  process.data->err = stderrfds.read;
 
   // Rather than directly exposing the future from process::reap, we
   // must use an explicit promise so that we can ensure we can receive


[10/11] mesos git commit: Implement the rotating container logger module.

Posted by be...@apache.org.
Implement the rotating container logger module.

Adds a non-default ContainerLogger that constrains total log size by
rotating logs (i.e. renaming the head log file).

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


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

Branch: refs/heads/master
Commit: 9fad619a4ceb3d7cfd2ef047ed92203c97b55145
Parents: a355f43
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Sun Jan 24 20:11:40 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:23 2016 -0800

----------------------------------------------------------------------
 src/slave/container_loggers/lib_logrotate.cpp | 216 ++++++++++++++++++
 src/slave/container_loggers/lib_logrotate.hpp | 171 ++++++++++++++
 src/slave/container_loggers/logrotate.cpp     | 247 +++++++++++++++++++++
 src/slave/container_loggers/logrotate.hpp     | 125 +++++++++++
 4 files changed, 759 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9fad619a/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
new file mode 100644
index 0000000..bfc7cad
--- /dev/null
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -0,0 +1,216 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <unistd.h>
+
+#include <map>
+#include <string>
+
+#include <mesos/mesos.hpp>
+
+#include <mesos/module/container_logger.hpp>
+
+#include <mesos/slave/container_logger.hpp>
+
+#include <process/dispatch.hpp>
+#include <process/future.hpp>
+#include <process/process.hpp>
+#include <process/subprocess.hpp>
+
+#include <stout/bytes.hpp>
+#include <stout/error.hpp>
+#include <stout/try.hpp>
+#include <stout/none.hpp>
+#include <stout/nothing.hpp>
+#include <stout/path.hpp>
+
+#include <stout/os/environment.hpp>
+#include <stout/os/killtree.hpp>
+
+#include "slave/container_loggers/logrotate.hpp"
+#include "slave/container_loggers/lib_logrotate.hpp"
+
+
+using namespace mesos;
+using namespace process;
+
+using mesos::slave::ContainerLogger;
+
+namespace mesos {
+namespace internal {
+namespace logger {
+
+using SubprocessInfo = ContainerLogger::SubprocessInfo;
+
+
+class LogrotateContainerLoggerProcess :
+  public Process<LogrotateContainerLoggerProcess>
+{
+public:
+  LogrotateContainerLoggerProcess(const Flags& _flags) : flags(_flags) {}
+
+  Future<Nothing> recover(
+      const ExecutorInfo& executorInfo,
+      const std::string& sandboxDirectory)
+  {
+    // No state to recover.
+    return Nothing();
+  }
+
+  // Spawns two subprocesses that read from their stdin and write to
+  // "stdout" and "stderr" files in the sandbox.  The subprocesses will rotate
+  // the files according to the configured maximum size and number of files.
+  Future<SubprocessInfo> prepare(
+      const ExecutorInfo& executorInfo,
+      const std::string& sandboxDirectory)
+  {
+    // Inherit most, but not all of the agent's environment.
+    // Since the subprocess links to libmesos, it will need some of the
+    // same environment used to launch the agent (also uses libmesos).
+    // The libprocess IP and port are explicitly removed because these
+    // will conflict with the already-running agent.
+    std::map<std::string, std::string> environment = os::environment();
+    environment.erase("LIBPROCESS_IP");
+    environment.erase("LIBPROCESS_PORT");
+    environment.erase("LIBPROCESS_ADVERTISE_IP");
+    environment.erase("LIBPROCESS_ADVERTISE_PORT");
+
+    // Spawn a process to handle stdout.
+    mesos::internal::logger::rotate::Flags outFlags;
+    outFlags.max_size = flags.max_stdout_size;
+    outFlags.logrotate_options = flags.logrotate_stdout_options;
+    outFlags.log_filename = path::join(sandboxDirectory, "stdout");
+    outFlags.logrotate_path = flags.logrotate_path;
+
+    Try<Subprocess> outProcess = subprocess(
+        path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
+        {mesos::internal::logger::rotate::NAME},
+        Subprocess::PIPE(),
+        Subprocess::PATH("/dev/null"),
+        Subprocess::FD(STDERR_FILENO),
+        outFlags,
+        environment);
+
+    if (outProcess.isError()) {
+      return Failure("Failed to create logger process: " + outProcess.error());
+    }
+
+    // Spawn a process to handle stderr.
+    mesos::internal::logger::rotate::Flags errFlags;
+    errFlags.max_size = flags.max_stderr_size;
+    errFlags.logrotate_options = flags.logrotate_stderr_options;
+    errFlags.log_filename = path::join(sandboxDirectory, "stderr");
+    errFlags.logrotate_path = flags.logrotate_path;
+
+    Try<Subprocess> errProcess = subprocess(
+        path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
+        {mesos::internal::logger::rotate::NAME},
+        Subprocess::PIPE(),
+        Subprocess::PATH("/dev/null"),
+        Subprocess::FD(STDERR_FILENO),
+        errFlags,
+        environment);
+
+    if (errProcess.isError()) {
+      os::killtree(outProcess.get().pid(), SIGKILL);
+      return Failure("Failed to create logger process: " + errProcess.error());
+    }
+
+    ContainerLogger::SubprocessInfo info;
+    info.out = SubprocessInfo::IO::FD(outProcess->in().get());
+    info.err = SubprocessInfo::IO::FD(errProcess->in().get());
+    return info;
+  }
+
+protected:
+  Flags flags;
+};
+
+
+LogrotateContainerLogger::LogrotateContainerLogger(const Flags& _flags)
+  : flags(_flags),
+    process(new LogrotateContainerLoggerProcess(flags))
+{
+  // Spawn and pass validated parameters to the process.
+  spawn(process.get());
+}
+
+
+LogrotateContainerLogger::~LogrotateContainerLogger()
+{
+  terminate(process.get());
+  wait(process.get());
+}
+
+
+Try<Nothing> LogrotateContainerLogger::initialize()
+{
+  return Nothing();
+}
+
+Future<Nothing> LogrotateContainerLogger::recover(
+    const ExecutorInfo& executorInfo,
+    const std::string& sandboxDirectory)
+{
+  return dispatch(
+      process.get(),
+      &LogrotateContainerLoggerProcess::recover,
+      executorInfo,
+      sandboxDirectory);
+}
+
+Future<SubprocessInfo> LogrotateContainerLogger::prepare(
+    const ExecutorInfo& executorInfo,
+    const std::string& sandboxDirectory)
+{
+  return dispatch(
+      process.get(),
+      &LogrotateContainerLoggerProcess::prepare,
+      executorInfo,
+      sandboxDirectory);
+}
+
+} // namespace logger {
+} // namespace internal {
+} // namespace mesos {
+
+
+mesos::modules::Module<ContainerLogger>
+org_apache_mesos_LogrotateContainerLogger(
+    MESOS_MODULE_API_VERSION,
+    MESOS_VERSION,
+    "Apache Mesos",
+    "modules@mesos.apache.org",
+    "Logrotate Container Logger module.",
+    NULL,
+    [](const Parameters& parameters) -> ContainerLogger* {
+      // Convert `parameters` into a map.
+      std::map<std::string, std::string> values;
+      foreach (const Parameter& parameter, parameters.parameter()) {
+        values[parameter.key()] = parameter.value();
+      }
+
+      // Load and validate flags from the map.
+      mesos::internal::logger::Flags flags;
+      Try<Nothing> load = flags.load(values);
+
+      if (load.isError()) {
+        LOG(ERROR) << "Failed to parse parameters: " << load.error();
+        return NULL;
+      }
+
+      return new mesos::internal::logger::LogrotateContainerLogger(flags);
+    });

http://git-wip-us.apache.org/repos/asf/mesos/blob/9fad619a/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
new file mode 100644
index 0000000..5f79fd8
--- /dev/null
+++ b/src/slave/container_loggers/lib_logrotate.hpp
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __SLAVE_CONTAINER_LOGGER_LIB_LOGROTATE_HPP__
+#define __SLAVE_CONTAINER_LOGGER_LIB_LOGROTATE_HPP__
+
+#include <stdio.h>
+
+#include <mesos/slave/container_logger.hpp>
+
+#include <stout/bytes.hpp>
+#include <stout/flags.hpp>
+#include <stout/option.hpp>
+
+#include <stout/os/exists.hpp>
+#include <stout/os/shell.hpp>
+
+#include "slave/container_loggers/logrotate.hpp"
+
+namespace mesos {
+namespace internal {
+namespace logger {
+
+// Forward declaration.
+class LogrotateContainerLoggerProcess;
+
+
+struct Flags : public virtual flags::FlagsBase
+{
+  Flags()
+  {
+    add(&max_stdout_size,
+        "max_stdout_size",
+        "Maximum size, in bytes, of a single stdout log file.\n"
+        "Defaults to 10 MB.  Must be at least 1 (memory) page.",
+        Megabytes(10),
+        &Flags::validateSize);
+
+    add(&logrotate_stdout_options,
+        "logrotate_stdout_options",
+        "Additional config options to pass into 'logrotate' for stdout.\n"
+        "This string will be inserted into a 'logrotate' configuration file.\n"
+        "i.e.\n"
+        "  /path/to/stdout {\n"
+        "    <logrotate_stdout_options>\n"
+        "    size <max_stdout_size>\n"
+        "  }\n"
+        "NOTE: The 'size' option will be overriden by this module.");
+
+    add(&max_stderr_size,
+        "max_stderr_size",
+        "Maximum size, in bytes, of a single stderr log file.\n"
+        "Defaults to 10 MB.  Must be at least 1 (memory) page.",
+        Megabytes(10),
+        &Flags::validateSize);
+
+    add(&logrotate_stderr_options,
+        "logrotate_stderr_options",
+        "Additional config options to pass into 'logrotate' for stderr.\n"
+        "This string will be inserted into a 'logrotate' configuration file.\n"
+        "i.e.\n"
+        "  /path/to/stderr {\n"
+        "    <logrotate_stderr_options>\n"
+        "    size <max_stderr_size>\n"
+        "  }\n"
+        "NOTE: The 'size' option will be overriden by this module.");
+
+    add(&launcher_dir,
+        "launcher_dir",
+        "Directory path of Mesos binaries.  The logrotate container logger\n"
+        "will find the '" + mesos::internal::logger::rotate::NAME + "'\n"
+        "binary file under this directory.",
+        PKGLIBEXECDIR,
+        [](const std::string& value) -> Option<Error> {
+          std::string executablePath =
+            path::join(value, mesos::internal::logger::rotate::NAME);
+
+          if (!os::exists(executablePath)) {
+            return Error("Cannot find: " + executablePath);
+          }
+
+          return None();
+        });
+
+    add(&logrotate_path,
+        "logrotate_path",
+        "If specified, the logrotate container logger will use the specified\n"
+        "'logrotate' instead of the system's 'logrotate'.",
+        "logrotate",
+        [](const std::string& value) -> Option<Error> {
+          // Check if `logrotate` exists via the version command.
+          // TODO(josephw): Consider a more comprehensive check.
+          Try<std::string> versionCommand = os::shell(value + " --version");
+
+          if (versionCommand.isError()) {
+            return Error(
+                "Failed to check logrotate version: " + versionCommand.error());
+          }
+
+          return None();
+        });
+  }
+
+  static Option<Error> validateSize(const Bytes& value)
+  {
+    if (value.bytes() < (size_t) sysconf(_SC_PAGE_SIZE)) {
+      return Error(
+          "Expected --max_stdout_size and --max_stderr_size of "
+          "at least " + stringify(sysconf(_SC_PAGE_SIZE)) + " bytes");
+    }
+
+    return None();
+  }
+
+  Bytes max_stdout_size;
+  Option<std::string> logrotate_stdout_options;
+
+  Bytes max_stderr_size;
+  Option<std::string> logrotate_stderr_options;
+
+  std::string launcher_dir;
+  std::string logrotate_path;
+};
+
+
+// The `LogrotateContainerLogger` is a container logger that utilizes the
+// `logrotate` utility to strictly constrain total size of a container's
+// stdout and stderr log files.  All `logrotate` configuration options
+// (besides `size`, which this module uses) are supported.  See `Flags` above.
+class LogrotateContainerLogger : public mesos::slave::ContainerLogger
+{
+public:
+  LogrotateContainerLogger(const Flags& _flags);
+
+  virtual ~LogrotateContainerLogger();
+
+  // This is a noop.  The logrotate container logger has nothing to initialize.
+  virtual Try<Nothing> initialize();
+
+  virtual process::Future<Nothing> recover(
+      const ExecutorInfo& executorInfo,
+      const std::string& sandboxDirectory);
+
+  virtual process::Future<mesos::slave::ContainerLogger::SubprocessInfo>
+  prepare(
+      const ExecutorInfo& executorInfo,
+      const std::string& sandboxDirectory);
+
+protected:
+  Flags flags;
+  process::Owned<LogrotateContainerLoggerProcess> process;
+};
+
+} // namespace logger {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __SLAVE_CONTAINER_LOGGER_LIB_LOGROTATE_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/9fad619a/src/slave/container_loggers/logrotate.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/logrotate.cpp b/src/slave/container_loggers/logrotate.cpp
new file mode 100644
index 0000000..4a9bc0c
--- /dev/null
+++ b/src/slave/container_loggers/logrotate.cpp
@@ -0,0 +1,247 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <unistd.h>
+
+#include <new>
+
+#include <functional>
+#include <string>
+
+#include <process/dispatch.hpp>
+#include <process/io.hpp>
+#include <process/process.hpp>
+
+#include <stout/bytes.hpp>
+#include <stout/error.hpp>
+#include <stout/exit.hpp>
+#include <stout/nothing.hpp>
+#include <stout/path.hpp>
+#include <stout/stringify.hpp>
+#include <stout/try.hpp>
+
+#include <stout/os/shell.hpp>
+#include <stout/os/write.hpp>
+
+#include "slave/container_loggers/logrotate.hpp"
+
+
+using namespace process;
+using namespace mesos::internal::logger::rotate;
+
+
+class LogrotateLoggerProcess : public Process<LogrotateLoggerProcess>
+{
+public:
+  LogrotateLoggerProcess(const Flags& _flags)
+    : flags(_flags),
+      leading(None()),
+      bytesWritten(0)
+  {
+    // Prepare a buffer for reading from the `incoming` pipe.
+    length = sysconf(_SC_PAGE_SIZE);
+    buffer = new char[length];
+  }
+
+  virtual ~LogrotateLoggerProcess()
+  {
+    if (buffer != NULL) {
+      delete[] buffer;
+      buffer = NULL;
+    }
+
+    if (leading.isSome()) {
+      os::close(leading.get());
+    }
+  }
+
+  // Prepares and starts the loop which reads from stdin, writes to the
+  // leading log file, and manages total log size.
+  Future<Nothing> run()
+  {
+    // Populate the `logrotate` configuration file.
+    // See `Flags::logrotate_options` for the format.
+    //
+    // NOTE: We specify a size of `--max_size - length` because `logrotate`
+    // has slightly different size semantics.  `logrotate` will rotate when the
+    // max size is *exceeded*.  We rotate to keep files *under* the max size.
+    const std::string config =
+      "\"" + flags.log_filename.get() + "\" {\n" +
+      flags.logrotate_options.getOrElse("") + "\n" +
+      "size " + stringify(flags.max_size.bytes() - length) + "\n" +
+      "}";
+
+    Try<Nothing> result = os::write(
+        flags.log_filename.get() + CONF_SUFFIX, config);
+
+    if (result.isError()) {
+      return Failure("Failed to write configuration file: " + result.error());
+    }
+
+    // NOTE: This is a prerequisuite for `io::read`.
+    Try<Nothing> nonblock = os::nonblock(STDIN_FILENO);
+    if (nonblock.isError()) {
+      return Failure("Failed to set nonblocking pipe: " + nonblock.error());
+    }
+
+    // NOTE: This does not block.
+    loop();
+
+    return promise.future();
+  }
+
+  // Reads from stdin and writes to the leading log file.
+  void loop() {
+    io::read(STDIN_FILENO, buffer, length)
+      .then([&](size_t readSize) -> Future<Nothing> {
+        // Check if EOF has been reached on the input stream.
+        // This indicates that the container (whose logs are being
+        // piped to this process) has exited.
+        if (readSize <= 0) {
+          promise.set(Nothing());
+          return Nothing();
+        }
+
+        // Do log rotation (if necessary) and write the bytes to the
+        // leading log file.
+        Try<Nothing> result = write(readSize);
+        if (result.isError()) {
+          promise.fail("Failed to write: " + result.error());
+          return Nothing();
+        }
+
+        // Use `dispatch` to limit the size of the call stack.
+        dispatch(self(), &LogrotateLoggerProcess::loop);
+
+        return Nothing();
+      });
+  }
+
+  // Writes the buffer from stdin to the leading log file.
+  // When the number of written bytes exceeds `--max_size`, the leading
+  // log file is rotated.  When the number of log files exceed `--max_files`,
+  // the oldest log file is deleted.
+  Try<Nothing> write(size_t readSize)
+  {
+    // Rotate the log file if it will grow beyond the `--max_size`.
+    if (bytesWritten + readSize > flags.max_size.bytes()) {
+      rotate();
+    }
+
+    // If the leading log file is not open, open it.
+    // NOTE: We open the file in append-mode as `logrotate` may sometimes fail.
+    if (leading.isNone()) {
+      Try<int> open = os::open(
+          flags.log_filename.get(),
+          O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC,
+          S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+      if (open.isError()) {
+        return Error(
+            "Failed to open '" + flags.log_filename.get() +
+            "': " + open.error());
+      }
+
+      leading = open.get();
+    }
+
+    // Write from stdin to `leading`.
+    // NOTE: We do not exit on error here since we are prioritizing
+    // clearing the STDIN pipe (which would otherwise potentially block
+    // the container on write) over log fidelity.
+    Try<Nothing> result =
+      os::write(leading.get(), std::string(buffer, readSize));
+
+    if (result.isError()) {
+      std::cerr << "Failed to write: " << result.error() << std::endl;
+    }
+
+    bytesWritten += readSize;
+
+    return Nothing();
+  }
+
+  // Calls `logrotate` on the leading log file and resets the `bytesWritten`.
+  void rotate()
+  {
+    if (leading.isSome()) {
+      os::close(leading.get());
+      leading = None();
+    }
+
+    // Call `logrotate` to move around the files.
+    // NOTE: If `logrotate` fails for whatever reason, we will ignore
+    // the error and continue logging.  In case the leading log file
+    // is not renamed, we will continue appending to the existing
+    // leading log file.
+    os::shell(
+        flags.logrotate_path +
+        " --state \"" + flags.log_filename.get() + STATE_SUFFIX + "\" \"" +
+        flags.log_filename.get() + CONF_SUFFIX + "\"");
+
+    // Reset the number of bytes written.
+    bytesWritten = 0;
+  }
+
+private:
+  Flags flags;
+
+  // For reading from stdin.
+  char* buffer;
+  size_t length;
+
+  // For writing and rotating the leading log file.
+  Option<int> leading;
+  size_t bytesWritten;
+
+  // Used to capture when log rotation has completed because the
+  // underlying process/input has terminated.
+  Promise<Nothing> promise;
+};
+
+
+int main(int argc, char** argv)
+{
+  Flags flags;
+
+  // Load and validate flags from the environment and command line.
+  Try<Nothing> load = flags.load(None(), &argc, &argv);
+
+  if (load.isError()) {
+    EXIT(EXIT_FAILURE) << flags.usage(load.error());
+  }
+
+  // Make sure this process is running in its own session.
+  // This ensures that, if the parent process (presumably the Mesos agent)
+  // terminates, this logger process will continue to run.
+  if (::setsid() == -1) {
+    EXIT(EXIT_FAILURE)
+      << ErrnoError("Failed to put child in a new session").message;
+  }
+
+  // Asynchronously control the flow and size of logs.
+  LogrotateLoggerProcess process(flags);
+  spawn(&process);
+
+  // Wait for the logging process to finish.
+  Future<Nothing> status = dispatch(process, &LogrotateLoggerProcess::run);
+  status.await();
+
+  terminate(process);
+  wait(process);
+
+  return status.isReady() ? EXIT_SUCCESS : EXIT_FAILURE;
+}

http://git-wip-us.apache.org/repos/asf/mesos/blob/9fad619a/src/slave/container_loggers/logrotate.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/logrotate.hpp b/src/slave/container_loggers/logrotate.hpp
new file mode 100644
index 0000000..fd9c071
--- /dev/null
+++ b/src/slave/container_loggers/logrotate.hpp
@@ -0,0 +1,125 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __SLAVE_CONTAINER_LOGGER_LOGROTATE_HPP__
+#define __SLAVE_CONTAINER_LOGGER_LOGROTATE_HPP__
+
+#include <stdio.h>
+#include <unistd.h>
+
+#include <stout/bytes.hpp>
+#include <stout/error.hpp>
+#include <stout/flags.hpp>
+#include <stout/option.hpp>
+#include <stout/path.hpp>
+
+
+namespace mesos {
+namespace internal {
+namespace logger {
+namespace rotate {
+
+const std::string NAME = "mesos-logrotate-logger";
+const std::string CONF_SUFFIX = ".logrotate.conf";
+const std::string STATE_SUFFIX = ".logrotate.state";
+
+struct Flags : public virtual flags::FlagsBase
+{
+  Flags()
+  {
+    setUsageMessage(
+      "Usage: " + NAME + " [options]\n"
+      "\n"
+      "This command pipes from STDIN to the given leading log file.\n"
+      "When the leading log file reaches '--max_size', the command.\n"
+      "uses 'logrotate' to rotate the logs.  All 'logrotate' options\n"
+      "are supported.  See '--logrotate_options'.\n"
+      "\n");
+
+    add(&max_size,
+        "max_size",
+        "Maximum size, in bytes, of a single log file.\n"
+        "Defaults to 10 MB.  Must be at least 1 (memory) page.",
+        Megabytes(10),
+        [](const Bytes& value) -> Option<Error> {
+          if (value.bytes() < (size_t) sysconf(_SC_PAGE_SIZE)) {
+            return Error(
+                "Expected --max_size of at least " +
+                stringify(sysconf(_SC_PAGE_SIZE)) + " bytes");
+          }
+          return None();
+        });
+
+    add(&logrotate_options,
+        "logrotate_options",
+        "Additional config options to pass into 'logrotate'.\n"
+        "This string will be inserted into a 'logrotate' configuration file.\n"
+        "i.e.\n"
+        "  /path/to/<log_filename> {\n"
+        "    <logrotate_options>\n"
+        "    size <max_size>\n"
+        "  }\n"
+        "NOTE: The 'size' option will be overriden by this command.");
+
+    add(&log_filename,
+        "log_filename",
+        "Absolute path to the leading log file.\n"
+        "NOTE: This command will also create two files by appending\n"
+        "'" + CONF_SUFFIX + "' and '" + STATE_SUFFIX + "' to the end of\n"
+        "'--log_filename'.  These files are used by 'logrotate'.",
+        [](const Option<std::string>& value) -> Option<Error> {
+          if (value.isNone()) {
+            return Error("Missing required option --log_filename");
+          }
+
+          if (!path::absolute(value.get())) {
+            return Error("Expected --log_filename to be an absolute path");
+          }
+
+          return None();
+        });
+
+    add(&logrotate_path,
+        "logrotate_path",
+        "If specified, this command will use the specified\n"
+        "'logrotate' instead of the system's 'logrotate'.",
+        "logrotate",
+        [](const std::string& value) -> Option<Error> {
+          // Check if `logrotate` exists via the version command.
+          // TODO(josephw): Consider a more comprehensive check.
+          Try<std::string> versionCommand = os::shell(value + " --version");
+
+          if (versionCommand.isError()) {
+            return Error(
+                "Failed to check logrotate version: " + versionCommand.error());
+          }
+
+          return None();
+        });
+  }
+
+  Bytes max_size;
+  Option<std::string> logrotate_options;
+  Option<std::string> log_filename;
+  std::string logrotate_path;
+};
+
+} // namespace rotate {
+} // namespace logger {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __SLAVE_CONTAINER_LOGGER_LOGROTATE_HPP__


[06/11] mesos git commit: Refactored Sandbox logger initialization.

Posted by be...@apache.org.
Refactored Sandbox logger initialization.

In retrospect, the `if (process != NULL)` initialization check in each
sandbox logger function is unnecessary for such a simple module.

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


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

Branch: refs/heads/master
Commit: 0bf8d2411e41d14de0539a4ff1c248850972b597
Parents: 2fb5a91
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:45:53 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

----------------------------------------------------------------------
 src/slave/container_loggers/sandbox.cpp | 28 +++++++++-------------------
 src/slave/container_loggers/sandbox.hpp |  2 ++
 2 files changed, 11 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0bf8d241/src/slave/container_loggers/sandbox.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.cpp b/src/slave/container_loggers/sandbox.cpp
index 1b954d6..00272f8 100644
--- a/src/slave/container_loggers/sandbox.cpp
+++ b/src/slave/container_loggers/sandbox.cpp
@@ -72,24 +72,22 @@ public:
 };
 
 
-SandboxContainerLogger::~SandboxContainerLogger()
+SandboxContainerLogger::SandboxContainerLogger()
+  : process(new SandboxContainerLoggerProcess())
 {
-  if (process.get() != NULL) {
-    terminate(process.get());
-    wait(process.get());
-  }
+  spawn(process.get());
 }
 
 
-Try<Nothing> SandboxContainerLogger::initialize()
+SandboxContainerLogger::~SandboxContainerLogger()
 {
-  if (process.get() != NULL) {
-    return Error("Sandbox container logger has already been initialized");
-  }
+  terminate(process.get());
+  wait(process.get());
+}
 
-  process.reset(new SandboxContainerLoggerProcess());
-  spawn(process.get());
 
+Try<Nothing> SandboxContainerLogger::initialize()
+{
   return Nothing();
 }
 
@@ -98,10 +96,6 @@ Future<Nothing> SandboxContainerLogger::recover(
     const ExecutorInfo& executorInfo,
     const std::string& sandboxDirectory)
 {
-  if (process.get() == NULL) {
-    return Failure("Sandbox container logger is not initialized");
-  }
-
   return dispatch(
       process.get(),
       &SandboxContainerLoggerProcess::recover,
@@ -115,10 +109,6 @@ SandboxContainerLogger::prepare(
     const ExecutorInfo& executorInfo,
     const std::string& sandboxDirectory)
 {
-  if (process.get() == NULL) {
-    return Failure("Sandbox container logger is not initialized");
-  }
-
   return dispatch(
       process.get(),
       &SandboxContainerLoggerProcess::prepare,

http://git-wip-us.apache.org/repos/asf/mesos/blob/0bf8d241/src/slave/container_loggers/sandbox.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.hpp b/src/slave/container_loggers/sandbox.hpp
index 17bb1d9..8b1f8ab 100644
--- a/src/slave/container_loggers/sandbox.hpp
+++ b/src/slave/container_loggers/sandbox.hpp
@@ -49,8 +49,10 @@ class SandboxContainerLoggerProcess;
 class SandboxContainerLogger : public mesos::slave::ContainerLogger
 {
 public:
+  SandboxContainerLogger();
   virtual ~SandboxContainerLogger();
 
+  // This is a noop.  The sandbox container logger has nothing to initialize.
   virtual Try<Nothing> initialize();
 
   // This is a noop.  The agent recovery process already exposes all files


[02/11] mesos git commit: Add test for the rotating container logger module.

Posted by be...@apache.org.
Add test for the rotating container logger module.

This test loads a non-default ContainerLogger module that rotates logs
(i.e. renaming the head log file) and constrains total log size.

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


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

Branch: refs/heads/master
Commit: 0f20d5e73ebe7eacc154867c53aea4b16f548abf
Parents: b97d2ab
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:48:43 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

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


http://git-wip-us.apache.org/repos/asf/mesos/blob/0f20d5e7/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp
index c6b2e59..0b1cbd8 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -22,12 +22,17 @@
 #include <process/future.hpp>
 #include <process/gtest.hpp>
 
+#include <stout/bytes.hpp>
 #include <stout/gtest.hpp>
 #include <stout/os.hpp>
 #include <stout/path.hpp>
 #include <stout/strings.hpp>
 #include <stout/try.hpp>
 
+#include <stout/os/exists.hpp>
+#include <stout/os/pstree.hpp>
+#include <stout/os/stat.hpp>
+
 #include "master/master.hpp"
 
 #include "slave/flags.hpp"
@@ -61,6 +66,10 @@ namespace mesos {
 namespace internal {
 namespace tests {
 
+const char LOGROTATE_CONTAINER_LOGGER_NAME[] =
+  "org_apache_mesos_LogrotateContainerLogger";
+
+
 class ContainerLoggerTest : public MesosTest {};
 
 
@@ -148,6 +157,136 @@ TEST_F(ContainerLoggerTest, DefaultToSandbox)
   Shutdown();
 }
 
+
+// Tests that the packaged logrotate container logger writes files into the
+// sandbox and keeps them at a reasonable size.
+TEST_F(ContainerLoggerTest, LOGROTATE_RotateInSandbox)
+{
+  // Create a master, agent, and framework.
+  Try<PID<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;
+
+  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);
+
+  Try<PID<Slave>> slave = StartSlave(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(), 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 11 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 five files with a total size of 9 MB.  The first 2 MB file
+  // should have been deleted.  The "stdout" file should be 1 MB large.
+  TaskInfo task = createTask(
+      offers.get()[0],
+      "i=0; while [ $i -lt 11264 ]; "
+      "do printf '%-1024d\\n' $i; i=$((i+1)); done");
+
+  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();
+
+  Shutdown();
+
+  // 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) {
+    ASSERT_EQ(pstree.process.pid, waitpid(pstree.process.pid, NULL, 0));
+  }
+
+  // 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 about half full (1 MB).
+  string stdoutPath = path::join(sandboxDirectory, "stdout");
+  ASSERT_TRUE(os::exists(stdoutPath));
+
+  // 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(1024, stdoutSize->kilobytes());
+  EXPECT_GE(1050, stdoutSize->kilobytes());
+
+  // We should only have files up to "stdout.4".
+  stdoutPath = path::join(sandboxDirectory, "stdout.5");
+  EXPECT_FALSE(os::exists(stdoutPath));
+
+  // The next four rotated log files (2 MB each) should be present.
+  for (int i = 1; i < 5; i++) {
+    stdoutPath = path::join(sandboxDirectory, "stdout." + stringify(i));
+    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);
+    EXPECT_LE(2040, stdoutSize->kilobytes());
+    EXPECT_GE(2048, stdoutSize->kilobytes());
+  }
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {


[04/11] mesos git commit: Extended Subprocess::FD to optionally not duplicate the given FD.

Posted by be...@apache.org.
Extended Subprocess::FD to optionally not duplicate the given FD.

This new option is useful when a subprocess is started with a FD, and
the parent process does not need the FD afterwards.  In this case, the
FD does not need to be duplicated before being sent to the child
process.

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


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

Branch: refs/heads/master
Commit: a93a58e49e3fa3566ac995b074d895a655827de2
Parents: 3673887
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:45:24 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

----------------------------------------------------------------------
 .../libprocess/include/process/subprocess.hpp   | 22 +++++++++++++-
 3rdparty/libprocess/src/subprocess.cpp          | 32 +++++++++++++++++---
 2 files changed, 48 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a93a58e4/3rdparty/libprocess/include/process/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index 5b2489e..482ea23 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -89,6 +89,26 @@ public:
       int write = -1;
     };
 
+    /**
+     * Describes the lifecycle of a file descriptor passed into a subprocess
+     * via the `Subprocess::FD` helper.
+     */
+    enum FDType {
+      /**
+       * The file descriptor is duplicated before being passed to the
+       * subprocess.  The original file descriptor remains open.
+       */
+      DUPLICATE,
+
+      /**
+       * The file descriptor is not duplicated before being passed to the
+       * subprocess.  Upon spawning the subprocess, the original file
+       * descriptor is closed in the parent and remains open in the child.
+       */
+      OWNED
+    };
+
+
   private:
     friend class Subprocess;
 
@@ -123,7 +143,7 @@ public:
   // Some syntactic sugar to create an IO::PIPE redirector.
   static IO PIPE();
   static IO PATH(const std::string& path);
-  static IO FD(int fd);
+  static IO FD(int fd, IO::FDType type = IO::DUPLICATE);
 
   /**
    * @return The operating system PID for this subprocess.

http://git-wip-us.apache.org/repos/asf/mesos/blob/a93a58e4/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index 63cdcc4..ef05687 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -169,11 +169,24 @@ Subprocess::IO Subprocess::PATH(const string& path)
 }
 
 
-Subprocess::IO Subprocess::FD(int fd)
+Subprocess::IO Subprocess::FD(int fd, IO::FDType type)
 {
   return Subprocess::IO(
-      [fd]() -> Try<InputFileDescriptors> {
-        int prepared_fd = ::dup(fd);
+      [fd, type]() -> Try<InputFileDescriptors> {
+        int prepared_fd = -1;
+        switch (type) {
+          case IO::DUPLICATE:
+            prepared_fd = ::dup(fd);
+            break;
+          case IO::OWNED:
+            prepared_fd = fd;
+            break;
+
+          // NOTE: By not setting a default we leverage the compiler
+          // errors when the enumeration is augmented to find all
+          // the cases we need to provide.  Same for below.
+        }
+
         if (prepared_fd == -1) {
           return ErrnoError("Failed to dup");
         }
@@ -182,8 +195,17 @@ Subprocess::IO Subprocess::FD(int fd)
         fds.read = prepared_fd;
         return fds;
       },
-      [fd]() -> Try<OutputFileDescriptors> {
-        int prepared_fd = ::dup(fd);
+      [fd, type]() -> Try<OutputFileDescriptors> {
+        int prepared_fd = -1;
+        switch (type) {
+          case IO::DUPLICATE:
+            prepared_fd = ::dup(fd);
+            break;
+          case IO::OWNED:
+            prepared_fd = fd;
+            break;
+        }
+
         if (prepared_fd == -1) {
           return ErrnoError("Failed to dup");
         }


[03/11] mesos git commit: Add test filter for tests requiring `logrotate`.

Posted by be...@apache.org.
Add test filter for tests requiring `logrotate`.

The `logrotate` binary is used by the non-default
`RotatingContainerLogger` module.  On some systems, `logrotate` is not
provided by default.

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


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

Branch: refs/heads/master
Commit: b97d2aba81b1179e2c45d6825f678e54c29b5f2f
Parents: 0bf8d24
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:47:45 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

----------------------------------------------------------------------
 src/tests/environment.cpp | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b97d2aba/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 4de46bc..e112270 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -312,6 +312,32 @@ private:
 };
 
 
+class LogrotateFilter : public TestFilter
+{
+public:
+  LogrotateFilter()
+  {
+    logrotateError = os::system("which logrotate") != 0;
+    if (logrotateError) {
+      std::cerr
+        << "-------------------------------------------------------------\n"
+        << "No 'logrotate' command found so no 'logrotate' tests\n"
+        << "will be run\n"
+        << "-------------------------------------------------------------"
+        << std::endl;
+    }
+  }
+
+  bool disable(const ::testing::TestInfo* test) const
+  {
+    return matches(test, "LOGROTATE_") && logrotateError;
+  }
+
+private:
+  bool logrotateError;
+};
+
+
 class NetcatFilter : public TestFilter
 {
 public:
@@ -610,6 +636,7 @@ Environment::Environment(const Flags& _flags) : flags(_flags)
   filters.push_back(Owned<TestFilter>(new CurlFilter()));
   filters.push_back(Owned<TestFilter>(new DockerFilter()));
   filters.push_back(Owned<TestFilter>(new InternetFilter()));
+  filters.push_back(Owned<TestFilter>(new LogrotateFilter()));
   filters.push_back(Owned<TestFilter>(new NetcatFilter()));
   filters.push_back(Owned<TestFilter>(new NetClsCgroupsFilter()));
   filters.push_back(Owned<TestFilter>(new NetworkIsolatorTestFilter()));


[05/11] mesos git commit: Wire up the rotating container logger module and test.

Posted by be...@apache.org.
Wire up the rotating container logger module and test.

Creates a new binary "mesos-rotate-logger" for use by the non-default
"Rotating" ContainerLogger module.

Adds the RotatingContainerLogger to the test module configuration.

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


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

Branch: refs/heads/master
Commit: a355f43843204db21156f239de44f3592270f3bb
Parents: 0f20d5e
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:48:58 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

----------------------------------------------------------------------
 src/Makefile.am      | 16 ++++++++++++++++
 src/tests/module.cpp | 40 +++++++++++++++++++++++++++++++++++++---
 src/tests/module.hpp |  1 +
 3 files changed, 54 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a355f438/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 19bf3a7..8657a86 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1101,6 +1101,13 @@ mesos_containerizer_SOURCES = slave/containerizer/mesos/main.cpp
 mesos_containerizer_CPPFLAGS = $(MESOS_CPPFLAGS)
 mesos_containerizer_LDADD = libmesos.la $(LDADD)
 
+pkglibexec_PROGRAMS += mesos-logrotate-logger
+mesos_logrotate_logger_SOURCES =		\
+  slave/container_loggers/logrotate.hpp		\
+  slave/container_loggers/logrotate.cpp
+mesos_logrotate_logger_CPPFLAGS = $(MESOS_CPPFLAGS)
+mesos_logrotate_logger_LDADD = libmesos.la $(LDADD)
+
 if WITH_NETWORK_ISOLATOR
 pkglibexec_PROGRAMS += mesos-network-helper
 mesos_network_helper_SOURCES = slave/containerizer/mesos/isolators/network/helper.cpp
@@ -1641,6 +1648,15 @@ check_PROGRAMS += mesos-tests
 # LDFLAGS to be used for the module libraries.
 MESOS_MODULE_LDFLAGS = -release $(PACKAGE_VERSION) -shared
 
+# Library containing the logrotate container logger.
+lib_LTLIBRARIES += liblogrotate_container_logger.la
+liblogrotate_container_logger_la_SOURCES =			\
+  slave/container_loggers/logrotate.hpp				\
+  slave/container_loggers/lib_logrotate.hpp			\
+  slave/container_loggers/lib_logrotate.cpp
+liblogrotate_container_logger_la_CPPFLAGS = $(MESOS_CPPFLAGS)
+liblogrotate_container_logger_la_LDFLAGS = $(MESOS_MODULE_LDFLAGS)
+
 # Library containing the fixed resource estimator.
 lib_LTLIBRARIES += libfixed_resource_estimator.la
 libfixed_resource_estimator_la_SOURCES = slave/resource_estimators/fixed.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/a355f438/src/tests/module.cpp
----------------------------------------------------------------------
diff --git a/src/tests/module.cpp b/src/tests/module.cpp
index f0e64f7..246f3a4 100644
--- a/src/tests/module.cpp
+++ b/src/tests/module.cpp
@@ -16,9 +16,11 @@
 
 #include <string>
 
+#include <stout/bytes.hpp>
 #include <stout/hashmap.hpp>
 #include <stout/os.hpp>
 #include <stout/path.hpp>
+#include <stout/stringify.hpp>
 
 #include "messages/messages.hpp"
 #include "module/manager.hpp"
@@ -101,21 +103,53 @@ static void addContainerLoggerModules(Modules* modules)
 {
   CHECK_NOTNULL(modules);
 
-  const string libraryPath = path::join(
+  const string libraryDirectory = path::join(
       tests::flags.build_dir,
       "src",
-      ".libs",
+      ".libs");
+
+  const string sandboxLoggerPath = path::join(
+      libraryDirectory,
       os::libraries::expandName("testcontainer_logger"));
 
   // Add our test container logger module.
   Modules::Library* library = modules->add_libraries();
-  library->set_file(libraryPath);
+  library->set_file(sandboxLoggerPath);
 
   // To add a new module from this library, create a new ModuleID enum
   // and tie it with a module name.
   addModule(library,
             TestSandboxContainerLogger,
             "org_apache_mesos_TestSandboxContainerLogger");
+
+  const string logrotateLoggerPath = path::join(
+      libraryDirectory,
+      os::libraries::expandName("logrotate_container_logger"));
+
+  // Add the second container logger module.
+  library = modules->add_libraries();
+  library->set_file(logrotateLoggerPath);
+
+  addModule(library,
+            LogrotateContainerLogger,
+            "org_apache_mesos_LogrotateContainerLogger");
+
+  // Pass in the directory for the binary test sources.
+  Modules::Library::Module* module = library->mutable_modules(0);
+  mesos::Parameter* moduleParameter = module->add_parameters();
+  moduleParameter->set_key("launcher_dir");
+  moduleParameter->set_value(path::join(tests::flags.build_dir, "src"));
+
+  // Set the size and number of log files to keep.
+  moduleParameter = module->add_parameters();
+  moduleParameter->set_key("max_stdout_size");
+  moduleParameter->set_value(stringify(Megabytes(2)));
+
+  // NOTE: This is a 'logrotate' configuration option.
+  // It means to "rotate" a file 4 times before removal.
+  moduleParameter = module->add_parameters();
+  moduleParameter->set_key("logrotate_stdout_options");
+  moduleParameter->set_value("rotate 4");
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/a355f438/src/tests/module.hpp
----------------------------------------------------------------------
diff --git a/src/tests/module.hpp b/src/tests/module.hpp
index 49a9203..4b32f29 100644
--- a/src/tests/module.hpp
+++ b/src/tests/module.hpp
@@ -49,6 +49,7 @@ enum ModuleID
   TestNoopResourceEstimator,
   TestLocalAuthorizer,
   TestSandboxContainerLogger,
+  LogrotateContainerLogger,
   TestHttpBasicAuthenticator
 };
 


[07/11] mesos git commit: Changed ContainerLogger and Fetcher to not duplicate FDs.

Posted by be...@apache.org.
Changed ContainerLogger and Fetcher to not duplicate FDs.

This change shifts the burden of FD lifecycle management away from
ContainerLogger module writers.

This change also cleans up the Fetcher code slightly.

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


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

Branch: refs/heads/master
Commit: 2fb5a91765cdc91239f43904ca013e62e14ed897
Parents: a93a58e
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Wed Jan 20 16:45:40 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:22 2016 -0800

----------------------------------------------------------------------
 include/mesos/slave/container_logger.hpp | 6 +++++-
 src/slave/containerizer/fetcher.cpp      | 9 ++-------
 2 files changed, 7 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2fb5a917/include/mesos/slave/container_logger.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/slave/container_logger.hpp b/include/mesos/slave/container_logger.hpp
index a236207..9623b0c 100644
--- a/include/mesos/slave/container_logger.hpp
+++ b/include/mesos/slave/container_logger.hpp
@@ -78,7 +78,11 @@ public:
 
       static IO FD(int fd)
       {
-        return IO(process::Subprocess::FD(fd));
+        // NOTE: The FD is not duplicated and will be closed (as seen by the
+        // agent process) when the container is spawned.  This shifts the
+        // burden of FD-lifecycle management into the Containerizer.
+        return IO(process::Subprocess::FD(
+            fd, process::Subprocess::IO::OWNED));
       }
 
       operator process::Subprocess::IO () const { return io; }

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fb5a917/src/slave/containerizer/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.cpp b/src/slave/containerizer/fetcher.cpp
index 4ac9149..f7e3f7d 100644
--- a/src/slave/containerizer/fetcher.cpp
+++ b/src/slave/containerizer/fetcher.cpp
@@ -763,13 +763,11 @@ Future<Nothing> FetcherProcess::run(
   Try<Subprocess> fetcherSubprocess = subprocess(
       command,
       Subprocess::PIPE(),
-      Subprocess::FD(out.get()),
-      Subprocess::FD(err.get()),
+      Subprocess::FD(out.get(), Subprocess::IO::OWNED),
+      Subprocess::FD(err.get(), Subprocess::IO::OWNED),
       environment);
 
   if (fetcherSubprocess.isError()) {
-    os::close(out.get());
-    os::close(err.get());
     return Failure("Failed to execute mesos-fetcher: " +
                    fetcherSubprocess.error());
   }
@@ -811,9 +809,6 @@ Future<Nothing> FetcherProcess::run(
     .onAny(defer(self(), [=](const Future<Nothing>&) {
       // Clear the subprocess PID remembered from running mesos-fetcher.
       subprocessPids.erase(containerId);
-
-      os::close(out.get());
-      os::close(err.get());
     }));
 }
 


[09/11] mesos git commit: Add tests for module recovery after agent failover.

Posted by be...@apache.org.
Add tests for module recovery after agent failover.

Adds two heavily-mocked tests for the Mesos containerizer and Docker
containerizer. Each checks that `ContainerLogger::recover` is called
during `Containerizer::recover`.

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


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

Branch: refs/heads/master
Commit: 9ab260a618a4f5985429560cdac49a3bd0d70fa2
Parents: 9fad619
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Sun Jan 24 20:12:27 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:23 2016 -0800

----------------------------------------------------------------------
 src/tests/container_logger_tests.cpp | 197 ++++++++++++++++++++++++++++++
 src/tests/mesos.cpp                  |  20 +++
 src/tests/mesos.hpp                  |  33 +++++
 3 files changed, 250 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9ab260a6/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp
index 0b1cbd8..ce551cb 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -14,6 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <list>
 #include <string>
 #include <vector>
 
@@ -30,6 +31,7 @@
 #include <stout/try.hpp>
 
 #include <stout/os/exists.hpp>
+#include <stout/os/mkdir.hpp>
 #include <stout/os/pstree.hpp>
 #include <stout/os/stat.hpp>
 
@@ -39,22 +41,39 @@
 #include "slave/paths.hpp"
 #include "slave/slave.hpp"
 
+#include "slave/containerizer/docker.hpp"
 #include "slave/containerizer/fetcher.hpp"
 
 #include "slave/containerizer/mesos/containerizer.hpp"
 
+#include "slave/containerizer/mesos/provisioner/provisioner.hpp"
+
 #include "tests/flags.hpp"
 #include "tests/mesos.hpp"
 #include "tests/utils.hpp"
 
+#include "tests/containerizer/launcher.hpp"
+
 using namespace process;
 
 using mesos::internal::master::Master;
 
 using mesos::internal::slave::Fetcher;
+using mesos::internal::slave::Launcher;
 using mesos::internal::slave::MesosContainerizer;
+using mesos::internal::slave::PosixLauncher;
+using mesos::internal::slave::Provisioner;
 using mesos::internal::slave::Slave;
 
+using mesos::internal::slave::state::ExecutorState;
+using mesos::internal::slave::state::FrameworkState;
+using mesos::internal::slave::state::RunState;
+using mesos::internal::slave::state::SlaveState;
+
+using mesos::slave::ContainerLogger;
+using mesos::slave::Isolator;
+
+using std::list;
 using std::string;
 using std::vector;
 
@@ -73,6 +92,184 @@ const char LOGROTATE_CONTAINER_LOGGER_NAME[] =
 class ContainerLoggerTest : public MesosTest {};
 
 
+// Tests that the Mesos Containerizer will pass recovered containers
+// to the container logger for its own bookkeeping.
+TEST_F(ContainerLoggerTest, MesosContainerizerRecover)
+{
+  // Prepare a MesosContainerizer with a mocked container logger.
+  slave::Flags flags = CreateSlaveFlags();
+
+  Try<Launcher*> launcher = PosixLauncher::create(flags);
+  ASSERT_SOME(launcher);
+
+  Fetcher fetcher;
+
+  MockContainerLogger* logger = new MockContainerLogger();
+
+  Try<Owned<Provisioner>> provisioner = Provisioner::create(flags);
+  ASSERT_SOME(provisioner);
+
+  // Launch a quick task so that we have a valid PID to put in our
+  // mock `SlaveState`.  This is necessary as the containerizer will
+  // try to reap the PID.
+  Try<Subprocess> s = subprocess("exit 0");
+  ASSERT_SOME(s);
+
+  // Construct a mock `SlaveState`.
+  ExecutorID executorId;
+  executorId.set_value(UUID::random().toString());
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  ExecutorInfo executorInfo;
+  executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
+
+  ExecutorState executorState;
+  executorState.id = executorId;
+  executorState.info = executorInfo;
+  executorState.latest = containerId;
+
+  RunState runState;
+  runState.id = containerId;
+  runState.forkedPid = s->pid();
+  executorState.runs.put(containerId, runState);
+
+  FrameworkState frameworkState;
+  frameworkState.executors.put(executorId, executorState);
+
+  SlaveState slaveState;
+  FrameworkID frameworkId;
+  frameworkId.set_value(UUID::random().toString());
+  slaveState.frameworks.put(frameworkId, frameworkState);
+
+  const string sandboxDirectory = slave::paths::getExecutorRunPath(
+      flags.work_dir,
+      slaveState.id,
+      frameworkState.id,
+      executorId,
+      containerId);
+
+  // This is the crux of the test.  The logger's `recover` method
+  // should be called with this specific set of arguments when
+  // we call `Containerizer::recover`.
+  EXPECT_CALL(*logger, recover(executorInfo, sandboxDirectory))
+    .WillOnce(Return(Nothing()));
+
+  MesosContainerizer containerizer(
+      flags,
+      true,
+      &fetcher,
+      Owned<ContainerLogger>(logger),
+      Owned<Launcher>(launcher.get()),
+      provisioner.get(),
+      vector<Owned<Isolator>>());
+
+  // Create the container's sandbox to get past a `CHECK` inside
+  // the MesosContainerizer's recovery validation logic.
+  ASSERT_SOME(os::mkdir(sandboxDirectory));
+
+  Future<Nothing> recover = containerizer.recover(slaveState);
+  AWAIT_READY(recover);
+}
+
+
+// Tests that the Docker Containerizer will pass recovered containers
+// to the container logger for its own bookkeeping.
+TEST_F(ContainerLoggerTest, ROOT_DOCKER_ContainerizerRecover)
+{
+  // Prepare a MockDockerContainerizer with a mocked container logger.
+  MockDocker* mockDocker =
+    new MockDocker(tests::flags.docker, tests::flags.docker_socket);
+
+  Shared<Docker> docker(mockDocker);
+
+  slave::Flags flags = CreateSlaveFlags();
+
+  Fetcher fetcher;
+
+  MockContainerLogger* logger = new MockContainerLogger();
+
+  // Launch a quick task so that we have a valid PID to put in our
+  // mock `SlaveState`.  This is necessary as the containerizer will
+  // try to reap the PID.
+  Try<Subprocess> s = subprocess("exit 0");
+  ASSERT_SOME(s);
+
+  // Construct a mock `SlaveState`.
+  ExecutorID executorId;
+  executorId.set_value(UUID::random().toString());
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  ExecutorInfo executorInfo;
+  executorInfo.mutable_container()->set_type(ContainerInfo::DOCKER);
+
+  ExecutorState executorState;
+  executorState.id = executorId;
+  executorState.info = executorInfo;
+  executorState.latest = containerId;
+
+  RunState runState;
+  runState.id = containerId;
+  runState.forkedPid = s->pid();
+  executorState.runs.put(containerId, runState);
+
+  FrameworkState frameworkState;
+  frameworkState.executors.put(executorId, executorState);
+
+  SlaveState slaveState;
+  FrameworkID frameworkId;
+  frameworkId.set_value(UUID::random().toString());
+  slaveState.frameworks.put(frameworkId, frameworkState);
+
+  const string sandboxDirectory = slave::paths::getExecutorRunPath(
+      flags.work_dir,
+      slaveState.id,
+      frameworkState.id,
+      executorId,
+      containerId);
+
+  // This is the crux of the test.  The logger's `recover` method
+  // should be called with this specific set of arguments when
+  // we call `Containerizer::recover`.
+  EXPECT_CALL(*logger, recover(executorInfo, sandboxDirectory))
+    .WillOnce(Return(Nothing()));
+
+  MockDockerContainerizer containerizer(
+      flags,
+      &fetcher,
+      Owned<ContainerLogger>(logger),
+      docker);
+
+  // Construct a mock response for `Docker::ps` that only has a meaningful
+  // ID field set.  The other fields are effectively ignored.
+  list<Docker::Container> containers;
+  Try<Docker::Container> container = Docker::Container::create(
+      "[{"
+      "  \"Id\": \"" + stringify(containerId) + "\","
+      "  \"Name\": \"mocked\","
+      "  \"State\": {"
+      "    \"Pid\": 0,"
+      "    \"StartedAt\": \"Totally not a time\""
+      "  },"
+      "  \"NetworkSettings\": { \"IPAddress\": \"Totally not an IP\" }"
+      "}]");
+
+  ASSERT_SOME(container);
+  containers.push_back(container.get());
+
+  // Intercept the `Docker::ps` call made inside `DockerContainerizer::Recover`.
+  // We will return a response, pretending that the test container exists.
+  EXPECT_CALL(*mockDocker, ps(_, _))
+    .WillOnce(Return(containers));
+
+  Future<Nothing> recover = containerizer.recover(slaveState);
+  AWAIT_READY(recover);
+}
+
+
 // Tests that the default container logger writes files into the sandbox.
 TEST_F(ContainerLoggerTest, DefaultToSandbox)
 {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9ab260a6/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 365ebe8..18d0d8f 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -625,11 +625,31 @@ MockFetcherProcess::MockFetcherProcess()
 MockFetcherProcess::~MockFetcherProcess() {}
 
 
+MockContainerLogger::MockContainerLogger()
+{
+  // Set up default behaviors.
+  EXPECT_CALL(*this, initialize())
+    .WillRepeatedly(Return(Nothing()));
+
+  EXPECT_CALL(*this, recover(_, _))
+    .WillRepeatedly(Return(Nothing()));
+
+  // All output is redirected to STDOUT_FILENO and STDERR_FILENO.
+  EXPECT_CALL(*this, prepare(_, _))
+    .WillRepeatedly(Return(mesos::slave::ContainerLogger::SubprocessInfo()));
+}
+
+MockContainerLogger::~MockContainerLogger() {}
+
+
 MockDocker::MockDocker(
     const std::string& path,
     const std::string &socket)
   : Docker(path, socket)
 {
+  EXPECT_CALL(*this, ps(_, _))
+    .WillRepeatedly(Invoke(this, &MockDocker::_ps));
+
   EXPECT_CALL(*this, pull(_, _, _))
     .WillRepeatedly(Invoke(this, &MockDocker::_pull));
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9ab260a6/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index ce6a12d..5a737a6 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -32,6 +32,7 @@
 
 #include <mesos/fetcher/fetcher.hpp>
 
+#include <mesos/slave/container_logger.hpp>
 #include <mesos/slave/qos_controller.hpp>
 #include <mesos/slave/resource_estimator.hpp>
 
@@ -1034,6 +1035,26 @@ public:
 };
 
 
+// Definition of a mock ContainerLogger to be used in tests with gmock.
+class MockContainerLogger : public mesos::slave::ContainerLogger
+{
+public:
+  MockContainerLogger();
+  virtual ~MockContainerLogger();
+
+  MOCK_METHOD0(initialize, Try<Nothing>(void));
+
+  MOCK_METHOD2(
+      recover,
+      process::Future<Nothing>(const ExecutorInfo&, const std::string&));
+
+  MOCK_METHOD2(
+      prepare,
+      process::Future<mesos::slave::ContainerLogger::SubprocessInfo>(
+          const ExecutorInfo&, const std::string&));
+};
+
+
 // Definition of a mock Docker to be used in tests with gmock.
 class MockDocker : public Docker
 {
@@ -1056,6 +1077,11 @@ public:
           const process::Subprocess::IO&,
           const process::Subprocess::IO&));
 
+  MOCK_CONST_METHOD2(
+      ps,
+      process::Future<std::list<Docker::Container>>(
+          bool, const Option<std::string>&));
+
   MOCK_CONST_METHOD3(
       pull,
       process::Future<Docker::Image>(
@@ -1099,6 +1125,13 @@ public:
         stderr);
   }
 
+  process::Future<std::list<Docker::Container>> _ps(
+      bool all,
+      const Option<std::string>& prefix) const
+  {
+    return Docker::ps(all, prefix);
+  }
+
   process::Future<Docker::Image> _pull(
       const std::string& directory,
       const std::string& image,


[11/11] mesos git commit: Add documentation for logging and ContainerLogger.

Posted by be...@apache.org.
Add documentation for logging and ContainerLogger.

Moves (and updates) logging flags from configuration.md into the new
logging.md.

Add documentation for the ContainerLogger.

Adds documentation about logging in Master/Agent/Framework and the
ContainerLogger.

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


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

Branch: refs/heads/master
Commit: 4543cdf975937f7b4f99b3115c9081a7fc643388
Parents: 1c19b50
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Sun Jan 24 20:12:55 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:23 2016 -0800

----------------------------------------------------------------------
 docs/configuration.md |  56 +--------
 docs/home.md          |   1 +
 docs/logging.md       | 283 +++++++++++++++++++++++++++++++++++++++++++++
 docs/modules.md       |   2 +-
 src/logging/flags.cpp |  35 ++++--
 5 files changed, 309 insertions(+), 68 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/4543cdf9/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index 1c1e1f5..b1ef131 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -33,6 +33,7 @@ be found by running the binary with the flag `--help`, for example
 ## Master and Slave Options
 
 *These options can be supplied to both masters and slaves.*
+Also see the [common options for logging](logging.md#Internal).
 
 <table class="table table-striped">
   <thead>
@@ -69,16 +70,6 @@ be found by running the binary with the flag `--help`, for example
   </tr>
   <tr>
     <td>
-      --external_log_file=VALUE
-    </td>
-    <td>
-      Specify the externally managed log file. This file will be
-      exposed in the webui and HTTP API. This is useful when using
-      stderr logging as the log file is otherwise unknown to Mesos.
-    </td>
-  </tr>
-  <tr>
-    <td>
       --firewall_rules=VALUE
     </td>
     <td>
@@ -111,15 +102,6 @@ be found by running the binary with the flag `--help`, for example
   </tr>
   <tr>
     <td>
-      --[no-]initialize_driver_logging
-    </td>
-    <td>
-      Whether to automatically initialize Google logging of scheduler
-      and/or executor drivers. (default: true)
-    </td>
-  </tr>
-  <tr>
-    <td>
       --ip=VALUE
     </td>
     <td>
@@ -139,34 +121,6 @@ be found by running the binary with the flag `--help`, for example
   </tr>
   <tr>
     <td>
-      --log_dir=VALUE
-    </td>
-    <td>
-      Location to put log files. (no default, nothing is written to disk unless
-      specified; does not affect logging to stderr)
-    </td>
-  </tr>
-  <tr>
-    <td>
-      --logbufsecs=VALUE
-    </td>
-    <td>
-      How many seconds to buffer log messages for. (default: 0)
-    </td>
-  </tr>
-  <tr>
-    <td>
-      --logging_level=VALUE
-    </td>
-    <td>
-      Log message at or above this level. Possible values: <code>INFO</code>,
-      <code>WARNING</code>, <code>ERROR</code>; if quiet flag is used, this will
-      affect just the logs from <code>log_dir</code>, if specified.
-      (default: INFO)
-    </td>
-  </tr>
-  <tr>
-    <td>
       --port=VALUE
     </td>
     <td>
@@ -175,14 +129,6 @@ be found by running the binary with the flag `--help`, for example
   </tr>
   <tr>
     <td>
-      --[no-]quiet
-    </td>
-    <td>
-      Disable logging to stderr. (default: false)
-    </td>
-  </tr>
-  <tr>
-    <td>
       --[no-]version
     </td>
     <td>

http://git-wip-us.apache.org/repos/asf/mesos/blob/4543cdf9/docs/home.md
----------------------------------------------------------------------
diff --git a/docs/home.md b/docs/home.md
index ff797fb..fa9c19c 100644
--- a/docs/home.md
+++ b/docs/home.md
@@ -23,6 +23,7 @@ layout: documentation
 * [Framework Authentication](/documentation/latest/authentication/)
 * [Framework Authorization](/documentation/latest/authorization/)
 * [Framework Rate Limiting](/documentation/latest/framework-rate-limiting/)
+* [Logging](/documentation/latest/logging/)
 * [High Availability](/documentation/latest/high-availability/) for running multiple masters simultaneously.
 * [Operational Guide](/documentation/latest/operational-guide/)
 * [Monitoring](/documentation/latest/monitoring/)

http://git-wip-us.apache.org/repos/asf/mesos/blob/4543cdf9/docs/logging.md
----------------------------------------------------------------------
diff --git a/docs/logging.md b/docs/logging.md
new file mode 100644
index 0000000..d79a747
--- /dev/null
+++ b/docs/logging.md
@@ -0,0 +1,283 @@
+---
+layout: documentation
+---
+
+# Logging
+
+Mesos handles the logs of each Mesos component differently depending on the
+degree of control Mesos has over the source code of the component.
+
+Roughly, these categories are:
+
+* [Internal](#Internal) - Master and Agent.
+* [Containers](#Containers) - Executors and Tasks.
+* External - Components launched outside of Mesos, like
+  Frameworks and [ZooKeeper](high-availability.md).  These are expected to
+  implement their own logging solution.
+
+## <a name="Internal"></a>Internal
+
+The Mesos Master and Agent use the
+[Google's logging library](https://github.com/google/glog).
+Google logging options that are not explicitly mentioned below can be
+configured via environment variables.
+
+<table class="table table-striped">
+  <thead>
+    <tr>
+      <th width="30%">
+        Flag
+      </th>
+      <th>
+        Explanation
+      </th>
+    </tr>
+  </thead>
+
+  <tr>
+    <td>
+      <code>--quiet</code>
+    </td>
+    <td>
+      Disable logging to stderr.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>--logging_level=VALUE</code>
+    </td>
+    <td>
+      Log messages at or above this level.  Defaults to <code>INFO</code>.
+      Possible values:
+        <code>INFO</code>, <code>WARNING</code>, <code>ERROR</code>.
+
+      If <code>--quiet</code> is specified, this will only affect the logs
+      written to <code>--log_dir</code>, if specified.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>--log_dir=VALUE</code>
+    </td>
+    <td>
+      Location to put log files.  By default, nothing is written to disk.
+      Does not affect logging to stderr.
+
+      If specified, the log file will appear in the Mesos WebUI.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>--logbufsecs=VALUE</code>
+    </td>
+    <td>
+      Maximum number of seconds that logs may be buffered for.
+      By default, logs are flushed immediately.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>--[no-]initialize_driver_logging</code>
+    </td>
+    <td>
+      Whether the Master/Agent should initialize Google logging for the Mesos
+      scheduler and executor drivers, in same way as described here.
+      The scheduler/executor drivers have separate logs and do not get
+      written to the Master/Agent.
+
+      This option has no effect when using the HTTP scheduler/executor APIs.
+
+      By default, this option is true.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>--external_log_file=VALUE</code>
+    </td>
+    <td>
+      Location of the externally managed log file.  Mesos does not write to
+      this file directly and merely exposes it in the WebUI and HTTP API.
+
+      This is only useful when logging to stderr in combination with an external
+      logging mechanism, like syslog or journald.
+
+      This option is meaningless when specified along with <code>--quiet</code>.
+
+      This option takes precedence over <code>--log_dir</code> in the WebUI.
+      However, logs will still be written to the <code>--log_dir</code> if
+      that option is specified.
+    </td>
+  </tr>
+</table>
+
+Both Master and Agent also expose an HTTP endpoint which temporarily toggles
+verbose logging:
+
+```
+POST <ip:port>/logging/toggle?level=[1|2|3]&duration=VALUE
+```
+
+The effect is analogous to setting the `GLOG_v` environment variable prior
+to starting the Master/Agent, except the logging level will revert to the
+original level after the given duration.
+
+## <a name="Containers"></a>Containers
+
+For background, see [the containerizer documentation](containerizer.md).
+
+Mesos does not assume any structured logging for entities running inside
+containers.  Instead, Mesos will store the stdout and stderr of containers
+into plain files ("stdout" and "stderr") located inside
+[the sandbox](sandbox.md#where-is-it).
+
+In some cases, the default Container logger behavior of Mesos is not ideal:
+
+* Logging may not be standardized across containers.
+* Logs are not easily aggregated.
+* Log file sizes are not managed.  Given enough time, the "stdout" and "stderr"
+  files can fill up the Agent's disk.
+
+## `ContainerLogger` Module
+
+The `ContainerLogger` module was introduced in Mesos 0.27.0 and aims to address
+the shortcomings of the default logging behavior for containers.  The module
+can be used to change how Mesos redirects the stdout and stderr of containers.
+
+The [interface for a `ContainerLogger` can be found here](https://github.com/apache/mesos/blob/master/include/mesos/slave/container_logger.hpp).
+
+Mesos comes with two `ContainerLogger` modules:
+
+* The `SandboxContainerLogger` implements the existing logging behavior as
+  a `ContainerLogger`.  This is the default behavior.
+* The `LogrotateContainerLogger` addresses the problem of unbounded log file
+  sizes.
+
+### `LogrotateContainerLogger`
+
+The `LogrotateContainerLogger` constrains the total size of a container's
+stdout and stderr files.  The module does this by rotating log files based
+on the parameters to the module.  When a log file reaches its specified
+maximum size, it is renamed by appending a `.N` to the end of the filename,
+where `N` increments each rotation.  Older log files are deleted when the
+specified maximum number of files is reached.
+
+#### Invoking the module
+
+The `LogrotateContainerLogger` can be loaded by specifying the library
+`liblogrotate_container_logger.so` in the
+[`--modules` flag](modules.md#Invoking) when starting the Agent and by
+setting the `--container_logger` Agent flag to
+`org_apache_mesos_LogrotateContainerLogger`.
+
+#### Module parameters
+
+<table class="table table-striped">
+  <thead>
+    <tr>
+      <th width="30%">
+        Key
+      </th>
+      <th>
+        Explanation
+      </th>
+    </tr>
+  </thead>
+
+  <tr>
+    <td>
+      <code>max_stdout_size</code>/<code>max_stderr_size</code>
+    </td>
+    <td>
+      Maximum size, in bytes, of a single stdout/stderr log file.
+      When the size is reached, the file will be rotated.
+
+      Defaults to 10 MB.  Minimum size of 1 (memory) page, usually around 4 KB.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>logrotate_stdout_options</code>/
+      <code>logrotate_stderr_options</code>
+    </td>
+    <td>
+      Additional config options to pass into <code>logrotate</code> for stdout.
+      This string will be inserted into a <code>logrotate</code> configuration
+      file. i.e. For "stdout":
+      <pre>
+/path/to/stdout {
+  [logrotate_stdout_options]
+  size [max_stdout_size]
+}</pre>
+      NOTE: The <code>size</code> option will be overriden by this module.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>launcher_dir</code>
+    </td>
+    <td>
+      Directory path of Mesos binaries.
+      The <code>LogrotateContainerLogger</code> will find the
+      <code>mesos-logrotate-logger</code> binary under this directory.
+
+      Defaults to <code>/usr/local/libexec/mesos</code>.
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>logrotate_path</code>
+    </td>
+    <td>
+      If specified, the <code>LogrotateContainerLogger</code> will use the
+      specified <code>logrotate</code> instead of the system's
+      <code>logrotate</code>.  If <code>logrotate</code> is not found, then
+      the module will exit with an error.
+    </td>
+  </tr>
+</table>
+
+#### How it works
+
+1. Every time a container starts up, the `LogrotateContainerLogger`
+   starts up companion subprocesses of the `mesos-logrotate-logger` binary.
+2. The module instructs Mesos to redirect the container's stdout/stderr
+   to the `mesos-logrotate-logger`.
+3. As the container outputs to stdout/stderr, `mesos-logrotate-logger` will
+   pipe the output into the "stdout"/"stderr" files.  As the files grow,
+   `mesos-logrotate-logger` will call `logrotate` to keep the files strictly
+   under the configured maximum size.
+4. When the container exits, `mesos-logrotate-logger` will finish logging before
+   exiting as well.
+
+The `LogrotateContainerLogger` is designed to be resilient across Agent
+failover.  If the Agent process dies, any instances of `mesos-logrotate-logger`
+will continue to run.
+
+### Writing a Custom `ContainerLogger`
+
+For basics on module writing, see [the modules documentation](modules.md).
+
+There are several caveats to consider when designing a new `ContainerLogger`:
+
+* Logging by the `ContainerLogger` should be resilient to Agent failover.
+  If the Agent process dies (which includes the `ContainerLogger` module),
+  logging should continue.  This is usually achieved by using subprocesses.
+* When containers shut down, the `ContainerLogger` is not explicitly notified.
+  Instead, encountering `EOF` in the container's stdout/stderr signifies
+  that the container has exited.  This provides a stronger guarantee that the
+  `ContainerLogger` has seen all the logs before exiting itself.
+* The `ContainerLogger` should not assume that containers have been launched
+  with any specific `ContainerLogger`.  The Agent may be restarted with a
+  different `ContainerLogger`.
+* Each [containerizer](containerizer.md) running on an Agent uses its own
+  instance of the `ContainerLogger`.  This means more than one `ContainerLogger`
+  may be running in a single Agent.  However, each Agent will only run a single
+  type of `ContainerLogger`.

http://git-wip-us.apache.org/repos/asf/mesos/blob/4543cdf9/docs/modules.md
----------------------------------------------------------------------
diff --git a/docs/modules.md b/docs/modules.md
index 1aad8e0..8e0249d 100644
--- a/docs/modules.md
+++ b/docs/modules.md
@@ -29,7 +29,7 @@ Finally, modules provide an easy way for third parties to easily extend Mesos
 without having to know all the internal details.
 
 
-## Invoking Mesos modules
+## <a name="Invoking"></a>Invoking Mesos modules
 
 The command-line flag `--modules` is available for Mesos master, slave, and
 tests to specify a list of modules to be loaded and be available to the internal

http://git-wip-us.apache.org/repos/asf/mesos/blob/4543cdf9/src/logging/flags.cpp
----------------------------------------------------------------------
diff --git a/src/logging/flags.cpp b/src/logging/flags.cpp
index b321c28..978d735 100644
--- a/src/logging/flags.cpp
+++ b/src/logging/flags.cpp
@@ -26,33 +26,44 @@ mesos::internal::logging::Flags::Flags()
 
   add(&Flags::logging_level,
       "logging_level",
-      "Log message at or above this level; possible values: \n"
-      "'INFO', 'WARNING', 'ERROR'; if quiet flag is used, this \n"
-      "will affect just the logs from log_dir (if specified)",
+      "Log message at or above this level.  Defaults to 'INFO'.\n"
+      "Possible values: 'INFO', 'WARNING', 'ERROR'.\n"
+      "If '--quiet' is specified, this will only affect the logs\n"
+      "written to '--log_dir', if specified.",
       "INFO");
 
   add(&Flags::log_dir,
       "log_dir",
-      "Directory path to put log files (no default, nothing\n"
-      "is written to disk unless specified;\n"
-      "does not affect logging to stderr).\n"
+      "Location to put log files.  By default, nothing is written to disk.\n"
+      "Does not affect logging to stderr.\n"
+      "If specified, the log file will appear in the Mesos WebUI."
       "NOTE: 3rd party log messages (e.g. ZooKeeper) are\n"
       "only written to stderr!");
 
   add(&Flags::logbufsecs,
       "logbufsecs",
-      "How many seconds to buffer log messages for",
+      "Maximum number of seconds that logs may be buffered for.\n"
+      "By default, logs are flushed immediately.",
       0);
 
   add(&Flags::initialize_driver_logging,
       "initialize_driver_logging",
-      "Whether to automatically initialize Google logging of scheduler\n"
-      "and/or executor drivers.",
+      "Whether the Master/Agent should initialize Google logging for the\n"
+      "Mesos scheduler and executor drivers, in same way as described here.\n"
+      "The scheduler/executor drivers have separate logs and do not get\n"
+      "written to the Master/Agent logs.\n\n"
+      "This option has no effect when using the HTTP scheduler/executor APIs.\n"
+      "By default, this option is true.",
       true);
 
   add(&Flags::external_log_file,
       "external_log_file",
-      "Specified the externally managed log file. This file will be\n"
-      "exposed in the webui and HTTP api. This is useful when using\n"
-      "stderr logging as the log file is otherwise unknown to Mesos.");
+      "Location of the externally managed log file.  Mesos does not write to\n"
+      "this file directly and merely exposes it in the WebUI and HTTP API.\n"
+      "This is only useful when logging to stderr in combination with an\n"
+      "external logging mechanism, like syslog or journald.\n\n"
+      "This option is meaningless when specified along with '--quiet'.\n\n"
+      "This option takes precedence over '--log_dir' in the WebUI.\n"
+      "However, logs will still be written to the '--log_dir' if\n"
+      "that option is specified.");
 }


[08/11] mesos git commit: Implement ContainerLogger recovery for the logger module.

Posted by be...@apache.org.
Implement ContainerLogger recovery for the logger module.

Adds a call to `ContainerLogger::recover` inside each containerizer's
`Containerizer::recover`.

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


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

Branch: refs/heads/master
Commit: 1c19b50fb66af9616e170ec7df5601f4c2e59440
Parents: 9ab260a
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Sun Jan 24 20:12:38 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jan 24 20:13:23 2016 -0800

----------------------------------------------------------------------
 src/slave/containerizer/docker.cpp              | 18 ++++++++++++++++++
 src/slave/containerizer/mesos/containerizer.cpp | 11 +++++++++++
 2 files changed, 29 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1c19b50f/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 40f6f0b..d2b77e3 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -691,6 +691,24 @@ Future<Nothing> DockerContainerizerProcess::_recover(
       }
 
       pids.put(containerId, pid);
+
+      const string sandboxDirectory = paths::getExecutorRunPath(
+          flags.work_dir,
+          state.id,
+          framework.id,
+          executor.id,
+          containerId);
+
+      // Pass recovered containers to the container logger.
+      // NOTE: The current implementation of the container logger only
+      // outputs a warning and does not have any other consequences.
+      // See `ContainerLogger::recover` for more information.
+      logger->recover(executorInfo, sandboxDirectory)
+        .onFailed(defer(self(), [executorInfo](const string& message) {
+          LOG(WARNING) << "Container logger failed to recover executor '"
+                       << executorInfo.executor_id() << "': "
+                       << message;
+        }));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/1c19b50f/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 624cd1a..4b504db 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -538,6 +538,17 @@ Future<Nothing> MesosContainerizerProcess::__recover(
       isolator->watch(containerId)
         .onAny(defer(self(), &Self::limited, containerId, lambda::_1));
     }
+
+    // Pass recovered containers to the container logger.
+    // NOTE: The current implementation of the container logger only
+    // outputs a warning and does not have any other consequences.
+    // See `ContainerLogger::recover` for more information.
+    logger->recover(run.executor_info(), run.directory())
+      .onFailed(defer(self(), [run](const string& message) {
+        LOG(WARNING) << "Container logger failed to recover executor '"
+                     << run.executor_info().executor_id() << "': "
+                     << message;
+      }));
   }
 
   // Destroy all the orphan containers.