You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/09/24 20:23:58 UTC

[1/3] mesos git commit: Updated launch helper to optionally spawn an 'init' process on linux.

Repository: mesos
Updated Branches:
  refs/heads/master 972f11800 -> 9c61f05ee


Updated launch helper to optionally spawn an 'init' process on linux.

Previously the 'mesos-containerizer launch' binary would simply exec
into the actual command we wanted to launch after doing some set of
preperatory work. The problem with this approach, however, is that
this gives us no opportunity to checkpoint the wait status of the
command so the agent can recover it in cases where it is offline at
the time the command completes.

To compensate for this, we now allow 'mesos-containerizer launch' to
take an optional '--runtime_directory' argument, which indicates where
to checkpint the status of the launched command (as returned by
'waitpid()'). In order to do this checkpointing, however, we cannot
simply exec into the command anymore. Instead we now fork-exec the
command and reap it once it completes. We then checkpoint its status
and return it as our own. We call the original process the 'init'
process of the container and the fork-exec'd process its child.  As a
side effect of doing things this way, we also have to be careful to
forward all signals sent to the init process down to the child.

In a subsequent commit we will update the mesos containerizer to use
this new functionality.

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


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

Branch: refs/heads/master
Commit: 0107bc320ed2dac8b878ef37951944c1db609aea
Parents: 9e71f10
Author: Kevin Klues <kl...@gmail.com>
Authored: Wed Aug 24 14:27:10 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Sep 24 12:21:29 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 295 +++++++++++++++++++++++---
 src/slave/containerizer/mesos/launch.hpp |   1 +
 2 files changed, 262 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0107bc32/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 777121c..11eaa0e 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -17,15 +17,18 @@
 #include <errno.h>
 #ifdef __linux__
 #include <sched.h>
+#include <signal.h>
 #endif // __linux__
 #include <string.h>
 
 #include <iostream>
 #include <set>
+#include <string>
 
 #include <stout/foreach.hpp>
 #include <stout/os.hpp>
 #include <stout/protobuf.hpp>
+#include <stout/path.hpp>
 #include <stout/unreachable.hpp>
 
 #ifdef __linux__
@@ -37,6 +40,7 @@
 #include "mesos/mesos.hpp"
 
 #include "slave/containerizer/mesos/launch.hpp"
+#include "slave/containerizer/mesos/paths.hpp"
 
 using std::cerr;
 using std::cout;
@@ -70,6 +74,10 @@ MesosContainerizerLaunch::Flags::Flags()
       "w.r.t. the root filesystem used for the command.");
 
 #ifndef __WINDOWS__
+  add(&runtime_directory,
+      "runtime_directory",
+      "The runtime directory for the container (used for checkpointing)");
+
   add(&rootfs,
       "rootfs",
       "Absolute path to the container root filesystem. The command will be \n"
@@ -114,12 +122,164 @@ MesosContainerizerLaunch::Flags::Flags()
 }
 
 
