Merge lp:~julian-edwards/maas/upstream-dns into lp:~maas-committers/maas/trunk

[Julian Edwards]

Have a DNS forwarders setting ("upstream DNS" in the settings UI) which is written to a special file that can be included in the DNS options {} block.

Add a new Config setting for upstream DNS ("forwarders" in the DNS config) which is written out to a file called named.conf.options.inside.maas from the similarly-named template. The file can then be used in an 'include' directive from the options {} block in the system's DNS config.

This branch turned out to be much bigger than expected because you can't have more than one options block (which would have been easy to just throw in our existing template for DNS). I got this technique from LaMont who set it up this way for his Puppet stuff.

I've also amended the BindFixture so it can take this option although it doesn't use it anywhere when you "make run", it's only set up in tests.

We will also need to make a change in the packaging so that the new file gets included in the named.conf.options file.

[Jeroen T. Vermeulen (jtv)]

The branch as a whole looks great, with lots of helpful explanation. It brings out the simplicity in what, in the wrong hands, could have been short but incomprehensible logic. As always though, I have remarks!

First off, I have a small suggestion for the UI label in src/maasserver/forms_settings.py (form_kwargs, ll. 81—88 of the diff):

            'label': "Upstream DNS used to resolve non-MAAS domains",

Instead of "non-MAAS domains" you could say something like "domains not managed by this MAAS," to avoid confusion? For example I imagine a beginning user might mistake "non-MAAS domains" for a way of saying "DNS domains, not domains as in MAAS domain controller."

.

In src/maastesting/tests/test_bindfixture.py, I don't get what test_setUp_honours_include_in_options does with “forwarders” and “expected_in_file” (ll. 255—258 of the diff):

        forwarders = "forwarders { 1.2.3.4; };"
….
            expected_in_file = (
                resources.homedir + '/' + forwarders.encode("ascii"))

It looks as if you're doing effectively an os.path.join() on a directory and the actual DNS options text. Didn't you mean for “forwarders” to look like a filename?

Also, I don't know about resources.homedir, but '/' is unicode and forwarders.encode("ascii") is bytes, right? What's the point of the encoding? If you mean to produce the whole thing as bytes, why not encode the whole thing at once with os.path.join(...).encode('ascii')? If resources.homedir is still raw bytes in python3, we can probably fix it in the BIND fixture.

.

In src/provisioningserver/dns/config.py, set_up_options_conf (ll. 290—317 of the diff):

    """Write out the named.conf.options.inside.maas file.

    This file should be included by the top-level named.conf.options
    inside its 'options' block. MAAS cannot write the options file itself,
    so relies on either the DNSFixture in the test suite, or the packaging.
    Both should set that file up appropriately to include our file.
    """

Only a small nitpick, but I'd lead with the production situation ("MAAS cannot write the options file itself, so relies on the packaging") and make the equivalent test rigging parenthetical.

And to make up for that nitpick, here are a few tricks to offload some work from this function:

    template_path = os.path.join(
        locate_config(TEMPLATES_DIR),
        "named.conf.options.inside.maas.template")

...can be simplified to:

    template_path = locate_config(
        TEMPLATES_DIR, "named.conf.options.inside.maas.template")

Next there's...

    with open(template_path, "r") as f:
        template = tempita.Template(f.read(), name=template_path)

Tempita has a Template factory called from_filename which does the reading for you, and also passes the filename as the template name.

So try simply:

    template = tempita.Template.from_filename(template_path)

.

In src/provisioningserver/dns/tests/test_config.py, test_set_up_options_conf_writes_configuration hard-codes an IP address (l. 353 of the diff):

        fake_dns = "1.2.3.4"

We have a factory method for that! Use getRandomIPAddress().

.

In that same file, test_set_up_options_conf_raises_on_bad_template does thi...

review: Approve

[Julian Edwards (julian-edwards)]

Thanks for the review Mr V.

On 08/11/13 15:21, Jeroen T. Vermeulen wrote:
> Instead of "non-MAAS domains" you could say something like "domains not managed by this MAAS," to avoid confusion? For example I imagine a beginning user might mistake "non-MAAS domains" for a way of saying "DNS domains, not domains as in MAAS domain controller."

Ok changed, thanks.

