You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2017/06/19 03:09:25 UTC
[1/4] mesos git commit: Updated the resources validation logic to
allow for the endpoint format.
Repository: mesos
Updated Branches:
refs/heads/master 07fff4ab2 -> cf96042ec
Updated the resources validation logic to allow for the endpoint format.
For backwards compatibility of external tools consuming endpoints,
We'll inject the "pre-reservation-refinement" format for resources
without refined reservations. That is, the `Resource.role` and
`Resource.reservation` fields will be present as well as
the in with the single `Resource.reservations` field.
Since the users of the endpoints are likely to feed the result back into
Mesos, we need to update the validation logic to allow these resources
to be considered valid.
Review: https://reviews.apache.org/r/60174
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/49e090db
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/49e090db
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/49e090db
Branch: refs/heads/master
Commit: 49e090dbb8ab1b7be9dc59cf31a4a26e337c9304
Parents: 07fff4a
Author: Michael Park <mp...@apache.org>
Authored: Sat Jun 17 20:47:20 2017 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Sun Jun 18 20:05:53 2017 -0700
----------------------------------------------------------------------
src/common/resources.cpp | 150 +++++++++++++++++++++++++++++++-----------
src/v1/resources.cpp | 150 +++++++++++++++++++++++++++++++-----------
2 files changed, 226 insertions(+), 74 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/49e090db/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index f12ab97..481c469 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -835,54 +835,57 @@ Option<Error> Resources::validate(const Resource& resource)
}
}
- if (resource.has_reservation() && resource.reservations_size() > 0) {
- return Error(
- "Invalid reservation: only one of 'reservation' or 'reservations'"
- " may be present");
- }
+ // Validate the reservation format.
- // Check role name.
- Option<Error> error = roles::validate(resource.role());
- if (error.isSome()) {
- return error;
- }
+ if (resource.reservations_size() == 0) {
+ // Check for the "pre-reservation-refinement" format.
- if (resource.has_reservation()) {
- if (resource.reservation().has_type()) {
- return Error(
- "'Resource.ReservationInfo.type' must not be set for the"
- " 'Resource.reservation' field");
+ // Check role name.
+ Option<Error> error = roles::validate(resource.role());
+ if (error.isSome()) {
+ return error;
}
- if (resource.reservation().has_role()) {
- return Error(
- "'Resource.ReservationInfo.role' must not be set for the"
- " 'Resource.reservation' field");
- }
+ // Check reservation.
+ if (resource.has_reservation()) {
+ if (resource.reservation().has_type()) {
+ return Error(
+ "'Resource.ReservationInfo.type' must not be set for"
+ " the 'Resource.reservation' field");
+ }
- // Checks for the invalid state of (role, reservation) pair.
- if (resource.role() == "*") {
- return Error(
- "Invalid reservation: role \"*\" cannot be dynamically reserved");
- }
- }
+ if (resource.reservation().has_role()) {
+ return Error(
+ "'Resource.ReservationInfo.role' must not be set for"
+ " the 'Resource.reservation' field");
+ }
- // Validate all of the roles in `reservations`.
- foreach (
- const Resource::ReservationInfo& reservation, resource.reservations()) {
- Option<Error> error = roles::validate(reservation.role());
- if (error.isSome()) {
- return error;
+ // Checks for the invalid state of (role, reservation) pair.
+ if (resource.role() == "*") {
+ return Error(
+ "Invalid reservation: role \"*\" cannot be dynamically reserved");
+ }
}
+ } else {
+ // Check for the "post-reservation-refinement" format.
+
+ CHECK_GE(resource.reservations_size(), 0);
+
+ // Validate all of the roles in `reservations`.
+ foreach (
+ const Resource::ReservationInfo& reservation, resource.reservations()) {
+ Option<Error> error = roles::validate(reservation.role());
+ if (error.isSome()) {
+ return error;
+ }
- if (reservation.role() == "*") {
- return Error("Invalid reservation: role \"*\" cannot be reserved");
+ if (reservation.role() == "*") {
+ return Error("Invalid reservation: role \"*\" cannot be reserved");
+ }
}
- }
- if (resource.reservations_size() > 0) {
+ // Check that the reservations are correctly refined.
string ancestor = resource.reservations(0).role();
-
for (int i = 1; i < resource.reservations_size(); ++i) {
const Resource::ReservationInfo& reservation = resource.reservations(i);
@@ -902,6 +905,79 @@ Option<Error> Resources::validate(const Resource& resource)
ancestor = descendant;
}
+
+ // Additionally, we allow the "pre-reservation-refinement" format to be set
+ // as long as there is only one reservation, and the `Resource.role` and
+ // `Resource.reservation` fields are consistent with the reservation.
+ if (resource.reservations_size() == 1) {
+ const Resource::ReservationInfo& reservation = resource.reservations(0);
+ if (resource.has_role() && resource.role() != reservation.role()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' field with"
+ " '" + resource.role() + "' does not match the role"
+ " '" + reservation.role() + "' in 'Resource.reservations'");
+ }
+
+ switch (reservation.type()) {
+ case Resource::ReservationInfo::STATIC: {
+ if (resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation' must not be"
+ " set if the single reservation in 'Resource.reservations' is"
+ " STATIC");
+ }
+
+ break;
+ }
+ case Resource::ReservationInfo::DYNAMIC: {
+ if (resource.has_role() != resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' and"
+ " 'Resource.reservation' must either be both set or both not"
+ " set if the single reservation in 'Resource.reservations' is"
+ " DYNAMIC");
+ }
+
+ if (resource.has_reservation() &&
+ resource.reservation().principal() != reservation.principal()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation.principal'"
+ " field with '" + resource.reservation().principal() + "' does"
+ " not match the principal '" + reservation.principal() + "'"
+ " in 'Resource.reservations'");
+ }
+
+ if (resource.has_reservation() &&
+ resource.reservation().labels() != reservation.labels()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation.labels' field"
+ " with '" + stringify(resource.reservation().labels()) + "'"
+ " does not match the labels"
+ " '" + stringify(reservation.labels()) + "'"
+ " in 'Resource.reservations'");
+ }
+
+ break;
+ }
+ case Resource::ReservationInfo::UNKNOWN: {
+ return Error("Unsupported 'Resource.ReservationInfo.Type'");
+ }
+ }
+
+ } else {
+ CHECK_GE(resource.reservations_size(), 1);
+ if (resource.has_role()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' must not be set if"
+ " there is more than one reservation in 'Resource.reservations'");
+ }
+
+ if (resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation' must not be set if"
+ " there is more than one reservation in 'Resource.reservations'");
+ }
+ }
}
// Check that shareability is enabled for supported resource types.
http://git-wip-us.apache.org/repos/asf/mesos/blob/49e090db/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 30644ee..42e903a 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -837,54 +837,57 @@ Option<Error> Resources::validate(const Resource& resource)
}
}
- if (resource.has_reservation() && resource.reservations_size() > 0) {
- return Error(
- "Invalid reservation: only one of 'reservation' or 'reservations'"
- " may be present");
- }
+ // Validate the reservation format.
- // Check role name.
- Option<Error> error = roles::validate(resource.role());
- if (error.isSome()) {
- return error;
- }
+ if (resource.reservations_size() == 0) {
+ // Check for the "pre-reservation-refinement" format.
- if (resource.has_reservation()) {
- if (resource.reservation().has_type()) {
- return Error(
- "'Resource.ReservationInfo.type' must not be set for the"
- " 'Resource.reservation' field");
+ // Check role name.
+ Option<Error> error = roles::validate(resource.role());
+ if (error.isSome()) {
+ return error;
}
- if (resource.reservation().has_role()) {
- return Error(
- "'Resource.ReservationInfo.role' must not be set for the"
- " 'Resource.reservation' field");
- }
+ // Check reservation.
+ if (resource.has_reservation()) {
+ if (resource.reservation().has_type()) {
+ return Error(
+ "'Resource.ReservationInfo.type' must not be set for"
+ " the 'Resource.reservation' field");
+ }
- // Checks for the invalid state of (role, reservation) pair.
- if (resource.role() == "*") {
- return Error(
- "Invalid reservation: role \"*\" cannot be dynamically reserved");
- }
- }
+ if (resource.reservation().has_role()) {
+ return Error(
+ "'Resource.ReservationInfo.role' must not be set for"
+ " the 'Resource.reservation' field");
+ }
- // Validate all of the roles in `reservations`.
- foreach (
- const Resource::ReservationInfo& reservation, resource.reservations()) {
- Option<Error> error = roles::validate(reservation.role());
- if (error.isSome()) {
- return error;
+ // Checks for the invalid state of (role, reservation) pair.
+ if (resource.role() == "*") {
+ return Error(
+ "Invalid reservation: role \"*\" cannot be dynamically reserved");
+ }
}
+ } else {
+ // Check for the "post-reservation-refinement" format.
+
+ CHECK_GE(resource.reservations_size(), 0);
+
+ // Validate all of the roles in `reservations`.
+ foreach (
+ const Resource::ReservationInfo& reservation, resource.reservations()) {
+ Option<Error> error = roles::validate(reservation.role());
+ if (error.isSome()) {
+ return error;
+ }
- if (reservation.role() == "*") {
- return Error("Invalid reservation: role \"*\" cannot be reserved");
+ if (reservation.role() == "*") {
+ return Error("Invalid reservation: role \"*\" cannot be reserved");
+ }
}
- }
- if (resource.reservations_size() > 0) {
+ // Check that the reservations are correctly refined.
string ancestor = resource.reservations(0).role();
-
for (int i = 1; i < resource.reservations_size(); ++i) {
const Resource::ReservationInfo& reservation = resource.reservations(i);
@@ -904,6 +907,79 @@ Option<Error> Resources::validate(const Resource& resource)
ancestor = descendant;
}
+
+ // Additionally, we allow the "pre-reservation-refinement" format to be set
+ // as long as there is only one reservation, and the `Resource.role` and
+ // `Resource.reservation` fields are consistent with the reservation.
+ if (resource.reservations_size() == 1) {
+ const Resource::ReservationInfo& reservation = resource.reservations(0);
+ if (resource.has_role() && resource.role() != reservation.role()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' field with"
+ " '" + resource.role() + "' does not match the role"
+ " '" + reservation.role() + "' in 'Resource.reservations'");
+ }
+
+ switch (reservation.type()) {
+ case Resource::ReservationInfo::STATIC: {
+ if (resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation' must not be"
+ " set if the single reservation in 'Resource.reservations' is"
+ " STATIC");
+ }
+
+ break;
+ }
+ case Resource::ReservationInfo::DYNAMIC: {
+ if (resource.has_role() != resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' and"
+ " 'Resource.reservation' must either be both set or both not"
+ " set if the single reservation in 'Resource.reservations' is"
+ " DYNAMIC");
+ }
+
+ if (resource.has_reservation() &&
+ resource.reservation().principal() != reservation.principal()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation.principal'"
+ " field with '" + resource.reservation().principal() + "' does"
+ " not match the principal '" + reservation.principal() + "'"
+ " in 'Resource.reservations'");
+ }
+
+ if (resource.has_reservation() &&
+ resource.reservation().labels() != reservation.labels()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation.labels' field"
+ " with '" + stringify(resource.reservation().labels()) + "'"
+ " does not match the labels"
+ " '" + stringify(reservation.labels()) + "'"
+ " in 'Resource.reservations'");
+ }
+
+ break;
+ }
+ case Resource::ReservationInfo::UNKNOWN: {
+ return Error("Unsupported 'Resource.ReservationInfo.Type'");
+ }
+ }
+
+ } else {
+ CHECK_GE(resource.reservations_size(), 1);
+ if (resource.has_role()) {
+ return Error(
+ "Invalid resource format: 'Resource.role' must not be set if"
+ " there is more than one reservation in 'Resource.reservations'");
+ }
+
+ if (resource.has_reservation()) {
+ return Error(
+ "Invalid resource format: 'Resource.reservation' must not be set if"
+ " there is more than one reservation in 'Resource.reservations'");
+ }
+ }
}
// Check that shareability is enabled for supported resource types.
[4/4] mesos git commit: Avoided exposing `role` in JSON for resources
with refined reservations.
Posted by mp...@apache.org.
Avoided exposing `role` in JSON for resources with refined reservations.
With reservation refinement, we use the `Resource.reservations` field to
express the reservation state. Due to the fact that `Resource.role` is
an optional field __and__ has a default value, our generic Protobuf to
JSON code injects the `role` field with the default value even if it's
not set. Since this will likely confuse users, here we manually remove
the `Resource.role` field if the resource has refined reservations.
Review: https://reviews.apache.org/r/60036
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cf96042e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf96042e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf96042e
Branch: refs/heads/master
Commit: cf96042ecf4f294723882ed5dfda9d12e63d6519
Parents: f39f846
Author: Michael Park <mp...@apache.org>
Authored: Sun Jun 18 18:21:09 2017 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Sun Jun 18 20:06:21 2017 -0700
----------------------------------------------------------------------
src/common/http.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
src/common/http.hpp | 15 +++++++++++++++
src/master/http.cpp | 20 ++++++++++++++++----
src/slave/http.cpp | 10 ++++++++--
4 files changed, 85 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/cf96042e/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 167dce2..98899a7 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -112,8 +112,7 @@ string serialize(
return message.SerializeAsString();
}
case ContentType::JSON: {
- JSON::Object object = JSON::protobuf(message);
- return stringify(object);
+ return stringify(modelProtobufJSON(message));
}
case ContentType::RECORDIO: {
LOG(FATAL) << "Serializing a RecordIO stream is not supported";
@@ -505,6 +504,51 @@ JSON::Object model(const quota::QuotaInfo& quotaInfo)
return object;
}
+
+JSON::Object modelProtobufJSON(const google::protobuf::Message& message)
+{
+ struct
+ {
+ using result_type = void;
+
+ void operator()(JSON::Boolean&) const { /* No-op. */ }
+ void operator()(JSON::Number&) const { /* No-op. */ }
+ void operator()(JSON::String&) const { /* No-op. */ }
+ void operator()(JSON::Object& object) const
+ {
+ // We assume that if an object has fields "role" and "reservations"
+ // that it is a `Resource` object. While this is clearly not robust,
+ // it's good enough for what we need.
+ auto role_iter = object.values.find("role");
+ auto reservations_iter = object.values.find("reservations");
+ if (role_iter != object.values.end() &&
+ role_iter->second.is<JSON::String>() &&
+ reservations_iter != object.values.end() &&
+ reservations_iter->second.is<JSON::Array>() &&
+ reservations_iter->second.as<JSON::Array>().values.size() > 1) {
+ CHECK_EQ(role_iter->second.as<JSON::String>().value, "*");
+ object.values.erase(role_iter);
+ }
+
+ foreachvalue (JSON::Value& value, object.values) {
+ boost::apply_visitor(*this, value);
+ }
+ }
+ void operator()(JSON::Array& array) const
+ {
+ foreach (JSON::Value& value, array.values) {
+ boost::apply_visitor(*this, value);
+ }
+ }
+ void operator()(JSON::Null&) const { /* No-op. */ }
+ } removeRoleFromResourcesWithRefinedReservations;
+
+ JSON::Object object = JSON::protobuf(message);
+ removeRoleFromResourcesWithRefinedReservations(object);
+
+ return object;
+}
+
} // namespace internal {
void json(JSON::ObjectWriter* writer, const Attributes& attributes)
http://git-wip-us.apache.org/repos/asf/mesos/blob/cf96042e/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index 93d6088..48f7c4a 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -122,6 +122,21 @@ JSON::Object model(const Task& task);
JSON::Object model(const FileInfo& fileInfo);
JSON::Object model(const quota::QuotaInfo& quotaInfo);
+
+// With reservation refinement, we use the `Resource.reservations` field to
+// express the reservation state. Due to the fact that `Resource.role` is
+// an optional field __and__ has a default value, our generic Protobuf to JSON
+// conversion (JSON::protobuf) injects the `role` field with the default value
+// even if it's not set. Since this will likely confuse users and also result in
+// resources we consider invalid (i.e., `Resources::validate` would fail),
+// we introduce a wrapper here which manually removes the `Resource.role` field
+// if the resource has refined reservations.
+//
+// TODO(mpark): Remove this once MESOS-7674 is resolved and
+// `Resource.role` is deprecated.
+JSON::Object modelProtobufJSON(const google::protobuf::Message& message);
+
+
void json(JSON::ObjectWriter* writer, const Task& task);
} // namespace internal {
http://git-wip-us.apache.org/repos/asf/mesos/blob/cf96042e/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e01c3fc..4f83849 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2348,7 +2348,10 @@ Future<Response> Master::Http::slaves(
reserved) {
writer->field(role, [&resources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, resources) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
}
@@ -2361,7 +2364,10 @@ Future<Response> Master::Http::slaves(
"unreserved_resources_full",
[&unreservedResources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, unreservedResources) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
@@ -2371,7 +2377,10 @@ Future<Response> Master::Http::slaves(
"used_resources_full",
[&usedResources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, usedResources) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
@@ -2381,7 +2390,10 @@ Future<Response> Master::Http::slaves(
"offered_resources_full",
[&offeredResources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, offeredResources) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
});
http://git-wip-us.apache.org/repos/asf/mesos/blob/cf96042e/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 78b3586..e03b886 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -1297,7 +1297,10 @@ Future<Response> Http::state(
totalResources.reservations()) {
writer->field(role, [&resources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, resources) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
}
@@ -1307,7 +1310,10 @@ Future<Response> Http::state(
"unreserved_resources_full",
[&totalResources](JSON::ArrayWriter* writer) {
foreach (const Resource& resource, totalResources.unreserved()) {
- writer->element(JSON::Protobuf(resource));
+ // TODO(mpark): Replace the `modelProtobufJSON` back to
+ // `JSON::Protobuf` once MESOS-7674 is resolved and
+ // `Resource.role` is deprecated.
+ writer->element(modelProtobufJSON(resource));
}
});
[2/4] mesos git commit: Introduced a utility function
`Resources::hasRefinedReservations`.
Posted by mp...@apache.org.
Introduced a utility function `Resources::hasRefinedReservations`.
Review: https://reviews.apache.org/r/60175
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5124602b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5124602b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5124602b
Branch: refs/heads/master
Commit: 5124602beca53dea153781167121aaaedb30291e
Parents: 49e090d
Author: Michael Park <mp...@apache.org>
Authored: Sat Jun 17 21:03:30 2017 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Sun Jun 18 20:06:11 2017 -0700
----------------------------------------------------------------------
include/mesos/resources.hpp | 3 +++
include/mesos/v1/resources.hpp | 3 +++
src/common/resources.cpp | 5 +++++
src/v1/resources.cpp | 5 +++++
4 files changed, 16 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5124602b/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index d380a59..be2e377 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -306,6 +306,9 @@ public:
// Tests if the given Resource object is shared.
static bool isShared(const Resource& resource);
+ // Tests if the given Resource object has refined reservations.
+ static bool hasRefinedReservations(const Resource& resource);
+
// Returns the summed up Resources given a hashmap<Key, Resources>.
//
// NOTE: While scalar resources such as "cpus" sum correctly,
http://git-wip-us.apache.org/repos/asf/mesos/blob/5124602b/include/mesos/v1/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index 66c2b21..fe62166 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -306,6 +306,9 @@ public:
// Tests if the given Resource object is shared.
static bool isShared(const Resource& resource);
+ // Tests if the given Resource object has refined reservations.
+ static bool hasRefinedReservations(const Resource& resource);
+
// Returns the summed up Resources given a hashmap<Key, Resources>.
//
// NOTE: While scalar resources such as "cpus" sum correctly,
http://git-wip-us.apache.org/repos/asf/mesos/blob/5124602b/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 481c469..cc305e6 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -1080,6 +1080,11 @@ bool Resources::isShared(const Resource& resource)
}
+bool Resources::hasRefinedReservations(const Resource& resource)
+{
+ return resource.reservations_size() > 1;
+}
+
/////////////////////////////////////////////////
// Public member functions.
/////////////////////////////////////////////////
http://git-wip-us.apache.org/repos/asf/mesos/blob/5124602b/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 42e903a..03d6c80 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -1082,6 +1082,11 @@ bool Resources::isShared(const Resource& resource)
}
+bool Resources::hasRefinedReservations(const Resource& resource)
+{
+ return resource.reservations_size() > 1;
+}
+
/////////////////////////////////////////////////
// Public member functions.
/////////////////////////////////////////////////
[3/4] mesos git commit: Introduced `convertResourceFormat` to convert
between resource formats.
Posted by mp...@apache.org.
Introduced `convertResourceFormat` to convert between resource formats.
With reservation refinement, we introduced a new resource format to
enable creating a refined reservation on top of an existing reservation.
We still have the "pre-reservation-refinement" format which uses
the `Resource.role` and `Resource.reservation` fields, and we now also
have the "post-reservation-refinement" format which uses
the `Resource.reservations` field to represent the reservation state.
In order to simplify our code, we use `convertResourceFormat` to
canonicalize the resources into the "post-reservation-refinement" in
memory, and convert them back as necessary.
Review: https://reviews.apache.org/r/60022
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f39f8461
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f39f8461
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f39f8461
Branch: refs/heads/master
Commit: f39f846160ff934e42294ace2662eee2a8f658fb
Parents: 5124602
Author: Michael Park <mp...@apache.org>
Authored: Sat Jun 17 15:11:12 2017 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Sun Jun 18 20:06:16 2017 -0700
----------------------------------------------------------------------
src/common/resources_utils.cpp | 177 ++++++++++++++++++++++++++++++++++++
src/common/resources_utils.hpp | 95 +++++++++++++++++++
2 files changed, 272 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f39f8461/src/common/resources_utils.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources_utils.cpp b/src/common/resources_utils.cpp
index c308843..aca2a0c 100644
--- a/src/common/resources_utils.cpp
+++ b/src/common/resources_utils.cpp
@@ -83,4 +83,181 @@ Try<Resources> applyCheckpointedResources(
return totalResources;
}
+
+void convertResourceFormat(Resource* resource, ResourceFormat format)
+{
+ switch (format) {
+ case PRE_RESERVATION_REFINEMENT:
+ case ENDPOINT: {
+ CHECK(!resource->has_role());
+ CHECK(!resource->has_reservation());
+ switch (resource->reservations_size()) {
+ // Unreserved resource.
+ case 0: {
+ resource->set_role("*");
+ break;
+ }
+ // Resource with a single reservation.
+ case 1: {
+ const Resource::ReservationInfo& source = resource->reservations(0);
+
+ if (source.type() == Resource::ReservationInfo::DYNAMIC) {
+ Resource::ReservationInfo* target = resource->mutable_reservation();
+ if (source.has_principal()) {
+ target->set_principal(source.principal());
+ }
+ if (source.has_labels()) {
+ target->mutable_labels()->CopyFrom(source.labels());
+ }
+ }
+
+ resource->set_role(source.role());
+
+ if (format == PRE_RESERVATION_REFINEMENT) {
+ resource->clear_reservations();
+ }
+ break;
+ }
+ // Resource with refined reservations.
+ default: {
+ CHECK_NE(PRE_RESERVATION_REFINEMENT, format)
+ << "Invalid resource format conversion: A 'Resource' object"
+ " being converted to the PRE_RESERVATION_REFINEMENT format"
+ " must not have refined reservations";
+ }
+ }
+ break;
+ }
+ case POST_RESERVATION_REFINEMENT: {
+ if (resource->reservations_size() > 0) {
+ // In this case, we're either already in
+ // the "post-reservation-refinement" format,
+ // or we're in the "endpoint" format.
+
+ // We clear out the "pre-reservation-refinement" fields
+ // in case the resources are in the "endpoint" format.
+ resource->clear_role();
+ resource->clear_reservation();
+ return;
+ }
+
+ // Unreserved resources.
+ if (resource->role() == "*" && !resource->has_reservation()) {
+ resource->clear_role();
+ return;
+ }
+
+ // Resource with a single reservation.
+ Resource::ReservationInfo* reservation = resource->add_reservations();
+
+ // Check the `Resource.reservation` to determine whether
+ // we have a static or dynamic reservation.
+ if (!resource->has_reservation()) {
+ reservation->set_type(Resource::ReservationInfo::STATIC);
+ } else {
+ reservation->CopyFrom(resource->reservation());
+ resource->clear_reservation();
+ reservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ }
+
+ reservation->set_role(resource->role());
+ resource->clear_role();
+ break;
+ }
+ }
+}
+
+
+void convertResourceFormat(
+ google::protobuf::RepeatedPtrField<Resource>* resources,
+ ResourceFormat format)
+{
+ foreach (Resource& resource, *resources) {
+ convertResourceFormat(&resource, format);
+ }
+}
+
+
+void convertResourceFormat(
+ std::vector<Resource>* resources,
+ ResourceFormat format)
+{
+ foreach (Resource& resource, *resources) {
+ convertResourceFormat(&resource, format);
+ }
+}
+
+
+void convertResourceFormat(Offer::Operation* operation, ResourceFormat format)
+{
+ switch (operation->type()) {
+ case Offer::Operation::LAUNCH: {
+ Offer::Operation::Launch* launch = operation->mutable_launch();
+
+ foreach (TaskInfo& task, *launch->mutable_task_infos()) {
+ convertResourceFormat(task.mutable_resources(), format);
+
+ if (task.has_executor()) {
+ convertResourceFormat(
+ task.mutable_executor()->mutable_resources(), format);
+ }
+ }
+ break;
+ }
+
+ case Offer::Operation::LAUNCH_GROUP: {
+ Offer::Operation::LaunchGroup* launchGroup =
+ operation->mutable_launch_group();
+
+ if (launchGroup->has_executor()) {
+ convertResourceFormat(
+ launchGroup->mutable_executor()->mutable_resources(), format);
+ }
+
+ TaskGroupInfo* taskGroup = launchGroup->mutable_task_group();
+
+ foreach (TaskInfo& task, *taskGroup->mutable_tasks()) {
+ convertResourceFormat(task.mutable_resources(), format);
+
+ if (task.has_executor()) {
+ convertResourceFormat(
+ task.mutable_executor()->mutable_resources(), format);
+ }
+ }
+ break;
+ }
+
+ case Offer::Operation::RESERVE: {
+ convertResourceFormat(
+ operation->mutable_reserve()->mutable_resources(), format);
+
+ break;
+ }
+
+ case Offer::Operation::UNRESERVE: {
+ convertResourceFormat(
+ operation->mutable_unreserve()->mutable_resources(), format);
+
+ break;
+ }
+
+ case Offer::Operation::CREATE: {
+ convertResourceFormat(
+ operation->mutable_create()->mutable_volumes(), format);
+
+ break;
+ }
+
+ case Offer::Operation::DESTROY: {
+ convertResourceFormat(
+ operation->mutable_destroy()->mutable_volumes(), format);
+
+ break;
+ }
+
+ case Offer::Operation::UNKNOWN:
+ break; // No-op.
+ }
+}
+
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/f39f8461/src/common/resources_utils.hpp
----------------------------------------------------------------------
diff --git a/src/common/resources_utils.hpp b/src/common/resources_utils.hpp
index 2840f45..c7ec3bd 100644
--- a/src/common/resources_utils.hpp
+++ b/src/common/resources_utils.hpp
@@ -17,6 +17,10 @@
#ifndef __RESOURCES_UTILS_HPP__
#define __RESOURCES_UTILS_HPP__
+#include <vector>
+
+#include <google/protobuf/repeated_field.h>
+
#include <mesos/mesos.hpp>
#include <mesos/resources.hpp>
@@ -38,6 +42,97 @@ Try<Resources> applyCheckpointedResources(
const Resources& resources,
const Resources& checkpointedResources);
+
+// Resource format options to be used with the `convertResourceFormat` function.
+//
+// The preconditions of the options are asymmetric, centered around the
+// "post-reservation-refinement" format. This is mainly due to the fact that
+// "post-reservation-refinement" format is our canonical representation.
+// The transformations are generally applied to any of the 3 formats to be
+// converted to the canonical format, then later converted back as necessary.
+//
+// See 'Resource Format' section in `mesos.proto` for more details.
+enum ResourceFormat
+{
+ // "post-reservation-refinement" -> "pre-reservation-refinement"
+ //
+ // The `Resource` objects must be in the "post-reservation-refinement" format,
+ // and must not have refined reservations.
+ //
+ // All resources end up with the `Resource.role` and `Resource.reservation`
+ // fields set, and the `Resource.reservations` field unset.
+ //
+ // We convert the resources to the "pre-reservation-refinement" format to
+ // checkpoint resources for example. This enables downgrading to an agent
+ // without a RESERVATION_REFINEMENT caapbility, since the resources will
+ // be checkpointed in a format that the downgraded agent can recover from.
+ PRE_RESERVATION_REFINEMENT,
+
+ // "pre-reservation-refinement" -> "post-reservation-refinement"
+ // "post-reservation-refinement" -> "post-reservation-refinement"
+ // "endpoint" -> "post-reservation-refinement"
+ //
+ // The `Resource` objects can be in any of the valid resource formats:
+ // "pre-reservation-refinement", "post-reservation-refinement", "endpoint".
+ //
+ // All resources end up with the `Resource.reservations` field set,
+ // and the `Resource.role` and `Resource.reservation` fields unset.
+ //
+ // If the `Resource` objects are already in the "post-reservation-refinement"
+ // format, this is a no-op.
+ //
+ // We convert the resources to the "post-reservation-refinement" format,
+ // for example, when a master receives a message from an agent without
+ // the RESERVATION_REFINEMENT capability. This allows a component
+ // (e.g. master) code to deal with a canonical format to simplify the code.
+ POST_RESERVATION_REFINEMENT,
+
+ // "post-reservation-refinement" -> "endpoint"
+ //
+ // This is a special case for endpoints, which injects
+ // the "pre-reservation-refinement" format.
+ //
+ // The `Resource` objects must be in the "post-reservation-refinement" format.
+ //
+ // All resources continue to have the `Resource.reservations` field set.
+ // The `Resource` objects without refined reservations end up with the
+ // `Resource.role` and `Resource.reservation` fields set, and the objects
+ // with refined reservations have them unset.
+ //
+ // We inject the resources with the "pre-reservation-refinement" format to
+ // enable backward compatibility with external tooling. If the master has been
+ // upgraded to a version that supports reservation refinement but no refined
+ // reservations have been made, the endpoints will return the data in both new
+ // and old formats to maximize backward compatibility. However, once
+ // a reservation refinement is made to a resource, that resource is only
+ // returned in the new format.
+ ENDPOINT
+};
+
+
+// Converts the given `Resource` to the specified `ResourceFormat`.
+//
+// See the "Resource Format" section in `mesos.proto` for more details.
+// See the `ResourceFormat` enum above for more details.
+void convertResourceFormat(Resource* resource, ResourceFormat format);
+
+
+// Converts the given `Resource`s to the specified `ResourceFormat`.
+void convertResourceFormat(
+ google::protobuf::RepeatedPtrField<Resource>* resources,
+ ResourceFormat format);
+
+
+// Converts the given `Resource`s to the specified `ResourceFormat`.
+void convertResourceFormat(
+ std::vector<Resource>* resources,
+ ResourceFormat format);
+
+
+// Converts all the `Resource` objects in the given `Operation`
+// to the specified `ResourceFormat`.
+void convertResourceFormat(Offer::Operation* operation, ResourceFormat format);
+
} // namespace mesos {
#endif // __RESOURCES_UTILS_HPP__