launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/remove-config-dhcp-master into lp:maas.

Commit message:
This branch removes the config_dhcp_master that is not used any more.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/remove-config-dhcp-master/+merge/129678

This branch removes the config_dhcp_master that is not used any more (the usage in prod has been removed ~1 week ago).  Now, all the DHCP config is done via the UI/API/CLI by configuring the cluster interfaces.
-- 
https://code.launchpad.net/~rvb/maas/remove-config-dhcp-master/+merge/129678
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/remove-config-dhcp-master into lp:maas.

=== modified file 'Makefile'
--- Makefile	2012-10-04 23:31:12 +0000
+++ Makefile	2012-10-15 13:55:43 +0000
@@ -175,7 +175,6 @@
 	$(dbrun) bin/maas syncdb --noinput
 	$(dbrun) bin/maas migrate maasserver --noinput
 	$(dbrun) bin/maas migrate metadataserver --noinput
-	$(dbrun) bin/maas config_master_dhcp --ensure
 
 define phony_targets
   build

=== removed file 'src/maasserver/management/commands/config_master_dhcp.py'
--- src/maasserver/management/commands/config_master_dhcp.py	2012-09-19 13:48:42 +0000
+++ src/maasserver/management/commands/config_master_dhcp.py	1970-01-01 00:00:00 +0000
@@ -1,107 +0,0 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Django command: Configure master DHCP.
-
-The master DHCP settings apply to the DHCP server running on the MAAS server
-itself.  They can be either disabled (if you don't want MAAS to manage DHCP)
-or fully configured using this command.
-"""
-
-from __future__ import (
-    absolute_import,
-    print_function,
-    unicode_literals,
-    )
-
-__metaclass__ = type
-__all__ = [
-    'Command',
-    ]
-
-from optparse import (
-    make_option,
-    OptionValueError,
-    )
-
-from django.core.management.base import BaseCommand
-from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
-from maasserver.models import (
-    NodeGroup,
-    NodeGroupInterface,
-    )
-
-
-dhcp_items = {
-    'interface': "The network interface that should service DHCP requests.",
-    'ip': "The IP address at which nodes can reach the DHCP server.",
-    'subnet_mask': "Subnet mask, e.g. 255.0.0.0.",
-    'broadcast_ip': "Broadcast address for this subnet, e.g. 10.255.255.255.",
-    'router_ip': "Address of default gateway.",
-    'ip_range_low': "Lowest IP address to assign to clients.",
-    'ip_range_high': "Highest IP address to assign to clients.",
-    }
-
-
-def get_settings(options):
-    """Get the DHCP settings from `options`, as a dict.
-
-    Checks validity of the settings.
-    """
-    settings = {
-        item: options.get(item)
-        for item in dhcp_items}
-    # All of the DHCP settings must have (non-empty) values.
-    if not all(settings.values()):
-        raise OptionValueError(
-            "Specify all DHCP settings: %s" % ', '.join(dhcp_items))
-    return settings
-
-
-def name_option(dhcp_setting):
-    """Formulate the option name corresponding to a DHCP setting name."""
-    return '--%s' % dhcp_setting.replace('_', '-')
-
-
-class Command(BaseCommand):
-
-    option_list = BaseCommand.option_list + (
-        make_option(
-            '--clear', dest='clear', action='store_true', default=False,
-            help=(
-                "Clear settings.  Do only when MAAS DHCP is disabled.  "
-                "If given, any DHCP parameters are ignored.")),
-        make_option(
-            '--ensure', dest='ensure', action='store_true', default=False,
-            help=(
-                "Ensure that the master node group is configured, "
-                "but if it was already set up, don't change its settings.  "
-                "If given, any DHCP parameters are ignored.")),
-        ) + tuple(
-            make_option(
-                name_option(item), dest=item, default=None,
-                help="DHCP parameter: %s" % help)
-            for item, help in dhcp_items.items())
-    help = "Initialize master DHCP settings."
-
-    def handle(self, *args, **options):
-        if options.get('ensure') and options.get('clear'):
-            raise OptionValueError(
-                "The --ensure option conflicts with --clear.")
-        master_nodegroup = NodeGroup.objects.ensure_master()
-        if not options.get('ensure'):
-            if options.get('clear'):
-                master_nodegroup.nodegroupinterface_set.all().delete()
-                return
-
-            settings = get_settings(options)
-            interface = master_nodegroup.get_managed_interface()
-            if interface is None:
-                interface = NodeGroupInterface(
-                    nodegroup=master_nodegroup,
-                    management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
-            # That kind of manipulation should really be done via a form
-            # rather than with 'setattr'.
-            for item, value in settings.items():
-                setattr(interface, item, value)
-            interface.save()

=== removed file 'src/maasserver/tests/test_commands_config_master_dhcp.py'
--- src/maasserver/tests/test_commands_config_master_dhcp.py	2012-09-30 20:58:03 +0000
+++ src/maasserver/tests/test_commands_config_master_dhcp.py	1970-01-01 00:00:00 +0000
@@ -1,144 +0,0 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for the config_master_dhcp command."""
-
-from __future__ import (
-    absolute_import,
-    print_function,
-    unicode_literals,
-    )
-
-__metaclass__ = type
-__all__ = []
-
-from optparse import OptionValueError
-
-from django.conf import settings
-from django.core.management import call_command
-from maasserver import dhcp
-from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
-from maasserver.management.commands.config_master_dhcp import name_option
-from maasserver.models import NodeGroup
-from maasserver.testing.factory import factory
-from maasserver.testing.testcase import TestCase
-from testtools.matchers import MatchesStructure
-
-
-def make_master_constants():
-    """Return the standard, unchanging config for the master nodegroup."""
-    return {
-        'name': 'master',
-    }
-
-
-def make_dhcp_settings():
-    """Return an arbitrary dict of DHCP settings."""
-    network = factory.getRandomNetwork()
-    return {
-        'ip': factory.getRandomIPInNetwork(network),
-        'interface': factory.make_name('interface'),
-        'subnet_mask': str(network.netmask),
-        'broadcast_ip': str(network.broadcast),
-        'router_ip': factory.getRandomIPInNetwork(network),
-        'ip_range_low': factory.getRandomIPInNetwork(network),
-        'ip_range_high': factory.getRandomIPInNetwork(network),
-    }
-
-
-def make_cleared_dhcp_settings():
-    """Return dict of cleared DHCP settings."""
-    return dict.fromkeys(make_dhcp_settings())
-
-
-class TestConfigMasterDHCP(TestCase):
-
-    def setUp(self):
-        super(TestConfigMasterDHCP, self).setUp()
-        # Make sure any attempts to write a dhcp config do nothing.
-        self.patch(dhcp, 'configure_dhcp')
-        self.patch(settings, 'DHCP_CONNECT', True)
-
-    def test_configures_dhcp_for_master_nodegroup(self):
-        settings = make_dhcp_settings()
-        call_command('config_master_dhcp', **settings)
-        master = NodeGroup.objects.get(name='master')
-        interface = master.get_managed_interface()
-        self.assertThat(
-            master,
-            MatchesStructure.byEquality(**make_master_constants()))
-        self.assertThat(
-            interface, MatchesStructure.byEquality(
-                management=NODEGROUPINTERFACE_MANAGEMENT.DHCP, **settings))
-
-    def test_configures_dhcp_for_master_nodegroup_existing_master(self):
-        management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
-        master = factory.make_node_group(uuid='master', management=management)
-        settings = make_dhcp_settings()
-        call_command('config_master_dhcp', **settings)
-        master = NodeGroup.objects.ensure_master()
-        interface = master.get_managed_interface()
-        self.assertThat(
-            interface, MatchesStructure.byEquality(
-                management=interface.management, **settings))
-
-    def test_clears_dhcp_settings(self):
-        master = NodeGroup.objects.ensure_master()
-        for attribute, value in make_dhcp_settings().items():
-            setattr(master, attribute, value)
-        master.save()
-        call_command('config_master_dhcp', clear=True)
-        self.assertThat(
-            master,
-            MatchesStructure.byEquality(**make_master_constants()))
-        self.assertIsNone(master.get_managed_interface())
-
-    def test_does_not_accept_partial_dhcp_settings(self):
-        settings = make_dhcp_settings()
-        del settings['subnet_mask']
-        self.assertRaises(
-            OptionValueError,
-            call_command, 'config_master_dhcp', **settings)
-
-    def test_ignores_nonsense_settings_when_clear_is_passed(self):
-        settings = make_dhcp_settings()
-        call_command('config_master_dhcp', **settings)
-        settings['subnet_mask'] = '@%$^&'
-        settings['broadcast_ip'] = ''
-        call_command('config_master_dhcp', clear=True, **settings)
-        master = NodeGroup.objects.get(name='master')
-        self.assertIsNone(master.get_managed_interface())
-
-    def test_clear_conflicts_with_ensure(self):
-        self.assertRaises(
-            OptionValueError,
-            call_command, 'config_master_dhcp', clear=True, ensure=True)
-
-    def test_ensure_creates_master_nodegroup_without_dhcp_settings(self):
-        call_command('config_master_dhcp', ensure=True)
-        self.assertIsNone(
-            NodeGroup.objects.get(name='master').get_managed_interface())
-
-    def test_ensure_leaves_cleared_settings_cleared(self):
-        call_command('config_master_dhcp', clear=True)
-        call_command('config_master_dhcp', ensure=True)
-        master = NodeGroup.objects.get(name='master')
-        self.assertIsNone(master.get_managed_interface())
-
-    def test_ensure_leaves_dhcp_settings_intact(self):
-        settings = make_dhcp_settings()
-        call_command('config_master_dhcp', **settings)
-        call_command('config_master_dhcp', ensure=True)
-        self.assertThat(
-            NodeGroup.objects.get(name='master').get_managed_interface(),
-            MatchesStructure.byEquality(**settings))
-
-    def test_name_option_turns_dhcp_setting_name_into_option(self):
-        self.assertEqual('--subnet-mask', name_option('subnet_mask'))
-
-    def test_configures_dhcp(self):
-        NodeGroup.objects.ensure_master()
-        self.patch(dhcp, 'configure_dhcp')
-        settings = make_dhcp_settings()
-        call_command('config_master_dhcp', **settings)
-        self.assertEqual(1, dhcp.configure_dhcp.call_count)