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 2a22035a1595837d215468859713281aa6dfb279
Author: Meng Zhu <[email protected]>
AuthorDate: Tue Mar 12 23:30:12 2019 -0700

    Updated a test for `UPDATE_QUOTA` call validation.
    
    Review: https://reviews.apache.org/r/70203
---
 src/tests/master_validation_tests.cpp | 243 +++++++---------------------------
 1 file changed, 50 insertions(+), 193 deletions(-)

diff --git a/src/tests/master_validation_tests.cpp 
b/src/tests/master_validation_tests.cpp
index 63418c7..19ec7a7 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -55,6 +55,7 @@
 
 using namespace mesos::internal::master::validation;
 
+using google::protobuf::Map;
 using google::protobuf::RepeatedPtrField;
 
 using mesos::internal::master::Master;
@@ -70,6 +71,7 @@ using process::Message;
 using process::Owned;
 using process::PID;
 
+using std::pair;
 using std::string;
 using std::vector;
 
@@ -100,229 +102,84 @@ TEST(MasterCallValidationTest, UpdateQuota)
   error = master::validation::master::call::validate(updateQuota);
   EXPECT_NONE(error);
 
-  // Test validation at the request level.
-  mesos::quota::QuotaRequest request;
+  // Test validation at the config level.
+  mesos::quota::QuotaConfig config;
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   ASSERT_SOME(error);
-  EXPECT_TRUE(strings::contains(
-      error->message,
-      "'QuotaRequest.role' must be set"))
+  EXPECT_TRUE(
+      strings::contains(error->message, "'QuotaConfig.role' must be set"))
     << error->message;
 
-  request.set_role("");
+  config.set_role("");
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   ASSERT_SOME(error);
-  EXPECT_TRUE(strings::contains(
-      error->message,
-      "Invalid 'QuotaRequest.role'"))
+  EXPECT_TRUE(strings::contains(error->message, "Invalid 'QuotaConfig.role'"))
     << error->message;
 
-  request.set_role("*");
+  config.set_role("*");
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   ASSERT_SOME(error);
   EXPECT_TRUE(strings::contains(
       error->message,
-      "Invalid 'QuotaRequest.role':"
+      "Invalid 'QuotaConfig.role':"
       " setting quota for the default '*' role is not supported"))
     << error->message;
 
   // Once a role is set, it is valid to have no guarantee and no limit.
-  request.set_role("role");
+  config.set_role("role");
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   ASSERT_NONE(error);
 
-  auto validateGuaranteeOrLimit = [](
-      const string& fieldName,
-      RepeatedPtrField<Resource>* (mesos::quota::QuotaRequest::*field)()) {
-    Option<Error> error;
-    mesos::quota::QuotaRequest request;
-    request.set_role("role");
-
-    // Ensure 'Resource.name' must be set.
-    Resource empty;
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(empty);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " Resource ':0' is invalid: Empty resource name"))
-      << error->message;
-
-    // Ensure 'Resource.reservation' cannot be set.
-    Resource reservation;
-    reservation.set_name("disk");
-    reservation.set_type(Value::SCALAR);
-    reservation.mutable_scalar()->set_value(1.0);
-    reservation.mutable_reservation()->set_role("role");
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(reservation);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.reservation' must not be set"))
-      << error->message;
-
-    // Ensure 'Resource.reservations' cannot be set.
-    Resource reservations =
-      CHECK_NOTERROR(Resources::parse("cpus", "1.0", "role"));
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(reservations);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.reservations' must not be set"))
-      << error->message;
-
-    // Ensure 'Resource.disk' cannot be set.
-    Resource disk = CHECK_NOTERROR(Resources::parse("disk", "1.0", "*"));
-    disk.mutable_disk();
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(disk);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.disk' must not be set"))
-      << error->message;
-
-    // Ensure 'Resource.revocable' cannot be set.
-    Resource revocable = CHECK_NOTERROR(Resources::parse("cpus", "1.0", "*"));
-    revocable.mutable_revocable();
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(revocable);
+  // Now test the guarantees <= limits validation.
 
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.revocable' must not be set"))
-      << error->message;
-
-    // Ensure non-SCALAR types are disallowed.
-    Resource ports;
-    ports.set_name("ports");
-    ports.set_type(Value::RANGES);
-    ports.mutable_ranges();
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(ports);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.type' must be 'SCALAR'"))
-      << error->message;
-
-    // Ensure 'Resource.shared' cannot be set.
-    Resource shared;
-    shared.set_name("disk");
-    shared.set_type(Value::SCALAR);
-    shared.mutable_scalar()->set_value(1.0);
-    shared.mutable_shared();
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(shared);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.shared' must not be set"))
-      << error->message;
-
-    // Ensure 'Resource.provider_id' cannot be set.
-    Resource withProvider;
-    withProvider.set_name("disk");
-    withProvider.set_type(Value::SCALAR);
-    withProvider.mutable_scalar()->set_value(1.0);
-    withProvider.mutable_provider_id()->set_value("ID");
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(withProvider);
-
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " 'Resource.provider_id' must not be set"))
-      << error->message;
+  auto resourceMap = [](const vector<pair<string, double>>& vector)
+    -> Map<string, Value::Scalar> {
+    Map<string, Value::Scalar> result;
 
-    // Ensure only a single entry for each resource name.
-    Resource cpu = CHECK_NOTERROR(Resources::parse("cpus", "1.0", "*"));
-    (request.*field)()->Clear();
-    (request.*field)()->Add()->CopyFrom(cpu);
-    (request.*field)()->Add()->CopyFrom(cpu);
+    foreachpair (const string& name, double value, vector) {
+      Value::Scalar scalar;
+      scalar.set_value(value);
+      result[name] = scalar;
+    }
 
-    error = mesos::internal::master::quota::validate(request);
-    ASSERT_SOME(error);
-    EXPECT_TRUE(strings::contains(
-        error->message,
-        "Invalid 'QuotaRequest." + fieldName + "':"
-        " Duplicate 'cpus'; only a single entry"
-        " for each resource is supported"))
-      << error->message;
+    return result;
   };
 
