Repository: mesos
Updated Branches:
  refs/heads/master 203a5544e -> b89667ca9


Set slave subsystems only in containerizer tests and cleaned up the
code using cgroups::enabled().

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


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

Branch: refs/heads/master
Commit: b89667ca993fc6263b0d28cf61b76004be94a70b
Parents: 203a554
Author: Jie Yu <yujie....@gmail.com>
Authored: Fri May 16 14:35:43 2014 -0700
Committer: Jie Yu <yujie....@gmail.com>
Committed: Mon May 19 10:01:13 2014 -0700

----------------------------------------------------------------------
 src/tests/allocator_tests.cpp       | 48 ++------------------------------
 src/tests/environment.cpp           |  4 +--
 src/tests/master_tests.cpp          |  5 +---
 src/tests/mesos.cpp                 | 20 ++++++-------
 src/tests/resource_offers_tests.cpp |  6 +---
 src/tests/slave_recovery_tests.cpp  | 20 +++++++------
 6 files changed, 26 insertions(+), 77 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/allocator_tests.cpp b/src/tests/allocator_tests.cpp
index 6b7b097..79ea09c 100644
--- a/src/tests/allocator_tests.cpp
+++ b/src/tests/allocator_tests.cpp
@@ -65,22 +65,7 @@ using testing::Eq;
 using testing::SaveArg;
 
 
-class DRFAllocatorTest : public MesosTest
-{
-public:
-  virtual slave::Flags CreateSlaveFlags()
-  {
-    slave::Flags flags = MesosTest::CreateSlaveFlags();
-
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because many of the allocator tests
-  // use multiple slaves.
-  flags.slave_subsystems = None();
-#endif // __linux
-
-  return flags;
-  }
-};
+class DRFAllocatorTest : public MesosTest {};
 
 
 // Checks that the DRF allocator implements the DRF algorithm
@@ -333,22 +318,7 @@ TEST_F(DRFAllocatorTest, DRFAllocatorProcess)
 }
 
 
-class ReservationAllocatorTest : public MesosTest
-{
-public:
-  virtual slave::Flags CreateSlaveFlags()
-  {
-    slave::Flags flags = MesosTest::CreateSlaveFlags();
-
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because many of the allocator tests
-  // use multiple slaves.
-  flags.slave_subsystems = None();
-#endif // __linux
-
-  return flags;
-  }
-};
+class ReservationAllocatorTest : public MesosTest {};
 
 
 // Checks that resources on a slave that are statically reserved to