> In src/maastesting/tests/test_bindfixture.py, I don't get what test_setUp_honours_include_in_options does with “forwarders” and “expected_in_file” (ll. 255—258 of the diff):
>
> forwarders = "forwarders { 1.2.3.4; };"
> ….
> expected_in_file = (
> resources.homedir + '/' + forwarders.encode("ascii"))
>
> It looks as if you're doing effectively an os.path.join() on a directory and the actual DNS options text. Didn't you mean for “forwarders” to look like a filename?

It's checking text exactly as it should appear in the file -
os.path.join would be inappropriate here.

> Also, I don't know about resources.homedir, but '/' is unicode and forwarders.encode("ascii") is bytes, right? What's the point of the encoding? If you mean to produce the whole thing as bytes, why not encode the whole thing at once with os.path.join(...).encode('ascii')? If resources.homedir is still raw bytes in python3, we can probably fix it in the BIND fixture.

Grar, that's a leftover from fiddling, I fixed it. Thanks for spotting.

> In src/provisioningserver/dns/config.py, set_up_options_conf (ll. 290—317 of the diff):
>
> """Write out the named.conf.options.inside.maas file.
>
> This file should be included by the top-level named.conf.options
> inside its 'options' block. MAAS cannot write the options file itself,
> so relies on either the DNSFixture in the test suite, or the packaging.
> Both should set that file up appropriately to include our file.
> """
>
> Only a small nitpick, but I'd lead with the production situation ("MAAS cannot write the options file itself, so relies on the packaging") and make the equivalent test rigging parenthetical.

Mind if I ignore that? Mostly because I'm not sure what you want and it
makes perfect sense already :)

> And to make up for that nitpick, here are a few tricks to offload some work from this function:
>
> template_path = os.path.join(
> locate_config(TEMPLATES_DIR),
> "named.conf.options.inside.maas.template")
>
> ...can be simplified to:
>
> template_path = locate_config(
> TEMPLATES_DIR, "named.conf.options.inside.maas.template")

You sure about that?

>
> Next there's...
>
> with open(template_path, "r") as f:
> template = tempita.Template(f.read(), name=template_path)
>
> Tempita has a Template factory called from_filename which does the reading for you, and also passes the filename as the template name.
>
> So try simply:
>
> template = tempita.Template.from_filename(template_path)

Right! Fixed.

