Maintenance Primitives: Handle inverse offers in pre-V1 scheduler. Review: https://reviews.apache.org/r/37621
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ea961908 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ea961908 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ea961908 Branch: refs/heads/master Commit: ea961908dadcf71234f95b2465e118c89cfca60c Parents: a127671 Author: Joris Van Remoortere <[email protected]> Authored: Tue Aug 25 18:50:41 2015 -0400 Committer: Joris Van Remoortere <[email protected]> Committed: Mon Sep 14 13:58:37 2015 -0400 ---------------------------------------------------------------------- src/sched/sched.cpp | 10 +++- src/tests/master_maintenance_tests.cpp | 90 +++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/ea961908/src/sched/sched.cpp ---------------------------------------------------------------------- diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp index 1fc9e73..a1723f3 100644 --- a/src/sched/sched.cpp +++ b/src/sched/sched.cpp @@ -505,6 +505,7 @@ protected: } resourceOffers(from, offers, pids); + break; } @@ -776,9 +777,16 @@ protected: return; } + // We exit early if `offers` is empty since we don't implement inverse + // offers in the old scheduler API. It could be empty when there are only + // inverse offers as part of the `ResourceOffersMessage`. + if (offers.empty()) { + return; + } + VLOG(2) << "Received " << offers.size() << " offers"; - CHECK(offers.size() == pids.size()); + CHECK_EQ(offers.size(), pids.size()); // Save the pid associated with each slave (one per offer) so // later we can send framework messages directly. http://git-wip-us.apache.org/repos/asf/mesos/blob/ea961908/src/tests/master_maintenance_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp index 8f39ac3..4a59389 100644 --- a/src/tests/master_maintenance_tests.cpp +++ b/src/tests/master_maintenance_tests.cpp @@ -474,6 +474,96 @@ TEST_F(MasterMaintenanceTest, PendingUnavailabilityTest) } +// Test ensures that old schedulers gracefully handle inverse offers, even if +// they aren't passed up to the top level API yet. +TEST_F(MasterMaintenanceTest, PreV1SchedulerSupport) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + MockExecutor exec(DEFAULT_EXECUTOR_ID); + + Try<PID<Slave>> slave = StartSlave(&exec); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + EXPECT_CALL(sched, registered(&driver, _, _)) + .Times(1); + + // Intercept offers sent to the scheduler. + Future<vector<Offer>> normalOffers; + Future<vector<Offer>> unavailabilityOffers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&normalOffers)) + .WillOnce(FutureArg<1>(&unavailabilityOffers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + // The original offers should be rescinded when the unavailability is changed. + Future<Nothing> offerRescinded; + EXPECT_CALL(sched, offerRescinded(&driver, _)) + .WillOnce(FutureSatisfy(&offerRescinded)); + + // Start the test. + driver.start(); + + // Wait for some normal offers. + AWAIT_READY(normalOffers); + EXPECT_NE(0u, normalOffers.get().size()); + + // Check that unavailability is not set. + foreach (const Offer& offer, normalOffers.get()) { + EXPECT_FALSE(offer.has_unavailability()); + } + + // Schedule this slave for maintenance. + MachineID machine; + machine.set_hostname(maintenanceHostname); + machine.set_ip(stringify(slave.get().address.ip)); + + // TODO(jmlvanre): Replace Time(0.0) with `Clock::now()` once JSON double + // conversion is fixed. For now using a rounded time avoids the issue. + const Time start = Time::create(0.0).get() + Seconds(60); + const Duration duration = Seconds(120); + const Unavailability unavailability = createUnavailability(start, duration); + + // Post a valid schedule with one machine. + maintenance::Schedule schedule = createSchedule( + {createWindow({machine}, unavailability)}); + + // We have a few seconds between the first set of offers and the next + // allocation of offers. This should be enough time to perform a maintenance + // schedule update. This update will also trigger the rescinding of offers + // from the scheduled slave. + Future<Response> response = + process::http::post( + master.get(), + "maintenance/schedule", + headers, + stringify(JSON::Protobuf(schedule))); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); + + // Wait for some offers. + AWAIT_READY(unavailabilityOffers); + EXPECT_NE(0u, unavailabilityOffers.get().size()); + + // Check that each offer has an unavailability. + foreach (const Offer& offer, unavailabilityOffers.get()) { + EXPECT_TRUE(offer.has_unavailability()); + EXPECT_EQ(unavailability.start(), offer.unavailability().start()); + EXPECT_EQ(unavailability.duration(), offer.unavailability().duration()); + } + + driver.stop(); + driver.join(); + + Shutdown(); // Must shutdown before 'containerizer' gets deallocated. +} + + // Posts valid and invalid machines to the maintenance start endpoint. TEST_F(MasterMaintenanceTest, BringDownMachines) {
