[mesos] 03/06: Added limits validation for `UPDATE_QUOTA` call.

[mz...@apache.org]

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 <mz...@mesosphere.io>
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.