From d275831841d9bb7e879f77eb28b0b236d70703d1 Mon Sep 17 00:00:00 2001 From: Gloria Gu Date: Wed, 10 Dec 2014 14:58:21 -0800 Subject: [PATCH] Update volume type name for volume type API This continues the change for updating volume type to include changing name. - Updated the following APIs and tests for volume type * update volume type PUT http://:8776/v2/${tenant_id}/types/${vol_type_id} body { "volume_type": { "name": "test_updated", "description":"updated_desc" } } ** user can update name ** if update name, name can not be empty spaces. ** if update name, can not change the name to be the same as the one in another volume type ** name and description can not be both None Implements: blueprint volume-type-description Change-Id: Idc08f796932df709ff20c17f6bc8d4753d85ad3c --- cinder/api/contrib/types_manage.py | 19 ++- cinder/db/sqlalchemy/api.py | 24 +++- cinder/tests/api/contrib/test_types_manage.py | 123 +++++++++++++++++- cinder/tests/test_volume_types.py | 3 + cinder/volume/volume_types.py | 5 +- 5 files changed, 157 insertions(+), 17 deletions(-) diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py index 2839fd939..434d4fa43 100644 --- a/cinder/api/contrib/types_manage.py +++ b/cinder/api/contrib/types_manage.py @@ -99,17 +99,22 @@ class VolumeTypesManageController(wsgi.Controller): raise webob.exc.HTTPBadRequest() vol_type = body['volume_type'] - description = vol_type.get('description', None) + description = vol_type.get('description') + name = vol_type.get('name') - if description is None: - msg = _("Specify the description to update.") + # Name and description can not be both None. + # If name specified, name can not be empty. + if name and len(name.strip()) == 0: + msg = _("Volume type name can not be empty.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + if name is None and description is None: + msg = _("Specify either volume type name and/or description.") raise webob.exc.HTTPBadRequest(explanation=msg) try: - # check it exists - vol_type = volume_types.get_volume_type(context, id) - volume_types.update(context, id, description) - # get the updated + volume_types.update(context, id, name, description) + # Get the updated vol_type = volume_types.get_volume_type(context, id) req.cache_resource(vol_type, name='types') self._notify_volume_type_info( diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 7b5cc1e63..e3137dc5c 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2028,13 +2028,35 @@ def _volume_type_get_query(context, session=None, read_deleted=None, def volume_type_update(context, volume_type_id, values): session = get_session() with session.begin(): + # Check it exists volume_type_ref = _volume_type_ref_get(context, volume_type_id, session) - if not volume_type_ref: raise exception.VolumeTypeNotFound(type_id=volume_type_id) + # No description change + if values['description'] is None: + del values['description'] + + # No name change + if values['name'] is None: + del values['name'] + else: + # Volume type name is unique. If change to a name that belongs to + # a different volume_type , it should be prevented. + check_vol_type = None + try: + check_vol_type = \ + _volume_type_get_by_name(context, + values['name'], + session=session) + except exception.VolumeTypeNotFoundByName: + pass + else: + if check_vol_type.get('id') != volume_type_id: + raise exception.VolumeTypeExists(id=values['name']) + volume_type_ref.update(values) volume_type_ref.save(session=session) volume_type = volume_type_get(context, volume_type_id) diff --git a/cinder/tests/api/contrib/test_types_manage.py b/cinder/tests/api/contrib/test_types_manage.py index 07901289e..6b8530ea5 100644 --- a/cinder/tests/api/contrib/test_types_manage.py +++ b/cinder/tests/api/contrib/test_types_manage.py @@ -84,19 +84,39 @@ def return_volume_types_create_duplicate_type(context, raise exception.VolumeTypeExists(id=name) -def return_volume_types_update(context, id, description): +def return_volume_types_update(context, id, name, description): pass -def return_volume_types_update_fail(context, id, description): +def return_volume_types_update_fail(context, id, name, description): raise exception.VolumeTypeUpdateFailed(id=id) +def stub_volume_type_updated_name_only(id): + return dict(id=id, + name='vol_type_%s_%s' % (six.text_type(id), six.text_type(id)), + description='vol_type_desc_%s' % six.text_type(id)) + + +def stub_volume_type_updated_name_after_delete(id): + return dict(id=id, + name='vol_type_%s' % six.text_type(id), + description='vol_type_desc_%s' % six.text_type(id)) + + +def return_volume_types_update_exist(context, id, name, description): + raise exception.VolumeTypeExists(id=id, name=name) + + def return_volume_types_get_volume_type_updated(context, id): if id == "777": raise exception.VolumeTypeNotFound(volume_type_id=id) if id == '888': return stub_volume_type_updated_desc_only(int(id)) + if id == '999': + return stub_volume_type_updated_name_only(int(id)) + if id == '666': + return stub_volume_type_updated_name_after_delete(int(id)) # anything else return stub_volume_type_updated(int(id)) @@ -213,7 +233,8 @@ class VolumeTypesManageApiTest(test.TestCase): self.stubs.Set(volume_types, 'get_volume_type', return_volume_types_get_volume_type_updated) - body = {"volume_type": {"description": "vol_type_desc_1_1"}} + body = {"volume_type": {"name": "vol_type_1_1", + "description": "vol_type_desc_1_1"}} req = fakes.HTTPRequest.blank('/v2/fake/types/1') req.method = 'PUT' @@ -221,7 +242,8 @@ class VolumeTypesManageApiTest(test.TestCase): res_dict = self.controller._update(req, '1', body) self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) self._check_test_results(res_dict, - {'expected_desc': 'vol_type_desc_1_1'}) + {'expected_desc': 'vol_type_desc_1_1', + 'expected_name': 'vol_type_1_1'}) def test_update_non_exist(self): self.stubs.Set(volume_types, 'update', @@ -229,7 +251,8 @@ class VolumeTypesManageApiTest(test.TestCase): self.stubs.Set(volume_types, 'get_volume_type', return_volume_types_get_volume_type) - body = {"volume_type": {"description": "vol_type_desc_1_1"}} + body = {"volume_type": {"name": "vol_type_1_1", + "description": "vol_type_desc_1_1"}} req = fakes.HTTPRequest.blank('/v2/fake/types/777') req.method = 'PUT' @@ -244,7 +267,8 @@ class VolumeTypesManageApiTest(test.TestCase): self.stubs.Set(volume_types, 'get_volume_type', return_volume_types_get_volume_type) - body = {"volume_type": {"description": "vol_type_desc_1_1"}} + body = {"volume_type": {"name": "vol_type_1_1", + "description": "vol_type_desc_1_1"}} req = fakes.HTTPRequest.blank('/v2/fake/types/1') req.method = 'PUT' @@ -253,7 +277,7 @@ class VolumeTypesManageApiTest(test.TestCase): self.controller._update, req, '1', body) self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) - def test_update_no_description(self): + def test_update_no_name_no_description(self): body = {"volume_type": {}} req = fakes.HTTPRequest.blank('/v2/fake/types/1') req.method = 'PUT' @@ -261,6 +285,91 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller._update, req, '1', body) + def test_update_empty_name(self): + body = {"volume_type": {"name": " ", + "description": "something"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/1') + req.method = 'PUT' + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._update, req, '1', body) + + def test_update_only_name(self): + self.stubs.Set(volume_types, 'update', + return_volume_types_update) + self.stubs.Set(volume_types, 'get_volume_type', + return_volume_types_get_volume_type_updated) + + body = {"volume_type": {"name": "vol_type_999_999"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/999') + req.method = 'PUT' + + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + res_dict = self.controller._update(req, '999', body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + self._check_test_results(res_dict, + {'expected_name': 'vol_type_999_999', + 'expected_desc': 'vol_type_desc_999'}) + + def test_update_only_description(self): + self.stubs.Set(volume_types, 'update', + return_volume_types_update) + self.stubs.Set(volume_types, 'get_volume_type', + return_volume_types_get_volume_type_updated) + + body = {"volume_type": {"description": "vol_type_desc_888_888"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/888') + req.method = 'PUT' + + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + res_dict = self.controller._update(req, '888', body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + self._check_test_results(res_dict, + {'expected_name': 'vol_type_888', + 'expected_desc': 'vol_type_desc_888_888'}) + + def test_rename_existing_name(self): + self.stubs.Set(volume_types, 'update', + return_volume_types_update_exist) + self.stubs.Set(volume_types, 'get_volume_type', + return_volume_types_get_volume_type_updated) + # first attempt fail + body = {"volume_type": {"name": "vol_type_666"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/666') + req.method = 'PUT' + + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + self.assertRaises(webob.exc.HTTPConflict, + self.controller._update, req, '666', body) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + + # delete + fake_notifier.reset() + self.stubs.Set(volume_types, 'destroy', + return_volume_types_destroy) + + req = fakes.HTTPRequest.blank('/v2/fake/types/1') + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + self.controller._delete(req, '1') + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + + # update again + self.stubs.Set(volume_types, 'update', + return_volume_types_update) + self.stubs.Set(volume_types, 'get_volume_type', + return_volume_types_get_volume_type_updated) + body = {"volume_type": {"name": "vol_type_666_666"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/666') + req.method = 'PUT' + + fake_notifier.reset() + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + res_dict = self.controller._update(req, '666', body) + self._check_test_results(res_dict, + {'expected_name': 'vol_type_666', + 'expected_desc': 'vol_type_desc_666'}) + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + def _check_test_results(self, results, expected_results): self.assertEqual(1, len(results)) self.assertEqual(expected_results['expected_desc'], diff --git a/cinder/tests/test_volume_types.py b/cinder/tests/test_volume_types.py index b6abc15a8..f308c442c 100644 --- a/cinder/tests/test_volume_types.py +++ b/cinder/tests/test_volume_types.py @@ -78,10 +78,13 @@ class VolumeTypeTestCase(test.TestCase): 'drive type was not created') # update + new_type_name = self.vol_type1_name + '_updated' new_type_desc = self.vol_type1_description + '_updated' type_ref_updated = volume_types.update(self.ctxt, type_ref.id, + new_type_name, new_type_desc) + self.assertEqual(new_type_name, type_ref_updated['name']) self.assertEqual(new_type_desc, type_ref_updated['description']) # destroy diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py index 8624a10f4..207186d4c 100644 --- a/cinder/volume/volume_types.py +++ b/cinder/volume/volume_types.py @@ -58,7 +58,7 @@ def create(context, return type_ref -def update(context, id, description): +def update(context, id, name, description): """Update volume type by id.""" if id is None: msg = _("id cannot be None") @@ -66,7 +66,8 @@ def update(context, id, description): try: type_updated = db.volume_type_update(context, id, - dict(description=description)) + dict(name=name, + description=description)) except db_exc.DBError as e: LOG.exception(_LE('DB error: %s') % six.text_type(e)) raise exception.VolumeTypeUpdateFailed(id=id) -- 2.45.2