launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11166
[Merge] lp:~jtv/maas/dhcp-switch into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/dhcp-switch into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/dhcp-switch/+merge/120542
Discussed, in bits and pieces, with Julian. Configuration item, form entry, and logic for the global switch between managed or manual DHCP. (This is not the same as enabling/disabling DHCP but rather controls whether we register host maps with the DHCP server when new machines acquire leases, hence the difference in naming compared to enable_dns).
The default is to disable DHCP management. It's the safe setting, even if it's not the full-auto-rich mode that we're hoping people will use.
A node group's is_dhcp_enabled method will return False when DHCP management is disabled, or (naturally) when full DHCP configuration is not available for the node group. Future multi-worker configurations will also want DHCP management disabled on the master worker, but we have yet to look at how to implement that.
For comfortable symmetry with our DNS code in maasserver, and to avoid future module bloat somewhere where the code fits poorly, I gave the DHCP switch a module of its own.
You'll also see an update to a test helper that previously sabotaged a node group's DHCP management by clearing its DHCP settings. It now goes the more comfortable route of setting the DHCP management option to False.
Jeroen
--
https://code.launchpad.net/~jtv/maas/dhcp-switch/+merge/120542
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/dhcp-switch into lp:maas.
=== added file 'src/maasserver/dhcp.py'
--- src/maasserver/dhcp.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/dhcp.py 2012-08-21 11:15:37 +0000
@@ -0,0 +1,25 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""DHCP management module."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'is_dhcp_management_enabled',
+ ]
+
+from maasserver.models import Config
+
+
+def is_dhcp_management_enabled():
+ """Is MAAS configured to manage DHCP?
+
+ This status is controlled by the `manage_dhcp` configuration item.
+ """
+ return Config.objects.get_config('manage_dhcp')
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-08-10 14:41:34 +0000
+++ src/maasserver/forms.py 2012-08-21 11:15:37 +0000
@@ -522,8 +522,14 @@
enable_dns = forms.BooleanField(
label="Enable DNS", required=False, help_text=(
"When enabled, MAAS will use the machine's BIND server to "
- "publish its DNS zones."
- ))
+ "publish its DNS zones."))
+ manage_dhcp = forms.BooleanField(
+ label="Manage DHCP", required=False, help_text=(
+ "When enabled, MAAS workers will work with the DHCP server(s) "
+ "to give each DHCP client its own host map. Unlike normal "
+ "leases, these host maps never expire. Thus enabling DHCP "
+ "management ensures that a node will never change its "
+ "IP address."))
class CommissioningForm(ConfigForm):
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py 2012-08-06 12:15:06 +0000
+++ src/maasserver/models/config.py 2012-08-21 11:15:37 +0000
@@ -48,8 +48,8 @@
# Network section configuration.
'maas_name': gethostname(),
'enlistment_domain': b'local',
- # DNS config.
'enable_dns': True,
+ 'manage_dhcp': False,
## /settings
}
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-08-21 06:42:14 +0000
+++ src/maasserver/models/nodegroup.py 2012-08-21 11:15:37 +0000
@@ -22,6 +22,7 @@
Manager,
)
from maasserver import DefaultMeta
+from maasserver.dhcp import is_dhcp_management_enabled
from maasserver.models.timestampedmodel import TimestampedModel
from piston.models import (
KEY_SIZE,
@@ -143,7 +144,7 @@
def is_dhcp_enabled(self):
"""Is the DHCP for this nodegroup enabled?"""
- return all([
+ return is_dhcp_management_enabled() and all([
self.subnet_mask,
self.broadcast_ip,
self.ip_range_low,
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-08-21 06:42:14 +0000
+++ src/maasserver/tests/test_api.py 2012-08-21 11:15:37 +0000
@@ -2443,14 +2443,9 @@
get_worker_user(), token=nodegroup.api_token)
-def disable_dhcp_management(nodegroup):
- """Turn off MAAS DHCP management on `nodegroup`."""
- nodegroup.subnet_mask = None
- nodegroup.broadcast_ip = None
- nodegroup.router_ip = None
- nodegroup.ip_range_low = None
- nodegroup.ip_range_high = None
- nodegroup.save()
+def enable_dhcp_management(enabled=True):
+ """Turn MAAS DHCP management on, or off."""
+ Config.objects.set_config('manage_dhcp', enabled)
class TestNodeGroupAPI(APITestCase):
@@ -2479,6 +2474,7 @@
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_update_leases_processes_empty_leases_dict(self):
+ enable_dhcp_management()
nodegroup = factory.make_node_group()
factory.make_dhcp_lease(nodegroup=nodegroup)
client = make_worker_client(nodegroup)
@@ -2513,8 +2509,8 @@
for lease in DHCPLease.objects.filter(nodegroup=nodegroup)])
def test_update_leases_stores_leases_even_if_not_managing_dhcp(self):
+ enable_dhcp_management(False)
nodegroup = factory.make_node_group()
- disable_dhcp_management(nodegroup)
lease = factory.make_random_leases()
client = make_worker_client(nodegroup)
response = client.post(
@@ -2531,6 +2527,7 @@
for lease in DHCPLease.objects.filter(nodegroup=nodegroup)])
def test_update_leases_adds_new_leases_on_worker(self):
+ enable_dhcp_management()
nodegroup = factory.make_node_group()
client = make_worker_client(nodegroup)
self.patch(Omshell, 'create', FakeMethod())
@@ -2550,6 +2547,7 @@
Omshell.create.extract_args())
def test_update_leases_does_not_add_old_leases(self):
+ enable_dhcp_management()
nodegroup = factory.make_node_group()
client = make_worker_client(nodegroup)
self.patch(tasks, 'add_new_dhcp_host_map', FakeMethod())
=== added file 'src/maasserver/tests/test_dhcp.py'
--- src/maasserver/tests/test_dhcp.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_dhcp.py 2012-08-21 11:15:37 +0000
@@ -0,0 +1,27 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for DHCP management."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver.dhcp import is_dhcp_management_enabled
+from maasserver.models import Config
+from maastesting.testcase import TestCase
+
+
+class TestDHCPManagement(TestCase):
+
+ def test_is_dhcp_management_enabled_defaults_to_False(self):
+ self.assertFalse(is_dhcp_management_enabled())
+
+ def test_is_dhcp_management_enabled_follows_manage_dhcp_config(self):
+ Config.objects.set_config('manage_dhcp', True)
+ self.assertTrue(is_dhcp_management_enabled())
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-08-21 06:42:14 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-08-21 11:15:37 +0000
@@ -12,7 +12,10 @@
__metaclass__ = type
__all__ = []
-from maasserver.models import NodeGroup
+from maasserver.models import (
+ Config,
+ NodeGroup,
+ )
from maasserver.testing import reload_object
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
@@ -159,27 +162,28 @@
('celery', FixtureResource(CeleryFixture())),
)
- def test_is_dhcp_enabled_false_if_one_element_is_none(self):
+ def test_is_dhcp_enabled_returns_True_if_fully_set_up(self):
+ Config.objects.set_config('manage_dhcp', True)
+ self.assertTrue(factory.make_node_group().is_dhcp_enabled())
+
+ def test_is_dhcp_enabled_returns_False_if_disabled(self):
+ Config.objects.set_config('manage_dhcp', False)
+ self.assertFalse(factory.make_node_group().is_dhcp_enabled())
+
+ def test_is_dhcp_enabled_returns_False_if_config_is_missing(self):
+ Config.objects.set_config('manage_dhcp', True)
required_fields = [
'subnet_mask', 'broadcast_ip', 'ip_range_low', 'ip_range_high']
- nodegroups = []
- for required_field in required_fields:
- nodegroup = factory.make_node_group()
- setattr(nodegroup, required_field, None)
+ nodegroups = {
+ field: factory.make_node_group()
+ for field in required_fields}
+ for field, nodegroup in nodegroups.items():
+ setattr(nodegroup, field, None)
nodegroup.save()
- nodegroups.append(nodegroup)
- self.assertEquals(
- [nodegroup.is_dhcp_enabled() for nodegroup in nodegroups],
- [False] * len(nodegroups))
-
- def test_is_dhcp_enabled_true_if_all_the_elements_defined(self):
- nodegroup = factory.make_node_group(
- subnet_mask=factory.getRandomIPAddress(),
- broadcast_ip=factory.getRandomIPAddress(),
- ip_range_low=factory.getRandomIPAddress(),
- ip_range_high=factory.getRandomIPAddress(),
- )
- self.assertTrue(nodegroup.is_dhcp_enabled())
+ self.assertEqual([], [
+ field
+ for field, nodegroup in nodegroups.items()
+ if nodegroup.is_dhcp_enabled()])
def test_set_up_dhcp_writes_dhcp_config(self):
conf_file = self.make_file(contents=factory.getRandomString())
=== modified file 'src/maasserver/tests/test_views_settings.py'
--- src/maasserver/tests/test_views_settings.py 2012-07-27 14:52:44 +0000
+++ src/maasserver/tests/test_views_settings.py 2012-08-21 11:15:37 +0000
@@ -79,6 +79,7 @@
new_name = factory.getRandomString()
new_domain = factory.getRandomString()
new_enable_dns = factory.getRandomBoolean()
+ new_manage_dhcp = factory.getRandomBoolean()
response = self.client.post(
reverse('settings'),
get_prefixed_form_data(
@@ -87,6 +88,7 @@
'maas_name': new_name,
'enlistment_domain': new_domain,
'enable_dns ': new_enable_dns,
+ 'manage_dhcp': new_manage_dhcp,
}))
self.assertEqual(httplib.FOUND, response.status_code)
@@ -95,6 +97,8 @@
new_domain, Config.objects.get_config('enlistment_domain'))
self.assertEqual(
new_enable_dns, Config.objects.get_config('enable_dns'))
+ self.assertEqual(
+ new_manage_dhcp, Config.objects.get_config('manage_dhcp'))
def test_settings_commissioning_POST(self):
new_after_commissioning = factory.getRandomEnum(