>
> .
>
> In src/provisioningserver/dns/tests/test_config.py, test_set_up_options_conf_writes_configuration hard-codes an IP address (l. 353 of the diff):
>
> fake_dns = "1.2.3.4"
>
> We have a factory method for that! Use ge...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~julian-edwards/maas/upstream-dns into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com saucy-security InRelease
Get:1 http://security.ubuntu.com saucy-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com saucy-security Release [49.6 kB]
Ign http://nova.clouds.archive.ubuntu.com saucy InRelease
Ign http://nova.clouds.archive.ubuntu.com saucy-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com saucy Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com saucy-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com saucy Release
Get:4 http://nova.clouds.archive.ubuntu.com saucy-updates Release [49.6 kB]
Get:5 http://security.ubuntu.com saucy-security/main Sources [7,939 B]
Get:6 http://security.ubuntu.com saucy-security/universe Sources [4,315 B]
Hit http://nova.clouds.archive.ubuntu.com saucy/main Sources
Hit http://nova.clouds.archive.ubuntu.com saucy/universe Sources
Get:7 http://security.ubuntu.com saucy-security/main amd64 Packages [32.4 kB]
Hit http://nova.clouds.archive.ubuntu.com saucy/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com saucy/universe amd64 Packages
Get:8 http://security.ubuntu.com saucy-security/universe amd64 Packages [9,325 B]
Hit http://nova.clouds.archive.ubuntu.com saucy/main Translation-en
Hit http://security.ubuntu.com saucy-security/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com saucy/universe Translation-en
Hit http://security.ubuntu.com saucy-security/universe Translation-en
Ign http://security.ubuntu.com saucy-security/main Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com saucy-updates/main Sources [28.0 kB]
Ign http://security.ubuntu.com saucy-security/universe Translation-en_US
Get:10 http://nova.clouds.archive.ubuntu.com saucy-updates/universe Sources [8,509 B]
Get:11 http://nova.clouds.archive.ubuntu.com saucy-updates/main amd64 Packages [70.5 kB]
Get:12 http://nova.clouds.archive.ubuntu.com saucy-updates/universe amd64 Packages [28.1 kB]
Hit http://nova.clouds.archive.ubuntu.com saucy-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com saucy-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com saucy/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com saucy/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com saucy-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com saucy-updates/universe Translation-en_US
Fetched 290 kB in 0s (1,411 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 avahi-daemon avahi-utils bind9 bind9utils build-essential curl daemontools distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql-9.1 python-amqplib python-avahi python-bzrlib python-celery python-convoy python-cssselect python-curtin python-dbus python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httplib2 python-jinja2 python-lockfile python-lxml pyth...

[Julian Edwards (julian-edwards)]

Idiot linter.

[Jeroen T. Vermeulen (jtv)]

Julian wrote:

> > In src/maastesting/tests/test_bindfixture.py, I don't get what test_setUp_honours_include_in_options does with “forwarders” and “expected_in_file” (ll. 255—258 of the diff):
> >
> > forwarders = "forwarders { 1.2.3.4; };"
> > ….
> > expected_in_file = (
> > resources.homedir + '/' + forwarders.encode("ascii"))
> >
> > It looks as if you're doing effectively an os.path.join() on a directory and the actual DNS options text. Didn't you mean for “forwarders” to look like a filename?
>
> It's checking text exactly as it should appear in the file -
> os.path.join would be inappropriate here.

Not my point. My point is that you seem to be conflating the text of the snippet that you want to include, with the path to the file that contains the snippet. You're constructing something like

    /tmp/tmp4SANQ84/forwarders { 1.2.3.4; };

Not what you meant, right? AFAICS you meant "forwarders" to look like a filename, not a piece of configuration.

.

> > template_path = os.path.join(
> > locate_config(TEMPLATES_DIR),
> > "named.conf.options.inside.maas.template")
> >
> > ...can be simplified to:
> >
> > template_path = locate_config(
> > TEMPLATES_DIR, "named.conf.options.inside.maas.template")
>
> You sure about that?

Quite sure, thank you.

.

> > And finally,
> >
> > 436 FileExists(),
> > 437 FileExists(),
> > 438 FileExists(),
> > 439 + FileExists(),
> >
> > WTF? :-)
>
> It's one of those crazy mass-checks that Raphers is so fond of. I just
> made it a bit bigger :)

And therefore, better.

Jeroen

[Julian Edwards (julian-edwards)]

On 08/11/13 17:41, Jeroen T. Vermeulen wrote:
> Julian wrote:
>
>>> In src/maastesting/tests/test_bindfixture.py, I don't get what test_setUp_honours_include_in_options does with “forwarders” and “expected_in_file” (ll. 255—258 of the diff):
>>>
>>> forwarders = "forwarders { 1.2.3.4; };"
>>> ….
>>> expected_in_file = (
>>> resources.homedir + '/' + forwarders.encode("ascii"))
>>>
>>> It looks as if you're doing effectively an os.path.join() on a directory and the actual DNS options text. Didn't you mean for “forwarders” to look like a filename?
>>
>> It's checking text exactly as it should appear in the file -
>> os.path.join would be inappropriate here.
>
> Not my point. My point is that you seem to be conflating the text of the snippet that you want to include, with the path to the file that contains the snippet. You're constructing something like
>
> /tmp/tmp4SANQ84/forwarders { 1.2.3.4; };
>
> Not what you meant, right? AFAICS you meant "forwarders" to look like a filename, not a piece of configuration.

No - I want it to look like configuration. You're the one who's
conflating two things here:

 1. The include file path
 2. The contents of the included file

The snippet you have above from test_bindfixture.py is very misleading,
remember that there's TWO files I am dealing with; the top-level options
that *includes* the other file which contains the "forwarders" option.

>
> .
>
>>> template_path = os.path.join(
>>> locate_config(TEMPLATES_DIR),
>>> "named.conf.options.inside.maas.template")
>>>
>>> ...can be simplified to:
>>>
>>> template_path = locate_config(
>>> TEMPLATES_DIR, "named.conf.options.inside.maas.template")
>>
>> You sure about that?
>
> Quite sure, thank you.

