Merge lp:~rvb/maas/no-dhcp-write-of-unvalid-config-1.5 into lp:maas/1.5

Proposed by Raphaël Badin on 2014-06-02
Status: Merged
Approved by: Raphaël Badin on 2014-06-02
Approved revision: 2281
Merged at revision: 2281
Proposed branch: lp:~rvb/maas/no-dhcp-write-of-unvalid-config-1.5
Merge into: lp:maas/1.5
Diff against target: 131 lines (+36/-17)
4 files modified
src/maasserver/dhcp.py (+10/-1)
src/maasserver/tests/test_dhcp.py (+13/-14)
src/provisioningserver/tasks.py (+9/-1)
src/provisioningserver/tests/test_tasks.py (+4/-1)
To merge this branch: bzr merge lp:~rvb/maas/no-dhcp-write-of-unvalid-config-1.5
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve on 2014-06-02
Review via email: mp+221675@code.launchpad.net

Commit message

Backport revision 2389: Return early and stop the DHCP server when the list of managed interfaces of the nodegroup is empty.

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Simple backport, self-approving.

review: Approve

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-02-24 09:30:57 +0000
3+++ src/maasserver/dhcp.py 2014-06-02 08:00:52 +0000
4@@ -22,6 +22,7 @@
5 from netaddr import IPAddress
6 from provisioningserver.tasks import (
7 restart_dhcp_server,
8+ stop_dhcp_server,
9 write_dhcp_config,
10 )
11
12@@ -52,7 +53,15 @@
13 from maasserver.dns import get_dns_server_address
14
15 interfaces = get_interfaces_managed_by(nodegroup)
16- if interfaces is None:
17+ if interfaces in [None, []]:
18+ # interfaces being None means the cluster isn't accepted: stop
19+ # the DHCP server in case it case started.
20+ # interfaces being [] means there is no interface configured: stop
21+ # the DHCP server; Note that a config generated with this setup
22+ # would not be valid and would result in the DHCP
23+ # server failing with the error: "Not configured to listen on any
24+ # interfaces!."
25+ stop_dhcp_server.apply_async(queue=nodegroup.work_queue)
26 return
27
28 # Make sure this nodegroup has a key to communicate with the dhcp
29
30=== modified file 'src/maasserver/tests/test_dhcp.py'
31--- src/maasserver/tests/test_dhcp.py 2014-02-24 09:30:57 +0000
32+++ src/maasserver/tests/test_dhcp.py 2014-06-02 08:00:52 +0000
33@@ -33,7 +33,6 @@
34 from maasserver.testing.testcase import MAASServerTestCase
35 from maasserver.utils import map_enum
36 from maastesting.celery import CeleryFixture
37-from mock import ANY
38 from netaddr import (
39 IPAddress,
40 IPNetwork,
41@@ -72,6 +71,16 @@
42 {status: None for status in unaccepted_statuses},
43 managed_interfaces)
44
45+ def test_configure_dhcp_stops_server_if_no_managed_interface(self):
46+ self.patch(settings, "DHCP_CONNECT", True)
47+ self.patch(dhcp, 'stop_dhcp_server')
48+ nodegroup = factory.make_node_group(
49+ status=NODEGROUP_STATUS.ACCEPTED,
50+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
51+ )
52+ configure_dhcp(nodegroup)
53+ self.assertEqual(1, dhcp.stop_dhcp_server.apply_async.call_count)
54+
55 def test_configure_dhcp_obeys_DHCP_CONNECT(self):
56 self.patch(settings, "DHCP_CONNECT", False)
57 self.patch(dhcp, 'write_dhcp_config')
58@@ -205,19 +214,6 @@
59 args, kwargs = task.subtask.call_args
60 self.assertEqual(nodegroup.work_queue, kwargs['options']['queue'])
61
62- def test_write_dhcp_config_called_when_no_managed_interfaces(self):
63- nodegroup = factory.make_node_group(
64- status=NODEGROUP_STATUS.ACCEPTED,
65- management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
66- [interface] = nodegroup.nodegroupinterface_set.all()
67- self.patch(settings, "DHCP_CONNECT", True)
68- self.patch(tasks, 'sudo_write_file')
69- self.patch(dhcp, 'write_dhcp_config')
70- interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
71- interface.save()
72- dhcp.write_dhcp_config.apply_async.assert_called_once_with(
73- queue=nodegroup.work_queue, kwargs=ANY)
74-
75 def test_dhcp_config_gets_written_when_interface_IP_changes(self):
76 nodegroup = factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
77 [interface] = nodegroup.nodegroupinterface_set.all()
78@@ -318,6 +314,9 @@
79 factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
80 for x in range(num_inactive_nodegroups):
81 factory.make_node_group(status=NODEGROUP_STATUS.PENDING)
82+ # Silence stop_dhcp_server: it will be called for the inactive
83+ # nodegroups.
84+ self.patch(dhcp, 'stop_dhcp_server')
85
86 self.patch(settings, "DHCP_CONNECT", True)
87 self.patch(dhcp, 'write_dhcp_config')
88
89=== modified file 'src/provisioningserver/tasks.py'
90--- src/provisioningserver/tasks.py 2014-05-21 17:30:26 +0000
91+++ src/provisioningserver/tasks.py 2014-06-02 08:00:52 +0000
92@@ -385,10 +385,18 @@
93 call_and_check(['sudo', '-n', 'service', 'maas-dhcp-server', 'restart'])
94
95
96+# Message to put in the DHCP config file when the DHCP server gets stopped.
97+DISABLED_DHCP_SERVER = "# DHCP server stopped."
98+
99+
100 @task
101 @log_exception_text
102 def stop_dhcp_server():
103- """Stop a DHCP server."""
104+ """Write a blank config file and stop a DHCP server."""
105+ # Write an empty config file to avoid having an outdated config laying
106+ # around.
107+ sudo_write_file(
108+ celery_config.DHCP_CONFIG_FILE, DISABLED_DHCP_SERVER)
109 call_and_check(['sudo', '-n', 'service', 'maas-dhcp-server', 'stop'])
110
111
112
113=== modified file 'src/provisioningserver/tests/test_tasks.py'
114--- src/provisioningserver/tests/test_tasks.py 2014-05-06 21:18:17 +0000
115+++ src/provisioningserver/tests/test_tasks.py 2014-06-02 08:00:52 +0000
116@@ -330,11 +330,14 @@
117 self.assertThat(tasks.call_and_check, MockCalledOnceWith(
118 ['sudo', '-n', 'service', 'maas-dhcp-server', 'restart']))
119
120- def test_stop_dhcp_server_sends_command(self):
121+ def test_stop_dhcp_server_sends_command_and_writes_empty_config(self):
122 self.patch(tasks, 'call_and_check')
123+ self.patch(tasks, 'sudo_write_file')
124 stop_dhcp_server()
125 self.assertThat(tasks.call_and_check, MockCalledOnceWith(
126 ['sudo', '-n', 'service', 'maas-dhcp-server', 'stop']))
127+ self.assertThat(tasks.sudo_write_file, MockCalledOnceWith(
128+ celery_config.DHCP_CONFIG_FILE, tasks.DISABLED_DHCP_SERVER))
129
130
131 def assertTaskRetried(runner, result, nb_retries, task_name):

Subscribers

People subscribed via source and target branches

to all changes: