You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2015/12/02 20:51:08 UTC

mesos git commit: Quota: Cleaned up `RemoveSingleQuota` test.

Repository: mesos
Updated Branches:
  refs/heads/master a5df87c63 -> 016c6cb36


Quota: Cleaned up `RemoveSingleQuota` test.

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


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

Branch: refs/heads/master
Commit: 016c6cb365703df1081d52afc57c14e56af6a5a3
Parents: a5df87c
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Wed Dec 2 14:25:29 2015 -0500
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Wed Dec 2 14:49:49 2015 -0500

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp |  4 +--
 src/master/quota_handler.cpp                |  4 +--
 src/tests/master_quota_tests.cpp            | 42 +++++++++++-------------
 3 files changed, 22 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/016c6cb3/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index b8f3be8..a8f65b7 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -901,8 +901,8 @@ void HierarchicalAllocatorProcess::setQuota(
   // dedicated sorter, while the later just updates the actual quota.
   CHECK_NONE(roles[role].quota);
 
-  // Persist quota in memory and add the role into the corresponding allocation
-  // group.
+  // Persist quota in memory and add the role into the corresponding
+  // allocation group.
   roles[role].quota = quota;
   quotaRoleSorter->add(role, roles[role].info.weight());
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/016c6cb3/src/master/quota_handler.cpp
----------------------------------------------------------------------
diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index e9c8791..825ee33 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -246,7 +246,6 @@ Future<http::Response> Master::QuotaHandler::set(
   CHECK_EQ("POST", request.method);
 
   // Validate request and extract JSON.
-
   Try<hashmap<string, string>> decode = http::query::decode(request.body);
   if (decode.isError()) {
     return BadRequest("Failed to decode set quota request query string ('" +
@@ -352,7 +351,7 @@ Future<http::Response> Master::QuotaHandler::set(
 Future<http::Response> Master::QuotaHandler::remove(
     const http::Request& request) const
 {
-  VLOG(1) << "Remove quota request for path: '" << request.url.path << "'";
+  VLOG(1) << "Removing quota for request path: '" << request.url.path << "'";
 
   // Authenticate and authorize the request.
   // TODO(alexr): Check Master::Http::authenticate() for an example.
@@ -401,7 +400,6 @@ Future<http::Response> Master::QuotaHandler::remove(
   return OK();
 }
 
-
 } // namespace master {
 } // namespace internal {
 } // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/016c6cb3/src/tests/master_quota_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 44e6f33..c9d78cf 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -135,7 +135,6 @@ protected:
     return request;
   }
 
-
 protected:
   const std::string ROLE1{"role1"};
   const std::string ROLE2{"role2"};
@@ -193,7 +192,7 @@ TEST_F(MasterQuotaTest, SetInvalidRequest)
   // We do not need an agent since a request should be rejected before
   // we start looking at available resources.
 
-  // We wrap the `http::post` into a lambda for readability of the tests.
+  // Wrap the `http::post` into a lambda for readability of the test.
   auto postQuota = [this, &master](const string& request) {
     return process::http::post(
         master.get(),
@@ -442,12 +441,17 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
   AWAIT_READY(agentTotalResources);
   EXPECT_EQ(defaultAgentResources, agentTotalResources.get());
 
-  // Ensure that we can't remove quota for a role that is unknown to the master.
-  {
-    Future<Response> response = process::http::requestDelete(
+  // Wrap the `http::requestDelete` into a lambda for readability of the test.
+  auto removeQuota = [this, &master](const string& path) {
+    return process::http::requestDelete(
         master.get(),
-        "quota/" + UNKNOWN_ROLE,
+        path,
         createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+  };
+
+  // Ensure that we can't remove quota for a role that is unknown to the master.
+  {
+    Future<Response> response = removeQuota("quota/" + UNKNOWN_ROLE);
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response)
       << response.get().body;
@@ -455,17 +459,15 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
 
   // Ensure that we can't remove quota for a role that has no quota set.
   {
-    Future<Response> response = process::http::requestDelete(
-        master.get(),
-        "quota/" + ROLE1,
-        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+    Future<Response> response = removeQuota("quota/" + ROLE1);
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response)
       << response.get().body;
   }
 
-  // We request quota for a portion of the resources available on the agent.
+  // Ensure we can remove the quota we have requested before.
   {
+    // Request quota for a portion of the resources available on the agent.
     Resources quotaResources = Resources::parse("cpus:1;mem:512", ROLE1).get();
     EXPECT_TRUE(agentTotalResources.get().contains(quotaResources.flatten()));
 
@@ -477,25 +479,19 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
       << response.get().body;
-  }
 
-  // Ensure we can remove the quota.
-  {
+    // Remove the previously requested quota.
     Future<Nothing> receivedRemoveRequest;
     EXPECT_CALL(allocator, removeQuota(Eq(ROLE1)))
-      .WillOnce(FutureSatisfy(&receivedRemoveRequest));
+      .WillOnce(DoAll(InvokeRemoveQuota(&allocator),
+                      FutureSatisfy(&receivedRemoveRequest)));
 
-    Future<Response> response = process::http::requestDelete(
-        master.get(),
-        "quota/" + ROLE1,
-        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+    response = removeQuota("quota/" + ROLE1);
 
-    // TODO(joerg84): Add more detailed error message.
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
-      << "Quota remove request failed:";
+      << response.get().body;
 
-    // Quota request is granted and reached the allocator. Make sure nothing
-    // got lost in-between.
+    // Ensure that the quota remove request has reached the allocator.
     AWAIT_READY(receivedRemoveRequest);
   }