Why does the rest of the code need to use it then? It takes into
account the environment to override the template location.

> And therefore, better.

\m/

Or something…

:)

[Julian Edwards (julian-edwards)]

On 11/11/13 09:42, Julian Edwards wrote:
> No - I want it to look like configuration. You're the one who's
> conflating two things here:
>
> 1. The include file path
> 2. The contents of the included file
>
> The snippet you have above from test_bindfixture.py is very misleading,
> remember that there's TWO files I am dealing with; the top-level options
> that *includes* the other file which contains the "forwarders" option.

Ok as soon as I write this I see why I sent you up the garden path....

forwarders = "forwarders { 1.2.3.4; };"

should read:

forwarders = "named.conf.options.inside.maas".

I will fix this in a subsequent branch. Sorry…

=== added file 'etc/maas/templates/dns/named.conf.options.inside.maas.template'
--- etc/maas/templates/dns/named.conf.options.inside.maas.template	1970-01-01 00:00:00 +0000
+++ etc/maas/templates/dns/named.conf.options.inside.maas.template	2013-11-08 06:27:21 +0000
@@ -0,0 +1,6 @@
+{{if upstream_dns}}
+    forwarders {
+        {{upstream_dns}};
+    };
+{{endif}}
+

=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py	2013-10-18 16:57:37 +0000
+++ src/maasserver/dns.py	2013-11-08 06:27:21 +0000
@@ -37,6 +37,7 @@
     )
 from maasserver.exceptions import MAASException
 from maasserver.models import (
+    Config,
     DHCPLease,
     NodeGroup,
     NodeGroupInterface,
@@ -312,7 +313,9 @@
     if not write_conf:
         return
     zones = ZoneGenerator(NodeGroup.objects.all()).as_list()
+    upstream_dns = Config.objects.get_config("upstream_dns")
     tasks.write_full_dns_config.delay(
         zones=zones,
         callback=tasks.rndc_command.subtask(
-            args=[['reload'], reload_retry]))
+            args=[['reload'], reload_retry]),
+        upstream_dns=upstream_dns)

=== modified file 'src/maasserver/dns_connect.py'
--- src/maasserver/dns_connect.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/dns_connect.py	2013-11-08 06:27:21 +0000
@@ -23,6 +23,7 @@
 from django.dispatch import receiver
 from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
 from maasserver.models import (
+    Config,
     Node,
     NodeGroup,
     NodeGroupInterface,
@@ -88,3 +89,11 @@
 
 
 connect_to_field_change(dns_post_edit_hostname_Node, Node, 'hostname')
+
+
+def upstream_dns_changed(sender, instance, created, **kwargs):
+    from maasserver.dns import write_full_dns_config
+    write_full_dns_config()
+
+
+Config.objects.config_changed_connect("upstream_dns", upstream_dns_changed)

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2013-10-15 11:51:48 +0000
+++ src/maasserver/forms.py	2013-11-08 06:27:21 +0000
@@ -626,6 +626,7 @@
     maas_name = get_config_field('maas_name')
     enlistment_domain = get_config_field('enlistment_domain')
     http_proxy = get_config_field('http_proxy')
+    upstream_dns = get_config_field('upstream_dns')
 
 
 class CommissioningForm(ConfigForm):

=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/forms_settings.py	2013-11-08 06:27:21 +0000
@@ -170,6 +170,20 @@
                 "proxy).")
         }
     },
