Repository: mesos
Updated Branches:
  refs/heads/master 246ac9768 -> 6a8aac9c2


Fixed a bug to properly re-register an authenticating framework.

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


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

Branch: refs/heads/master
Commit: 6a8aac9c26af67c283e6645ef9149f83d350f338
Parents: 246ac97
Author: Vinod Kone <[email protected]>
Authored: Fri Mar 21 11:38:23 2014 -0700
Committer: Vinod Kone <[email protected]>
Committed: Fri Mar 21 16:57:21 2014 -0700

----------------------------------------------------------------------
 src/master/master.cpp               | 24 ++++++++++++++++--------
 src/tests/fault_tolerance_tests.cpp | 12 +++++++++++-
 src/tests/master_tests.cpp          | 12 +++++++++++-
 3 files changed, 38 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6a8aac9c/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 90fd7b5..544144d 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -966,6 +966,16 @@ void Master::reregisterFramework(
       LOG(INFO) << "Allowing the Framework " << frameworkInfo.id()
                 << " to re-register with an already used id";
 
+      // Make sure we can get offers again.
+      // The framework might have been deactivated if it was doing
+      // authentication.
+      // TODO(vinod): Consider adding 'Master::activate(Framework*)'.
+      // TODO(vinod): Do this after we recover resources below.
+      if (!framework->active) {
+        framework->active = true;
+        allocator->frameworkActivated(framework->id, framework->info);
+      }
+
       // Remove any offers sent to this framework.
       // NOTE: We need to do this because the scheduler might have
       // replied to the offers but the driver might have dropped
@@ -1103,10 +1113,7 @@ void Master::deactivate(Framework* framework)
   // Remove the framework's offers.
   foreach (Offer* offer, utils::copy(framework->offers)) {
     allocator->resourcesRecovered(
-        offer->framework_id(),
-        offer->slave_id(),
-        Resources(offer->resources()));
-
+        offer->framework_id(), offer->slave_id(), offer->resources());
     removeOffer(offer);
   }
 }
@@ -2699,6 +2706,7 @@ void Master::failoverFramework(Framework* framework, 
const UPID& newPid)
   link(newPid);
 
   // Make sure we can get offers again.
+  // TODO(vinod): Do this after we recover resources below.
   if (!framework->active) {
     framework->active = true;
     allocator->frameworkActivated(framework->id, framework->info);
@@ -2717,11 +2725,9 @@ void Master::failoverFramework(Framework* framework, 
const UPID& newPid)
   // We do this after we have updated the pid and sent the framework
   // registered message so that the allocator can immediately re-offer
   // these resources to this framework if it wants.
-  // TODO(benh): Consider just reoffering these to
   foreach (Offer* offer, utils::copy(framework->offers)) {
-    allocator->resourcesRecovered(offer->framework_id(),
-                                  offer->slave_id(),
-                                  Resources(offer->resources()));
+    allocator->resourcesRecovered(
+        offer->framework_id(), offer->slave_id(), offer->resources());
     removeOffer(offer);
   }
 }
@@ -2735,6 +2741,8 @@ void Master::removeFramework(Framework* framework)
 
   if (framework->active) {
     // Tell the allocator to stop allocating resources to this framework.
+    // TODO(vinod): Consider setting  framework->active to false here
+    // or just calling 'deactivate(Framework*)'.
     allocator->frameworkDeactivated(framework->id);
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6a8aac9c/src/tests/fault_tolerance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fault_tolerance_tests.cpp 
b/src/tests/fault_tolerance_tests.cpp
index 3f4796a..6828011 100644
--- a/src/tests/fault_tolerance_tests.cpp
+++ b/src/tests/fault_tolerance_tests.cpp
@@ -1052,8 +1052,9 @@ TEST_F(FaultToleranceTest, FrameworkReregister)
   EXPECT_CALL(sched, registered(&driver, _, _))
     .WillOnce(FutureSatisfy(&registered));
 
+  Future<Nothing> resourceOffers;
   EXPECT_CALL(sched, resourceOffers(&driver, _))
-    .WillRepeatedly(Return());
+    .WillOnce(FutureSatisfy(&resourceOffers));
 
   Future<process::Message> message =
     FUTURE_MESSAGE(Eq(FrameworkRegisteredMessage().GetTypeName()), _, _);
@@ -1062,6 +1063,7 @@ TEST_F(FaultToleranceTest, FrameworkReregister)
 
   AWAIT_READY(message); // Framework registered message, to get the pid.
   AWAIT_READY(registered); // Framework registered call.
+  AWAIT_READY(resourceOffers);
 
   Future<Nothing> disconnected;
   EXPECT_CALL(sched, disconnected(&driver))
@@ -1071,6 +1073,11 @@ TEST_F(FaultToleranceTest, FrameworkReregister)
   EXPECT_CALL(sched, reregistered(&driver, _))
     .WillOnce(FutureSatisfy(&reregistered));
 
+  Future<Nothing> resourceOffers2;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureSatisfy(&resourceOffers2))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
   EXPECT_CALL(sched, offerRescinded(&driver, _))
     .Times(AtMost(1));
 
@@ -1082,6 +1089,9 @@ TEST_F(FaultToleranceTest, FrameworkReregister)
 
   AWAIT_READY(reregistered);
 
+  // The re-registered framework should get offers.
+  AWAIT_READY(resourceOffers2);
+
   driver.stop();
   driver.join();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6a8aac9c/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 42c5a77..4395883 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -899,8 +899,9 @@ TEST_F(MasterTest, MasterInfoOnReElection)
   EXPECT_CALL(sched, registered(&driver, _, _))
     .Times(1);
 
+  Future<Nothing> resourceOffers;
   EXPECT_CALL(sched, resourceOffers(&driver, _))
-    .WillRepeatedly(Return()); // Ignore offers.
+    .WillOnce(FutureSatisfy(&resourceOffers));
 
   Future<process::Message> message =
     FUTURE_MESSAGE(Eq(FrameworkRegisteredMessage().GetTypeName()), _, _);
@@ -908,6 +909,7 @@ TEST_F(MasterTest, MasterInfoOnReElection)
   driver.start();
 
   AWAIT_READY(message);
+  AWAIT_READY(resourceOffers);
 
   Future<Nothing> disconnected;
   EXPECT_CALL(sched, disconnected(&driver))
@@ -917,6 +919,11 @@ TEST_F(MasterTest, MasterInfoOnReElection)
   EXPECT_CALL(sched, reregistered(&driver, _))
     .WillOnce(FutureArg<1>(&masterInfo));
 
+  Future<Nothing> resourceOffers2;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureSatisfy(&resourceOffers2))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
   // Simulate a spurious event (e.g., due to ZooKeeper
   // expiration) at the scheduler.
   detector->appoint(master.get());
@@ -927,6 +934,9 @@ TEST_F(MasterTest, MasterInfoOnReElection)
   EXPECT_EQ(master.get().port, masterInfo.get().port());
   EXPECT_EQ(master.get().ip, masterInfo.get().ip());
 
+  // The re-registered framework should get offers.
+  AWAIT_READY(resourceOffers2);
+
   driver.stop();
   driver.join();
 

Reply via email to