Merge lp:~julian-edwards/maas/revert-2819 into lp:~maas-committers/maas/trunk

Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 2822
Proposed branch: lp:~julian-edwards/maas/revert-2819
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 468 lines (+62/-254)
8 files modified
etc/celeryconfig_cluster.py (+8/-0)
src/provisioningserver/dhcp/detect.py (+26/-8)
src/provisioningserver/dhcp/dhcp_probe_service.py (+0/-85)
src/provisioningserver/dhcp/tests/test_detect.py (+13/-4)
src/provisioningserver/dhcp/tests/test_dhcp_probe_service.py (+0/-128)
src/provisioningserver/plugin.py (+0/-12)
src/provisioningserver/tasks.py (+12/-1)
src/provisioningserver/tests/test_plugin.py (+3/-16)
To merge this branch: bzr merge lp:~julian-edwards/maas/revert-2819
Reviewer Review Type Date Requested Status
Julian Edwards (community) selfie Approve
Review via email: mp+232338@code.launchpad.net

Commit message

Revert r2819 (pserv-based dhcp probing service), which could never work as it was using API credentials that are not sent to pserv.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) :
review: Approve (selfie)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/celeryconfig_cluster.py'
2--- etc/celeryconfig_cluster.py 2014-08-22 14:47:15 +0000
3+++ etc/celeryconfig_cluster.py 2014-08-26 23:57:04 +0000
4@@ -56,4 +56,12 @@
5 'expires': int(REPORT_BOOT_IMAGES_SCHEDULE.total_seconds()),
6 },
7 },
8+ 'probe-dhcp-servers': {
9+ 'task': 'provisioningserver.tasks.periodic_probe_dhcp',
10+ 'schedule': PROBE_DHCP_SERVERS_SCHEDULE,
11+ 'options': {
12+ 'queue': CLUSTER_UUID,
13+ 'expires': int(PROBE_DHCP_SERVERS_SCHEDULE.total_seconds()),
14+ },
15+ },
16 }
17
18=== modified file 'src/provisioningserver/dhcp/detect.py'
19--- src/provisioningserver/dhcp/detect.py 2014-08-26 10:19:58 +0000
20+++ src/provisioningserver/dhcp/detect.py 2014-08-26 23:57:04 +0000
21@@ -33,6 +33,11 @@
22 MAASDispatcher,
23 MAASOAuth,
24 )
25+from provisioningserver.auth import (
26+ get_recorded_api_credentials,
27+ get_recorded_nodegroup_uuid,
28+ )
29+from provisioningserver.cluster_config import get_maas_url
30 from provisioningserver.logger import get_maas_logger
31
32
33@@ -316,7 +321,7 @@
34 client.post, api_path, 'report_foreign_dhcp', foreign_dhcp_ip=server)
35
36
37-def periodic_probe_task(api_knowledge):
38+def periodic_probe_task():
39 """Probe for DHCP servers and set NodeGroupInterface.foriegn_dhcp.
40
41 This should be run periodically so that the database has an up-to-date
42@@ -325,12 +330,26 @@
43 NOTE: This uses blocking I/O with sequential polling of interfaces, and
44 hence doesn't scale well. It's a future improvement to make
45 to throw it in parallel threads or async I/O.
46-
47- :param api_knowledge: A dict of the information needed to be able to
48- make requests to the region's REST API.
49 """
50+ # Items that the server must have sent us before we can do this.
51+ knowledge = {
52+ 'maas_url': get_maas_url(),
53+ 'api_credentials': get_recorded_api_credentials(),
54+ 'nodegroup_uuid': get_recorded_nodegroup_uuid(),
55+ }
56+
57+ if None in knowledge.values():
58+ # The MAAS server hasn't sent us enough information for us to do
59+ # this yet. Leave it for another time.
60+ maaslog.info(
61+ "Not probing for rogue DHCP servers; not all required knowledge "
62+ "received from server yet. "
63+ "Missing: %s" % ', '.join(sorted(
64+ name for name, value in knowledge.items() if value is None)))
65+ return
66+
67 # Determine all the active interfaces on this cluster (nodegroup).
68- interfaces = determine_cluster_interfaces(api_knowledge)
69+ interfaces = determine_cluster_interfaces(knowledge)
70 if interfaces is None:
71 maaslog.info("No interfaces on cluster, not probing DHCP.")
72 return
73@@ -349,7 +368,6 @@
74 # Only send one, if it gets cleared out then the
75 # next detection pass will send a different one, if it
76 # still exists.
77- update_region_controller(
78- api_knowledge, interface, servers.pop())
79+ update_region_controller(knowledge, interface, servers.pop())
80 else:
81- update_region_controller(api_knowledge, interface, None)
82+ update_region_controller(knowledge, interface, None)
83
84=== removed file 'src/provisioningserver/dhcp/dhcp_probe_service.py'
85--- src/provisioningserver/dhcp/dhcp_probe_service.py 2014-08-26 15:47:52 +0000
86+++ src/provisioningserver/dhcp/dhcp_probe_service.py 1970-01-01 00:00:00 +0000
87@@ -1,85 +0,0 @@
88-# Copyright 2014 Canonical Ltd. This software is licensed under the
89-# GNU Affero General Public License version 3 (see the file LICENSE).
90-
91-"""Periodic DHCP probing service."""
92-
93-from __future__ import (
94- absolute_import,
95- print_function,
96- unicode_literals,
97- )
98-
99-str = None
100-
101-__metaclass__ = type
102-__all__ = [
103- "PeriodicDHCPProbeService",
104- ]
105-
106-
107-from datetime import timedelta
108-
109-from provisioningserver.auth import get_recorded_api_credentials
110-from provisioningserver.cluster_config import get_maas_url
111-from provisioningserver.dhcp import detect
112-from provisioningserver.logger.log import get_maas_logger
113-from twisted.application.internet import TimerService
114-from twisted.internet.defer import inlineCallbacks
115-from twisted.internet.threads import deferToThread
116-
117-
118-maaslog = get_maas_logger("dhcp.probe")
119-
120-
121-class PeriodicDHCPProbeService(TimerService, object):
122- """Service to probe for DHCP servers on this cluster's network.
123-
124- Built on top of Twisted's `TimerService`.
125-
126- :param reactor: An `IReactor` instance.
127- :param cluster_uuid: This cluster's UUID.
128- """
129-
130- check_interval = timedelta(minutes=1).total_seconds()
131-
132- def __init__(self, reactor, cluster_uuid):
133- # Call self.check() every self.check_interval.
134- super(PeriodicDHCPProbeService, self).__init__(
135- self.check_interval, self.try_probe_dhcp)
136- self.clock = reactor
137- self.uuid = cluster_uuid
138-
139- def _probe_dhcp(self, knowledge):
140- """Probe for DHCP servers."""
141- return deferToThread(detect.periodic_probe_task, knowledge)
142-
143- @inlineCallbacks
144- def try_probe_dhcp(self):
145- # Items that the server must have sent us before we can do
146- # this.
147- knowledge = {
148- 'api_credentials': get_recorded_api_credentials(),
149- 'maas_url': get_maas_url(),
150- 'nodegroup_uuid': self.uuid,
151- }
152-
153- if None in knowledge.values():
154- # The MAAS server hasn't sent us enough information for
155- # us to do this yet. Leave it for another time.
156- maaslog.info(
157- "Not probing for rogue DHCP servers; not all "
158- "required knowledge received from server yet. "
159- "Missing: %s" % ', '.join(sorted(
160- name for name, value in knowledge.items()
161- if value is None)))
162- return
163- else:
164- maaslog.debug("Running periodic DHCP probe.")
165- try:
166- d = self._probe_dhcp(knowledge)
167- yield d
168- maaslog.debug("Finished periodic DHCP probe.")
169- except Exception as error:
170- maaslog.error(
171- "Unable to probe for rogue DHCP servers: %s"
172- % unicode(error))
173
174=== modified file 'src/provisioningserver/dhcp/tests/test_detect.py'
175--- src/provisioningserver/dhcp/tests/test_detect.py 2014-08-26 10:19:58 +0000
176+++ src/provisioningserver/dhcp/tests/test_detect.py 2014-08-26 23:57:04 +0000
177@@ -30,7 +30,10 @@
178 from maastesting.testcase import MAASTestCase
179 import mock
180 from provisioningserver import cache
181-from provisioningserver.auth import NODEGROUP_UUID_CACHE_KEY
182+from provisioningserver.auth import (
183+ NODEGROUP_UUID_CACHE_KEY,
184+ record_api_credentials,
185+ )
186 from provisioningserver.dhcp.detect import (
187 BOOTP_CLIENT_PORT,
188 BOOTP_SERVER_PORT,
189@@ -510,11 +513,17 @@
190 "Failed talking to region controller, it returned:",
191 self.maaslog.output)
192
193+ def test_periodic_probe_task_exits_with_not_enough_knowledge(self):
194+ mocked = self.patch(detect_module, 'determine_cluster_interfaces')
195+ record_api_credentials(None)
196+ periodic_probe_task()
197+ self.assertFalse(mocked.called)
198+
199 def test_periodic_probe_task_exits_if_no_interfaces(self):
200 mocked = self.patch(detect_module, 'probe_interface')
201 self.patch(
202 detect_module, 'determine_cluster_interfaces').return_value = None
203- periodic_probe_task(self.knowledge)
204+ periodic_probe_task()
205 self.assertFalse(mocked.called)
206
207 def test_periodic_probe_task_updates_region_with_detected_server(self):
208@@ -526,7 +535,7 @@
209 self.patch(
210 detect_module, 'probe_dhcp').return_value = {detected_server}
211 mocked_update = self.patch(detect_module, 'update_region_controller')
212- periodic_probe_task(self.knowledge)
213+ periodic_probe_task()
214 calls = [
215 mock.call(self.knowledge, 'eth0', detected_server),
216 mock.call(self.knowledge, 'wlan0', detected_server),
217@@ -541,7 +550,7 @@
218 self.patch(
219 detect_module, 'probe_dhcp').return_value = set()
220 mocked_update = self.patch(detect_module, 'update_region_controller')
221- periodic_probe_task(self.knowledge)
222+ periodic_probe_task()
223 calls = [
224 mock.call(self.knowledge, 'eth0', None),
225 mock.call(self.knowledge, 'wlan0', None),
226
227=== removed file 'src/provisioningserver/dhcp/tests/test_dhcp_probe_service.py'
228--- src/provisioningserver/dhcp/tests/test_dhcp_probe_service.py 2014-08-26 15:45:21 +0000
229+++ src/provisioningserver/dhcp/tests/test_dhcp_probe_service.py 1970-01-01 00:00:00 +0000
230@@ -1,128 +0,0 @@
231-# Copyright 2014 Canonical Ltd. This software is licensed under the
232-# GNU Affero General Public License version 3 (see the file LICENSE).
233-
234-"""Tests for periodic DHCP prober."""
235-
236-from __future__ import (
237- absolute_import,
238- print_function,
239- unicode_literals,
240- )
241-
242-str = None
243-
244-__metaclass__ = type
245-__all__ = []
246-
247-import os
248-
249-from apiclient.testing.credentials import make_api_credentials
250-from maastesting.factory import factory
251-from maastesting.matchers import (
252- get_mock_calls,
253- HasLength,
254- MockCalledOnceWith,
255- MockNotCalled,
256- )
257-from mock import ANY
258-from provisioningserver import cache
259-from provisioningserver.auth import (
260- NODEGROUP_UUID_CACHE_KEY,
261- record_api_credentials,
262- )
263-from provisioningserver.dhcp import dhcp_probe_service
264-from provisioningserver.dhcp.dhcp_probe_service import (
265- PeriodicDHCPProbeService,
266- )
267-from provisioningserver.testing.testcase import PservTestCase
268-from testtools.deferredruntest import AsynchronousDeferredRunTest
269-from twisted.internet import defer
270-from twisted.internet.task import Clock
271-
272-
273-class TestDHCPProbeService(PservTestCase):
274-
275- run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
276-
277- def setUp(self):
278- super(TestDHCPProbeService, self).setUp()
279- self.cluster_uuid = factory.make_UUID()
280- maas_url = 'http://%s.example.com/%s/' % (
281- factory.make_name('host'),
282- factory.make_string(),
283- )
284- api_credentials = make_api_credentials()
285-
286- cache.cache.set(NODEGROUP_UUID_CACHE_KEY, self.cluster_uuid)
287- os.environ["MAAS_URL"] = maas_url
288- cache.cache.set('api_credentials', ':'.join(api_credentials))
289-
290- self.knowledge = dict(
291- api_credentials=api_credentials,
292- maas_url=maas_url,
293- nodegroup_uuid=self.cluster_uuid)
294-
295- def test_is_called_every_interval(self):
296- clock = Clock()
297- service = PeriodicDHCPProbeService(clock, self.cluster_uuid)
298-
299- # Avoid actually probing
300- _probe_dhcp = self.patch(service, '_probe_dhcp')
301-
302- # Until the service has started, periodic_probe_dhcp() won't
303- # be called.
304- self.assertThat(_probe_dhcp, MockNotCalled())
305-
306- # The first call is issued at startup.
307- service.startService()
308- self.assertThat(_probe_dhcp, MockCalledOnceWith(ANY))
309-
310- # Wind clock forward one second less than the desired interval.
311- clock.advance(service.check_interval - 1)
312-
313- # No more periodic calls made.
314- self.assertEqual(1, len(get_mock_calls(_probe_dhcp)))
315-
316- # Wind clock forward one second, past the interval.
317- clock.advance(1)
318-
319- # Now there were two calls.
320- self.assertThat(get_mock_calls(_probe_dhcp), HasLength(2))
321-
322- def test_download_is_initiated_in_new_thread(self):
323- clock = Clock()
324-
325- # We could patch out 'periodic_probe_task' instead here but this
326- # is better because:
327- # 1. The former requires spinning the reactor again before being
328- # able to test the result.
329- # 2. This way there's no thread to clean up after the test.
330- deferToThread = self.patch(dhcp_probe_service, 'deferToThread')
331- deferToThread.return_value = defer.succeed(None)
332- service = PeriodicDHCPProbeService(clock, self.cluster_uuid)
333- service.startService()
334- self.assertThat(
335- deferToThread, MockCalledOnceWith(
336- dhcp_probe_service.detect.periodic_probe_task,
337- self.knowledge))
338-
339- def test_no_probe_if_api_credentials_not_set(self):
340- record_api_credentials(None)
341- clock = Clock()
342- service = PeriodicDHCPProbeService(clock, self.cluster_uuid)
343- _probe_dhcp = self.patch(service, '_probe_dhcp')
344- service.startService()
345- self.assertThat(_probe_dhcp, MockNotCalled())
346-
347- def test_logs_errors(self):
348- clock = Clock()
349- maaslog = self.patch(dhcp_probe_service, 'maaslog')
350- service = PeriodicDHCPProbeService(clock, self.cluster_uuid)
351- error_message = factory.make_string()
352- self.patch(service, '_probe_dhcp').side_effect = Exception(
353- error_message)
354- service.startService()
355- self.assertThat(
356- maaslog.error, MockCalledOnceWith(
357- "Unable to probe for rogue DHCP servers: %s" %
358- error_message))
359
360=== modified file 'src/provisioningserver/plugin.py'
361--- src/provisioningserver/plugin.py 2014-08-26 17:23:16 +0000
362+++ src/provisioningserver/plugin.py 2014-08-26 23:57:04 +0000
363@@ -31,9 +31,6 @@
364 import provisioningserver
365 from provisioningserver.cluster_config import get_cluster_uuid
366 from provisioningserver.config import Config
367-from provisioningserver.dhcp.dhcp_probe_service import (
368- PeriodicDHCPProbeService,
369- )
370 from provisioningserver.rpc.boot_images import PeriodicImageDownloadService
371 from provisioningserver.rpc.clusterservice import ClusterClientService
372 from provisioningserver.rpc.power import NodePowerMonitorService
373@@ -242,12 +239,6 @@
374 rpc_service.setName("rpc")
375 return rpc_service
376
377- def _makePeriodicDHCPProbeService(self, rpc_service):
378- dhcp_probe_service = PeriodicDHCPProbeService(
379- reactor, get_cluster_uuid())
380- dhcp_probe_service.setName("dhcp_probe")
381- return dhcp_probe_service
382-
383 def makeService(self, options):
384 """Construct a service."""
385 services = provisioningserver.services
386@@ -272,7 +263,4 @@
387 rpc_service)
388 image_download_service.setServiceParent(services)
389
390- dhcp_probe_service = self._makePeriodicDHCPProbeService(rpc_service)
391- dhcp_probe_service.setServiceParent(services)
392-
393 return services
394
395=== modified file 'src/provisioningserver/tasks.py'
396--- src/provisioningserver/tasks.py 2014-08-26 10:23:23 +0000
397+++ src/provisioningserver/tasks.py 2014-08-26 23:57:04 +0000
398@@ -43,7 +43,10 @@
399 record_api_credentials,
400 record_nodegroup_uuid,
401 )
402-from provisioningserver.dhcp import config
403+from provisioningserver.dhcp import (
404+ config,
405+ detect,
406+ )
407 from provisioningserver.dhcp.control import (
408 restart_dhcpv4,
409 stop_dhcpv4,
410@@ -445,6 +448,14 @@
411 raise
412
413
414+@task
415+@log_task_events(level=logging.DEBUG)
416+@log_exception_text
417+def periodic_probe_dhcp():
418+ """Probe for foreign DHCP servers."""
419+ detect.periodic_probe_task()
420+
421+
422 # =====================================================================
423 # Boot images-related tasks
424 # =====================================================================
425
426=== modified file 'src/provisioningserver/tests/test_plugin.py'
427--- src/provisioningserver/tests/test_plugin.py 2014-08-26 17:23:16 +0000
428+++ src/provisioningserver/tests/test_plugin.py 2014-08-26 23:57:04 +0000
429@@ -21,9 +21,6 @@
430 from maastesting.testcase import MAASTestCase
431 import provisioningserver
432 from provisioningserver import plugin as plugin_module
433-from provisioningserver.dhcp.dhcp_probe_service import (
434- PeriodicDHCPProbeService,
435- )
436 from provisioningserver.plugin import (
437 Options,
438 ProvisioningRealm,
439@@ -112,11 +109,9 @@
440 service_maker = ProvisioningServiceMaker("Harry", "Hill")
441 service = service_maker.makeService(options)
442 self.assertIsInstance(service, MultiService)
443- expected_services = [
444- "dhcp_probe", "image_download", "log", "node_monitor",
445- "oops", "rpc", "tftp",
446- ]
447- self.assertItemsEqual(expected_services, service.namedServices)
448+ self.assertSequenceEqual(
449+ ["image_download", "log", "node_monitor", "oops", "rpc", "tftp"],
450+ sorted(service.namedServices))
451 self.assertEqual(
452 len(service.namedServices), len(service.services),
453 "Not all services are named.")
454@@ -138,14 +133,6 @@
455 node_monitor = service.getServiceNamed("node_monitor")
456 self.assertIsInstance(node_monitor, NodePowerMonitorService)
457
458- def test_dhcp_probe_service(self):
459- options = Options()
460- options["config-file"] = self.write_config({})
461- service_maker = ProvisioningServiceMaker("Spike", "Milligan")
462- service = service_maker.makeService(options)
463- dhcp_probe = service.getServiceNamed("dhcp_probe")
464- self.assertIsInstance(dhcp_probe, PeriodicDHCPProbeService)
465-
466 def test_tftp_service(self):
467 # A TFTP service is configured and added to the top-level service.
468 config = {