You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/03/11 03:08:45 UTC
[1/2] mesos git commit: Update PendingUnavailabilityTest to use the
new scheduler mock.
Repository: mesos
Updated Branches:
refs/heads/master 639917ca1 -> 5172b50a6
Update PendingUnavailabilityTest to use the new scheduler mock.
Based on the modified test Anand posted in the description of MESOS-4831.
Review: https://reviews.apache.org/r/44537/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5172b50a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5172b50a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5172b50a
Branch: refs/heads/master
Commit: 5172b50a618202bf57f6fe9ada940f8d5cab9f97
Parents: eda74aa
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Thu Mar 10 19:29:44 2016 -0500
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Thu Mar 10 21:08:30 2016 -0500
----------------------------------------------------------------------
src/tests/master_maintenance_tests.cpp | 85 +++++++++++++++++------------
1 file changed, 49 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5172b50a/src/tests/master_maintenance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp
index 6e5c576..3c7024c 100644
--- a/src/tests/master_maintenance_tests.cpp
+++ b/src/tests/master_maintenance_tests.cpp
@@ -373,7 +373,11 @@ TEST_F(MasterMaintenanceTest, FailToUnscheduleDeactivatedMachines)
// slave is scheduled to go down for maintenance.
TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
{
- Try<PID<Master>> master = StartMaster();
+ // Set up a master.
+ // NOTE: We don't use `StartMaster()` because we need to access these flags.
+ master::Flags flags = CreateMasterFlags();
+
+ Try<PID<Master>> master = StartMaster(flags);
ASSERT_SOME(master);
MockExecutor exec(DEFAULT_EXECUTOR_ID);
@@ -381,26 +385,27 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
Try<PID<Slave>> slave = StartSlave(&exec);
ASSERT_SOME(slave);
- Callbacks callbacks;
+ auto scheduler = std::make_shared<MockV1HTTPScheduler>();
Future<Nothing> connected;
- EXPECT_CALL(callbacks, connected())
+ EXPECT_CALL(*scheduler, connected(_))
.WillOnce(FutureSatisfy(&connected))
.WillRepeatedly(Return()); // Ignore future invocations.
- Mesos mesos(
- master.get(),
- ContentType::PROTOBUF,
- lambda::bind(&Callbacks::connected, lambda::ref(callbacks)),
- lambda::bind(&Callbacks::disconnected, lambda::ref(callbacks)),
- lambda::bind(&Callbacks::received, lambda::ref(callbacks), lambda::_1));
+ scheduler::TestV1Mesos mesos(master.get(), ContentType::PROTOBUF, scheduler);
AWAIT_READY(connected);
- Queue<Event> events;
+ Future<Event::Subscribed> subscribed;
+ EXPECT_CALL(*scheduler, subscribed(_, _))
+ .WillOnce(FutureArg<1>(&subscribed));
- EXPECT_CALL(callbacks, received(_))
- .WillRepeatedly(Enqueue(&events));
+ EXPECT_CALL(*scheduler, heartbeat(_))
+ .WillRepeatedly(Return()); // Ignore heartbeats.
+
+ Future<Event::Offers> normalOffers;
+ EXPECT_CALL(*scheduler, offers(_, _))
+ .WillOnce(FutureArg<1>(&normalOffers));
{
Call call;
@@ -412,19 +417,26 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
mesos.send(call);
}
- Future<Event> event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::SUBSCRIBED, event.get().type());
+ AWAIT_READY(subscribed);
- v1::FrameworkID id(event.get().subscribed().framework_id());
+ v1::FrameworkID frameworkId(subscribed->framework_id());
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_EQ(1, event.get().offers().offers().size());
+ AWAIT_READY(normalOffers);
+ EXPECT_EQ(1, normalOffers->offers().size());
// Regular offers shouldn't have unavailability.
- EXPECT_FALSE(event.get().offers().offers(0).has_unavailability());
+ EXPECT_FALSE(normalOffers->offers(0).has_unavailability());
+
+ // The original offers should be rescinded when the unavailability is changed.
+ Future<Nothing> offerRescinded;
+ EXPECT_CALL(*scheduler, rescind(_, _))
+ .WillOnce(FutureSatisfy(&offerRescinded));
+
+ Future<Event::Offers> unavailabilityOffers;
+ Future<Event::Offers> inverseOffers;
+ EXPECT_CALL(*scheduler, offers(_, _))
+ .WillOnce(FutureArg<1>(&unavailabilityOffers))
+ .WillOnce(FutureArg<1>(&inverseOffers));
// Schedule this slave for maintenance.
MachineID machine;
@@ -452,18 +464,14 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
// The original offers should be rescinded when the unavailability
- // is changed. We expect as many rescind events as we received
- // original offers.
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::RESCIND, event.get().type());
+ // is changed.
+ AWAIT_READY(offerRescinded);
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_EQ(1, event.get().offers().offers().size());
+ AWAIT_READY(unavailabilityOffers);
+ EXPECT_EQ(1, unavailabilityOffers->offers().size());
- v1::Offer offer = event.get().offers().offers(0);
+ // Make sure the new offers have the unavailability set.
+ const v1::Offer& offer = unavailabilityOffers->offers(0);
EXPECT_TRUE(offer.has_unavailability());
EXPECT_EQ(
unavailability.start().nanoseconds(),
@@ -475,15 +483,20 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
// We also expect an inverse offer for the slave to go under
// maintenance.
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_EQ(1, event.get().offers().inverse_offers().size());
+ AWAIT_READY(inverseOffers);
+ EXPECT_EQ(1, inverseOffers->inverse_offers().size());
+
+ // Flush any possible remaining events from the master.
+ // The mocked scheduler will fail if any additional offers are sent.
+ Clock::pause();
+ Clock::settle();
+ Clock::advance(flags.allocation_interval);
+ Clock::resume();
EXPECT_CALL(exec, shutdown(_))
.Times(AtMost(1));
- EXPECT_CALL(callbacks, disconnected())
+ EXPECT_CALL(*scheduler, disconnected(_))
.Times(AtMost(1));
Shutdown(); // Must shutdown before 'containerizer' gets deallocated.
[2/2] mesos git commit: Maintenance: Fixed bug that sent two inverse
offers.
Posted by jo...@apache.org.
Maintenance: Fixed bug that sent two inverse offers.
Review: https://reviews.apache.org/r/44258/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/eda74aa6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/eda74aa6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/eda74aa6
Branch: refs/heads/master
Commit: eda74aa6cc68981051e0e8ceff431677cc536529
Parents: 639917c
Author: Guangya Liu <gy...@gmail.com>
Authored: Thu Mar 10 19:29:37 2016 -0500
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Thu Mar 10 21:08:30 2016 -0500
----------------------------------------------------------------------
src/master/http.cpp | 25 +++++++++++++++----
src/tests/master_maintenance_tests.cpp | 37 ++++++++++++-----------------
2 files changed, 35 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/eda74aa6/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 7304bfd..54a2569 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2043,19 +2043,29 @@ Future<Response> Master::Http::maintenanceSchedule(const Request& request) const
// Put the machines in the updated schedule into a set.
// Save the unavailability, to help with updating some machines.
- hashmap<MachineID, Unavailability> updated;
+ hashmap<MachineID, Unavailability> unavailabilities;
foreach (const mesos::maintenance::Window& window, schedule.windows()) {
foreach (const MachineID& id, window.machine_ids()) {
- updated[id] = window.unavailability();
+ unavailabilities[id] = window.unavailability();
}
}
// NOTE: Copies are needed because `updateUnavailability()` in this loop
// modifies the container.
foreachkey (const MachineID& id, utils::copy(master->machines)) {
- // Update the entry for each updated machine.
- if (updated.contains(id)) {
- master->updateUnavailability(id, updated[id]);
+ // Update the `unavailability` for each existing machine, except for
+ // machines going from `UP` to `DRAINING` (handled in the next loop).
+ // Each machine will only be touched by 1 of the 2 loops here to
+ // avoid sending inverse offer twice for a single machine since
+ // `updateUnavailability` will trigger an inverse offer.
+ // TODO(gyliu513): Merge this logic with `Master::updateUnavailability`,
+ // having it in two places results in more conditionals to handle.
+ if (unavailabilities.contains(id)) {
+ if (master->machines[id].info.mode() == MachineInfo::UP) {
+ continue;
+ }
+
+ master->updateUnavailability(id, unavailabilities[id]);
continue;
}
@@ -2069,6 +2079,11 @@ Future<Response> Master::Http::maintenanceSchedule(const Request& request) const
// and starting in `DRAINING` mode.
foreach (const mesos::maintenance::Window& window, schedule.windows()) {
foreach (const MachineID& id, window.machine_ids()) {
+ if (master->machines.contains(id) &&
+ master->machines[id].info.mode() != MachineInfo::UP) {
+ continue;
+ }
+
MachineInfo info;
info.mutable_id()->CopyFrom(id);
info.set_mode(MachineInfo::DRAINING);
http://git-wip-us.apache.org/repos/asf/mesos/blob/eda74aa6/src/tests/master_maintenance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp
index 3faa813..6e5c576 100644
--- a/src/tests/master_maintenance_tests.cpp
+++ b/src/tests/master_maintenance_tests.cpp
@@ -421,13 +421,10 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().offers().size());
- const size_t numberOfOffers = event.get().offers().offers().size();
+ EXPECT_EQ(1, event.get().offers().offers().size());
// Regular offers shouldn't have unavailability.
- foreach (const v1::Offer& offer, event.get().offers().offers()) {
- EXPECT_FALSE(offer.has_unavailability());
- }
+ EXPECT_FALSE(event.get().offers().offers(0).has_unavailability());
// Schedule this slave for maintenance.
MachineID machine;
@@ -457,35 +454,31 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest)
// The original offers should be rescinded when the unavailability
// is changed. We expect as many rescind events as we received
// original offers.
- for (size_t offerNumber = 0; offerNumber < numberOfOffers; ++offerNumber) {
- event = events.get();
- AWAIT_READY(event);
- EXPECT_EQ(Event::RESCIND, event.get().type());
- }
+ event = events.get();
+ AWAIT_READY(event);
+ EXPECT_EQ(Event::RESCIND, event.get().type());
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().offers().size());
+ EXPECT_EQ(1, event.get().offers().offers().size());
- // Make sure the new offers have the unavailability set.
- foreach (const v1::Offer& offer, event.get().offers().offers()) {
- EXPECT_TRUE(offer.has_unavailability());
- EXPECT_EQ(
- unavailability.start().nanoseconds(),
- offer.unavailability().start().nanoseconds());
+ v1::Offer offer = event.get().offers().offers(0);
+ EXPECT_TRUE(offer.has_unavailability());
+ EXPECT_EQ(
+ unavailability.start().nanoseconds(),
+ offer.unavailability().start().nanoseconds());
- EXPECT_EQ(
- unavailability.duration().nanoseconds(),
- offer.unavailability().duration().nanoseconds());
- }
+ EXPECT_EQ(
+ unavailability.duration().nanoseconds(),
+ offer.unavailability().duration().nanoseconds());
// We also expect an inverse offer for the slave to go under
// maintenance.
event = events.get();
AWAIT_READY(event);
EXPECT_EQ(Event::OFFERS, event.get().type());
- EXPECT_NE(0, event.get().offers().inverse_offers().size());
+ EXPECT_EQ(1, event.get().offers().inverse_offers().size());
EXPECT_CALL(exec, shutdown(_))
.Times(AtMost(1));