This is an automated email from the ASF dual-hosted git repository. mzhu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 2462cfbc831b1fdb9905904eeab586be570cae6d Author: Meng Zhu <[email protected]> AuthorDate: Thu Jul 11 16:45:08 2019 -0700 Added limits validation for `UPDATE_QUOTA` call. The validation checks the newly requested limits against its current consumption. If a role's consumption is already more than the limits, the update request will be denied (unless force flag is set to true). Also added a test. Review: https://reviews.apache.org/r/71061 --- src/master/quota_handler.cpp | 25 +++++++++- src/tests/master_quota_tests.cpp | 100 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index 2a3ca56..4388fe6 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -466,9 +466,30 @@ Future<http::Response> Master::QuotaHandler::update( } } - // TODO(mzhu): Validate a role's limit is below its current consumption + // Validate a role's requested limit is below its current consumption // (otherwise a `force` flag is needed). - // + foreach (const auto& config, configs) { + ResourceLimits limits{config.limits()}; + ResourceQuantities consumedQuota = + RoleResourceBreakdown(master, config.role()).consumedQuota(); + + if (!limits.contains(consumedQuota)) { + if (call.update_quota().force()) { + LOG(INFO) << "Updating '" << config.role() << "' quota limit to" + << " '" + stringify(limits) + "';" + << " this is below its current quota consumption" + << " '" + stringify(consumedQuota) + "';" + << " Ignored violation since the force flag is provided."; + } else { + return BadRequest("Invalid QuotaConfig: Role '" + config.role() + "'" + " is already consuming '" + stringify(consumedQuota) + "';" + " this is more than the requested limits" + " '" + stringify(limits) + "'" + " (use 'force' flag to bypass this check)"); + } + } + } + // TODO(mzhu): Pull out these validation in a function that can be shared // between this and the old handlers. diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp index 8105765..58ecc50 100644 --- a/src/tests/master_quota_tests.cpp +++ b/src/tests/master_quota_tests.cpp @@ -1401,6 +1401,106 @@ TEST_F(MasterQuotaTest, AvailableResourcesMultipleAgents) } +// This test ensures that quota limits update request is validated +// against the roles current quota consumption. A role's current +// consumption must not exceed the requested limits otherwise +// the update will not be granted. A force flag can override the check. +TEST_F(MasterQuotaTest, ValidateLimitAgainstConsumed) +{ + TestAllocator<> allocator; + EXPECT_CALL(allocator, initialize(_, _, _)); + + Try<Owned<cluster::Master>> master = StartMaster(&allocator); + ASSERT_SOME(master); + + // Start an agent and allocate all its resources to a task. + slave::Flags flags = CreateSlaveFlags(); + flags.resources = "cpus:2;mem:1024"; + + Owned<MasterDetector> detector = master.get()->createDetector(); + Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags); + ASSERT_SOME(slave); + + FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO; + frameworkInfo.set_roles(0, ROLE1); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, frameworkInfo, master.get()->pid, DEFAULT_CREDENTIAL); + + Future<FrameworkID> frameworkId; + EXPECT_CALL(sched, registered(&driver, _, _)) + .WillOnce(FutureArg<1>(&frameworkId)); + + ExecutorInfo executorInfo = createExecutorInfo("dummy", "sleep 3600"); + + Future<Nothing> taskLaunched; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(DoAll( + LaunchTasks(executorInfo, 1, 2, 1024, ROLE1), + FutureSatisfy(&taskLaunched))); + + EXPECT_CALL(sched, statusUpdate(&driver, _)).WillRepeatedly(Return()); + + driver.start(); + + AWAIT_READY(taskLaunched); + + // Now we have: + // - Allocated resources: cpus:2;mem:1024 + + // Request a limit of 1 cpu which will be rejected since `ROLE1` + // is already consuming 2 cpus. + { + process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL); + headers["Content-Type"] = "application/json"; + + Future<Response> response = process::http::post( + master.get()->pid, + "/api/v1", + headers, + createUpdateQuotaRequestBody(createQuotaConfig(ROLE1, "", "cpus:1"))); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); + + EXPECT_EQ( + "Invalid QuotaConfig: Role 'role1' is already consuming" + " 'cpus:2; mem:1024'; this is more than the requested limits" + " 'cpus:1' (use 'force' flag to bypass this check)", + response->body); + } + + // Request a limit of 2 cpus is fine. + { + process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL); + headers["Content-Type"] = "application/json"; + + Future<Response> response = process::http::post( + master.get()->pid, + "/api/v1", + headers, + createUpdateQuotaRequestBody(createQuotaConfig(ROLE1, "", "cpus:2"))); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); + } + + // Force request a limit of 1 cpu. This will be granted. + { + process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL); + headers["Content-Type"] = "application/json"; + + Future<Response> response = process::http::post( + master.get()->pid, + "/api/v1", + headers, + createUpdateQuotaRequestBody( + createQuotaConfig(ROLE1, "", "cpus:1"), true)); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); + } +} + + // Checks that a quota request succeeds if there are sufficient total // resources in the cluster, even though they are blocked in outstanding // offers, i.e. quota request rescinds offers.
