Repository: mesos Updated Branches: refs/heads/master 05543cf29 -> cc0a84790
Maintenance Primitives: Replaced MachineIDs with RepeatedPtrField<T>>. This removes the MachineIDs protobuf and changes it to instead use the RepeatedFieldPtr. This also changes the maintenance primitives (alpha) API for /machine/up and /machine/down, which both take an array instead of an object now. Review: https://reviews.apache.org/r/38011 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cc0a8479 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cc0a8479 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cc0a8479 Branch: refs/heads/master Commit: cc0a847904510b80c72b9de70af9191b3d5b6c22 Parents: 05543cf Author: Joseph Wu <[email protected]> Authored: Fri Sep 18 13:07:38 2015 -0400 Committer: Joris Van Remoortere <[email protected]> Committed: Fri Sep 18 15:21:12 2015 -0400 ---------------------------------------------------------------------- include/mesos/mesos.proto | 11 ----- include/mesos/v1/mesos.proto | 11 ----- src/common/protobuf_utils.cpp | 9 ++-- src/common/protobuf_utils.hpp | 6 +-- src/master/http.cpp | 42 +++++++++---------- src/master/maintenance.cpp | 16 ++++---- src/master/maintenance.hpp | 6 +-- src/tests/master_maintenance_tests.cpp | 64 ++++++++++++++++++----------- src/tests/registrar_tests.cpp | 16 ++++++-- 9 files changed, 91 insertions(+), 90 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/include/mesos/mesos.proto ---------------------------------------------------------------------- diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto index c9b310d..4a16be1 100644 --- a/include/mesos/mesos.proto +++ b/include/mesos/mesos.proto @@ -168,17 +168,6 @@ message MachineID { /** - * A list of machines. - * - * TODO(josephw): Remove this when https://reviews.apache.org/r/37826/ - * is submitted. - */ -message MachineIDs { - repeated MachineID values = 1; -} - - -/** * Holds information about a single machine, its `mode`, and any other * relevant information which may affect the behavior of the machine. */ http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/include/mesos/v1/mesos.proto ---------------------------------------------------------------------- diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto index bcce340..58e5a6b 100644 --- a/include/mesos/v1/mesos.proto +++ b/include/mesos/v1/mesos.proto @@ -168,17 +168,6 @@ message MachineID { /** - * A list of machines. - * - * TODO(josephw): Remove this when https://reviews.apache.org/r/37826/ - * is submitted. - */ -message MachineIDs { - repeated MachineID values = 1; -} - - -/** * Holds information about a single machine, its `mode`, and any other * relevant information which may affect the behavior of the machine. */ http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/common/protobuf_utils.cpp ---------------------------------------------------------------------- diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp index c8e6211..4dc58fe 100644 --- a/src/common/protobuf_utils.cpp +++ b/src/common/protobuf_utils.cpp @@ -33,6 +33,8 @@ using std::string; +using google::protobuf::RepeatedPtrField; + using mesos::slave::ContainerLimitation; using mesos::slave::ContainerState; @@ -267,12 +269,13 @@ Unavailability createUnavailability( } -MachineIDs createMachineList(std::initializer_list<MachineID> ids) +RepeatedPtrField<MachineID> createMachineList( + std::initializer_list<MachineID> ids) { - MachineIDs array; + RepeatedPtrField<MachineID> array; foreach (const MachineID& id, ids) { - array.add_values()->CopyFrom(id); + array.Add()->CopyFrom(id); } return array; http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/common/protobuf_utils.hpp ---------------------------------------------------------------------- diff --git a/src/common/protobuf_utils.hpp b/src/common/protobuf_utils.hpp index 86474ea..3817c6a 100644 --- a/src/common/protobuf_utils.hpp +++ b/src/common/protobuf_utils.hpp @@ -111,11 +111,9 @@ Unavailability createUnavailability( /** * Helper for constructing a list of `MachineID`. - * - * TODO(josephw): Remove this when https://reviews.apache.org/r/37826/ - * is submitted. */ -MachineIDs createMachineList(std::initializer_list<MachineID> ids); +google::protobuf::RepeatedPtrField<MachineID> createMachineList( + std::initializer_list<MachineID> ids); /** http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/master/http.cpp ---------------------------------------------------------------------- diff --git a/src/master/http.cpp b/src/master/http.cpp index 8bde4c7..8bb5935 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -69,6 +69,8 @@ #include "mesos/mesos.hpp" #include "mesos/resources.hpp" +using google::protobuf::RepeatedPtrField; + using process::Clock; using process::DESCRIPTION; using process::Future; @@ -1550,29 +1552,26 @@ Future<Response> Master::Http::machineDown(const Request& request) const } // Parse the POST body as JSON. - Try<JSON::Object> jsonIds = JSON::parse<JSON::Object>(request.body); + Try<JSON::Array> jsonIds = JSON::parse<JSON::Array>(request.body); if (jsonIds.isError()) { return BadRequest(jsonIds.error()); } // Convert the machines to a protobuf. - Try<MachineIDs> protoIds = - ::protobuf::parse<MachineIDs>(jsonIds.get()); - - if (protoIds.isError()) { - return BadRequest(protoIds.error()); + auto ids = ::protobuf::parse<RepeatedPtrField<MachineID>>(jsonIds.get()); + if (ids.isError()) { + return BadRequest(ids.error()); } // Validate every machine in the list. - MachineIDs ids = protoIds.get(); - Try<Nothing> isValid = maintenance::validation::machines(ids); + Try<Nothing> isValid = maintenance::validation::machines(ids.get()); if (isValid.isError()) { return BadRequest(isValid.error()); } // Check that all machines are part of a maintenance schedule. // TODO(josephw): Allow a transition from `UP` to `DOWN`. - foreach (const MachineID& id, ids.values()) { + foreach (const MachineID& id, ids.get()) { if (!master->machines.contains(id)) { return BadRequest( "Machine '" + stringify(JSON::Protobuf(id)) + @@ -1587,7 +1586,7 @@ Future<Response> Master::Http::machineDown(const Request& request) const } return master->registrar->apply(Owned<Operation>( - new maintenance::StartMaintenance(ids))) + new maintenance::StartMaintenance(ids.get()))) .then(defer(master->self(), [=](bool result) -> Future<Response> { // See the top comment in "master/maintenance.hpp" for why this check // is here, and is appropriate. @@ -1599,7 +1598,7 @@ Future<Response> Master::Http::machineDown(const Request& request) const // for all the tasks that were running on the slave and `LostSlaveMessage` // messages to the framework. This guards against the slave having dropped // the `ShutdownMessage`. - foreach (const MachineID& machineId, ids.values()) { + foreach (const MachineID& machineId, ids.get()) { // The machine may not be in machines. This means no slaves are // currently registered on that machine so this is a no-op. if (master->machines.contains(machineId)) { @@ -1625,7 +1624,7 @@ Future<Response> Master::Http::machineDown(const Request& request) const } // Update the master's local state with the downed machines. - foreach (const MachineID& id, ids.values()) { + foreach (const MachineID& id, ids.get()) { master->machines[id].info.set_mode(MachineInfo::DOWN); } @@ -1652,28 +1651,25 @@ Future<Response> Master::Http::machineUp(const Request& request) const } // Parse the POST body as JSON. - Try<JSON::Object> jsonIds = JSON::parse<JSON::Object>(request.body); + Try<JSON::Array> jsonIds = JSON::parse<JSON::Array>(request.body); if (jsonIds.isError()) { return BadRequest(jsonIds.error()); } // Convert the machines to a protobuf. - Try<MachineIDs> protoIds = - ::protobuf::parse<MachineIDs>(jsonIds.get()); - - if (protoIds.isError()) { - return BadRequest(protoIds.error()); + auto ids = ::protobuf::parse<RepeatedPtrField<MachineID>>(jsonIds.get()); + if (ids.isError()) { + return BadRequest(ids.error()); } // Validate every machine in the list. - MachineIDs ids = protoIds.get(); - Try<Nothing> isValid = maintenance::validation::machines(ids); + Try<Nothing> isValid = maintenance::validation::machines(ids.get()); if (isValid.isError()) { return BadRequest(isValid.error()); } // Check that all machines are part of a maintenance schedule. - foreach (const MachineID& id, ids.values()) { + foreach (const MachineID& id, ids.get()) { if (!master->machines.contains(id)) { return BadRequest( "Machine '" + stringify(JSON::Protobuf(id)) + @@ -1688,7 +1684,7 @@ Future<Response> Master::Http::machineUp(const Request& request) const } return master->registrar->apply(Owned<Operation>( - new maintenance::StopMaintenance(ids))) + new maintenance::StopMaintenance(ids.get()))) .then(defer(master->self(), [=](bool result) -> Future<Response> { // See the top comment in "master/maintenance.hpp" for why this check // is here, and is appropriate. @@ -1696,7 +1692,7 @@ Future<Response> Master::Http::machineUp(const Request& request) const // Update the master's local state with the reactivated machines. hashset<MachineID> updated; - foreach (const MachineID& id, ids.values()) { + foreach (const MachineID& id, ids.get()) { master->machines[id].info.set_mode(MachineInfo::UP); master->machines[id].info.clear_unavailability(); updated.insert(id); http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/master/maintenance.cpp ---------------------------------------------------------------------- diff --git a/src/master/maintenance.cpp b/src/master/maintenance.cpp index 7d7d40b..5fe9358 100644 --- a/src/master/maintenance.cpp +++ b/src/master/maintenance.cpp @@ -38,6 +38,8 @@ namespace maintenance { using namespace mesos::maintenance; +using google::protobuf::RepeatedPtrField; + UpdateSchedule::UpdateSchedule( const maintenance::Schedule& _schedule) : schedule(_schedule) {} @@ -114,9 +116,9 @@ Try<bool> UpdateSchedule::perform( StartMaintenance::StartMaintenance( - const MachineIDs& _ids) + const RepeatedPtrField<MachineID>& _ids) { - foreach (const MachineID& id, _ids.values()) { + foreach (const MachineID& id, _ids) { ids.insert(id); } } @@ -144,9 +146,9 @@ Try<bool> StartMaintenance::perform( StopMaintenance::StopMaintenance( - const MachineIDs& _ids) + const RepeatedPtrField<MachineID>& _ids) { - foreach (const MachineID& id, _ids.values()) { + foreach (const MachineID& id, _ids) { ids.insert(id); } } @@ -265,15 +267,15 @@ Try<Nothing> unavailability(const Unavailability& unavailability) } -Try<Nothing> machines(const MachineIDs& ids) +Try<Nothing> machines(const RepeatedPtrField<MachineID>& ids) { - if (ids.values().size() <= 0) { + if (ids.size() <= 0) { return Error("List of machines is empty"); } // Verify that the machine has at least one non-empty field value. hashset<MachineID> uniques; - foreach (const MachineID& id, ids.values()) { + foreach (const MachineID& id, ids) { // Validate the single machine. Try<Nothing> validId = validation::machine(id); if (validId.isError()) { http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/master/maintenance.hpp ---------------------------------------------------------------------- diff --git a/src/master/maintenance.hpp b/src/master/maintenance.hpp index 8d134aa..a37412f 100644 --- a/src/master/maintenance.hpp +++ b/src/master/maintenance.hpp @@ -77,7 +77,7 @@ class StartMaintenance : public Operation { public: explicit StartMaintenance( - const MachineIDs& _ids); + const google::protobuf::RepeatedPtrField<MachineID>& _ids); protected: Try<bool> perform( @@ -100,7 +100,7 @@ class StopMaintenance : public Operation { public: explicit StopMaintenance( - const MachineIDs& _ids); + const google::protobuf::RepeatedPtrField<MachineID>& _ids); protected: Try<bool> perform( @@ -139,7 +139,7 @@ Try<Nothing> unavailability( * - The list is non-empty. * - All checks in the `machine` method below. */ -Try<Nothing> machines(const MachineIDs& ids); +Try<Nothing> machines(const google::protobuf::RepeatedPtrField<MachineID>& ids); /** http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/tests/master_maintenance_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp index 4478505..45112f1 100644 --- a/src/tests/master_maintenance_tests.cpp +++ b/src/tests/master_maintenance_tests.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ +#include <initializer_list> #include <string> #include <mesos/maintenance/maintenance.hpp> @@ -53,6 +54,8 @@ #include "tests/mesos.hpp" #include "tests/utils.hpp" +using google::protobuf::RepeatedPtrField; + using mesos::internal::master::Master; using mesos::internal::slave::Slave; @@ -71,7 +74,6 @@ using process::http::BadRequest; using process::http::OK; using process::http::Response; -using mesos::internal::protobuf::maintenance::createMachineList; using mesos::internal::protobuf::maintenance::createSchedule; using mesos::internal::protobuf::maintenance::createUnavailability; using mesos::internal::protobuf::maintenance::createWindow; @@ -86,6 +88,20 @@ namespace mesos { namespace internal { namespace tests { +JSON::Array createMachineList(std::initializer_list<MachineID> _ids) +{ + RepeatedPtrField<MachineID> ids = + internal::protobuf::maintenance::createMachineList(_ids); + + JSON::Array array; + foreach (const MachineID& id, ids) { + array.values.emplace_back(JSON::Protobuf(id)); + } + + return array; +} + + class MasterMaintenanceTest : public MesosTest { public: @@ -314,12 +330,12 @@ TEST_F(MasterMaintenanceTest, FailToUnscheduleDeactivatedMachines) AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); // Deactivate machine1. - MachineIDs machines = createMachineList({machine1}); + JSON::Array machines = createMachineList({machine1}); response = process::http::post( master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -340,7 +356,7 @@ TEST_F(MasterMaintenanceTest, FailToUnscheduleDeactivatedMachines) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); } @@ -657,7 +673,7 @@ TEST_F(MasterMaintenanceTest, EnterMaintenanceMode) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(createMachineList({machine})))); + stringify(createMachineList({machine}))); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -695,7 +711,7 @@ TEST_F(MasterMaintenanceTest, EnterMaintenanceMode) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(createMachineList({machine})))); + stringify(createMachineList({machine}))); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -728,12 +744,12 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) MachineID badMachine; // Try to start maintenance on an unscheduled machine. - MachineIDs machines = createMachineList({machine1, machine2}); + JSON::Array machines = createMachineList({machine1, machine2}); Future<Response> response = process::http::post( master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -743,7 +759,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -753,7 +769,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -775,7 +791,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -784,7 +800,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -794,7 +810,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -804,7 +820,7 @@ TEST_F(MasterMaintenanceTest, BringDownMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); } @@ -818,12 +834,12 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) ASSERT_SOME(master); // Try to bring up an unscheduled machine. - MachineIDs machines = createMachineList({machine1, machine2}); + JSON::Array machines = createMachineList({machine1, machine2}); Future<Response> response = process::http::post( master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -846,7 +862,7 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response); @@ -856,7 +872,7 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -865,7 +881,7 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -894,7 +910,7 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -903,7 +919,7 @@ TEST_F(MasterMaintenanceTest, BringUpMachines) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -962,12 +978,12 @@ TEST_F(MasterMaintenanceTest, MachineStatus) ASSERT_EQ(0, statuses.get().down_machines().size()); // Deactivate machine1. - MachineIDs machines = createMachineList({machine1}); + JSON::Array machines = createMachineList({machine1}); response = process::http::post( master.get(), "machine/down", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); @@ -992,7 +1008,7 @@ TEST_F(MasterMaintenanceTest, MachineStatus) master.get(), "machine/up", headers, - stringify(JSON::Protobuf(machines))); + stringify(machines)); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); http://git-wip-us.apache.org/repos/asf/mesos/blob/cc0a8479/src/tests/registrar_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/registrar_tests.cpp b/src/tests/registrar_tests.cpp index aa49c86..5131b57 100644 --- a/src/tests/registrar_tests.cpp +++ b/src/tests/registrar_tests.cpp @@ -72,6 +72,8 @@ using std::vector; using process::Clock; +using google::protobuf::RepeatedPtrField; + using mesos::internal::protobuf::maintenance::createMachineList; using mesos::internal::protobuf::maintenance::createSchedule; using mesos::internal::protobuf::maintenance::createUnavailability; @@ -530,7 +532,8 @@ TEST_P(RegistrarTest, StartMaintenance) Owned<Operation>(new UpdateSchedule(schedule)))); // Transition machine two into `DOWN` mode. - MachineIDs machines = createMachineList({machine2}); + RepeatedPtrField<MachineID> machines = createMachineList({machine2}); + AWAIT_READY(registrar.apply( Owned<Operation>(new StartMaintenance(machines)))); } @@ -558,7 +561,9 @@ TEST_P(RegistrarTest, StartMaintenance) Owned<Operation>(new UpdateSchedule(schedule)))); // Deactivate the two `DRAINING` machines. - MachineIDs machines = createMachineList({machine1, machine3}); + RepeatedPtrField<MachineID> machines = + createMachineList({machine1, machine3}); + AWAIT_READY(registrar.apply( Owned<Operation>(new StartMaintenance(machines)))); } @@ -615,7 +620,8 @@ TEST_P(RegistrarTest, StopMaintenance) Owned<Operation>(new UpdateSchedule(schedule)))); // Transition machine three into `DOWN` mode. - MachineIDs machines = createMachineList({machine3}); + RepeatedPtrField<MachineID> machines = createMachineList({machine3}); + AWAIT_READY(registrar.apply( Owned<Operation>(new StartMaintenance(machines)))); @@ -643,7 +649,9 @@ TEST_P(RegistrarTest, StopMaintenance) registry.get().machines().machines(1).info().mode()); // Transition machine one and two into `DOWN` mode. - MachineIDs machines = createMachineList({machine1, machine2}); + RepeatedPtrField<MachineID> machines = + createMachineList({machine1, machine2}); + AWAIT_READY(registrar.apply( Owned<Operation>(new StartMaintenance(machines))));
