You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2017/10/20 22:16:01 UTC

[1/3] mesos git commit: Refactored installing rlimits in the container launch.

Repository: mesos
Updated Branches:
  refs/heads/master 549fa6145 -> a85a22baa


Refactored installing rlimits in the container launch.

Hoisted the platform-dependent rlimit installation into a helper
function to improve readability. Improved the error message to
specify which rlimit we failed to set.

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


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

Branch: refs/heads/master
Commit: 6a411962782208098ac71f2433b62d6d183c927c
Parents: 549fa61
Author: James Peach <jp...@apache.org>
Authored: Fri Oct 20 14:56:15 2017 -0700
Committer: James Peach <jp...@apache.org>
Committed: Fri Oct 20 14:56:15 2017 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 40 +++++++++++++++++----------
 1 file changed, 26 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6a411962/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index d137298..8deb7fd 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -236,6 +236,26 @@ static void exitWithStatus(int status)
 }
 
 
+static Try<Nothing> installResourceLimits(const RLimitInfo& limits)
+{
+#ifdef __WINDOWS__
+  return Error("Rlimits are not supported on Windows");
+#else
+  foreach (const RLimitInfo::RLimit& limit, limits.rlimits()) {
+    Try<Nothing> set = rlimits::set(limit);
+    if (set.isError()) {
+      return Error(
+          "Failed to set " +
+          RLimitInfo::RLimit::Type_Name(limit.type()) + " limit: " +
+          set.error());
+    }
+  }
+
+  return Nothing();
+#endif // __WINDOWS__
+}
+
+
 int MesosContainerizerLaunch::execute()
 {
   if (flags.help) {
@@ -552,23 +572,15 @@ int MesosContainerizerLaunch::execute()
   }
 #endif // __WINDOWS__
 
-#ifndef __WINDOWS__
-  // Setting resource limits for the process.
+  // Install resource limits for the process.
   if (launchInfo.has_rlimits()) {
-    foreach (const RLimitInfo::RLimit& limit, launchInfo.rlimits().rlimits()) {
-      Try<Nothing> set = rlimits::set(limit);
-      if (set.isError()) {
-        cerr << "Failed to set rlimit: " << set.error() << endl;
-        exitWithStatus(EXIT_FAILURE);
-      }
+    Try<Nothing> set = installResourceLimits(launchInfo.rlimits());
+
+    if (set.isError()) {
+      cerr << set.error() << endl;
+      exitWithStatus(EXIT_FAILURE);
     }
   }
-#else
-  if (launchInfo.has_rlimits()) {
-    cerr << "Rlimits are not supported on Windows" << endl;
-    exitWithStatus(EXIT_FAILURE);
-  }
-#endif // __WINDOWS__
 
   if (launchInfo.has_working_directory()) {
     // If working directory does not exist (e.g., being removed from


[3/3] mesos git commit: Refactored entering the chroot in the container launch.

Posted by jp...@apache.org.
Refactored entering the chroot in the container launch.

Hoisted the platform-dependent chroot code out into a helper function
to improve readability. Improved the error message to always include
the rootfs we attempted to enter.

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


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

Branch: refs/heads/master
Commit: a85a22baa32f66ecaa13c4602a195d57f6abf9be
Parents: 45c5079
Author: James Peach <jp...@apache.org>
Authored: Fri Oct 20 14:57:55 2017 -0700
Committer: James Peach <jp...@apache.org>
Committed: Fri Oct 20 14:57:55 2017 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 66 +++++++++++++++------------
 1 file changed, 37 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a85a22ba/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index fc5bd3d..49f11f1 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -256,6 +256,41 @@ static Try<Nothing> installResourceLimits(const RLimitInfo& limits)
 }
 
 
+static Try<Nothing> enterChroot(const string& rootfs)
+{
+#ifdef __WINDOWS__
+  return Error("Changing rootfs is not supported on Windows");
+#else
+  // Verify that rootfs is an absolute path.
+  Result<string> realpath = os::realpath(rootfs);
+  if (realpath.isError()) {
+    return Error(
+        "Failed to determine if rootfs '" + rootfs +
+        "' is an absolute path: " + realpath.error());
+  } else if (realpath.isNone()) {
+    return Error("Rootfs path '" + rootfs + "' does not exist");
+  } else if (realpath.get() != rootfs) {
+    return Error("Rootfs path '" + rootfs + "' is not an absolute path");
+  }
+
+#ifdef __linux__
+  Try<Nothing> chroot = fs::chroot::enter(rootfs);
+#else
+  // For any other platform we'll just use POSIX chroot.
+  Try<Nothing> chroot = os::chroot(rootfs);
+#endif // __linux__
+
+  if (chroot.isError()) {
+    return Error(
+        "Failed to enter chroot '" + rootfs + "': " +
+        chroot.error());
+  }
+
+  return Nothing();
+#endif // __WINDOWS__
+}
+
+
 int MesosContainerizerLaunch::execute()
 {
   if (flags.help) {
@@ -533,44 +568,17 @@ int MesosContainerizerLaunch::execute()
   }
 #endif // __linux__
 
-#ifndef __WINDOWS__
   // Change root to a new root, if provided.
   if (launchInfo.has_rootfs()) {
     cout << "Changing root to " << launchInfo.rootfs() << endl;
 
-    // Verify that rootfs is an absolute path.
-    Result<string> realpath = os::realpath(launchInfo.rootfs());
-    if (realpath.isError()) {
-      cerr << "Failed to determine if rootfs is an absolute path: "
-           << realpath.error() << endl;
-      exitWithStatus(EXIT_FAILURE);
-    } else if (realpath.isNone()) {
-      cerr << "Rootfs path does not exist" << endl;
-      exitWithStatus(EXIT_FAILURE);
-    } else if (realpath.get() != launchInfo.rootfs()) {
-      cerr << "Rootfs path is not an absolute path" << endl;
-      exitWithStatus(EXIT_FAILURE);
-    }
-
-#ifdef __linux__
-    Try<Nothing> chroot = fs::chroot::enter(launchInfo.rootfs());
-#else
-    // For any other platform we'll just use POSIX chroot.
-    Try<Nothing> chroot = os::chroot(launchInfo.rootfs());
-#endif // __linux__
+    Try<Nothing> chroot = enterChroot(launchInfo.rootfs());
 
     if (chroot.isError()) {
-      cerr << "Failed to enter chroot '" << launchInfo.rootfs()
-           << "': " << chroot.error();
+      cerr << chroot.error() << endl;
       exitWithStatus(EXIT_FAILURE);
     }
   }
-#else
-  if (launchInfo.has_rootfs()) {
-    cerr << "Changing rootfs is not supported on Windows" << endl;
-    exitWithStatus(EXIT_FAILURE);
-  }
-#endif // __WINDOWS__
 
   // Install resource limits for the process.
   if (launchInfo.has_rlimits()) {


[2/3] mesos git commit: Moved the search for the containerizer executable launch path.

Posted by jp...@apache.org.
Moved the search for the containerizer executable launch path.

Improved code readability by hoisting the code that searches for
the containerizer executable launch path up to be near where the
executable path is first determined.

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


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

Branch: refs/heads/master
Commit: 45c50792f609beebc34b67a583407bcf187eee64
Parents: 6a41196
Author: James Peach <jp...@apache.org>
Authored: Fri Oct 20 14:57:52 2017 -0700
Committer: James Peach <jp...@apache.org>
Committed: Fri Oct 20 14:57:52 2017 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 36 +++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/45c50792/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 8deb7fd..fc5bd3d 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -709,6 +709,24 @@ int MesosContainerizerLaunch::execute()
     ? os::Shell::name
     : launchInfo.command().value().c_str());
 
+#ifndef __WINDOWS__
+  // Search executable in the current working directory as well.
+  // execvpe and execvp will only search executable from the current
+  // working directory if environment variable PATH is not set.
+  // TODO(aaron.wood): 'os::which' current does not work on Windows.
+  // Remove the ifndef guard once it's supported on Windows.
+  if (!path::absolute(executable) &&
+      launchInfo.has_working_directory()) {
+    Option<string> which = os::which(
+        executable,
+        launchInfo.working_directory());
+
+    if (which.isSome()) {
+      executable = which.get();
+    }
+  }
+#endif // __WINDOWS__
+
   os::raw::Argv argv(launchInfo.command().shell()
     ? vector<string>({
           os::Shell::arg0,
@@ -837,24 +855,6 @@ int MesosContainerizerLaunch::execute()
   }
 #endif // __WINDOWS__
 
-#ifndef __WINDOWS__
-  // Search executable in the current working directory as well.
-  // execvpe and execvp will only search executable from the current
-  // working directory if environment variable PATH is not set.
-  // TODO(aaron.wood): 'os::which' current does not work on Windows.
-  // Remove the ifndef guard once it's supported on Windows.
-  if (!path::absolute(executable) &&
-      launchInfo.has_working_directory()) {
-    Option<string> which = os::which(
-        executable,
-        launchInfo.working_directory());
-
-    if (which.isSome()) {
-      executable = which.get();
-    }
-  }
-#endif // __WINDOWS__
-
   if (envp.isSome()) {
     os::execvpe(executable.c_str(), argv, envp.get());
   } else {