[1/2] mesos git commit: Added tests for cgroups devices subsystem.

[bm...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 4c81f29b4 -> 50b904033


Added tests for cgroups devices subsystem.

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


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

Branch: refs/heads/master
Commit: 50b904033ce495ad6e1375c17ec3faf2bea77091
Parents: e716189
Author: Abhishek Dasgupta <a1...@linux.vnet.ibm.com>
Authored: Mon Mar 28 18:26:35 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Mar 28 19:00:37 2016 -0700

----------------------------------------------------------------------
 src/tests/containerizer/cgroups_tests.cpp | 142 +++++++++++++++++++++++++
 1 file changed, 142 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/50b90403/src/tests/containerizer/cgroups_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/cgroups_tests.cpp b/src/tests/containerizer/cgroups_tests.cpp
index b29ce09..ecd83c7 100644
--- a/src/tests/containerizer/cgroups_tests.cpp
+++ b/src/tests/containerizer/cgroups_tests.cpp
@@ -218,6 +218,15 @@ public:
 };
 
 
+class CgroupsAnyHierarchyDevicesTest
+  : public CgroupsAnyHierarchyTest
+{
+public:
+  CgroupsAnyHierarchyDevicesTest()
+    : CgroupsAnyHierarchyTest("devices") {}
+};
+
+
 TEST_F(CgroupsAnyHierarchyTest, ROOT_CGROUPS_Enabled)
 {
   EXPECT_SOME_TRUE(cgroups::enabled(""));
@@ -1268,6 +1277,139 @@ TEST_F(CgroupsAnyHierarchyWithCpuAcctMemoryTest, ROOT_CGROUPS_CpuAcctsStats)
   AWAIT_READY(cgroups::destroy(hierarchy, TEST_CGROUPS_ROOT));
 }
 
