Merge lp:~julian-edwards/maas/stop-dhcp-server-bug-1283114 into lp:maas/trunk

Proposed by Julian Edwards on 2014-02-24
Status: Merged
Approved by: Julian Edwards on 2014-02-25
Approved revision: 2014
Merged at revision: 2015
Proposed branch: lp:~julian-edwards/maas/stop-dhcp-server-bug-1283114
Merge into: lp:maas/trunk
Diff against target: 374 lines (+131/-45)
9 files modified
src/maasserver/dhcp.py (+14/-8)
src/maasserver/testing/__init__.py (+24/-0)
src/maasserver/testing/factory.py (+2/-1)
src/maasserver/testing/tests/test_factory.py (+3/-1)
src/maasserver/testing/tests/test_module.py (+39/-0)
src/maasserver/tests/test_dhcp.py (+22/-8)
src/maasserver/tests/test_forms.py (+10/-22)
src/provisioningserver/tasks.py (+7/-0)
src/provisioningserver/tests/test_tasks.py (+10/-5)
To merge this branch: bzr merge lp:~julian-edwards/maas/stop-dhcp-server-bug-1283114
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) 2014-02-24 Approve on 2014-02-25
Review via email: mp+207870@code.launchpad.net

Commit message

Make sure the DHCP config is rewritten when all a cluster's interfaces go unmanaged. Previously, this condition was ignored so the DHCP server carried on serving when it should not be.

Description of the change

get_interfaces_managed_by() was the source of this bug, really. It was being used for two things - both finding interfaces and detecting whether DHCP_CONNECT was being used, and returning an empty list []. The call site assumed the empty list meant that there were no managed interfaces! I've fixed it by moving the call to check DHCP_CONNECT into the main config_dhcp() function.

In addition I vastly simplified the test_report_foreign_dhcp_does_not_trigger_update_signal() test which was depending on this odd behaviour of get_interfaces_managed_by(). It's about a third of the size now.

There's a few extra fixes and helpers here due to the initial flawed approach to my fix but I've left them in as they may be useful (according to Raphaël!).

 - There's a new task stop_dhcp_server which is currently unused but I've left it in anyway as we might need it and it would save a future coder some time.
 - I added a context manager to temporarily disable signal receivers for a signal. This would be most useful in factory methods where you need to set up state without being encumbered by signal side effects. It's also presently not used.

To post a comment you must log in.
Jeroen T. Vermeulen (jtv) wrote :

I see no test to establish that NoReceivers actually works, nor that it only affects the signals you pass it. Given the earlier confusion with the global signals list, neither is a given. Landing this code both unused and untested seems irresponsible, whereas putting more work into it when we don't really know that we'll be needing it seems pointless. So if I were you I'd just drop it. If we do ever need it, searching the internet for NoReceivers will bring us back to this MP.

.

The simplification of test_report_foreign_dhcp_does_not_trigger_update_signal is great. One small thing though:

303 - self.assertEqual([], patch.mock_calls)
304 + self.assertEqual(0, tasks.write_dhcp_config.apply_async.call_count)

Testing against [] is usually more helpful than testing against a call_count of zero, because the test's failure will tell you what calls were made, which can help in debugging.

.

Twice in src/provisioningserver/tests/test_tasks.py, as well as once in test_write_dhcp_config_called_when_no_managed_interfaces, you use Mock.assert_called_once_with. Please avoid this method — it's prone to false negatives due to a pitfall in Mock's design. If there is ever a typo in the assertion method's name, the Mock will see it as a regular method call, and very helpfully mock it. The result: an assertion that can't fail.

Use an equality check on Mock.mock_calls instead. It's harder to lose a typo in "mock_calls," and when we do, it will lead to a false positive which will immediately compel us to fix the typo.

review: Approve
Julian Edwards (julian-edwards) wrote :

On 25/02/14 16:35, Jeroen T. Vermeulen wrote:
> Review: Approve

Thanks for the review.

> I see no test to establish that NoReceivers actually works,

What?! I added one, it's in the diff?:

+class TestNoReceivers(MAASServerTestCase):

If you are implying that I should test that no signal receivers are
actually called - then you'd be asking for a test that tests Django
signals functionality.

