launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/dns-config-option into lp:maas with lp:~rvb/maas/cope-empty-network-info as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This branch adds a configuration option to enable/disable DNS.

= Notes =

- I changed write_full_dns_config() so that it can write an empty config when DNS is disabled.  You might wonder why we simply don't register our config files from the system's BIND in that case.  The reason is that, in case DNS gets enabled back, we still want our configuration to be register as we want to keep the ability to reload the dns server and that requires our rndc key to be registered in the DNS server.

- Drive-by fix: add +./contrib/python-tx-tftp/twisted/plugins/dropin.cache to .bzrignore

- Drive-by fix: fix the address of an image in title_form.css, since the css are now served by the combo loader, the path of the images is relative the combo loader.

= Want to see this in action? =

First, disable the connection to cobbler: set USE_REAL_PSERV to False in src/maas/demo.py.
make sampledata
make run
make harness
(in the python shell)
from django.core.management import call_command 
from maasserver.models import DHCPLease, NodeGroup, Node, MACAddress
# Create the master nodegroup, jtv is fixing the code to get that done as part of a migration
call_command('config_master_dhcp', subnet_mask='255.255.0.0', broadcast_ip='10.111.255.255', ip_range_low= '10.111.123.9', ip_range_high= '10.111.244.18', router_ip='10.111.123.9')
# At this stage, the zone is defined and all the auto-generated hostname are in place:
# dig -p 5246 @127.0.0.1 10-111-123-9.master +short

# Create a node in that nodegroup. Its hostname is 'myhostname'
master = NodeGroup.objects.get(id=1)
node = Node(hostname='myhostname', nodegroup=master)
node.save()
mac = MACAddress(node=node, mac_address='11:22:33:44:55:66')
mac.save()
# Create a lease for that node:
DHCPLease.objects.update_leases(master, {'10.111.123.10': '11:22:33:44:55:66'})

# Now the hostname of the node can be queried: 
# dig -p 5246 @127.0.0.1 myhostname.master +short
-- 
https://code.launchpad.net/~rvb/maas/dns-config-option/+merge/117075
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/dns-config-option into lp:maas.

=== modified file '.bzrignore'
--- .bzrignore	2012-07-12 15:18:10 +0000
+++ .bzrignore	2012-07-27 14:59:31 +0000
@@ -7,6 +7,7 @@
 ./bin
 ./build
 ./celerybeat-schedule
+./contrib/python-tx-tftp/twisted/plugins/dropin.cache
 ./db
 ./develop-eggs
 ./dist

=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py	2012-07-27 14:59:31 +0000
+++ src/maasserver/dns.py	2012-07-27 14:59:31 +0000
@@ -31,6 +31,7 @@
 from django.dispatch import receiver
 from maasserver.exceptions import MAASException
 from maasserver.models import (
+    Config,
     DHCPLease,
     Node,
     NodeGroup,
@@ -59,7 +60,7 @@
 
 
 def is_dns_enabled():
-    return settings.DNS_CONNECT
+    return settings.DNS_CONNECT and Config.objects.get_config('enable_dns')
 
 
 class DNSException(MAASException):
@@ -103,6 +104,15 @@
         "in MAAS' zone files.")
 
 
+def dns_config_changed(sender, config, created, **kwargs):
+    """Signal callback called when the DNS config changed."""
+    if is_dns_enabled():
+        write_full_dns_config(active=config.value)
+
+
+Config.objects.config_changed_connect('enable_dns', dns_config_changed)
+
+
 @receiver(post_save, sender=NodeGroup)
 def dns_post_save_NodeGroup(sender, instance, created, **kwargs):
     """Create or update DNS zones related to the new nodegroup."""
@@ -212,10 +222,17 @@
         zone=zone, callback=write_dns_config_subtask)
 
 
-def write_full_dns_config():
-    """Write the DNS configuration for all the nodegroups."""
-    serial = next_zone_serial()
-    zones = get_zones(NodeGroup.objects.all(), serial)
+def write_full_dns_config(active=True):
+    """Write the DNS configuration.
+
+    If active is True, write the DNS config for all the nodegroups.
+    If active is False, write an empty DNS config (with no zones).
+    """
+    if active:
+        serial = next_zone_serial()
+        zones = get_zones(NodeGroup.objects.all(), serial)
+    else:
+        zones = []
     tasks.write_full_dns_config.delay(
         zones=zones,
         callback=tasks.rndc_command.subtask(args=[['reload']]))

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-06-28 13:15:04 +0000
+++ src/maasserver/forms.py	2012-07-27 14:59:31 +0000
@@ -497,6 +497,11 @@
         label="Default domain for new nodes", required=False, help_text=(
             "If 'local' is chosen, nodes must be using mDNS. Leave empty to "
             "use hostnames without a domain for newly enlisted nodes."))
+    enable_dns = forms.BooleanField(
+        label="Enable DNS", required=False, help_text=(
+            "When enabled, MAAS will use the machine's BIND server to "
+            "publish its DNS zones."
+        ))
 
 
 class CommissioningForm(ConfigForm):