+
+// TODO(klueska): Ideally we would call this test CgroupsDevicesTest,
+// but currently we filter tests on the keyword 'Cgroup' and only run
+// them if we have root access. However, this test doesn't require
+// root access since it's just testing the parsing aspects of the
+// cgroups devices whitelist. If we ever modify this filter to be less
+// restrictive, we should rename this test accordingly.
+TEST(DevicesTest, Parse)
+{
+  EXPECT_ERROR(cgroups::devices::Entry::parse(""));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("x"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b x"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b *:*"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b *:* x"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b *:* rwmx"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b *:* rwm x"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b x:x rwm"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b *:x rwm"));
+  EXPECT_ERROR(cgroups::devices::Entry::parse("b x:* rwm"));
+
+  Try<cgroups::devices::Entry> entry =
+    cgroups::devices::Entry::parse("a");
+  EXPECT_EQ("a *:* rwm", stringify(entry.get()));
+
+  // Note that if the first character is "a", the rest
+  // of the input is ignored, even if it is invalid!
+  entry = cgroups::devices::Entry::parse("a x x");
+  EXPECT_EQ("a *:* rwm", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b *:* rwm");
+  EXPECT_EQ("b *:* rwm", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b *:* r");
+  EXPECT_EQ("b *:* r", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b *:* w");
+  EXPECT_EQ("b *:* w", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b *:* m");
+  EXPECT_EQ("b *:* m", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b 1:* rwm");
+  EXPECT_EQ("b 1:* rwm", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b *:3 rwm");
+  EXPECT_EQ("b *:3 rwm", stringify(entry.get()));
+
+  entry = cgroups::devices::Entry::parse("b 1:3 rwm");
+  EXPECT_EQ("b 1:3 rwm", stringify(entry.get()));
+}
+
+
+TEST_F(CgroupsAnyHierarchyDevicesTest, ROOT_CGROUPS_Devices)
+{
+  // Create a cgroup in the devices hierarchy.
+  string hierarchy = path::join(baseHierarchy, "devices");
+  ASSERT_SOME(cgroups::create(hierarchy, TEST_CGROUPS_ROOT));
+
+  // Assign ourselves to the cgroup.
+  CHECK_SOME(cgroups::assign(hierarchy, TEST_CGROUPS_ROOT, ::getpid()));
+
+  // When a devices cgroup is first created, its whitelist inherits
+  // all devices from its parent's whitelist (i.e., "a *:* rwm" by
+  // default). In theory, we should be able to add and remove devices
+  // from the whitelist by writing to the respective `devices.allow`
+  // and `devices.deny` files associated with the cgroup. However, the
+  // semantics of the whitelist are such that writing to the deny file
+  // will only remove entries in the whitelist that are explicitly
+  // listed in there (i.e., denying "b 1:3 rwm" when the whitelist
+  // only contains "a *:* rwm" will not modify the whitelist because
+  // "b 1:3 rwm" is not explicitly listed).  Although the whitelist
+  // doesn't change, access to the device is still denied as expected
+  // (there is just no way of querying the system to detect it).
+  // Because of this, we first deny access to all devices and
+  // selectively add some back in so we can control the entries in the
+  // whitelist explicitly.
+  Try<cgroups::devices::Entry> entry =
+    cgroups::devices::Entry::parse("a *:* rwm");
+
+  ASSERT_SOME(entry);
+
+  Try<Nothing> deny = cgroups::devices::deny(
+      hierarchy,
+      TEST_CGROUPS_ROOT,
+      entry.get());
+
+  EXPECT_SOME(deny);
+
+  // Verify that there are no entries in the whitelist.
+  Try<vector<cgroups::devices::Entry>> whitelist =
+      cgroups::devices::list(hierarchy, TEST_CGROUPS_ROOT);
+
+  ASSERT_SOME(whitelist);
+
+  EXPECT_TRUE(whitelist->empty());
+
+  // Verify that we can't open /dev/null.
+  Try<int> fd = os::open("/dev/null", O_RDWR);
+  EXPECT_ERROR(fd);
+
+  // Add /dev/null to the list of allowed devices.
+  entry = cgroups::devices::Entry::parse("c 1:3 rwm");
+
+  ASSERT_SOME(entry);
+
+  Try<Nothing> allow = cgroups::devices::allow(
+      hierarchy,
+      TEST_CGROUPS_ROOT,
+      entry.get());
+
+  EXPECT_SOME(allow);
+
+  // Verify that /dev/null is in the whitelist.
+  whitelist = cgroups::devices::list(hierarchy, TEST_CGROUPS_ROOT);
+
+  ASSERT_SOME(whitelist);
+
+  ASSERT_EQ(1u, whitelist->size());
+
+  EXPECT_EQ(entry.get(), whitelist.get()[0]);
+
+  // Verify that we can now open and write to /dev/null.
+  fd = os::open("/dev/null", O_WRONLY);
+  ASSERT_SOME(fd);
+
+  Try<Nothing> write = os::write(fd.get(), "nonsense");
+  EXPECT_SOME(write);
+
+  // Move ourselves to the root cgroup.
+  CHECK_SOME(cgroups::assign(hierarchy, "", ::getpid()));
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {

[2/2] mesos git commit: Added devices subsystem support in cgroups library.

[bm...@apache.org]

Added devices subsystem support in cgroups library.

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


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

Branch: refs/heads/master
Commit: e7161896782f5e5ddc5d32e332b090b7bae833ae
Parents: 4c81f29
Author: Abhishek Dasgupta <a1...@linux.vnet.ibm.com>
Authored: Mon Mar 28 17:34:09 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Mon Mar 28 19:00:37 2016 -0700

----------------------------------------------------------------------
 src/linux/cgroups.cpp | 249 +++++++++++++++++++++++++++++++++++++++++++++
 src/linux/cgroups.hpp |  79 ++++++++++++++
 2 files changed, 328 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e7161896/src/linux/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index df18ed4..fd3df55 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -2418,6 +2418,255 @@ Future<uint64_t> Counter::value() const
 } // namespace memory {
 
 
+namespace devices {
+
+ostream& operator<<(ostream& stream, const Entry::Selector& selector)
+{
+  switch (selector.type) {
+    case Entry::Selector::Type::ALL:
+      stream << "a";
+      break;
+    case Entry::Selector::Type::BLOCK:
+      stream << "b";
+      break;
+    case Entry::Selector::Type::CHARACTER:
+      stream << "c";
+      break;
+    // We omit the default case because we assume -Wswitch
+    // will trigger a compile-time error if a case is missed.
+  }
+
+  stream << " ";
+
+  if (selector.major.isSome()) {
+    stream << stringify(selector.major.get());
+  } else {
+    stream << "*";
+  }
+
+  stream << ":";
+
+  if (selector.minor.isSome()) {
+    stream << stringify(selector.minor.get());
+  } else {
+    stream << "*";
+  }
+
+  return stream;
+}
+
+
+ostream& operator<<(ostream& stream, const Entry::Access& access)
+{
+  if (access.read) {
+    stream << "r";
+  }
+  if (access.write) {
+    stream << "w";
+  }
+  if (access.mknod) {
+    stream << "m";
+  }
+
+  return stream;
+}
+
+
+ostream& operator<<(ostream& stream, const Entry& ListEntry)
+{
+  return stream << ListEntry.selector << " " << ListEntry.access;
+}
+
+
+bool operator==(const Entry::Selector& left, const Entry::Selector& right)
+{
+  return left.type == right.type &&
+         left.minor == right.minor &&
+         left.major == right.major;
+}
+
+
+bool operator==(const Entry::Access& left, const Entry::Access& right)
+{
+  return left.read == right.read &&
+         left.write == right.write &&
+         left.mknod == right.mknod;
+}
+
+
+bool operator==(const Entry& left, const Entry& right)
+{
+  return left.selector == right.selector &&
+         left.access == right.access;
+}
+
+
+Try<Entry> Entry::parse(const string& s)
+{
+  vector<string> tokens = strings::tokenize(s, " ");
+
+  if (tokens.empty()) {
+    return Error("Invalid format");
+  }
+
+  Entry entry;
+
+  // Parse the device type.
+  // By default, when "a" is set as the device type
+  // all other fields in the device entry are ignored!
+  // (i.e. "a" implies a *:* rwm").
+  if (tokens[0] == "a") {
+    entry.selector.type = Selector::Type::ALL;
+    entry.selector.major = None();
+    entry.selector.minor = None();
+    entry.access.read = true;
+    entry.access.write = true;
+    entry.access.mknod = true;
+    return entry;
+  }
+
+  if (tokens.size() != 3) {
+    return Error("Invalid format");
+  }
+
+  // Other than for the "a" device, a correctly formed entry must
+  // contain exactly 3 space-separated tokens of the form:
+  // "{b,c} <major>:<minor> [r][w][m]".
+  if (tokens[0] == "b") {
+    entry.selector.type = Selector::Type::BLOCK;
+  } else if (tokens[0] == "c") {
+    entry.selector.type = Selector::Type::CHARACTER;
+  } else {
+    return Error("Invalid format");
+  }
+
+  // Parse the device major/minor numbers.
+  vector<string> deviceNumbers = strings::tokenize(tokens[1], ":");
+
+  if (deviceNumbers.size() != 2) {
+    return Error("Invalid format");
+  }
+
+  // The device major/minor numbers must either be "*"
+  // or an unsigned integer. A value of None() means "*".
+  entry.selector.major = None();
+  entry.selector.minor = None();
+
+  if (deviceNumbers[0] != "*") {
+    Try<dev_t> major = numify<dev_t>(deviceNumbers[0]);
+
+    if (major.isError()) {
+      return Error("Invalid format");
+    }
+
+    entry.selector.major = major.get();
+  }
+
+  if (deviceNumbers[1] != "*") {
+    Try<dev_t> minor = numify<dev_t>(deviceNumbers[1]);
+
+    if (minor.isError()) {
+      return Error("Invalid format");
+    }
+
+    entry.selector.minor = minor.get();
+  }
+
+  // Parse the access bits.
+  string permissions = tokens[2];
+
+  if (permissions.size() > 3) {
+    return Error("Invalid format");
+  }
+
+  entry.access.read = false;
+  entry.access.write = false;
+  entry.access.mknod = false;
+
+  foreach (char permission, permissions) {
+    if (permission == 'r') {
+      entry.access.read = true;
+    } else if (permission == 'w') {
+      entry.access.write = true;
+    } else if (permission == 'm') {
+      entry.access.mknod = true;
+    } else {
+      return Error("Invalid format");
+    }
+  }
+
+  return entry;
+}
+
+
+Try<vector<Entry>> list(
+    const string& hierarchy,
+    const string& cgroup)
+{
+  Try<string> read = cgroups::read(hierarchy, cgroup, "devices.list");
+
+  if (read.isError()) {
+    return Error("Failed to read from 'devices.list': " + read.error());
+  }
+
+  vector<Entry> entries;
+
+  foreach (const string& s, strings::tokenize(read.get(), "\n")) {
+    Try<Entry> entry = Entry::parse(s);
+
+    if (entry.isError()) {
+      return Error("Failed to parse device entry '" + s + "'"
+                   " from 'devices.list': " + entry.error());
+    }
+
+    entries.push_back(entry.get());
+  }
+
+  return entries;
+}
+
+
+Try<Nothing> allow(
+    const string& hierarchy,
+    const string& cgroup,
+    const Entry& entry)
+{
+  Try<Nothing> write = cgroups::write(
+     hierarchy,
+     cgroup,
+     "devices.allow",
+     stringify(entry));
+
+  if (write.isError()) {
+    return Error("Failed to write to 'devices.allow': " + write.error());
+  }
+
+  return Nothing();
+}
+
+
+Try<Nothing> deny(
+    const string& hierarchy,
+    const string& cgroup,
+    const Entry& entry)
+{
+  Try<Nothing> write = cgroups::write(
+     hierarchy,
+     cgroup,
+     "devices.deny",
+     stringify(entry));
+
+  if (write.isError()) {
+    return Error("Failed to write to 'devices.deny': " + write.error());
+  }
+
+  return Nothing();
+}
+
+
+} // namespace devices {
+
+
 namespace freezer {
 
 Future<Nothing> freeze(

http://git-wip-us.apache.org/repos/asf/mesos/blob/e7161896/src/linux/cgroups.hpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp
index 51ccefd..c040962 100644
--- a/src/linux/cgroups.hpp
+++ b/src/linux/cgroups.hpp
@@ -622,6 +622,85 @@ private:
 } // namespace memory {
 
 
+// Device controls.
+namespace devices {
+
+struct Entry
+{
+  static Try<Entry> parse(const std::string& s);
+
+  struct Selector
+  {
+    enum Type
+    {
+      ALL,
+      BLOCK,
+      CHARACTER,
+    };
+
+    Type type;
+    Option<dev_t> major; // Matches all `major` numbers if None.
+    Option<dev_t> minor; // Matches all `minor` numbers if None.
+  };
+
+  struct Access
+  {
+    bool read;
+    bool write;
+    bool mknod;
+  };
+
+  Selector selector;
+  Access access;
+};
+
+
+std::ostream& operator<<(
+    std::ostream& stream,
+    const Entry::Selector& selector);
+
+std::ostream& operator<<(
+    std::ostream& stream,
+    const Entry::Access& access);
+
+std::ostream& operator<<(
+    std::ostream& stream,
+    const Entry& ListEntry);
+
+
+bool operator==(
+    const Entry::Selector& left,
+    const Entry::Selector& right);
+
+bool operator==(
+    const Entry::Access& left,
+    const Entry::Access& right);
+
+bool operator==(
+    const Entry& left,
+    const Entry& right);
+
+
+// Returns the entries within devices.list.
+Try<std::vector<Entry>> list(
+    const std::string& hierarchy,
+    const std::string& cgroup);
+
+// Writes the provided `entry` into devices.allow.
+Try<Nothing> allow(
+    const std::string& hierarchy,
+    const std::string& cgroup,
+    const Entry& entry);
+
+// Writes the provided `entry` into devices.deny.
+Try<Nothing> deny(
+    const std::string& hierarchy,
+    const std::string& cgroup,
+    const Entry& entry);
+
+} // namespace devices {
+
+
 // Freezer controls.
 // The freezer can be in one of three states:
 // 1. THAWED   : No process in the cgroup is frozen.