> nor that it only affects the signals you pass it.

That would be impossible unless there was a registry of all known
signals, which AFAIK there isn't. They can be created at any time.

> Given the earlier confusion with the global signals list, neither is a given. Landing this code both unused and untested seems irresponsible, whereas putting more work into it when we don't really know that we'll be needing it seems pointless. So if I were you I'd just drop it. If we do ever need it, searching the internet for NoReceivers will bring us back to this MP.

I think that's a rather harsh assessment. IIRC Raphaël made a good case
for it after I wanted to drop it but I can't remember what he said now.
 Perhaps he'll chime in.

(I think it was something to do with speeding up the test suite, because
we have huge numbers of useless signals firing in every test that sets
up dhcp/dns related objects.)

> The simplification of test_report_foreign_dhcp_does_not_trigger_update_signal is great. One small thing though:
>
> 303 - self.assertEqual([], patch.mock_calls)
> 304 + self.assertEqual(0, tasks.write_dhcp_config.apply_async.call_count)
>
> Testing against [] is usually more helpful than testing against a call_count of zero, because the test's failure will tell you what calls were made, which can help in debugging.

You're right - I originally tried to do that and got caught up with
mock's innards. It doesn't have an assert_not_called() function :(

It turns out that this works:
    tasks.write_dhcp_config.apply_async.assert_has_calls([])

>
> .
>
> Twice in src/provisioningserver/tests/test_tasks.py, as well as once in test_write_dhcp_config_called_when_no_managed_interfaces, you use Mock.assert_called_once_with. Please avoid this method — it's prone to false negatives due to a pitfall in Mock's design. If there is ever a typo in the assertion method's name, the Mock will see it as a regular method call, and very helpfully mock it. The result: an assertion that can't fail.
>
> Use an equality check on Mock.mock_calls instead. It's harder to lose a typo in "mock_calls," and when we do, it will lead to a false positive which will immediately compel us to fix the typo.
>

Can I leave them here pretty please? If I promise to write a testtools
matcher so this can't happen again....

:)

2013. By Julian Edwards on 2014-02-25

Fix assertion to fail more explicitly

Raphaël Badin (rvb) wrote :

> I think that's a rather harsh assessment. IIRC Raphaël made a good case
> for it after I wanted to drop it but I can't remember what he said now.
> Perhaps he'll chime in.

> (I think it was something to do with speeding up the test suite, because
> we have huge numbers of useless signals firing in every test that sets
> up dhcp/dns related objects.)

Yes, my comment was about the potential speed-up that could result from disconnecting the signals when we create test objects. It needs to be tested but I think the speed-up could be humongous: each time we create a node without explicitly passing a nodegroup, make_node() creates a nodegroup object for us. Given that we have signals fired when a nodegroup gets saved and that the test suite creates thousands of nodes, I think it's definitely worth trying to use the NoReceivers context manager in the factory module.

Jeroen T. Vermeulen (jtv) wrote :

> > I see no test to establish that NoReceivers actually works,
>
> What?! I added one, it's in the diff?:
>
> +class TestNoReceivers(MAASServerTestCase):
>
> If you are implying that I should test that no signal receivers are
> actually called - then you'd be asking for a test that tests Django
> signals functionality.

You wrote tests that match the code in NoReceivers, but none that verify that the thing actually works as intended. Your initial implementation would have passed that kind of test, but didn't actually work.

> > nor that it only affects the signals you pass it.
>
> That would be impossible unless there was a registry of all known
> signals, which AFAIK there isn't. They can be created at any time.

No, it wouldn't — all you'd need was _an example_ of a signal that would otherwise have been affected. Remember that your initial implementation operated on a global signals list.

Julian Edwards (julian-edwards) wrote :

On 25/02/14 19:44, Jeroen T. Vermeulen wrote:
>
>>> I see no test to establish that NoReceivers actually works,
>>
>> What?! I added one, it's in the diff?:
>>
>> +class TestNoReceivers(MAASServerTestCase):
>>
>> If you are implying that I should test that no signal receivers are
>> actually called - then you'd be asking for a test that tests Django
>> signals functionality.
>
> You wrote tests that match the code in NoReceivers, but none that verify that the thing actually works as intended. Your initial implementation would have passed that kind of test, but didn't actually work.