-  validateGuaranteeOrLimit(
-      "guarantee",
-      &mesos::quota::QuotaRequest::mutable_guarantee);
-
-  validateGuaranteeOrLimit(
-      "limit",
-      &mesos::quota::QuotaRequest::mutable_limit);
-
-  // Now test the guarantee <= limit validation.
-  // No guarantee and no limit.
-  error = mesos::internal::master::quota::validate(request);
-  EXPECT_NONE(error);
-
-  // Guarantee > limit.
-  Resources subset = CHECK_NOTERROR(Resources::parse("cpus:10;mem:20"));
-  Resources superset = CHECK_NOTERROR(Resources::parse("cpus:20;mem:40"));
+  // Guarantees > limits.
+  Map<string, Value::Scalar> superset =
+    resourceMap({{"cpus", 20}, {"mem", 40}});
+  Map<string, Value::Scalar> subset = resourceMap({{"cpus", 10}, {"mem", 20}});
 
-  request.mutable_guarantee()->CopyFrom(superset);
-  request.mutable_limit()->CopyFrom(subset);
+  *config.mutable_guarantees() = superset;
+  *config.mutable_limits() = subset;
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   ASSERT_SOME(error);
   EXPECT_TRUE(strings::contains(
       error->message,
-      "'QuotaRequest.guarantee' (cpus:20; mem:40)"
-      " is not contained within the 'QuotaRequest.limit' (cpus:10; mem:20)"))
+      "'QuotaConfig.guarantees' { cpus: 20, mem: 40 } is not"
+      " contained within the 'QuotaConfig.limits' { cpus: 10, mem: 20 }"))
     << error->message;
 
-  // Guarantee = limit.
-  request.mutable_guarantee()->CopyFrom(subset);
-  request.mutable_limit()->CopyFrom(subset);
+  // Guarantees = limits.
+  *config.mutable_guarantees() = subset;
+  *config.mutable_limits() = subset;
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   EXPECT_NONE(error);
 
-  // Guarantee < limit.
-  request.mutable_guarantee()->CopyFrom(subset);
-  request.mutable_limit()->CopyFrom(superset);
+  // Guarantees < limits.
+  *config.mutable_guarantees() = subset;
+  *config.mutable_limits() = superset;
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   EXPECT_NONE(error);
 
-  // Now we ensure that the guarantee <= limit check is a
+  // Now we ensure that the guarantees <= limits check is a
   // per-resource check. This is important because it's ok to:
   //
   //   (1) Set a limit for a resource when there is no guarantee
@@ -333,15 +190,15 @@ TEST(MasterCallValidationTest, UpdateQuota)
   //
   // We test both cases at once by having both guarantee and
   // limit contain a resource not specified in the other.
-  Resources cpuMemWithDisk =
-    CHECK_NOTERROR(Resources::parse("cpus:10;mem:20;disk:10"));
-  Resources cpuMemWithGpu =
-    CHECK_NOTERROR(Resources::parse("cpus:10;mem:20;gpu:1"));
+  Map<string, Value::Scalar> cpuMemWithDisk =
+    resourceMap({{"cpus", 10}, {"mem", 20}, {"disk", 10}});
+  Map<string, Value::Scalar> cpuMemWithGpu =
+    resourceMap({{"cpus", 10}, {"mem", 20}, {"gpu", 1}});
 
-  request.mutable_guarantee()->CopyFrom(cpuMemWithDisk);
-  request.mutable_limit()->CopyFrom(cpuMemWithGpu);
+  *config.mutable_guarantees() = cpuMemWithDisk;
+  *config.mutable_limits() = cpuMemWithGpu;
 
-  error = mesos::internal::master::quota::validate(request);
+  error = mesos::internal::master::quota::validate(config);
   EXPECT_NONE(error);
 }
 

Reply via email to