+static Option<pid_t> containerPid = None();
+static Option<string> containerStatusPath = None();
+static Option<int> containerStatusFd = None();
+
+static void exitWithSignal(int sig);
+static void exitWithStatus(int status);
+
+
+#ifndef __WINDOWS__
+static void signalSafeWriteStatus(int status)
+{
+  const string statusString = std::to_string(status);
+
+  Try<Nothing> write = os::write(
+      containerStatusFd.get(),
+      statusString);
+
+  if (write.isError()) {
+    os::write(STDERR_FILENO,
+              "Failed to write container status '" +
+              statusString + "': " + ::strerror(errno));
+  }
+}
+
+
+// When launching the executor with an 'init' process, we need to
+// forward all relevant signals to it. The functions below help to
+// enable this forwarding.
+static void signalHandler(int sig)
+{
+  // If we dn't yet have a container pid, we treat
+  // receiving a signal like a failure and exit.
+  if (containerPid.isNone()) {
+    exitWithSignal(sig);
+  }
+
+  // Otherwise we simply forward the signal to `containerPid`. We
+  // purposefully ignore the error here since we have to remain async
+  // signal safe. The only possible error scenario relevant to us is
+  // ESRCH, but if that happens that means our pid is already gone and
+  // the process will exit soon. So we are safe.
+  os::kill(containerPid.get(), sig);
+}
+
+
+static Try<Nothing> installSignalHandlers()
+{
+  // Install handlers for all standard POSIX signals
+  // (i.e. any signal less than `NSIG`).
+  for (int i = 1; i < NSIG; i++) {
+    // We don't want to forward the SIGCHLD signal, nor do we want to
+    // handle it ourselves because we reap all children inline in the
+    // `execute` function.
+    if (i == SIGCHLD) {
+      continue;
+    }
+
+    // We can't catch or ignore these signals, so we shouldn't try
+    // to register a handler for them.
+    if (i == SIGKILL || i == SIGSTOP) {
+      continue;
+    }
+
+    // The NSIG constant is used to determine the number of signals
+    // available on a system. However, Darwin, Linux, and BSD differ
+    // on their interpretation of of the value of NSIG. Linux, for
+    // example, sets it to 65, where Darwin sets it to 32. The reason
+    // for the discrepency is that Linux includes the real-time
+    // signals in this count, where Darwin does not. However, even on
+    // linux, we are not able to arbitrarily install signal handlers
+    // for all the real-time signals -- they must have not been
+    // registered with the system first. For this reason, we
+    // standardize on verifying the installation of handlers for
+    // signals 1-31 (since these are defined in the POSIX standard),
+    // but we continue to attempt to install handlers up to the value
+    // of NSIG without verification.
+    const int posixLimit = 32;
+    if (os::signals::install(i, signalHandler) != 0 && i < posixLimit) {
+      return ErrnoError("Unable to register signal"
+                        " '" + stringify(strsignal(i)) + "'");
+    }
+  }
+
+  return Nothing();
+}
+#endif // __WINDOWS__
+
+
+static void exitWithSignal(int sig)
+{
+#ifndef __WINDOWS__
+  if (containerStatusFd.isSome()) {
+    signalSafeWriteStatus(W_EXITCODE(0, sig));
+    os::close(containerStatusFd.get());
+  }
+#endif // __WINDOWS__
+  ::_exit(EXIT_FAILURE);
+}
+
+
+static void exitWithStatus(int status)
+{
+#ifndef __WINDOWS__
+  if (containerStatusFd.isSome()) {
+    signalSafeWriteStatus(W_EXITCODE(status, 0));
+    os::close(containerStatusFd.get());
+  }
+#endif // __WINDOWS__
+  ::_exit(status);
+}
+
+
 int MesosContainerizerLaunch::execute()
 {
+#ifndef __WINDOWS__
+  // The existence of the `runtime_directory` flag implies that we
+  // want to checkpoint the container's status upon exit.
+  if (flags.runtime_directory.isSome()) {
+    containerStatusPath = path::join(
+        flags.runtime_directory.get(),
+        containerizer::paths::STATUS_FILE);
+
+    Try<int> open = os::open(
+        containerStatusPath.get(),
+        O_WRONLY | O_CREAT | O_CLOEXEC,
+        S_IRUSR | S_IWUSR);
+
+    if (open.isError()) {
+      cerr << "Failed to open file for writing the container status"
+           << " '" << containerStatusPath.get() << "':"
+           << " " << open.error() << endl;
+      exitWithStatus(EXIT_FAILURE);
+    }
+
+    containerStatusFd = open.get();
+  }
+
+  // We need a signal fence here to ensure that `containerStatusFd` is
+  // actually written to memory and not just to a temporary register.
+  // Without this, it's possible that the signal handler we are about
+  // to install would never see the correct value since there's no
+  // guarantee that it is written to memory until this function
+  // completes (which won't happen for a really long time because we
+  // do a blocking `waitpid()` below).
+  std::atomic_signal_fence(std::memory_order_relaxed);
+
+  // Install signal handlers for all incoming signals.
+  Try<Nothing> signals = installSignalHandlers();
+  if (signals.isError()) {
+    cerr << "Failed to install signal handlers: " << signals.error() << endl;
+    exitWithStatus(EXIT_FAILURE);
+  }
+#endif // __WINDOWS__
+
   // Check command line flags.
   if (flags.command.isNone()) {
     cerr << "Flag --command is not specified" << endl;
-    return EXIT_FAILURE;
+    exitWithStatus(EXIT_FAILURE);
   }
 
   bool controlPipeSpecified =
@@ -129,7 +289,7 @@ int MesosContainerizerLaunch::execute()
       (flags.pipe_read.isNone() && flags.pipe_write.isSome())) {
     cerr << "Flag --pipe_read and --pipe_write should either be "
          << "both set or both not set" << endl;
-    return EXIT_FAILURE;
+    exitWithStatus(EXIT_FAILURE);
   }
 
   // Parse the command.
