[mesos] branch master updated: Stripped metadata and non-scalars from GET_ROLES resources.

[bm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 3ee3c5d  Stripped metadata and non-scalars from GET_ROLES resources.
3ee3c5d is described below

commit 3ee3c5d50bbf192fddfe6bd6d5a6df6cec9d4456
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Aug 19 16:05:27 2019 -0400

    Stripped metadata and non-scalars from GET_ROLES resources.
    
    This endpoint is meant to expose resource statistics, and only
    accidentally exposed the metadata and non-scalar resources. It
    does not make sense to expose non-scalar resources in this way.
    
    The `resources` field will be deprecated in favor of a breakdown
    between offered, allocated, and reserved resources (similar to
    the /roles endpoint).
    
    Review: https://reviews.apache.org/r/71310
---
 src/master/http.cpp             | 39 +++++++++++++++++++++++++--------------
 src/master/master.hpp           | 19 -------------------
 src/master/readonly_handler.cpp |  5 +++--
 src/tests/api_tests.cpp         | 10 +++++-----
 4 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/src/master/http.cpp b/src/master/http.cpp
index 6400771..0987d93 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2705,28 +2705,39 @@ Future<Response> Master::Http::getRoles(
           continue;
         }
 
-        mesos::Role role;
+        mesos::Role* role = getRoles->add_roles();
 
-        if (master->weights.contains(name)) {
-          role.set_weight(master->weights[name]);
-        } else {
-          role.set_weight(1.0);
-        }
+        role->set_name(name);
+
+        role->set_weight(master->weights.get(name).getOrElse(DEFAULT_WEIGHT));
 
-        if (master->roles.contains(name)) {
-          Role* role_ = master->roles.at(name);
+        RoleResourceBreakdown resourceBreakdown(master, name);
 
-          *role.mutable_resources() =
-            role_->allocatedAndOfferedResources();
+        ResourceQuantities allocatedAndOffered =
+          resourceBreakdown.allocated() + resourceBreakdown.offered();
 
-          foreachkey (const FrameworkID& frameworkId, role_->frameworks) {
-            role.add_frameworks()->CopyFrom(frameworkId);
+        // `resources` will be deprecated in favor of
+        // `offered`, `allocated`, `reserved`, and quota consumption.
+        // As a result, we don't bother trying to expose more
+        // than {cpus, mem, disk, gpus} since we don't know if
+        // anything outside this set is of type SCALAR.
+        foreach (const auto& quantity, allocatedAndOffered) {
+          if (quantity.first == "cpus" || quantity.first == "mem" ||
+              quantity.first == "disk" || quantity.first == "gpus") {
+            Resource* resource = role->add_resources();
+            resource->set_name(quantity.first);
+            resource->set_type(Value::SCALAR);
+            *resource->mutable_scalar() = quantity.second;
           }
         }
 
-        role.set_name(name);
+        Option<Role*> role_ = master->roles.get(name);
 
-        getRoles->add_roles()->CopyFrom(role);
+        if (role_.isSome()) {
+          foreachkey (const FrameworkID& frameworkId, (*role_)->frameworks) {
+            *role->add_frameworks() = frameworkId;
+          }
+        }
       }
 
       return OK(serialize(contentType, evolve(response)),
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 09a70df..8d9ce3e 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -2778,25 +2778,6 @@ struct Role
     frameworks.erase(framework->id());
   }
 
-  Resources allocatedAndOfferedResources() const
-  {
-    Resources resources;
-
-    auto allocatedTo = [](const std::string& role) {
-      return [role](const Resource& resource) {
-        CHECK(resource.has_allocation_info());
-        return resource.allocation_info().role() == role;
-      };
-    };
-
-    foreachvalue (Framework* framework, frameworks) {
-      resources += framework->totalUsedResources.filter(allocatedTo(role));
-      resources += framework->totalOfferedResources.filter(allocatedTo(role));
-    }
-
-    return resources;
-  }
-
   const Master* master;
   const std::string role;
 
diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp
index b226ae1..500ce3f 100644
--- a/src/master/readonly_handler.cpp
+++ b/src/master/readonly_handler.cpp
@@ -738,8 +738,9 @@ process::http::Response Master::ReadOnlyHandler::roles(
             writer->element([&](JSON::ObjectWriter* writer) {
               writer->field("name", name);
 
-              // Default weight is 1.0.
-              writer->field("weight", master->weights.get(name).getOrElse(1.0));
+              writer->field(
+                  "weight",
+                  master->weights.get(name).getOrElse(DEFAULT_WEIGHT));
 
               Option<Role*> role = master->roles.get(name);
 
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index e202cd3..a735a20 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -1150,11 +1150,11 @@ TEST_P(MasterAPITest, GetRoles)
   ASSERT_EQ(2, v1Response->get_roles().roles().size());
   EXPECT_EQ("role1", v1Response->get_roles().roles(1).name());
   EXPECT_EQ(2.5, v1Response->get_roles().roles(1).weight());
-  ASSERT_EQ(
-      allocatedResources(
-          devolve(v1::Resources::parse(slaveFlags.resources.get()).get()),
-          "role1"),
-      devolve(v1Response->get_roles().roles(1).resources()));
+
+  EXPECT_EQ(
+      CHECK_NOTERROR(v1::Resources::parse(
+          "cpus:0.5; disk:1024; mem:512")),
+      v1Response->get_roles().roles(1).resources());
 
   driver.stop();
   driver.join();