Disabled support for setting quota on nested roles. Correct support for quota on nested roles will require further work in the allocator (see MESOS-7402 for details). For now, setting quota on nested roles is disabled until MESOS-7402 can be fixed. This commit disables any tests that rely on setting quota on nested roles; it also adds a (disabled) test to cover the behavior that will be fixed as part of MESOS-7402.
Review: https://reviews.apache.org/r/58584 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/50a71c03 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/50a71c03 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/50a71c03 Branch: refs/heads/master Commit: 50a71c03a374ef9ff834ab5dd3fe0a4f195cd380 Parents: cfd1b48 Author: Neil Conway <[email protected]> Authored: Wed Apr 19 16:28:43 2017 -0700 Committer: Neil Conway <[email protected]> Committed: Wed Apr 26 14:19:27 2017 -0400 ---------------------------------------------------------------------- src/master/quota_handler.cpp | 9 +++ src/tests/hierarchical_allocator_tests.cpp | 93 +++++++++++++++++++++++++ src/tests/master_quota_tests.cpp | 24 +++++-- 3 files changed, 120 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/50a71c03/src/master/quota_handler.cpp ---------------------------------------------------------------------- diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index 281fa1d..7fe5580 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -512,6 +512,15 @@ Future<http::Response> Master::QuotaHandler::_set( } } + // Setting quota on a nested role is temporarily disabled. + // + // TODO(neilc): Remove this check when MESOS-7402 is fixed. + bool nestedRole = strings::contains(quotaInfo.role(), "/"); + if (nestedRole) { + return BadRequest("Setting quota on nested role '" + + quotaInfo.role() + "' is not supported yet"); + } + // The force flag is used to overwrite the `capacityHeuristic` check. const bool forced = quotaRequest.force(); http://git-wip-us.apache.org/repos/asf/mesos/blob/50a71c03/src/tests/hierarchical_allocator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp index ad03e17..84bb6f3 100644 --- a/src/tests/hierarchical_allocator_tests.cpp +++ b/src/tests/hierarchical_allocator_tests.cpp @@ -4594,6 +4594,99 @@ TEST_F(HierarchicalAllocatorTest, NestedRoleQuotaAllocateToParent) } +// This test checks that when quota resources are allocated to a +// nested role, those resources are also counted against the quota of +// the parent role as well. +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(HierarchicalAllocatorTest, DISABLED_NestedQuotaAccounting) +{ + Clock::pause(); + + initialize(); + + const string PARENT_ROLE = "x/b"; + const string CHILD_ROLE = "x/b/c"; + const string NON_QUOTA_ROLE = "aaa"; + + // Create `framework1` in the non-quota role. + FrameworkInfo framework1 = createFrameworkInfo({NON_QUOTA_ROLE}); + allocator->addFramework(framework1.id(), framework1, {}, true); + + // Set quota for parent role. + const Quota parentQuota = createQuota(PARENT_ROLE, "cpus:3;mem:300"); + allocator->setQuota(PARENT_ROLE, parentQuota); + + // Create `framework2` in the parent role. + FrameworkInfo framework2 = createFrameworkInfo({PARENT_ROLE}); + allocator->addFramework(framework2.id(), framework2, {}, true); + + SlaveInfo agent1 = createSlaveInfo("cpus:2;mem:200"); + allocator->addSlave( + agent1.id(), + agent1, + AGENT_CAPABILITIES(), + None(), + agent1.resources(), + {}); + + // `framework2` will be offered all of the resources on `agent1`. + { + Allocation expected = Allocation( + framework2.id(), + {{PARENT_ROLE, {{agent1.id(), agent1.resources()}}}}); + + AWAIT_EXPECT_EQ(expected, allocations.get()); + } + + // Set quota for child role. + const Quota childQuota = createQuota(CHILD_ROLE, "cpus:1;mem:100"); + allocator->setQuota(CHILD_ROLE, childQuota); + + // Create `framework3` in the child role. + FrameworkInfo framework3 = createFrameworkInfo({CHILD_ROLE}); + allocator->addFramework(framework3.id(), framework3, {}, true); + + SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:100"); + allocator->addSlave( + agent2.id(), + agent2, + AGENT_CAPABILITIES(), + None(), + agent2.resources(), + {}); + + // `framework3` will be offered all of the resources on `agent2`. + { + Allocation expected = Allocation( + framework3.id(), + {{CHILD_ROLE, {{agent2.id(), agent2.resources()}}}}); + + AWAIT_EXPECT_EQ(expected, allocations.get()); + } + + SlaveInfo agent3 = createSlaveInfo("cpus:1;mem:100"); + allocator->addSlave( + agent3.id(), + agent3, + AGENT_CAPABILITIES(), + None(), + agent3.resources(), + {}); + + // Quota of both frameworks are satisfied at this point, therefore + // resources of agent3 should follow the rule of fair share and be + // offered to `framework1`. + { + Allocation expected = Allocation( + framework1.id(), + {{PARENT_ROLE, {{agent3.id(), agent3.resources()}}}}); + + AWAIT_EXPECT_EQ(expected, allocations.get()); + } +} + + class HierarchicalAllocatorTestWithParam : public HierarchicalAllocatorTestBase, public WithParamInterface<bool> {}; http://git-wip-us.apache.org/repos/asf/mesos/blob/50a71c03/src/tests/master_quota_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp index 7f94b92..2c7ec5a 100644 --- a/src/tests/master_quota_tests.cpp +++ b/src/tests/master_quota_tests.cpp @@ -1481,7 +1481,9 @@ TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequestsWithoutPrincipal) // This test checks that quota can be successfully set, queried, and // removed on a child role. -TEST_F(MasterQuotaTest, ChildRole) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ChildRole) { Try<Owned<cluster::Master>> master = StartMaster(); ASSERT_SOME(master); @@ -1570,7 +1572,9 @@ TEST_F(MasterQuotaTest, ChildRole) // This test checks that attempting to set quota on a child role is // rejected if the child's parent does not have quota set. -TEST_F(MasterQuotaTest, ChildRoleWithNoParentQuota) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ChildRoleWithNoParentQuota) { Try<Owned<cluster::Master>> master = StartMaster(); ASSERT_SOME(master); @@ -1599,7 +1603,9 @@ TEST_F(MasterQuotaTest, ChildRoleWithNoParentQuota) // This test checks that a request to set quota for a child role is // rejected if it exceeds the parent role's quota. -TEST_F(MasterQuotaTest, ChildRoleExceedsParentQuota) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ChildRoleExceedsParentQuota) { Try<Owned<cluster::Master>> master = StartMaster(); ASSERT_SOME(master); @@ -1644,7 +1650,9 @@ TEST_F(MasterQuotaTest, ChildRoleExceedsParentQuota) // This test checks that a request to set quota for a child role is // rejected if it would result in the parent role's quota being // smaller than the sum of the quota of its children. -TEST_F(MasterQuotaTest, ChildRoleSumExceedsParentQuota) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ChildRoleSumExceedsParentQuota) { Try<Owned<cluster::Master>> master = StartMaster(); ASSERT_SOME(master); @@ -1703,7 +1711,9 @@ TEST_F(MasterQuotaTest, ChildRoleSumExceedsParentQuota) // This test checks that a request to delete quota for a parent role // is rejected since this would result in the child role's quota // exceeding the parent role's quota. -TEST_F(MasterQuotaTest, ChildRoleDeleteParentQuota) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ChildRoleDeleteParentQuota) { Try<Owned<cluster::Master>> master = StartMaster(); ASSERT_SOME(master); @@ -1761,7 +1771,9 @@ TEST_F(MasterQuotaTest, ChildRoleDeleteParentQuota) // child roles should not be double-counted with the quota on the // child's parent role. In other words, the total quota'd resources in // the cluster is the sum of the quota on the top-level roles. -TEST_F(MasterQuotaTest, ClusterCapacityWithNestedRoles) +// +// TODO(neilc): Re-enable this test when MESOS-7402 is fixed. +TEST_F(MasterQuotaTest, DISABLED_ClusterCapacityWithNestedRoles) { TestAllocator<> allocator; EXPECT_CALL(allocator, initialize(_, _, _, _));
