← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/maas/reverse-zones into lp:maas

 

Raphaël Badin has proposed merging lp:~rvb/maas/reverse-zones into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/reverse-zones/+merge/115388

This branch adds the writing of reverse DNS zone files to the module src/provisioningserver/dns/config.py.  Also, and that's the highlight of this branch (yes, there is a 'highlight' in this branch :)), I've extended the tests in src/maasserver/tests/test_dns.py to check that the reverse DNS records can be queried using 'dig'.  Now that I've got the visibility of all the data that needs to be used, I refactored a bit the classes in src/provisioningserver/dns/config.py (although that branch can seem big, it removes fair amount of SLOC).

= Notes =

- I extended DNSZoneConfig so that it's now able to write reverse zone files as well.  That class is an object (constructed from a nodegroup) which encapsulates all the methods required to write reverse zone files.  I confess some of the methods are a little bit messy (reverse_mapping or reverse_zone_name for instance) but I'm afraid writing reverse zone files is a messy operation.

- I extended rndc_command so that it can now accept a 'callback' argument (like the rest of the DNS related tasks).  This is required to chain reload operations (for instance when we want to reload the zone and then reload the corresponding reverse zone).

- Note that if we ever want to change the pattern used here and have the worker load all the data needed using an API call (as opposed to using the data passed in), the refactoring I've done will make that very easy to do: we will simply need to create an API call that will return the information needed to create a DNSZoneConfig object from a nodegroup ID.  The change, in essence, will be really simple: we will replace the call to src/maasserver/dns.get_zone by an API call inside the worker code and pass nodegroup.id instead of the zone object created by 'get_zone'.

- Using a small trick (fwiw, this trick is also used by Cobbler), I was able to use the exact same template for the zone files and the reverse zone files.  The trick is only to specify a different {{type}} which is used when writing the records (see the changes made to src/provisioningserver/dns/templates/zone.template).
-- 
https://code.launchpad.net/~rvb/maas/reverse-zones/+merge/115388
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/reverse-zones into lp:maas.
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py	2012-07-17 09:08:49 +0000
+++ src/maasserver/dns.py	2012-07-17 17:12:22 +0000
@@ -27,6 +27,7 @@
     Sequence,
     )
 from provisioningserver import tasks
+from provisioningserver.dns.config import DNSZoneConfig
 
 # A DNS zone's serial is a 32-bit integer.  Also, we start with the
 # value 1 because 0 has special meaning for some DNS servers.  Even if
@@ -39,20 +40,34 @@
     return '%0.10d' % zone_serial.nextval()
 
 
+def get_zone(nodegroup, serial=None):
+    """Create a :class:`DNSZoneConfig` object from a nodegroup.
+
+    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()
+    return DNSZoneConfig(
+        zone_name=nodegroup.name, serial=serial,
+        mapping=DHCPLease.objects.get_hostname_ip_mapping(nodegroup),
+        bcast=nodegroup.broadcast_ip, mask=nodegroup.subnet_mask)
+
+
 def change_dns_zone(nodegroup):
     """Update the zone configurtion for the given `nodegroup`.
 
     :param nodegroup: The nodegroup for which the zone should be cupdated.
     :type nodegroup: :class:`NodeGroup`
     """
-    mapping = DHCPLease.objects.get_hostname_ip_mapping(nodegroup)
-    zone_name = nodegroup.name
+    zone = get_zone(nodegroup)
+    reverse_zone_reload_subtask = tasks.rndc_command.subtask(
+        args=[['reload', zone.reverse_zone_name]])
     zone_reload_subtask = tasks.rndc_command.subtask(
-        args=['reload', zone_name])
+        args=[['reload', zone.zone_name]],
+        callback=reverse_zone_reload_subtask)
     tasks.write_dns_zone_config.delay(
-        zone_name=zone_name, domain=zone_name,
-        serial=next_zone_serial(), hostname_ip_mapping=mapping,
-        callback=zone_reload_subtask)
+        zone=zone, callback=zone_reload_subtask)
 
 
 def add_zone(nodegroup):
@@ -61,35 +76,23 @@
     :param nodegroup: The nodegroup for which the zone should be added.
     :type nodegroup: :class:`NodeGroup`
     """
