openstack
/
cinder
Code Issues Proposed changes

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
This commit is contained in:
liyingjun 2015-07-29 09:23:28 +08:00
parent 1bbae62b3d
commit f92d71219e
7 changed files with 76 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cinder/api/contrib/types_manage.py
View File

@ -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')

1
cinder/api/views/types.py
View File

@ -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)

4
cinder/db/sqlalchemy/api.py
View File

@ -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']

105
cinder/tests/unit/api/contrib/test_types_manage.py
View File

@ -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'])

2
cinder/tests/unit/api/v1/test_types.py
View File

@ -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)

4
cinder/tests/unit/api/v2/test_types.py
View File

@ -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
)

5
cinder/volume/volume_types.py
View File

@ -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)