Tidy up handling of exceptions in floating_ip_dns

Removes translation of NotAuthorized which is already handled at a
higher level. Also makes delete methods more typical, switches http
status code on success to 202 rather than 200, includes details when
handling NotFound and adds test coverage.

Change-Id: Id59e397891b80b45ea38e42654a6f7f9859379f8

2 files changed, 53 insertions, 45 deletions

diff --git a/nova/api/openstack/compute/contrib/floating_ip_dns.py b/nova/api/openstack/compute/contrib/floating_ip_dns.py
index 788d83b2d..2029ab2aa 100644
--- a/nova/api/openstack/compute/contrib/floating_ip_dns.py
+++ b/nova/api/openstack/compute/contrib/floating_ip_dns.py
@@ -162,28 +162,21 @@ class FloatingIPDNSDomainController(object):
             raise webob.exc.HTTPUnprocessableEntity()
         project = entry.get('project', None)
         av_zone = entry.get('availability_zone', None)
-        if (not scope or
+        if (scope not in ('private', 'public') or
             project and av_zone or
             scope == 'private' and project or
             scope == 'public' and av_zone):
             raise webob.exc.HTTPUnprocessableEntity()
-        try:
-            if scope == 'private':
-                self.network_api.create_private_dns_domain(context,
-                                                           fqdomain,
-                                                           av_zone)
-                return _translate_domain_entry_view({'domain': fqdomain,
-                                                   'scope': scope,
-                                             'availability_zone': av_zone})
-            else:
-                self.network_api.create_public_dns_domain(context,
-                                                          fqdomain,
-                                                          project)
-                return _translate_domain_entry_view({'domain': fqdomain,
-                                                   'scope': 'public',
-                                                   'project': project})
-        except exception.NotAuthorized or exception.AdminRequired:
-            return webob.Response(status_int=403)
+        if scope == 'private':
+            create_dns_domain = self.network_api.create_private_dns_domain
+            area_name, area = 'availability_zone', av_zone
+        else:
+            create_dns_domain = self.network_api.create_public_dns_domain
+            area_name, area = 'project', project
+        create_dns_domain(context, fqdomain, area)
+        return _translate_domain_entry_view({'domain': fqdomain,
+                                             'scope': scope,
+                                             area_name: area})
 
     def delete(self, req, id):
         """Delete the domain identified by id. """
@@ -194,12 +187,10 @@ class FloatingIPDNSDomainController(object):
         # Delete the whole domain
         try:
             self.network_api.delete_dns_domain(context, domain)
-        except exception.NotAuthorized or exception.AdminRequired:
-            return webob.Response(status_int=403)
-        except exception.NotFound:
-            return webob.Response(status_int=404)
+        except exception.NotFound as e:
+            raise webob.exc.HTTPNotFound(explanation=unicode(e))
 
-        return webob.Response(status_int=200)
+        return webob.Response(status_int=202)
 
 
 class FloatingIPDNSEntryController(object):
@@ -280,10 +271,10 @@ class FloatingIPDNSEntryController(object):
 
         try:
             self.network_api.delete_dns_entry(context, name, domain)
-        except exception.NotFound:
-            return webob.Response(status_int=404)
+        except exception.NotFound as e:
+            raise webob.exc.HTTPNotFound(explanation=unicode(e))
 
-        return webob.Response(status_int=200)
+        return webob.Response(status_int=202)
 
 
 class Floating_ip_dns(extensions.ExtensionDescriptor):
diff --git a/nova/tests/api/openstack/compute/contrib/test_floating_ip_dns.py b/nova/tests/api/openstack/compute/contrib/test_floating_ip_dns.py
index e2d6bb8e1..4753ede32 100644
--- a/nova/tests/api/openstack/compute/contrib/test_floating_ip_dns.py
+++ b/nova/tests/api/openstack/compute/contrib/test_floating_ip_dns.py
@@ -20,6 +20,7 @@ import webob
 from nova.api.openstack.compute.contrib import floating_ip_dns
 from nova import context
 from nova import db
+from nova import exception
 from nova import network
 from nova import test
 from nova.tests.api.openstack import fakes
@@ -204,14 +205,10 @@ class FloatingIpDNSTest(test.TestCase):
         self.assertEqual(entry['domain_entry']['availability_zone'], 'zone1')
 
     def test_delete_entry(self):
-        self.called = False
-        self.deleted_domain = ""
-        self.deleted_name = ""
+        calls = []
 
         def network_delete_dns_entry(fakeself, context, name, domain):
-            self.called = True
-            self.deleted_domain = domain
-            self.deleted_name = name
+            calls.append((name, domain))
 
         self.stubs.Set(network.api.API, "delete_dns_entry",
                        network_delete_dns_entry)
@@ -219,31 +216,51 @@ class FloatingIpDNSTest(test.TestCase):
         req = fakes.HTTPRequest.blank(
               '/v2/123/os-floating-ip-dns/%s/entries/%s' %
               (_quote_domain(domain), name))
-        entries = self.entry_controller.delete(req, _quote_domain(domain),
-                                               name)
+        res = self.entry_controller.delete(req, _quote_domain(domain), name)
 
-        self.assertTrue(self.called)
-        self.assertEquals(self.deleted_domain, domain)
-        self.assertEquals(self.deleted_name, name)
+        self.assertEqual(202, res.status_int)
+        self.assertEqual([(name, domain)], calls)
+
+    def test_delete_entry_notfound(self):
+        def delete_dns_entry_notfound(fakeself, context, name, domain):
+            raise exception.NotFound
+
+        self.stubs.Set(network.api.API, "delete_dns_entry",
+                       delete_dns_entry_notfound)
+
+        req = fakes.HTTPRequest.blank(
+              '/v2/123/os-floating-ip-dns/%s/entries/%s' %
+              (_quote_domain(domain), name))
+        self.assertRaises(webob.exc.HTTPNotFound,
+            self.entry_controller.delete, req, _quote_domain(domain), name)
 
     def test_delete_domain(self):
-        self.called = False
-        self.deleted_domain = ""
-        self.deleted_name = ""
+        calls = []
 
         def network_delete_dns_domain(fakeself, context, fqdomain):
-            self.called = True
-            self.deleted_domain = fqdomain
+            calls.append(fqdomain)
 
         self.stubs.Set(network.api.API, "delete_dns_domain",
                        network_delete_dns_domain)
 
         req = fakes.HTTPRequest.blank('/v2/123/os-floating-ip-dns/%s' %
                                       _quote_domain(domain))
-        entries = self.domain_controller.delete(req, _quote_domain(domain))
+        res = self.domain_controller.delete(req, _quote_domain(domain))
 
-        self.assertTrue(self.called)
-        self.assertEquals(self.deleted_domain, domain)
+        self.assertEqual(202, res.status_int)
+        self.assertEqual([domain], calls)
+
+    def test_delete_domain_notfound(self):
+        def delete_dns_domain_notfound(fakeself, context, fqdomain):
+            raise exception.NotFound
+
+        self.stubs.Set(network.api.API, "delete_dns_domain",
+                       delete_dns_domain_notfound)
+
+        req = fakes.HTTPRequest.blank('/v2/123/os-floating-ip-dns/%s' %
+                                      _quote_domain(domain))
+        self.assertRaises(webob.exc.HTTPNotFound,
+            self.domain_controller.delete, req, _quote_domain(domain))
 
     def test_modify(self):
         body = {'dns_entry':