Fixes volume snapshotting issues and tests

 * changes call for create_snapshot from contrib/volumes.py
 * fixes some raises in volume/snaphsots.py
 * rewrites volume api tests to use the right endpoint
 * updates contrib snapshots test to catch the error
 * fixes bug 928567
 * fixes bug 928588

Change-Id: Ifa6bd8b70dc0df5c8811b0123072ba83eb170a45

4 files changed, 42 insertions, 123 deletions

diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index 6d0110069..e235ad0fe 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -517,18 +517,20 @@ class SnapshotController(object):
 
         snapshot = body['snapshot']
         volume_id = snapshot['volume_id']
+        volume = self.volume_api.get(context, volume_id)
+
         force = snapshot.get('force', False)
         LOG.audit(_("Create snapshot from volume %s"), volume_id,
                 context=context)
 
         if force:
             new_snapshot = self.volume_api.create_snapshot_force(context,
-                                        volume_id,
+                                        volume,
                                         snapshot.get('display_name'),
                                         snapshot.get('display_description'))
         else:
             new_snapshot = self.volume_api.create_snapshot(context,
-                                        volume_id,
+                                        volume,
                                         snapshot.get('display_name'),
                                         snapshot.get('display_description'))
 
diff --git a/nova/api/openstack/volume/snapshots.py b/nova/api/openstack/volume/snapshots.py
index 36687d736..67cfca3ea 100644
--- a/nova/api/openstack/volume/snapshots.py
+++ b/nova/api/openstack/volume/snapshots.py
@@ -98,7 +98,7 @@ class SnapshotsController(object):
         try:
             vol = self.volume_api.get_snapshot(context, id)
         except exception.NotFound:
-            return exc.HTTPNotFound()
+            raise exc.HTTPNotFound()
 
         return {'snapshot': _translate_snapshot_detail_view(context, vol)}
 
@@ -112,7 +112,7 @@ class SnapshotsController(object):
             snapshot = self.volume_api.get_snapshot(context, id)
             self.volume_api.delete_snapshot(context, snapshot)
         except exception.NotFound:
-            return exc.HTTPNotFound()
+            raise exc.HTTPNotFound()
         return webob.Response(status_int=202)
 
     @wsgi.serializers(xml=SnapshotsTemplate)
diff --git a/nova/tests/api/openstack/compute/contrib/test_snapshots.py b/nova/tests/api/openstack/compute/contrib/test_snapshots.py
index 69784e516..721c76f41 100644
--- a/nova/tests/api/openstack/compute/contrib/test_snapshots.py
+++ b/nova/tests/api/openstack/compute/contrib/test_snapshots.py
@@ -105,6 +105,7 @@ class SnapshotApiTest(test.TestCase):
         self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get)
         self.stubs.Set(volume.api.API, "get_all_snapshots",
             stub_snapshot_get_all)
+        self.stubs.Set(volume.api.API, "get", fakes.stub_volume_get)
 
         self.context = context.get_admin_context()
 
diff --git a/nova/tests/api/openstack/volume/test_snapshots.py b/nova/tests/api/openstack/volume/test_snapshots.py
index 0d2220727..04d55e2f5 100644
--- a/nova/tests/api/openstack/volume/test_snapshots.py
+++ b/nova/tests/api/openstack/volume/test_snapshots.py
@@ -14,14 +14,11 @@
 #    under the License.
 
 import datetime
-import json
-import stubout
 
 from lxml import etree
 import webob
 
 from nova.api.openstack.volume import snapshots
-from nova import context
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -33,8 +30,6 @@ FLAGS = flags.FLAGS
 
 LOG = logging.getLogger('nova.tests.api.openstack.snapshot')
 
