← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/maas/dns-config-tasks into lp:maas

 

Raphaël Badin has proposed merging lp:~rvb/maas/dns-config-tasks into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/dns-config-tasks/+merge/113588

This branch adds the tasks required to write the DNS config files to provisioningserver.tasks.

= Notes =

I've added the ability to run rndc commands.

I've updated the two templates (src/provisioningserver/dns/templates/zone.template and src/provisioningserver/dns/templates/named.conf.template) so that:
- all the paths are relative to {{ DNS_CONFIG_DIR }}
Anyway, I'm afraid these templates will need to be seriously rewritten (they were copied verbatim from cobbler) so I didn't spend too much time on them right now.

In case you wonder, the 'reload_config' parameter in the tasks will allow us to write many configuration files at once and reload the DNS server only at the end (and only once).  But obviously, the default is to reload the DNS server each time the configuration is modified.

Note that I considered extracting:
"""
if reload_config:
    subtask(reload_dns_config.subtask()).delay()
"""
…in a central location but none of the solutions I found was really appealing: my main alternative was to put that code in a class inheriting from celery's Task class and use that class as a base for all the tasks.  But writing tasks as classes makes the code really obscure. At the expense of a little duplication, I think the code is much more clean and simple this way.

Drive-by fixes:
- improve assertion in TestTFTPTasks.test_write_tftp_config_for_node_writes_files
- use "from celery.task import task" instead of "from celery.decorators import task" as the latter is deprecated.
-- 
https://code.launchpad.net/~rvb/maas/dns-config-tasks/+merge/113588
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/dns-config-tasks into lp:maas.
=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-07-04 09:47:09 +0000
+++ src/provisioningserver/dns/config.py	2012-07-05 14:00:28 +0000
@@ -23,7 +23,10 @@
     abstractproperty,
     )
 import os.path
-from subprocess import check_output
+from subprocess import (
+    check_call,
+    check_output,
+    )
 
 from celery.conf import conf
 import tempita
@@ -75,6 +78,12 @@
         f.write(named_content)
 
 
+def execute_rndc_command(command):
+    """Execute a rndc command."""
+    rndc_conf = os.path.join(conf.DNS_CONFIG_DIR, 'rndc.conf')
+    check_call(['rndc', '-c', rndc_conf, command])
+
+
 # Directory where the DNS configuration template files can be found.
 TEMPLATES_PATH = os.path.join(os.path.dirname(__file__), 'templates')
 
@@ -108,9 +117,15 @@
         except NameError as error:
             raise DNSConfigFail(*error.args)
 
+    def get_extra_context(self):
+        """Dictionary containing extra 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())
         rendered = self.render_template(template, **kwargs)
         with open(self.target_path, "wb") as f:
             f.write(rendered)
@@ -133,6 +148,11 @@
     def target_path(self):
         return os.path.join(self.target_dir, self.target_file_name)
 
+    def get_extra_context(self):
+        return {
+            'DNS_CONFIG_DIR': conf.DNS_CONFIG_DIR,
+        }
+
 
 class BlankDNSConfig(DNSConfig):
     """A specialized version of DNSConfig that simply writes a blank/empty

=== modified file 'src/provisioningserver/dns/templates/named.conf.template'
--- src/provisioningserver/dns/templates/named.conf.template	2012-06-26 08:01:06 +0000
+++ src/provisioningserver/dns/templates/named.conf.template	2012-07-05 14:00:28 +0000
@@ -1,10 +1,12 @@
 
+include "{{ DNS_CONFIG_DIR }}/named.conf.rndc";
+
 options {
         listen-on port 53 { any; };
-        directory       "/var/named";
-        dump-file       "/var/named/data/cache_dump.db";
-        statistics-file "/var/named/data/named_stats.txt";
-        memstatistics-file "/var/named/data/named_mem_stats.txt";
+        directory       "{{ DNS_CONFIG_DIR }}/";
+        dump-file       "{{ DNS_CONFIG_DIR }}/data/cache_dump.db";
+        statistics-file "{{ DNS_CONFIG_DIR }}/data/named_stats.txt";
+        memstatistics-file "{{ DNS_CONFIG_DIR }}/data/named_mem_stats.txt";
         recursion yes;
 };
 
