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 b23b4e52a24637231a85faf2416b75180cfd9063 Author: Meng Zhu <[email protected]> AuthorDate: Thu Jun 20 17:17:41 2019 -0700 Made `/roles` endpoint also return quota limits. Now that guarantees are decoupled from limits, we should return limits and guarantees separately in the `/roles` endpoint. Three incompatible changes are introduced: - The `principal` field is removed. This legacy field was used to record the principal of the operator who configured the quota. So that later, if a different operator with a different principal wants to modify the quota, the action can be properly authorized. This use case has since been deprecated and the principal field will no longer be filled going forward. - Resources with zero quantity will no longer be included in the `guarantee` field. - The `guarantee` field will continue to be filled. However, since we are decoupling the quota guarantee from the limit. One can no longer assume that the limit will be the same as guarantee. A separate `limit` field is introduced. Before, the response might contain: ``` { "quota": { "guarantee": { "cpus": 1, "disk": 0, "gpus": 0, "mem": 512 }, "principal": "test-principal", "role": "foo" } } ``` After: ``` { "quota": { "guarantee": { "cpus": 1, "mem": 512 }, "limit": { "cpus": 1, "mem": 512 }, "role": "foo" } } ``` Also fixed an affected test. Review: https://reviews.apache.org/r/70915 --- src/common/http.cpp | 31 ++++++++++++++++--------------- src/common/http.hpp | 7 ++----- src/master/readonly_handler.cpp | 21 ++++++++++++++++++++- src/tests/role_tests.cpp | 10 ++++++---- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/common/http.cpp b/src/common/http.cpp index 6962959..ae026f6 100644 --- a/src/common/http.cpp +++ b/src/common/http.cpp @@ -753,6 +753,22 @@ void json( } +void json(JSON::ObjectWriter* writer, const ResourceQuantities& quantities) +{ + foreachpair (const string& name, const Value::Scalar& scalar, quantities) { + writer->field(name, scalar.value()); + } +} + + +void json(JSON::ObjectWriter* writer, const ResourceLimits& limits) +{ + foreachpair (const string& name, const Value::Scalar& scalar, limits) { + writer->field(name, scalar.value()); + } +} + + void json(JSON::ObjectWriter* writer, const SlaveInfo& slaveInfo) { writer->field("id", slaveInfo.id().value()); @@ -855,21 +871,6 @@ static void json(JSON::StringWriter* writer, const Value::Text& text) } -namespace quota { - -void json(JSON::ObjectWriter* writer, const QuotaInfo& quotaInfo) -{ - writer->field("role", quotaInfo.role()); - writer->field("guarantee", quotaInfo.guarantee()); - - if (quotaInfo.has_principal()) { - writer->field("principal", quotaInfo.principal()); - } -} - -} // namespace quota { - - Future<Owned<ObjectApprovers>> ObjectApprovers::create( const Option<Authorizer*>& authorizer, const Option<Principal>& principal, diff --git a/src/common/http.hpp b/src/common/http.hpp index b8c71ec..062586c 100644 --- a/src/common/http.hpp +++ b/src/common/http.hpp @@ -209,6 +209,8 @@ void json(JSON::ObjectWriter* writer, const Resources& resources); void json( JSON::ObjectWriter* writer, const google::protobuf::RepeatedPtrField<Resource>& resources); +void json(JSON::ObjectWriter* writer, const ResourceQuantities& quantities); +void json(JSON::ObjectWriter* writer, const ResourceLimits& limits); void json(JSON::ObjectWriter* writer, const SlaveInfo& slaveInfo); void json( JSON::StringWriter* writer, const SlaveInfo::Capability& capability); @@ -216,11 +218,6 @@ void json(JSON::ObjectWriter* writer, const Task& task); void json(JSON::ObjectWriter* writer, const TaskStatus& status); -namespace quota { -void json(JSON::ObjectWriter* writer, const QuotaInfo& quotaInfo); -} // namespace quota { - - // Implementation of the `ObjectApprover` interface authorizing all objects. class AcceptingObjectApprover : public ObjectApprover { diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp index d62f139..694f3af 100644 --- a/src/master/readonly_handler.cpp +++ b/src/master/readonly_handler.cpp @@ -713,7 +713,26 @@ process::http::Response Master::ReadOnlyHandler::roles( writer->field("weight", master->weights.get(name).getOrElse(1.0)); if (master->quotas.contains(name)) { - writer->field("quota", master->quotas.at(name).info); + // Prior to Mesos 1.9, this field is filled based on + // `QuotaInfo` which is now deprecated. For backward + // compatibility reasons, we do not use any formatter + // for the new struct but construct the response by hand. + // Specifically: + // + // - We keep the `role` field which was present in the + // `QuotaInfo`. + // + // - We name the field using singular `guarantee` and `limit` + // which is different from the plural used in `QuotaConfig`. + // + // TODO(mzhu): This conversion will not be needed if we + // store `Quota2` in master. + Quota2 quota{master->quotas.at(name).info}; + writer->field("quota", [&](JSON::ObjectWriter* writer) { + writer->field("role", name); + writer->field("guarantee", quota.guarantees); + writer->field("limit", quota.limits); + }); } Option<Role*> role = master->roles.get(name); diff --git a/src/tests/role_tests.cpp b/src/tests/role_tests.cpp index e428fb4..bf5b3cb 100644 --- a/src/tests/role_tests.cpp +++ b/src/tests/role_tests.cpp @@ -432,15 +432,17 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, RolesEndpointContainsQuota) "{" "\"quota\":" "{" + "\"role\":\"foo\"," "\"guarantee\":" "{" "\"cpus\":1.0," - "\"disk\":0," - "\"gpus\":0," "\"mem\":512.0" "}," - "\"principal\":\"test-principal\"," - "\"role\":\"foo\"" + "\"limit\":" + "{" + "\"cpus\":1.0," + "\"mem\":512.0" + "}" "}" "}" );
