launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/dns-setup-command/+merge/114364

This branch is preparation work for a follow-up branch which will add a new 'set_up_dns' command which will return the configuration snippet used to hook up MAAS' dns configuration within an existing Bind instance.

This branch:
- adds a democeleryconfig module used when celery is run in a dev environment.  This is required to be able to have dev-specific settings.  Note that I also modified src/maas/development.py to instruct celery to use the democelery config when django is run with these settings.  This is required because the Django code (and maasserver in particular) can call code located 'provisioningserver' (note that the reverse is not true).
- modifies the bindfixture so that the configuration won't be overwritten by default (I've added the possibility to specify whether or not the configuration should be overwritten only as a convenience).  This is required because otherwise, each run of "make services/dns/@{run,start}" would overwrite the config and this would destruct all modifications made to include the MAAS-specific configuration file in the main named conf file.

Drive-by fixes:
- use double quotes in src/maastesting/bindfixture.py.
- ditch the usage of os.path.join to define DNS_CONFIG_DIR in 'etc/celeryconfig.py: this is more readable and more in sync with how the other configuration settings are defined in that file.
-- 
https://code.launchpad.net/~rvb/maas/dns-setup-command/+merge/114364
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/dns-setup-command into lp:maas.

=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py	2012-07-05 09:15:02 +0000
+++ etc/celeryconfig.py	2012-07-11 09:26:20 +0000
@@ -15,7 +15,6 @@
 
 __metaclass__ = type
 
-import os
 
 from maas import import_settings
 
@@ -31,7 +30,7 @@
 TFTPROOT = "/var/lib/tftpboot"
 
 # Location of MAAS' bind configuration files.
-DNS_CONFIG_DIR = os.path.join(os.sep, 'var', 'cache', 'bind', 'maas')
+DNS_CONFIG_DIR = '/var/cache/bind/maas'
 
 # DHCP leases file, as maintained by ISC dhcpd.
 DHCP_LEASES_FILE = '/var/lib/dhcp/dhcpd.leases'

=== added file 'etc/democeleryconfig.py'
--- etc/democeleryconfig.py	1970-01-01 00:00:00 +0000
+++ etc/democeleryconfig.py	2012-07-11 09:26:20 +0000
@@ -0,0 +1,28 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Celery demo settings for the maas project."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+)
+
+__metaclass__ = type
+
+import os
+
+import celeryconfig
+from maas import import_settings
+
+# Extend base settings.
+import_settings(celeryconfig)
+
+
+DEV_ROOT_DIRECTORY = os.path.join(
+    os.path.dirname(__file__), os.pardir)
+
+
+DNS_CONFIG_DIR = os.path.join(
+    DEV_ROOT_DIRECTORY, 'run/named/')

=== modified file 'services/celeryd/run'
--- services/celeryd/run	2012-06-13 08:01:35 +0000
+++ services/celeryd/run	2012-07-11 09:26:20 +0000
@@ -14,4 +14,5 @@
 
 # Run celeryd.
 export PYTHONPATH=$PYTHONPATH:`pwd`/src:`pwd`/etc
+export CELERY_CONFIG_MODULE=democeleryconfig
 celeryd -l INFO

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-07-06 10:12:25 +0000
+++ src/maas/development.py	2012-07-11 09:26:20 +0000
@@ -99,5 +99,9 @@
     DEV_ROOT_DIRECTORY, 'etc/maas/commissioning-user-data')
 
 
+# Set up celery to use the demo settings.
+os.environ['CELERY_CONFIG_MODULE'] = 'democeleryconfig'
+
+
 # Allow the user to override settings in maas_local_settings.
 import_local_settings()

=== modified file 'src/maastesting/bindfixture.py'
--- src/maastesting/bindfixture.py	2012-07-09 16:14:45 +0000
+++ src/maastesting/bindfixture.py	2012-07-11 09:26:20 +0000
@@ -88,13 +88,13 @@
             log_file=log_file,
             )
 
-    def setUp(self):
+    def setUp(self, overwrite=False):
         super(BINDServerResources, self).setUp()
         self.__dict__.update(self._defaults)
         self.set_up_config()
-        self.set_up_named()
+        self.set_up_named(overwrite=overwrite)
 
-    def set_up_named(self):
+    def set_up_named(self, overwrite=True):
         """Setup an environment to run 'named'.
 
         - Create the default configuration for 'named' and setup rndc.
@@ -105,14 +105,16 @@
         rndcconf, namedrndcconf = generate_rndc(
             self.rndc_port, 'dnsfixture-rndc-key')
         # Write main BIND config file.
-        named_conf = (
-            self.NAMED_CONF_TEMPLATE.substitute(
-                homedir=self.homedir, port=self.port,
-                log_file=self.log_file,
-                extra=namedrndcconf))
-        atomic_write(named_conf, self.conf_file)
+        if not os.path.exists(self.conf_file) or overwrite:
+            named_conf = (
+                self.NAMED_CONF_TEMPLATE.substitute(
+                    homedir=self.homedir, port=self.port,
+                    log_file=self.log_file,
+                    extra=namedrndcconf))
+            atomic_write(named_conf, self.conf_file)
         # Write rndc config file.
-        atomic_write(rndcconf, self.rndcconf_file)
+        if not os.path.exists(self.rndcconf_file) or overwrite:
+            atomic_write(rndcconf, self.rndcconf_file)
 
         # Copy named executable to home dir.  This is done to avoid
         # the limitations imposed by apparmor if the executable
@@ -120,13 +122,14 @@
         # named's apparmor profile prevents loading of zone and
         # configuration files from outside of a restricted set,
         # none of which an ordinary user has write access to.
-        named_path = self.NAMED_PATH
-        assert os.path.exists(named_path), (
-            "'%s' executable not found.  Install the package "
-            "'bind9' or define an environment variable named "
-            "NAMED_PATH with the path where the 'named' "
-            "executable can be found." % named_path)
-        copy(named_path, self.named_file)
+        if not os.path.exists(self.named_file) or overwrite:
+            named_path = self.NAMED_PATH
+            assert os.path.exists(named_path), (
+                "'%s' executable not found.  Install the package "
+                "'bind9' or define an environment variable named "
+                "NAMED_PATH with the path where the 'named' "
+                "executable can be found." % named_path)
+            copy(named_path, self.named_file)
 
     def set_up_config(self):
         if self.port is None:
@@ -270,18 +273,22 @@
     parser.add_argument(
         '--homedir',
         help=(
-            'A directory where to put all the files the BIND'
-            'server needs (configuration files and executable)'
+            "A directory where to put all the files the BIND"
+            "server needs (configuration files and executable)"
            ))
     parser.add_argument(
         '--log-file',
-        help='The log file allocated for the server')
+        help="The log file allocated for the server")
     parser.add_argument(
         '--port', type=int,
-        help='The port that will be used by BIND')
+        help="The port that will be used by BIND")
     parser.add_argument(
         '--rndc-port', type=int,
-        help='The rndc port that will be used by BIND')
+        help="The rndc port that will be used by BIND")
+    parser.add_argument(
+        '--overwrite', type=bool,
+        help="Whether or not to overwrite the configuration files "
+             "if they already exist", default=False)
     arguments = parser.parse_args()
 
     # Create homedir if it does not already exist.
@@ -293,7 +300,7 @@
     resources = BINDServerResources(
         homedir=arguments.homedir, log_file=arguments.log_file,
         port=arguments.port, rndc_port=arguments.rndc_port)
-    resources.setUp()
+    resources.setUp(overwrite=arguments.overwrite)
     # exec named.
     os.execlp(
         resources.named_file, resources.named_file, "-g", "-c",