launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/maas/bug-977797-dns-lookup into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #977797 in MAAS: "Wrong error shown if Cobbler host not found"
  https://bugs.launchpad.net/maas/+bug/977797

For more details, see:
https://code.launchpad.net/~rvb/maas/bug-977797-dns-lookup/+merge/101705

This branch fixes how the provisioning server handles DNS lookup errors.  It adds a special error for DNS lookup errors.  It also adds the two messages to be displayed when such an error occurs (a short one that will be displayed as a one-off message and a more detailed one that will be displayed for as long as the problem persists).

= Pre-imp notes =

I had a pre-imp call with Gavin about that branch during which I showed him the code I had written at the time.  So it was more of a post-imp really but it was definitely worth it since I haven't touched the provisioning server's code so far.  Gavin seemed to be happy with how I've fixed this.
-- 
https://code.launchpad.net/~rvb/maas/bug-977797-dns-lookup/+merge/101705
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-977797-dns-lookup into lp:maas.

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-04-12 05:00:51 +0000
+++ src/maasserver/models.py	2012-04-12 10:01:21 +0000
@@ -40,7 +40,6 @@
 import re
 from socket import gethostname
 from string import whitespace
-from textwrap import dedent
 import time
 from uuid import uuid1
 

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-04-11 23:29:45 +0000
+++ src/maasserver/provisioning.py	2012-04-12 10:01:21 +0000
@@ -78,6 +78,13 @@
         If the error message is not clear, you may need to check the
         Cobbler logs in /var/log/cobbler/ or pserv.log.
         """,
+    PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: """
+        The provisioning server was unable to resolve the Cobbler server's
+        DNS address: %(fault_string)s.
+
+        Has Cobbler been properly installed and is it accessible by the
+        provisioning server?  Check /var/log/cobbler/ and pserv.log.
+        """,
     8002: """
         Unable to reach provisioning server (%(fault_string)s).
 
@@ -104,6 +111,10 @@
     PSERV_FAULT.GENERIC_COBBLER_ERROR: """
         Unknown problem encountered with the Cobbler server.
         """,
+    PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: """
+        Unable to resolve the Cobbler server's DNS address:
+        %(fault_string)s.
+        """,
     8002: """
         Unable to reach provisioning server.
         """,
@@ -182,6 +193,7 @@
     PSERV_FAULT.COBBLER_AUTH_ERROR: COMPONENT.COBBLER,
     PSERV_FAULT.NO_SUCH_PROFILE: COMPONENT.IMPORT_ISOS,
     PSERV_FAULT.GENERIC_COBBLER_ERROR: COMPONENT.COBBLER,
+    PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: COMPONENT.COBBLER,
     8002: COMPONENT.PSERV,
 }
 

=== modified file 'src/provisioningserver/cobblerclient.py'
--- src/provisioningserver/cobblerclient.py	2012-04-05 13:35:36 +0000
+++ src/provisioningserver/cobblerclient.py	2012-04-12 10:01:21 +0000
@@ -31,9 +31,13 @@
     ]
 
 from functools import partial
+from urlparse import urlparse
 import xmlrpclib
 
-from provisioningserver.cobblercatcher import convert_cobbler_exception
+from provisioningserver.cobblercatcher import (
+    convert_cobbler_exception,
+    ProvisioningError,
+    )
 from provisioningserver.enum import PSERV_FAULT
 from twisted.internet import reactor as default_reactor
 from twisted.internet.defer import (
@@ -41,6 +45,7 @@
     inlineCallbacks,
     returnValue,
     )
+from twisted.internet.error import DNSLookupError
 from twisted.python.log import msg
 from twisted.web.xmlrpc import Proxy
 
@@ -248,6 +253,11 @@
                 self.proxy.callRemote(method, *args))
         except xmlrpclib.Fault as e:
             raise convert_cobbler_exception(e)
+        except DNSLookupError as e:
+            hostname = urlparse(self.url).hostname
+            raise ProvisioningError(
+                faultCode=PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR,
+                faultString=hostname)
         returnValue(result)
 
     @inlineCallbacks

=== modified file 'src/provisioningserver/enum.py'
--- src/provisioningserver/enum.py	2012-03-19 04:47:43 +0000
+++ src/provisioningserver/enum.py	2012-04-12 10:01:21 +0000
@@ -31,6 +31,9 @@
     # Profile does not exist.
     NO_SUCH_PROFILE = 5
 
+    # Error looking up cobbler server.
+    COBBLER_DNS_LOOKUP_ERROR = 6
+
     # Non-specific error inside Cobbler.
     GENERIC_COBBLER_ERROR = 99
 

=== modified file 'src/provisioningserver/tests/test_cobblersession.py'
--- src/provisioningserver/tests/test_cobblersession.py	2012-03-19 04:47:43 +0000
+++ src/provisioningserver/tests/test_cobblersession.py	2012-04-12 10:01:21 +0000
@@ -15,8 +15,10 @@
 from xmlrpclib import Fault
 
 import fixtures
+from maastesting.factory import factory
 from provisioningserver import cobblerclient
 from provisioningserver.cobblercatcher import ProvisioningError
+from provisioningserver.enum import PSERV_FAULT
 from provisioningserver.testing.fakecobbler import (
     fake_auth_failure_string,
     fake_object_not_found_string,
@@ -34,6 +36,7 @@
     )
 from twisted.internet import defer
 from twisted.internet.defer import inlineCallbacks
+from twisted.internet.error import DNSLookupError
 from twisted.internet.task import Clock
 
 
@@ -342,6 +345,23 @@
         self.assertEqual(
             [('authenticate_me_first', session.token)], session.proxy.calls)
 
+    @inlineCallbacks
+    def test_dns_lookup_exception_handled(self):
+        url = factory.getRandomString()
+        session_args = (
+            'http://%s/%d' % (url, pick_number()),
+            factory.getRandomString(),  # username.
+            factory.getRandomString(),  # password.
+            )
+        session = make_recording_session(session_args=session_args)
+        failure = DNSLookupError(factory.getRandomString())
+        session.proxy.set_return_values([failure])
+        expected_failure = ProvisioningError(
+            faultCode=PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR,
+            faultString=url.lower())
+        with ExpectedException(ProvisioningError, unicode(expected_failure)):
+            yield session.call('failing_method')
+
 
 class TestConnectionTimeouts(TestCase, fixtures.TestWithFixtures):
     """Tests for connection timeouts on `CobblerSession`."""