Merge lp:~blake-rouse/maas/external-dhcp-probe into lp:~maas-committers/maas/trunk
- external-dhcp-probe
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Blake Rouse | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 4884 | ||||
Proposed branch: | lp:~blake-rouse/maas/external-dhcp-probe | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
1353 lines (+240/-688) 12 files modified
Makefile (+1/-2) buildout.cfg (+0/-2) src/maasserver/rpc/rackcontrollers.py (+28/-47) src/maasserver/rpc/regionservice.py (+5/-18) src/maasserver/rpc/tests/test_rackcontrollers.py (+54/-1) src/maasserver/rpc/tests/test_regionservice.py (+36/-90) src/provisioningserver/dhcp/detect.py (+4/-127) src/provisioningserver/dhcp/probe.py (+0/-41) src/provisioningserver/dhcp/tests/test_detect.py (+4/-213) src/provisioningserver/pserv_services/dhcp_probe_service.py (+36/-47) src/provisioningserver/pserv_services/tests/test_dhcp_probe_service.py (+68/-77) src/provisioningserver/rpc/region.py (+4/-23) |
||||
To merge this branch: | bzr merge lp:~blake-rouse/maas/external-dhcp-probe | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email:
|
Commit message
Update the external_dhcp field on the VLAN for discovered DHCP servers. Remove code that is no longer be used for DHCP probe.
Description of the change
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Blake Rouse (blake-rouse) wrote : | # |
Thanks for the review. I fixed you inline comment.
Believe it or not the bulk of this code could have been removed back in 1.7. When MAAS was switched from celery to RPC most of this code stopped being used. I have tested this on a installed system and MAAS has permission. Really I didn't change any of the probing code, I just removed what is no longer used.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'Makefile' | |||
2 | --- Makefile 2016-03-28 13:54:47 +0000 | |||
3 | +++ Makefile 2016-04-07 13:22:34 +0000 | |||
4 | @@ -54,7 +54,6 @@ | |||
5 | 54 | bin/buildout \ | 54 | bin/buildout \ |
6 | 55 | bin/database \ | 55 | bin/database \ |
7 | 56 | bin/maas \ | 56 | bin/maas \ |
8 | 57 | bin/maas-probe-dhcp \ | ||
9 | 58 | bin/maas-rack \ | 57 | bin/maas-rack \ |
10 | 59 | bin/maas-region \ | 58 | bin/maas-region \ |
11 | 60 | bin/twistd.rack \ | 59 | bin/twistd.rack \ |
12 | @@ -135,7 +134,7 @@ | |||
13 | 135 | $(buildout) install testing-test | 134 | $(buildout) install testing-test |
14 | 136 | @touch --no-create $@ | 135 | @touch --no-create $@ |
15 | 137 | 136 | ||
17 | 138 | bin/maas-probe-dhcp bin/maas-rack bin/twistd.rack: \ | 137 | bin/maas-rack bin/twistd.rack: \ |
18 | 139 | bin/buildout buildout.cfg versions.cfg setup.py | 138 | bin/buildout buildout.cfg versions.cfg setup.py |
19 | 140 | $(buildout) install rack | 139 | $(buildout) install rack |
20 | 141 | @touch --no-create $@ | 140 | @touch --no-create $@ |
21 | 142 | 141 | ||
22 | === modified file 'buildout.cfg' | |||
23 | --- buildout.cfg 2016-03-28 13:54:47 +0000 | |||
24 | +++ buildout.cfg 2016-04-07 13:22:34 +0000 | |||
25 | @@ -189,13 +189,11 @@ | |||
26 | 189 | recipe = zc.recipe.egg | 189 | recipe = zc.recipe.egg |
27 | 190 | eggs = | 190 | eggs = |
28 | 191 | entry-points = | 191 | entry-points = |
29 | 192 | maas-probe-dhcp=provisioningserver.dhcp.probe:main | ||
30 | 193 | maas-rack=provisioningserver.__main__:main | 192 | maas-rack=provisioningserver.__main__:main |
31 | 194 | twistd.rack=twisted.scripts.twistd:run | 193 | twistd.rack=twisted.scripts.twistd:run |
32 | 195 | extra-paths = | 194 | extra-paths = |
33 | 196 | ${common:extra-paths} | 195 | ${common:extra-paths} |
34 | 197 | scripts = | 196 | scripts = |
35 | 198 | maas-probe-dhcp | ||
36 | 199 | maas-rack | 197 | maas-rack |
37 | 200 | twistd.rack | 198 | twistd.rack |
38 | 201 | initialization = | 199 | initialization = |
39 | 202 | 200 | ||
40 | === modified file 'src/maasserver/rpc/rackcontrollers.py' | |||
41 | --- src/maasserver/rpc/rackcontrollers.py 2016-03-28 13:54:47 +0000 | |||
42 | +++ src/maasserver/rpc/rackcontrollers.py 2016-04-07 13:22:34 +0000 | |||
43 | @@ -16,10 +16,10 @@ | |||
44 | 16 | from maasserver import worker_user | 16 | from maasserver import worker_user |
45 | 17 | from maasserver.enum import NODE_TYPE | 17 | from maasserver.enum import NODE_TYPE |
46 | 18 | from maasserver.models import ( | 18 | from maasserver.models import ( |
47 | 19 | Interface, | ||
48 | 20 | Node, | 19 | Node, |
49 | 21 | NodeGroupToRackController, | 20 | NodeGroupToRackController, |
50 | 22 | RackController, | 21 | RackController, |
51 | 22 | StaticIPAddress, | ||
52 | 23 | ) | 23 | ) |
53 | 24 | from maasserver.models.node import typecast_node | 24 | from maasserver.models.node import typecast_node |
54 | 25 | from maasserver.utils.orm import transactional | 25 | from maasserver.utils.orm import transactional |
55 | @@ -163,52 +163,33 @@ | |||
56 | 163 | 163 | ||
57 | 164 | 164 | ||
58 | 165 | @transactional | 165 | @transactional |
105 | 166 | def update_foreign_dhcp_ip(cluster_uuid, interface_name, foreign_dhcp_ip): | 166 | def update_foreign_dhcp(system_id, interface_name, dhcp_ip=None): |
106 | 167 | """Update the foreign_dhcp_ip field of a given interface on a cluster. | 167 | """Update the external_dhcp field of the VLAN for the interface. |
107 | 168 | 168 | ||
108 | 169 | Note: We do this through an update, not a read/modify/write. | 169 | :param system_id: Rack controller system_id. |
109 | 170 | Updating NodeGroupInterface client-side may inadvertently trigger | 170 | :param interface_name: The name of the interface. |
110 | 171 | Django signals that cause a rewrite of the DHCP config, plus restart | 171 | :param dhcp_ip: The IP address of the responding DHCP server. |
111 | 172 | of the DHCP server. The inadvertent triggering has been known to | 172 | """ |
112 | 173 | happen because of race conditions between read/modify/write | 173 | rack_controller = RackController.objects.get(system_id=system_id) |
113 | 174 | transactions that were enabled by Django defaulting to, and being | 174 | interface = rack_controller.interface_set.filter( |
114 | 175 | designed for, the READ COMMITTED isolation level; the ORM writing | 175 | name=interface_name).select_related("vlan").first() |
115 | 176 | back even unmodified fields; and GenericIPAddressField's default | 176 | if interface is not None: |
116 | 177 | value being prone to problems where NULL is sometimes represented as | 177 | if dhcp_ip is not None: |
117 | 178 | None, sometimes as an empty string, and the difference being enough | 178 | sip = StaticIPAddress.objects.filter(ip=dhcp_ip).first() |
118 | 179 | to convince the signal machinery that these fields have changed when | 179 | if sip is not None: |
119 | 180 | in fact they have not. | 180 | # Check that its not an IP address of a rack controller |
120 | 181 | 181 | # providing that DHCP service. | |
121 | 182 | :param cluster_uuid: Cluster's UUID. | 182 | rack_interfaces_serving_dhcp = sip.interface_set.filter( |
122 | 183 | :param interface_name: The name of the cluster interface on which the | 183 | node__node_type__in=[ |
123 | 184 | foreign DHCP server was (or wasn't) discovered. | 184 | NODE_TYPE.RACK_CONTROLLER, |
124 | 185 | :param foreign_dhcp_ip: IP address of foreign DCHP server, if any. | 185 | NODE_TYPE.REGION_AND_RACK_CONTROLLER], |
125 | 186 | """ | 186 | vlan__dhcp_on=True) |
126 | 187 | # XXX 2016-01-20 blake_r - Currently no where to place this information. | 187 | if rack_interfaces_serving_dhcp.exists(): |
127 | 188 | # Need to add to the model to store this information. | 188 | # Not external. It's a MAAS DHCP server. |
128 | 189 | pass | 189 | dhcp_ip = None |
129 | 190 | 190 | if interface.vlan.external_dhcp != dhcp_ip: | |
130 | 191 | 191 | interface.vlan.external_dhcp = dhcp_ip | |
131 | 192 | @transactional | 192 | interface.vlan.save() |
86 | 193 | def get_rack_controllers_interfaces_as_dicts(system_id): | ||
87 | 194 | """Return all the interfaces on a given rack controller as a list of dicts. | ||
88 | 195 | |||
89 | 196 | :return: A list of dicts in the form {'name': interface.name, | ||
90 | 197 | 'interface': interface.interface, 'ip': interface.ip}, one dict per | ||
91 | 198 | interface on the cluster. | ||
92 | 199 | """ | ||
93 | 200 | interfaces = Interface.objects.filter(node__system_id=system_id) | ||
94 | 201 | # XXX 2016-01-20 blake_r - Currently not passing any IP address as it now | ||
95 | 202 | # should take a list of IP addresses and not just one IP address. To make | ||
96 | 203 | # it work for now nothing its filtered out. | ||
97 | 204 | return [ | ||
98 | 205 | { | ||
99 | 206 | 'name': interface.name, | ||
100 | 207 | 'interface': interface.name, | ||
101 | 208 | 'ip': '', | ||
102 | 209 | } | ||
103 | 210 | for interface in interfaces | ||
104 | 211 | ] | ||
132 | 212 | 193 | ||
133 | 213 | 194 | ||
134 | 214 | @synchronous | 195 | @synchronous |
135 | 215 | 196 | ||
136 | === modified file 'src/maasserver/rpc/regionservice.py' | |||
137 | --- src/maasserver/rpc/regionservice.py 2016-03-31 23:34:55 +0000 | |||
138 | +++ src/maasserver/rpc/regionservice.py 2016-04-07 13:22:34 +0000 | |||
139 | @@ -345,32 +345,19 @@ | |||
140 | 345 | return succeed({}) | 345 | return succeed({}) |
141 | 346 | 346 | ||
142 | 347 | @region.ReportForeignDHCPServer.responder | 347 | @region.ReportForeignDHCPServer.responder |
145 | 348 | def report_foreign_dhcp_server(self, cluster_uuid, interface_name, | 348 | def report_foreign_dhcp_server( |
146 | 349 | foreign_dhcp_ip): | 349 | self, system_id, interface_name, dhcp_ip=None): |
147 | 350 | """report_foreign_dhcp_server() | 350 | """report_foreign_dhcp_server() |
148 | 351 | 351 | ||
149 | 352 | Implementation of | 352 | Implementation of |
151 | 353 | :py:class:`~provisioningserver.rpc.region.SendEvent`. | 353 | :py:class:`~provisioningserver.rpc.region.ReportForeignDHCPServer`. |
152 | 354 | """ | 354 | """ |
153 | 355 | d = deferToDatabase( | 355 | d = deferToDatabase( |
156 | 356 | rackcontrollers.update_foreign_dhcp_ip, | 356 | rackcontrollers.update_foreign_dhcp, |
157 | 357 | cluster_uuid, interface_name, foreign_dhcp_ip) | 357 | system_id, interface_name, dhcp_ip) |
158 | 358 | d.addCallback(lambda _: {}) | 358 | d.addCallback(lambda _: {}) |
159 | 359 | return d | 359 | return d |
160 | 360 | 360 | ||
161 | 361 | @region.GetClusterInterfaces.responder | ||
162 | 362 | def get_cluster_interfaces(self, cluster_uuid): | ||
163 | 363 | """get_cluster_interfaces() | ||
164 | 364 | |||
165 | 365 | Implementation of | ||
166 | 366 | :py:class:`~provisioningserver.rpc.region.GetClusterInterfaces`. | ||
167 | 367 | """ | ||
168 | 368 | d = deferToDatabase( | ||
169 | 369 | rackcontrollers.get_rack_controllers_interfaces_as_dicts, | ||
170 | 370 | cluster_uuid) | ||
171 | 371 | d.addCallback(lambda interfaces: {'interfaces': interfaces}) | ||
172 | 372 | return d | ||
173 | 373 | |||
174 | 374 | @region.CreateNode.responder | 361 | @region.CreateNode.responder |
175 | 375 | def create_node(self, architecture, power_type, power_parameters, | 362 | def create_node(self, architecture, power_type, power_parameters, |
176 | 376 | mac_addresses, domain=None, hostname=None): | 363 | mac_addresses, domain=None, hostname=None): |
177 | 377 | 364 | ||
178 | === modified file 'src/maasserver/rpc/tests/test_rackcontrollers.py' | |||
179 | --- src/maasserver/rpc/tests/test_rackcontrollers.py 2016-03-28 13:54:47 +0000 | |||
180 | +++ src/maasserver/rpc/tests/test_rackcontrollers.py 2016-04-07 13:22:34 +0000 | |||
181 | @@ -10,7 +10,11 @@ | |||
182 | 10 | from django.db import IntegrityError | 10 | from django.db import IntegrityError |
183 | 11 | from fixtures import FakeLogger | 11 | from fixtures import FakeLogger |
184 | 12 | from maasserver import worker_user | 12 | from maasserver import worker_user |
186 | 13 | from maasserver.enum import NODE_TYPE | 13 | from maasserver.enum import ( |
187 | 14 | INTERFACE_TYPE, | ||
188 | 15 | IPADDRESS_TYPE, | ||
189 | 16 | NODE_TYPE, | ||
190 | 17 | ) | ||
191 | 14 | from maasserver.models import ( | 18 | from maasserver.models import ( |
192 | 15 | Node, | 19 | Node, |
193 | 16 | NodeGroupToRackController, | 20 | NodeGroupToRackController, |
194 | @@ -20,6 +24,7 @@ | |||
195 | 20 | handle_upgrade, | 24 | handle_upgrade, |
196 | 21 | register_new_rackcontroller, | 25 | register_new_rackcontroller, |
197 | 22 | register_rackcontroller, | 26 | register_rackcontroller, |
198 | 27 | update_foreign_dhcp, | ||
199 | 23 | update_interfaces, | 28 | update_interfaces, |
200 | 24 | ) | 29 | ) |
201 | 25 | from maasserver.testing.factory import factory | 30 | from maasserver.testing.factory import factory |
202 | @@ -248,6 +253,54 @@ | |||
203 | 248 | hostname, logger.output.strip()) | 253 | hostname, logger.output.strip()) |
204 | 249 | 254 | ||
205 | 250 | 255 | ||
206 | 256 | class TestUpdateForeignDHCP(MAASServerTestCase): | ||
207 | 257 | |||
208 | 258 | def test__doesnt_fail_if_interface_missing(self): | ||
209 | 259 | rack_controller = factory.make_RackController() | ||
210 | 260 | # No error should be raised. | ||
211 | 261 | update_foreign_dhcp( | ||
212 | 262 | rack_controller.system_id, factory.make_name("eth"), None) | ||
213 | 263 | |||
214 | 264 | def test__clears_external_dhcp_on_vlan(self): | ||
215 | 265 | rack_controller = factory.make_RackController(interface=False) | ||
216 | 266 | interface = factory.make_Interface( | ||
217 | 267 | INTERFACE_TYPE.PHYSICAL, node=rack_controller) | ||
218 | 268 | interface.vlan.external_dhcp = factory.make_ip_address() | ||
219 | 269 | interface.vlan.save() | ||
220 | 270 | update_foreign_dhcp( | ||
221 | 271 | rack_controller.system_id, interface.name, None) | ||
222 | 272 | self.assertIsNone(reload_object(interface.vlan).external_dhcp) | ||
223 | 273 | |||
224 | 274 | def test__sets_external_dhcp_when_not_managed_vlan(self): | ||
225 | 275 | rack_controller = factory.make_RackController(interface=False) | ||
226 | 276 | interface = factory.make_Interface( | ||
227 | 277 | INTERFACE_TYPE.PHYSICAL, node=rack_controller) | ||
228 | 278 | dhcp_ip = factory.make_ip_address() | ||
229 | 279 | update_foreign_dhcp( | ||
230 | 280 | rack_controller.system_id, interface.name, dhcp_ip) | ||
231 | 281 | self.assertEquals( | ||
232 | 282 | dhcp_ip, reload_object(interface.vlan).external_dhcp) | ||
233 | 283 | |||
234 | 284 | def test__clears_external_dhcp_when_managed_vlan(self): | ||
235 | 285 | rack_controller = factory.make_RackController(interface=False) | ||
236 | 286 | fabric = factory.make_Fabric() | ||
237 | 287 | vlan = fabric.get_default_vlan() | ||
238 | 288 | interface = factory.make_Interface( | ||
239 | 289 | INTERFACE_TYPE.PHYSICAL, node=rack_controller, vlan=vlan) | ||
240 | 290 | subnet = factory.make_Subnet() | ||
241 | 291 | dhcp_ip = factory.pick_ip_in_Subnet(subnet) | ||
242 | 292 | vlan.dhcp_on = True | ||
243 | 293 | vlan.primary_rack = rack_controller | ||
244 | 294 | vlan.external_dhcp = dhcp_ip | ||
245 | 295 | vlan.save() | ||
246 | 296 | factory.make_StaticIPAddress( | ||
247 | 297 | alloc_type=IPADDRESS_TYPE.STICKY, ip=dhcp_ip, | ||
248 | 298 | subnet=subnet, interface=interface) | ||
249 | 299 | update_foreign_dhcp( | ||
250 | 300 | rack_controller.system_id, interface.name, dhcp_ip) | ||
251 | 301 | self.assertIsNone(reload_object(interface.vlan).external_dhcp) | ||
252 | 302 | |||
253 | 303 | |||
254 | 251 | class TestUpdateInterfaces(MAASServerTestCase): | 304 | class TestUpdateInterfaces(MAASServerTestCase): |
255 | 252 | 305 | ||
256 | 253 | def test__calls_update_interfaces_on_rack_controller(self): | 306 | def test__calls_update_interfaces_on_rack_controller(self): |
257 | 254 | 307 | ||
258 | === modified file 'src/maasserver/rpc/tests/test_regionservice.py' | |||
259 | --- src/maasserver/rpc/tests/test_regionservice.py 2016-03-31 23:34:55 +0000 | |||
260 | +++ src/maasserver/rpc/tests/test_regionservice.py 2016-04-07 13:22:34 +0000 | |||
261 | @@ -121,7 +121,6 @@ | |||
262 | 121 | GetBootConfig, | 121 | GetBootConfig, |
263 | 122 | GetBootSources, | 122 | GetBootSources, |
264 | 123 | GetBootSourcesV2, | 123 | GetBootSourcesV2, |
265 | 124 | GetClusterInterfaces, | ||
266 | 125 | GetProxies, | 124 | GetProxies, |
267 | 126 | Identify, | 125 | Identify, |
268 | 127 | ListNodePowerParameters, | 126 | ListNodePowerParameters, |
269 | @@ -129,6 +128,7 @@ | |||
270 | 129 | RegisterEventType, | 128 | RegisterEventType, |
271 | 130 | RegisterRackController, | 129 | RegisterRackController, |
272 | 131 | ReportBootImages, | 130 | ReportBootImages, |
273 | 131 | ReportForeignDHCPServer, | ||
274 | 132 | RequestNodeInfoByMACAddress, | 132 | RequestNodeInfoByMACAddress, |
275 | 133 | SendEvent, | 133 | SendEvent, |
276 | 134 | SendEventMACAddress, | 134 | SendEventMACAddress, |
277 | @@ -2590,101 +2590,47 @@ | |||
278 | 2590 | # If the RPC service is down, _get_addresses() returns nothing. | 2590 | # If the RPC service is down, _get_addresses() returns nothing. |
279 | 2591 | self.assertItemsEqual([], service._get_addresses()) | 2591 | self.assertItemsEqual([], service._get_addresses()) |
280 | 2592 | 2592 | ||
344 | 2593 | # NODE_GROUP_REMOVAL - blake_r - Fix. | 2593 | |
345 | 2594 | #class TestRegionProtocol_ReportForeignDHCPServer( | 2594 | class TestRegionProtocol_ReportForeignDHCPServer( |
346 | 2595 | # MAASTransactionServerTestCase): | 2595 | MAASTransactionServerTestCase): |
347 | 2596 | # | 2596 | |
348 | 2597 | # def test_create_node_is_registered(self): | 2597 | def test_is_registered(self): |
286 | 2598 | # protocol = Region() | ||
287 | 2599 | # responder = protocol.locateResponder( | ||
288 | 2600 | # ReportForeignDHCPServer.commandName) | ||
289 | 2601 | # self.assertIsNotNone(responder) | ||
290 | 2602 | # | ||
291 | 2603 | # @transactional | ||
292 | 2604 | # def create_cluster_interface(self): | ||
293 | 2605 | # cluster = factory.make_NodeGroup() | ||
294 | 2606 | # return factory.make_NodeGroupInterface(cluster) | ||
295 | 2607 | # | ||
296 | 2608 | # @wait_for_reactor | ||
297 | 2609 | # @inlineCallbacks | ||
298 | 2610 | # def test_sets_foreign_dhcp_value(self): | ||
299 | 2611 | # foreign_dhcp_ip = factory.make_ipv4_address() | ||
300 | 2612 | # cluster_interface = yield deferToDatabase( | ||
301 | 2613 | # self.create_cluster_interface) | ||
302 | 2614 | # cluster = cluster_interface.nodegroup | ||
303 | 2615 | # | ||
304 | 2616 | # response = yield call_responder( | ||
305 | 2617 | # Region(), ReportForeignDHCPServer, | ||
306 | 2618 | # { | ||
307 | 2619 | # 'cluster_uuid': cluster.uuid, | ||
308 | 2620 | # 'interface_name': cluster_interface.name, | ||
309 | 2621 | # 'foreign_dhcp_ip': foreign_dhcp_ip, | ||
310 | 2622 | # }) | ||
311 | 2623 | # | ||
312 | 2624 | # self.assertEqual({}, response) | ||
313 | 2625 | # cluster_interface = yield deferToDatabase( | ||
314 | 2626 | # transactional_reload_object, cluster_interface) | ||
315 | 2627 | # | ||
316 | 2628 | # self.assertEqual( | ||
317 | 2629 | # foreign_dhcp_ip, cluster_interface.foreign_dhcp_ip) | ||
318 | 2630 | # | ||
319 | 2631 | # @wait_for_reactor | ||
320 | 2632 | # @inlineCallbacks | ||
321 | 2633 | # def test_does_not_trigger_update_signal(self): | ||
322 | 2634 | # configure_dhcp = self.patch_autospec(dhcp, "configure_dhcp") | ||
323 | 2635 | # | ||
324 | 2636 | # foreign_dhcp_ip = factory.make_ipv4_address() | ||
325 | 2637 | # cluster_interface = yield deferToDatabase( | ||
326 | 2638 | # self.create_cluster_interface) | ||
327 | 2639 | # cluster = cluster_interface.nodegroup | ||
328 | 2640 | # | ||
329 | 2641 | # response = yield call_responder( | ||
330 | 2642 | # Region(), ReportForeignDHCPServer, | ||
331 | 2643 | # { | ||
332 | 2644 | # 'cluster_uuid': cluster.uuid, | ||
333 | 2645 | # 'interface_name': cluster_interface.name, | ||
334 | 2646 | # 'foreign_dhcp_ip': foreign_dhcp_ip, | ||
335 | 2647 | # }) | ||
336 | 2648 | # | ||
337 | 2649 | # self.assertEqual({}, response) | ||
338 | 2650 | # self.assertThat(configure_dhcp, MockNotCalled()) | ||
339 | 2651 | |||
340 | 2652 | |||
341 | 2653 | class TestRegionProtocol_GetClusterInterfaces(MAASTransactionServerTestCase): | ||
342 | 2654 | |||
343 | 2655 | def test_create_node_is_registered(self): | ||
349 | 2656 | protocol = Region() | 2598 | protocol = Region() |
350 | 2657 | responder = protocol.locateResponder( | 2599 | responder = protocol.locateResponder( |
352 | 2658 | GetClusterInterfaces.commandName) | 2600 | ReportForeignDHCPServer.commandName) |
353 | 2659 | self.assertIsNotNone(responder) | 2601 | self.assertIsNotNone(responder) |
354 | 2660 | 2602 | ||
355 | 2661 | @transactional | 2603 | @transactional |
361 | 2662 | def create_controller_and_interfaces(self): | 2604 | def create_rack_interface(self): |
362 | 2663 | controller = factory.make_RackController() | 2605 | rack_controller = factory.make_RackController(interface=False) |
363 | 2664 | for _ in range(3): | 2606 | interface = factory.make_Interface( |
364 | 2665 | factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=controller) | 2607 | INTERFACE_TYPE.PHYSICAL, node=rack_controller) |
365 | 2666 | interfaces = [ | 2608 | return rack_controller, interface |
366 | 2609 | |||
367 | 2610 | @transactional | ||
368 | 2611 | def get_vlan_for_interface(self, interface): | ||
369 | 2612 | return reload_object(interface.vlan) | ||
370 | 2613 | |||
371 | 2614 | @wait_for_reactor | ||
372 | 2615 | @inlineCallbacks | ||
373 | 2616 | def test_sets_external_dhcp_value(self): | ||
374 | 2617 | dhcp_ip = factory.make_ipv4_address() | ||
375 | 2618 | rack, interface = yield deferToDatabase( | ||
376 | 2619 | self.create_rack_interface) | ||
377 | 2620 | |||
378 | 2621 | response = yield call_responder( | ||
379 | 2622 | Region(), ReportForeignDHCPServer, | ||
380 | 2667 | { | 2623 | { |
401 | 2668 | 'name': interface.name, | 2624 | 'system_id': rack.system_id, |
402 | 2669 | 'interface': interface.name, | 2625 | 'interface_name': interface.name, |
403 | 2670 | 'ip': '', | 2626 | 'dhcp_ip': dhcp_ip, |
404 | 2671 | } | 2627 | }) |
405 | 2672 | for interface in controller.interface_set.all()] | 2628 | |
406 | 2673 | return controller, interfaces | 2629 | self.assertEqual({}, response) |
407 | 2674 | 2630 | vlan = yield deferToDatabase( | |
408 | 2675 | @wait_for_reactor | 2631 | self.get_vlan_for_interface, interface) |
409 | 2676 | @inlineCallbacks | 2632 | self.assertEqual( |
410 | 2677 | def test_returns_all_cluster_interfaces(self): | 2633 | dhcp_ip, vlan.external_dhcp) |
391 | 2678 | controller, expected_interfaces = yield deferToDatabase( | ||
392 | 2679 | self.create_controller_and_interfaces) | ||
393 | 2680 | |||
394 | 2681 | response = yield call_responder( | ||
395 | 2682 | Region(), GetClusterInterfaces, | ||
396 | 2683 | {'cluster_uuid': controller.system_id}) | ||
397 | 2684 | |||
398 | 2685 | self.assertIsNot(None, response) | ||
399 | 2686 | self.assertItemsEqual( | ||
400 | 2687 | expected_interfaces, response["interfaces"]) | ||
411 | 2688 | 2634 | ||
412 | 2689 | 2635 | ||
413 | 2690 | class TestRegionProtocol_CreateNode(MAASTransactionServerTestCase): | 2636 | class TestRegionProtocol_CreateNode(MAASTransactionServerTestCase): |
414 | 2691 | 2637 | ||
415 | === modified file 'src/provisioningserver/dhcp/detect.py' | |||
416 | --- src/provisioningserver/dhcp/detect.py 2016-03-28 13:54:47 +0000 | |||
417 | +++ src/provisioningserver/dhcp/detect.py 2016-04-07 13:22:34 +0000 | |||
418 | @@ -1,3 +1,4 @@ | |||
419 | 1 | |||
420 | 1 | # Copyright 2013-2016 Canonical Ltd. This software is licensed under the | 2 | # Copyright 2013-2016 Canonical Ltd. This software is licensed under the |
421 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 3 | # GNU Affero General Public License version 3 (see the file LICENSE). |
422 | 3 | 4 | ||
423 | @@ -10,21 +11,10 @@ | |||
424 | 10 | from contextlib import contextmanager | 11 | from contextlib import contextmanager |
425 | 11 | import errno | 12 | import errno |
426 | 12 | import fcntl | 13 | import fcntl |
427 | 13 | import http.client | ||
428 | 14 | import json | ||
429 | 15 | from random import randint | 14 | from random import randint |
430 | 16 | import socket | 15 | import socket |
431 | 17 | import struct | 16 | import struct |
432 | 18 | from urllib.error import ( | ||
433 | 19 | HTTPError, | ||
434 | 20 | URLError, | ||
435 | 21 | ) | ||
436 | 22 | 17 | ||
437 | 23 | from apiclient.maas_client import ( | ||
438 | 24 | MAASClient, | ||
439 | 25 | MAASDispatcher, | ||
440 | 26 | MAASOAuth, | ||
441 | 27 | ) | ||
442 | 28 | from provisioningserver.logger import get_maas_logger | 18 | from provisioningserver.logger import get_maas_logger |
443 | 29 | 19 | ||
444 | 30 | 20 | ||
445 | @@ -204,64 +194,10 @@ | |||
446 | 204 | return receive_offers(transaction_id) | 194 | return receive_offers(transaction_id) |
447 | 205 | 195 | ||
448 | 206 | 196 | ||
503 | 207 | def process_request(client_func, *args, **kwargs): | 197 | def probe_interface(interface): |
450 | 208 | """Run a MAASClient query and check for common errors. | ||
451 | 209 | |||
452 | 210 | :return: None if there is an error, otherwise the decoded response body. | ||
453 | 211 | """ | ||
454 | 212 | try: | ||
455 | 213 | response = client_func(*args, **kwargs) | ||
456 | 214 | except (HTTPError, URLError) as e: | ||
457 | 215 | maaslog.warning("Failed to contact region controller:\n%s", e) | ||
458 | 216 | return None | ||
459 | 217 | code = response.getcode() | ||
460 | 218 | if code != http.client.OK: | ||
461 | 219 | maaslog.error( | ||
462 | 220 | "Failed talking to region controller, it returned:\n%s\n%s", | ||
463 | 221 | code, response.read()) | ||
464 | 222 | return None | ||
465 | 223 | try: | ||
466 | 224 | raw_data = response.read() | ||
467 | 225 | if len(raw_data) > 0: | ||
468 | 226 | data = json.loads(raw_data) | ||
469 | 227 | else: | ||
470 | 228 | return None | ||
471 | 229 | except ValueError as e: | ||
472 | 230 | maaslog.error( | ||
473 | 231 | "Failed to decode response from region controller:\n%s", e) | ||
474 | 232 | return None | ||
475 | 233 | return data | ||
476 | 234 | |||
477 | 235 | |||
478 | 236 | def determine_cluster_interfaces(knowledge): | ||
479 | 237 | """Given server knowledge, determine network interfaces on this cluster. | ||
480 | 238 | |||
481 | 239 | :return: a list of tuples of (interface name, ip) for all interfaces. | ||
482 | 240 | |||
483 | 241 | :note: this uses an API call and not local probing because the | ||
484 | 242 | region controller has the definitive and final say in what does and | ||
485 | 243 | doesn't exist. | ||
486 | 244 | """ | ||
487 | 245 | api_path = ( | ||
488 | 246 | 'api/2.0/nodegroups/%s/interfaces/' % knowledge['nodegroup_uuid']) | ||
489 | 247 | oauth = MAASOAuth(*knowledge['api_credentials']) | ||
490 | 248 | client = MAASClient(oauth, MAASDispatcher(), knowledge['maas_url']) | ||
491 | 249 | interfaces = process_request(client.get, api_path, 'list') | ||
492 | 250 | if interfaces is None: | ||
493 | 251 | return None | ||
494 | 252 | |||
495 | 253 | interface_names = sorted( | ||
496 | 254 | (interface['interface'], interface['ip']) | ||
497 | 255 | for interface in interfaces | ||
498 | 256 | if interface['interface'] != '') | ||
499 | 257 | return interface_names | ||
500 | 258 | |||
501 | 259 | |||
502 | 260 | def probe_interface(interface, ip): | ||
504 | 261 | """Probe the given interface for DHCP servers. | 198 | """Probe the given interface for DHCP servers. |
505 | 262 | 199 | ||
508 | 263 | :param interface: interface as returned from determine_cluster_interfaces | 200 | :param interface: interface name |
507 | 264 | :param ip: ip as returned from determine_cluster_interfaces | ||
509 | 265 | :return: A set of IP addresses of detected servers. | 201 | :return: A set of IP addresses of detected servers. |
510 | 266 | 202 | ||
511 | 267 | :note: Any servers running on the IP address of the local host are | 203 | :note: Any servers running on the IP address of the local host are |
512 | @@ -287,63 +223,4 @@ | |||
513 | 287 | "your cluster interfaces configuration.", interface) | 223 | "your cluster interfaces configuration.", interface) |
514 | 288 | else: | 224 | else: |
515 | 289 | raise | 225 | raise |
576 | 290 | # Using servers.discard(ip) here breaks Mock in the tests, so | 226 | return servers |
517 | 291 | # we're creating a copy of the set instead. | ||
518 | 292 | results = servers.difference([ip]) | ||
519 | 293 | return results | ||
520 | 294 | |||
521 | 295 | |||
522 | 296 | def update_region_controller(knowledge, interface, server): | ||
523 | 297 | """Update the region controller with the status of the probe. | ||
524 | 298 | |||
525 | 299 | :param knowledge: dictionary of server info | ||
526 | 300 | :param interface: name of interface, e.g. eth0 | ||
527 | 301 | :param server: IP address of detected DHCP server, or None | ||
528 | 302 | """ | ||
529 | 303 | api_path = 'api/2.0/nodegroups/%s/interfaces/%s/' % ( | ||
530 | 304 | knowledge['nodegroup_uuid'], interface) | ||
531 | 305 | oauth = MAASOAuth(*knowledge['api_credentials']) | ||
532 | 306 | client = MAASClient(oauth, MAASDispatcher(), knowledge['maas_url']) | ||
533 | 307 | if server is None: | ||
534 | 308 | server = '' | ||
535 | 309 | process_request( | ||
536 | 310 | client.post, api_path, 'report_foreign_dhcp', foreign_dhcp_ip=server) | ||
537 | 311 | |||
538 | 312 | |||
539 | 313 | def periodic_probe_task(api_knowledge): | ||
540 | 314 | """Probe for DHCP servers and set NodeGroupInterface.foriegn_dhcp. | ||
541 | 315 | |||
542 | 316 | This should be run periodically so that the database has an up-to-date | ||
543 | 317 | view of any rogue DHCP servers on the network. | ||
544 | 318 | |||
545 | 319 | NOTE: This uses blocking I/O with sequential polling of interfaces, and | ||
546 | 320 | hence doesn't scale well. It's a future improvement to make | ||
547 | 321 | to throw it in parallel threads or async I/O. | ||
548 | 322 | |||
549 | 323 | :param api_knowledge: A dict of the information needed to be able to | ||
550 | 324 | make requests to the region's REST API. | ||
551 | 325 | """ | ||
552 | 326 | # Determine all the active interfaces on this cluster (nodegroup). | ||
553 | 327 | interfaces = determine_cluster_interfaces(api_knowledge) | ||
554 | 328 | if interfaces is None: | ||
555 | 329 | maaslog.info("No interfaces on cluster, not probing DHCP.") | ||
556 | 330 | return | ||
557 | 331 | |||
558 | 332 | # Iterate over interfaces and probe each one. | ||
559 | 333 | for interface, ip in interfaces: | ||
560 | 334 | try: | ||
561 | 335 | servers = probe_interface(interface, ip) | ||
562 | 336 | except socket.error: | ||
563 | 337 | maaslog.error( | ||
564 | 338 | "Failed to probe sockets; did you configure authbind as per " | ||
565 | 339 | "HACKING.txt?") | ||
566 | 340 | return | ||
567 | 341 | else: | ||
568 | 342 | if len(servers) > 0: | ||
569 | 343 | # Only send one, if it gets cleared out then the | ||
570 | 344 | # next detection pass will send a different one, if it | ||
571 | 345 | # still exists. | ||
572 | 346 | update_region_controller( | ||
573 | 347 | api_knowledge, interface, servers.pop()) | ||
574 | 348 | else: | ||
575 | 349 | update_region_controller(api_knowledge, interface, None) | ||
577 | 350 | 227 | ||
578 | === removed file 'src/provisioningserver/dhcp/probe.py' | |||
579 | --- src/provisioningserver/dhcp/probe.py 2015-12-01 18:12:59 +0000 | |||
580 | +++ src/provisioningserver/dhcp/probe.py 1970-01-01 00:00:00 +0000 | |||
581 | @@ -1,41 +0,0 @@ | |||
582 | 1 | #!/usr/bin/env python2.7 | ||
583 | 2 | # Copyright 2013-2015 Canonical Ltd. This software is licensed under the | ||
584 | 3 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
585 | 4 | |||
586 | 5 | """Probe network on given network interface for a DHCP server. | ||
587 | 6 | |||
588 | 7 | This needs to be run as root, in order to be allowed to broadcast on the | ||
589 | 8 | BOOTP port. | ||
590 | 9 | |||
591 | 10 | Exit code is zero ("success") if no servers were detected, or the number of | ||
592 | 11 | DHCP servers that were found. | ||
593 | 12 | """ | ||
594 | 13 | |||
595 | 14 | import argparse | ||
596 | 15 | from sys import exit | ||
597 | 16 | |||
598 | 17 | from provisioningserver.dhcp.detect import probe_dhcp | ||
599 | 18 | |||
600 | 19 | |||
601 | 20 | argument_parser = argparse.ArgumentParser(description=__doc__) | ||
602 | 21 | |||
603 | 22 | |||
604 | 23 | def main(): | ||
605 | 24 | argument_parser.add_argument( | ||
606 | 25 | 'interface', | ||
607 | 26 | help="Probe network on this network interface.") | ||
608 | 27 | |||
609 | 28 | args = argument_parser.parse_args() | ||
610 | 29 | |||
611 | 30 | servers = probe_dhcp(args.interface) | ||
612 | 31 | |||
613 | 32 | num_servers = len(servers) | ||
614 | 33 | if num_servers == 0: | ||
615 | 34 | print("No DHCP servers detected.") | ||
616 | 35 | exit(0) | ||
617 | 36 | else: | ||
618 | 37 | print("DHCP servers detected: %s" % ', '.join(sorted(servers))) | ||
619 | 38 | exit(num_servers) | ||
620 | 39 | |||
621 | 40 | if __name__ == "__main__": | ||
622 | 41 | main() | ||
623 | 42 | 0 | ||
624 | === modified file 'src/provisioningserver/dhcp/tests/test_detect.py' | |||
625 | --- src/provisioningserver/dhcp/tests/test_detect.py 2016-03-28 13:54:47 +0000 | |||
626 | +++ src/provisioningserver/dhcp/tests/test_detect.py 2016-04-07 13:22:34 +0000 | |||
627 | @@ -7,39 +7,25 @@ | |||
628 | 7 | 7 | ||
629 | 8 | import errno | 8 | import errno |
630 | 9 | import fcntl | 9 | import fcntl |
631 | 10 | import http.client | ||
632 | 11 | import socket | 10 | import socket |
633 | 12 | import textwrap | ||
634 | 13 | import urllib.error | ||
635 | 14 | import urllib.parse | ||
636 | 15 | import urllib.request | ||
637 | 16 | 11 | ||
638 | 17 | from apiclient.maas_client import MAASClient | ||
639 | 18 | from apiclient.testing.credentials import make_api_credentials | ||
640 | 19 | from fixtures import FakeLogger | ||
641 | 20 | from maastesting.factory import factory | 12 | from maastesting.factory import factory |
642 | 21 | from maastesting.matchers import MockCalledOnceWith | ||
643 | 22 | from maastesting.testcase import MAASTestCase | 13 | from maastesting.testcase import MAASTestCase |
644 | 23 | import mock | 14 | import mock |
645 | 24 | from mock import sentinel | ||
646 | 25 | from provisioningserver.dhcp.detect import ( | 15 | from provisioningserver.dhcp.detect import ( |
647 | 26 | BOOTP_CLIENT_PORT, | 16 | BOOTP_CLIENT_PORT, |
648 | 27 | BOOTP_SERVER_PORT, | 17 | BOOTP_SERVER_PORT, |
649 | 28 | determine_cluster_interfaces, | ||
650 | 29 | DHCPDiscoverPacket, | 18 | DHCPDiscoverPacket, |
651 | 30 | DHCPOfferPacket, | 19 | DHCPOfferPacket, |
652 | 31 | get_interface_IP, | 20 | get_interface_IP, |
653 | 32 | get_interface_MAC, | 21 | get_interface_MAC, |
654 | 33 | make_transaction_ID, | 22 | make_transaction_ID, |
655 | 34 | periodic_probe_task, | ||
656 | 35 | probe_interface, | 23 | probe_interface, |
657 | 36 | receive_offers, | 24 | receive_offers, |
658 | 37 | request_dhcp, | 25 | request_dhcp, |
659 | 38 | udp_socket, | 26 | udp_socket, |
660 | 39 | update_region_controller, | ||
661 | 40 | ) | 27 | ) |
662 | 41 | import provisioningserver.dhcp.detect as detect_module | 28 | import provisioningserver.dhcp.detect as detect_module |
663 | 42 | from provisioningserver.testing.config import ClusterConfigurationFixture | ||
664 | 43 | from provisioningserver.testing.testcase import PservTestCase | 29 | from provisioningserver.testing.testcase import PservTestCase |
665 | 44 | 30 | ||
666 | 45 | 31 | ||
667 | @@ -306,230 +292,35 @@ | |||
668 | 306 | receive_offers, factory.make_bytes(4)) | 292 | receive_offers, factory.make_bytes(4)) |
669 | 307 | 293 | ||
670 | 308 | 294 | ||
671 | 309 | class MockResponse: | ||
672 | 310 | # This implements just enough to look lke a urllib2 response object. | ||
673 | 311 | def __init__(self, code=None, response=None): | ||
674 | 312 | if code is None: | ||
675 | 313 | code = http.client.OK | ||
676 | 314 | self.code = code | ||
677 | 315 | if response is None: | ||
678 | 316 | response = "" | ||
679 | 317 | self.response = response | ||
680 | 318 | |||
681 | 319 | def getcode(self): | ||
682 | 320 | return self.code | ||
683 | 321 | |||
684 | 322 | def read(self): | ||
685 | 323 | return self.response | ||
686 | 324 | |||
687 | 325 | |||
688 | 326 | class TestPeriodicTask(PservTestCase): | 295 | class TestPeriodicTask(PservTestCase): |
689 | 327 | 296 | ||
690 | 328 | def setUp(self): | ||
691 | 329 | # Initialise the knowledge cache. | ||
692 | 330 | super(TestPeriodicTask, self).setUp() | ||
693 | 331 | self.maaslog = self.useFixture(FakeLogger("maas.dhcp.detect")) | ||
694 | 332 | uuid = factory.make_UUID() | ||
695 | 333 | maas_url = 'http://%s.example.com/%s/' % ( | ||
696 | 334 | factory.make_name('host'), | ||
697 | 335 | factory.make_string(), | ||
698 | 336 | ) | ||
699 | 337 | api_credentials = make_api_credentials() | ||
700 | 338 | self.useFixture(ClusterConfigurationFixture(maas_url=maas_url)) | ||
701 | 339 | self.knowledge = dict( | ||
702 | 340 | nodegroup_uuid=uuid, | ||
703 | 341 | api_credentials=api_credentials, | ||
704 | 342 | maas_url=maas_url) | ||
705 | 343 | |||
706 | 344 | def make_fake_interfaces_response(self, interfaces_pairs): | ||
707 | 345 | stanzas = [] | ||
708 | 346 | for interfaces_pair in interfaces_pairs: | ||
709 | 347 | stanza = textwrap.dedent(""" | ||
710 | 348 | {{ | ||
711 | 349 | "ip_range_high": null, | ||
712 | 350 | "ip_range_low": null, | ||
713 | 351 | "broadcast_ip": null, | ||
714 | 352 | "ip": "{1}", | ||
715 | 353 | "subnet_mask": "255.255.255.0", | ||
716 | 354 | "management": 0, | ||
717 | 355 | "interface": "{0}" | ||
718 | 356 | }}""").format(*interfaces_pair) | ||
719 | 357 | stanzas.append(stanza) | ||
720 | 358 | interfaces_json = "[" | ||
721 | 359 | interfaces_json += ",".join(stanzas) | ||
722 | 360 | interfaces_json += "]" | ||
723 | 361 | return interfaces_json | ||
724 | 362 | |||
725 | 363 | def patch_fake_interfaces_list(self, interfaces_pairs): | ||
726 | 364 | # Set up the api client to return a fake set of interfaces. | ||
727 | 365 | # Determine_cluster_interfaces calls the API to discover what | ||
728 | 366 | # interfaces are available, so any test code that calls it | ||
729 | 367 | # should first call this helper to set up the required fake response. | ||
730 | 368 | interfaces_json = self.make_fake_interfaces_response(interfaces_pairs) | ||
731 | 369 | self.patch(MAASClient, 'get').return_value = MockResponse( | ||
732 | 370 | http.client.OK, interfaces_json) | ||
733 | 371 | |||
734 | 372 | def test_determine_cluster_interfaces_returns_interface_names(self): | ||
735 | 373 | eth0_addr = factory.make_ipv4_address() | ||
736 | 374 | wlan0_addr = factory.make_ipv4_address() | ||
737 | 375 | self.patch_fake_interfaces_list( | ||
738 | 376 | [("eth0", eth0_addr), ("wlan0", wlan0_addr)]) | ||
739 | 377 | self.assertEqual( | ||
740 | 378 | [("eth0", eth0_addr), ("wlan0", wlan0_addr)], | ||
741 | 379 | determine_cluster_interfaces(self.knowledge)) | ||
742 | 380 | |||
743 | 381 | def test_probe_interface_returns_empty_set_when_nothing_detected(self): | 297 | def test_probe_interface_returns_empty_set_when_nothing_detected(self): |
744 | 382 | eth0_addr = factory.make_ipv4_address() | ||
745 | 383 | self.patch_fake_interfaces_list([("eth0", eth0_addr)]) | ||
746 | 384 | self.patch(detect_module, 'probe_dhcp').return_value = set() | 298 | self.patch(detect_module, 'probe_dhcp').return_value = set() |
749 | 385 | interfaces = determine_cluster_interfaces(self.knowledge) | 299 | results = probe_interface("eth0") |
748 | 386 | results = probe_interface(*interfaces[0]) | ||
750 | 387 | self.assertEqual(set(), results) | 300 | self.assertEqual(set(), results) |
751 | 388 | 301 | ||
752 | 389 | def test_probe_interface_returns_empty_set_when_IP_missing(self): | 302 | def test_probe_interface_returns_empty_set_when_IP_missing(self): |
753 | 390 | # If the interface being probed has no IP address, the | 303 | # If the interface being probed has no IP address, the |
754 | 391 | # request_dhcr() method will raise IOError with errno 99. Make | 304 | # request_dhcr() method will raise IOError with errno 99. Make |
755 | 392 | # sure this is caught and ignored. | 305 | # sure this is caught and ignored. |
756 | 393 | eth0_addr = factory.make_ipv4_address() | ||
757 | 394 | self.patch_fake_interfaces_list([("eth0", eth0_addr)]) | ||
758 | 395 | ioerror = IOError( | 306 | ioerror = IOError( |
759 | 396 | errno.EADDRNOTAVAIL, "Cannot assign requested address") | 307 | errno.EADDRNOTAVAIL, "Cannot assign requested address") |
760 | 397 | self.patch(fcntl, 'ioctl').side_effect = ioerror | 308 | self.patch(fcntl, 'ioctl').side_effect = ioerror |
763 | 398 | interfaces = determine_cluster_interfaces(self.knowledge) | 309 | results = probe_interface("eth0") |
762 | 399 | results = probe_interface(*interfaces[0]) | ||
764 | 400 | self.assertEqual(set(), results) | 310 | self.assertEqual(set(), results) |
765 | 401 | 311 | ||
766 | 402 | def test_probe_interface_returns_empty_set_when_device_missing(self): | 312 | def test_probe_interface_returns_empty_set_when_device_missing(self): |
767 | 403 | # If the interface being probed does not exist, the | 313 | # If the interface being probed does not exist, the |
768 | 404 | # request_dhcp() method will raise IOError with errno 19. Make | 314 | # request_dhcp() method will raise IOError with errno 19. Make |
769 | 405 | # sure this is caught and ignored. | 315 | # sure this is caught and ignored. |
770 | 406 | eth0_addr = factory.make_ipv4_address() | ||
771 | 407 | self.patch_fake_interfaces_list([("eth0", eth0_addr)]) | ||
772 | 408 | ioerror = IOError(errno.ENODEV, "No such device") | 316 | ioerror = IOError(errno.ENODEV, "No such device") |
773 | 409 | self.patch(fcntl, 'ioctl').side_effect = ioerror | 317 | self.patch(fcntl, 'ioctl').side_effect = ioerror |
776 | 410 | interfaces = determine_cluster_interfaces(self.knowledge) | 318 | results = probe_interface("eth0") |
775 | 411 | results = probe_interface(*interfaces[0]) | ||
777 | 412 | self.assertEqual(set(), results) | 319 | self.assertEqual(set(), results) |
778 | 413 | 320 | ||
779 | 414 | def test_probe_interface_returns_populated_set(self): | 321 | def test_probe_interface_returns_populated_set(self): |
780 | 415 | # Test that the detected DHCP server is returned. | 322 | # Test that the detected DHCP server is returned. |
781 | 416 | eth0_addr = factory.make_ipv4_address() | ||
782 | 417 | self.patch_fake_interfaces_list([("eth0", eth0_addr)]) | ||
783 | 418 | self.patch( | 323 | self.patch( |
784 | 419 | detect_module, 'probe_dhcp').return_value = {'10.2.2.2'} | 324 | detect_module, 'probe_dhcp').return_value = {'10.2.2.2'} |
787 | 420 | interfaces = determine_cluster_interfaces(self.knowledge) | 325 | results = probe_interface("eth0") |
786 | 421 | results = probe_interface(*interfaces[0]) | ||
788 | 422 | self.assertEqual({'10.2.2.2'}, results) | 326 | self.assertEqual({'10.2.2.2'}, results) |
789 | 423 | |||
790 | 424 | def test_probe_interface_filters_interface_own_ip(self): | ||
791 | 425 | # Test that the interface shows the detected DHCP server except | ||
792 | 426 | # if it is the same IP as the interface's. | ||
793 | 427 | eth0_addr = factory.make_ipv4_address() | ||
794 | 428 | self.patch_fake_interfaces_list([("eth0", eth0_addr)]) | ||
795 | 429 | detected_dhcp = eth0_addr | ||
796 | 430 | self.patch(detect_module, 'probe_dhcp').return_value = {detected_dhcp} | ||
797 | 431 | interfaces = determine_cluster_interfaces(self.knowledge) | ||
798 | 432 | results = probe_interface(*interfaces[0]) | ||
799 | 433 | self.assertEqual(set(), results) | ||
800 | 434 | |||
801 | 435 | def test_determine_cluster_interfaces_catchs_HTTPError_in_MAASClient(self): | ||
802 | 436 | self.patch(MAASClient, 'get').side_effect = urllib.error.HTTPError( | ||
803 | 437 | sentinel.url, sentinel.code, sentinel.msg, sentinel.hdrs, None) | ||
804 | 438 | determine_cluster_interfaces(self.knowledge) | ||
805 | 439 | self.assertIn( | ||
806 | 440 | "Failed to contact region controller:", self.maaslog.output) | ||
807 | 441 | |||
808 | 442 | def test_determine_cluster_interfaces_catches_URLError_in_MAASClient(self): | ||
809 | 443 | self.patch(MAASClient, 'get').side_effect = urllib.error.URLError( | ||
810 | 444 | sentinel.arg1) | ||
811 | 445 | determine_cluster_interfaces(self.knowledge) | ||
812 | 446 | self.assertIn( | ||
813 | 447 | "Failed to contact region controller:", self.maaslog.output) | ||
814 | 448 | |||
815 | 449 | def test_determine_cluster_interfaces_catches_non_OK_response(self): | ||
816 | 450 | self.patch(MAASClient, 'get').return_value = MockResponse( | ||
817 | 451 | http.client.NOT_FOUND, "error text") | ||
818 | 452 | determine_cluster_interfaces(self.knowledge) | ||
819 | 453 | self.assertIn( | ||
820 | 454 | "Failed talking to region controller, it returned:", | ||
821 | 455 | self.maaslog.output) | ||
822 | 456 | |||
823 | 457 | def test_update_region_controller_sets_detected_dhcp(self): | ||
824 | 458 | mocked_post = self.patch(MAASClient, 'post') | ||
825 | 459 | mocked_post.return_value = MockResponse() | ||
826 | 460 | detected_server = factory.make_ipv4_address() | ||
827 | 461 | update_region_controller(self.knowledge, "eth0", detected_server) | ||
828 | 462 | uuid = self.knowledge['nodegroup_uuid'] | ||
829 | 463 | self.assertThat(mocked_post, MockCalledOnceWith( | ||
830 | 464 | 'api/2.0/nodegroups/%s/interfaces/eth0/' % uuid, | ||
831 | 465 | 'report_foreign_dhcp', foreign_dhcp_ip=detected_server)) | ||
832 | 466 | |||
833 | 467 | def test_update_region_controller_clears_detected_dhcp(self): | ||
834 | 468 | mocked_post = self.patch(MAASClient, 'post') | ||
835 | 469 | mocked_post.return_value = MockResponse() | ||
836 | 470 | detected_server = None | ||
837 | 471 | update_region_controller(self.knowledge, "eth0", detected_server) | ||
838 | 472 | uuid = self.knowledge['nodegroup_uuid'] | ||
839 | 473 | self.assertThat(mocked_post, MockCalledOnceWith( | ||
840 | 474 | 'api/2.0/nodegroups/%s/interfaces/eth0/' % uuid, | ||
841 | 475 | 'report_foreign_dhcp', foreign_dhcp_ip='')) | ||
842 | 476 | |||
843 | 477 | def test_update_region_controller_catches_HTTPError_in_MAASClient(self): | ||
844 | 478 | self.patch(MAASClient, 'post').side_effect = urllib.error.HTTPError( | ||
845 | 479 | sentinel.url, sentinel.code, sentinel.msg, sentinel.hdrs, None) | ||
846 | 480 | update_region_controller(self.knowledge, "eth0", None) | ||
847 | 481 | self.assertIn( | ||
848 | 482 | "Failed to contact region controller:", self.maaslog.output) | ||
849 | 483 | |||
850 | 484 | def test_update_region_controller_catches_URLError_in_MAASClient(self): | ||
851 | 485 | self.patch(MAASClient, 'post').side_effect = urllib.error.URLError( | ||
852 | 486 | sentinel.arg1) | ||
853 | 487 | update_region_controller(self.knowledge, "eth0", None) | ||
854 | 488 | self.assertIn( | ||
855 | 489 | "Failed to contact region controller:", self.maaslog.output) | ||
856 | 490 | |||
857 | 491 | def test_update_region_controller_catches_non_OK_response(self): | ||
858 | 492 | mock_response = MockResponse(http.client.NOT_FOUND, "error text") | ||
859 | 493 | self.patch(MAASClient, 'post').return_value = mock_response | ||
860 | 494 | update_region_controller(self.knowledge, "eth0", None) | ||
861 | 495 | self.assertIn( | ||
862 | 496 | "Failed talking to region controller, it returned:", | ||
863 | 497 | self.maaslog.output) | ||
864 | 498 | |||
865 | 499 | def test_periodic_probe_task_exits_if_no_interfaces(self): | ||
866 | 500 | mocked = self.patch(detect_module, 'probe_interface') | ||
867 | 501 | self.patch( | ||
868 | 502 | detect_module, 'determine_cluster_interfaces').return_value = None | ||
869 | 503 | periodic_probe_task(self.knowledge) | ||
870 | 504 | self.assertFalse(mocked.called) | ||
871 | 505 | |||
872 | 506 | def test_periodic_probe_task_updates_region_with_detected_server(self): | ||
873 | 507 | eth0_addr = factory.make_ipv4_address() | ||
874 | 508 | wlan0_addr = factory.make_ipv4_address() | ||
875 | 509 | detected_server = factory.make_ipv4_address() | ||
876 | 510 | self.patch_fake_interfaces_list( | ||
877 | 511 | [("eth0", eth0_addr), ("wlan0", wlan0_addr)]) | ||
878 | 512 | self.patch( | ||
879 | 513 | detect_module, 'probe_dhcp').return_value = {detected_server} | ||
880 | 514 | mocked_update = self.patch(detect_module, 'update_region_controller') | ||
881 | 515 | periodic_probe_task(self.knowledge) | ||
882 | 516 | calls = [ | ||
883 | 517 | mock.call(self.knowledge, 'eth0', detected_server), | ||
884 | 518 | mock.call(self.knowledge, 'wlan0', detected_server), | ||
885 | 519 | ] | ||
886 | 520 | mocked_update.assert_has_calls(calls, any_order=True) | ||
887 | 521 | |||
888 | 522 | def test_periodic_probe_task_updates_region_with_no_detected_server(self): | ||
889 | 523 | eth0_addr = factory.make_ipv4_address() | ||
890 | 524 | wlan0_addr = factory.make_ipv4_address() | ||
891 | 525 | self.patch_fake_interfaces_list( | ||
892 | 526 | [("eth0", eth0_addr), ("wlan0", wlan0_addr)]) | ||
893 | 527 | self.patch( | ||
894 | 528 | detect_module, 'probe_dhcp').return_value = set() | ||
895 | 529 | mocked_update = self.patch(detect_module, 'update_region_controller') | ||
896 | 530 | periodic_probe_task(self.knowledge) | ||
897 | 531 | calls = [ | ||
898 | 532 | mock.call(self.knowledge, 'eth0', None), | ||
899 | 533 | mock.call(self.knowledge, 'wlan0', None), | ||
900 | 534 | ] | ||
901 | 535 | mocked_update.assert_has_calls(calls, any_order=True) | ||
902 | 536 | 327 | ||
903 | === modified file 'src/provisioningserver/pserv_services/dhcp_probe_service.py' | |||
904 | --- src/provisioningserver/pserv_services/dhcp_probe_service.py 2016-03-28 13:54:47 +0000 | |||
905 | +++ src/provisioningserver/pserv_services/dhcp_probe_service.py 2016-04-07 13:22:34 +0000 | |||
906 | @@ -14,19 +14,14 @@ | |||
907 | 14 | from provisioningserver.dhcp.detect import probe_interface | 14 | from provisioningserver.dhcp.detect import probe_interface |
908 | 15 | from provisioningserver.logger.log import get_maas_logger | 15 | from provisioningserver.logger.log import get_maas_logger |
909 | 16 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable | 16 | from provisioningserver.rpc.exceptions import NoConnectionsAvailable |
914 | 17 | from provisioningserver.rpc.region import ( | 17 | from provisioningserver.rpc.region import ReportForeignDHCPServer |
915 | 18 | GetClusterInterfaces, | 18 | from provisioningserver.utils.network import get_all_interfaces_definition |
912 | 19 | ReportForeignDHCPServer, | ||
913 | 20 | ) | ||
916 | 21 | from provisioningserver.utils.twisted import ( | 19 | from provisioningserver.utils.twisted import ( |
917 | 22 | pause, | 20 | pause, |
918 | 23 | retries, | 21 | retries, |
919 | 24 | ) | 22 | ) |
920 | 25 | from twisted.application.internet import TimerService | 23 | from twisted.application.internet import TimerService |
925 | 26 | from twisted.internet.defer import ( | 24 | from twisted.internet.defer import inlineCallbacks |
922 | 27 | inlineCallbacks, | ||
923 | 28 | returnValue, | ||
924 | 29 | ) | ||
926 | 30 | from twisted.internet.threads import deferToThread | 25 | from twisted.internet.threads import deferToThread |
927 | 31 | from twisted.protocols.amp import UnhandledCommand | 26 | from twisted.protocols.amp import UnhandledCommand |
928 | 32 | 27 | ||
929 | @@ -35,12 +30,11 @@ | |||
930 | 35 | 30 | ||
931 | 36 | 31 | ||
932 | 37 | class DHCPProbeService(TimerService, object): | 32 | class DHCPProbeService(TimerService, object): |
934 | 38 | """Service to probe for DHCP servers on this cluster's network. | 33 | """Service to probe for DHCP servers on the rack controller interface's. |
935 | 39 | 34 | ||
936 | 40 | Built on top of Twisted's `TimerService`. | 35 | Built on top of Twisted's `TimerService`. |
937 | 41 | 36 | ||
938 | 42 | :param reactor: An `IReactor` instance. | 37 | :param reactor: An `IReactor` instance. |
939 | 43 | :param cluster_uuid: This cluster's UUID. | ||
940 | 44 | """ | 38 | """ |
941 | 45 | 39 | ||
942 | 46 | check_interval = timedelta(minutes=10).total_seconds() | 40 | check_interval = timedelta(minutes=10).total_seconds() |
943 | @@ -52,48 +46,44 @@ | |||
944 | 52 | self.clock = reactor | 46 | self.clock = reactor |
945 | 53 | self.client_service = client_service | 47 | self.client_service = client_service |
946 | 54 | 48 | ||
963 | 55 | @inlineCallbacks | 49 | def _get_interfaces(self): |
964 | 56 | def _get_cluster_interfaces(self, client): | 50 | """Return the interfaces for this rack controller.""" |
965 | 57 | """Return the interfaces for this cluster.""" | 51 | d = deferToThread(get_all_interfaces_definition) |
966 | 58 | try: | 52 | d.addCallback(lambda interfaces: [ |
967 | 59 | response = yield client( | 53 | name |
968 | 60 | GetClusterInterfaces, cluster_uuid=client.localIdent) | 54 | for name, info in interfaces.items() |
969 | 61 | except UnhandledCommand: | 55 | if info["enabled"] |
970 | 62 | # The region hasn't been upgraded to support this method | 56 | ]) |
971 | 63 | # yet, so give up. Returning an empty dict means that this | 57 | return d |
956 | 64 | # run will end, since there are no interfaces to check. | ||
957 | 65 | maaslog.error( | ||
958 | 66 | "Unable to query region for interfaces: Region does not " | ||
959 | 67 | "support the GetClusterInterfaces RPC method.") | ||
960 | 68 | returnValue({}) | ||
961 | 69 | else: | ||
962 | 70 | returnValue(response['interfaces']) | ||
972 | 71 | 58 | ||
977 | 72 | @inlineCallbacks | 59 | def _inform_region_of_dhcp(self, client, name, dhcp_ip): |
978 | 73 | def _inform_region_of_foreign_dhcp(self, client, name, | 60 | """Tell the region about the DHCP server. |
975 | 74 | foreign_dhcp_ip): | ||
976 | 75 | """Tell the region that there's a rogue DHCP server. | ||
979 | 76 | 61 | ||
980 | 77 | :param client: The RPC client to use. | 62 | :param client: The RPC client to use. |
981 | 78 | :param name: The name of the network interface where the rogue | 63 | :param name: The name of the network interface where the rogue |
982 | 79 | DHCP server was found. | 64 | DHCP server was found. |
984 | 80 | :param foreign_dhcp_ip: The IP address of the rogue server. | 65 | :param dhcp_ip: The IP address of the DHCP server. |
985 | 81 | """ | 66 | """ |
991 | 82 | try: | 67 | |
992 | 83 | yield client( | 68 | def eb_unhandled(failure): |
993 | 84 | ReportForeignDHCPServer, cluster_uuid=client.localIdent, | 69 | failure.trap(UnhandledCommand) |
989 | 85 | interface_name=name, foreign_dhcp_ip=foreign_dhcp_ip) | ||
990 | 86 | except UnhandledCommand: | ||
994 | 87 | # Not a lot we can do here... The region doesn't support | 70 | # Not a lot we can do here... The region doesn't support |
995 | 88 | # this method yet. | 71 | # this method yet. |
996 | 89 | maaslog.error( | 72 | maaslog.error( |
998 | 90 | "Unable to inform region of rogue DHCP server: the region " | 73 | "Unable to inform region of DHCP server: the region " |
999 | 91 | "does not yet support the ReportForeignDHCPServer RPC " | 74 | "does not yet support the ReportForeignDHCPServer RPC " |
1000 | 92 | "method.") | 75 | "method.") |
1001 | 93 | 76 | ||
1002 | 77 | d = client( | ||
1003 | 78 | ReportForeignDHCPServer, system_id=client.localIdent, | ||
1004 | 79 | interface_name=name, dhcp_ip=dhcp_ip) | ||
1005 | 80 | d.addErrback(eb_unhandled) | ||
1006 | 81 | return d | ||
1007 | 82 | |||
1008 | 94 | @inlineCallbacks | 83 | @inlineCallbacks |
1009 | 95 | def probe_dhcp(self): | 84 | def probe_dhcp(self): |
1011 | 96 | """Find all the interfaces on this cluster and probe for DHCP servers. | 85 | """Find all the interfaces on this rack controller and probe for |
1012 | 86 | DHCP servers. | ||
1013 | 97 | """ | 87 | """ |
1014 | 98 | client = None | 88 | client = None |
1015 | 99 | for elapsed, remaining, wait in retries(15, 5, self.clock): | 89 | for elapsed, remaining, wait in retries(15, 5, self.clock): |
1016 | @@ -107,12 +97,11 @@ | |||
1017 | 107 | "Can't initiate DHCP probe, no RPC connection to region.") | 97 | "Can't initiate DHCP probe, no RPC connection to region.") |
1018 | 108 | return | 98 | return |
1019 | 109 | 99 | ||
1020 | 110 | cluster_interfaces = yield self._get_cluster_interfaces(client) | ||
1021 | 111 | # Iterate over interfaces and probe each one. | 100 | # Iterate over interfaces and probe each one. |
1023 | 112 | for interface in cluster_interfaces: | 101 | interfaces = yield self._get_interfaces() |
1024 | 102 | for interface in interfaces: | ||
1025 | 113 | try: | 103 | try: |
1028 | 114 | servers = yield deferToThread( | 104 | servers = yield deferToThread(probe_interface, interface) |
1027 | 115 | probe_interface, interface['interface'], interface['ip']) | ||
1029 | 116 | except socket.error: | 105 | except socket.error: |
1030 | 117 | maaslog.error( | 106 | maaslog.error( |
1031 | 118 | "Failed to probe sockets; did you configure authbind as " | 107 | "Failed to probe sockets; did you configure authbind as " |
1032 | @@ -123,11 +112,11 @@ | |||
1033 | 123 | # Only send one, if it gets cleared out then the | 112 | # Only send one, if it gets cleared out then the |
1034 | 124 | # next detection pass will send a different one, if it | 113 | # next detection pass will send a different one, if it |
1035 | 125 | # still exists. | 114 | # still exists. |
1038 | 126 | yield self._inform_region_of_foreign_dhcp( | 115 | yield self._inform_region_of_dhcp( |
1039 | 127 | client, interface['name'], servers.pop()) | 116 | client, interface, servers.pop()) |
1040 | 128 | else: | 117 | else: |
1043 | 129 | yield self._inform_region_of_foreign_dhcp( | 118 | yield self._inform_region_of_dhcp( |
1044 | 130 | client, interface['name'], None) | 119 | client, interface, None) |
1045 | 131 | 120 | ||
1046 | 132 | @inlineCallbacks | 121 | @inlineCallbacks |
1047 | 133 | def try_probe_dhcp(self): | 122 | def try_probe_dhcp(self): |
1048 | @@ -136,7 +125,7 @@ | |||
1049 | 136 | yield self.probe_dhcp() | 125 | yield self.probe_dhcp() |
1050 | 137 | except Exception as error: | 126 | except Exception as error: |
1051 | 138 | maaslog.error( | 127 | maaslog.error( |
1053 | 139 | "Unable to probe for rogue DHCP servers: %s", | 128 | "Unable to probe for DHCP servers: %s", |
1054 | 140 | str(error)) | 129 | str(error)) |
1055 | 141 | else: | 130 | else: |
1056 | 142 | maaslog.debug("Finished periodic DHCP probe.") | 131 | maaslog.debug("Finished periodic DHCP probe.") |
1057 | 143 | 132 | ||
1058 | === modified file 'src/provisioningserver/pserv_services/tests/test_dhcp_probe_service.py' | |||
1059 | --- src/provisioningserver/pserv_services/tests/test_dhcp_probe_service.py 2016-03-28 13:54:47 +0000 | |||
1060 | +++ src/provisioningserver/pserv_services/tests/test_dhcp_probe_service.py 2016-04-07 13:22:34 +0000 | |||
1061 | @@ -11,10 +11,12 @@ | |||
1062 | 11 | get_mock_calls, | 11 | get_mock_calls, |
1063 | 12 | HasLength, | 12 | HasLength, |
1064 | 13 | MockCalledOnceWith, | 13 | MockCalledOnceWith, |
1065 | 14 | MockCallsMatch, | ||
1066 | 14 | MockNotCalled, | 15 | MockNotCalled, |
1067 | 15 | ) | 16 | ) |
1068 | 16 | from maastesting.testcase import MAASTwistedRunTest | 17 | from maastesting.testcase import MAASTwistedRunTest |
1069 | 17 | from mock import ( | 18 | from mock import ( |
1070 | 19 | call, | ||
1071 | 18 | Mock, | 20 | Mock, |
1072 | 19 | sentinel, | 21 | sentinel, |
1073 | 20 | ) | 22 | ) |
1074 | @@ -39,19 +41,9 @@ | |||
1075 | 39 | def patch_rpc_methods(self): | 41 | def patch_rpc_methods(self): |
1076 | 40 | fixture = self.useFixture(MockLiveClusterToRegionRPCFixture()) | 42 | fixture = self.useFixture(MockLiveClusterToRegionRPCFixture()) |
1077 | 41 | protocol, connecting = fixture.makeEventLoop( | 43 | protocol, connecting = fixture.makeEventLoop( |
1079 | 42 | region.GetClusterInterfaces, region.ReportForeignDHCPServer) | 44 | region.ReportForeignDHCPServer) |
1080 | 43 | return protocol, connecting | 45 | return protocol, connecting |
1081 | 44 | 46 | ||
1082 | 45 | def make_cluster_interface_values(self, ip=None): | ||
1083 | 46 | """Return a dict describing a cluster interface.""" | ||
1084 | 47 | if ip is None: | ||
1085 | 48 | ip = factory.make_ipv4_address() | ||
1086 | 49 | return { | ||
1087 | 50 | 'name': factory.make_name('interface'), | ||
1088 | 51 | 'interface': factory.make_name('eth'), | ||
1089 | 52 | 'ip': ip, | ||
1090 | 53 | } | ||
1091 | 54 | |||
1092 | 55 | def test_is_called_every_interval(self): | 47 | def test_is_called_every_interval(self): |
1093 | 56 | clock = Clock() | 48 | clock = Clock() |
1094 | 57 | service = DHCPProbeService( | 49 | service = DHCPProbeService( |
1095 | @@ -82,67 +74,47 @@ | |||
1096 | 82 | 74 | ||
1097 | 83 | def test_probe_is_initiated_in_new_thread(self): | 75 | def test_probe_is_initiated_in_new_thread(self): |
1098 | 84 | clock = Clock() | 76 | clock = Clock() |
1105 | 85 | interface = self.make_cluster_interface_values() | 77 | interface_name = factory.make_name("eth") |
1106 | 86 | rpc_service = Mock() | 78 | interfaces = { |
1107 | 87 | rpc_client = rpc_service.getClient.return_value | 79 | interface_name: { |
1108 | 88 | rpc_client.side_effect = [ | 80 | "enabled": True, |
1109 | 89 | defer.succeed(dict(interfaces=[interface])), | 81 | } |
1110 | 90 | ] | 82 | } |
1111 | 91 | 83 | ||
1112 | 92 | # We could patch out 'periodic_probe_task' instead here but this | ||
1113 | 93 | # is better because: | ||
1114 | 94 | # 1. The former requires spinning the reactor again before being | ||
1115 | 95 | # able to test the result. | ||
1116 | 96 | # 2. This way there's no thread to clean up after the test. | ||
1117 | 97 | deferToThread = self.patch(dhcp_probe_service, 'deferToThread') | 84 | deferToThread = self.patch(dhcp_probe_service, 'deferToThread') |
1121 | 98 | deferToThread.return_value = defer.succeed(None) | 85 | deferToThread.side_effect = [ |
1122 | 99 | service = DHCPProbeService( | 86 | defer.succeed(interfaces), |
1123 | 100 | rpc_service, clock) | 87 | defer.succeed(None), |
1124 | 88 | ] | ||
1125 | 89 | service = DHCPProbeService(Mock(), clock) | ||
1126 | 101 | service.startService() | 90 | service.startService() |
1127 | 102 | self.assertThat( | 91 | self.assertThat( |
1153 | 103 | deferToThread, MockCalledOnceWith( | 92 | deferToThread, MockCallsMatch( |
1154 | 104 | dhcp_probe_service.probe_interface, | 93 | call(dhcp_probe_service.get_all_interfaces_definition), |
1155 | 105 | interface['interface'], interface['ip'])) | 94 | call(dhcp_probe_service.probe_interface, interface_name))) |
1131 | 106 | |||
1132 | 107 | @defer.inlineCallbacks | ||
1133 | 108 | def test_exits_gracefully_if_cant_get_interfaces(self): | ||
1134 | 109 | clock = Clock() | ||
1135 | 110 | maaslog = self.patch(dhcp_probe_service, 'maaslog') | ||
1136 | 111 | |||
1137 | 112 | protocol, connecting = self.patch_rpc_methods() | ||
1138 | 113 | self.addCleanup((yield connecting)) | ||
1139 | 114 | |||
1140 | 115 | del protocol._commandDispatch[ | ||
1141 | 116 | region.GetClusterInterfaces.commandName] | ||
1142 | 117 | rpc_service = Mock() | ||
1143 | 118 | rpc_service.getClient.return_value = getRegionClient() | ||
1144 | 119 | service = DHCPProbeService( | ||
1145 | 120 | rpc_service, clock) | ||
1146 | 121 | yield service.startService() | ||
1147 | 122 | yield service.stopService() | ||
1148 | 123 | |||
1149 | 124 | self.assertThat( | ||
1150 | 125 | maaslog.error, MockCalledOnceWith( | ||
1151 | 126 | "Unable to query region for interfaces: Region does not " | ||
1152 | 127 | "support the GetClusterInterfaces RPC method.")) | ||
1156 | 128 | 95 | ||
1157 | 129 | @defer.inlineCallbacks | 96 | @defer.inlineCallbacks |
1158 | 130 | def test_exits_gracefully_if_cant_report_foreign_dhcp_server(self): | 97 | def test_exits_gracefully_if_cant_report_foreign_dhcp_server(self): |
1159 | 131 | clock = Clock() | 98 | clock = Clock() |
1160 | 99 | interface_name = factory.make_name("eth") | ||
1161 | 100 | interfaces = { | ||
1162 | 101 | interface_name: { | ||
1163 | 102 | "enabled": True, | ||
1164 | 103 | } | ||
1165 | 104 | } | ||
1166 | 105 | |||
1167 | 132 | maaslog = self.patch(dhcp_probe_service, 'maaslog') | 106 | maaslog = self.patch(dhcp_probe_service, 'maaslog') |
1168 | 133 | deferToThread = self.patch( | 107 | deferToThread = self.patch( |
1169 | 134 | dhcp_probe_service, 'deferToThread') | 108 | dhcp_probe_service, 'deferToThread') |
1171 | 135 | deferToThread.return_value = defer.succeed(['192.168.0.100']) | 109 | deferToThread.side_effect = [ |
1172 | 110 | defer.succeed(interfaces), | ||
1173 | 111 | defer.succeed(['192.168.0.100']), | ||
1174 | 112 | ] | ||
1175 | 136 | protocol, connecting = self.patch_rpc_methods() | 113 | protocol, connecting = self.patch_rpc_methods() |
1176 | 137 | self.addCleanup((yield connecting)) | 114 | self.addCleanup((yield connecting)) |
1177 | 138 | 115 | ||
1178 | 139 | del protocol._commandDispatch[ | 116 | del protocol._commandDispatch[ |
1179 | 140 | region.ReportForeignDHCPServer.commandName] | 117 | region.ReportForeignDHCPServer.commandName] |
1180 | 141 | protocol.GetClusterInterfaces.return_value = { | ||
1181 | 142 | 'interfaces': [ | ||
1182 | 143 | self.make_cluster_interface_values(ip='192.168.0.1'), | ||
1183 | 144 | ], | ||
1184 | 145 | } | ||
1185 | 146 | 118 | ||
1186 | 147 | rpc_service = Mock() | 119 | rpc_service = Mock() |
1187 | 148 | rpc_service.getClient.return_value = getRegionClient() | 120 | rpc_service.getClient.return_value = getRegionClient() |
1188 | @@ -153,13 +125,23 @@ | |||
1189 | 153 | 125 | ||
1190 | 154 | self.assertThat( | 126 | self.assertThat( |
1191 | 155 | maaslog.error, MockCalledOnceWith( | 127 | maaslog.error, MockCalledOnceWith( |
1193 | 156 | "Unable to inform region of rogue DHCP server: the region " | 128 | "Unable to inform region of DHCP server: the region " |
1194 | 157 | "does not yet support the ReportForeignDHCPServer RPC " | 129 | "does not yet support the ReportForeignDHCPServer RPC " |
1195 | 158 | "method.")) | 130 | "method.")) |
1196 | 159 | 131 | ||
1197 | 160 | def test_logs_errors(self): | 132 | def test_logs_errors(self): |
1198 | 161 | clock = Clock() | 133 | clock = Clock() |
1199 | 134 | interface_name = factory.make_name("eth") | ||
1200 | 135 | interfaces = { | ||
1201 | 136 | interface_name: { | ||
1202 | 137 | "enabled": True, | ||
1203 | 138 | } | ||
1204 | 139 | } | ||
1205 | 140 | |||
1206 | 162 | maaslog = self.patch(dhcp_probe_service, 'maaslog') | 141 | maaslog = self.patch(dhcp_probe_service, 'maaslog') |
1207 | 142 | mock_interfaces = self.patch( | ||
1208 | 143 | dhcp_probe_service, 'get_all_interfaces_definition') | ||
1209 | 144 | mock_interfaces.return_value = interfaces | ||
1210 | 163 | service = DHCPProbeService( | 145 | service = DHCPProbeService( |
1211 | 164 | sentinel.service, clock) | 146 | sentinel.service, clock) |
1212 | 165 | error_message = factory.make_string() | 147 | error_message = factory.make_string() |
1213 | @@ -168,25 +150,29 @@ | |||
1214 | 168 | service.startService() | 150 | service.startService() |
1215 | 169 | self.assertThat( | 151 | self.assertThat( |
1216 | 170 | maaslog.error, MockCalledOnceWith( | 152 | maaslog.error, MockCalledOnceWith( |
1218 | 171 | "Unable to probe for rogue DHCP servers: %s", | 153 | "Unable to probe for DHCP servers: %s", |
1219 | 172 | error_message)) | 154 | error_message)) |
1220 | 173 | 155 | ||
1221 | 174 | @defer.inlineCallbacks | 156 | @defer.inlineCallbacks |
1222 | 175 | def test_reports_foreign_dhcp_servers_to_region(self): | 157 | def test_reports_foreign_dhcp_servers_to_region(self): |
1223 | 176 | clock = Clock() | 158 | clock = Clock() |
1224 | 159 | interface_name = factory.make_name("eth") | ||
1225 | 160 | interfaces = { | ||
1226 | 161 | interface_name: { | ||
1227 | 162 | "enabled": True, | ||
1228 | 163 | } | ||
1229 | 164 | } | ||
1230 | 165 | |||
1231 | 177 | protocol, connecting = self.patch_rpc_methods() | 166 | protocol, connecting = self.patch_rpc_methods() |
1232 | 178 | self.addCleanup((yield connecting)) | 167 | self.addCleanup((yield connecting)) |
1233 | 179 | 168 | ||
1234 | 180 | deferToThread = self.patch( | 169 | deferToThread = self.patch( |
1235 | 181 | dhcp_probe_service, 'deferToThread') | 170 | dhcp_probe_service, 'deferToThread') |
1236 | 182 | foreign_dhcp_ip = factory.make_ipv4_address() | 171 | foreign_dhcp_ip = factory.make_ipv4_address() |
1244 | 183 | deferToThread.return_value = defer.succeed( | 172 | deferToThread.side_effect = [ |
1245 | 184 | [foreign_dhcp_ip]) | 173 | defer.succeed(interfaces), |
1246 | 185 | 174 | defer.succeed([foreign_dhcp_ip]), | |
1247 | 186 | interface = self.make_cluster_interface_values() | 175 | ] |
1241 | 187 | protocol.GetClusterInterfaces.return_value = { | ||
1242 | 188 | 'interfaces': [interface], | ||
1243 | 189 | } | ||
1248 | 190 | 176 | ||
1249 | 191 | client = getRegionClient() | 177 | client = getRegionClient() |
1250 | 192 | rpc_service = Mock() | 178 | rpc_service = Mock() |
1251 | @@ -201,24 +187,29 @@ | |||
1252 | 201 | protocol.ReportForeignDHCPServer, | 187 | protocol.ReportForeignDHCPServer, |
1253 | 202 | MockCalledOnceWith( | 188 | MockCalledOnceWith( |
1254 | 203 | protocol, | 189 | protocol, |
1258 | 204 | cluster_uuid=client.localIdent, | 190 | system_id=client.localIdent, |
1259 | 205 | interface_name=interface['name'], | 191 | interface_name=interface_name, |
1260 | 206 | foreign_dhcp_ip=foreign_dhcp_ip)) | 192 | dhcp_ip=foreign_dhcp_ip)) |
1261 | 207 | 193 | ||
1262 | 208 | @defer.inlineCallbacks | 194 | @defer.inlineCallbacks |
1263 | 209 | def test_reports_lack_of_foreign_dhcp_servers_to_region(self): | 195 | def test_reports_lack_of_foreign_dhcp_servers_to_region(self): |
1264 | 210 | clock = Clock() | 196 | clock = Clock() |
1265 | 197 | interface_name = factory.make_name("eth") | ||
1266 | 198 | interfaces = { | ||
1267 | 199 | interface_name: { | ||
1268 | 200 | "enabled": True, | ||
1269 | 201 | } | ||
1270 | 202 | } | ||
1271 | 203 | |||
1272 | 211 | protocol, connecting = self.patch_rpc_methods() | 204 | protocol, connecting = self.patch_rpc_methods() |
1273 | 212 | self.addCleanup((yield connecting)) | 205 | self.addCleanup((yield connecting)) |
1274 | 213 | 206 | ||
1275 | 214 | deferToThread = self.patch( | 207 | deferToThread = self.patch( |
1276 | 215 | dhcp_probe_service, 'deferToThread') | 208 | dhcp_probe_service, 'deferToThread') |
1283 | 216 | deferToThread.return_value = defer.succeed([]) | 209 | deferToThread.side_effect = [ |
1284 | 217 | 210 | defer.succeed(interfaces), | |
1285 | 218 | interface = self.make_cluster_interface_values() | 211 | defer.succeed([]), |
1286 | 219 | protocol.GetClusterInterfaces.return_value = { | 212 | ] |
1281 | 220 | 'interfaces': [interface], | ||
1282 | 221 | } | ||
1287 | 222 | 213 | ||
1288 | 223 | client = getRegionClient() | 214 | client = getRegionClient() |
1289 | 224 | rpc_service = Mock() | 215 | rpc_service = Mock() |
1290 | @@ -232,6 +223,6 @@ | |||
1291 | 232 | protocol.ReportForeignDHCPServer, | 223 | protocol.ReportForeignDHCPServer, |
1292 | 233 | MockCalledOnceWith( | 224 | MockCalledOnceWith( |
1293 | 234 | protocol, | 225 | protocol, |
1297 | 235 | cluster_uuid=client.localIdent, | 226 | system_id=client.localIdent, |
1298 | 236 | interface_name=interface['name'], | 227 | interface_name=interface_name, |
1299 | 237 | foreign_dhcp_ip=None)) | 228 | dhcp_ip=None)) |
1300 | 238 | 229 | ||
1301 | === modified file 'src/provisioningserver/rpc/region.py' | |||
1302 | --- src/provisioningserver/rpc/region.py 2016-03-28 13:54:47 +0000 | |||
1303 | +++ src/provisioningserver/rpc/region.py 2016-04-07 13:22:34 +0000 | |||
1304 | @@ -14,7 +14,6 @@ | |||
1305 | 14 | "GetBootConfig", | 14 | "GetBootConfig", |
1306 | 15 | "GetBootSources", | 15 | "GetBootSources", |
1307 | 16 | "GetBootSourcesV2", | 16 | "GetBootSourcesV2", |
1308 | 17 | "GetClusterInterfaces", | ||
1309 | 18 | "GetProxies", | 17 | "GetProxies", |
1310 | 19 | "Identify", | 18 | "Identify", |
1311 | 20 | "ListNodePowerParameters", | 19 | "ListNodePowerParameters", |
1312 | @@ -337,38 +336,20 @@ | |||
1313 | 337 | 336 | ||
1314 | 338 | 337 | ||
1315 | 339 | class ReportForeignDHCPServer(amp.Command): | 338 | class ReportForeignDHCPServer(amp.Command): |
1317 | 340 | """Report a foreign DHCP server on the cluster's network. | 339 | """Report a foreign DHCP server on a rack controller's interface. |
1318 | 341 | 340 | ||
1320 | 342 | :since: 1.7 | 341 | :since: 2.0 |
1321 | 343 | """ | 342 | """ |
1322 | 344 | 343 | ||
1323 | 345 | arguments = [ | 344 | arguments = [ |
1325 | 346 | (b"cluster_uuid", amp.Unicode()), | 345 | (b"system_id", amp.Unicode()), |
1326 | 347 | (b"interface_name", amp.Unicode()), | 346 | (b"interface_name", amp.Unicode()), |
1328 | 348 | (b"foreign_dhcp_ip", amp.Unicode(optional=True)), | 347 | (b"dhcp_ip", amp.Unicode(optional=True)), |
1329 | 349 | ] | 348 | ] |
1330 | 350 | response = [] | 349 | response = [] |
1331 | 351 | errors = [] | 350 | errors = [] |
1332 | 352 | 351 | ||
1333 | 353 | 352 | ||
1334 | 354 | class GetClusterInterfaces(amp.Command): | ||
1335 | 355 | """Fetch the known interfaces for a cluster from the region. | ||
1336 | 356 | |||
1337 | 357 | :since: 1.7 | ||
1338 | 358 | """ | ||
1339 | 359 | |||
1340 | 360 | arguments = [ | ||
1341 | 361 | (b"cluster_uuid", amp.Unicode()), | ||
1342 | 362 | ] | ||
1343 | 363 | response = [ | ||
1344 | 364 | (b"interfaces", AmpList( | ||
1345 | 365 | [(b"name", amp.Unicode()), | ||
1346 | 366 | (b"interface", amp.Unicode()), | ||
1347 | 367 | (b"ip", amp.Unicode())])) | ||
1348 | 368 | ] | ||
1349 | 369 | errors = [] | ||
1350 | 370 | |||
1351 | 371 | |||
1352 | 372 | class CreateNode(amp.Command): | 353 | class CreateNode(amp.Command): |
1353 | 373 | """Create a node on a given cluster. | 354 | """Create a node on a given cluster. |
1354 | 374 | 355 |
This looks like a welcome simplification. What's made it possible now?
How is rackd given permission to access the necessary port?
I think you can also remove p.dhcp. detect. process_ request( ) and its imports.
Good stuff though.