-_last_param = {}
-
 
 def _get_default_snapshot_param():
     return {
@@ -49,94 +44,51 @@ def _get_default_snapshot_param():
 
 
 def stub_snapshot_create(self, context, volume_id, name, description):
-    global _last_param
     snapshot = _get_default_snapshot_param()
     snapshot['volume_id'] = volume_id
     snapshot['display_name'] = name
     snapshot['display_description'] = description
-
-    LOG.debug(_("_create: %s"), snapshot)
-    _last_param = snapshot
     return snapshot
 
 
-def stub_snapshot_delete(self, context, snapshot_id):
-    global _last_param
-    _last_param = dict(snapshot_id=snapshot_id)
-
-    LOG.debug(_("_delete: %s"), locals())
-    if snapshot_id != '123':
+def stub_snapshot_delete(self, context, snapshot):
+    if snapshot['id'] != 123:
         raise exception.NotFound
 
 
 def stub_snapshot_get(self, context, snapshot_id):
-    global _last_param
-    _last_param = dict(snapshot_id=snapshot_id)
-
-    LOG.debug(_("_get: %s"), locals())
-    if snapshot_id != '123':
+    if snapshot_id != 123:
         raise exception.NotFound
 
     param = _get_default_snapshot_param()
-    param['id'] = snapshot_id
     return param
 
 
 def stub_snapshot_get_all(self, context):
-    LOG.debug(_("_get_all: %s"), locals())
     param = _get_default_snapshot_param()
-    param['id'] = 123
     return [param]
 
 
 class SnapshotApiTest(test.TestCase):
     def setUp(self):
         super(SnapshotApiTest, self).setUp()
-        self.stubs = stubout.StubOutForTesting()
-        fakes.FakeAuthManager.reset_fake_data()
-        fakes.FakeAuthDatabase.data = {}
-        fakes.stub_out_networking(self.stubs)
-        fakes.stub_out_rate_limiting(self.stubs)
-        fakes.stub_out_auth(self.stubs)
-        self.stubs.Set(volume.api.API, "create_snapshot", stub_snapshot_create)
-        self.stubs.Set(volume.api.API, "create_snapshot_force",
-            stub_snapshot_create)
-        self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete)
+        self.controller = snapshots.SnapshotsController()
+
         self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get)
         self.stubs.Set(volume.api.API, "get_all_snapshots",
             stub_snapshot_get_all)
 
-        self.context = context.get_admin_context()
-
-    def tearDown(self):
-        self.stubs.UnsetAll()
-        super(SnapshotApiTest, self).tearDown()
-
     def test_snapshot_create(self):
-        global _last_param
-        _last_param = {}
-
+        self.stubs.Set(volume.api.API, "create_snapshot", stub_snapshot_create)
+        self.stubs.Set(volume.api.API, 'get', fakes.stub_volume_get)
         snapshot = {"volume_id": 12,
                 "force": False,
                 "display_name": "Snapshot Test Name",
                 "display_description": "Snapshot Test Desc"}
         body = dict(snapshot=snapshot)
-        req = webob.Request.blank('/v1.1/fake/os-snapshots')
-        req.method = 'POST'
-        req.body = json.dumps(body)
-        req.headers['content-type'] = 'application/json'
-
-        resp = req.get_response(fakes.wsgi_app())
-        LOG.debug(_("test_snapshot_create: param=%s"), _last_param)
-        self.assertEqual(resp.status_int, 200)
-
-        # Compare if parameters were correctly passed to stub
-        self.assertEqual(_last_param['display_name'], "Snapshot Test Name")
-        self.assertEqual(_last_param['display_description'],
-            "Snapshot Test Desc")
-
-        resp_dict = json.loads(resp.body)
-        LOG.debug(_("test_snapshot_create: resp_dict=%s"), resp_dict)
+        req = fakes.HTTPRequest.blank('/v1/snapshots')
+        resp_dict = self.controller.create(req, body)
+
         self.assertTrue('snapshot' in resp_dict)
         self.assertEqual(resp_dict['snapshot']['displayName'],
                         snapshot['display_name'])
@@ -144,30 +96,17 @@ class SnapshotApiTest(test.TestCase):
                         snapshot['display_description'])
 
     def test_snapshot_create_force(self):
-        global _last_param
-        _last_param = {}
-
+        self.stubs.Set(volume.api.API, "create_snapshot_force",
+            stub_snapshot_create)
+        self.stubs.Set(volume.api.API, 'get', fakes.stub_volume_get)
         snapshot = {"volume_id": 12,
                 "force": True,
                 "display_name": "Snapshot Test Name",
                 "display_description": "Snapshot Test Desc"}
         body = dict(snapshot=snapshot)
