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

Proposed by Gavin Panella on 2016-05-13
Status: Merged
Approved by: Andres Rodriguez on 2016-05-13
Approved revision: 5023
Merged at revision: 5024
Proposed branch: lp:~allenap/maas/region-rpc-broken--bug-1581654
Merge into: lp:maas/trunk
Diff against target: 136 lines (+46/-12)
2 files modified
src/maasserver/rpc/regionservice.py (+17/-8)
src/maasserver/rpc/tests/test_regionservice.py (+29/-4)
To merge this branch: bzr merge lp:~allenap/maas/region-rpc-broken--bug-1581654
Reviewer Review Type Date Requested Status
Lee Trager 2016-05-13 Approve on 2016-05-13
Review via email: mp+294685@code.launchpad.net

Commit message

Fix getRegionID.

To post a comment you must log in.
Lee Trager (ltrager) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/rpc/regionservice.py'
2--- src/maasserver/rpc/regionservice.py 2016-05-13 17:29:37 +0000
3+++ src/maasserver/rpc/regionservice.py 2016-05-13 21:52:14 +0000
4@@ -106,6 +106,7 @@
5 )
6 from twisted.internet.defer import (
7 CancelledError,
8+ fail,
9 inlineCallbacks,
10 maybeDeferred,
11 returnValue,
12@@ -482,6 +483,20 @@
13 endpoint=endpoint, rack_controller=rack_controller).delete()
14
15
16+def getRegionID():
17+ """Obtain the region ID from the advertising service.
18+
19+ :return: :class:`Deferred`
20+ """
21+ try:
22+ advertise = eventloop.services.getServiceNamed("rpc-advertise")
23+ except:
24+ return fail()
25+ else:
26+ return advertise.advertising.get().addCallback(
27+ lambda advertising: advertising.region_id)
28+
29+
30 @implementer(IConnection)
31 class RegionServer(Region):
32 """The RPC protocol supported by a region controller, server version.
33@@ -512,12 +527,6 @@
34 digest_local = calculate_digest(secret, message, salt)
35 returnValue(digest == digest_local)
36
37- def getRegionID(self):
38- """Obtain the region ID from the advertising service."""
39- advertise = eventloop.services.getServiceNamed("rpc-advertise")
40- advertise.advertising.get().addCallback(
41- lambda advertising: advertising.region_id)
42-
43 @region.RegisterRackController.responder
44 @inlineCallbacks
45 def register(
46@@ -549,7 +558,7 @@
47 # Get the region ID if we're dealing with a non-local rack; we
48 # won't need to bother for local racks.
49 if self.hostIsRemote:
50- region_id = yield self.getRegionID()
51+ region_id = yield getRegionID()
52
53 # Only register the connection into the database when its a valid
54 # IPv4 and IPv6. Only time it is not an IPv4 or IPv6 address is
55@@ -626,7 +635,7 @@
56
57 def connectionLost(self, reason):
58 if self.hostIsRemote:
59- self.getRegionID().addCallback(
60+ getRegionID().addCallback(
61 lambda region_id: deferToDatabase(
62 unregisterConnection, region_id, self.ident, self.host))
63 self.factory.service._removeConnectionFor(self.ident, self)
64
65=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
66--- src/maasserver/rpc/tests/test_regionservice.py 2016-05-13 17:29:37 +0000
67+++ src/maasserver/rpc/tests/test_regionservice.py 2016-05-13 21:52:14 +0000
68@@ -65,6 +65,7 @@
69 regionservice,
70 )
71 from maasserver.rpc.regionservice import (
72+ getRegionID,
73 Region,
74 RegionAdvertising,
75 RegionAdvertisingService,
76@@ -90,6 +91,8 @@
77 )
78 from maasserver.utils.threads import deferToDatabase
79 from maastesting.matchers import (
80+ IsFiredDeferred,
81+ IsUnfiredDeferred,
82 MockAnyCall,
83 MockCalledOnce,
84 MockCalledOnceWith,
85@@ -1155,6 +1158,28 @@
86 endpoint=endpoint, rack_controller=rack_controller).first())
87
88
89+class TestGetRegionID(MAASTestCase):
90+ """Tests for `getRegionID`."""
91+
92+ def test__getRegionID_fails_when_advertising_service_not_running(self):
93+ region_id = getRegionID()
94+ self.assertThat(region_id, IsFiredDeferred())
95+ error = self.assertRaises(KeyError, extract_result, region_id)
96+ self.assertThat(str(error), Equals(repr('rpc-advertise')))
97+
98+ def test__getRegionID_returns_the_region_ID_when_available(self):
99+ service = RegionAdvertisingService()
100+ service.setName("rpc-advertise")
101+ service.setServiceParent(eventloop.services)
102+ self.addCleanup(service.disownServiceParent)
103+ region_id = getRegionID()
104+ self.assertThat(region_id, IsUnfiredDeferred())
105+ service.advertising.set(
106+ RegionAdvertising(sentinel.region_id, sentinel.process_id))
107+ self.assertThat(region_id, IsFiredDeferred())
108+ self.assertThat(region_id.result, Is(sentinel.region_id))
109+
110+
111 class TestRegionServer(MAASTransactionServerTestCase):
112
113 def test_interfaces(self):
114@@ -1209,7 +1234,8 @@
115 protocol.ident = factory.make_name("node")
116 protocol.host = sentinel.host
117 protocol.hostIsRemote = True
118- protocol.getRegionID = lambda: succeed(sentinel.region_id)
119+ getRegionID = self.patch_autospec(regionservice, "getRegionID")
120+ getRegionID.return_value = succeed(sentinel.region_id)
121 connectionLost_up_call = self.patch(amp.AMP, "connectionLost")
122 service.connections[protocol.ident] = {protocol}
123
124@@ -1348,10 +1374,9 @@
125 self.assertIsNotNone(responder)
126
127 def installFakeRegionAdvertisingService(self):
128- service = Service()
129+ service = RegionAdvertisingService()
130 service.setName("rpc-advertise")
131- service.advertising = DeferredValue()
132- service.advertising.set(Mock(
133+ service.advertising.set(RegionAdvertising(
134 region_id=factory.make_name("region-id"),
135 process_id=randint(1000, 9999)))
136 service.setServiceParent(eventloop.services)