Repository: mesos
Updated Branches:
  refs/heads/master e048c22a6 -> eb3d11bc8


Added a `ns::supported` convenience API.

Added a `ns::supported` convenience API which directly expresses the
intent to probe whether a specific Linux namespace is supported on
the running kernel and takes care of kernel versioning special cases.

This API subsumes the previous usages of `ns::namespaces`, so that API
is now used only retained as an internal helper.

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


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

Branch: refs/heads/master
Commit: eb3d11bc89be941cef14c341615253adc40ab1e6
Parents: e048c22
Author: James Peach <jpe...@apache.org>
Authored: Fri Feb 23 20:10:11 2018 -0800
Committer: James Peach <jpe...@apache.org>
Committed: Fri Feb 23 20:10:11 2018 -0800

----------------------------------------------------------------------
 src/linux/ns.cpp                                | 74 +++++++++++++++++++-
 src/linux/ns.hpp                                | 15 +++-
 .../mesos/isolators/namespaces/ipc.cpp          |  3 +-
 .../mesos/isolators/namespaces/pid.cpp          |  3 +-
 .../mesos/isolators/network/port_mapping.cpp    |  3 +-
 src/tests/containerizer/ns_tests.cpp            | 64 ++++++++++++-----
 src/tests/containerizer/setns_test_helper.cpp   | 13 ++--
 7 files changed, 143 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/linux/ns.cpp
----------------------------------------------------------------------
diff --git a/src/linux/ns.cpp b/src/linux/ns.cpp
index 49f8fca..64722c7 100644
--- a/src/linux/ns.cpp
+++ b/src/linux/ns.cpp
@@ -39,6 +39,7 @@
 #include <stout/stringify.hpp>
 #include <stout/strings.hpp>
 #include <stout/try.hpp>
+#include <stout/version.hpp>
 
 #include <stout/os/exists.hpp>
 #include <stout/os/ls.hpp>
