launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10217
[Merge] lp:~rvb/maas/ip-based_dns_config into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/ip-based_dns_config into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/ip-based_dns_config/+merge/116431
This branch changes to way we write forward and reverse zone files: we now pre-populate the forward and reverse zone files with the mapping between IP addresses and the generated hostnames for all the possible IP addresses in the zone. The custom hostnames are CNAME pointing to the generated hostnames.
Let me give you an example:
Imagine a nodegroup named 'nodename' with following network range: ip_low=192.12.3.255, ip_high=192.12.4.1 containing only one node with a custom hostname: 'myhostname' <-> 192.12.4.0.
The forward zone file will look like:
"""
192-12-3-255 IN A 192.12.3.255
192-12-4-0 IN A 192.12.4.0
192-12-4-1 IN A 192.12.4.1
myhostname IN CNAME 192-12-4-0
"""
The reverse zone file will look like:
"""
254.3 IN PTR 192-12-3-255.nodename
0.4 IN PTR 192-12-4-0.nodename
1.4 IN PTR 192-12-4-1.nodename
"""
= Notes =
- Right now, generated_hostname simply replaces the dots of the IP with dashes (see example above) but I've made sure that this is not hardcoded in any of the tests (apart from the test which tests the method generated_hostname) so that we will be able to change that if we want to.
- For clarity, I've renamed the fields of DNSZoneConfig so that they now match those from Nodegroup.
- I've moved the method ip_range (and the related tests) from maasserver.utils.network to provisioningserver.dns.utils because that method is actually used by the provisioning server and we can't import from maasserver in the provisioningserver.
- although I don't generally like had-hoc assertion methods like assertDNSMatches, I've made an exception here because the code is much more clear that way.
- all the methods in DNSZoneConfig which return mappings are now methods and not properties. Since what they do is not trivial, I think that the right think to do.
--
https://code.launchpad.net/~rvb/maas/ip-based_dns_config/+merge/116431
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/ip-based_dns_config into lp:maas.
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py 2012-07-18 12:38:15 +0000
+++ src/maasserver/dns.py 2012-07-24 10:06:21 +0000
@@ -50,8 +50,11 @@
serial = next_zone_serial()
return DNSZoneConfig(
zone_name=nodegroup.name, serial=serial,
+ ip_range_high=nodegroup.ip_range_high,
+ ip_range_low=nodegroup.ip_range_low,
mapping=DHCPLease.objects.get_hostname_ip_mapping(nodegroup),
- bcast=nodegroup.broadcast_ip, mask=nodegroup.subnet_mask)
+ broadcast_ip=nodegroup.broadcast_ip,
+ subnet_mask=nodegroup.subnet_mask)
def change_dns_zone(nodegroup):
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-07-19 16:44:01 +0000
+++ src/maasserver/models/nodegroup.py 2012-07-24 10:06:21 +0000
@@ -23,11 +23,11 @@
)
from maasserver import DefaultMeta
from maasserver.models.timestampedmodel import TimestampedModel
-from maasserver.utils.network import ip_range
from piston.models import (
KEY_SIZE,
Token,
)
+from provisioningserver.dns.utils import ip_range
worker_user_name = 'maas-nodegroup-worker'
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-07-17 16:43:28 +0000
+++ src/maasserver/tests/test_dns.py 2012-07-24 10:06:21 +0000
@@ -27,6 +27,7 @@
from maastesting.celery import CeleryFixture
from maastesting.tests.test_bindfixture import dig_call
from provisioningserver.dns.config import conf
+from provisioningserver.dns.utils import generated_hostname
from testresources import FixtureResource
from testtools.matchers import MatchesStructure
@@ -76,6 +77,8 @@
def create_nodegroup_with_lease(self, lease_number=1, nodegroup=None):
if nodegroup is None:
nodegroup = factory.make_node_group(
+ ip_range_low='192.168.0.0',
+ ip_range_high='192.168.0.254',
broadcast_ip='192.168.0.255',
subnet_mask='255.255.255.0')
node = factory.make_node(
@@ -87,24 +90,34 @@
return nodegroup, node, lease
def dig_resolve(self, fqdn):
+ """Resolve `fqdn` using dig. Returns a list of results."""
return dig_call(
- port=self.bind.config.port, commands=[fqdn, '+short'])
+ port=self.bind.config.port,
+ commands=[fqdn, '+short']).split('\n')
def dig_reverse_resolve(self, ip):
+ """Reverse resolve `ip` using dig. Returns a list of results."""
return dig_call(
port=self.bind.config.port,
- commands=['-x', ip, '+short'])
+ commands=['-x', ip, '+short']).split('\n')
+
+ def assertDNSMatches(self, hostname, domain, ip):
+ fqdn = "%s.%s" % (hostname, domain)
+ autogenerated_hostname = '%s.' % generated_hostname(ip, domain)
+ # The fqdn resolves to the autogenerated hostname (CNAME record) and
+ # the IP address (A record).
+ self.assertItemsEqual(
+ [autogenerated_hostname, ip],
+ self.dig_resolve(fqdn))
+ # A reverse lookup on the IP returns the autogenerated
+ # hostname.
+ self.assertEqual(
+ [autogenerated_hostname], self.dig_reverse_resolve(ip))
def test_add_zone_loads_dns_zone(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
add_zone(nodegroup)
- fqdn = "%s.%s" % (node.hostname, nodegroup.name)
- self.assertEqual(
- (lease.ip, '%s.' % fqdn),
- (
- self.dig_resolve(fqdn),
- self.dig_reverse_resolve(lease.ip),
- ))
+ self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
def test_change_zone_changes_dns_zone(self):
nodegroup, _, _ = self.create_nodegroup_with_lease()
@@ -113,21 +126,9 @@
self.create_nodegroup_with_lease(
nodegroup=nodegroup, lease_number=2))
change_dns_zone(nodegroup)
- fqdn = "%s.%s" % (new_node.hostname, nodegroup.name)
- self.assertEqual(
- (new_lease.ip, '%s.' % fqdn),
- (
- self.dig_resolve(fqdn),
- self.dig_reverse_resolve(new_lease.ip),
- ))
+ self.assertDNSMatches(new_node.hostname, nodegroup.name, new_lease.ip)
def test_write_full_dns_loads_full_dns_config(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
write_full_dns_config()
- fqdn = "%s.%s" % (node.hostname, nodegroup.name)
- self.assertEqual(
- (lease.ip, '%s.' % fqdn),
- (
- self.dig_resolve(fqdn),
- self.dig_reverse_resolve(lease.ip),
- ))
+ self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py 2012-07-18 15:48:08 +0000
+++ src/provisioningserver/dns/config.py 2012-07-24 10:06:21 +0000
@@ -29,6 +29,10 @@
)
from celery.conf import conf
+from provisioningserver.dns.utils import (
+ generated_hostname,
+ ip_range,
+ )
from provisioningserver.utils import (
atomic_write,
incremental_write,
@@ -186,44 +190,80 @@
return '\ninclude "%s";\n' % self.target_path
+def shortened_reversed_ip(ip, byte_num):
+ """Get a reversed version of this IP with only the significant bytes.
+
+ This method is a utility used when generating reverse zone files.
+
+ >>> shortened_reversed_ip('192.156.0.3', 2)
+ '3.0'
+ """
+ assert 0 <= byte_num <= 4, ("byte_num should be >=0 and <= 4.")
+ ip_octets = ip.split('.')
+ significant_octets = list(
+ reversed(ip_octets))[:4 - byte_num]
+ return '.'.join(significant_octets)
+
+
class DNSZoneConfig(DNSConfig):
"""A specialized version of DNSConfig that writes zone files."""
template_file_name = 'zone.template'
def __init__(self, zone_name, serial=None, mapping={},
- bcast=None, mask=None):
+ broadcast_ip=None, subnet_mask=None, ip_range_low=None,
+ ip_range_high=None):
self.zone_name = zone_name
self.mapping = mapping
- self.mask = mask
- self.bcast = bcast
+ self.subnet_mask = subnet_mask
+ self.broadcast_ip = broadcast_ip
+ self.ip_range_low = ip_range_low
+ self.ip_range_high = ip_range_high
self.serial = serial
@property
def byte_num(self):
"""Number of significant octets for the IPs of this zone."""
return len(
- [byte for byte in self.mask.split('.')
+ [byte for byte in self.subnet_mask.split('.')
if byte == '255'])
@property
def reverse_zone_name(self):
"""Return the name of the reverse zone."""
- significant_bits = self.bcast.split('.')[:self.byte_num]
+ significant_bits = self.broadcast_ip.split('.')[:self.byte_num]
return '%s.in-addr.arpa' % '.'.join(reversed(significant_bits))
- @property
- def reverse_mapping(self):
- """Return the reverse mapping: (shortened) ip->fqdn."""
- reverse_mapping = {}
- for hostname, ip in self.mapping.items():
- ip_octets = ip.split('.')
- significant_octets = list(
- reversed(ip_octets))[:4 - self.byte_num]
- rel_zone_ip = '.'.join(significant_octets)
- fqdn = '%s.%s.' % (hostname, self.zone_name)
- reverse_mapping[rel_zone_ip] = fqdn
- return reverse_mapping
+ def get_mapping(self):
+ """Return the mapping: hostname->generated hostname."""
+ return {
+ hostname: generated_hostname(ip)
+ for hostname, ip in self.mapping.items()
+ }
+
+ def get_generated_mapping(self):
+ """Return the generated mapping: fqdn->ip.
+
+ The generated mapping is the mapping between the generated hostnames
+ and the IP addresses for all the possible IP addresses in zone.
+ """
+ return {
+ generated_hostname(ip): ip
+ for ip in ip_range(self.ip_range_low, self.ip_range_high)
+ }
+
+ def get_generated_reverse_mapping(self):
+ """Return the reverse generated mapping: (shortened) ip->fqdn.
+
+ The reverse generated mapping is the mapping between the IP addresses
+ and the generated hostnames for all the possible IP addresses in zone.
+ """
+ return dict(
+ (
+ shortened_reversed_ip(ip, self.byte_num),
+ '%s.%s.' % (hostname, self.zone_name)
+ )
+ for hostname, ip in self.get_generated_mapping().items())
@property
def template_path(self):
@@ -255,10 +295,14 @@
That context dict is used to render the DNS zone file.
"""
context = self.get_base_context()
- mapping = self.mapping.copy()
+ mapping = self.get_generated_mapping()
# TODO: Add NS record.
mapping['%s.' % self.zone_name] = '127.0.0.1'
- context.update(mapping=mapping, type='A')
+ mappings = {
+ 'CNAME': self.get_mapping(),
+ 'A': mapping,
+ }
+ context.update(mappings=mappings)
return context
def get_reverse_context(self):
@@ -267,7 +311,8 @@
That context dict is used to render the DNS reverse zone file.
"""
context = self.get_base_context()
- context.update(mapping=self.reverse_mapping, type='PTR')
+ mappings = {'PTR': self.get_generated_reverse_mapping()}
+ context.update(mappings=mappings)
return context
def write_config(self, **kwargs):
=== modified file 'src/provisioningserver/dns/templates/zone.template'
--- src/provisioningserver/dns/templates/zone.template 2012-07-17 16:43:28 +0000
+++ src/provisioningserver/dns/templates/zone.template 2012-07-24 10:06:21 +0000
@@ -12,6 +12,8 @@
)
IN NS {{domain}}.
+{{for type, mapping in mappings.items()}}
{{for item_from, item_to in mapping.items()}}
{{item_from}} IN {{type}} {{item_to}}
{{endfor}}
+{{endfor}}
=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py 2012-07-17 17:09:40 +0000
+++ src/provisioningserver/dns/tests/test_config.py 2012-07-24 10:06:21 +0000
@@ -34,8 +34,10 @@
MAAS_NAMED_RNDC_CONF_NAME,
MAAS_RNDC_CONF_NAME,
setup_rndc,
+ shortened_reversed_ip,
TEMPLATES_PATH,
)
+from provisioningserver.dns.utils import generated_hostname
import tempita
from testtools.matchers import (
Contains,
@@ -118,8 +120,8 @@
self.patch(DNSConfig, 'target_dir', target_dir)
zone_name = factory.getRandomString()
zone = DNSZoneConfig(
- zone_name, bcast='192.168.0.255',
- mask='255.255.255.0',
+ zone_name, broadcast_ip='192.168.0.255',
+ subnet_mask='255.255.255.0',
mapping={factory.getRandomString(): '192.168.0.5'})
dnsconfig = DNSConfig(zones=[zone])
dnsconfig.write_config()
@@ -147,27 +149,35 @@
Contains('include "%s"' % dnsconfig.target_path)))
+class TestUtilities(TestCase):
+
+ def test_shortened_reversed_ip(self):
+ self.assertEqual(
+ '3.0',
+ shortened_reversed_ip('192.156.0.3', 2))
+
+
class TestDNSZoneConfig(TestCase):
"""Tests for DNSZoneConfig."""
def test_DNSZoneConfig_fields(self):
zone_name = factory.getRandomString()
serial = random.randint(1, 200)
- bcast = factory.getRandomIPAddress()
- mask = factory.getRandomIPAddress()
+ broadcast_ip = factory.getRandomIPAddress()
+ subnet_mask = factory.getRandomIPAddress()
hostname = factory.getRandomString()
ip = factory.getRandomString()
mapping = {hostname: ip}
dns_zone_config = DNSZoneConfig(
- zone_name, serial, mapping, bcast, mask)
+ zone_name, serial, mapping, broadcast_ip, subnet_mask)
self.assertThat(
dns_zone_config,
MatchesStructure.byEquality(
zone_name=zone_name,
serial=serial,
mapping=mapping,
- bcast=bcast,
- mask=mask,
+ broadcast_ip=broadcast_ip,
+ subnet_mask=subnet_mask,
)
)
@@ -191,19 +201,20 @@
# a /24 network.
zone_name = factory.getRandomString()
hostname = factory.getRandomString()
+ ip = '192.168.0.5'
dns_zone_config = DNSZoneConfig(
- zone_name, bcast='192.168.0.255',
- mask='255.255.255.0',
- mapping={hostname: '192.168.0.5'})
+ zone_name, broadcast_ip='192.168.0.255',
+ subnet_mask='255.255.255.0',
+ mapping={hostname: ip})
self.assertEqual(
(
3,
- {'5': '%s.%s.' % (hostname, zone_name)},
+ {hostname: generated_hostname(ip)},
'0.168.192.in-addr.arpa',
),
(
dns_zone_config.byte_num,
- dns_zone_config.reverse_mapping,
+ dns_zone_config.get_mapping(),
dns_zone_config.reverse_zone_name,
))
@@ -212,22 +223,52 @@
# a /22 network.
zone_name = factory.getRandomString()
hostname = factory.getRandomString()
+ ip = '192.12.2.10'
dns_zone_config = DNSZoneConfig(
- zone_name, bcast='192.12.3.255',
- mask='255.255.252.0',
- mapping={hostname: '192.12.2.10'})
+ zone_name, broadcast_ip='192.12.3.255',
+ subnet_mask='255.255.252.0',
+ mapping={hostname: ip})
self.assertEqual(
(
2,
- {'10.2': '%s.%s.' % (hostname, zone_name)},
+ {hostname: generated_hostname(ip)},
'12.192.in-addr.arpa',
),
(
dns_zone_config.byte_num,
- dns_zone_config.reverse_mapping,
+ dns_zone_config.get_mapping(),
dns_zone_config.reverse_zone_name,
))
+ def test_DNSZoneConfig_get_generated_mapping(self):
+ name = factory.getRandomString()
+ dns_zone_config = DNSZoneConfig(
+ name, ip_range_low='192.12.3.255',
+ subnet_mask='255.255.252.0', ip_range_high='192.12.4.1')
+ self.assertEqual(
+ {
+ generated_hostname('192.12.3.255'): '192.12.3.255',
+ generated_hostname('192.12.4.0'): '192.12.4.0',
+ generated_hostname('192.12.4.1'): '192.12.4.1',
+ },
+ dns_zone_config.get_generated_mapping(),
+ )
+
+ def test_DNSZoneConfig_get_generated_reverse_mapping(self):
+ name = factory.getRandomString()
+ dns_zone_config = DNSZoneConfig(
+ name, ip_range_low='192.12.3.254',
+ subnet_mask='255.255.252.0', ip_range_high='192.12.4.1')
+ self.assertEqual(
+ {
+ '254.3': '%s.' % generated_hostname('192.12.3.254', name),
+ '255.3': '%s.' % generated_hostname('192.12.3.255', name),
+ '0.4': '%s.' % generated_hostname('192.12.4.0', name),
+ '1.4': '%s.' % generated_hostname('192.12.4.1', name),
+ },
+ dns_zone_config.get_generated_reverse_mapping(),
+ )
+
def test_DNSZoneConfig_writes_dns_zone_config(self):
target_dir = self.make_dir()
self.patch(DNSConfig, 'target_dir', target_dir)
@@ -236,6 +277,7 @@
ip = factory.getRandomIPAddress()
dns_zone_config = DNSZoneConfig(
zone_name, serial=random.randint(1, 100),
+ ip_range_low='192.12.2.0', ip_range_high='192.12.2.254',
mapping={hostname: ip})
dns_zone_config.write_config()
self.assertThat(
@@ -244,19 +286,20 @@
matcher=ContainsAll(
[
'IN NS %s.' % zone_name,
- '%s IN A %s' % (hostname, ip),
+ '%s IN CNAME %s' % (hostname, generated_hostname(ip)),
+ '%s IN A %s' % (
+ generated_hostname('192.12.2.30'),
+ '192.12.2.30'),
])))
def test_DNSZoneConfig_writes_reverse_dns_zone_config(self):
target_dir = self.make_dir()
self.patch(DNSConfig, 'target_dir', target_dir)
zone_name = factory.getRandomString()
- hostname = factory.getRandomString()
- ip = '192.12.2.10'
dns_zone_config = DNSZoneConfig(
zone_name, serial=random.randint(1, 100),
- bcast='192.12.3.255', mask='255.255.252.0',
- mapping={hostname: ip})
+ broadcast_ip='192.12.2.255', subnet_mask='255.255.252.0',
+ ip_range_low='192.12.2.0', ip_range_high='192.12.2.254')
dns_zone_config.write_reverse_config()
self.assertThat(
os.path.join(target_dir, 'zone.rev.%s' % zone_name),
@@ -264,4 +307,5 @@
matcher=ContainsAll(
['%s IN PTR %s' % (
'10.2',
- '%s.%s.' % (hostname, zone_name))])))
+ generated_hostname(
+ '192.12.2.10'))])))
=== renamed file 'src/maasserver/utils/tests/test_network.py' => 'src/provisioningserver/dns/tests/test_utils.py'
--- src/maasserver/utils/tests/test_network.py 2012-07-19 16:44:01 +0000
+++ src/provisioningserver/dns/tests/test_utils.py 2012-07-24 10:06:21 +0000
@@ -15,12 +15,14 @@
import socket
import struct
-from maasserver.utils.network import (
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from provisioningserver.dns.utils import (
dotted_quad_to_int,
+ generated_hostname,
int_to_dotted_quad,
ip_range,
)
-from maastesting.testcase import TestCase
dottedquad_int = [
@@ -57,3 +59,13 @@
self.assertEqual(
['192.168.0.1', '192.168.0.2', '192.168.0.3'],
list(ip_range('192.168.0.1', '192.168.0.3')))
+
+ def test_generated_hostname_returns_hostname(self):
+ self.assertEqual(
+ '192-168-0-1', generated_hostname('192.168.0.1'))
+
+ def test_generated_hostname_returns_hostname_plus_domain(self):
+ domain = factory.getRandomString()
+ self.assertEqual(
+ '192-168-0-1.%s' % domain,
+ generated_hostname('192.168.0.1', domain))
=== renamed file 'src/maasserver/utils/network.py' => 'src/provisioningserver/dns/utils.py'
--- src/maasserver/utils/network.py 2012-07-19 16:44:01 +0000
+++ src/provisioningserver/dns/utils.py 2012-07-24 10:06:21 +0000
@@ -11,14 +11,30 @@
__metaclass__ = type
__all__ = [
+ 'generated_hostname',
'int_to_dotted_quad',
'dotted_quad_to_int',
'ip_range',
]
+from itertools import imap
import socket
import struct
-from itertools import imap
+
+
+def generated_hostname(ip, domain=None):
+ """Return the auto-generated hostname for the give IP.
+
+ >>> generated_hostname('192.168.0.1')
+ '192-168-0-1'
+ >>> generated_hostname('192.168.0.1', 'mydomain.com')
+ '192-168-0-1.mydomain.com'
+ """
+ hostname = ip.replace('.', '-')
+ if domain is not None:
+ return '%s.%s' % (hostname, domain)
+ else:
+ return hostname
def int_to_dotted_quad(n):
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py 2012-07-17 16:43:28 +0000
+++ src/provisioningserver/tests/test_tasks.py 2012-07-24 10:06:21 +0000
@@ -153,8 +153,9 @@
command = factory.getRandomString()
zone_name = factory.getRandomString()
zone = DNSZoneConfig(
- zone_name, bcast='192.168.0.255',
- mask='255.255.255.0',
+ zone_name, broadcast_ip='192.168.0.255',
+ ip_range_low='192.168.0.0', ip_range_high='192.168.0.254',
+ subnet_mask='255.255.255.0',
serial=random.randint(1, 100),
mapping={factory.getRandomString(): '192.168.0.5'})
result = write_dns_zone_config.delay(
@@ -215,8 +216,9 @@
# the zone files, and reloads the dns service.
zone_name = factory.getRandomString()
zones = [DNSZoneConfig(
- zone_name, bcast='192.168.0.255',
- mask='255.255.255.0',
+ zone_name, broadcast_ip='192.168.0.255',
+ ip_range_low='192.168.0.0', ip_range_high='192.168.0.254',
+ subnet_mask='255.255.255.0',
serial=random.randint(1, 100),
mapping={factory.getRandomString(): '192.168.0.5'})]
command = factory.getRandomString()
