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

[Julian Edwards]

Implicitly enable DHCP management when using the command "maas config-master-dhcp" to set up the DHCP parameters.

This will cause the DHCPD config file to also get written, which is reasonable behaviour given that someone is calling a command to set up DHCP parameters.

[Jeroen T. Vermeulen (jtv)]

Makes sense. One small thing: in that last test, it may help to disable manage_dhcp explicitly. If we ever decide to change the default, or there's another test isolation problem, it'll be good to know that the test isn't passing for the wrong reason.

Jeroen

review: Approve

[Julian Edwards (julian-edwards)]

On Tuesday 04 September 2012 06:10:44 you wrote:
> Review: Approve
>
> Makes sense. One small thing: in that last test, it may help to disable
> manage_dhcp explicitly. If we ever decide to change the default, or
> there's another test isolation problem, it'll be good to know that the test
> isn't passing for the wrong reason.
>
>
> Jeroen

Good call, thanks.

=== modified file 'src/maasserver/management/commands/config_master_dhcp.py'
--- src/maasserver/management/commands/config_master_dhcp.py	2012-09-04 04:47:28 +0000
+++ src/maasserver/management/commands/config_master_dhcp.py	2012-09-04 10:20:24 +0000
@@ -106,7 +106,7 @@
                 setattr(master_nodegroup, item, value)
             master_nodegroup.save()
 
-            # If DHCP management is enabled, create a Task that will
+            # Enable DHCP management and create a Task that will
             # write the config out.
-            if Config.objects.get_config('manage_dhcp'):
-                master_nodegroup.set_up_dhcp()
+            Config.objects.set_config('manage_dhcp', True)
+            master_nodegroup.set_up_dhcp()

=== modified file 'src/maasserver/tests/test_commands_config_master_dhcp.py'
--- src/maasserver/tests/test_commands_config_master_dhcp.py	2012-09-04 04:47:28 +0000
+++ src/maasserver/tests/test_commands_config_master_dhcp.py	2012-09-04 10:20:24 +0000
@@ -58,12 +58,8 @@
 
     def setUp(self):
         super(TestConfigMasterDHCP, self).setUp()
-        # Make sure any attempts to write a dhcp config end up in a temp
-        # file rather than the system one.
-        conf_file = self.make_file()
-        self.patch(tasks, "DHCP_CONFIG_FILE", conf_file)
-        # Prevent DHCPD restarts.
-        self.patch(tasks, 'check_call', Mock())
+        # Make sure any attempts to write a dhcp config do nothing.
+        self.patch(NodeGroup, 'set_up_dhcp')
 
     def test_configures_dhcp_for_master_nodegroup(self):
         settings = make_dhcp_settings()
@@ -133,18 +129,10 @@
     def test_name_option_turns_dhcp_setting_name_into_option(self):
         self.assertEqual('--subnet-mask', name_option('subnet_mask'))
 
-    def test_sets_up_dhcp_if_dhcp_enabled(self):
+    def test_sets_up_dhcp_and_enables_it(self):
         master = NodeGroup.objects.ensure_master()
-        self.patch(NodeGroup, 'set_up_dhcp', Mock())
         settings = make_dhcp_settings()
-        Config.objects.set_config('manage_dhcp', True)
+        Config.objects.set_config('manage_dhcp', False)
         call_command('config_master_dhcp', **settings)
         self.assertEqual(1, master.set_up_dhcp.call_count)
-
-    def test_ignores_set_up_dhcp_if_dhcp_disabled(self):
-        master = NodeGroup.objects.ensure_master()
-        self.patch(NodeGroup, 'set_up_dhcp', Mock())
-        settings = make_dhcp_settings()
-        Config.objects.set_config('manage_dhcp', False)
-        call_command('config_master_dhcp', **settings)
-        self.assertEqual(0, master.set_up_dhcp.call_count)
+        self.assertTrue(Config.objects.get_config('manage_dhcp'))