+    'upstream_dns': {
+        'default': None,
+        'form': forms.GenericIPAddressField,
+        'form_kwargs': {
+            'label': (
+                "Upstream DNS used to resolve domains not managed by this "
+                "MAAS"),
+            'required': False,
+            'help_text': (
+                "Only used when MAAS is running its own DNS server. This "
+                "value is used as the value of 'forwarders' in the DNS "
+                "server config.")
+        }
+    },
     'default_distro_series': {
         'default': DISTRO_SERIES.precise,
         'form': forms.ChoiceField,

=== modified file 'src/maasserver/management/commands/set_up_dns.py'
--- src/maasserver/management/commands/set_up_dns.py	2013-10-18 16:57:37 +0000
+++ src/maasserver/management/commands/set_up_dns.py	2013-11-08 06:27:21 +0000
@@ -26,8 +26,10 @@
 from optparse import make_option
 
 from django.core.management.base import BaseCommand
+from maasserver.models import Config
 from provisioningserver.dns.config import (
     DNSConfig,
+    set_up_options_conf,
     setup_rndc,
     )
 
@@ -49,6 +51,9 @@
     def handle(self, *args, **options):
         no_clobber = options.get('no_clobber')
         setup_rndc()
+        upstream_dns = Config.objects.get_config("upstream_dns")
+        set_up_options_conf(
+            overwrite=not no_clobber, upstream_dns=upstream_dns)
         config = DNSConfig()
         config.write_config(
             overwrite=not no_clobber, zone_names=(), reverse_zone_names=())

=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py	2013-10-18 17:02:17 +0000
+++ src/maasserver/models/config.py	2013-11-08 06:27:21 +0000
@@ -56,6 +56,7 @@
         'default_distro_series': DISTRO_SERIES.precise,
         'commissioning_distro_series': DISTRO_SERIES.precise,
         'http_proxy': None,
+        'upstream_dns': None,
         ## /settings
         }
 

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2013-10-18 16:35:07 +0000
+++ src/maasserver/tests/test_dns.py	2013-11-08 06:27:21 +0000
@@ -30,7 +30,10 @@
     NODEGROUP_STATUS,
     NODEGROUPINTERFACE_MANAGEMENT,
     )
-from maasserver.models import node as node_module
+from maasserver.models import (
+    Config,
+    node as node_module,
+    )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maastesting.bindfixture import BINDServer
@@ -38,6 +41,7 @@
 from maastesting.fakemethod import FakeMethod
 from maastesting.tests.test_bindfixture import dig_call
 from mock import (
+    ANY,
     call,
     Mock,
     )
@@ -285,9 +289,19 @@
         self.assertEqual(
             ([(['reload'], True)]), recorder.extract_args())
 
+    def test_write_full_dns_passes_upstream_dns_parameter(self):
+        self.patch(settings, 'DNS_CONNECT', True)
+        self.create_managed_nodegroup()
+        random_ip = factory.getRandomIPAddress()
+        Config.objects.set_config("upstream_dns", random_ip)
+        patched_task = self.patch(dns.tasks.write_full_dns_config, "delay")
+        dns.write_full_dns_config()
+        patched_task.assert_called_once_with(
+            zones=ANY, callback=ANY, upstream_dns=random_ip)
+
     def test_write_full_dns_doesnt_call_task_it_no_interface_configured(self):
         self.patch(settings, 'DNS_CONNECT', True)
-        patched_task = self.patch(tasks, 'write_full_dns_config')
+        patched_task = self.patch(dns.tasks.write_full_dns_config, "delay")
         dns.write_full_dns_config()
         self.assertEqual(0, patched_task.call_count)
 

=== modified file 'src/maastesting/bindfixture.py'
--- src/maastesting/bindfixture.py	2013-10-18 16:57:37 +0000
+++ src/maastesting/bindfixture.py	2013-11-08 06:27:21 +0000
@@ -69,6 +69,8 @@
     :ivar homedir: A directory where to put all the files the
         BIND server needs (configuration files and executable).
     :ivar log_file: The log file allocated for the server.
+    :ivar include_in_options: Name of a file under homedir to include inside
+        the options block.
     """
 
     # The full path where the 'named' executable can be
@@ -88,6 +90,9 @@
         listen-on port {{port}} {127.0.0.1;};
         pid-file "{{homedir}}/named.pid";
         session-keyfile "{{homedir}}/session.key";
+        {{if include_in_options}}
+        include "{{homedir}}/{{include_in_options}}";
+        {{endif}}
       };
 
       logging{
@@ -105,13 +110,14 @@
     """))
 
     def __init__(self, port=None, rndc_port=None, homedir=None,
-                 log_file=None):
+                 log_file=None, include_in_options=None):
         super(BINDServerResources, self).__init__()
         self._defaults = dict(
             port=port,
             rndc_port=rndc_port,
             homedir=homedir,
             log_file=log_file,
+            include_in_options=include_in_options,
             )
 
     def setUp(self, overwrite_config=False):
