Merge lp:~gundlach/nova/rsapi-faults into lp:~hudson-openstack/nova/trunk

[Michael Gundlach]

Support fault notation in error messages in the RS API.

[lp:~gundlach/nova/rsapi-faults]

296. By Michael Gundlach on 2010-09-29

Merge from trunk

297. By Michael Gundlach on 2010-09-29

After update from trunk, a few more exceptions that need to be converted to Faults

[Matt Dietz (cerberus)]

lgtm.

Not for now, but the API also supports more granular information per exception. In the future, we should modify the faults to include an optional extended_description field.

review: Approve

[Michael Gundlach (gundlach)]

Cool -- webob.exc.HTTPException's ctor supports 'detail=', which I imagine we could map into the <details> of the Fault response body.

=== modified file 'nova/api/rackspace/__init__.py'
--- nova/api/rackspace/__init__.py	2010-09-28 21:46:21 +0000
+++ nova/api/rackspace/__init__.py	2010-09-29 14:20:56 +0000
@@ -31,6 +31,7 @@
 from nova import flags
 from nova import utils
 from nova import wsgi
+from nova.api.rackspace import faults
 from nova.api.rackspace import backup_schedules
 from nova.api.rackspace import flavors
 from nova.api.rackspace import images
@@ -67,7 +68,7 @@
         user = self.auth_driver.authorize_token(req.headers["X-Auth-Token"])
 
         if not user:
-            return webob.exc.HTTPUnauthorized()
+            return faults.Fault(webob.exc.HTTPUnauthorized())
 
         if not req.environ.has_key('nova.context'):
             req.environ['nova.context'] = {}
@@ -112,8 +113,10 @@
         delay = self.get_delay(action_name, username)
         if delay:
             # TODO(gundlach): Get the retry-after format correct.
-            raise webob.exc.HTTPRequestEntityTooLarge(headers={
-                    'Retry-After': time.time() + delay})
+            exc = webob.exc.HTTPRequestEntityTooLarge(
+                    explanation='Too many requests.',
+                    headers={'Retry-After': time.time() + delay})
+            raise faults.Fault(exc)
         return self.application
 
     def get_delay(self, action_name, username):

=== modified file 'nova/api/rackspace/auth.py'
--- nova/api/rackspace/auth.py	2010-09-28 21:46:21 +0000
+++ nova/api/rackspace/auth.py	2010-09-29 14:20:56 +0000
@@ -11,6 +11,7 @@
 from nova import flags
 from nova import manager
 from nova import utils
+from nova.api.rackspace import faults
 
 FLAGS = flags.FLAGS
 
@@ -36,13 +37,13 @@
         # honor it
         path_info = req.path_info
         if len(path_info) > 1:
-            return webob.exc.HTTPUnauthorized()
+            return faults.Fault(webob.exc.HTTPUnauthorized())
 
         try:
             username, key = req.headers['X-Auth-User'], \
                 req.headers['X-Auth-Key']
         except KeyError:
-            return webob.exc.HTTPUnauthorized()
+            return faults.Fault(webob.exc.HTTPUnauthorized())
 
         username, key = req.headers['X-Auth-User'], req.headers['X-Auth-Key']
         token, user = self._authorize_user(username, key)
@@ -57,7 +58,7 @@
             res.status = '204'
             return res
         else:
