This is an automated email from the ASF dual-hosted git repository. tillt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 3501ac59fbe407e4d6372c69af283ea1cb7d9ae0 Author: Till Toenshoff <[email protected]> AuthorDate: Tue Nov 20 14:45:50 2018 +0100 Added collectAuthorizations helper to master.hpp. Adds the helper function 'collectAuthorizations' to master.hpp. This function allows for a simple way to collect authorization futures and only if all supplied futures result in an approved authorization will the returned future return true. All identified areas that were formally triggering MESOS-9317 are being updated to make use of this new helper. A helper function has been chosen and preferred over copying this pattern into the areas that needed a fix to allow for an efficient and complete test coverage. Additionally we are adding a test validating that new helper. Review: https://reviews.apache.org/r/69369/ --- src/master/master.cpp | 54 ++++----------------------------------- src/master/master.hpp | 14 +++++++++++ src/master/weights_handler.cpp | 12 +-------- src/tests/master_tests.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 60 deletions(-) diff --git a/src/master/master.cpp b/src/master/master.cpp index 9458ff1..6e34cc4 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -3641,17 +3641,7 @@ Future<bool> Master::authorizeReserveResources( return authorizer.get()->authorized(request); } - return await(authorizations) - .then([](const vector<Future<bool>>& authorizations) - -> Future<bool> { - // Compute a disjunction. - foreach (const Future<bool>& authorization, authorizations) { - if (!authorization.get()) { - return false; - } - } - return true; - }); + return collectAuthorizations(authorizations); } @@ -3704,17 +3694,7 @@ Future<bool> Master::authorizeUnreserveResources( return authorizer.get()->authorized(request); } - return await(authorizations) - .then([](const vector<Future<bool>>& authorizations) - -> Future<bool> { - // Compute a disjunction. - foreach (const Future<bool>& authorization, authorizations) { - if (!authorization.get()) { - return false; - } - } - return true; - }); + return collectAuthorizations(authorizations); } @@ -3771,17 +3751,7 @@ Future<bool> Master::authorizeCreateVolume( return authorizer.get()->authorized(request); } - return await(authorizations) - .then([](const vector<Future<bool>>& authorizations) - -> Future<bool> { - // Compute a disjunction. - foreach (const Future<bool>& authorization, authorizations) { - if (!authorization.get()) { - return false; - } - } - return true; - }); + return collectAuthorizations(authorizations); } @@ -3823,17 +3793,7 @@ Future<bool> Master::authorizeDestroyVolume( return authorizer.get()->authorized(request); } - return await(authorizations) - .then([](const vector<Future<bool>>& authorizations) - -> Future<bool> { - // Compute a disjunction. - foreach (const Future<bool>& authorization, authorizations) { - if (!authorization.get()) { - return false; - } - } - return true; - }); + return collectAuthorizations(authorizations); } @@ -4014,11 +3974,7 @@ Future<bool> Master::authorizeSlave( authorizeReserveResources(slaveInfo.resources(), principal)); } - return collect(authorizations) - .then([](const vector<bool>& results) - -> Future<bool> { - return std::find(results.begin(), results.end(), false) == results.end(); - }); + return collectAuthorizations(authorizations); } diff --git a/src/master/master.hpp b/src/master/master.hpp index e77babf..afd829e 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -19,6 +19,7 @@ #include <stdint.h> +#include <algorithm> #include <list> #include <memory> #include <set> @@ -42,6 +43,7 @@ #include <mesos/scheduler/scheduler.hpp> +#include <process/collect.hpp> #include <process/future.hpp> #include <process/limiter.hpp> #include <process/http.hpp> @@ -2351,6 +2353,18 @@ private: }; +// Collects authorization results. Any discarded or failed future +// results in a failure; any false future results in false. +inline process::Future<bool> collectAuthorizations( + const std::vector<process::Future<bool>>& authorizations) +{ + return process::collect(authorizations) + .then([](const std::vector<bool>& results) -> process::Future<bool> { + return std::find(results.begin(), results.end(), false) == results.end(); + }); +} + + inline std::ostream& operator<<( std::ostream& stream, const Framework& framework); diff --git a/src/master/weights_handler.cpp b/src/master/weights_handler.cpp index 222ec75..1d34356 100644 --- a/src/master/weights_handler.cpp +++ b/src/master/weights_handler.cpp @@ -345,17 +345,7 @@ Future<bool> Master::WeightsHandler::authorizeUpdateWeights( return master->authorizer.get()->authorized(request); } - return await(authorizations) - .then([](const vector<Future<bool>>& authorizations) - -> Future<bool> { - // Compute a disjunction. - foreach (const Future<bool>& authorization, authorizations) { - if (!authorization.get()) { - return false; - } - } - return true; - }); + return collectAuthorizations(authorizations); } diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index 8ed1e89..658051f 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -10152,6 +10152,63 @@ TEST_F(MasterTest, CreateVolumesV1AuthorizationFailure) AWAIT_EXPECT_RESPONSE_STATUS_EQ(InternalServerError().status, response); } + +// Test for the authorization collect helper. +TEST_F(MasterTest, CollectAuthorizations) +{ + { + Promise<bool> promise1; + Promise<bool> promise2; + + Future<bool> result = + master::collectAuthorizations({promise1.future(), promise2.future()}); + + promise1.set(true); + promise2.set(false); + + AWAIT_EXPECT_FALSE(result); + } + + { + Promise<bool> promise1; + Promise<bool> promise2; + + Future<bool> result = + master::collectAuthorizations({promise1.future(), promise2.future()}); + + promise1.set(true); + promise2.fail("Authorization failure"); + + AWAIT_EXPECT_FAILED(result); + } + + { + Promise<bool> promise1; + Promise<bool> promise2; + + Future<bool> result = + master::collectAuthorizations({promise1.future(), promise2.future()}); + + promise1.set(true); + promise2.discard(); + + AWAIT_EXPECT_FAILED(result); + } + + { + Promise<bool> promise1; + Promise<bool> promise2; + + Future<bool> result = + master::collectAuthorizations({promise1.future(), promise2.future()}); + + promise1.set(true); + promise2.set(true); + + AWAIT_EXPECT_TRUE(result); + } +} + } // namespace tests { } // namespace internal { } // namespace mesos {
