Repository: mesos Updated Branches: refs/heads/master f9cb6f496 -> 491371c77
Avoided resource validation when flattening resources. Instead of validating each resource, in flatten(...) we validate the arguments just once. We can then use validation-free `add()` instead of `+=`. 1) Previously if role or reservation info are invalid, the result is an empty Resources object, now it returns an error. 2) Added a new flatten() method to flatten resources with star role and empty reservation. Review: https://reviews.apache.org/r/51683/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/491371c7 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/491371c7 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/491371c7 Branch: refs/heads/master Commit: 491371c773cec8b565fe643f33682ee5cce267c7 Parents: f9cb6f4 Author: Guangya Liu <gyliu...@gmail.com> Authored: Fri Sep 16 15:50:09 2016 -0700 Committer: Jiang Yan Xu <xuj...@apple.com> Committed: Fri Sep 16 16:06:44 2016 -0700 ---------------------------------------------------------------------- include/mesos/resources.hpp | 10 ++- include/mesos/v1/resources.hpp | 10 ++- src/cli/execute.cpp | 8 ++- src/common/resources.cpp | 37 +++++++++-- src/examples/dynamic_reservation_framework.cpp | 5 +- src/examples/test_framework.cpp | 5 +- src/examples/test_http_framework.cpp | 5 +- src/tests/api_tests.cpp | 4 +- src/tests/hierarchical_allocator_tests.cpp | 6 +- src/tests/mesos.hpp | 2 +- src/tests/persistent_volume_endpoints_tests.cpp | 18 +++--- src/tests/reservation_endpoints_tests.cpp | 48 +++++++-------- src/tests/reservation_tests.cpp | 64 +++++++++++++------- src/tests/resources_tests.cpp | 24 +++++++- src/tests/role_tests.cpp | 3 +- src/v1/resources.cpp | 35 +++++++++-- 16 files changed, 200 insertions(+), 84 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/include/mesos/resources.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp index 7ba422d..3ef8cac 100644 --- a/include/mesos/resources.hpp +++ b/include/mesos/resources.hpp @@ -348,10 +348,16 @@ public: // If the optional ReservationInfo is given, the resource's // 'reservation' field is set. Otherwise, the resource's // 'reservation' field is cleared. - Resources flatten( - const std::string& role = "*", + // Returns an Error when the role is invalid or the reservation + // is set when the role is '*'. + Try<Resources> flatten( + const std::string& role, const Option<Resource::ReservationInfo>& reservation = None()) const; + // Equivalent to `flatten("*")` except it returns a Resources directly + // because the result is always a valid in this case. + Resources flatten() const; + // Returns a Resources object that contains all the scalar resources // in this object, but with their ReservationInfo and DiskInfo // omitted. Note that the `role` and RevocableInfo, if any, are http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/include/mesos/v1/resources.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp index add48c7..ef56b49 100644 --- a/include/mesos/v1/resources.hpp +++ b/include/mesos/v1/resources.hpp @@ -348,10 +348,16 @@ public: // If the optional ReservationInfo is given, the resource's // 'reservation' field is set. Otherwise, the resource's // 'reservation' field is cleared. - Resources flatten( - const std::string& role = "*", + // Returns an Error when the role is invalid or the reservation + // is set when the role is '*'. + Try<Resources> flatten( + const std::string& role, const Option<Resource::ReservationInfo>& reservation = None()) const; + // Equivalent to `flatten("*")` except it returns a Resources directly + // because the result is always a valid in this case. + Resources flatten() const; + // Returns a Resources object that contains all the scalar resources // in this object, but with their ReservationInfo and DiskInfo // omitted. Note that the `role` and RevocableInfo, if any, are http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/cli/execute.cpp ---------------------------------------------------------------------- diff --git a/src/cli/execute.cpp b/src/cli/execute.cpp index c9f56af..525c898 100644 --- a/src/cli/execute.cpp +++ b/src/cli/execute.cpp @@ -373,8 +373,12 @@ protected: task.mutable_agent_id()->MergeFrom(offer.agent_id()); // Takes resources first from the specified role, then from '*'. - Option<Resources> resources = - offered.find(TASK_RESOURCES.get().flatten(frameworkInfo.role())); + Try<Resources> flattened = + TASK_RESOURCES.get().flatten(frameworkInfo.role()); + + // `frameworkInfo.role()` must be valid as it's allowed to register. + CHECK_SOME(flattened); + Option<Resources> resources = offered.find(flattened.get()); CHECK_SOME(resources); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/common/resources.cpp ---------------------------------------------------------------------- diff --git a/src/common/resources.cpp b/src/common/resources.cpp index aa4d6b5..4602bff 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -1077,26 +1077,48 @@ Resources Resources::nonShared() const } -Resources Resources::flatten( +Try<Resources> Resources::flatten( const string& role, const Option<Resource::ReservationInfo>& reservation) const { + // Check role name. + Option<Error> error = roles::validate(role); + if (error.isSome()) { + return error.get(); + } + + // Checks for the invalid state of (role, reservation) pair. + if (role == "*" && reservation.isSome()) { + return Error( + "Invalid reservation: role \"*\" cannot be dynamically reserved"); + } + Resources flattened; foreach (Resource_ resource_, resources) { + // With the above checks, we are certain that `resource_` will + // remain valid after the modifications. resource_.resource.set_role(role); if (reservation.isNone()) { resource_.resource.clear_reservation(); } else { resource_.resource.mutable_reservation()->CopyFrom(reservation.get()); } - flattened += resource_; + flattened.add(resource_); } return flattened; } +Resources Resources::flatten() const +{ + Try<Resources> flattened = flatten("*"); + CHECK_SOME(flattened); + return flattened.get(); +} + + Resources Resources::createStrippedScalarQuantity() const { Resources stripped; @@ -1507,10 +1529,17 @@ Option<Resources> Resources::find(const Resource& target) const if (flattened.contains(remaining)) { // The target has been found, return the result. if (!resource_.resource.has_reservation()) { - return found + remaining.flatten(resource_.resource.role()); + Try<Resources> _flattened = + remaining.flatten(resource_.resource.role()); + + CHECK_SOME(_flattened); + return found + _flattened.get(); } else { - return found + remaining.flatten( + Try<Resources> _flattened = remaining.flatten( resource_.resource.role(), resource_.resource.reservation()); + + CHECK_SOME(_flattened); + return found + _flattened.get(); } } else if (remaining.contains(flattened)) { found.add(resource_); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/examples/dynamic_reservation_framework.cpp ---------------------------------------------------------------------- diff --git a/src/examples/dynamic_reservation_framework.cpp b/src/examples/dynamic_reservation_framework.cpp index 850bb2a..c9a6863 100644 --- a/src/examples/dynamic_reservation_framework.cpp +++ b/src/examples/dynamic_reservation_framework.cpp @@ -61,7 +61,10 @@ public: principal(_principal) { reservationInfo.set_principal(principal); - taskResources = TASK_RESOURCES.flatten(role, reservationInfo); + + Try<Resources> flattened = TASK_RESOURCES.flatten(role, reservationInfo); + CHECK_SOME(flattened); + taskResources = flattened.get(); } virtual ~DynamicReservationScheduler() {} http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/examples/test_framework.cpp ---------------------------------------------------------------------- diff --git a/src/examples/test_framework.cpp b/src/examples/test_framework.cpp index a83766a..a910640 100644 --- a/src/examples/test_framework.cpp +++ b/src/examples/test_framework.cpp @@ -106,8 +106,9 @@ public: task.mutable_slave_id()->MergeFrom(offer.slave_id()); task.mutable_executor()->MergeFrom(executor); - Option<Resources> resources = - remaining.find(TASK_RESOURCES.flatten(role)); + Try<Resources> flattened = TASK_RESOURCES.flatten(role); + CHECK_SOME(flattened); + Option<Resources> resources = remaining.find(flattened.get()); CHECK_SOME(resources); task.mutable_resources()->MergeFrom(resources.get()); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/examples/test_http_framework.cpp ---------------------------------------------------------------------- diff --git a/src/examples/test_http_framework.cpp b/src/examples/test_http_framework.cpp index 441e86c..d6e248c 100644 --- a/src/examples/test_http_framework.cpp +++ b/src/examples/test_http_framework.cpp @@ -244,8 +244,9 @@ private: task.mutable_agent_id()->MergeFrom(offer.agent_id()); task.mutable_executor()->MergeFrom(executor); - Option<Resources> resources = - remaining.find(TASK_RESOURCES.flatten(framework.role())); + Try<Resources> flattened = TASK_RESOURCES.flatten(framework.role()); + CHECK_SOME(flattened); + Option<Resources> resources = remaining.find(flattened.get()); CHECK_SOME(resources); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/api_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp index e440d1b..26f99f7 100644 --- a/src/tests/api_tests.cpp +++ b/src/tests/api_tests.cpp @@ -966,7 +966,7 @@ TEST_P(MasterAPITest, ReserveResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); MockScheduler sched; MesosSchedulerDriver driver( @@ -1060,7 +1060,7 @@ TEST_P(MasterAPITest, UnreserveResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); v1::master::Call v1Call; v1Call.set_type(v1::master::Call::RESERVE_RESOURCES); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/hierarchical_allocator_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp index 621f1f4..46553f6 100644 --- a/src/tests/hierarchical_allocator_tests.cpp +++ b/src/tests/hierarchical_allocator_tests.cpp @@ -1479,7 +1479,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateAvailableSuccess) // Construct an offer operation for the framework's allocation. Resources unreserved = Resources::parse("cpus:25;mem:50").get(); Resources dynamicallyReserved = - unreserved.flatten("role1", createReservationInfo("ops")); + unreserved.flatten("role1", createReservationInfo("ops")).get(); Offer::Operation reserve = RESERVE(dynamicallyReserved); @@ -1532,7 +1532,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateAvailableFail) // Construct an offer operation for the framework's allocation. Resources unreserved = Resources::parse("cpus:25;mem:50").get(); Resources dynamicallyReserved = - unreserved.flatten("role1", createReservationInfo("ops")); + unreserved.flatten("role1", createReservationInfo("ops")).get(); Offer::Operation reserve = RESERVE(dynamicallyReserved); @@ -2633,7 +2633,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaSetAsideReservedResources) // Reserve 4 CPUs and 512MB of memory on `agent2` for non-quota'ed role. Resources unreserved = Resources::parse("cpus:4;mem:512").get(); Resources dynamicallyReserved = - unreserved.flatten(NO_QUOTA_ROLE, createReservationInfo("ops")); + unreserved.flatten(NO_QUOTA_ROLE, createReservationInfo("ops")).get(); Offer::Operation reserve = RESERVE(dynamicallyReserved); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index fa6789b..7095101 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -844,7 +844,7 @@ ACTION_P5(LaunchTasks, executor, tasks, cpus, mem, role) task.mutable_executor()->MergeFrom(executor); Option<Resources> resources = - remaining.find(TASK_RESOURCES.flatten(role)); + remaining.find(TASK_RESOURCES.flatten(role).get()); CHECK_SOME(resources); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/persistent_volume_endpoints_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/persistent_volume_endpoints_tests.cpp b/src/tests/persistent_volume_endpoints_tests.cpp index 266c2a0..f35592a 100644 --- a/src/tests/persistent_volume_endpoints_tests.cpp +++ b/src/tests/persistent_volume_endpoints_tests.cpp @@ -231,7 +231,7 @@ TEST_F(PersistentVolumeEndpointsTest, DynamicReservation) Resources unreserved = Resources::parse("disk:1024").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -351,7 +351,7 @@ TEST_F(PersistentVolumeEndpointsTest, DynamicReservationRoleMismatch) Resources unreserved = Resources::parse("disk:1024").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -432,7 +432,7 @@ TEST_F(PersistentVolumeEndpointsTest, UnreserveVolumeResources) Resources unreserved = Resources::parse("disk:1024").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -1557,7 +1557,7 @@ TEST_F(PersistentVolumeEndpointsTest, OfferCreateThenEndpointRemove) Resources unreserved = Resources::parse("disk:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); EXPECT_CALL(allocator, addFramework(_, _, _)); @@ -1709,7 +1709,7 @@ TEST_F(PersistentVolumeEndpointsTest, EndpointCreateThenOfferRemove) Resources unreserved = Resources::parse("disk:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -1843,7 +1843,7 @@ TEST_F(PersistentVolumeEndpointsTest, ReserveAndSlaveRemoval) Resources slave1Unreserved = Resources::parse("cpus:4").get(); Resources slave1Reserved = slave1Unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -1882,7 +1882,7 @@ TEST_F(PersistentVolumeEndpointsTest, ReserveAndSlaveRemoval) Resources slave2Unreserved = Resources::parse("cpus:3").get(); Resources slave2Reserved = slave2Unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); for (size_t i = 0; i < offers.get().size(); i++) { Offer offer = offers.get()[i]; @@ -1962,7 +1962,7 @@ TEST_F(PersistentVolumeEndpointsTest, SlavesEndpointFullResources) Resources unreserved = Resources::parse("cpus:1;mem:512;disk:1024").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -2014,7 +2014,7 @@ TEST_F(PersistentVolumeEndpointsTest, SlavesEndpointFullResources) Resources taskUnreserved = Resources::parse("cpus:1;mem:256").get(); Resources taskResources = taskUnreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); TaskInfo taskInfo = createTask(offer.slave_id(), taskResources, "sleep 1000"); http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/reservation_endpoints_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/reservation_endpoints_tests.cpp b/src/tests/reservation_endpoints_tests.cpp index bee5ea6..28b497e 100644 --- a/src/tests/reservation_endpoints_tests.cpp +++ b/src/tests/reservation_endpoints_tests.cpp @@ -125,7 +125,7 @@ TEST_F(ReservationEndpointsTest, AvailableResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -222,7 +222,7 @@ TEST_F(ReservationEndpointsTest, ReserveOfferedResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); MockScheduler sched; MesosSchedulerDriver driver( @@ -295,7 +295,7 @@ TEST_F(ReservationEndpointsTest, UnreserveOfferedResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -383,7 +383,7 @@ TEST_F(ReservationEndpointsTest, ReserveAvailableAndOfferedResources) Resources total = available + offered; Resources dynamicallyReserved = total.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); MockScheduler sched; MesosSchedulerDriver driver( @@ -530,12 +530,12 @@ TEST_F(ReservationEndpointsTest, UnreserveAvailableAndOfferedResources) Resources available = Resources::parse("cpus:1;mem:128").get(); available = available.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources offered = Resources::parse("mem:384").get(); offered = offered.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources total = available + offered; Resources unreserved = total.flatten(); @@ -700,10 +700,10 @@ TEST_F(ReservationEndpointsTest, LabeledResources) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources labeledResources1 = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal(), labels1)); + createReservationInfo(DEFAULT_CREDENTIAL.principal(), labels1)).get(); Resources labeledResources2 = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal(), labels2)); + createReservationInfo(DEFAULT_CREDENTIAL.principal(), labels2)).get(); // Make two resource reservations with different labels. Future<Response> response = process::http::post( @@ -900,7 +900,7 @@ TEST_F(ReservationEndpointsTest, InsufficientResources) Resources unreserved = Resources::parse("cpus:4;mem:4096").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL); string body = createRequestBody(slaveId.get(), dynamicallyReserved); @@ -941,7 +941,7 @@ TEST_F(ReservationEndpointsTest, NoHeader) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -991,7 +991,7 @@ TEST_F(ReservationEndpointsTest, BadCredentials) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - "role", createReservationInfo(DEFAULT_CREDENTIAL.principal())); + "role", createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); process::http::Headers headers = createBasicAuthHeaders(credential); string body = createRequestBody(slaveId.get(), dynamicallyReserved); @@ -1063,7 +1063,7 @@ TEST_F(ReservationEndpointsTest, GoodReserveAndUnreserveACL) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); // Reserve the resources. Future<Response> response = process::http::post( @@ -1123,10 +1123,10 @@ TEST_F(ReservationEndpointsTest, GoodReserveACLMultipleRoles) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved1 = unreserved.flatten( "jedi_master", - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources dynamicallyReserved2 = unreserved.flatten( "sith_lord", - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources dynamicallyReservedMultipleRoles = dynamicallyReserved1 + dynamicallyReserved2; @@ -1186,7 +1186,7 @@ TEST_F(ReservationEndpointsTest, BadReserveACL) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); // Attempt to reserve the resources. Future<Response> response = process::http::post( @@ -1245,7 +1245,7 @@ TEST_F(ReservationEndpointsTest, BadUnreserveACL) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); // Reserve the resources. Future<Response> response = process::http::post( @@ -1316,10 +1316,10 @@ TEST_F(ReservationEndpointsTest, BadReserveACLMultipleRoles) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved1 = unreserved.flatten( AUTHORIZED_ROLE, - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources dynamicallyReserved2 = unreserved.flatten( UNAUTHORIZED_ROLE, - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources dynamicallyReservedMultipleRoles = dynamicallyReserved1 + dynamicallyReserved2; @@ -1347,7 +1347,7 @@ TEST_F(ReservationEndpointsTest, NoSlaveId) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - "role", createReservationInfo(DEFAULT_CREDENTIAL.principal())); + "role", createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); process::http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL); string body = @@ -1422,7 +1422,7 @@ TEST_F(ReservationEndpointsTest, NonMatchingPrincipal) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = - unreserved.flatten("role", createReservationInfo("badPrincipal")); + unreserved.flatten("role", createReservationInfo("badPrincipal")).get(); Future<Response> response = process::http::post( master.get()->pid, @@ -1474,7 +1474,7 @@ TEST_F(ReservationEndpointsTest, ReserveAndUnreserveNoAuthentication) Resources dynamicallyReservedWithNoPrincipal = unreserved.flatten( frameworkInfo.role(), - createReservationInfo()); + createReservationInfo()).get(); // Try a reservation with no principal in `ReservationInfo` and no // authentication headers. @@ -1498,7 +1498,7 @@ TEST_F(ReservationEndpointsTest, ReserveAndUnreserveNoAuthentication) Resources dynamicallyReservedWithPrincipal = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); // Try a reservation with a principal in `ReservationInfo` and no // authentication headers. @@ -1551,11 +1551,11 @@ TEST_F(ReservationEndpointsTest, DifferentPrincipalsSameRole) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved1 = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL.principal())); + createReservationInfo(DEFAULT_CREDENTIAL.principal())).get(); Resources dynamicallyReserved2 = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(DEFAULT_CREDENTIAL_2.principal())); + createReservationInfo(DEFAULT_CREDENTIAL_2.principal())).get(); Future<Response> response = process::http::post( master.get()->pid, http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/reservation_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/reservation_tests.cpp b/src/tests/reservation_tests.cpp index 93e95a9..6c28ab4 100644 --- a/src/tests/reservation_tests.cpp +++ b/src/tests/reservation_tests.cpp @@ -115,7 +115,8 @@ TEST_F(ReservationTest, ReserveThenUnreserve) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -209,7 +210,7 @@ TEST_F(ReservationTest, ReserveTwiceWithDoubleValue) Resources dynamicallyReserved = unreserved.flatten( frameworkInfo.role(), - createReservationInfo(frameworkInfo.principal())); + createReservationInfo(frameworkInfo.principal())).get(); Future<vector<Offer>> offers; EXPECT_CALL(sched, resourceOffers(&driver, _)) @@ -262,7 +263,7 @@ TEST_F(ReservationTest, ReserveTwiceWithDoubleValue) Resources finalReservation = reserved.flatten( frameworkInfo.role(), - createReservationInfo(frameworkInfo.principal())); + createReservationInfo(frameworkInfo.principal())).get(); EXPECT_TRUE(Resources(offer.resources()).contains(finalReservation)); @@ -309,7 +310,8 @@ TEST_F(ReservationTest, ReserveAndLaunchThenUnreserve) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -430,7 +432,7 @@ TEST_F(ReservationTest, ReserveShareWithinRole) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - role, createReservationInfo(frameworkInfo1.principal())); + role, createReservationInfo(frameworkInfo1.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -539,7 +541,8 @@ TEST_F(ReservationTest, DropReserveTooLarge) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources unreservedTooLarge = Resources::parse("cpus:1;mem:1024").get(); Resources dynamicallyReservedTooLarge = unreservedTooLarge.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture the offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -629,7 +632,8 @@ TEST_F(ReservationTest, DropReserveStaticReservation) Resources staticallyReserved = Resources::parse("cpus(role):1;mem(role):512").get(); Resources dynamicallyReserved = staticallyReserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -712,11 +716,13 @@ TEST_F(ReservationTest, SendingCheckpointResourcesMessage) Resources unreserved1 = Resources::parse("cpus:8").get(); Resources reserved1 = unreserved1.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); Resources unreserved2 = Resources::parse("mem:2048").get(); Resources reserved2 = unreserved2.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -812,7 +818,8 @@ TEST_F(ReservationTest, ResourcesCheckpointing) Resources unreserved = Resources::parse("cpus:8;mem:2048").get(); Resources reserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -901,7 +908,8 @@ TEST_F(ReservationTest, MasterFailover) Resources unreserved = Resources::parse("cpus:8;mem:2048").get(); Resources reserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1018,7 +1026,8 @@ TEST_F(ReservationTest, CompatibleCheckpointedResources) Resources unreserved = Resources::parse("cpus:8;mem:2048").get(); Resources reserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1124,7 +1133,8 @@ TEST_F(ReservationTest, CompatibleCheckpointedResourcesWithPersistentVolumes) Resources unreserved = Resources::parse("cpus:8;mem:2048").get(); Resources reserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); Resource unreservedDisk = Resources::parse("disk", "1024", "*").get(); Resource reservedDisk = unreservedDisk; @@ -1273,7 +1283,8 @@ TEST_F(ReservationTest, IncompatibleCheckpointedResources) Resources unreserved = Resources::parse("cpus:8;mem:2048").get(); Resources reserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1395,7 +1406,8 @@ TEST_F(ReservationTest, GoodACLReserveThenUnreserve) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1494,7 +1506,8 @@ TEST_F(ReservationTest, BadACLDropReserve) Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1585,10 +1598,13 @@ TEST_F(ReservationTest, BadACLDropUnreserve) // Define the resources to be reserved. Resources unreserved1 = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved1 = unreserved1.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); + Resources unreserved2 = Resources::parse("cpus:0.5;mem:256").get(); Resources dynamicallyReserved2 = unreserved2.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1716,10 +1732,13 @@ TEST_F(ReservationTest, ACLMultipleOperations) // Define the resources to be reserved. Resources unreserved1 = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved1 = unreserved1.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); + Resources unreserved2 = Resources::parse("cpus:0.5;mem:256").get(); Resources dynamicallyReserved2 = unreserved2.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from 'resourceOffers'. Future<vector<Offer>> offers; @@ -1903,7 +1922,7 @@ TEST_F(ReservationTest, WithoutAuthenticationWithoutPrincipal) // Create dynamically reserved resources whose `ReservationInfo` does not // contain a principal. Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo()); + frameworkInfo.role(), createReservationInfo()).get(); // We use this to capture offers from `resourceOffers`. Future<vector<Offer>> offers; @@ -2005,7 +2024,8 @@ TEST_F(ReservationTest, WithoutAuthenticationWithPrincipal) // Create dynamically reserved resources whose `ReservationInfo` contains a // principal. Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from `resourceOffers`. Future<vector<Offer>> offers; http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/resources_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp index 157b9f6..3e49300 100644 --- a/src/tests/resources_tests.cpp +++ b/src/tests/resources_tests.cpp @@ -2184,7 +2184,7 @@ TEST(ResourcesOperationTest, ReserveResources) Resources unreserved = unreservedCpus + unreservedMem; Resources reservedCpus1 = - unreservedCpus.flatten("role", createReservationInfo("principal")); + unreservedCpus.flatten("role", createReservationInfo("principal")).get(); EXPECT_SOME_EQ(unreservedMem + reservedCpus1, unreserved.apply(RESERVE(reservedCpus1))); @@ -2272,7 +2272,7 @@ TEST(ResourcesOperationTest, StrippedResourcesReserved) { Resources unreserved = Resources::parse("cpus:1;mem:512").get(); Resources dynamicallyReserved = unreserved.flatten( - "role", createReservationInfo("principal")); + "role", createReservationInfo("principal")).get(); Resources stripped = dynamicallyReserved.createStrippedScalarQuantity(); @@ -2361,6 +2361,24 @@ TEST(ResourcesOperationTest, CreateSharedPersistentVolume) } +TEST(ResourcesOperationTest, FlattenResources) +{ + Resources unreservedCpus = Resources::parse("cpus:1").get(); + Resources unreservedMem = Resources::parse("mem:512").get(); + + Resources unreserved = unreservedCpus + unreservedMem; + + EXPECT_ERROR(unreserved.flatten("*", createReservationInfo("principal"))); + EXPECT_ERROR(unreserved.flatten("-role")); + + Resources reservedCpus = + unreservedCpus.flatten("role", createReservationInfo("principal")).get(); + + EXPECT_SOME_EQ(unreservedMem + reservedCpus, + unreserved.apply(RESERVE(reservedCpus))); +} + + // Helper for creating a revocable resource. static Resource createRevocableResource( const string& name, @@ -2762,7 +2780,7 @@ public: reservation.mutable_labels()->add_labels()->CopyFrom(label); reservations.resources += - scalars.resources.flatten(stringify(i), reservation); + scalars.resources.flatten(stringify(i), reservation).get(); } reservations.totalOperations = 10; http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/tests/role_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/role_tests.cpp b/src/tests/role_tests.cpp index 162c941..47fe0f0 100644 --- a/src/tests/role_tests.cpp +++ b/src/tests/role_tests.cpp @@ -116,7 +116,8 @@ TEST_F(RoleTest, ImplicitRoleRegister) Resources unreserved = Resources::parse("disk:1024").get(); Resources dynamicallyReserved = unreserved.flatten( - frameworkInfo.role(), createReservationInfo(frameworkInfo.principal())); + frameworkInfo.role(), + createReservationInfo(frameworkInfo.principal())).get(); // We use this to capture offers from `resourceOffers`. Future<vector<Offer>> offers; http://git-wip-us.apache.org/repos/asf/mesos/blob/491371c7/src/v1/resources.cpp ---------------------------------------------------------------------- diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp index 4b66acb..aefba49 100644 --- a/src/v1/resources.cpp +++ b/src/v1/resources.cpp @@ -1080,10 +1080,22 @@ Resources Resources::nonShared() const } -Resources Resources::flatten( +Try<Resources> Resources::flatten( const string& role, const Option<Resource::ReservationInfo>& reservation) const { + // Check role name. + Option<Error> error = roles::validate(role); + if (error.isSome()) { + return error.get(); + } + + // Checks for the invalid state of (role, reservation) pair. + if (role == "*" && reservation.isSome()) { + return Error( + "Invalid reservation: role \"*\" cannot be dynamically reserved"); + } + Resources flattened; foreach (Resource_ resource_, resources) { @@ -1093,13 +1105,21 @@ Resources Resources::flatten( } else { resource_.resource.mutable_reservation()->CopyFrom(reservation.get()); } - flattened += resource_; + flattened.add(resource_); } return flattened; } +Resources Resources::flatten() const +{ + Try<Resources> flattened = flatten("*"); + CHECK_SOME(flattened); + return flattened.get(); +} + + Resources Resources::createStrippedScalarQuantity() const { Resources stripped; @@ -1510,10 +1530,17 @@ Option<Resources> Resources::find(const Resource& target) const if (flattened.contains(remaining)) { // The target has been found, return the result. if (!resource_.resource.has_reservation()) { - return found + remaining.flatten(resource_.resource.role()); + Try<Resources> _flattened = + remaining.flatten(resource_.resource.role()); + + CHECK_SOME(_flattened); + return found + _flattened.get(); } else { - return found + remaining.flatten( + Try<Resources> _flattened = remaining.flatten( resource_.resource.role(), resource_.resource.reservation()); + + CHECK_SOME(_flattened); + return found + _flattened.get(); } } else if (remaining.contains(flattened)) { found.add(resource_);
