mesos git commit: Fixed flakiness in MasterAllocatorTest.ResourcesUnused.

[ne...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master d225d4d41 -> 6fe4be921


Fixed flakiness in MasterAllocatorTest.ResourcesUnused.

In the test, `sched1` registers with the master and launches a task on
half the resources it is offered. Then `sched2` registers and should be
offered the remaining resources. The problem is that, because the test
did not pause the clock, a batch allocation might occur between the
first task launch and the second scheduler registering. The test tried
to account for this (by having `sched1` decline any additional offers it
receives), but this was not done correctly: declining the offer will
result in calling `recoverResources` again, which the test did not
account for.

Seems simpler to instead pause the clock, which should prevent the
possibility of a batch allocation in the first place.

Review: https://reviews.apache.org/r/59509


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6fe4be92
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6fe4be92
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6fe4be92

Branch: refs/heads/master
Commit: 6fe4be92180d125753729f26887aa9fe853203a7
Parents: d225d4d
Author: Neil Conway <ne...@gmail.com>
Authored: Tue May 23 16:37:36 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed May 24 09:54:55 2017 -0700

----------------------------------------------------------------------
 src/tests/master_allocator_tests.cpp | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6fe4be92/src/tests/master_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 49d87af..750e030 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -208,6 +208,8 @@ TYPED_TEST(MasterAllocatorTest, SingleFramework)
 // reoffered appropriately.
 TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 {
+  Clock::pause();
+
   TestAllocator<TypeParam> allocator;
 
   EXPECT_CALL(allocator, initialize(_, _, _, _));
@@ -221,7 +223,10 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
   slave::Flags slaveFlags = this->CreateSlaveFlags();
   slaveFlags.resources = Some("cpus:2;mem:1024");
 
-  EXPECT_CALL(allocator, addSlave(_, _, _, _, _, _));
+  Future<Nothing> addSlave;
+  EXPECT_CALL(allocator, addSlave(_, _, _, _, _, _))
+    .WillOnce(DoAll(InvokeAddSlave(&allocator),
+                    FutureSatisfy(&addSlave)));
 
   Owned<MasterDetector> detector = master.get()->createDetector();
 
@@ -229,6 +234,11 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
     this->StartSlave(detector.get(), &containerizer, slaveFlags);
   ASSERT_SOME(slave);
 
+  Clock::advance(slaveFlags.authentication_backoff_factor);
+  Clock::advance(slaveFlags.registration_backoff_factor);
+
+  AWAIT_READY(addSlave);
+
   MockScheduler sched1;
   MesosSchedulerDriver driver1(
       &sched1, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
@@ -237,13 +247,6 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 
   EXPECT_CALL(sched1, registered(_, _, _));
 
-  // We decline offers that we aren't expecting so that the resources
-  // get aggregated. Note that we need to do this _first_ and
-  // _separate_ from the expectation below so that this expectation is
-  // checked last and matches all possible offers.
-  EXPECT_CALL(sched1, resourceOffers(_, _))
-    .WillRepeatedly(DeclineOffers());
-
   // The first offer will contain all of the slave's resources, since
   // this is the only framework running so far. Launch a task that
   // uses less than that to leave some resources unused.
@@ -267,7 +270,8 @@ TYPED_TEST(MasterAllocatorTest, ResourcesUnused)
 
   // We need to wait until the allocator knows about the unused
   // resources to start the second framework so that we get the
-  // expected offer.
+  // expected offer. Because the clock is paused, a batch allocation
+  // will not occur in the time before the second framework registers.
   AWAIT_READY(recoverResources);
 
   FrameworkInfo frameworkInfo2 = DEFAULT_FRAMEWORK_INFO;