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 <bmah...@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();