@@ -16,17 +18,16 @@
 };
 
 # Zone declarations
-
-{{for zone_bunch in zones}}
-zone "{{zone_bunch.zone}}." {
+{{for id in zone_ids}}
+zone "{{id}}" {
     type master;
-    file "{{zone_bunch.file}}";
+    file "{{ DNS_CONFIG_DIR }}/zone.{{id}}";
 };
 {{endfor}}
 
-{{for reverse_bunch in reverse_zones}}
-zone "{{reverse_bunch.arpa}}." {
+{{for id in zone_ids}}
+zone "{{id}}.rev" {
     type master;
-    file "{{reverse_bunch.file}}";
+    file "{{ DNS_CONFIG_DIR }}/zone.rev.{{id}}";
 };
 {{endfor}}

=== modified file 'src/provisioningserver/dns/templates/zone.template'
--- src/provisioningserver/dns/templates/zone.template	2012-06-26 08:01:06 +0000
+++ src/provisioningserver/dns/templates/zone.template	2012-07-05 14:00:28 +0000
@@ -1,5 +1,5 @@
 $TTL 300
-@                       IN      SOA     {{domain}}. nobody.example.com. (
+@                       IN      SOA     {{maas_server}}. nobody.example.com. (
                                         {{serial}}    ; Serial (increment on changes)
                                         600         ; Refresh
                                         1800        ; Retry
@@ -10,6 +10,6 @@
                         IN      NS      {{maas_server}}.
 
 
-{{for host_bunch in hosts}}
-{{host_bunch.host}}   IN  A  {{host_bunch.ip}}
+{{for host in hosts}}
+{{host.hostname}}   IN  A  {{host.ip}}
 {{endfor}}

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-07-04 09:47:09 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-07-05 14:00:28 +0000
@@ -17,12 +17,15 @@
 
 from celery.conf import conf
 from maastesting.factory import factory
+from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
+from provisioningserver.dns import config
 from provisioningserver.dns.config import (
     BlankDNSConfig,
     DNSConfig,
     DNSConfigFail,
     DNSZoneConfig,
+    execute_rndc_command,
     generate_rndc,
     setup_rndc,
     TEMPLATES_PATH,
@@ -31,7 +34,7 @@
 from testtools.matchers import FileContains
 
 
-class TestRNDCGeneration(TestCase):
+class TestRNDCUtilities(TestCase):
 
     def test_generate_rndc_returns_configurations(self):
         rndc_content, named_content = generate_rndc()
@@ -53,6 +56,18 @@
                 conf_content = stream.read()
                 self.assertIn(content, conf_content)
 
+    def test_execute_rndc_command_executes_command(self):
+        recorder = FakeMethod()
+        fake_dir = factory.getRandomString()
+        self.patch(config, 'check_call', recorder)
+        self.patch(conf, 'DNS_CONFIG_DIR', fake_dir)
+        command = factory.getRandomString()
+        execute_rndc_command(command)
+        rndc_conf_path = os.path.join(fake_dir, 'rndc.conf')
+        self.assertSequenceEqual(
+            [((['rndc', '-c', rndc_conf_path, command],), {})],
+            recorder.calls)
+
 
 class TestDNSConfig(TestCase):
     """Tests for DNSConfig."""

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-06-26 04:22:16 +0000
+++ src/provisioningserver/tasks.py	2012-07-05 14:00:28 +0000
@@ -13,10 +13,19 @@
 __all__ = [
     'power_off',
     'power_on',
+    'reload_dns_config',
     ]
 
 
-from celery.decorators import task
+from celery.task import task
+from celery.task.sets import subtask
+from provisioningserver.dns.config import (
+    BlankDNSConfig,
+    DNSConfig,
+    DNSZoneConfig,
+    execute_rndc_command,
+    setup_rndc,
+    )
 from provisioningserver.power.poweraction import (
     PowerAction,
     PowerActionFail,
@@ -80,3 +89,59 @@
     for mac in macs:
         pxeconfig = PXEConfig(arch, subarch, mac, tftproot)
         pxeconfig.write_config(**kwargs)
+
+
+@task
+def reload_dns_config():
+    """Use rndc to reload the DNS configuration."""
+    execute_rndc_command('reload')
+
+
+@task
+def write_dns_config(blank=False, reload_config=True, **kwargs):
+    """Write out the DNS configuration file.
+
+    :param blank: Whether or not a blank configuration should be written.
+        False by default.
+    :type blank: boolean
+    :param reload_config: Whether or not to reload the configuration after it
+        has been written.  True by default.
+    :type reload_config: boolean
+    :param **kwargs: Keyword args passed to DNSConfig.write_config()
+    """
+    if blank:
+        BlankDNSConfig().write_config()
+    else:
+        DNSConfig().write_config(**kwargs)
+    if reload_config:
+        subtask(reload_dns_config.subtask()).delay()
+
+
+@task
+def write_dns_zone_config(zone_id, reload_config=True, **kwargs):
+    """Write out a DNS zone configuration file.
+
+    :param id: The identifier of the zone to write the configuration for.
+    :type id: int
+    :param reload_config: Whether or not to reload the configuration after it
+        has been written.  True by default.
+    :type reload_config: boolean
+    :param **kwargs: Keyword args passed to DNSZoneConfig.write_config()
+    """
+    DNSZoneConfig(zone_id).write_config(**kwargs)
+    if reload_config:
+        subtask(reload_dns_config.subtask()).delay()
+
+
+@task
+def setup_rndc_configuration(reload_config=True):
+    """Write out the two rndc configuration files (rndc.conf and
+    named.conf.rndc).
+
+    :param reload_config: Whether or not to reload the configuration after it
+        has been written.  True by default.
+    :type reload_config: boolean
+    """
+    setup_rndc()
+    if reload_config:
+        subtask(reload_dns_config.subtask()).delay()

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-06-26 04:22:16 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-07-05 14:00:28 +0000
@@ -13,21 +13,34 @@
 __all__ = []
 
 import os
+import random
 
 from maasserver.enum import ARCHITECTURE
 from maastesting.celery import CeleryFixture
 from maastesting.factory import factory
+from maastesting.fakemethod import FakeMethod
 from maastesting.matchers import ContainsAll
 from maastesting.testcase import TestCase
+from provisioningserver import tasks
+from provisioningserver.dns.config import conf
 from provisioningserver.enum import POWER_TYPE
 from provisioningserver.power.poweraction import PowerActionFail
 from provisioningserver.tasks import (
     power_off,
     power_on,
+    setup_rndc_configuration,
+    write_dns_config,
+    write_dns_zone_config,
     write_tftp_config_for_node,
     )
 from testresources import FixtureResource
-from testtools.matchers import FileContains
+from testtools.matchers import (
+    AllMatch,
+    Equals,
+    FileContains,
+    FileExists,
+    MatchesListwise,
+    )
 
 # An arbitrary MAC address.  Not using a properly random one here since
 # we might accidentally affect real machines on the network.
@@ -84,8 +97,124 @@
             tftproot, 'maas', arch, "generic", "pxelinux.cfg",
             mac2.replace(":", "-"))
         self.assertThat(
-            expected_file1,
-            FileContains(matcher=ContainsAll((kernel, menutitle, append))))
-        self.assertThat(
-            expected_file2,
-            FileContains(matcher=ContainsAll((kernel, menutitle, append))))
+            [expected_file1, expected_file2],
+            AllMatch(
+                FileContains(
+                    matcher=ContainsAll((kernel, menutitle, append)))))
+
+
+class TestDNSTasks(TestCase):
+
+    resources = (
+        ("celery", FixtureResource(CeleryFixture())),
+        )
+
+    def setUp(self):
+        super(TestDNSTasks, self).setUp()
+        # Path DNS_CONFIG_DIR so that the configuration files will be
+        # written in a temporary directory.
+        self.dns_conf_dir = self.make_dir()
+        self.patch(conf, 'DNS_CONFIG_DIR', self.dns_conf_dir)
+        # Record the calls to 'execute_rndc_command' (instead of
+        # executing real rndc commands).
+        self.rndc_recorder = FakeMethod()
+        self.patch(tasks, 'execute_rndc_command', self.rndc_recorder)
+
+    def test_write_dns_config_writes_file(self):
+        zone_ids = [random.randint(1, 100), random.randint(1, 100)]
+        result = write_dns_config.delay(blank=False, zone_ids=zone_ids)
+
+        self.assertThat(
+            (
+                result.successful(),
+                os.path.join(self.dns_conf_dir, 'named.conf'),
+                self.rndc_recorder.calls,
+            ),
+            MatchesListwise(
+                (
+                    Equals(True),
+                    FileExists(),
+                    Equals([(('reload',), {})]),
+                )),
+            result)
+
+    def test_write_dns_config_with_blank_True(self):
+        result = write_dns_config.delay(blank=True)
+
+        self.assertThat(
+            (
+                result.successful(),
+                os.path.join(self.dns_conf_dir, 'named.conf'),
+                self.rndc_recorder.calls,
+            ),
+            MatchesListwise(
+                (
+                    Equals(True),
+                    FileContains(''),
+                    Equals([(('reload',), {})]),
+                )),
+            result)
+
+    def test_write_dns_config_skip_config_reload(self):
+        result = write_dns_config.delay(blank=True, reload_config=False)
+
+        self.assertEqual(
+            (result.successful(), self.rndc_recorder.call_count),
+            (True, 0))
+
+    def test_write_dns_zone_config_writes_file(self):
+        zone_id = random.randint(1, 100)
+        result = write_dns_zone_config.delay(
+            zone_id=zone_id, maas_server=factory.getRandomString(),
+            serial=random.randint(1, 100), hosts=[])
+
+        self.assertThat(
+            (
+                result.successful(),
+                os.path.join(self.dns_conf_dir, 'zone.%d' % zone_id),
+                self.rndc_recorder.calls,
+            ),
+            MatchesListwise(
+                (
+                    Equals(True),
+                    FileExists(),
+                    Equals([(('reload',), {})]),
+                )),
+            result)
+
+    def test_write_dns_zone_config_skip_config_reload(self):
+        zone_id = random.randint(1, 100)
+        result = write_dns_zone_config.delay(
+            zone_id=zone_id, maas_server=factory.getRandomString(),
+            serial=random.randint(1, 100), hosts=[],
+            reload_config=False)
+
+        self.assertEqual(
+            (result.successful(), self.rndc_recorder.call_count),
+            (True, 0))
+
+    def test_setup_rndc_configuration_writes_files(self):
+        result = setup_rndc_configuration.delay()
+
+        self.assertThat(
+            (
+                result.successful(),
+                os.path.join(self.dns_conf_dir, 'rndc.conf'),
+                os.path.join(self.dns_conf_dir, 'named.conf.rndc'),
+                self.rndc_recorder.calls,
+            ),
+            MatchesListwise(
+                (
+                    Equals(True),
+                    FileExists(),
+                    FileExists(),
+                    Equals([(('reload',), {})]),
+                )),
+            result)
+
+    def test_setup_rndc_configuration_skip_config_reload(self):
+        result = setup_rndc_configuration.delay(reload_config=False)
+
+        self.assertEqual(
+            (result.successful(), self.rndc_recorder.call_count),
+            (True, 0))