-        req = webob.Request.blank('/v1.1/fake/os-snapshots')
-        req.method = 'POST'
-        req.body = json.dumps(body)
-        req.headers['content-type'] = 'application/json'
-
-        resp = req.get_response(fakes.wsgi_app())
-        LOG.debug(_("test_snapshot_create_force: param=%s"), _last_param)
-        self.assertEqual(resp.status_int, 200)
-
-        # Compare if parameters were correctly passed to stub
-        self.assertEqual(_last_param['display_name'], "Snapshot Test Name")
-        self.assertEqual(_last_param['display_description'],
-            "Snapshot Test Desc")
-
-        resp_dict = json.loads(resp.body)
-        LOG.debug(_("test_snapshot_create_force: resp_dict=%s"), resp_dict)
+        req = fakes.HTTPRequest.blank('/v1/snapshots')
+        resp_dict = self.controller.create(req, body)
+
         self.assertTrue('snapshot' in resp_dict)
         self.assertEqual(resp_dict['snapshot']['displayName'],
                         snapshot['display_name'])
@@ -175,65 +114,42 @@ class SnapshotApiTest(test.TestCase):
                         snapshot['display_description'])
 
     def test_snapshot_delete(self):
-        global _last_param
-        _last_param = {}
+        self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete)
 
         snapshot_id = 123
-        req = webob.Request.blank('/v1.1/fake/os-snapshots/%d' % snapshot_id)
-        req.method = 'DELETE'
-
-        resp = req.get_response(fakes.wsgi_app())
+        req = fakes.HTTPRequest.blank('/v1/snapshots/%d' % snapshot_id)
+        resp = self.controller.delete(req, snapshot_id)
         self.assertEqual(resp.status_int, 202)
-        self.assertEqual(str(_last_param['snapshot_id']), str(snapshot_id))
 
     def test_snapshot_delete_invalid_id(self):
-        global _last_param
-        _last_param = {}
-
+        self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete)
         snapshot_id = 234
-        req = webob.Request.blank('/v1.1/fake/os-snapshots/%d' % snapshot_id)
-        req.method = 'DELETE'
-
-        resp = req.get_response(fakes.wsgi_app())
-        self.assertEqual(resp.status_int, 404)
-        self.assertEqual(str(_last_param['snapshot_id']), str(snapshot_id))
+        req = fakes.HTTPRequest.blank('/v1/snapshots/%d' % snapshot_id)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.delete,
+                          req,
+                          snapshot_id)
 
     def test_snapshot_show(self):
-        global _last_param
-        _last_param = {}
-
         snapshot_id = 123
-        req = webob.Request.blank('/v1.1/fake/os-snapshots/%d' % snapshot_id)
-        req.method = 'GET'
-        resp = req.get_response(fakes.wsgi_app())
-
-        LOG.debug(_("test_snapshot_show: resp=%s"), resp)
-        self.assertEqual(resp.status_int, 200)
-        self.assertEqual(str(_last_param['snapshot_id']), str(snapshot_id))
+        req = fakes.HTTPRequest.blank('/v1/snapshots/%d' % snapshot_id)
+        resp_dict = self.controller.show(req, snapshot_id)
 
-        resp_dict = json.loads(resp.body)
         self.assertTrue('snapshot' in resp_dict)
-        self.assertEqual(resp_dict['snapshot']['id'], str(snapshot_id))
+        self.assertEqual(resp_dict['snapshot']['id'], snapshot_id)
 
     def test_snapshot_show_invalid_id(self):
-        global _last_param
-        _last_param = {}
-
         snapshot_id = 234
-        req = webob.Request.blank('/v1.1/fake/os-snapshots/%d' % snapshot_id)
-        req.method = 'GET'
-        resp = req.get_response(fakes.wsgi_app())
-        self.assertEqual(resp.status_int, 404)
-        self.assertEqual(str(_last_param['snapshot_id']), str(snapshot_id))
+        req = fakes.HTTPRequest.blank('/v1/snapshots/%d' % snapshot_id)
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.show,
+                          req,
+                          snapshot_id)
 
     def test_snapshot_detail(self):
-        req = webob.Request.blank('/v1.1/fake/os-snapshots/detail')
-        req.method = 'GET'
-        resp = req.get_response(fakes.wsgi_app())
-        self.assertEqual(resp.status_int, 200)
+        req = fakes.HTTPRequest.blank('/v1/snapshots/detail')
+        resp_dict = self.controller.detail(req)
 
-        resp_dict = json.loads(resp.body)
-        LOG.debug(_("test_snapshot_detail: resp_dict=%s"), resp_dict)
         self.assertTrue('snapshots' in resp_dict)
         resp_snapshots = resp_dict['snapshots']
         self.assertEqual(len(resp_snapshots), 1)