=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py	2012-06-25 13:59:23 +0000
+++ src/maasserver/models/config.py	2012-07-27 14:59:31 +0000
@@ -52,6 +52,8 @@
         # Network section configuration.
         'maas_name': gethostname(),
         'enlistment_domain': b'local',
+        # DNS config.
+        'enable_dns': True,
         ## /settings
         }
 

=== modified file 'src/maasserver/static/css/components/title_form.css'
--- src/maasserver/static/css/components/title_form.css	2012-03-01 06:22:38 +0000
+++ src/maasserver/static/css/components/title_form.css	2012-07-27 14:59:31 +0000
@@ -1,5 +1,5 @@
 .page-title-form {
-    background: transparent url(../../img/content_tail.png) no-repeat 50px bottom;
+    background: transparent url(../?img/content_tail.png) no-repeat 50px bottom;
     }
 .page-title-form input {
     border: 2px solid transparent;

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2012-07-27 14:59:31 +0000
+++ src/maasserver/tests/test_dns.py	2012-07-27 14:59:31 +0000
@@ -30,6 +30,7 @@
     write_full_dns_config,
     zone_serial,
     )
+from maasserver.models import Config
 from maasserver.models.dhcplease import (
     DHCPLease,
     post_updates,
@@ -227,6 +228,19 @@
         change_dns_zones(nodegroup)
         self.assertDNSMatches(new_node.hostname, nodegroup.name, new_lease.ip)
 
+    def test_is_dns_enabled_return_false_if_DNS_CONNECT_False(self):
+        self.patch(settings, 'DNS_CONNECT', False)
+        self.assertFalse(is_dns_enabled())
+
+    def test_is_dns_enabled_return_false_if_confif_enable_dns_False(self):
+        Config.objects.set_config('enable_dns', False)
+        self.assertFalse(is_dns_enabled())
+
+    def test_is_dns_enabled_return_True(self):
+        self.patch(settings, 'DNS_CONNECT', True)
+        Config.objects.set_config('enable_dns', True)
+        self.assertTrue(is_dns_enabled())
+
     def test_change_dns_zone_changes_doesnt_write_conf_if_dhcp_disabled(self):
         recorder = FakeMethod()
         self.patch(DNSZoneConfig, 'write_config', recorder)
@@ -250,6 +264,11 @@
         write_full_dns_config()
         self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
 
+    def test_write_full_dns_can_write_inactive_config(self):
+        nodegroup, node, lease = self.create_nodegroup_with_lease()
+        write_full_dns_config(active=False)
+        self.assertEqual([''], self.dig_resolve(generated_hostname(lease.ip)))
+
     def test_dns_config_has_NS_record(self):
         ip = factory.getRandomIPAddress()
         self.patch(settings, 'DEFAULT_MAAS_URL', 'http://%s/' % ip)
@@ -314,3 +333,16 @@
         node.delete()
         fqdn = "%s.%s" % (node.hostname, nodegroup.name)
         self.assertEqual([''], self.dig_resolve(fqdn))
+
+    def test_change_config_enable_dns_enables_dns(self):
+        self.patch(settings, "DNS_CONNECT", False)
+        nodegroup, node, lease = self.create_nodegroup_with_lease()
+        settings.DNS_CONNECT = True
+        Config.objects.set_config('enable_dns', True)
+        self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
+
+    def test_change_config_enable_dns_disables_dns(self):
+        self.patch(settings, "DNS_CONNECT", True)
+        nodegroup, node, lease = self.create_nodegroup_with_lease()
+        Config.objects.set_config('enable_dns', False)
+        self.assertEqual([''], self.dig_resolve(generated_hostname(lease.ip)))

=== modified file 'src/maasserver/tests/test_views_settings.py'
--- src/maasserver/tests/test_views_settings.py	2012-06-25 09:03:14 +0000
+++ src/maasserver/tests/test_views_settings.py	2012-07-27 14:59:31 +0000
@@ -14,6 +14,7 @@
 
 import httplib
 
+from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from lxml.html import fromstring
@@ -72,21 +73,28 @@
                     reverse('accounts-del', args=[user.username]), links)
 
     def test_settings_maas_and_network_POST(self):
+        # Disable the DNS machinery so that we can skip the required
+        # setup.
+        self.patch(settings, "DNS_CONNECT", False)
         new_name = factory.getRandomString()
         new_domain = factory.getRandomString()
+        new_enable_dns = factory.getRandomBoolean()
         response = self.client.post(
-            '/settings/',
+            reverse('settings'),
             get_prefixed_form_data(
                 prefix='maas_and_network',
                 data={
                     'maas_name': new_name,
                     'enlistment_domain': new_domain,
+                    'enable_dns ': new_enable_dns,
                 }))
 
         self.assertEqual(httplib.FOUND, response.status_code)
         self.assertEqual(new_name, Config.objects.get_config('maas_name'))
         self.assertEqual(
             new_domain, Config.objects.get_config('enlistment_domain'))
+        self.assertEqual(
+            new_enable_dns, Config.objects.get_config('enable_dns'))
 
     def test_settings_commissioning_POST(self):
         new_after_commissioning = factory.getRandomEnum(