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);