Merge ~blake-rouse/maas:cache-rack-service-state into maas:master

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: 83f0c913ee479f73e0ca77c696999c453198a1a7
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:cache-rack-service-state
Merge into: maas:master
Diff against target: 146 lines (+101/-2)
2 files modified
src/provisioningserver/rackdservices/service_monitor_service.py (+7/-1)
src/provisioningserver/rackdservices/tests/test_service_monitor_service.py (+94/-1)
Reviewer Review Type Date Requested Status
Newell Jensen (community) Approve
Review via email: mp+337604@code.launchpad.net

Commit message

Fixes LP: #1748569 - Only update the region when the service status change.

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

Looks good +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/provisioningserver/rackdservices/service_monitor_service.py b/src/provisioningserver/rackdservices/service_monitor_service.py
2index 8a1da9a..f791161 100644
3--- a/src/provisioningserver/rackdservices/service_monitor_service.py
4+++ b/src/provisioningserver/rackdservices/service_monitor_service.py
5@@ -60,6 +60,7 @@ class ServiceMonitorService(TimerService, object):
6 self.check_interval, self.monitorServices)
7 self.client_service = client_service
8 self.clock = clock
9+ self._services = None
10
11 def monitorServices(self):
12 """Monitors all of the external services and makes sure they
13@@ -79,6 +80,12 @@ class ServiceMonitorService(TimerService, object):
14 @inlineCallbacks
15 def _updateRegion(self, services):
16 """Update region about services status."""
17+ services = yield self._buildServices(services)
18+ if self._services is not None and self._services == services:
19+ # The updated status to the region hasn't changed no reason
20+ # to update the region controller.
21+ return None
22+ self._services = services
23 client = None
24 for elapsed, remaining, wait in retries(30, 10, self.clock):
25 try:
26@@ -91,7 +98,6 @@ class ServiceMonitorService(TimerService, object):
27 "Can't update service statuses, no RPC "
28 "connection to region.")
29 return
30- services = yield self._buildServices(services)
31 yield client(
32 UpdateServices,
33 system_id=client.localIdent,
34diff --git a/src/provisioningserver/rackdservices/tests/test_service_monitor_service.py b/src/provisioningserver/rackdservices/tests/test_service_monitor_service.py
35index a971e84..5bb7e87 100644
36--- a/src/provisioningserver/rackdservices/tests/test_service_monitor_service.py
37+++ b/src/provisioningserver/rackdservices/tests/test_service_monitor_service.py
38@@ -105,7 +105,7 @@ class TestServiceMonitorService(MAASTestCase):
39 ...""", logger.output)
40
41 @inlineCallbacks
42- def test_reports_services_to_region(self):
43+ def test_reports_services_to_region_on_start(self):
44 # Pretend we're in a production environment.
45 self.patch(sms, "is_dev_environment").return_value = False
46
47@@ -149,6 +149,99 @@ class TestServiceMonitorService(MAASTestCase):
48 services=expected_services))
49
50 @inlineCallbacks
51+ def test_reports_services_to_region_when_changed(self):
52+ # Pretend we're in a production environment.
53+ self.patch(sms, "is_dev_environment").return_value = False
54+
55+ protocol, connecting = self.patch_rpc_methods()
56+ self.addCleanup((yield connecting))
57+
58+ class ExampleService(AlwaysOnService):
59+ name = service_name = snap_service_name = (
60+ factory.make_name("service"))
61+
62+ service = ExampleService()
63+ # Inveigle this new service into the service monitor.
64+ self.addCleanup(service_monitor._services.pop, service.name)
65+ service_monitor._services[service.name] = service
66+
67+ state = ServiceState(SERVICE_STATE.ON, "running")
68+ mock_ensureServices = self.patch(service_monitor, "ensureServices")
69+ mock_ensureServices.return_value = succeed({
70+ service.name: state
71+ })
72+
73+ client = getRegionClient()
74+ rpc_service = Mock()
75+ rpc_service.getClientNow.return_value = succeed(client)
76+ monitor_service = sms.ServiceMonitorService(
77+ rpc_service, Clock())
78+ monitor_service._services = yield monitor_service._buildServices({
79+ service.name: state
80+ })
81+
82+ # Force last reported state to dead. That way an update is performed.
83+ orig_cached_services = monitor_service._services
84+ for ser in orig_cached_services:
85+ ser['status'] = 'dead'
86+
87+ yield monitor_service.startService()
88+ yield monitor_service.stopService()
89+
90+ expected_services = list(monitor_service.ALWAYS_RUNNING_SERVICES)
91+ expected_services.append({
92+ "name": service.name,
93+ "status": "running",
94+ "status_info": "",
95+ })
96+ self.assertThat(
97+ protocol.UpdateServices,
98+ MockCalledOnceWith(
99+ protocol,
100+ system_id=client.localIdent,
101+ services=expected_services))
102+ self.assertIsNot(orig_cached_services, monitor_service._services)
103+
104+ @inlineCallbacks
105+ def test_doesnt_reports_services_to_region_when_the_same_status(self):
106+ # Pretend we're in a production environment.
107+ self.patch(sms, "is_dev_environment").return_value = False
108+
109+ protocol, connecting = self.patch_rpc_methods()
110+ self.addCleanup((yield connecting))
111+
112+ class ExampleService(AlwaysOnService):
113+ name = service_name = snap_service_name = (
114+ factory.make_name("service"))
115+
116+ service = ExampleService()
117+ # Inveigle this new service into the service monitor.
118+ self.addCleanup(service_monitor._services.pop, service.name)
119+ service_monitor._services[service.name] = service
120+
121+ state = ServiceState(SERVICE_STATE.ON, "running")
122+ mock_ensureServices = self.patch(service_monitor, "ensureServices")
123+ mock_ensureServices.return_value = succeed({
124+ service.name: state
125+ })
126+
127+ client = getRegionClient()
128+ rpc_service = Mock()
129+ rpc_service.getClientNow.return_value = succeed(client)
130+ monitor_service = sms.ServiceMonitorService(
131+ rpc_service, Clock())
132+ monitor_service._services = yield monitor_service._buildServices({
133+ service.name: state
134+ })
135+
136+ yield monitor_service.startService()
137+ yield monitor_service.stopService()
138+
139+ self.assertThat(
140+ protocol.UpdateServices,
141+ MockNotCalled())
142+
143+ @inlineCallbacks
144 def test__buildServices_includes_always_running_services(self):
145 monitor_service = sms.ServiceMonitorService(
146 sentinel.client_service, Clock())

Subscribers

People subscribed via source and target branches