@@ -51,6 +52,26 @@ using std::vector;
 
 namespace ns {
 
+static Try<Version> kernelVersion()
+{
+  Try<os::UTSInfo> uname = os::uname();
+  if (!uname.isSome()) {
+    return Error("Unable to determine kernel version: " + uname.error());
+  }
+
+  vector<string> parts = strings::split(uname->release, ".");
+  parts.resize(2);
+
+  Try<Version> version = Version::parse(strings::join(".", parts));
+  if (!version.isSome()) {
+    return Error("Failed to parse kernel version '" + uname->release +
+        "': " + version.error());
+  }
+
+  return version;
+}
+
+
 Try<int> nstype(const string& ns)
 {
   const hashmap<string, int> nstypes = {
@@ -73,7 +94,32 @@ Try<int> nstype(const string& ns)
 }
 
 
-set<string> namespaces()
+Try<string> nsname(int nsType)
+{
+  const hashmap<int, string> nsnames = {
+    {CLONE_NEWNS, "mnt"},
+    {CLONE_NEWUTS, "uts"},
+    {CLONE_NEWIPC, "ipc"},
+    {CLONE_NEWNET, "net"},
+    {CLONE_NEWUSER, "user"},
+    {CLONE_NEWPID, "pid"},
+    {CLONE_NEWCGROUP, "cgroup"}
+  };
+
+  Option<string> nsname = nsnames.get(nsType);
+
+  if (nsname.isNone()) {
+    return Error("Unknown namespace");
+  }
+
+  return nsname.get();
+}
+
+
+// TODO(jpeach): As we move namespace parameters from strings to CLONE
+// constants, we should be able to eventually remove the internal uses
+// of this function.
+static set<string> namespaces()
 {
   set<string> result;
 
@@ -107,6 +153,32 @@ set<int> nstypes()
 }
 
 
+Try<bool> supported(int nsTypes)
+{
+  int supported = 0;
+
+  foreach (const int n, nstypes()) {
+    if (nsTypes & n) {
+      supported |= n;
+    }
+  }
+
+  if ((nsTypes & CLONE_NEWUSER) && (supported & CLONE_NEWUSER)) {
+    Try<Version> version = kernelVersion();
+
+    if (version.isError()) {
+      return Error(version.error());
+    }
+
+    if (version.get() < Version(3, 12, 0)) {
+      return false;
+    }
+  }
+
+  return supported == nsTypes;
+}
+
+
 Try<Nothing> setns(
     const string& path,
     const string& ns,

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/linux/ns.hpp
----------------------------------------------------------------------
diff --git a/src/linux/ns.hpp b/src/linux/ns.hpp
index 8b64067..0b4136b 100644
--- a/src/linux/ns.hpp
+++ b/src/linux/ns.hpp
@@ -85,14 +85,23 @@ namespace ns {
 Try<int> nstype(const std::string& ns);
 
 
-// Returns all the supported namespaces by the kernel.
-std::set<std::string> namespaces();
+// Given a single CLONE_NEW* constant, return the corresponding namespace
+// name. This is the inverse of ns::nstype().
+Try<std::string> nsname(int nsType);
 
 
-// Returns all the supported namespaces by the kernel.
+// Returns all the configured kernel namespaces.
 std::set<int> nstypes();
 
 
+// Returns true if all the given CLONE_NEW* constants are supported
+// in the running kernel. If CLONE_NEWUSER is specified, the kernel
+// version must be at least 3.12.0 since prior to that version, major
+// kernel subsystems (e.g. XFS) did not implement user namespace
+// support. See also user_namespaces(7).
+Try<bool> supported(int nsTypes);
+
+
 // Re-associate the calling process with the specified namespace. The
 // path refers to one of the corresponding namespace entries in the
 // /proc/[pid]/ns/ directory (or bind mounted elsewhere). We do not

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
b/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
index 90773d7..6c8e8ee 100644
--- a/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
+++ b/src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
@@ -39,7 +39,8 @@ Try<Isolator*> NamespacesIPCIsolatorProcess::create(const 
Flags& flags)
   }
 
   // Verify that IPC namespaces are available on this kernel.
-  if (ns::namespaces().count("ipc") == 0) {
+  Try<bool> ipcSupported = ns::supported(CLONE_NEWIPC);
+  if (ipcSupported.isError() || !ipcSupported.get()) {
     return Error("IPC namespaces are not supported by this kernel");
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
b/src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
index d765929..73e0963 100644
--- a/src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
+++ b/src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
@@ -46,7 +46,8 @@ Try<Isolator*> NamespacesPidIsolatorProcess::create(const 
Flags& flags)
   }
 
   // Verify that pid namespaces are available on this kernel.
-  if (ns::namespaces().count("pid") == 0) {
+  Try<bool> pidSupported = ns::supported(CLONE_NEWPID);
+  if (pidSupported.isError() || !pidSupported.get()) {
     return Error("Pid namespaces are not supported by this kernel");
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
index d60052e..96e1a2b 100644
--- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
@@ -1366,7 +1366,8 @@ Try<Isolator*> PortMappingIsolatorProcess::create(const 
Flags& flags)
 
   // Verify that the network namespace is available by checking the
   // existence of the network namespace handle of the current process.
-  if (ns::namespaces().count("net") == 0) {
+  Try<bool> netSupported = ns::supported(CLONE_NEWNET);
+  if (netSupported.isError() || !netSupported.get()) {
     return Error(
         "Using network isolator requires network namespace. "
         "Make sure your kernel is newer than 3.4");

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/tests/containerizer/ns_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/ns_tests.cpp 
b/src/tests/containerizer/ns_tests.cpp
index 61adf85..76bd2ca 100644
--- a/src/tests/containerizer/ns_tests.cpp
+++ b/src/tests/containerizer/ns_tests.cpp
@@ -59,25 +59,21 @@ TEST(NsTest, ROOT_setns)
 {
   // Clone then exec the setns-test-helper into a new namespace for
   // each available namespace.
-  set<string> namespaces = ns::namespaces();
-  ASSERT_FALSE(namespaces.empty());
-
   int flags = 0;
 
-  foreach (const string& ns, namespaces) {
+  foreach (int nsType, ns::nstypes()) {
     // Skip 'user' namespace because it causes 'clone' to change us
     // from being user 'root' to user 'nobody', but these tests
     // require root. See MESOS-3083.
-    if (ns == "user") {
+    if (nsType == CLONE_NEWUSER) {
       continue;
     }
 
-    Try<int> nstype = ns::nstype(ns);
-    ASSERT_SOME(nstype);
-
-    flags |= nstype.get();
+    flags |= nsType;
   }
 
+  ASSERT_NE(0, flags);
+
   vector<string> argv;
   argv.push_back("test-helper");
   argv.push_back(SetnsTestHelper::NAME);
@@ -102,11 +98,44 @@ TEST(NsTest, ROOT_setns)
 }
 
 
+// Test the ns::supported() API.
+TEST(NsTest, SupportedNamespaces)
+{
+  set<int> namespaces = ns::nstypes();
+  ASSERT_FALSE(namespaces.empty());
+
+  int allNamespaces = 0;
+
+  // SIG* constants are guaranteed to not collide with genuine CLONE_NEW*
+  // constants, so we can use them to test the negative case.
+  EXPECT_SOME_FALSE(ns::supported(SIGCHLD));
+
+  foreach (const int& n, namespaces) {
+    // Exclude user namespaces because they depend on the kernel version.
+    if (n == CLONE_NEWNS) {
+      continue;
+    }
+
+    EXPECT_SOME_TRUE(ns::supported(n)) << ns::stringify(n);
+    allNamespaces |= n;
+  }
+
+  // Verify that ns::supported() correctly handles a bitmask.
+  EXPECT_SOME_TRUE(ns::supported(allNamespaces))
+    << ns::stringify(allNamespaces);
+
+  // Verify that the user namespace special casing matches the generic probe.
+  if (ns::supported(CLONE_NEWUSER).get()) {
+    EXPECT_EQ(1u, namespaces.count(CLONE_NEWUSER));
+  }
+}
+
+
 // Test that setns correctly refuses to re-associate to a namespace if
 // the caller is multi-threaded.
 TEST(NsTest, ROOT_setnsMultipleThreads)
 {
-  set<string> namespaces = ns::namespaces();
+  set<int> namespaces = ns::nstypes();
   EXPECT_LT(0u, namespaces.size());
 
   Latch* latch = new Latch();
@@ -117,8 +146,8 @@ TEST(NsTest, ROOT_setnsMultipleThreads)
     latch->await();
   });
 
-  foreach (const string& ns, namespaces) {
-    EXPECT_ERROR(ns::setns(::getpid(), ns));
+  foreach (int nsType, namespaces) {
+    EXPECT_ERROR(ns::setns(::getpid(), ns::nsname(nsType).get()));
   }
 
   // Terminate the thread.
@@ -142,14 +171,14 @@ static int childGetns()
 // Test that we can get the namespace inodes for a forked child.
 TEST(NsTest, ROOT_getns)
 {
-  set<string> namespaces = ns::namespaces();
+  set<int> namespaces = ns::nstypes();
 
   // ns::setns() does not support "pid".
-  namespaces.erase("pid");
+  namespaces.erase(CLONE_NEWPID);
 
   // Use the first other namespace available.
   ASSERT_FALSE(namespaces.empty());
-  string ns = *(namespaces.begin());
+  string ns = ns::nsname(*(namespaces.begin())).get();
 
   ASSERT_SOME(ns::getns(::getpid(), ns));
 
@@ -211,12 +240,13 @@ TEST(NsTest, ROOT_clone)
 
   ASSERT_SOME(child);
 
-  foreach (const string& ns, ns::namespaces()) {
+  foreach (int nsType, ns::nstypes()) {
     // See comment above as to why we're skipping the namespace.
-    if (ns == "user") {
+    if (nsType == CLONE_NEWUSER) {
       continue;
     }
 
+    const string ns = ns::nsname(nsType).get();
     Result<ino_t> inode = ns::getns(parent, ns);
     ASSERT_SOME(inode);
     EXPECT_SOME_NE(inode.get(), ns::getns(getpid(), ns));

http://git-wip-us.apache.org/repos/asf/mesos/blob/eb3d11bc/src/tests/containerizer/setns_test_helper.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/setns_test_helper.cpp 
b/src/tests/containerizer/setns_test_helper.cpp
index 045caf6..c5ac2d0 100644
--- a/src/tests/containerizer/setns_test_helper.cpp
+++ b/src/tests/containerizer/setns_test_helper.cpp
@@ -37,25 +37,22 @@ const char SetnsTestHelper::NAME[] = "Setns";
 
 int SetnsTestHelper::execute()
 {
-  // Get all the available namespaces.
-  set<string> namespaces = ns::namespaces();
-
   // Note: /proc has not been remounted so we can look up pid 1's
   // namespaces, even if we're in a separate pid namespace.
-  foreach (const string& ns, namespaces) {
-    if (ns == "pid") {
+  foreach (int nsType, ns::nstypes()) {
+    if (nsType == CLONE_NEWPID) {
       // ns::setns() does not (currently) support pid namespaces so
       // this should return an error.
-      Try<Nothing> setns = ns::setns(1, ns);
+      Try<Nothing> setns = ns::setns(1, ns::nsname(nsType).get());
       if (!setns.isError()) {
         return 1;
       }
-    } else if (ns == "user") {
+    } else if (nsType == CLONE_NEWUSER) {
       // ns::setns() will also fail with user namespaces, so we skip
       // for now. See MESOS-3083.
       continue;
     } else {
-      Try<Nothing> setns = ns::setns(1, ns);
+      Try<Nothing> setns = ns::setns(1, ns::nsname(nsType).get());
       if (!setns.isSome()) {
         return 1;
       }

Reply via email to