Merge lp:~jtv/maas/report_missing_config_dir into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

New helper for writing DNS config and zone files: report_missing_config_dir.

This is another small step towards making small amounts of duplication of the write_config and inner_write_config methods painless. The next step is to inline the two inner_write_config implementations into small, slightly different write_config implementations — one using atomic_write and the other incremental_write to create its output file. This is a subtlety that came up late (with unexpected filesystem and BIND complications) in the development of the original code, hence the awkwardness.

With that bit of duplication-with-slight-variation, we will then be able to free ourselves of some of the forced commonality between the 6 classes that together give us the ability to write the 3 kinds of DNS files: config, forward zone, reverse zone.

Jeroen

[Raphaël Badin (rvb)]

review: Approve

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2014-01-21 11:20:51 +0000
+++ src/provisioningserver/dns/config.py	2014-01-21 12:19:30 +0000
@@ -25,6 +25,7 @@
     ABCMeta,
     abstractproperty,
     )
+from contextlib import contextmanager
 from datetime import datetime
 import errno
 from itertools import (
@@ -210,6 +211,26 @@
         raise DNSConfigFail(*error.args)
 
 
+@contextmanager
+def report_missing_config_dir():
+    """Report missing DNS config dir as `DNSConfigDirectoryMissing`.
+
+    Use this around code that writes a new DNS configuration or zone file.
+    It catches a "no such file or directory" error and raises a more helpful
+    `DNSConfigDirectoryMissing` in its place.
+    """
+    try:
+        yield
+    except (IOError, OSError) as e:
+        if e.errno == errno.ENOENT:
+            raise DNSConfigDirectoryMissing(
+                "The directory where the DNS config files should be "
+                "written does not exist.  Make sure the 'maas-dns' "
+                "package is installed on this region controller.")
+        else:
+            raise
+
+
 class DNSConfigBase:
     __metaclass__ = ABCMeta
 
@@ -246,18 +267,8 @@
         "No such file or directory" error is raised because that would mean
         that the directory containing the write to be written does not exist.
         """
-        try:
+        with report_missing_config_dir():
             self.inner_write_config(overwrite=overwrite, **kwargs)
-        except OSError as exception:
-            # Only raise a DNSConfigDirectoryMissing exception if this error
-            # is a "No such file or directory" exception.
-            if exception.errno == errno.ENOENT:
-                raise DNSConfigDirectoryMissing(
-                    "The directory where the DNS config files should be "
-                    "written does not exist.  Make sure the 'maas-dns' "
-                    "package is installed on this region controller.")
-            else:
-                raise
 
     def inner_write_config(self, overwrite=True, **kwargs):
         """Write out this DNS config file."""

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2014-01-21 11:42:40 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2014-01-21 12:19:30 +0000
@@ -49,6 +49,7 @@
     MAAS_NAMED_RNDC_CONF_NAME,
     MAAS_RNDC_CONF_NAME,
     render_dns_template,
+    report_missing_config_dir,
     set_up_options_conf,
     setup_rndc,
     shortened_reversed_ip,
@@ -69,6 +70,7 @@
     Not,
     StartsWith,
     )
+from testtools.testcase import ExpectedException
 from twisted.python.filepath import FilePath
 
 
@@ -263,6 +265,34 @@
         self.assertIn("'x' is not defined", unicode(e))
 
 
+class TestReportMissingConfigDir(MAASTestCase):
+    """Tests for the `report_missing_config_dir` context manager."""
+
+    def test_specially_reports_missing_config_dir(self):
+        with ExpectedException(DNSConfigDirectoryMissing):
+            with report_missing_config_dir():
+                open(os.path.join(self.make_dir(), 'nonexistent-file.txt'))
+
+    def test_succeeds_if_no_exceptions(self):
+        with report_missing_config_dir():
+            pass
+        # The real test is that we get here without error.
+        pass
+
+    def test_passes_on_other_similar_errors(self):
+        with ExpectedException(OSError):
+            with report_missing_config_dir():
+                raise OSError(errno.EACCES, "Deliberate error for testing.")
+
+    def test_passes_on_dissimilar_errors(self):
+        class DeliberateError(Exception):
+            """Deliberately induced error for testing."""
+
+        with ExpectedException(DeliberateError):
+            with report_missing_config_dir():
+                raise DeliberateError("This exception propagates unchanged.")
+
+
 class TestDNSConfig(MAASTestCase):
     """Tests for DNSConfig."""