@@ -139,6 +145,7 @@
                 self.NAMED_CONF_TEMPLATE.substitute(
                     homedir=self.homedir, port=self.port,
                     log_file=self.log_file,
+                    include_in_options=self.include_in_options,
                     extra=namedrndcconf))
             atomic_write(
                 GENERATED_HEADER + named_conf, self.conf_file)

=== modified file 'src/maastesting/tests/test_bindfixture.py'
--- src/maastesting/tests/test_bindfixture.py	2013-10-07 09:12:40 +0000
+++ src/maastesting/tests/test_bindfixture.py	2013-11-08 06:27:21 +0000
@@ -27,6 +27,7 @@
     Contains,
     FileContains,
     FileExists,
+    Not,
     )
 from testtools.testcase import gather_details
 
@@ -90,6 +91,7 @@
             self.assertIsInstance(resources.rndc_port, int)
             self.assertIsInstance(resources.homedir, unicode)
             self.assertIsInstance(resources.log_file, unicode)
+            self.assertIs(resources.include_in_options, None)
             self.assertIsInstance(resources.named_file, unicode)
             self.assertIsInstance(resources.conf_file, unicode)
             self.assertIsInstance(
@@ -110,6 +112,23 @@
                 FileContains(matcher=Contains(
                     b'default-port %s' % (
                         resources.rndc_port))))
+            # This should ideally be in its own test but it's here to cut
+            # test run time. See test_setUp_honours_include_in_options()
+            # as its counterpart.
+            self.assertThat(
+                resources.conf_file,
+                Not(FileContains(matcher=Contains(
+                    "forwarders"))))
+
+    def test_setUp_honours_include_in_options(self):
+        forwarders = "forwarders { 1.2.3.4; };"
+        with BINDServerResources(include_in_options=forwarders) as resources:
+            expected_in_file = (
+                resources.homedir + '/' + forwarders).encode("ascii")
+            self.assertThat(
+                resources.conf_file,
+                FileContains(matcher=Contains(
+                    expected_in_file)))
 
     def test_defaults_reallocated_after_teardown(self):
         seen_homedirs = set()

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2013-10-15 11:35:10 +0000
+++ src/provisioningserver/dns/config.py	2013-11-08 06:27:21 +0000
@@ -17,6 +17,7 @@
     'DNSForwardZoneConfig',
     'DNSReverseZoneConfig',
     'setup_rndc',
+    'set_up_options_conf',
     ]
 
 
@@ -47,6 +48,7 @@
 
 
 MAAS_NAMED_CONF_NAME = 'named.conf.maas'
+MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME = 'named.conf.options.inside.maas'
 MAAS_NAMED_RNDC_CONF_NAME = 'named.conf.rndc.maas'
 MAAS_RNDC_CONF_NAME = 'rndc.conf.maas'
 
@@ -154,6 +156,35 @@
 TEMPLATES_DIR = 'templates/dns'
 
 
