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

[Raphaël Badin]

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

[Raphaël Badin (rvb)]

Simple backport, self-approving.

review: Approve

=== modified file 'src/maasserver/dhcp.py'
--- src/maasserver/dhcp.py	2014-02-24 09:30:57 +0000
+++ src/maasserver/dhcp.py	2014-06-02 08:00:52 +0000
@@ -22,6 +22,7 @@
 from netaddr import IPAddress
 from provisioningserver.tasks import (
     restart_dhcp_server,
+    stop_dhcp_server,
     write_dhcp_config,
     )
 
@@ -52,7 +53,15 @@
     from maasserver.dns import get_dns_server_address
 
     interfaces = get_interfaces_managed_by(nodegroup)
-    if interfaces is None:
+    if interfaces in [None, []]:
+        # interfaces being None means the cluster isn't accepted: stop
+        # the DHCP server in case it case started.
+        # interfaces being [] means there is no interface configured: stop
+        # the DHCP server;  Note that a config generated with this setup
+        # would not be valid and would result in the DHCP
+        # server failing with the error: "Not configured to listen on any
+        # interfaces!."
+        stop_dhcp_server.apply_async(queue=nodegroup.work_queue)
         return
 
     # Make sure this nodegroup has a key to communicate with the dhcp

=== modified file 'src/maasserver/tests/test_dhcp.py'
--- src/maasserver/tests/test_dhcp.py	2014-02-24 09:30:57 +0000
+++ src/maasserver/tests/test_dhcp.py	2014-06-02 08:00:52 +0000
@@ -33,7 +33,6 @@
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import map_enum
 from maastesting.celery import CeleryFixture
-from mock import ANY
 from netaddr import (
     IPAddress,
     IPNetwork,
@@ -72,6 +71,16 @@
             {status: None for status in unaccepted_statuses},
             managed_interfaces)
 
+    def test_configure_dhcp_stops_server_if_no_managed_interface(self):
+        self.patch(settings, "DHCP_CONNECT", True)
+        self.patch(dhcp, 'stop_dhcp_server')
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
+            )
+        configure_dhcp(nodegroup)
+        self.assertEqual(1, dhcp.stop_dhcp_server.apply_async.call_count)
+
     def test_configure_dhcp_obeys_DHCP_CONNECT(self):
         self.patch(settings, "DHCP_CONNECT", False)
         self.patch(dhcp, 'write_dhcp_config')
@@ -205,19 +214,6 @@
         args, kwargs = task.subtask.call_args
         self.assertEqual(nodegroup.work_queue, kwargs['options']['queue'])
 
-    def test_write_dhcp_config_called_when_no_managed_interfaces(self):
-        nodegroup = factory.make_node_group(
-            status=NODEGROUP_STATUS.ACCEPTED,
-            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
-        [interface] = nodegroup.nodegroupinterface_set.all()
-        self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(tasks, 'sudo_write_file')
-        self.patch(dhcp, 'write_dhcp_config')
-        interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
-        interface.save()
-        dhcp.write_dhcp_config.apply_async.assert_called_once_with(
-            queue=nodegroup.work_queue, kwargs=ANY)
-
     def test_dhcp_config_gets_written_when_interface_IP_changes(self):
         nodegroup = factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
         [interface] = nodegroup.nodegroupinterface_set.all()
@@ -318,6 +314,9 @@
             factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED)
         for x in range(num_inactive_nodegroups):
             factory.make_node_group(status=NODEGROUP_STATUS.PENDING)
+        # Silence stop_dhcp_server: it will be called for the inactive
+        # nodegroups.
+        self.patch(dhcp, 'stop_dhcp_server')
 
         self.patch(settings, "DHCP_CONNECT", True)
         self.patch(dhcp, 'write_dhcp_config')

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2014-05-21 17:30:26 +0000
+++ src/provisioningserver/tasks.py	2014-06-02 08:00:52 +0000
@@ -385,10 +385,18 @@
     call_and_check(['sudo', '-n', 'service', 'maas-dhcp-server', 'restart'])
 
 
+# Message to put in the DHCP config file when the DHCP server gets stopped.
+DISABLED_DHCP_SERVER = "# DHCP server stopped."
+
+
 @task
 @log_exception_text
 def stop_dhcp_server():
-    """Stop a DHCP server."""
+    """Write a blank config file and stop a DHCP server."""
+    # Write an empty config file to avoid having an outdated config laying
+    # around.
+    sudo_write_file(
+        celery_config.DHCP_CONFIG_FILE, DISABLED_DHCP_SERVER)
     call_and_check(['sudo', '-n', 'service', 'maas-dhcp-server', 'stop'])
 
 

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2014-05-06 21:18:17 +0000
+++ src/provisioningserver/tests/test_tasks.py	2014-06-02 08:00:52 +0000
@@ -330,11 +330,14 @@
         self.assertThat(tasks.call_and_check, MockCalledOnceWith(
             ['sudo', '-n', 'service', 'maas-dhcp-server', 'restart']))
 
-    def test_stop_dhcp_server_sends_command(self):
+    def test_stop_dhcp_server_sends_command_and_writes_empty_config(self):
         self.patch(tasks, 'call_and_check')
+        self.patch(tasks, 'sudo_write_file')
         stop_dhcp_server()
         self.assertThat(tasks.call_and_check, MockCalledOnceWith(
             ['sudo', '-n', 'service', 'maas-dhcp-server', 'stop']))
+        self.assertThat(tasks.sudo_write_file, MockCalledOnceWith(
+            celery_config.DHCP_CONFIG_FILE, tasks.DISABLED_DHCP_SERVER))
 
 
 def assertTaskRetried(runner, result, nb_retries, task_name):