My initial implementation was no different to this one other than this
takes multiple signals.

>>> nor that it only affects the signals you pass it.
>>
>> That would be impossible unless there was a registry of all known
>> signals, which AFAIK there isn't. They can be created at any time.
>
> No, it wouldn't — all you'd need was _an example_ of a signal that would otherwise have been affected. Remember that your initial implementation operated on a global signals list.
>

Ah ok so what you mean is, test that it doesn't affect a subset of
signals that we don't pass it. ;)

2014. By Julian Edwards on 2014-02-25

Add another NoRecievers test to check that it leaves other signals alone

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/dhcp.py'
2--- src/maasserver/dhcp.py 2014-01-29 10:22:57 +0000
3+++ src/maasserver/dhcp.py 2014-02-25 10:05:30 +0000
4@@ -29,24 +29,30 @@
5 def get_interfaces_managed_by(nodegroup):
6 """Return `NodeGroupInterface` objects for which `nodegroup` manages DHCP.
7
8- Returns only interfaces for which MAAS is supposed to serve DHCP. If DHCP
9- is disabled, or the node group is not accepted, an empty list will be
10- returned. Interfaces whose DHCP is not managed are not returned in any
11- case.
12+ Returns only interfaces for which MAAS is supposed to serve DHCP.
13+ If the node group is not accepted, an empty list will be returned.
14+ Interfaces whose DHCP is not managed are not returned in any case.
15 """
16- if settings.DHCP_CONNECT and nodegroup.status == NODEGROUP_STATUS.ACCEPTED:
17+ if nodegroup.status == NODEGROUP_STATUS.ACCEPTED:
18 return nodegroup.get_managed_interfaces()
19- else:
20- return []
21+
22+ return None
23
24
25 def configure_dhcp(nodegroup):
26 """Write the DHCP configuration file and restart the DHCP server."""
27+ # Let's get this out of the way first up shall we?
28+ if not settings.DHCP_CONNECT:
29+ # For the uninitiated, DHCP_CONNECT is set, by default, to False
30+ # in all tests and True in non-tests. This avoids unnecessary
31+ # calls to async tasks.
32+ return
33+
34 # Circular imports.
35 from maasserver.dns import get_dns_server_address
36
37 interfaces = get_interfaces_managed_by(nodegroup)
38- if interfaces == []:
39+ if interfaces is None:
40 return
41
42 # Make sure this nodegroup has a key to communicate with the dhcp
43
44=== modified file 'src/maasserver/testing/__init__.py'
45--- src/maasserver/testing/__init__.py 2013-12-18 11:21:05 +0000
46+++ src/maasserver/testing/__init__.py 2014-02-25 10:05:30 +0000
47@@ -17,10 +17,13 @@
48 "get_content_links",
49 "get_data",
50 "get_prefixed_form_data",
51+ "NoReceivers",
52 "reload_object",
53 "reload_objects",
54 ]
55
56+import collections
57+from contextlib import contextmanager
58 import httplib
59 import os
60 from urlparse import urlparse
61@@ -139,3 +142,24 @@
62 for matching_node in doc.cssselect(element)
63 ]
64 return sum(links_per_matching_node, [])
65+
66+
67+@contextmanager
68+def NoReceivers(signals):
69+ """Disconnect signal receivers from the supplied signals.
70+
71+ :param signals: A signal (or iterable of signals) for which to disable
72+ signal receivers while in the context manager.
73+ :type signal: django.dispatch.Signal
74+ """
75+ saved = dict()
76+ if not isinstance(signals, collections.Iterable):
77+ signals = [signals]
78+ for signal in signals:
79+ saved[signal] = signal.receivers
80+ signal.receivers = []
81+ try:
82+ yield
83+ finally:
84+ for signal in signals:
85+ signal.receivers = saved[signal]
86
87=== modified file 'src/maasserver/testing/factory.py'
88--- src/maasserver/testing/factory.py 2014-02-25 07:55:47 +0000
89+++ src/maasserver/testing/factory.py 2014-02-25 10:05:30 +0000
90@@ -245,7 +245,8 @@
91 ip_range_low=ip_range_low, ip_range_high=ip_range_high,
92 interface=interface, management=management)
93 interface_settings.update(kwargs)
94- ng = NodeGroup.objects.new(name=name, uuid=uuid, **interface_settings)
95+ ng = NodeGroup.objects.new(
96+ name=name, uuid=uuid, **interface_settings)
97 ng.cluster_name = cluster_name
98 ng.status = status
99 ng.maas_url = maas_url
100
101=== modified file 'src/maasserver/testing/tests/test_factory.py'
102--- src/maasserver/testing/tests/test_factory.py 2014-02-21 08:06:43 +0000
103+++ src/maasserver/testing/tests/test_factory.py 2014-02-25 10:05:30 +0000
104@@ -20,7 +20,9 @@
105 Network,
106 NodeGroup,
107 )
108-from maasserver.testing import reload_object
109+from maasserver.testing import (
110+ reload_object,
111+ )
112 from maasserver.testing.factory import factory
113 from maastesting.factory import TooManyRandomRetries
114 from maastesting.testcase import MAASTestCase
115
116=== modified file 'src/maasserver/testing/tests/test_module.py'
117--- src/maasserver/testing/tests/test_module.py 2013-10-07 09:12:40 +0000
118+++ src/maasserver/testing/tests/test_module.py 2014-02-25 10:05:30 +0000
119@@ -16,12 +16,17 @@
120
121 import httplib
122
123+from django.db.models.signals import (
124+ post_save,
125+ pre_save,
126+ )
127 from django.http import (
128 HttpResponse,
129 HttpResponseRedirect,
130 )
131 from maasserver.testing import (
132 extract_redirect,
133+ NoReceivers,
134 reload_object,
135 reload_objects,
136 )
137@@ -97,3 +102,37 @@
138 dead_obj = objs.pop(0)
139 TestModel.objects.filter(id=dead_obj.id).delete()
140 self.assertItemsEqual(objs, reload_objects(TestModel, objs))
141+
142+
143+class TestNoReceivers(MAASServerTestCase):
144+
145+ def test_clears_and_restores_signal(self):
146+ # post_save already has some receivers on it, but make sure.
147+ self.assertNotEqual(0, len(post_save.receivers))
148+ old_values = list(post_save.receivers)
149+
150+ with NoReceivers(post_save):
151+ self.assertEqual([], post_save.receivers)
152+
153+ self.assertItemsEqual(old_values, post_save.receivers)
154+
155+ def test_clears_and_restores_many_signals(self):
156+ self.assertNotEqual(0, len(post_save.receivers))
157+ self.assertNotEqual(0, len(pre_save.receivers))
158+ old_pre_values = pre_save.receivers
159+ old_post_values = post_save.receivers
160+
161+ with NoReceivers((post_save, pre_save)):
162+ self.assertEqual([], post_save.receivers)
163+ self.assertEqual([], pre_save.receivers)
164+
165+ self.assertItemsEqual(old_pre_values, pre_save.receivers)
166+ self.assertItemsEqual(old_post_values, post_save.receivers)
167+
168+ def test_leaves_some_other_signals_alone(self):
169+ self.assertNotEqual(0, len(post_save.receivers))
170+
171+ old_pre_values = pre_save.receivers
172+
173+ with NoReceivers(post_save):
174+ self.assertItemsEqual(old_pre_values, pre_save.receivers)
175
176=== modified file 'src/maasserver/tests/test_dhcp.py'
177--- src/maasserver/tests/test_dhcp.py 2014-02-07 15:04:49 +0000
178+++ src/maasserver/tests/test_dhcp.py 2014-02-25 10:05:30 +0000
179@@ -33,6 +33,7 @@
180 from maasserver.testing.testcase import MAASServerTestCase
181 from maasserver.utils import map_enum
182 from maastesting.celery import CeleryFixture
183+from mock import ANY
184 from netaddr import (
185 IPAddress,
186 IPNetwork,
187@@ -59,13 +60,7 @@
188 list(nodegroup.nodegroupinterface_set.all()),
189 managed_interfaces)
190
191- def test_get_interfaces_managed_by_obeys_DHCP_CONNECT(self):
192- self.patch(settings, "DHCP_CONNECT", False)
193- nodegroup = factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
194- self.assertEqual([], get_interfaces_managed_by(nodegroup))
195-
196- def test_get_interfaces_managed_by_returns_empty_if_not_accepted(self):
197- self.patch(settings, "DHCP_CONNECT", True)
198+ def test_get_interfaces_managed_by_returns_None_if_not_accepted(self):
199 unaccepted_statuses = set(map_enum(NODEGROUP_STATUS).values())
200 unaccepted_statuses.remove(NODEGROUP_STATUS.ACCEPTED)
201 managed_interfaces = {
202@@ -74,9 +69,15 @@
203 for status in unaccepted_statuses
204 }
205 self.assertEqual(
206- {status: [] for status in unaccepted_statuses},
207+ {status: None for status in unaccepted_statuses},
208 managed_interfaces)
209
210+ def test_configure_dhcp_obeys_DHCP_CONNECT(self):
211+ self.patch(settings, "DHCP_CONNECT", False)
212+ self.patch(dhcp, 'write_dhcp_config')
213+ factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
214+ self.assertEqual(0, dhcp.write_dhcp_config.apply_async.call_count)
215+
216 def test_configure_dhcp_writes_dhcp_config(self):
217 mocked_task = self.patch(dhcp, 'write_dhcp_config')
218 self.patch(
219@@ -204,6 +205,19 @@
220 args, kwargs = task.subtask.call_args
221 self.assertEqual(nodegroup.work_queue, kwargs['options']['queue'])
222
223+ def test_write_dhcp_config_called_when_no_managed_interfaces(self):
224+ nodegroup = factory.make_node_group(
225+ status=NODEGROUP_STATUS.ACCEPTED,
226+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
227+ [interface] = nodegroup.nodegroupinterface_set.all()
228+ self.patch(settings, "DHCP_CONNECT", True)
229+ self.patch(tasks, 'sudo_write_file')
230+ self.patch(dhcp, 'write_dhcp_config')
231+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
232+ interface.save()
233+ dhcp.write_dhcp_config.apply_async.assert_called_once_with(
234+ queue=nodegroup.work_queue, kwargs=ANY)
235+
236 def test_dhcp_config_gets_written_when_interface_IP_changes(self):
237 nodegroup = factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
238 [interface] = nodegroup.nodegroupinterface_set.all()
239
240=== modified file 'src/maasserver/tests/test_forms.py'
241--- src/maasserver/tests/test_forms.py 2014-02-20 10:48:04 +0000
242+++ src/maasserver/tests/test_forms.py 2014-02-25 10:05:30 +0000
243@@ -17,6 +17,7 @@
244 import json
245
246 from django import forms
247+from django.conf import settings
248 from django.contrib.auth.models import User
249 from django.core.exceptions import ValidationError
250 from django.core.files.uploadedfile import SimpleUploadedFile
251@@ -85,6 +86,7 @@
252 from maasserver.utils import map_enum
253 from metadataserver.models import CommissioningScript
254 from netaddr import IPNetwork
255+from provisioningserver import tasks
256 from testtools import TestCase
257 from testtools.matchers import (
258 AllMatch,
259@@ -726,13 +728,13 @@
260 {'ip': ['Enter a valid IPv4 or IPv6 address.']}, form._errors)
261
262 def test_NodeGroupInterfaceForm_can_save_fields_being_None(self):
263- settings = make_interface_settings()
264- settings['management'] = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
265+ int_settings = make_interface_settings()
266+ int_settings['management'] = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
267 for field_name in nullable_fields:
268- del settings[field_name]
269+ del int_settings[field_name]
270 nodegroup = factory.make_node_group()
271 form = NodeGroupInterfaceForm(
272- data=settings, instance=NodeGroupInterface(nodegroup=nodegroup))
273+ data=int_settings, instance=NodeGroupInterface(nodegroup=nodegroup))
274 interface = form.save()
275 field_values = [
276 getattr(interface, field_name) for field_name in nullable_fields]
277@@ -761,26 +763,12 @@
278 self.assertFalse(form.is_valid())
279
280 def test_report_foreign_dhcp_does_not_trigger_update_signal(self):
281+ self.patch(settings, "DHCP_CONNECT", False)
282 nodegroup = factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
283 [interface] = nodegroup.get_managed_interfaces()
284
285- # Patch out the DHCP rewrites as triggered by some NodeGroupInterface
286- # changes, both to stop them from happening in a test and to detect
287- # whether the signal was actually fired.
288- # We can't patch the signal handlers themselves, because they are
289- # already set up. Nor can we patch configure_dhcp, because the signal
290- # handlers import it locally to avoid import cycles. Patching this
291- # underlying function instead will stop configure_dhcp() from doing
292- # anything real.
293- patch = self.patch(dhcp_module, 'get_interfaces_managed_by')
294- patch.return_value = []
295- # The patch is in fact called when we make changes to the interface
296- # that should trigger a DHCP config rewrite.
297- interface.interface = factory.make_name('eth')
298- interface.save()
299- self.assertNotEqual([], patch.mock_calls)
300- # Reset the calls list for the actual test.
301- patch.mock_calls = []
302+ self.patch(settings, "DHCP_CONNECT", True)
303+ self.patch(tasks, 'write_dhcp_config')
304
305 foreign_dhcp_ip = factory.getRandomIPAddress()
306 form = NodeGroupInterfaceForeignDHCPForm(
307@@ -791,7 +779,7 @@
308 form.save()
309 self.assertEqual(
310 foreign_dhcp_ip, reload_object(interface).foreign_dhcp_ip)
311- self.assertEqual([], patch.mock_calls)
312+ tasks.write_dhcp_config.apply_async.assert_has_calls([])
313
314
315 class TestValidateNonoverlappingNetworks(TestCase):
316
317=== modified file 'src/provisioningserver/tasks.py'
318--- src/provisioningserver/tasks.py 2014-02-07 13:24:56 +0000
319+++ src/provisioningserver/tasks.py 2014-02-25 10:05:30 +0000
320@@ -19,6 +19,7 @@
321 'rndc_command',
322 'setup_rndc_configuration',
323 'restart_dhcp_server',
324+ 'stop_dhcp_server',
325 'write_dhcp_config',
326 'write_dns_config',
327 'write_dns_zone_config',
328@@ -346,6 +347,12 @@
329
330
331 @task
332+def stop_dhcp_server():
333+ """Stop a DHCP server."""
334+ call_and_check(['sudo', '-n', 'service', 'maas-dhcp-server', 'stop'])
335+
336+
337+@task
338 def periodic_probe_dhcp():
339 """Probe for foreign DHCP servers."""
340 detect.periodic_probe_task()
341
342=== modified file 'src/provisioningserver/tests/test_tasks.py'
343--- src/provisioningserver/tests/test_tasks.py 2014-02-13 11:16:21 +0000
344+++ src/provisioningserver/tests/test_tasks.py 2014-02-25 10:05:30 +0000
345@@ -79,6 +79,7 @@
346 rndc_command,
347 RNDC_COMMAND_MAX_RETRY,
348 setup_rndc_configuration,
349+ stop_dhcp_server,
350 update_node_tags,
351 UPDATE_NODE_TAGS_MAX_RETRY,
352 write_dhcp_config,
353@@ -315,12 +316,16 @@
354 celery_config.DHCP_INTERFACES_FILE, "--mode", "0644"], stdin=PIPE)
355
356 def test_restart_dhcp_server_sends_command(self):
357- recorder = FakeMethod()
358- self.patch(tasks, 'call_and_check', recorder)
359+ self.patch(tasks, 'call_and_check')
360 restart_dhcp_server()
361- self.assertEqual(
362- (1, (['sudo', '-n', 'service', 'maas-dhcp-server', 'restart'],)),
363- (recorder.call_count, recorder.extract_args()[0]))
364+ tasks.call_and_check.assert_called_once_with(
365+ ['sudo', '-n', 'service', 'maas-dhcp-server', 'restart'])
366+
367+ def test_stop_dhcp_server_sends_command(self):
368+ self.patch(tasks, 'call_and_check')
369+ stop_dhcp_server()
370+ tasks.call_and_check.assert_called_once_with(
371+ ['sudo', '-n', 'service', 'maas-dhcp-server', 'stop'])
372
373
374 def assertTaskRetried(runner, result, nb_retries, task_name):