From a3596658c83ed84970cfaba40d7f489728a00d48 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 7 Feb 2012 16:56:04 -0800 Subject: 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 --- nova/api/openstack/compute/contrib/volumes.py | 6 +- nova/api/openstack/volume/snapshots.py | 4 +- .../openstack/compute/contrib/test_snapshots.py | 1 + nova/tests/api/openstack/volume/test_snapshots.py | 154 +++++---------------- 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) -- cgit