launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10314
[Merge] lp:~rvb/maas/cope-empty-network-info into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/cope-empty-network-info into lp:maas with lp:~rvb/maas/add-ns-server2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/cope-empty-network-info/+merge/116889
This branch changes the DNS-related methods in src/maasserver/dns.py so that they now can cope with nodegroups with a disabled DHCP (i.e. a nodegroup missing one of the DHCP-related info).
= Pre-imp =
This was discussed with Jeroen
= Notes =
- I've added a missing test for 'get_zone'.
- the new tests in test_dns.py could have used assertions based on actual DNS checks using dig but since these are a little bit heavy, I decided to use FakeMethod instead.
--
https://code.launchpad.net/~rvb/maas/cope-empty-network-info/+merge/116889
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/cope-empty-network-info into lp:maas.
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py 2012-07-26 15:34:35 +0000
+++ src/maasserver/dns.py 2012-07-26 15:34:35 +0000
@@ -145,18 +145,25 @@
def get_zone(nodegroup, serial=None):
"""Create a :class:`DNSZoneConfig` object from a nodegroup.
+ Return a :class:`DNSZoneConfig` if DHCP is enabled on the
+ nodegroup or None if it is not the case.
+
This method also accepts a serial to reuse the same serial when
we are creating DNSZoneConfig objects in bulk.
"""
- if serial is None:
- serial = next_zone_serial()
- dns_ip = get_dns_server_address()
- return DNSZoneConfig(
- zone_name=nodegroup.name, serial=serial, dns_ip=dns_ip,
- subnet_mask=nodegroup.subnet_mask, broadcast_ip=nodegroup.broadcast_ip,
- ip_range_low=nodegroup.ip_range_low,
- ip_range_high=nodegroup.ip_range_high,
- mapping=DHCPLease.objects.get_hostname_ip_mapping(nodegroup))
+ if nodegroup.is_dhcp_enabled():
+ if serial is None:
+ serial = next_zone_serial()
+ dns_ip = get_dns_server_address()
+ return DNSZoneConfig(
+ zone_name=nodegroup.name, serial=serial, dns_ip=dns_ip,
+ subnet_mask=nodegroup.subnet_mask,
+ broadcast_ip=nodegroup.broadcast_ip,
+ ip_range_low=nodegroup.ip_range_low,
+ ip_range_high=nodegroup.ip_range_high,
+ mapping=DHCPLease.objects.get_hostname_ip_mapping(nodegroup))
+ else:
+ return None
def change_dns_zones(nodegroups):
@@ -171,13 +178,14 @@
serial = next_zone_serial()
for nodegroup in nodegroups:
zone = get_zone(nodegroup, serial)
- reverse_zone_reload_subtask = tasks.rndc_command.subtask(
- args=[['reload', zone.reverse_zone_name]])
- zone_reload_subtask = tasks.rndc_command.subtask(
- args=[['reload', zone.zone_name]],
- callback=reverse_zone_reload_subtask)
- tasks.write_dns_zone_config.delay(
- zone=zone, callback=zone_reload_subtask)
+ if zone is not None:
+ reverse_zone_reload_subtask = tasks.rndc_command.subtask(
+ args=[['reload', zone.reverse_zone_name]])
+ zone_reload_subtask = tasks.rndc_command.subtask(
+ args=[['reload', zone.zone_name]],
+ callback=reverse_zone_reload_subtask)
+ tasks.write_dns_zone_config.delay(
+ zone=zone, callback=zone_reload_subtask)
def add_zone(nodegroup):
@@ -190,24 +198,31 @@
:param nodegroup: The nodegroup for which the zone should be added.
:type nodegroup: :class:`NodeGroup`
"""
- serial = next_zone_serial()
- zones = [
- get_zone(nodegroup, serial)
- for nodegroup in NodeGroup.objects.all()]
- reconfig_subtask = tasks.rndc_command.subtask(args=[['reconfig']])
- write_dns_config_subtask = tasks.write_dns_config.subtask(
- zones=zones, callback=reconfig_subtask)
zone = get_zone(nodegroup)
- tasks.write_dns_zone_config.delay(
- zone=zone, callback=write_dns_config_subtask)
+ if zone is not None:
+ serial = next_zone_serial()
+ zones = filter(
+ None,
+ [
+ get_zone(nodegroup, serial)
+ for nodegroup in NodeGroup.objects.all()
+ ])
+ reconfig_subtask = tasks.rndc_command.subtask(args=[['reconfig']])
+ write_dns_config_subtask = tasks.write_dns_config.subtask(
+ zones=zones, callback=reconfig_subtask)
+ tasks.write_dns_zone_config.delay(
+ zone=zone, callback=write_dns_config_subtask)
def write_full_dns_config():
"""Write the DNS configuration for all the nodegroups."""
serial = next_zone_serial()
- zones = [
- get_zone(nodegroup, serial)
- for nodegroup in NodeGroup.objects.all()]
+ zones = filter(
+ None,
+ [
+ get_zone(nodegroup, serial)
+ for nodegroup in NodeGroup.objects.all()
+ ])
tasks.write_full_dns_config.delay(
zones=zones,
callback=tasks.rndc_command.subtask(args=[['reload']]))
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-07-26 09:11:26 +0000
+++ src/maasserver/models/nodegroup.py 2012-07-26 15:34:35 +0000
@@ -124,3 +124,9 @@
editable=True, unique=True, blank=True, null=True, default='')
ip_range_high = IPAddressField(
editable=True, unique=True, blank=True, null=True, default='')
+
+ def is_dhcp_enabled(self):
+ """Is the DHCP for this nodegroup enabled?"""
+ return all(
+ [self.subnet_mask, self.broadcast_ip, self.ip_range_low,
+ self.ip_range_high])
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-07-26 15:34:35 +0000
+++ src/maasserver/tests/test_dns.py 2012-07-26 15:34:35 +0000
@@ -14,6 +14,7 @@
from itertools import islice
+import random
import socket
from django.conf import settings
@@ -23,6 +24,7 @@
change_dns_zones,
DNSException,
get_dns_server_address,
+ get_zone,
is_dns_enabled,
next_zone_serial,
write_full_dns_config,
@@ -42,7 +44,10 @@
IPNetwork,
IPRange,
)
-from provisioningserver.dns.config import conf
+from provisioningserver.dns.config import (
+ conf,
+ DNSZoneConfig,
+ )
from provisioningserver.dns.utils import generated_hostname
from testresources import FixtureResource
from testtools.matchers import MatchesStructure
@@ -113,6 +118,28 @@
self.patch(settings, 'DEFAULT_MAAS_URL', url)
self.assertRaises(DNSException, get_dns_server_address)
+ def test_get_zone_creates_DNSZoneConfig(self):
+ nodegroup = factory.make_node_group()
+ serial = random.randint(1, 100)
+ zone = get_zone(nodegroup, serial)
+ self.assertAttributes(
+ zone,
+ dict(
+ zone_name=nodegroup.name,
+ serial=serial,
+ subnet_mask=nodegroup.subnet_mask,
+ broadcast_ip=nodegroup.broadcast_ip,
+ ip_range_low=nodegroup.ip_range_low,
+ ip_range_high=nodegroup.ip_range_high,
+ mapping=DHCPLease.objects.get_hostname_ip_mapping(nodegroup),
+ ))
+
+ def test_get_zone_returns_None_if_dhcp_not_enabled(self):
+ nodegroup = factory.make_node_group()
+ nodegroup.subnet_mask = None
+ nodegroup.save()
+ self.assertIsNone(get_zone(nodegroup))
+
class TestDNSConfigModifications(TestCase):
@@ -196,6 +223,15 @@
add_zone(nodegroup)
self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
+ def test_add_zone_doesnt_write_config_if_dhcp_disabled(self):
+ recorder = FakeMethod()
+ self.patch(DNSZoneConfig, 'write_config', recorder)
+ nodegroup = factory.make_node_group()
+ nodegroup.subnet_mask = None
+ nodegroup.save()
+ add_zone(nodegroup)
+ self.assertEqual(0, recorder.call_count)
+
def test_change_zone_changes_dns_zone(self):
nodegroup, _, _ = self.create_nodegroup_with_lease()
write_full_dns_config()
@@ -205,6 +241,24 @@
change_dns_zones(nodegroup)
self.assertDNSMatches(new_node.hostname, nodegroup.name, new_lease.ip)
+ def test_change_zone_changes_doesnt_write_config_if_dhcp_disabled(self):
+ recorder = FakeMethod()
+ self.patch(DNSZoneConfig, 'write_config', recorder)
+ nodegroup = factory.make_node_group()
+ nodegroup.subnet_mask = None
+ nodegroup.save()
+ change_dns_zones(nodegroup)
+ self.assertEqual(0, recorder.call_count)
+
+ def test_write_full_dns_doesnt_write_config_if_dhcp_disabled(self):
+ recorder = FakeMethod()
+ self.patch(DNSZoneConfig, 'write_config', recorder)
+ nodegroup = factory.make_node_group()
+ nodegroup.subnet_mask = None
+ nodegroup.save()
+ write_full_dns_config()
+ self.assertEqual(0, recorder.call_count)
+
def test_write_full_dns_loads_full_dns_config(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
write_full_dns_config()
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-07-26 09:11:26 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-07-26 15:34:35 +0000
@@ -17,7 +17,11 @@
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
from maasserver.worker_user import get_worker_user
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+ AllMatch,
+ Equals,
+ MatchesStructure,
+ )
def make_dhcp_settings():
@@ -76,7 +80,6 @@
self.assertEqual(nodegroup.api_key, nodegroup.api_token.key)
def test_ensure_master_creates_minimal_master_nodegroup(self):
- NodeGroup.objects._delete_master()
self.assertThat(
NodeGroup.objects.ensure_master(),
MatchesStructure.fromExample({
@@ -126,3 +129,28 @@
master = NodeGroup.objects.ensure_master()
self.assertEqual(master, reload_object(groupless_node).nodegroup)
self.assertNotEqual(master, reload_object(groupful_node).nodegroup)
+
+
+class TestNodeGroup(TestCase):
+
+ def test_is_dhcp_enabled_false_if_one_element_is_none(self):
+ 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)
+ nodegroup.save()
+ nodegroups.append(nodegroup)
+ self.assertThat(
+ [nodegroup.is_dhcp_enabled() for nodegroup in nodegroups],
+ AllMatch(Equals(False)))
+
+ 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())
