Tidy up exception handling in contrib api consoles

Removes redundant exception handling from VNC consoles extension api module.
Both NotAuthorized and Invalid subclasses are already handled at the wsgi
application layer. Also gives a better message, including the id, when an
instance is not found. Includes tweaks suggested by Vish in review.

Change-Id: Iefa4deeb46e4a12d012e7abe3c73d7d9c18afeff

3 files changed, 27 insertions, 18 deletions

diff --git a/nova/api/openstack/compute/contrib/consoles.py b/nova/api/openstack/compute/contrib/consoles.py
index 712cda87b..915ca5c87 100644
--- a/nova/api/openstack/compute/contrib/consoles.py
+++ b/nova/api/openstack/compute/contrib/consoles.py
@@ -38,26 +38,16 @@ class ConsolesController(wsgi.Controller):
         context = req.environ['nova.context']
         authorize(context)
 
+        # If type is not supplied or unknown, get_vnc_console below will cope
         console_type = body['os-getVNCConsole'].get('type')
 
-        if not console_type:
-            raise webob.exc.HTTPBadRequest(_('Missing type specification'))
-
         try:
             instance = self.compute_api.get(context, id)
-        except exception.NotFound:
-            raise webob.exc.HTTPNotFound(_('Instance not found'))
-
-        try:
             output = self.compute_api.get_vnc_console(context,
                                                       instance,
                                                       console_type)
-        except exception.ConsoleTypeInvalid, e:
-            raise webob.exc.HTTPBadRequest(_('Invalid type specification'))
-        except exception.NotAuthorized:
-            raise webob.exc.HTTPUnauthorized()
-        except exception.NotFound:
-            raise webob.exc.HTTPNotFound(_('Instance not found'))
+        except exception.InstanceNotFound as e:
+            raise webob.exc.HTTPNotFound(explanation=unicode(e))
 
         return {'console': {'type': console_type, 'url': output['url']}}
 
diff --git a/nova/tests/api/openstack/compute/contrib/test_consoles.py b/nova/tests/api/openstack/compute/contrib/test_consoles.py
index 8749aa5ed..8d727a556 100644
--- a/nova/tests/api/openstack/compute/contrib/test_consoles.py
+++ b/nova/tests/api/openstack/compute/contrib/test_consoles.py
@@ -28,15 +28,19 @@ def fake_get_vnc_console(self, _context, _instance, _console_type):
 
 def fake_get_vnc_console_invalid_type(self, _context,
                                       _instance, _console_type):
-    raise exception.ConsoleTypeInvalid()
+    raise exception.ConsoleTypeInvalid(console_type=_console_type)
+
+
+def fake_get_vnc_console_not_found(self, _context, instance, _console_type):
+    raise exception.InstanceNotFound(instance_id=instance["uuid"])
 
 
 def fake_get(self, context, instance_uuid):
     return {'uuid': instance_uuid}
 
 
-def fake_get_not_found(*args, **kwargs):
-    raise exception.NotFound()
+def fake_get_not_found(self, context, instance_uuid):
+    raise exception.InstanceNotFound(instance_id=instance_uuid)
 
 
 class ConsolesExtensionTest(test.TestCase):
@@ -62,6 +66,8 @@ class ConsolesExtensionTest(test.TestCase):
 
     def test_get_vnc_console_no_type(self):
         self.stubs.Set(compute.API, 'get', fake_get)
+        self.stubs.Set(compute.API, 'get_vnc_console',
+                       fake_get_vnc_console_invalid_type)
         body = {'os-getVNCConsole': {}}
         req = webob.Request.blank('/v2/fake/servers/1/action')
         req.method = "POST"
@@ -83,7 +89,8 @@ class ConsolesExtensionTest(test.TestCase):
         self.assertEqual(res.status_int, 404)
 
     def test_get_vnc_console_no_instance_on_console_get(self):
-        self.stubs.Set(compute.API, 'get_vnc_console', fake_get_not_found)
+        self.stubs.Set(compute.API, 'get_vnc_console',
+            fake_get_vnc_console_not_found)
         body = {'os-getVNCConsole': {'type': 'novnc'}}
         req = webob.Request.blank('/v2/fake/servers/1/action')
         req.method = "POST"
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index e4d224584..0a90a754c 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -789,7 +789,7 @@ class ComputeTestCase(BaseTestCase):
         self.compute.terminate_instance(self.context, instance['uuid'])
 
     def test_invalid_vnc_console_type(self):
-        """Make sure we can a vnc console for an instance."""
+        """Raise useful error if console type is an unrecognised string"""
         instance = self._create_fake_instance()
         self.compute.run_instance(self.context, instance['uuid'])
 
@@ -800,6 +800,18 @@ class ComputeTestCase(BaseTestCase):
                           'invalid')
         self.compute.terminate_instance(self.context, instance['uuid'])
 
+    def test_missing_vnc_console_type(self):
+        """Raise useful error is console type is None"""
+        instance = self._create_fake_instance()
+        self.compute.run_instance(self.context, instance['uuid'])
+
+        self.assertRaises(exception.ConsoleTypeInvalid,
+                          self.compute.get_vnc_console,
+                          self.context,
+                          instance['uuid'],
+                          None)
+        self.compute.terminate_instance(self.context, instance['uuid'])
+
     def test_diagnostics(self):
         """Make sure we can get diagnostics for an instance."""
         instance = self._create_fake_instance()