-    zone_names = [
-        result[0]
-        for result in NodeGroup.objects.all().values_list('name')]
-    tasks.write_dns_config(zone_names=zone_names)
-    mapping = DHCPLease.objects.get_hostname_ip_mapping(nodegroup)
-    zone_name = nodegroup.name
-    reconfig_subtask = tasks.rndc_command.subtask(args=['reconfig'])
+    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(
-        zone_names=zone_names, callback=reconfig_subtask)
+        zones=zones, callback=reconfig_subtask)
+    zone = get_zone(nodegroup)
     tasks.write_dns_zone_config.delay(
-        zone_name=zone_name, domain=zone_name,
-        serial=next_zone_serial(), hostname_ip_mapping=mapping,
-        callback=write_dns_config_subtask)
+        zone=zone, callback=write_dns_config_subtask)
 
 
 def write_full_dns_config():
     """Write the DNS configuration for all the nodegroups."""
-    groups = NodeGroup.objects.all()
     serial = next_zone_serial()
-    zones = {
-        group.name: {
-            'serial': serial,
-            'zone_name': group.name,
-            'domain': group.name,
-            'hostname_ip_mapping': (
-                DHCPLease.objects.get_hostname_ip_mapping(
-                    group))
-            }
-        for group in groups
-        }
+    zones = [
+        get_zone(nodegroup, serial)
+        for nodegroup in NodeGroup.objects.all()]
     tasks.write_full_dns_config(
-        zones,  callback=tasks.rndc_command.subtask(args=['reload']))
+        zones, callback=tasks.rndc_command.subtask(args=[['reload']]))

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2012-07-16 13:07:14 +0000
+++ src/maasserver/tests/test_dns.py	2012-07-17 17:12:22 +0000
@@ -73,42 +73,61 @@
         # Reload BIND.
         self.bind.runner.rndc('reload')
 
-    def create_nodegroup_with_lease(self, nodegroup=None):
+    def create_nodegroup_with_lease(self, lease_number=1, nodegroup=None):
         if nodegroup is None:
-            nodegroup = factory.make_node_group()
+            nodegroup = factory.make_node_group(
+                broadcast_ip='192.168.0.255',
+                subnet_mask='255.255.255.0')
         node = factory.make_node(
             nodegroup=nodegroup, set_hostname=True)
         mac = factory.make_mac_address(node=node)
         lease = factory.make_dhcp_lease(
-            nodegroup=nodegroup, mac=mac.mac_address)
+            nodegroup=nodegroup, mac=mac.mac_address,
+            ip='192.168.0.%d' % lease_number)
         return nodegroup, node, lease
 
     def dig_resolve(self, fqdn):
         return dig_call(
+            port=self.bind.config.port, commands=[fqdn, '+short'])
+
+    def dig_reverse_resolve(self, ip):
+        return dig_call(
             port=self.bind.config.port,
-            commands=[fqdn, '+short'])
+            commands=['-x', ip, '+short'])
 
     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, self.dig_resolve(fqdn))
