From: liyingjun Date: Wed, 29 Jul 2015 01:23:28 +0000 (+0800) Subject: Add the ability to update type public status X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f92d71219e0273e3d4937b1f232c5a24ce0f872e;p=openstack-build%2Fcinder-build.git Add the ability to update type public status Currently, the v2 volume type update api doesn't support updating volume type's public status. All volume types created are public by default if not specified. It is not possible to update an existing volume_type is_public status. It is necessary to add updating public status for volume type. If a volume type updated from public to private, the volumes previously created with this type will not be affected, but the user without access will not be able to create volume with this type anymore. APIImpact: Add 'is_public' for v2 type update api. Implements blueprint: volume-types-public-update Change-Id: I4a5b2cf8f747c87908a09835a5edb8873c4e946b --- diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py index 9c32528d7..56afb9317 100644 --- a/cinder/api/contrib/types_manage.py +++ b/cinder/api/contrib/types_manage.py @@ -112,6 +112,7 @@ class VolumeTypesManageController(wsgi.Controller): vol_type = body['volume_type'] description = vol_type.get('description') name = vol_type.get('name') + is_public = vol_type.get('is_public') # Name and description can not be both None. # If name specified, name can not be empty. @@ -123,6 +124,11 @@ class VolumeTypesManageController(wsgi.Controller): msg = _("Specify either volume type name and/or description.") raise webob.exc.HTTPBadRequest(explanation=msg) + if is_public is not None and not utils.is_valid_boolstr(is_public): + msg = _("Invalid value '%s' for is_public. Accepted values: " + "True or False.") % is_public + raise webob.exc.HTTPBadRequest(explanation=msg) + if name: utils.check_string_length(name, 'Type name', min_length=1, max_length=255) @@ -132,7 +138,8 @@ class VolumeTypesManageController(wsgi.Controller): min_length=0, max_length=255) try: - volume_types.update(context, id, name, description) + volume_types.update(context, id, name, description, + is_public=is_public) # Get the updated vol_type = volume_types.get_volume_type(context, id) req.cache_resource(vol_type, name='types') diff --git a/cinder/api/views/types.py b/cinder/api/views/types.py index 78058df0a..c8c1d4ff9 100644 --- a/cinder/api/views/types.py +++ b/cinder/api/views/types.py @@ -22,6 +22,7 @@ class ViewBuilder(common.ViewBuilder): """Trim away extraneous volume type attributes.""" trimmed = dict(id=volume_type.get('id'), name=volume_type.get('name'), + is_public=volume_type.get('is_public'), extra_specs=volume_type.get('extra_specs'), description=volume_type.get('description')) return trimmed if brief else dict(volume_type=trimmed) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index fd1bfb54f..ec96b1725 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2302,6 +2302,10 @@ def volume_type_update(context, volume_type_id, values): if values['description'] is None: del values['description'] + # No is_public change + if values['is_public'] is None: + del values['is_public'] + # No name change if values['name'] is None: del values['name'] diff --git a/cinder/tests/unit/api/contrib/test_types_manage.py b/cinder/tests/unit/api/contrib/test_types_manage.py index f613f7487..542206ef2 100644 --- a/cinder/tests/unit/api/contrib/test_types_manage.py +++ b/cinder/tests/unit/api/contrib/test_types_manage.py @@ -36,9 +36,10 @@ def stub_volume_type(id): extra_specs=specs) -def stub_volume_type_updated(id): +def stub_volume_type_updated(id, is_public=True): return dict(id=id, name='vol_type_%s_%s' % (six.text_type(id), six.text_type(id)), + is_public=is_public, description='vol_type_desc_%s_%s' % ( six.text_type(id), six.text_type(id))) @@ -84,14 +85,6 @@ def return_volume_types_create_duplicate_type(context, raise exception.VolumeTypeExists(id=name) -def return_volume_types_update(context, id, name, description): - pass - - -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)), @@ -104,11 +97,7 @@ def stub_volume_type_updated_name_after_delete(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): +def return_volume_types_get_volume_type_updated(id, is_public=True): if id == "777": raise exception.VolumeTypeNotFound(volume_type_id=id) if id == '888': @@ -119,7 +108,7 @@ def return_volume_types_get_volume_type_updated(context, id): return stub_volume_type_updated_name_after_delete(int(id)) # anything else - return stub_volume_type_updated(int(id)) + return stub_volume_type_updated(int(id), is_public=is_public) def return_volume_types_get_by_name(context, name): @@ -273,14 +262,15 @@ class VolumeTypesManageApiTest(test.TestCase): body = {'volume_type': 'string'} self._create_volume_type_bad_body(body=body) - def test_update(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) + @mock.patch('cinder.volume.volume_types.update') + @mock.patch('cinder.volume.volume_types.get_volume_type') + def test_update(self, mock_get, mock_update): + mock_get.return_value = return_volume_types_get_volume_type_updated( + '1', is_public=False) body = {"volume_type": {"name": "vol_type_1_1", - "description": "vol_type_desc_1_1"}} + "description": "vol_type_desc_1_1", + "is_public": False}} req = fakes.HTTPRequest.blank('/v2/fake/types/1') req.method = 'PUT' @@ -289,7 +279,8 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertEqual(1, len(self.notifier.notifications)) self._check_test_results(res_dict, {'expected_desc': 'vol_type_desc_1_1', - 'expected_name': 'vol_type_1_1'}) + 'expected_name': 'vol_type_1_1', + 'is_public': False}) @mock.patch('cinder.volume.volume_types.update') @mock.patch('cinder.volume.volume_types.get_volume_type') @@ -325,11 +316,11 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertRaises(exception.InvalidInput, self.controller._update, req, '1', body) - def test_update_non_exist(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) + @mock.patch('cinder.volume.volume_types.get_volume_type') + @mock.patch('cinder.volume.volume_types.update') + def test_update_non_exist(self, mock_update, mock_get_volume_type): + mock_get_volume_type.side_effect = exception.VolumeTypeNotFound( + volume_type_id=777) body = {"volume_type": {"name": "vol_type_1_1", "description": "vol_type_desc_1_1"}} @@ -341,11 +332,11 @@ class VolumeTypesManageApiTest(test.TestCase): self.controller._update, req, '777', body) self.assertEqual(1, len(self.notifier.notifications)) - def test_update_db_fail(self): - self.stubs.Set(volume_types, 'update', - return_volume_types_update_fail) - self.stubs.Set(volume_types, 'get_volume_type', - return_volume_types_get_volume_type) + @mock.patch('cinder.volume.volume_types.get_volume_type') + @mock.patch('cinder.volume.volume_types.update') + def test_update_db_fail(self, mock_update, mock_get_volume_type): + mock_update.side_effect = exception.VolumeTypeUpdateFailed(id='1') + mock_get_volume_type.return_value = stub_volume_type(1) body = {"volume_type": {"name": "vol_type_1_1", "description": "vol_type_desc_1_1"}} @@ -374,11 +365,11 @@ class VolumeTypesManageApiTest(test.TestCase): 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) + @mock.patch('cinder.volume.volume_types.update') + @mock.patch('cinder.volume.volume_types.get_volume_type') + def test_update_only_name(self, mock_get, mock_update): + mock_get.return_value = return_volume_types_get_volume_type_updated( + '999') body = {"volume_type": {"name": "vol_type_999_999"}} req = fakes.HTTPRequest.blank('/v2/fake/types/999') @@ -391,11 +382,11 @@ class VolumeTypesManageApiTest(test.TestCase): {'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) + @mock.patch('cinder.volume.volume_types.update') + @mock.patch('cinder.volume.volume_types.get_volume_type') + def test_update_only_description(self, mock_get, mock_update): + mock_get.return_value = return_volume_types_get_volume_type_updated( + '888') body = {"volume_type": {"description": "vol_type_desc_888_888"}} req = fakes.HTTPRequest.blank('/v2/fake/types/888') @@ -408,11 +399,23 @@ class VolumeTypesManageApiTest(test.TestCase): {'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) + def test_update_invalid_is_public(self): + body = {"volume_type": {"name": "test", + "description": "something", + "is_public": "fake"}} + req = fakes.HTTPRequest.blank('/v2/fake/types/1') + req.method = 'PUT' + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._update, req, '1', body) + + @mock.patch('cinder.volume.volume_types.update') + @mock.patch('cinder.volume.volume_types.get_volume_type') + def test_rename_existing_name(self, mock_get, mock_update): + mock_update.side_effect = exception.VolumeTypeExists( + id="666", name="vol_type_666") + mock_get.return_value = return_volume_types_get_volume_type_updated( + '666') # first attempt fail body = {"volume_type": {"name": "vol_type_666"}} req = fakes.HTTPRequest.blank('/v2/fake/types/666') @@ -434,10 +437,7 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertEqual(1, len(self.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) + mock_update.side_effect = mock.MagicMock() body = {"volume_type": {"name": "vol_type_666_666"}} req = fakes.HTTPRequest.blank('/v2/fake/types/666') req.method = 'PUT' @@ -457,3 +457,6 @@ class VolumeTypesManageApiTest(test.TestCase): if expected_results.get('expected_name'): self.assertEqual(expected_results['expected_name'], results['volume_type']['name']) + if expected_results.get('is_public') is not None: + self.assertEqual(expected_results['is_public'], + results['volume_type']['is_public']) diff --git a/cinder/tests/unit/api/v1/test_types.py b/cinder/tests/unit/api/v1/test_types.py index 86c791569..cdb9b4e83 100644 --- a/cinder/tests/unit/api/v1/test_types.py +++ b/cinder/tests/unit/api/v1/test_types.py @@ -128,6 +128,7 @@ class VolumeTypesApiTest(test.TestCase): expected_volume_type = dict(name='new_type', extra_specs={}, description=None, + is_public=None, id=42) self.assertDictMatch(output['volume_type'], expected_volume_type) @@ -154,6 +155,7 @@ class VolumeTypesApiTest(test.TestCase): expected_volume_type = dict(name='new_type', extra_specs={}, id=42 + i, + is_public=None, description=None) self.assertDictMatch(output['volume_types'][i], expected_volume_type) diff --git a/cinder/tests/unit/api/v2/test_types.py b/cinder/tests/unit/api/v2/test_types.py index 9fc86169e..f819cbdcd 100644 --- a/cinder/tests/unit/api/v2/test_types.py +++ b/cinder/tests/unit/api/v2/test_types.py @@ -153,6 +153,7 @@ class VolumeTypesApiTest(test.TestCase): raw_volume_type = dict( name='new_type', description='new_type_desc', + is_public=True, deleted=False, created_at=now, updated_at=now, @@ -168,6 +169,7 @@ class VolumeTypesApiTest(test.TestCase): expected_volume_type = dict( name='new_type', description='new_type_desc', + is_public=True, extra_specs={}, id=42, ) @@ -183,6 +185,7 @@ class VolumeTypesApiTest(test.TestCase): dict( name='new_type', description='new_type_desc', + is_public=True, deleted=False, created_at=now, updated_at=now, @@ -200,6 +203,7 @@ class VolumeTypesApiTest(test.TestCase): expected_volume_type = dict( name='new_type', description='new_type_desc', + is_public=True, extra_specs={}, id=42 + i ) diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py index 8aa9b2916..a6fc17141 100644 --- a/cinder/volume/volume_types.py +++ b/cinder/volume/volume_types.py @@ -57,7 +57,7 @@ def create(context, return type_ref -def update(context, id, name, description): +def update(context, id, name, description, is_public=None): """Update volume type by id.""" if id is None: msg = _("id cannot be None") @@ -66,7 +66,8 @@ def update(context, id, name, description): type_updated = db.volume_type_update(context, id, dict(name=name, - description=description)) + description=description, + is_public=is_public)) except db_exc.DBError: LOG.exception(_LE('DB error:')) raise exception.VolumeTypeUpdateFailed(id=id)