Merge lp:~allenap/maas/rack-controller-service--bug-1578800--service-monitor into lp:~maas-committers/maas/trunk

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 5006
Proposed branch: lp:~allenap/maas/rack-controller-service--bug-1578800--service-monitor
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 157 lines (+26/-27)
3 files modified
src/maasserver/service_monitor.py (+3/-2)
src/maasserver/tests/test_rack_controller.py (+14/-15)
src/maasserver/tests/test_service_monitor.py (+9/-10)
To merge this branch: bzr merge lp:~allenap/maas/rack-controller-service--bug-1578800--service-monitor
Reviewer Review Type Date Requested Status
LaMont Jones (community) Approve
Review via email: mp+294218@code.launchpad.net

Commit message

Update ServiceMonitorService to use the new RegionAdvertisingService API.

In addition, improve testing for the other consumer of RegionAdvertisingService such that future API changes will result in test failures.

To post a comment you must log in.
Revision history for this message
LaMont Jones (lamont) wrote :

I especially like the testing changes! +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/service_monitor.py'
2--- src/maasserver/service_monitor.py 2016-04-15 20:43:22 +0000
3+++ src/maasserver/service_monitor.py 2016-05-10 10:27:58 +0000
4@@ -98,9 +98,10 @@
5 @inlineCallbacks
6 def _updateDatabase(self, services):
7 """Update database about services status."""
8- processId = yield self.advertisingService.processId.get()
9+ advertising = yield self.advertisingService.advertising.get()
10 services = yield self._buildServices(services)
11- yield deferToDatabase(self._saveIntoDatabase, processId, services)
12+ yield deferToDatabase(
13+ self._saveIntoDatabase, advertising.process_id, services)
14
15 @transactional
16 def _saveIntoDatabase(self, processId, services):
17
18=== modified file 'src/maasserver/tests/test_rack_controller.py'
19--- src/maasserver/tests/test_rack_controller.py 2016-05-06 10:23:49 +0000
20+++ src/maasserver/tests/test_rack_controller.py 2016-05-10 10:27:58 +0000
21@@ -8,6 +8,7 @@
22 import random
23 from unittest.mock import (
24 call,
25+ create_autospec,
26 Mock,
27 sentinel,
28 )
29@@ -29,7 +30,6 @@
30 MockCallsMatch,
31 MockNotCalled,
32 )
33-from provisioningserver.utils.twisted import DeferredValue
34 from testtools import ExpectedException
35 from testtools.matchers import MatchesStructure
36 from twisted.internet import reactor
37@@ -58,11 +58,11 @@
38 postgresListener=sentinel.listener,
39 advertisingService=sentinel.advertiser))
40
41- def test_startService_sets_starting_to_result_of_processId_get(self):
42- advertising = Mock(DeferredValue, get=Mock())
43- advertiser = Mock(RegionAdvertisingService, advertising=advertising)
44+ def test_startService_sets_starting_to_result_of_advertising_get(self):
45+ advertiser = create_autospec(RegionAdvertisingService(), spec_set=True)
46 service = RackControllerService(sentinel.listener, advertiser)
47 observed = service.startService()
48+ advertising = advertiser.advertising
49 self.assertEqual(advertising.get.return_value, observed)
50 self.assertEqual(advertising.get.return_value, service.starting)
51
52@@ -71,9 +71,9 @@
53 def test_startService_registers_with_postgres_listener(self):
54 regionProcessId = random.randint(0, 100)
55
56- advertising = DeferredValue()
57- advertising.set(RegionAdvertising(sentinel.region_id, regionProcessId))
58- advertiser = Mock(RegionAdvertisingService, advertising=advertising)
59+ advertiser = RegionAdvertisingService()
60+ advertiser.advertising.set(
61+ RegionAdvertising(sentinel.region_id, regionProcessId))
62
63 listener = Mock()
64 service = RackControllerService(listener, advertiser)
65@@ -90,9 +90,9 @@
66 def test_startService_clears_starting_once_complete(self):
67 regionProcessId = random.randint(0, 100)
68
69- advertising = DeferredValue()
70- advertising.set(RegionAdvertising(sentinel.region_id, regionProcessId))
71- advertiser = Mock(RegionAdvertisingService, advertising=advertising)
72+ advertiser = RegionAdvertisingService()
73+ advertiser.advertising.set(
74+ RegionAdvertising(sentinel.region_id, regionProcessId))
75
76 listener = Mock()
77 service = RackControllerService(listener, advertiser)
78@@ -101,8 +101,7 @@
79
80 @wait_for_reactor
81 def test_startService_handles_cancel(self):
82- advertising = DeferredValue()
83- advertiser = Mock(RegionAdvertisingService, advertising=advertising)
84+ advertiser = RegionAdvertisingService()
85
86 listener = Mock()
87 service = RackControllerService(listener, advertiser)
88@@ -128,9 +127,9 @@
89 process, rack_controllers = yield deferToDatabase(
90 create_process_and_racks)
91
92- advertising = DeferredValue()
93- advertising.set(RegionAdvertising(sentinel.region_id, process.id))
94- advertiser = Mock(RegionAdvertisingService, advertising=advertising)
95+ advertiser = RegionAdvertisingService()
96+ advertiser.advertising.set(
97+ RegionAdvertising(sentinel.region_id, process.id))
98
99 listener = Mock()
100 service = RackControllerService(listener, advertiser)
101
102=== modified file 'src/maasserver/tests/test_service_monitor.py'
103--- src/maasserver/tests/test_service_monitor.py 2016-05-06 11:41:05 +0000
104+++ src/maasserver/tests/test_service_monitor.py 2016-05-10 10:27:58 +0000
105@@ -7,10 +7,7 @@
106
107 import os
108 import random
109-from unittest.mock import (
110- Mock,
111- sentinel,
112-)
113+from unittest.mock import sentinel
114
115 from crochet import wait_for
116 from maasserver import (
117@@ -20,6 +17,10 @@
118 from maasserver.enum import SERVICE_STATUS
119 from maasserver.models.config import Config
120 from maasserver.models.service import Service
121+from maasserver.rpc.regionservice import (
122+ RegionAdvertising,
123+ RegionAdvertisingService,
124+)
125 from maasserver.service_monitor import (
126 ProxyService,
127 service_monitor,
128@@ -39,7 +40,6 @@
129 SERVICE_STATE,
130 ServiceState,
131 )
132-from provisioningserver.utils.twisted import DeferredValue
133 from testtools.matchers import MatchesStructure
134 from twisted.internet.defer import (
135 fail,
136@@ -131,17 +131,16 @@
137 service.name: state
138 })
139
140- advertisingService = Mock()
141- advertisingService.processId = DeferredValue()
142- monitor_service = ServiceMonitorService(
143- advertisingService, Clock())
144+ advertiser = RegionAdvertisingService()
145+ monitor_service = ServiceMonitorService(advertiser, Clock())
146 yield monitor_service.startService()
147
148 region = yield deferToDatabase(
149 transactional(factory.make_RegionController))
150 region_process = yield deferToDatabase(
151 transactional(factory.make_RegionControllerProcess), region)
152- advertisingService.processId.set(region_process.id)
153+ advertiser.advertising.set(
154+ RegionAdvertising(region.id, region_process.id))
155 yield monitor_service.stopService()
156
157 service = yield deferToDatabase(