@@ -667,20 +637,6 @@ template <typename T>
 class AllocatorTest : public MesosTest
 {
 protected:
-  virtual slave::Flags CreateSlaveFlags()
-  {
-    slave::Flags flags = MesosTest::CreateSlaveFlags();
-
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because many of the allocator tests
-  // use multiple slaves.
-  flags.slave_subsystems = None();
-#endif // __linux
-
-  return flags;
-  }
-
-
   virtual void SetUp()
   {
     MesosTest::SetUp();

http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/environment.cpp
----------------------------------------------------------------------
diff --git a/src/tests/environment.cpp b/src/tests/environment.cpp
index 1267b3e..3f4eaad 100644
--- a/src/tests/environment.cpp
+++ b/src/tests/environment.cpp
@@ -81,7 +81,7 @@ static bool enable(const ::testing::TestInfo& test)
       return false;
     }
 
-    if (strings::contains(name, "CGROUPS_") && !os::exists("/proc/cgroups")) {
+    if (strings::contains(name, "CGROUPS_") && !cgroups::enabled()) {
       return false;
     }
 
@@ -129,7 +129,7 @@ static bool enable(const ::testing::TestInfo& test)
   if (test.type_param() != NULL) {
     const string& type = test.type_param();
     if (strings::contains(type, "Cgroups") &&
-        (os::user() != "root" || !os::exists("/proc/cgroups"))) {
+        (os::user() != "root" || !cgroups::enabled())) {
       return false;
     }
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 87427d1..1ea1da6 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -1244,10 +1244,7 @@ TEST_F(MasterTest, LaunchAcrossSlavesTest)
   Resources twoSlaves = fullSlave + fullSlave;
 
   slave::Flags flags = CreateSlaveFlags();
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because this is a multi-slave test.
-  flags.slave_subsystems = None();
-#endif // __linux
+
   flags.resources = Option<string>(stringify(fullSlave));
 
   Try<PID<Slave> > slave1 = StartSlave(&containerizer, flags);

http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 317b5a2..7e5e96a 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -145,13 +145,6 @@ slave::Flags MesosTest::CreateSlaveFlags()
 
   flags.registration_backoff_factor = Milliseconds(10);
 
-#ifdef __linux__
-  // Enable putting the slave into memory and cpuacct cgroups.
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
-    flags.slave_subsystems = "memory,cpuacct";
-  }
-#endif // __linux__
-
   return flags;
 }
 
@@ -327,10 +320,13 @@ slave::Flags 
ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
 #ifdef __linux__
   // Use cgroup isolators if they're available and we're root.
   // TODO(idownes): Refactor the cgroups/non-cgroups code.
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
+  if (cgroups::enabled() && os::user() == "root") {
     flags.isolation = "cgroups/cpu,cgroups/mem";
     flags.cgroups_hierarchy = baseHierarchy;
     flags.cgroups_root = TEST_CGROUPS_ROOT + "_" + UUID::random().toString();
+
+    // Enable putting the slave into memory and cpuacct cgroups.
+    flags.slave_subsystems = "memory,cpuacct";
   } else {
     flags.isolation = "posix/cpu,posix/mem";
   }
@@ -345,7 +341,7 @@ slave::Flags 
ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
 #ifdef __linux__
 void ContainerizerTest<slave::MesosContainerizer>::SetUpTestCase()
 {
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
+  if (cgroups::enabled() && os::user() == "root") {
     // Clean up any testing hierarchies.
     Try<std::set<string> > hierarchies = cgroups::hierarchies();
     ASSERT_SOME(hierarchies);
@@ -360,7 +356,7 @@ void 
ContainerizerTest<slave::MesosContainerizer>::SetUpTestCase()
 
 void ContainerizerTest<slave::MesosContainerizer>::TearDownTestCase()
 {
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
+  if (cgroups::enabled() && os::user() == "root") {
     // Clean up any testing hierarchies.
     Try<std::set<string> > hierarchies = cgroups::hierarchies();
     ASSERT_SOME(hierarchies);
@@ -382,7 +378,7 @@ void ContainerizerTest<slave::MesosContainerizer>::SetUp()
   subsystems.insert("memory");
   subsystems.insert("freezer");
 
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
+  if (cgroups::enabled() && os::user() == "root") {
     foreach (const string& subsystem, subsystems) {
       // Establish the base hierarchy if this is the first subsystem checked.
       if (baseHierarchy.empty()) {
@@ -428,7 +424,7 @@ void 
ContainerizerTest<slave::MesosContainerizer>::TearDown()
 {
   MesosTest::TearDown();
 
-  if (os::exists("/proc/cgroups") && os::user() == "root") {
+  if (cgroups::enabled() && os::user() == "root") {
     foreach (const string& subsystem, subsystems) {
       string hierarchy = path::join(baseHierarchy, subsystem);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/resource_offers_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resource_offers_tests.cpp 
b/src/tests/resource_offers_tests.cpp
index 56682de..653f72d 100644
--- a/src/tests/resource_offers_tests.cpp
+++ b/src/tests/resource_offers_tests.cpp
@@ -64,12 +64,8 @@ TEST_F(ResourceOffersTest, ResourceOfferWithMultipleSlaves)
   for (int i = 0; i < 10; i++) {
     slave::Flags flags = CreateSlaveFlags();
 
-#ifdef __linux__
-    // Disable putting slave into cgroup(s) because this is a multi-slave test.
-    flags.slave_subsystems = None();
-#endif // __linux
-
     flags.resources = Option<std::string>("cpus:2;mem:1024");
+
     Try<PID<Slave> > slave = StartSlave(flags);
     ASSERT_SOME(slave);
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/b89667ca/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp 
b/src/tests/slave_recovery_tests.cpp
index a6262e8..11a1430 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -2857,10 +2857,12 @@ TYPED_TEST(SlaveRecoveryTest, MultipleSlaves)
 
   // Start the first slave.
   slave::Flags flags1 = this->CreateSlaveFlags();
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because this is a multi-slave test.
-  flags1.slave_subsystems = None();
-#endif // __linux
+
+  if (flags1.slave_subsystems.isSome()) {
+    // Disable putting slave into cgroup(s) because this is a multi-slave test.
+    flags1.slave_subsystems = None();
+  }
+
   Try<TypeParam*> containerizer1 = TypeParam::create(flags1, true);
   ASSERT_SOME(containerizer1);
 
@@ -2892,10 +2894,12 @@ TYPED_TEST(SlaveRecoveryTest, MultipleSlaves)
 
   // Start the second slave.
   slave::Flags flags2 = this->CreateSlaveFlags();
-#ifdef __linux__
-  // Disable putting slave into cgroup(s) because this is a multi-slave test.
-  flags2.slave_subsystems = "";
-#endif // __linux
+
+  if (flags2.slave_subsystems.isSome()) {
+    // Disable putting slave into cgroup(s) because this is a multi-slave test.
+    flags2.slave_subsystems = None();
+  }
+
   Try<TypeParam*> containerizer2 = TypeParam::create(flags2, true);
   ASSERT_SOME(containerizer2);
 

Reply via email to