You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mz...@apache.org on 2019/03/26 00:37:34 UTC
[mesos] 03/03: Removed `limits` from `QuotaInfo` and `QuotaRequest`
protobuf.
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 294b70a2190f01952f7ec6f542c60999373ea0fc
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Thu Mar 21 19:07:48 2019 -0700
Removed `limits` from `QuotaInfo` and `QuotaRequest` protobuf.
`Limits` was added as an experimental field and the
related calls were not implementation complete.
`QuotaConfig` has been added where limits can be
specified. This patch cleans up the experimental
field and related validation and tests.
Review: https://reviews.apache.org/r/70268
---
include/mesos/quota/quota.proto | 2 -
include/mesos/v1/quota/quota.proto | 41 +------------
src/master/quota.cpp | 122 +------------------------------------
src/tests/master_quota_tests.cpp | 49 ---------------
4 files changed, 5 insertions(+), 209 deletions(-)
diff --git a/include/mesos/quota/quota.proto b/include/mesos/quota/quota.proto
index 34d4b69..682c0cc 100644
--- a/include/mesos/quota/quota.proto
+++ b/include/mesos/quota/quota.proto
@@ -31,7 +31,6 @@ message QuotaInfo {
optional string role = 1;
optional string principal = 2;
repeated Resource guarantee = 3;
- repeated Resource limit = 4;
}
@@ -42,7 +41,6 @@ message QuotaRequest {
optional bool force = 1 [default = false];
optional string role = 2;
repeated Resource guarantee = 3;
- repeated Resource limit = 4;
}
diff --git a/include/mesos/v1/quota/quota.proto b/include/mesos/v1/quota/quota.proto
index 8a6a64a..b109ca2 100644
--- a/include/mesos/v1/quota/quota.proto
+++ b/include/mesos/v1/quota/quota.proto
@@ -25,7 +25,7 @@ option java_outer_classname = "Protos";
/**
- * Describes the resource guarantees and limits for a role.
+ * Describes the resource guarantees for a role.
* Persisted in the registry.
*/
message QuotaInfo {
@@ -35,7 +35,6 @@ message QuotaInfo {
optional string principal = 2;
repeated Resource guarantee = 3;
- repeated Resource limit = 4;
}
@@ -43,12 +42,10 @@ message QuotaInfo {
* Describes an update to a role's quota. This is a copy of
* `QuotaInfo` which omits the principal since it is determined
* during authentication. Also allows the user to force the update
- * in the case of a guarantee overcommit or a limit exceeding the
- * parent's limit (or overall cluster size if a top level role).
+ * in the case of a guarantee overcommit.
*/
message QuotaRequest {
- // See `guarantee` and `limit` for the behavior of `force`
- // on these two fields.
+ // See `guarantee` for the behavior of `force`.
optional bool force = 1;
optional string role = 2;
@@ -76,38 +73,6 @@ message QuotaRequest {
// NOTE: The resources must be scalars without additional
// metadata like reservations, disk information, etc.
repeated Resource guarantee = 3;
-
- // EXPERIMENTAL DO NOT USE.
- //
- // This feature is not implementation complete.
- //
- // Imposes a limit on the amount of resources allocated to the
- // role. Mesos will try its best to ensure that the role does
- // not exceed this limit. Despite this, the limit can be exceeded
- // when:
- // (1) The limit is lowered below the allocation.
- // (2) Some agents are partitioned and re-connect with
- // resources allocated to the role.
- //
- // The provided limit will be validated to ensure it does not
- // exceed the total cluster size. The operator can disable
- // this check via `QuotaRequest.force`.
- //
- // If the limit is omitted, there is no limit.
- //
- // Operators may want to set up alerting to let them know
- // when the limit is exceeded.
- //
- // NOTE: The resources must be scalars without additional
- // metadata like reservations, disk information, etc.
- //
- // NOTE: The limit was introduced alongside the v1 `UPDATE_QUOTA`
- // `master::Call`. Note that the old v0 `POST /quota` endpoint and
- // the v1 `SetQuota` `master::Call` continue to implicitly set
- // limit to the guarantee for backwards compatibility. Users
- // should switch to the v1 `UPDATE_QUOTA` `master::Call` which
- // does not implicitly set the limit.
- repeated Resource limit = 4;
}
/**
diff --git a/src/master/quota.cpp b/src/master/quota.cpp
index 62b788f..bad19a2 100644
--- a/src/master/quota.cpp
+++ b/src/master/quota.cpp
@@ -95,14 +95,12 @@ namespace {
QuotaInfo createQuotaInfo(
const string& role,
- const RepeatedPtrField<Resource>& guarantee,
- const RepeatedPtrField<Resource>& limit)
+ const RepeatedPtrField<Resource>& guarantee)
{
QuotaInfo quota;
quota.set_role(role);
quota.mutable_guarantee()->CopyFrom(guarantee);
- quota.mutable_limit()->CopyFrom(limit);
return quota;
}
@@ -112,7 +110,7 @@ QuotaInfo createQuotaInfo(
QuotaInfo createQuotaInfo(const QuotaRequest& request)
{
- return createQuotaInfo(request.role(), request.guarantee(), request.limit());
+ return createQuotaInfo(request.role(), request.guarantee());
}
@@ -175,127 +173,11 @@ Option<Error> quotaInfo(const QuotaInfo& quotaInfo)
names.insert(resource.name());
}
- // TODO(bmahler): We ensure the limit is not set here to enforce
- // that the v0 API and v1 SET_QUOTA Call cannot set an explicit
- // limit. Ideally this check would be done during call validation
- // but, at the time of writing this comment, call validation does
- // not validate the call type specific messages within `Call`.
- // Once we start to allow limit to be set via UPDATE_QUOTA, this
- // check will have to be moved into a SET_QUOTA validator and
- // a v0 /quota validator.
- if (quotaInfo.limit_size() > 0) {
- return Error("Setting QuotaInfo.limit is not supported via"
- " /quota and the SET_QUOTA Call,"
- "please use the UPDATE_QUOTA Call");
- }
-
return None();
}
} // namespace validation {
-Option<Error> validate(const QuotaRequest& request)
-{
- if (!request.has_role()) {
- return Error("'QuotaRequest.role' must be set");
- }
-
- // Check the provided role is valid.
- Option<Error> error = roles::validate(request.role());
- if (error.isSome()) {
- return Error("Invalid 'QuotaRequest.role': " + error->message);
- }
-
- // Disallow quota for '*' role.
- //
- // TODO(alexr): Consider allowing setting quota for '*' role,
- // see MESOS-3938.
- if (request.role() == "*") {
- return Error("Invalid 'QuotaRequest.role': setting quota for the"
- " default '*' role is not supported");
- }
-
- // Define a helper for use against guarantee and limit.
- auto validateQuotaResources = [](
- const RepeatedPtrField<Resource>& resources) -> Option<Error> {
- hashset<string> names;
-
- // Check that each resource does not contain fields
- // that are disallowed for quota resources.
- foreach (const Resource& resource, resources) {
- if (resource.has_reservation()) {
- return Error("'Resource.reservation' must not be set");
- }
-
- if (resource.reservations_size() > 0) {
- return Error("'Resource.reservations' must not be set");
- }
-
- if (resource.has_disk()) {
- return Error("'Resource.disk' must not be set");
- }
-
- if (resource.has_revocable()) {
- return Error("'Resource.revocable' must not be set");
- }
-
- if (resource.type() != Value::SCALAR) {
- return Error("'Resource.type' must be 'SCALAR'");
- }
-
- if (resource.has_shared()) {
- return Error("'Resource.shared' must not be set");
- }
-
- if (resource.has_provider_id()) {
- return Error("'Resource.provider_id' must not be set");
- }
-
- // Check that resource names do not repeat.
- if (names.contains(resource.name())) {
- return Error("Duplicate '" + resource.name() + "'; only a single"
- " entry for each resource is supported");
- }
-
- names.insert(resource.name());
- }
-
- // Finally, ensure the resources are valid.
- return Resources::validate(resources);
- };
-
- error = validateQuotaResources(request.guarantee());
- if (error.isSome()) {
- return Error("Invalid 'QuotaRequest.guarantee': " + error->message);
- }
-
- error = validateQuotaResources(request.limit());
- if (error.isSome()) {
- return Error("Invalid 'QuotaRequest.limit': " + error->message);
- }
-
- // Validate that guarantee <= limit.
- Resources guarantee = request.guarantee();
- Resources limit = request.limit();
-
- // This needs to be checked on a per-resource basis for those
- // resources that are specified in both the guarantee and limit.
- foreach (const string& name, guarantee.names() & limit.names()) {
- Value::Scalar guaranteeScalar =
- CHECK_NOTNONE(guarantee.get<Value::Scalar>(name));
- Value::Scalar limitScalar =
- CHECK_NOTNONE(limit.get<Value::Scalar>(name));
-
- if (!(guaranteeScalar <= limitScalar)) {
- return Error("'QuotaRequest.guarantee' (" + stringify(guarantee) + ")"
- " is not contained within the 'QuotaRequest.limit'"
- " (" + stringify(limit) + ")");
- }
- }
-
- return None();
-}
-
Option<Error> validate(const QuotaConfig& config)
{
if (!config.has_role()) {
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 3e7a94c..23aa831 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -268,55 +268,6 @@ TEST_F(MasterQuotaTest, InvalidSetRequest)
}
-// v0 /quota requests and SET_QUOTA calls cannot specify a limit.
-TEST_F(MasterQuotaTest, V0WithLimitDisallowed)
-{
- Try<Owned<cluster::Master>> master = StartMaster();
- ASSERT_SOME(master);
-
- // Ensure a /quota POST request with limit is rejected.
- QuotaRequest quotaRequest;
- quotaRequest.set_role("role");
- *quotaRequest.mutable_guarantee() =
- CHECK_NOTERROR(Resources::parse("cpus:10;mem:20"));
- *quotaRequest.mutable_limit() =
- CHECK_NOTERROR(Resources::parse("cpus:20;mem:40"));
-
- process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
-
- Future<Response> response = process::http::post(
- master.get()->pid,
- "quota",
- headers,
- string(jsonify(JSON::Protobuf(quotaRequest))));
-
- AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response);
- EXPECT_TRUE(strings::contains(
- response->body,
- "Setting QuotaInfo.limit is not supported"))
- << response->body;
-
- // Ensure a SET_QUOTA Call with limit is rejected.
- mesos::master::Call call;
- call.set_type(mesos::master::Call::SET_QUOTA);
- *call.mutable_set_quota()->mutable_quota_request() = quotaRequest;
-
- headers["Content-Type"] = APPLICATION_PROTOBUF;
-
- response = process::http::post(
- master.get()->pid,
- "api/v1",
- headers,
- call.SerializeAsString());
-
- AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response);
- EXPECT_TRUE(strings::contains(
- response->body,
- "Setting QuotaInfo.limit is not supported"))
- << response->body;
-}
-
-
// Checks that a quota set request is not satisfied if an invalid
// field is set or provided data are not supported.
TEST_F(MasterQuotaTest, SetRequestWithInvalidData)