Merge lp:~allenap/maas/region-rpc-broken--bug-1581654 into lp:~maas-committers/maas/trunk

[Gavin Panella]

Fix getRegionID.

[Lee Trager (ltrager)]

LGTM! Ran with make run, didn't see any exceptions.

review: Approve

=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2016-05-13 17:29:37 +0000
+++ src/maasserver/rpc/regionservice.py	2016-05-13 21:52:14 +0000
@@ -106,6 +106,7 @@
 )
 from twisted.internet.defer import (
     CancelledError,
+    fail,
     inlineCallbacks,
     maybeDeferred,
     returnValue,
@@ -482,6 +483,20 @@
         endpoint=endpoint, rack_controller=rack_controller).delete()
 
 
+def getRegionID():
+    """Obtain the region ID from the advertising service.
+
+    :return: :class:`Deferred`
+    """
+    try:
+        advertise = eventloop.services.getServiceNamed("rpc-advertise")
+    except:
+        return fail()
+    else:
+        return advertise.advertising.get().addCallback(
+            lambda advertising: advertising.region_id)
+
+
 @implementer(IConnection)
 class RegionServer(Region):
     """The RPC protocol supported by a region controller, server version.
@@ -512,12 +527,6 @@
         digest_local = calculate_digest(secret, message, salt)
         returnValue(digest == digest_local)
 
-    def getRegionID(self):
-        """Obtain the region ID from the advertising service."""
-        advertise = eventloop.services.getServiceNamed("rpc-advertise")
-        advertise.advertising.get().addCallback(
-            lambda advertising: advertising.region_id)
-
     @region.RegisterRackController.responder
     @inlineCallbacks
     def register(
@@ -549,7 +558,7 @@
             # Get the region ID if we're dealing with a non-local rack; we
             # won't need to bother for local racks.
             if self.hostIsRemote:
-                region_id = yield self.getRegionID()
+                region_id = yield getRegionID()
 
             # Only register the connection into the database when its a valid
             # IPv4 and IPv6. Only time it is not an IPv4 or IPv6 address is
@@ -626,7 +635,7 @@
 
     def connectionLost(self, reason):
         if self.hostIsRemote:
-            self.getRegionID().addCallback(
+            getRegionID().addCallback(
                 lambda region_id: deferToDatabase(
                     unregisterConnection, region_id, self.ident, self.host))
         self.factory.service._removeConnectionFor(self.ident, self)
 
=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2016-05-13 17:29:37 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2016-05-13 21:52:14 +0000
@@ -65,6 +65,7 @@
     regionservice,
 )
 from maasserver.rpc.regionservice import (
+    getRegionID,
     Region,
     RegionAdvertising,
     RegionAdvertisingService,
@@ -90,6 +91,8 @@
 )
 from maasserver.utils.threads import deferToDatabase
 from maastesting.matchers import (
+    IsFiredDeferred,
+    IsUnfiredDeferred,
     MockAnyCall,
     MockCalledOnce,
     MockCalledOnceWith,
@@ -1155,6 +1158,28 @@
                 endpoint=endpoint, rack_controller=rack_controller).first())
 
 
+class TestGetRegionID(MAASTestCase):
+    """Tests for `getRegionID`."""
+
+    def test__getRegionID_fails_when_advertising_service_not_running(self):
+        region_id = getRegionID()
+        self.assertThat(region_id, IsFiredDeferred())
+        error = self.assertRaises(KeyError, extract_result, region_id)
+        self.assertThat(str(error), Equals(repr('rpc-advertise')))
+
+    def test__getRegionID_returns_the_region_ID_when_available(self):
+        service = RegionAdvertisingService()
+        service.setName("rpc-advertise")
+        service.setServiceParent(eventloop.services)
+        self.addCleanup(service.disownServiceParent)
+        region_id = getRegionID()
+        self.assertThat(region_id, IsUnfiredDeferred())
+        service.advertising.set(
+            RegionAdvertising(sentinel.region_id, sentinel.process_id))
+        self.assertThat(region_id, IsFiredDeferred())
+        self.assertThat(region_id.result, Is(sentinel.region_id))
+
+
 class TestRegionServer(MAASTransactionServerTestCase):
 
     def test_interfaces(self):
@@ -1209,7 +1234,8 @@
         protocol.ident = factory.make_name("node")
         protocol.host = sentinel.host
         protocol.hostIsRemote = True
-        protocol.getRegionID = lambda: succeed(sentinel.region_id)
+        getRegionID = self.patch_autospec(regionservice, "getRegionID")
+        getRegionID.return_value = succeed(sentinel.region_id)
         connectionLost_up_call = self.patch(amp.AMP, "connectionLost")
         service.connections[protocol.ident] = {protocol}
 
@@ -1348,10 +1374,9 @@
         self.assertIsNotNone(responder)
 
     def installFakeRegionAdvertisingService(self):
-        service = Service()
+        service = RegionAdvertisingService()
         service.setName("rpc-advertise")
-        service.advertising = DeferredValue()
-        service.advertising.set(Mock(
+        service.advertising.set(RegionAdvertising(
             region_id=factory.make_name("region-id"),
             process_id=randint(1000, 9999)))
         service.setServiceParent(eventloop.services)