launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12944
[Merge] lp:~rvb/maas/use-generator-dns into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/use-generator-dns into lp:maas.
Commit message:
Instead of generating the full ip mappings needed to populate the zone file in memory, use generators to minimize the memory impact of the whole process.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/use-generator-dns/+merge/128051
This branch improves the way the DNS zone files are populated. Instead of generating the full ip mappings needed to populate the zone file in memory, use generators to minimize the memory impact of the whole process.
= Pre-imp =
This was discussed with Gavin.
= Notes =
Drive by fix 1: renamed get_mapping->get_cname_mapping and get_generated_mapping->get_static_mapping
Drive by fix 2: have the A record added in get_static_mapping() (where it belongs) instead of in get_context().
--
https://code.launchpad.net/~rvb/maas/use-generator-dns/+merge/128051
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/use-generator-dns into lp:maas.
=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py 2012-09-30 22:37:05 +0000
+++ src/provisioningserver/dns/config.py 2012-10-04 16:01:24 +0000
@@ -276,37 +276,42 @@
"""Return the name of the forward zone."""
return self.domain
- def get_mapping(self):
- """Return the mapping: hostname->generated hostname."""
- return {
- hostname: generated_hostname(ip)
+ def get_cname_mapping(self):
+ """Return a generator with the mapping: hostname->generated hostname.
+
+ Note that we return a list of tuples instead of a dictionary in order
+ to be able to return a generator.
+ """
+ return (
+ (hostname, generated_hostname(ip))
for hostname, ip in self.mapping.items()
- }
+ )
- def get_generated_mapping(self):
- """Return the generated mapping: fqdn->ip.
+ def get_static_mapping(self):
+ """Return a generator with the mapping fqdn->ip for the generated ips.
The generated mapping is the mapping between the generated hostnames
and the IP addresses for all the possible IP addresses in zone.
+ Note that we return a list of tuples instead of a dictionary in order
+ to be able to return a generator.
"""
ips = imap(unicode, chain.from_iterable(self.networks))
- return {generated_hostname(ip): ip for ip in ips}
+ static_mapping = ((generated_hostname(ip), ip) for ip in ips)
+ # Add A record for the name server's IP.
+ return chain([('%s.' % self.domain, self.dns_ip)], static_mapping)
def get_context(self):
"""Return the dict used to render the DNS zone file.
That context dict is used to render the DNS zone file.
"""
- mapping = self.get_generated_mapping()
- # Add A record for the name server's IP.
- mapping['%s.' % self.domain] = self.dns_ip
return {
'domain': self.domain,
'serial': self.serial,
'modified': unicode(datetime.today()),
'mappings': {
- 'CNAME': self.get_mapping(),
- 'A': mapping,
+ 'CNAME': self.get_cname_mapping(),
+ 'A': self.get_static_mapping(),
}
}
@@ -330,18 +335,18 @@
octets = broadcast.words[:netmask.words.count(255)]
return '%s.in-addr.arpa' % '.'.join(imap(unicode, reversed(octets)))
- def get_generated_mapping(self):
+ def get_static_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.
"""
byte_num = 4 - self.network.netmask.words.count(255)
- return {
- shortened_reversed_ip(ip, byte_num):
- '%s.%s.' % (generated_hostname(ip), self.domain)
+ return (
+ (shortened_reversed_ip(ip, byte_num),
+ '%s.%s.' % (generated_hostname(ip), self.domain))
for ip in self.network
- }
+ )
def get_context(self):
"""Return the dict used to render the DNS reverse zone file.
@@ -353,6 +358,6 @@
'serial': self.serial,
'modified': unicode(datetime.today()),
'mappings': {
- 'PTR': self.get_generated_mapping(),
+ 'PTR': self.get_static_mapping(),
}
}
=== modified file 'src/provisioningserver/dns/templates/zone.template'
--- src/provisioningserver/dns/templates/zone.template 2012-07-24 09:35:25 +0000
+++ src/provisioningserver/dns/templates/zone.template 2012-10-04 16:01:24 +0000
@@ -13,7 +13,7 @@
IN NS {{domain}}.
{{for type, mapping in mappings.items()}}
-{{for item_from, item_to in mapping.items()}}
+{{for item_from, item_to in mapping}}
{{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-09-30 22:40:33 +0000
+++ src/provisioningserver/dns/tests/test_config.py 2012-10-04 16:01:24 +0000
@@ -12,6 +12,10 @@
__metaclass__ = type
__all__ = []
+from collections import (
+ Iterable,
+ Sequence,
+ )
import os.path
import random
@@ -49,7 +53,9 @@
EndsWith,
FileContains,
FileExists,
+ IsInstance,
MatchesStructure,
+ Not,
StartsWith,
)
from twisted.python.filepath import FilePath
@@ -246,32 +252,62 @@
dns_zone_config.target_path,
))
- def test_get_generated_mapping(self):
- name = factory.getRandomString()
- network = IPNetwork('192.12.0.1/30')
- dns_zone_config = DNSForwardZoneConfig(name, networks=[network])
- self.assertEqual(
- {
- generated_hostname('192.12.0.0'): '192.12.0.0',
- generated_hostname('192.12.0.1'): '192.12.0.1',
- generated_hostname('192.12.0.2'): '192.12.0.2',
- generated_hostname('192.12.0.3'): '192.12.0.3',
- },
- dns_zone_config.get_generated_mapping(),
+ def test_forward_zone_get_cname_mapping_returns_iterator(self):
+ name = factory.getRandomString()
+ network = IPNetwork('192.12.0.1/30')
+ dns_ip = factory.getRandomIPInNetwork(network)
+ dns_zone_config = DNSForwardZoneConfig(
+ name, networks=[network], dns_ip=dns_ip,
+ mapping={
+ factory.make_name('hostname'): factory.getRandomIPAddress()})
+ self.assertThat(
+ dns_zone_config.get_cname_mapping(),
+ MatchesAll(
+ IsInstance(Iterable), Not(IsInstance(Sequence))))
+
+ def test_get_static_mapping(self):
+ name = factory.getRandomString()
+ network = IPNetwork('192.12.0.1/30')
+ dns_ip = factory.getRandomIPInNetwork(network)
+ dns_zone_config = DNSForwardZoneConfig(
+ name, networks=[network], dns_ip=dns_ip)
+ self.assertItemsEqual(
+ [
+ ('%s.' % name, dns_ip),
+ (generated_hostname('192.12.0.0'), '192.12.0.0'),
+ (generated_hostname('192.12.0.1'), '192.12.0.1'),
+ (generated_hostname('192.12.0.2'), '192.12.0.2'),
+ (generated_hostname('192.12.0.3'), '192.12.0.3'),
+ ],
+ dns_zone_config.get_static_mapping(),
)
- def test_get_generated_mapping_multiple_networks(self):
+ def test_forward_zone_get_static_mapping_returns_iterator(self):
+ name = factory.getRandomString()
+ network = IPNetwork('192.12.0.1/30')
+ dns_ip = factory.getRandomIPInNetwork(network)
+ dns_zone_config = DNSForwardZoneConfig(
+ name, networks=[network], dns_ip=dns_ip)
+ self.assertThat(
+ dns_zone_config.get_static_mapping(),
+ MatchesAll(
+ IsInstance(Iterable), Not(IsInstance(Sequence))))
+
+ def test_get_static_mapping_multiple_networks(self):
name = factory.getRandomString()
networks = IPNetwork('11.11.11.11/31'), IPNetwork('22.22.22.22/31')
- dns_zone_config = DNSForwardZoneConfig(name, networks=networks)
- self.assertEqual(
- {
- generated_hostname('11.11.11.10'): '11.11.11.10',
- generated_hostname('11.11.11.11'): '11.11.11.11',
- generated_hostname('22.22.22.22'): '22.22.22.22',
- generated_hostname('22.22.22.23'): '22.22.22.23',
- },
- dns_zone_config.get_generated_mapping(),
+ dns_ip = factory.getRandomIPInNetwork(networks[0])
+ dns_zone_config = DNSForwardZoneConfig(
+ name, networks=networks, dns_ip=dns_ip)
+ self.assertItemsEqual(
+ [
+ ('%s.' % name, dns_ip),
+ (generated_hostname('11.11.11.10'), '11.11.11.10'),
+ (generated_hostname('11.11.11.11'), '11.11.11.11'),
+ (generated_hostname('22.22.22.22'), '22.22.22.22'),
+ (generated_hostname('22.22.22.23'), '22.22.22.23'),
+ ],
+ dns_zone_config.get_static_mapping(),
)
def test_writes_dns_zone_config(self):
@@ -386,18 +422,26 @@
'168.192.in-addr.arpa',
dns_zone_config.zone_name)
- def test_get_generated_mapping(self):
+ def test_get_static_mapping_returns_iterator(self):
+ dns_zone_config = DNSReverseZoneConfig(
+ factory.getRandomString(), network=IPNetwork('192.12.0.1/30'))
+ self.assertThat(
+ dns_zone_config.get_static_mapping(),
+ MatchesAll(
+ IsInstance(Iterable), Not(IsInstance(Sequence))))
+
+ def test_get_static_mapping(self):
name = factory.getRandomString()
network = IPNetwork('192.12.0.1/30')
dns_zone_config = DNSReverseZoneConfig(name, network=network)
- self.assertEqual(
- {
- '0': '%s.' % generated_hostname('192.12.0.0', name),
- '1': '%s.' % generated_hostname('192.12.0.1', name),
- '2': '%s.' % generated_hostname('192.12.0.2', name),
- '3': '%s.' % generated_hostname('192.12.0.3', name),
- },
- dns_zone_config.get_generated_mapping(),
+ self.assertItemsEqual(
+ [
+ ('0', '%s.' % generated_hostname('192.12.0.0', name)),
+ ('1', '%s.' % generated_hostname('192.12.0.1', name)),
+ ('2', '%s.' % generated_hostname('192.12.0.2', name)),
+ ('3', '%s.' % generated_hostname('192.12.0.3', name)),
+ ],
+ dns_zone_config.get_static_mapping(),
)
def test_writes_dns_zone_config_with_NS_record(self):