+def set_up_options_conf(overwrite=True, **kwargs):
+    """Write out the named.conf.options.inside.maas file.
+
+    This file should be included by the top-level named.conf.options
+    inside its 'options' block.  MAAS cannot write the options file itself,
+    so relies on either the DNSFixture in the test suite, or the packaging.
+    Both should set that file up appropriately to include our file.
+    """
+    template_path = os.path.join(
+        locate_config(TEMPLATES_DIR),
+        "named.conf.options.inside.maas.template")
+    template = tempita.Template.from_filename(template_path)
+
+    # Make sure "upstream_dns" is set at least to None.  It's a
+    # special piece of config that can't be obtained in celery
+    # task code and we don't want to require that every call site
+    # has to specify it.  If it's not set, the substitution will
+    # fail with the default template that uses this value.
+    kwargs.setdefault("upstream_dns")
+    try:
+        rendered = template.substitute(kwargs)
+    except NameError as error:
+        raise DNSConfigFail(*error.args)
+
+    target_path = os.path.join(
+        conf.DNS_CONFIG_DIR, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+    atomic_write(rendered, target_path, overwrite=overwrite, mode=0644)
+
+
 class DNSConfigBase:
     __metaclass__ = ABCMeta
 
@@ -183,6 +214,7 @@
             return tempita.Template(f.read(), name=self.template_path)
 
     def render_template(self, template, **kwargs):
+        """Substitute supplied kwargs into the supplied Tempita template."""
         try:
             return template.substitute(kwargs)
         except NameError as error:

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2013-10-15 11:51:48 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2013-11-08 06:27:21 +0000
@@ -48,8 +48,10 @@
     extract_suggested_named_conf,
     generate_rndc,
     MAAS_NAMED_CONF_NAME,
+    MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME,
     MAAS_NAMED_RNDC_CONF_NAME,
     MAAS_RNDC_CONF_NAME,
+    set_up_options_conf,
     setup_rndc,
     shortened_reversed_ip,
     TEMPLATES_DIR,
@@ -93,6 +95,33 @@
                 conf_content = stream.read()
                 self.assertIn(content, conf_content)
 
+    def test_set_up_options_conf_writes_configuration(self):
+        dns_conf_dir = self.make_dir()
+        self.patch(conf, 'DNS_CONFIG_DIR', dns_conf_dir)
+        fake_dns = factory.getRandomIPAddress()
+        set_up_options_conf(upstream_dns=fake_dns)
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        self.assertThat(
+            target_file,
+            FileContains(matcher=Contains(fake_dns)))
+
+    def test_set_up_options_conf_handles_no_upstream_dns(self):
+        dns_conf_dir = self.make_dir()
+        self.patch(conf, 'DNS_CONFIG_DIR', dns_conf_dir)
+        set_up_options_conf()
+        target_file = os.path.join(
+            dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
+        self.assertThat(target_file, FileExists())
+
+    def test_set_up_options_conf_raises_on_bad_template(self):
+        template = self.make_file(
+            name="named.conf.options.inside.maas.template",
+            contents=b"{{nonexistent}}")
+        self.patch(config, "TEMPLATES_DIR", os.path.dirname(template))
+        exception = self.assertRaises(DNSConfigFail, set_up_options_conf)
+        self.assertIn("name 'nonexistent' is not defined", repr(exception))
+
     def test_rndc_config_includes_default_controls(self):
         dns_conf_dir = self.make_dir()
         self.patch(conf, 'DNS_CONFIG_DIR', dns_conf_dir)

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2013-10-15 11:51:48 +0000
+++ src/provisioningserver/tasks.py	2013-11-08 06:27:21 +0000
@@ -43,6 +43,7 @@
 from provisioningserver.dns.config import (
     DNSConfig,
     execute_rndc_command,
+    set_up_options_conf,
     setup_rndc,
     )
 from provisioningserver.omshell import Omshell
@@ -197,6 +198,8 @@
     # Write main config file.
     dns_config = DNSConfig(zones=zones)
     dns_config.write_config(**kwargs)
+    # Write the included options file.
+    set_up_options_conf(**kwargs)
     if callback is not None:
         callback.delay()
 

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2013-10-15 11:51:48 +0000
+++ src/provisioningserver/tests/test_tasks.py	2013-11-08 06:27:21 +0000
@@ -57,6 +57,7 @@
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
     MAAS_NAMED_CONF_NAME,
+    MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME,
     MAAS_NAMED_RNDC_CONF_NAME,
     MAAS_RNDC_CONF_NAME,
     )
@@ -450,7 +451,8 @@
         command = factory.getRandomString()
         result = write_full_dns_config.delay(
             zones=zones,
-            callback=rndc_command.subtask(args=[command]))
+            callback=rndc_command.subtask(args=[command]),
+            upstream_dns=factory.getRandomIPAddress())
 
         forward_file_name = 'zone.%s' % domain
         reverse_file_name = 'zone.0.168.192.in-addr.arpa'
@@ -461,6 +463,8 @@
                 os.path.join(self.dns_conf_dir, forward_file_name),
                 os.path.join(self.dns_conf_dir, reverse_file_name),
                 os.path.join(self.dns_conf_dir, MAAS_NAMED_CONF_NAME),
+                os.path.join(
+                    self.dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME),
             ),
             MatchesListwise(
                 (
@@ -469,6 +473,7 @@
                     FileExists(),
                     FileExists(),
                     FileExists(),
+                    FileExists(),
                 )))
 
     def test_write_full_dns_attached_to_dns_worker_queue(self):