launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/enable_dhcp_when_setting_up_params into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1044229 in MAAS: "DHCP config doesn't get written unless an inhuman combination of scripts is run"
  https://bugs.launchpad.net/maas/+bug/1044229

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/enable_dhcp_when_setting_up_params/+merge/122613
-- 
https://code.launchpad.net/~julian-edwards/maas/enable_dhcp_when_setting_up_params/+merge/122613
Your team MAAS Maintainers is requested to review the proposed merge of lp:~julian-edwards/maas/enable_dhcp_when_setting_up_params into lp:maas.

=== 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 05:54: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 05:54: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,9 @@
     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)
         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'))