+        self.assertEqual(
+            (lease.ip, '%s.' % fqdn),
+            (
+                self.dig_resolve(fqdn),
+                self.dig_reverse_resolve(lease.ip),
+            ))
 
     def test_change_zone_changes_dns_zone(self):
         nodegroup, _, _ = self.create_nodegroup_with_lease()
         write_full_dns_config()
         nodegroup, new_node, new_lease = (
-            self.create_nodegroup_with_lease(nodegroup=nodegroup))
+            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, self.dig_resolve(fqdn))
+        self.assertEqual(
+            (new_lease.ip, '%s.' % fqdn),
+            (
+                self.dig_resolve(fqdn),
+                self.dig_reverse_resolve(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, self.dig_resolve(fqdn))
+        self.assertEqual(
+            (lease.ip, '%s.' % fqdn),
+            (
+                self.dig_resolve(fqdn),
+                self.dig_reverse_resolve(lease.ip),
+            ))

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-07-13 16:33:09 +0000
+++ src/provisioningserver/dns/config.py	2012-07-17 17:12:22 +0000
@@ -98,7 +98,7 @@
         f.write(named_content)
 
 
-def execute_rndc_command(*arguments):
+def execute_rndc_command(arguments):
     """Execute a rndc command."""
     rndc_conf = os.path.join(
         conf.DNS_CONFIG_DIR, MAAS_RNDC_CONF_NAME)
@@ -115,8 +115,6 @@
 class DNSConfigBase:
     __metaclass__ = ABCMeta
 
-    incremental_write = False
-
     @abstractproperty
     def template_path(self):
         """Return the full path of the template to be used."""
@@ -143,20 +141,17 @@
         except NameError as error:
             raise DNSConfigFail(*error.args)
 
-    def get_extra_context(self):
-        """Dictionary containing extra parameters to be included in the
+    def get_context(self):
+        """Dictionary containing parameters to be included in the
         parameters used when rendering the template."""
         return {}
 
     def write_config(self, **kwargs):
         """Write out this DNS config file."""
         template = self.get_template()
-        kwargs.update(self.get_extra_context())
+        kwargs.update(self.get_context())
         rendered = self.render_template(template, **kwargs)
-        if self.incremental_write:
-            incremental_write(rendered, self.target_path)
-        else:
-            atomic_write(rendered, self.target_path)
+        atomic_write(rendered, self.target_path)
 
 
 class DNSConfig(DNSConfigBase):
@@ -168,9 +163,8 @@
     template_file_name = 'named.conf.template'
     target_file_name = MAAS_NAMED_CONF_NAME
 
-    def __init__(self, zone_names=(), reverse_zone_names=()):
-        self.zone_names = zone_names
-        self.reverse_zone_names = reverse_zone_names
+    def __init__(self, zones=()):
+        self.zones = zones
 
     @property
     def template_path(self):
@@ -180,14 +174,9 @@
     def target_path(self):
         return os.path.join(self.target_dir, self.target_file_name)
 
-    def get_extra_context(self):
+    def get_context(self):
         return {
-            'zones': [
-                DNSZoneConfig(zone_name)
-                for zone_name in self.zone_names],
-            'rev_zones': [
-                RevDNSZoneConfig(reverse_zone_name)
-                for reverse_zone_name in self.reverse_zone_names],
+            'zones': self.zones,
             'DNS_CONFIG_DIR': conf.DNS_CONFIG_DIR,
             'named_rndc_conf_path':  get_named_rndc_conf_path(),
             'modified': unicode(datetime.today()),
@@ -201,16 +190,41 @@
     """A specialized version of DNSConfig that writes zone files."""
 
     template_file_name = 'zone.template'
-    zone_name_string = '%s'
-    zone_filename_string = 'zone.%s'
-    incremental_write = True
 
-    def __init__(self, zone_name):
+    def __init__(self, zone_name, serial=None, mapping={},
+                 bcast=None, mask=None):
         self.zone_name = zone_name
-
-    @property
-    def name(self):
-        return self.zone_name_string % self.zone_name
+        self.mapping = mapping
+        self.mask = mask
+        self.bcast = bcast
+        self.serial = serial
+        # How many bytes should be returned in the reverse mapping.
+
+    @property
+    def byte_num(self):
+        """Return the number of significant bits for the IPs of this
+        zone.
+        """
+        return len(
+            [byte for byte in self.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]
+        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():
+            rel_zone_ip = '.'.join(list(
+                reversed(ip.split('.')))[:4 - self.byte_num])
+            fqdn = '%s.%s.' % (hostname, self.zone_name)
+            reverse_mapping[rel_zone_ip] = fqdn
+        return reverse_mapping
 
     @property
     def template_path(self):
@@ -218,21 +232,55 @@
 
     @property
     def target_path(self):
-        return os.path.join(
-            self.target_dir, self.zone_filename_string % self.zone_name)
-
-    def get_extra_context(self):
+        """Return the full path of the DNS zone config file."""
+        return os.path.join(
+            self.target_dir, 'zone.%s' % self.zone_name)
+
+    @property
+    def target_reverse_path(self):
+        """Return the full path of the DNS reverse zone config file."""
+        return os.path.join(
+            self.target_dir, 'zone.rev.%s' % self.zone_name)
+
+    def get_base_context(self):
+        """Return the context used to render both zone files."""
         return {
+            'domain': self.zone_name,
+            'serial': self.serial,
             'modified': unicode(datetime.today()),
         }
 
-
-class RevDNSZoneConfig(DNSZoneConfig):
-    """A specialized version of DNSZoneConfig that writes reverse zone
-    files.
-    """
-
-    template_file_name = 'zone.template'
-    # TODO: create a proper reverse zone template, create test for this class.
-    zone_name_string = '%s.rev'
-    zone_filename_string = 'zone.rev.%s'
+    def get_context(self):
+        """Return the context used to render the DNS zone file."""
+        context = self.get_base_context()
+        mapping = self.mapping.copy()
+        # TODO: Add NS record.
+        mapping['%s.' % self.zone_name] = '127.0.0.1'
+        context.update({
+            'mapping': mapping,
+            'type': 'A',
+        })
+        return context
+
+    def get_reverse_context(self):
+        """Return the context used to render the DNS reverse zone file."""
+        context = self.get_base_context()
+        context.update({
+            'mapping': self.reverse_mapping,
+            'type': 'PTR',
+        })
+        return context
+
+    def write_config(self, **kwargs):
+        """Write out the DNS config file for this zone."""
+        template = self.get_template()
+        kwargs.update(self.get_context())
+        rendered = self.render_template(template, **kwargs)
+        incremental_write(rendered, self.target_path)
+
+    def write_reverse_config(self, **kwargs):
+        """Write out the DNS reverse config file for this zone."""
+        template = self.get_template()
+        kwargs.update(self.get_reverse_context())
+        rendered = self.render_template(template, **kwargs)
+        incremental_write(rendered, self.target_reverse_path)

=== modified file 'src/provisioningserver/dns/templates/named.conf.template'
--- src/provisioningserver/dns/templates/named.conf.template	2012-07-11 11:13:24 +0000
+++ src/provisioningserver/dns/templates/named.conf.template	2012-07-17 17:12:22 +0000
@@ -2,15 +2,15 @@
 
 # Zone declarations
 {{for zone in zones}}
-zone "{{zone.name}}" {
+zone "{{zone.zone_name}}" {
     type master;
     file "{{zone.target_path}}";
 };
 {{endfor}}
 
-{{for rev_zone in rev_zones}}
-zone "{{rev_zone.name}}" {
+{{for zone in zones}}
+zone "{{zone.reverse_zone_name}}" {
     type master;
-    file "{{rev_zone.target_path}}";
+    file "{{zone.target_reverse_path}}";
 };
 {{endfor}}

=== modified file 'src/provisioningserver/dns/templates/zone.template'
--- src/provisioningserver/dns/templates/zone.template	2012-07-13 13:13:50 +0000
+++ src/provisioningserver/dns/templates/zone.template	2012-07-17 17:12:22 +0000
@@ -12,7 +12,6 @@
               )
 
     IN  NS  {{domain}}.
-{{domain}}.  IN A 127.0.0.1; TODO: use master worker IP.
-{{for hostname, ip in hostname_ip_mapping.items()}}
-{{hostname}} IN A {{ip}}
+{{for item_from, item_to in mapping.items()}}
+{{item_from}} IN {{type}} {{item_to}}
 {{endfor}}

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-07-13 13:45:11 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-07-17 17:12:22 +0000
@@ -41,6 +41,7 @@
     Contains,
     EndsWith,
     FileContains,
+    MatchesStructure,
     StartsWith,
     )
 
@@ -73,7 +74,7 @@
         self.patch(config, 'check_call', recorder)
         self.patch(conf, 'DNS_CONFIG_DIR', fake_dir)
         command = factory.getRandomString()
-        execute_rndc_command(command)
+        execute_rndc_command([command])
         rndc_conf_path = os.path.join(fake_dir, MAAS_RNDC_CONF_NAME)
         expected_command = ['rndc', '-c', rndc_conf_path, command]
         self.assertEqual((expected_command,), recorder.calls[0][0])
@@ -115,18 +116,20 @@
     def test_write_config_writes_config(self):
         target_dir = self.make_dir()
         self.patch(DNSConfig, 'target_dir', target_dir)
-        zone_names = [factory.getRandomString()]
-        reverse_zone_names = [factory.getRandomString()]
-        dnsconfig = DNSConfig(
-            zone_names=zone_names, reverse_zone_names=reverse_zone_names)
+        zone_name = factory.getRandomString()
+        zone = DNSZoneConfig(
+            zone_name, bcast='192.168.0.255',
+            mask='255.255.255.0',
+            mapping={factory.getRandomString(): '192.168.0.5'})
+        dnsconfig = DNSConfig(zones=[zone])
         dnsconfig.write_config()
         self.assertThat(
             os.path.join(target_dir, MAAS_NAMED_CONF_NAME),
             FileContains(
                 matcher=ContainsAll(
                     [
-                        'zone "%s"' % zone_names[0],
-                        'zone "%s.rev"' % reverse_zone_names[0],
+                        'zone.%s' % zone_name,
+                        'zone.rev.%s' % zone_name,
                         MAAS_NAMED_RNDC_CONF_NAME,
                     ])))
 
@@ -147,38 +150,118 @@
 class TestDNSZoneConfig(TestCase):
     """Tests for DNSZoneConfig."""
 
-    def test_name_returns_zone_name(self):
-        zone_name = factory.getRandomString()
-        dnszoneconfig = DNSZoneConfig(zone_name)
-        self.assertEqual(dnszoneconfig.name, '%s' % zone_name)
-
     def test_DNSZoneConfig_fields(self):
         zone_name = factory.getRandomString()
-        dnszoneconfig = DNSZoneConfig(zone_name)
+        serial = random.randint(1, 200)
+        bcast = factory.getRandomIPAddress()
+        mask = factory.getRandomIPAddress()
+        hostname = factory.getRandomString()
+        ip = factory.getRandomString()
+        mapping = {hostname: ip}
+        dns_zone_config = DNSZoneConfig(
+            zone_name, serial, mapping, bcast, mask)
+        self.assertThat(
+            dns_zone_config,
+            MatchesStructure.byEquality(
+                zone_name=zone_name,
+                serial=serial,
+                mapping=mapping,
+                bcast=bcast,
+                mask=mask,
+                )
+            )
+
+    def test_DNSZoneConfig_computes_dns_config_file_paths(self):
+        zone_name = factory.getRandomString()
+        dns_zone_config = DNSZoneConfig(zone_name)
         self.assertEqual(
             (
                 os.path.join(TEMPLATES_PATH, 'zone.template'),
-                os.path.join(conf.DNS_CONFIG_DIR, 'zone.%s' % zone_name)
-            ),
-            (dnszoneconfig.template_path, dnszoneconfig.target_path))
-
-    def test_write_config_writes_zone_config(self):
+                os.path.join(conf.DNS_CONFIG_DIR, 'zone.%s' % zone_name),
+                os.path.join(conf.DNS_CONFIG_DIR, 'zone.rev.%s' % zone_name)
+            ),
+            (
+                dns_zone_config.template_path,
+                dns_zone_config.target_path,
+                dns_zone_config.target_reverse_path,
+            ))
+
+    def test_DNSZoneConfig_reverse_data_slash_24(self):
+        # DNSZoneConfig calculates the reverse data correctly for
+        # a /24 network.
+        zone_name = factory.getRandomString()
+        hostname = factory.getRandomString()
+        dns_zone_config = DNSZoneConfig(
+            zone_name, bcast='192.168.0.255',
+            mask='255.255.255.0',
+            mapping={hostname: '192.168.0.5'})
+        self.assertEqual(
+            (
+                3,
+                {'5': '%s.%s.' % (hostname, zone_name)},
+                '0.168.192.in-addr.arpa',
+            ),
+            (
+                dns_zone_config.byte_num,
+                dns_zone_config.reverse_mapping,
+                dns_zone_config.reverse_zone_name,
+            ))
+
+    def test_DNSZoneConfig_reverse_data_slash_22(self):
+        # DNSZoneConfig calculates the reverse data correctly for
+        # a /22 network.
+        zone_name = factory.getRandomString()
+        hostname = factory.getRandomString()
+        dns_zone_config = DNSZoneConfig(
+            zone_name, bcast='192.12.3.255',
+            mask='255.255.252.0',
+            mapping={hostname: '192.12.2.10'})
+        self.assertEqual(
+            (
+                2,
+                {'10.2': '%s.%s.' % (hostname, zone_name)},
+                '12.192.in-addr.arpa',
+            ),
+            (
+                dns_zone_config.byte_num,
+                dns_zone_config.reverse_mapping,
+                dns_zone_config.reverse_zone_name,
+            ))
+
+    def test_DNSZoneConfig_writes_dns_zone_config(self):
         target_dir = self.make_dir()
         self.patch(DNSConfig, 'target_dir', target_dir)
         zone_name = factory.getRandomString()
-        dnszoneconfig = DNSZoneConfig(zone_name)
-        domain = factory.getRandomString()
-        serial = random.randint(1, 100)
         hostname = factory.getRandomString()
         ip = factory.getRandomIPAddress()
-        dnszoneconfig.write_config(
-            domain=domain, serial=serial,
-            hostname_ip_mapping={hostname: ip})
+        dns_zone_config = DNSZoneConfig(
+            zone_name, serial=random.randint(1, 100),
+            mapping={hostname: ip})
+        dns_zone_config.write_config()
         self.assertThat(
             os.path.join(target_dir, 'zone.%s' % zone_name),
             FileContains(
                 matcher=ContainsAll(
                     [
-                        'IN  NS  %s.' % domain,
+                        'IN  NS  %s.' % zone_name,
                         '%s IN A %s' % (hostname, ip),
                     ])))
+
+    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})
+        dns_zone_config.write_reverse_config()
+        self.assertThat(
+            os.path.join(target_dir, 'zone.rev.%s' % zone_name),
+            FileContains(
+                matcher=ContainsAll(
+                    ['%s IN PTR %s' % (
+                        '10.2',
+                        '%s.%s.' % (hostname, zone_name))])))

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-07-13 06:49:06 +0000
+++ src/provisioningserver/tasks.py	2012-07-17 17:12:22 +0000
@@ -24,7 +24,6 @@
 from celery.task import task
 from provisioningserver.dns.config import (
     DNSConfig,
-    DNSZoneConfig,
     execute_rndc_command,
     setup_rndc,
     )
@@ -94,81 +93,66 @@
 
 
 @task
-def rndc_command(*arguments):
-    """Use rndc to execute a command."""
-    execute_rndc_command(*arguments)
+def rndc_command(arguments, callback=None):
+    """Use rndc to execute a command.
+    :param callback: Callback subtask.
+    :type callback: callable
+    """
+    execute_rndc_command(arguments)
+    if callback is not None:
+        callback.delay()
 
 
 @task
-def write_full_dns_config(zones=None, reverse_zones=None,
-                          callback=None, **kwargs):
+def write_full_dns_config(zones=None, callback=None, **kwargs):
     """Write out the DNS configuration files: the main configuration
     file and the zone files.
-    :param zones: Mapping between zone names and the zone data used
-        to write the zone config files.
-    :type zones: dict
-    :param reverse_zones: Mapping between reverse zone names and the
-        reverse zone data used to write the reverse zone config
-        files.
-    :type reverse_zones: dict
+    :param zones: List of zones to write.
+    :type zones: list of :class:`DNSZoneData`
     :param callback: Callback subtask.
     :type callback: callable
     :param **kwargs: Keyword args passed to DNSConfig.write_config()
     """
-    if zones is None:
-        zones = {}
-    if reverse_zones is None:
-        reverse_zones = {}
-    # Write zone files.
-    for zone_name, zone_data in zones.items():
-        DNSZoneConfig(zone_name).write_config(**zone_data)
-    # TODO: Write reverse zone files.
-    # for zone_name, zone_data in zones.items():
-    #    DNSZoneConfig(zone_name).write_config(**zone_data)
+    if zones is not None:
+        for zone in zones:
+            zone.write_config()
+            zone.write_reverse_config()
     # Write main config file.
-    config = DNSConfig(
-        zone_names=list(zones),
-        reverse_zone_names=list(reverse_zones))
+    config = DNSConfig(zones=zones)
     config.write_config(**kwargs)
     if callback is not None:
         callback.delay()
 
 
 @task
-def write_dns_config(zone_names=(), reverse_zone_names=(),
-                     callback=None, **kwargs):
+def write_dns_config(zones=(), callback=None, **kwargs):
     """Write out the DNS configuration file.
 
-    :param zone_names: List of zone names to include as part of the
-        main config.
-    :type zone_names: list
-    :param reverse_zone_names: List of reverse zone names to include as part of
-        the main config.
-    :type reverse_zone_names: list
+    :param zones: List of zones to include as part of the main
+        config.
+    :type zones: list of :class:`DNSZoneData`
     :param callback: Callback subtask.
     :type callback: callable
     :param **kwargs: Keyword args passed to DNSConfig.write_config()
     """
-    config = DNSConfig(
-        zone_names=zone_names,
-        reverse_zone_names=reverse_zone_names)
+    config = DNSConfig(zones=zones)
     config.write_config(**kwargs)
     if callback is not None:
         callback.delay()
 
 
 @task
-def write_dns_zone_config(zone_name, callback=None, **kwargs):
+def write_dns_zone_config(zone, callback=None, **kwargs):
     """Write out a DNS zone configuration file.
 
-    :param zone_name: The identifier of the zone to write the configuration
-        for.
-    :type zone_name: basestring
+    :param zone: The zone data to write the configuration for.
+    :type zone: :class:`DNSZoneData`
     :param callback: Callback subtask.
     :type callback: callable
     :param **kwargs: Keyword args passed to DNSZoneConfig.write_config()
     """
-    DNSZoneConfig(zone_name).write_config(**kwargs)
+    zone.write_config()
+    zone.write_reverse_config()
     if callback is not None:
         callback.delay()
 

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-07-13 06:49:06 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-07-17 17:12:22 +0000
@@ -24,6 +24,7 @@
 from provisioningserver import tasks
 from provisioningserver.dns.config import (
     conf,
+    DNSZoneConfig,
     MAAS_NAMED_CONF_NAME,
     MAAS_NAMED_RNDC_CONF_NAME,
     MAAS_RNDC_CONF_NAME,
@@ -151,21 +152,26 @@
     def test_write_dns_zone_config_writes_file(self):
         command = factory.getRandomString()
         zone_name = factory.getRandomString()
+        zone = DNSZoneConfig(
+            zone_name, bcast='192.168.0.255',
+            mask='255.255.255.0',
+            serial=random.randint(1, 100),
+            mapping={factory.getRandomString(): '192.168.0.5'})
         result = write_dns_zone_config.delay(
-            zone_name=zone_name, domain=factory.getRandomString(),
-            serial=random.randint(1, 100), hostname_ip_mapping={},
-            callback=rndc_command.subtask(args=[command]))
+            zone=zone, callback=rndc_command.subtask(args=[command]))
 
         self.assertThat(
             (
                 result.successful(),
                 os.path.join(self.dns_conf_dir, 'zone.%s' % zone_name),
+                os.path.join(self.dns_conf_dir, 'zone.rev.%s' % zone_name),
                 self.rndc_recorder.calls,
             ),
             MatchesListwise(
                 (
                     Equals(True),
                     FileExists(),
+                    FileExists(),
                     Equals([((command, ), {})]),
                 )),
             result)
@@ -207,17 +213,12 @@
     def test_write_full_dns_config_sets_up_config(self):
         # write_full_dns_config writes the config file, writes
         # the zone files, and reloads the dns service.
-        hostname = factory.getRandomString()
-        ip = factory.getRandomIPAddress()
         zone_name = factory.getRandomString()
-        domain = factory.getRandomString()
-        zones = {
-            zone_name: {
-                'serial': random.randint(1, 100),
-                'hostname_ip_mapping': {hostname: ip},
-                'domain': domain,
-            }
-        }
+        zones = [DNSZoneConfig(
+            zone_name, bcast='192.168.0.255',
+            mask='255.255.255.0',
+            serial=random.randint(1, 100),
+            mapping={factory.getRandomString(): '192.168.0.5'})]
         command = factory.getRandomString()
         result = write_full_dns_config.delay(
             zones=zones,
@@ -230,6 +231,8 @@
                 os.path.join(
                     self.dns_conf_dir, 'zone.%s' % zone_name),
                 os.path.join(
+                    self.dns_conf_dir, 'zone.rev.%s' % zone_name),
+                os.path.join(
                     self.dns_conf_dir, MAAS_NAMED_CONF_NAME),
             ),
             MatchesListwise(
@@ -238,4 +241,5 @@
                     Equals([((command,), {})]),
                     FileExists(),
                     FileExists(),
+                    FileExists(),
                 )))