@@ -138,19 +298,19 @@ int MesosContainerizerLaunch::execute()
 
   if (command.isError()) {
     cerr << "Failed to parse the command: " << command.error() << endl;
-    return EXIT_FAILURE;
+    exitWithStatus(EXIT_FAILURE);
   }
 
   // Validate the command.
   if (command.get().shell()) {
     if (!command.get().has_value()) {
       cerr << "Shell command is not specified" << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   } else {
     if (!command.get().has_value()) {
       cerr << "Executable path is not specified" << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 
@@ -169,7 +329,7 @@ int MesosContainerizerLaunch::execute()
     Try<Nothing> close = os::close(pipe[1]);
     if (close.isError()) {
       cerr << "Failed to close pipe[1]: " << close.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     // Do a blocking read on the pipe until the parent signals us to continue.
@@ -179,17 +339,17 @@ int MesosContainerizerLaunch::execute()
            errno == EINTR);
 
     if (length != sizeof(dummy)) {
-       // There's a reasonable probability this will occur during
-       // agent restarts across a large/busy cluster.
-       cerr << "Failed to synchronize with agent "
-            << "(it's probably exited)" << endl;
-       return EXIT_FAILURE;
+      // There's a reasonable probability this will occur during
+      // agent restarts across a large/busy cluster.
+      cerr << "Failed to synchronize with agent "
+           << "(it's probably exited)" << endl;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     close = os::close(pipe[0]);
     if (close.isError()) {
       cerr << "Failed to close pipe[0]: " << close.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 
@@ -198,7 +358,7 @@ int MesosContainerizerLaunch::execute()
     if (unshare(CLONE_NEWNS) != 0) {
       cerr << "Failed to unshare mount namespace: "
            << os::strerror(errno) << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 #endif // __linux__
@@ -211,19 +371,19 @@ int MesosContainerizerLaunch::execute()
     foreach (const JSON::Value& value, array.values) {
       if (!value.is<JSON::Object>()) {
         cerr << "Invalid JSON format for flag --commands" << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
 
       Try<CommandInfo> parse = ::protobuf::parse<CommandInfo>(value);
       if (parse.isError()) {
         cerr << "Failed to parse a preparation command: "
              << parse.error() << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
 
       if (!parse.get().has_value()) {
         cerr << "The 'value' of a preparation command is not specified" << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
 
       cout << "Executing pre-exec command '" << value << "'" << endl;
@@ -246,7 +406,7 @@ int MesosContainerizerLaunch::execute()
 
       if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) {
         cerr << "Failed to execute pre-exec command '" << value << "'" << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
     }
   }
@@ -271,7 +431,7 @@ int MesosContainerizerLaunch::execute()
     if (!_uid.isSome()) {
       cerr << "Failed to get the uid of user '" << flags.user.get() << "': "
            << (_uid.isError() ? _uid.error() : "not found") << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     // No need to change user/groups if the specified user is the same
@@ -281,7 +441,7 @@ int MesosContainerizerLaunch::execute()
       if (!_gid.isSome()) {
         cerr << "Failed to get the gid of user '" << flags.user.get() << "': "
              << (_gid.isError() ? _gid.error() : "not found") << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
 
       Try<vector<gid_t>> _gids = os::getgrouplist(flags.user.get());
@@ -289,7 +449,7 @@ int MesosContainerizerLaunch::execute()
         cerr << "Failed to get the supplementary gids of user '"
              << flags.user.get() << "': "
              << (_gids.isError() ? _gids.error() : "not found") << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
 
       uid = _uid.get();
@@ -308,7 +468,7 @@ int MesosContainerizerLaunch::execute()
     if (capabilitiesManager.isError()) {
       cerr << "Failed to initialize capabilities support: "
            << capabilitiesManager.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     // Prevent clearing of capabilities on `setuid`.
@@ -317,7 +477,7 @@ int MesosContainerizerLaunch::execute()
       if (keepCaps.isError()) {
         cerr << "Failed to set process control for keeping capabilities "
              << "on potential uid change: " << keepCaps.error() << endl;
-        return EXIT_FAILURE;
+        exitWithStatus(EXIT_FAILURE);
       }
     }
   }
@@ -339,13 +499,13 @@ int MesosContainerizerLaunch::execute()
     if (realpath.isError()) {
       cerr << "Failed to determine if rootfs is an absolute path: "
            << realpath.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     } else if (realpath.isNone()) {
       cerr << "Rootfs path does not exist" << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     } else if (realpath.get() != rootfs.get()) {
       cerr << "Rootfs path is not an absolute path" << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
 #ifdef __linux__
@@ -358,7 +518,7 @@ int MesosContainerizerLaunch::execute()
     if (chroot.isError()) {
       cerr << "Failed to enter chroot '" << rootfs.get()
            << "': " << chroot.error();
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 
@@ -371,21 +531,21 @@ int MesosContainerizerLaunch::execute()
     if (setgid.isError()) {
       cerr << "Failed to set gid to " << gid.get()
            << ": " << setgid.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     Try<Nothing> setgroups = os::setgroups(gids, uid);
     if (setgroups.isError()) {
       cerr << "Failed to set supplementary gids: "
            << setgroups.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     Try<Nothing> setuid = os::setuid(uid.get());
     if (setuid.isError()) {
       cerr << "Failed to set uid to " << uid.get()
            << ": " << setuid.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 #endif // __WINDOWS__
@@ -398,14 +558,14 @@ int MesosContainerizerLaunch::execute()
     if (requestedCapabilities.isError()) {
       cerr << "Failed to parse capabilities: "
            << requestedCapabilities.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     Try<ProcessCapabilities> capabilities = capabilitiesManager->get();
     if (capabilities.isError()) {
       cerr << "Failed to get capabilities for the current process: "
            << capabilities.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     // After 'setuid', 'effective' set is cleared. Since `SETPCAP` is
@@ -417,7 +577,7 @@ int MesosContainerizerLaunch::execute()
     if (setPcap.isError()) {
       cerr << "Failed to add SETPCAP to the effective set: "
            << setPcap.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
 
     // Set up requested capabilities.
@@ -431,7 +591,7 @@ int MesosContainerizerLaunch::execute()
     Try<Nothing> set = capabilitiesManager->set(capabilities.get());
     if (set.isError()) {
       cerr << "Failed to set process capabilities: " << set.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 #endif // __linux__
@@ -442,13 +602,80 @@ int MesosContainerizerLaunch::execute()
       cerr << "Failed to chdir into current working directory "
            << "'" << flags.working_directory.get() << "': "
            << chdir.error() << endl;
-      return EXIT_FAILURE;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
 
   // Relay the environment variables.
   // TODO(jieyu): Consider using a clean environment.
 
+#ifndef __WINDOWS__
+  // If we have `containerStatusFd` set, then we need to fork-exec the
+  // command we are launching and checkpoint its status on exit. We
+  // use fork-exec directly (as opposed to `process::subprocess()`) to
+  // avoid intializing libprocess for this simple helper binary.
+  //
+  // TODO(klueska): Once we move the majority of `process::subprocess()`
+  // into stout, update the code below to use it.
+  if (containerStatusFd.isSome()) {
+    pid_t pid = ::fork();
+
+    if (pid == -1) {
+      cerr << "Failed to fork() the command: " << os::strerror(errno) << endl;
+      exitWithStatus(EXIT_FAILURE);
+    }
+
+    // If we are the parent...
+    if (pid > 0) {
+      // Set the global `containerPid` variable to enable signal forwarding.
+      //
+      // NOTE: We need a signal fence here to ensure that `containerPid`
+      // is actually written to memory and not just to a temporary register.
+      // Without this, it's possible that the signal handler would
+      // never notice the change since there's no guarantee that it is
+      // written out to memory until this function completes (which
+      // won't happen until it's too late because we loop inside a
+      // blocking `waitpid()` call below).
+      containerPid = pid;
+      std::atomic_signal_fence(std::memory_order_relaxed);
+
+      // Wait for the newly created process to finish.
+      int status = 0;
+      Result<pid_t> waitpid = None();
+
+      // Reap all decendants, but only continue once we reap the
+      // process we just launched.
+      while (true) {
+        waitpid = os::waitpid(-1, &status, 0);
+
+        if (waitpid.isError()) {
+          // If the error was an EINTR, we were interrupted by a
+          // signal and should just call `waitpid()` over again.
+          if (errno == EINTR) {
+            continue;
+          }
+          cerr << "Failed to os::waitpid(): " << waitpid.error() << endl;
+          exitWithStatus(EXIT_FAILURE);
+        }
+
+        if (waitpid.isNone()) {
+          cerr << "Calling os::waitpid() with blocking semantics"
+               << "returned asynchronously" << endl;
+          exitWithStatus(EXIT_FAILURE);
+        }
+
+        if (pid == waitpid.get()) {
+          break;
+        }
+      }
+
+      signalSafeWriteStatus(status);
+      os::close(containerStatusFd.get());
+      ::_exit(EXIT_SUCCESS);
+    }
+  }
+#endif // __WINDOWS__
+
   if (command->shell()) {
     // Execute the command using shell.
     os::execlp(os::Shell::name,

http://git-wip-us.apache.org/repos/asf/mesos/blob/0107bc32/src/slave/containerizer/mesos/launch.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.hpp b/src/slave/containerizer/mesos/launch.hpp
index 859990c..625b904 100644
--- a/src/slave/containerizer/mesos/launch.hpp
+++ b/src/slave/containerizer/mesos/launch.hpp
@@ -37,6 +37,7 @@ public:
     Option<JSON::Object> command;
     Option<std::string> working_directory;
 #ifndef __WINDOWS__
+    Option<std::string> runtime_directory;
     Option<std::string> rootfs;
     Option<std::string> user;
 #endif // __WINDOWS__


[2/3] mesos git commit: Added stout/os/kill.hpp to stout/os.hpp.

Posted by ji...@apache.org.
Added stout/os/kill.hpp to stout/os.hpp.


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

Branch: refs/heads/master
Commit: 9e71f10239f651f2191a97eb603546dd5d66b6bb
Parents: 972f118
Author: Jie Yu <yu...@gmail.com>
Authored: Sat Sep 24 12:05:01 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Sep 24 12:21:29 2016 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/os.hpp | 1 +
 1 file changed, 1 insertion(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9e71f102/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os.hpp b/3rdparty/stout/include/stout/os.hpp
index cb53c02..c38e434 100644
--- a/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/stout/include/stout/os.hpp
@@ -48,6 +48,7 @@
 #include <stout/os/chroot.hpp>
 #include <stout/os/exists.hpp>
 #include <stout/os/fcntl.hpp>
+#include <stout/os/kill.hpp>
 #include <stout/os/ls.hpp>
 #include <stout/os/mkdir.hpp>
 #include <stout/os/mkdtemp.hpp>


[3/3] mesos git commit: Reverted "Added extra functions to the 'Launcher' abstraction.".

Posted by ji...@apache.org.
Reverted "Added extra functions to the 'Launcher' abstraction.".

This reverts commit bb047cd72936aa1836b6e959127a572c72fb824b.

We decided to revert this commit in favor of keeping all container
checkpointing information in the containerizer. A subsequent commit
will reintroduce this functionality there.

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


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

Branch: refs/heads/master
Commit: 9c61f05ee79f7e7071c8796cc6630670098dd39a
Parents: 0107bc3
Author: Kevin Klues <kl...@gmail.com>
Authored: Sat Sep 17 19:47:49 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Sep 24 12:21:34 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launcher.cpp      | 44 ++------------------
 src/slave/containerizer/mesos/launcher.hpp      | 26 +-----------
 .../containerizer/mesos/linux_launcher.cpp      | 15 -------
 .../containerizer/mesos/linux_launcher.hpp      |  3 --
 src/tests/containerizer/launcher.hpp            |  4 --
 5 files changed, 5 insertions(+), 87 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9c61f05e/src/slave/containerizer/mesos/launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp
index 2155e7a..5d1410d 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -21,7 +21,7 @@
 #include <process/process.hpp>
 #include <process/reap.hpp>
 
-#include <stout/path.hpp>
+#include <stout/unreachable.hpp>
 
 #include <stout/os/killtree.hpp>
 
@@ -46,35 +46,9 @@ namespace mesos {
 namespace internal {
 namespace slave {
 
-constexpr char POSIX_LAUNCHER_NAME[] = "posix";
-
-
-string Launcher::buildPathFromHierarchy(
-    const ContainerID& _containerId,
-    const string& prefix)
-{
-  // Build the path in reverse order
-  // by following the parent hierarchy.
-  ContainerID containerId = _containerId;
-
-  string path = path::join(prefix, containerId.value());
-
-  while (containerId.has_parent()) {
-    containerId = containerId.parent();
-
-    path = path::join(
-        prefix,
-        containerId.value(),
-        path);
-  }
-
-  return path;
-}
-
-
 Try<Launcher*> PosixLauncher::create(const Flags& flags)
 {
-  return new PosixLauncher(flags);
+  return new PosixLauncher();
 }
 
 
@@ -208,21 +182,9 @@ Future<ContainerStatus> PosixLauncher::status(const ContainerID& containerId)
 }
 
 
-string PosixLauncher::getExitStatusCheckpointPath(
-    const ContainerID& containerId)
-{
-  return path::join(
-      flags.runtime_dir,
-      "launcher",
-      POSIX_LAUNCHER_NAME,
-      buildPathFromHierarchy(containerId, "containers"),
-      "exit_status");
-}
-
-
 Try<Launcher*> WindowsLauncher::create(const Flags& flags)
 {
-  return new WindowsLauncher(flags);
+  return new WindowsLauncher();
 }
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c61f05e/src/slave/containerizer/mesos/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launcher.hpp b/src/slave/containerizer/mesos/launcher.hpp
index 6d9fb8d..6ceb02d 100644
--- a/src/slave/containerizer/mesos/launcher.hpp
+++ b/src/slave/containerizer/mesos/launcher.hpp
@@ -79,21 +79,6 @@ public:
   // Currently only returns Executor PID info.
   virtual process::Future<ContainerStatus> status(
       const ContainerID& containerId) = 0;
-
-  // Get the the path where to checkpoint the
-  // exit status of the container with `containerId`.
-  virtual std::string getExitStatusCheckpointPath(
-      const ContainerID& containerId) = 0;
-
-protected:
-  // Build a path based on a hierarchy of containers whose leaf container
-  // is `containerId`. The `prefix` parameter is prepended to the
-  // `containerId` as we build the path up the tree (e.g. passing a
-  // `containersId` with a value of 3 and a prefix of `foo` would
-  // result in .../foo/1/foo/2/foo/3).
-  std::string buildPathFromHierarchy(
-      const ContainerID& containerId,
-      const string& prefix);
 };
 
 
@@ -128,18 +113,12 @@ public:
   virtual process::Future<ContainerStatus> status(
       const ContainerID& containerId);
 
-  virtual std::string getExitStatusCheckpointPath(
-      const ContainerID& containerId);
-
 protected:
-  PosixLauncher(const Flags& _flags)
-    : flags(_flags) {}
+  PosixLauncher() {}
 
   // The 'pid' is the process id of the first process and also the
   // process group id and session id.
   hashmap<ContainerID, pid_t> pids;
-
-  const Flags flags;
 };
 
 
@@ -153,8 +132,7 @@ public:
   virtual ~WindowsLauncher() {}
 
 private:
-  WindowsLauncher(const Flags& flags)
-    : PosixLauncher(flags) {}
+  WindowsLauncher() {}
 };
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c61f05e/src/slave/containerizer/mesos/linux_launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp b/src/slave/containerizer/mesos/linux_launcher.cpp
index f277570..1f66220 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -52,9 +52,6 @@ namespace mesos {
 namespace internal {
 namespace slave {
 
-constexpr char LINUX_LAUNCHER_NAME[] = "linux";
-
-
 static ContainerID container(const string& cgroup)
 {
   string basename = Path(cgroup).basename();
@@ -365,18 +362,6 @@ Future<ContainerStatus> LinuxLauncher::status(const ContainerID& containerId)
 }
 
 
-string LinuxLauncher::getExitStatusCheckpointPath(
-    const ContainerID& containerId)
-{
-  return path::join(
-      flags.runtime_dir,
-      "launcher",
-      LINUX_LAUNCHER_NAME,
-      buildPathFromHierarchy(containerId, "containers"),
-      "exit_status");
-}
-
-
 string LinuxLauncher::cgroup(const ContainerID& containerId)
 {
   return path::join(flags.cgroups_root, containerId.value());

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c61f05e/src/slave/containerizer/mesos/linux_launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/linux_launcher.hpp b/src/slave/containerizer/mesos/linux_launcher.hpp
index 2cd1e4d..28930fe 100644
--- a/src/slave/containerizer/mesos/linux_launcher.hpp
+++ b/src/slave/containerizer/mesos/linux_launcher.hpp
@@ -55,9 +55,6 @@ public:
   virtual process::Future<ContainerStatus> status(
       const ContainerID& containerId);
 
-  virtual std::string getExitStatusCheckpointPath(
-      const ContainerID& containerId);
-
 private:
   LinuxLauncher(
       const Flags& flags,

http://git-wip-us.apache.org/repos/asf/mesos/blob/9c61f05e/src/tests/containerizer/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launcher.hpp b/src/tests/containerizer/launcher.hpp
index a149220..773b458 100644
--- a/src/tests/containerizer/launcher.hpp
+++ b/src/tests/containerizer/launcher.hpp
@@ -78,10 +78,6 @@ public:
       status,
       process::Future<ContainerStatus>(const ContainerID& containerId));
 
-  MOCK_METHOD1(
-      getExitStatusCheckpointPath,
-      std::string(const ContainerID& containerId));
-
   process::Owned<slave::Launcher> real;
 };