Merge "Move createBackup server action into extension"

4 files changed, 202 insertions, 201 deletions

diff --git a/nova/api/openstack/v2/contrib/admin_actions.py b/nova/api/openstack/v2/contrib/admin_actions.py
index 46a4ec6f0..27bcf96c7 100644
--- a/nova/api/openstack/v2/contrib/admin_actions.py
+++ b/nova/api/openstack/v2/contrib/admin_actions.py
@@ -12,11 +12,13 @@
 #   License for the specific language governing permissions and limitations
 #   under the License.
 
+import os.path
 import traceback
 
 import webob
 from webob import exc
 
+from nova.api.openstack import common
 from nova.api.openstack.v2 import extensions
 from nova import compute
 from nova import exception
@@ -30,8 +32,10 @@ LOG = logging.getLogger("nova.api.openstack.v2.contrib.admin_actions")
 
 
 class Admin_actions(extensions.ExtensionDescriptor):
-    """Adds admin-only server actions: pause, unpause, suspend,
-    resume, migrate, resetNetwork, injectNetworkInfo, lock and unlock
+    """Enable admin-only server actions
+
+    Actions include: pause, unpause, suspend, resume, migrate,
+    resetNetwork, injectNetworkInfo, lock, unlock, createBackup
     """
 
     name = "AdminActions"
@@ -173,6 +177,75 @@ class Admin_actions(extensions.ExtensionDescriptor):
             raise exc.HTTPUnprocessableEntity()
         return webob.Response(status_int=202)
 
+    def _create_backup(self, input_dict, req, instance_id):
+        """Backup a server instance.
+
+        Images now have an `image_type` associated with them, which can be
+        'snapshot' or the backup type, like 'daily' or 'weekly'.
+
+        If the image_type is backup-like, then the rotation factor can be
+        included and that will cause the oldest backups that exceed the
+        rotation factor to be deleted.
+
+        """
+        context = req.environ["nova.context"]
+
+        try:
+            entity = input_dict["createBackup"]
+        except (KeyError, TypeError):
+            raise exc.HTTPBadRequest(_("Malformed request body"))
+
+        try:
+            image_name = entity["name"]
+            backup_type = entity["backup_type"]
+            rotation = entity["rotation"]
+
+        except KeyError as missing_key:
+            msg = _("createBackup entity requires %s attribute") % missing_key
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        except TypeError:
+            msg = _("Malformed createBackup entity")
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        try:
+            rotation = int(rotation)
+        except ValueError:
+            msg = _("createBackup attribute 'rotation' must be an integer")
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        # preserve link to server in image properties
+        server_ref = os.path.join(req.application_url, 'servers', instance_id)
+        props = {'instance_ref': server_ref}
+
+        metadata = entity.get('metadata', {})
+        common.check_img_metadata_quota_limit(context, metadata)
+        try:
+            props.update(metadata)
+        except ValueError:
+            msg = _("Invalid metadata")
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        try:
+            instance = self.compute_api.get(context, instance_id)
+        except exception.NotFound:
+            raise exc.HTTPNotFound(_("Instance not found"))
+
+        image = self.compute_api.backup(context,
+                                        instance,
+                                        image_name,
+                                        backup_type,
+                                        rotation,
+                                        extra_properties=props)
+
+        # build location of newly-created image entity
+        image_id = str(image['id'])
+        image_ref = os.path.join(req.application_url, 'images', image_id)
+
+        resp = webob.Response(status_int=202)
+        resp.headers['Location'] = image_ref
+        return resp
+
     def get_actions(self):
         actions = [
             #TODO(bcwaldon): These actions should be prefixed with 'os-'
@@ -183,6 +256,10 @@ class Admin_actions(extensions.ExtensionDescriptor):
             extensions.ActionExtension("servers", "migrate", self._migrate),
 
             extensions.ActionExtension("servers",
+                                       "createBackup",
+                                       self._create_backup),
+
+            extensions.ActionExtension("servers",
                                        "resetNetwork",
                                        self._reset_network),
 
diff --git a/nova/api/openstack/v2/servers.py b/nova/api/openstack/v2/servers.py
index cb0ece1f0..57ae5306f 100644
--- a/nova/api/openstack/v2/servers.py
+++ b/nova/api/openstack/v2/servers.py
@@ -500,12 +500,6 @@ class Controller(wsgi.Controller):
             'createImage': self._action_create_image,
         }
 
-        if FLAGS.allow_admin_api:
-            admin_actions = {
-                'createBackup': self._action_create_backup,
-            }
-            _actions.update(admin_actions)
-
         for key in body:
             if key in _actions:
                 return _actions[key](body, req, id)
@@ -516,68 +510,6 @@ class Controller(wsgi.Controller):
         msg = _("Invalid request body")
         raise exc.HTTPBadRequest(explanation=msg)
 
-    def _action_create_backup(self, input_dict, req, instance_id):
-        """Backup a server instance.
-
-        Images now have an `image_type` associated with them, which can be
-        'snapshot' or the backup type, like 'daily' or 'weekly'.
-
-        If the image_type is backup-like, then the rotation factor can be
-        included and that will cause the oldest backups that exceed the
-        rotation factor to be deleted.
-
-        """
-        context = req.environ["nova.context"]
-        entity = input_dict["createBackup"]
-
-        try:
-            image_name = entity["name"]
-            backup_type = entity["backup_type"]
-            rotation = entity["rotation"]
-
-        except KeyError as missing_key:
-            msg = _("createBackup entity requires %s attribute") % missing_key
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        except TypeError:
-            msg = _("Malformed createBackup entity")
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        try:
-            rotation = int(rotation)
-        except ValueError:
-            msg = _("createBackup attribute 'rotation' must be an integer")
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        # preserve link to server in image properties
-        server_ref = os.path.join(req.application_url, 'servers', instance_id)
-        props = {'instance_ref': server_ref}
-
-        metadata = entity.get('metadata', {})
-        common.check_img_metadata_quota_limit(context, metadata)
-        try:
-            props.update(metadata)
-        except ValueError:
-            msg = _("Invalid metadata")
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        instance = self._get_server(context, instance_id)
-
-        image = self.compute_api.backup(context,
-                                        instance,
-                                        image_name,
-                                        backup_type,
-                                        rotation,
-                                        extra_properties=props)
-
-        # build location of newly-created image entity
-        image_id = str(image['id'])
-        image_ref = os.path.join(req.application_url, 'images', image_id)
-
-        resp = webob.Response(status_int=202)
-        resp.headers['Location'] = image_ref
-        return resp
-
     def _action_confirm_resize(self, input_dict, req, id):
         context = req.environ['nova.context']
         instance = self._get_server(context, id)
@@ -1021,7 +953,6 @@ class ServerXMLDeserializer(wsgi.MetadataXMLDeserializer):
 
         action_deserializer = {
             'createImage': self._action_create_image,
-            'createBackup': self._action_create_backup,
             'changePassword': self._action_change_password,
             'reboot': self._action_reboot,
             'rebuild': self._action_rebuild,
@@ -1037,10 +968,6 @@ class ServerXMLDeserializer(wsgi.MetadataXMLDeserializer):
     def _action_create_image(self, node):
         return self._deserialize_image_action(node, ('name',))
 
-    def _action_create_backup(self, node):
-        attributes = ('name', 'backup_type', 'rotation')
-        return self._deserialize_image_action(node, attributes)
-
     def _action_change_password(self, node):
         if not node.hasAttribute("adminPass"):
             raise AttributeError("No adminPass was specified in request")
diff --git a/nova/tests/api/openstack/v2/contrib/test_admin_actions.py b/nova/tests/api/openstack/v2/contrib/test_admin_actions.py
index a7237ae58..53016636d 100644
--- a/nova/tests/api/openstack/v2/contrib/test_admin_actions.py
+++ b/nova/tests/api/openstack/v2/contrib/test_admin_actions.py
@@ -17,6 +17,9 @@ import json
 
 import webob
 
+from nova.api.openstack import v2
+from nova.api.openstack.v2 import extensions
+from nova.api.openstack import wsgi
 from nova import compute
 from nova import flags
 from nova import test
@@ -77,3 +80,123 @@ class AdminActionsTest(test.TestCase):
             req.content_type = 'application/json'
             res = req.get_response(app)
             self.assertEqual(res.status_int, 202)
+
+
+class CreateBackupTests(test.TestCase):
+
+    def setUp(self):
+        super(CreateBackupTests, self).setUp()
+
+        self.stubs.Set(compute.API, 'get', fake_compute_api_get)
+        self.backup_stubs = fakes.stub_out_compute_api_backup(self.stubs)
+
+        self.flags(allow_admin_api=True)
+        router = v2.APIRouter()
+        ext_middleware = extensions.ExtensionMiddleware(router)
+        self.app = wsgi.LazySerializationMiddleware(ext_middleware)
+
+        self.uuid = utils.gen_uuid()
+
+    def _get_request(self, body):
+        url = '/fake/servers/%s/action' % self.uuid
+        req = fakes.HTTPRequest.blank(url)
+        req.method = 'POST'
+        req.content_type = 'application/json'
+        req.body = json.dumps(body)
+        return req
+
+    def test_create_backup_with_metadata(self):
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+                'rotation': 1,
+                'metadata': {'123': 'asdf'},
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+
+        self.assertEqual(response.status_int, 202)
+        self.assertTrue(response.headers['Location'])
+
+    def test_create_backup_with_too_much_metadata(self):
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+                'rotation': 1,
+                'metadata': {'123': 'asdf'},
+            },
+        }
+        for num in range(FLAGS.quota_metadata_items + 1):
+            body['createBackup']['metadata']['foo%i' % num] = "bar"
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 413)
+
+    def test_create_backup_no_name(self):
+        """Name is required for backups"""
+        body = {
+            'createBackup': {
+                'backup_type': 'daily',
+                'rotation': 1,
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 400)
+
+    def test_create_backup_no_rotation(self):
+        """Rotation is required for backup requests"""
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 400)
+
+    def test_create_backup_no_backup_type(self):
+        """Backup Type (daily or weekly) is required for backup requests"""
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'rotation': 1,
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 400)
+
+    def test_create_backup_bad_entity(self):
+        body = {'createBackup': 'go'}
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 400)
+
+    def test_create_backup(self):
+        """The happy path for creating backups"""
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+                'rotation': 1,
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+
+        self.assertTrue(response.headers['Location'])
+        instance_ref = self.backup_stubs.extra_props_last_call['instance_ref']
+        expected_server_location = 'http://localhost/v2/servers/%s' % self.uuid
+        self.assertEqual(expected_server_location, instance_ref)
diff --git a/nova/tests/api/openstack/v2/test_server_actions.py b/nova/tests/api/openstack/v2/test_server_actions.py
index 9a96c7337..8d9db71ef 100644
--- a/nova/tests/api/openstack/v2/test_server_actions.py
+++ b/nova/tests/api/openstack/v2/test_server_actions.py
@@ -158,7 +158,6 @@ class ServerActionsControllerTest(test.TestCase):
         fakes.stub_out_nw_api(self.stubs)
         fakes.stub_out_rate_limiting(self.stubs)
         self.snapshot = fakes.stub_out_compute_api_snapshot(self.stubs)
-        self.backup = fakes.stub_out_compute_api_backup(self.stubs)
         service_class = 'nova.image.glance.GlanceImageService'
         self.service = utils.import_object(service_class)
         self.context = context.RequestContext(1, None)
@@ -602,131 +601,6 @@ class ServerActionsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPConflict,
                           self.controller.action, req, FAKE_UUID, body)
 
-    def test_create_backup(self):
-        """The happy path for creating backups"""
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'backup_type': 'daily',
-                'rotation': 1,
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        response = self.controller.action(req, FAKE_UUID, body)
-
-        self.assertTrue(response.headers['Location'])
-        server_location = self.backup.extra_props_last_call['instance_ref']
-        expected_server_location = 'http://localhost/v2/servers/' + self.uuid
-        self.assertEqual(expected_server_location, server_location)
-
-    def test_create_backup_admin_api_off(self):
-        """The happy path for creating backups"""
-        self.flags(allow_admin_api=False)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'backup_type': 'daily',
-                'rotation': 1,
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.action, req, FAKE_UUID, body)
-
-    def test_create_backup_with_metadata(self):
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'backup_type': 'daily',
-                'rotation': 1,
-                'metadata': {'123': 'asdf'},
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        response = self.controller.action(req, FAKE_UUID, body)
-
-        self.assertTrue(response.headers['Location'])
-
-    def test_create_backup_with_too_much_metadata(self):
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'backup_type': 'daily',
-                'rotation': 1,
-                'metadata': {'123': 'asdf'},
-            },
-        }
-        for num in range(FLAGS.quota_metadata_items + 1):
-            body['createBackup']['metadata']['foo%i' % num] = "bar"
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
-                          self.controller.action, req, FAKE_UUID, body)
-
-    def test_create_backup_no_name(self):
-        """Name is required for backups"""
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'backup_type': 'daily',
-                'rotation': 1,
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.action, req, FAKE_UUID, body)
-
-    def test_create_backup_no_rotation(self):
-        """Rotation is required for backup requests"""
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'backup_type': 'daily',
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.action, req, FAKE_UUID, body)
-
-    def test_create_backup_no_backup_type(self):
-        """Backup Type (daily or weekly) is required for backup requests"""
-        self.flags(allow_admin_api=True)
-
-        body = {
-            'createBackup': {
-                'name': 'Backup 1',
-                'rotation': 1,
-            },
-        }
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.action, req, FAKE_UUID, body)
-
-    def test_create_backup_bad_entity(self):
-        self.flags(allow_admin_api=True)
-
-        body = {'createBackup': 'go'}
-
-        req = fakes.HTTPRequest.blank(self.url)
-        self.assertRaises(webob.exc.HTTPBadRequest,
-                          self.controller.action, req, FAKE_UUID, body)
-
 
 class TestServerActionXMLDeserializer(test.TestCase):