-            return webob.exc.HTTPUnauthorized()
+            return faults.Fault(webob.exc.HTTPUnauthorized())
 
     def authorize_token(self, token_hash):
         """ retrieves user information from the datastore given a token

=== modified file 'nova/api/rackspace/backup_schedules.py'
--- nova/api/rackspace/backup_schedules.py	2010-09-28 21:46:21 +0000
+++ nova/api/rackspace/backup_schedules.py	2010-09-29 14:20:56 +0000
@@ -20,6 +20,7 @@
 
 from nova import wsgi
 from nova.api.rackspace import _id_translator
+from nova.api.rackspace import faults
 import nova.image.service
 
 class Controller(wsgi.Controller):
@@ -27,12 +28,12 @@
         pass
 
     def index(self, req, server_id):
-        return exc.HTTPNotFound()
+        return faults.Fault(exc.HTTPNotFound())
 
     def create(self, req, server_id):
         """ No actual update method required, since the existing API allows
         both create and update through a POST """
-        return exc.HTTPNotFound()
+        return faults.Fault(exc.HTTPNotFound())
 
     def delete(self, req, server_id):
-        return exc.HTTPNotFound()
+        return faults.Fault(exc.HTTPNotFound())

=== added file 'nova/api/rackspace/faults.py'
--- nova/api/rackspace/faults.py	1970-01-01 00:00:00 +0000
+++ nova/api/rackspace/faults.py	2010-09-29 14:20:56 +0000
@@ -0,0 +1,61 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+import webob.dec
+
+from nova import wsgi
+
+
+class Fault(wsgi.Application):
+
+    """An RS API fault response."""
+
+    _fault_names = {
+            400: "badRequest",
+            401: "unauthorized",
+            403: "resizeNotAllowed",
+            404: "itemNotFound",
+            405: "badMethod",
+            409: "inProgress",
+            413: "overLimit",
+            415: "badMediaType",
+            501: "notImplemented",
+            503: "serviceUnavailable"}
+
+    def __init__(self, exception):
+        """Create a Fault for the given webob.exc.exception."""
+        self.exception = exception
+
+    @webob.dec.wsgify
+    def __call__(self, req):
+        """Generate a WSGI response based on self.exception."""
+        # Replace the body with fault details.
+        code = self.exception.status_int
+        fault_name = self._fault_names.get(code, "cloudServersFault")
+        fault_data = {
+            fault_name: {
+                'code': code,
+                'message': self.exception.explanation}}
+        if code == 413:
+            retry = self.exception.headers['Retry-After']
+            fault_data[fault_name]['retryAfter'] = retry
+        # 'code' is an attribute on the fault tag itself 
+        metadata = {'application/xml': {'attributes': {fault_name: 'code'}}}
+        serializer = wsgi.Serializer(req.environ, metadata)
+        self.exception.body = serializer.to_content_type(fault_data)
+        return self.exception

=== modified file 'nova/api/rackspace/flavors.py'
--- nova/api/rackspace/flavors.py	2010-09-28 05:23:49 +0000
+++ nova/api/rackspace/flavors.py	2010-09-29 14:20:56 +0000
@@ -15,6 +15,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from nova.api.rackspace import faults
 from nova.compute import instance_types
 from nova import wsgi
 from webob import exc
@@ -47,7 +48,7 @@
                 item = dict(ram=val['memory_mb'], disk=val['local_gb'],
                             id=val['flavorid'], name=name)
                 return dict(flavor=item)
-        raise exc.HTTPNotFound()
+        raise faults.Fault(exc.HTTPNotFound())
 
     def _all_ids(self):
         """Return the list of all flavorids."""

=== modified file 'nova/api/rackspace/images.py'
--- nova/api/rackspace/images.py	2010-09-28 21:46:21 +0000
+++ nova/api/rackspace/images.py	2010-09-29 14:20:56 +0000
@@ -20,6 +20,7 @@
 from nova import wsgi
 from nova.api.rackspace import _id_translator
 import nova.image.service
+from nova.api.rackspace import faults
 
 class Controller(wsgi.Controller):
 
@@ -58,14 +59,14 @@
 
     def delete(self, req, id):
         # Only public images are supported for now.
-        raise exc.HTTPNotFound()
+        raise faults.Fault(exc.HTTPNotFound())
 
     def create(self, req):
         # Only public images are supported for now, so a request to
         # make a backup of a server cannot be supproted.
-        raise exc.HTTPNotFound()
+        raise faults.Fault(exc.HTTPNotFound())
 
     def update(self, req, id):
         # Users may not modify public images, and that's all that 
         # we support for now.
-        raise exc.HTTPNotFound()
+        raise faults.Fault(exc.HTTPNotFound())

=== modified file 'nova/api/rackspace/servers.py'
--- nova/api/rackspace/servers.py	2010-09-28 21:46:21 +0000
+++ nova/api/rackspace/servers.py	2010-09-29 14:20:56 +0000
@@ -24,6 +24,7 @@
 from nova import utils
 from nova import wsgi
 from nova.api.rackspace import _id_translator
+from nova.api.rackspace import faults
 from nova.compute import power_state
 import nova.image.service
 
@@ -120,7 +121,7 @@
         if inst:
             if inst.user_id == user_id:
                 return _entity_detail(inst)
-        raise exc.HTTPNotFound()
+        raise faults.Fault(exc.HTTPNotFound())
 
     def delete(self, req, id):
         """ Destroys a server """
@@ -128,13 +129,13 @@
         instance = self.db_driver.instance_get(None, id)
         if instance and instance['user_id'] == user_id:
             self.db_driver.instance_destroy(None, id)
-            return exc.HTTPAccepted()
-        return exc.HTTPNotFound()
+            return faults.Fault(exc.HTTPAccepted())
+        return faults.Fault(exc.HTTPNotFound())
 
     def create(self, req):
         """ Creates a new server for a given user """
         if not req.environ.has_key('inst_dict'):
-            return exc.HTTPUnprocessableEntity()
+            return faults.Fault(exc.HTTPUnprocessableEntity())
 
         inst = self._build_server_instance(req)
 
@@ -147,22 +148,22 @@
     def update(self, req, id):
         """ Updates the server name or password """
         if not req.environ.has_key('inst_dict'):
-            return exc.HTTPUnprocessableEntity()
+            return faults.Fault(exc.HTTPUnprocessableEntity())
 
         instance = self.db_driver.instance_get(None, id)
         if not instance:
-            return exc.HTTPNotFound()
+            return faults.Fault(exc.HTTPNotFound())
 
         attrs = req.environ['nova.context'].get('model_attributes', None)
         if attrs:
             self.db_driver.instance_update(None, id, _filter_params(attrs))
-        return exc.HTTPNoContent()
+        return faults.Fault(exc.HTTPNoContent())
 
     def action(self, req, id):
         """ multi-purpose method used to reboot, rebuild, and 
         resize a server """
         if not req.environ.has_key('inst_dict'):
-            return exc.HTTPUnprocessableEntity()
+            return faults.Fault(exc.HTTPUnprocessableEntity())
 
     def _build_server_instance(self, req):
         """Build instance data structure and save it to the data store."""

=== added file 'nova/tests/api/rackspace/testfaults.py'
--- nova/tests/api/rackspace/testfaults.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/rackspace/testfaults.py	2010-09-29 14:20:56 +0000
@@ -0,0 +1,30 @@
+import unittest
+import webob
+import webob.exc
+
+from nova.api.rackspace import faults
+
+class TestFaults(unittest.TestCase):
+
+    def test_fault_parts(self):
+        req = webob.Request.blank('/.xml')
+        f = faults.Fault(webob.exc.HTTPBadRequest(explanation='scram'))
+        resp = req.get_response(f)
+
+        first_two_words = resp.body.strip().split()[:2]
+        self.assertEqual(first_two_words, ['<badRequest', 'code="400">'])
+        body_without_spaces = ''.join(resp.body.split())
+        self.assertTrue('<message>scram</message>' in body_without_spaces)
+
+    def test_retry_header(self):
+        req = webob.Request.blank('/.xml')
+        exc = webob.exc.HTTPRequestEntityTooLarge(explanation='sorry', 
+                                                  headers={'Retry-After': 4})
+        f = faults.Fault(exc)
+        resp = req.get_response(f)
+        first_two_words = resp.body.strip().split()[:2]
+        self.assertEqual(first_two_words, ['<overLimit', 'code="413">'])
+        body_sans_spaces = ''.join(resp.body.split())
+        self.assertTrue('<message>sorry</message>' in body_sans_spaces)
+        self.assertTrue('<retryAfter>4</retryAfter>' in body_sans_spaces)
+        self.assertEqual(resp.headers['Retry-After'], 4)