openstack
/
cinder
Code Issues Proposed changes

Fix deleting qos specs key 

Previously deleting a key in certain qos specs was accomplished via
'update' API. Unfortunately, 'update' isn't able to tell the
difference between setting a key with no value and deleting a key (and
its value).
This change adds an new API 'delete_keys' to qos_specs API extension.
'delete_keys' API allows client to specify a list of to-be-deleted keys
in one single request (batch mode!), which can be handy when removing
multiple keys in a qos specs.

Example URL and request body for 'delete_keys' API:
 PUT to http://127.0.0.1:8776/v2/qos-specs/QOS_SPECS_UUID/delete_keys
 with body: {'keys': ['foo', 'bar', 'zoo']}

Above example request will result in 'foo', 'bar', 'zoo' key/value
pairs of QOS_SPECS_UUID be marked as deleted in DB.  If QOS_SPECS_UUID
doesn't exist, a 404 error will return; if any key in 'foo', 'bar',
'zoo' couldn't be found in QOS_SPECS_UUID, a 400 error will return with
error message telling which key couldn't be found. Note that Cinder
will puke 400 and stop trying the rest once it encounters a
non-existing (or deleted) key amoung the given list of keys.

This change also fixes 'list'/'show' API includes deleted keys in
result.

Fix bug: # 1223660
Fix bug: # 1223677

Change-Id: Ia3cb07e204d655a9b837b317ce7117feb3c86a2d
This commit is contained in:
Zhiteng Huang 2013-09-10 16:39:52 +08:00
parent 6ae44d99fe
commit ca85d237e2
7 changed files with 186 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
cinder/api/contrib/qos_specs_manage.py
View File

@ -26,6 +26,7 @@ from cinder.api import xmlutil
from cinder import exception
from cinder.openstack.common import log as logging
from cinder.openstack.common.notifier import api as notifier_api
from cinder.openstack.common import strutils
from cinder.volume import qos_specs
@ -157,6 +158,7 @@ class QoSSpecsController(wsgi.Controller):
'qos_specs.update',
notifier_err)
raise webob.exc.HTTPInternalServerError(explanation=str(err))
return body
@wsgi.serializers(xml=QoSSpecsTemplate)
@ -179,11 +181,12 @@ class QoSSpecsController(wsgi.Controller):
force = req.params.get('force', None)
#convert string to bool type in strict manner
force = strutils.bool_from_string(force)
LOG.debug("Delete qos_spec: %(id)s, force: %(force)s" %
{'id': id, 'force': force})
try:
qos_specs.get_qos_specs(context, id)
qos_specs.delete(context, id, force)
notifier_info = dict(id=id)
notifier_api.notify(context, 'QoSSpecs',
@ -208,6 +211,40 @@ class QoSSpecsController(wsgi.Controller):
return webob.Response(status_int=202)
def delete_keys(self, req, id, body):
"""Deletes specified keys in qos specs."""
context = req.environ['cinder.context']
authorize(context)
if not (body and 'keys' in body
and isinstance(body.get('keys'), list)):
raise webob.exc.HTTPBadRequest()
keys = body['keys']
LOG.debug("Delete_key spec: %(id)s, keys: %(keys)s" %
{'id': id, 'keys': keys})
try:
qos_specs.delete_keys(context, id, keys)
notifier_info = dict(id=id)
notifier_api.notify(context, 'QoSSpecs',
'qos_specs.delete_keys',
notifier_api.INFO, notifier_info)
except exception.QoSSpecsNotFound as err:
notifier_err = dict(id=id, error_message=str(err))
self._notify_qos_specs_error(context,
'qos_specs.delete_keys',
notifier_err)
raise webob.exc.HTTPNotFound(explanation=str(err))
except exception.QoSSpecsKeyNotFound as err:
notifier_err = dict(id=id, error_message=str(err))
self._notify_qos_specs_error(context,
'qos_specs.delete_keys',
notifier_err)
raise webob.exc.HTTPBadRequest(explanation=str(err))
return webob.Response(status_int=202)
@wsgi.serializers(xml=QoSSpecsTemplate)
def associations(self, req, id):
"""List all associations of given qos specs."""
@ -342,7 +379,6 @@ class QoSSpecsController(wsgi.Controller):
LOG.debug("Disassociate qos_spec: %s from all." % id)
try:
qos_specs.get_qos_specs(context, id)
qos_specs.disassociate_all(context, id)
notifier_info = dict(id=id)
notifier_api.notify(context, 'QoSSpecs',
@ -380,7 +416,8 @@ class Qos_specs_manage(extensions.ExtensionDescriptor):
member_actions={"associations": "GET",
"associate": "GET",
"disassociate": "GET",
"disassociate_all": "GET"})
"disassociate_all": "GET",
"delete_keys": "PUT"})
resources.append(res)

5
cinder/db/api.py
View File

@ -524,6 +524,11 @@ def qos_specs_delete(context, qos_specs_id):
IMPL.qos_specs_delete(context, qos_specs_id)
def qos_specs_item_delete(context, qos_specs_id, key):
"""Delete specified key in the qos_specs."""
IMPL.qos_specs_item_delete(context, qos_specs_id, key)
def qos_specs_update(context, qos_specs_id, specs):
"""Update qos specs.

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

@ -2081,7 +2081,9 @@ def _dict_with_children_specs(specs):
"""Convert specs list to a dict."""
result = {}
for spec in specs:
result.update({spec['key']: spec['value']})
# Skip deleted keys
if not spec['deleted']:
result.update({spec['key']: spec['value']})
return result
@ -2203,12 +2205,15 @@ def qos_specs_disassociate_all(context, qos_specs_id):
@require_admin_context
def qos_specs_item_delete(context, qos_specs_id, key):
_qos_specs_get_item(context, qos_specs_id, key)
_qos_specs_get_ref(context, qos_specs_id, None). \
filter_by(key=key). \
update({'deleted': True,
'deleted_at': timeutils.utcnow(),
'updated_at': literal_column('updated_at')})
session = get_session()
with session.begin():
_qos_specs_get_item(context, qos_specs_id, key)
session.query(models.QualityOfServiceSpecs). \
filter(models.QualityOfServiceSpecs.key == key). \
filter(models.QualityOfServiceSpecs.specs_id == qos_specs_id). \
update({'deleted': True,
'deleted_at': timeutils.utcnow(),
'updated_at': literal_column('updated_at')})
@require_admin_context

42
cinder/tests/api/contrib/test_qos_specs_manage.py
View File

@ -67,6 +67,15 @@ def return_qos_specs_delete(context, id, force):
pass
def return_qos_specs_delete_keys(context, id, keys):
if id == "777":
raise exception.QoSSpecsNotFound(specs_id=id)
if 'foo' in keys:
raise exception.QoSSpecsKeyNotFound(specs_id=id,
specs_key='foo')
def return_qos_specs_update(context, id, specs):
if id == "777":
raise exception.QoSSpecsNotFound(specs_id=id)
@ -134,7 +143,7 @@ class QoSSpecManageApiTest(test.TestCase):
host='fake',
notification_driver=[test_notifier.__name__])
self.controller = qos_specs_manage.QoSSpecsController()
"""to reset notifier drivers left over from other api/contrib tests"""
#reset notifier drivers left over from other api/contrib tests
notifier_api._reset_drivers()
test_notifier.NOTIFICATIONS = []
@ -205,6 +214,37 @@ class QoSSpecManageApiTest(test.TestCase):
req, '666')
self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
def test_qos_specs_delete_keys(self):
self.stubs.Set(qos_specs, 'delete_keys',
return_qos_specs_delete_keys)
body = {"keys": ['bar', 'zoo']}
req = fakes.HTTPRequest.blank('/v2/fake/qos-specs/666/delete_keys')
self.assertEqual(len(test_notifier.NOTIFICATIONS), 0)
self.controller.delete_keys(req, '666', body)
self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
def test_qos_specs_delete_keys_qos_notfound(self):
self.stubs.Set(qos_specs, 'delete_keys',
return_qos_specs_delete_keys)
body = {"keys": ['bar', 'zoo']}
req = fakes.HTTPRequest.blank('/v2/fake/qos-specs/777/delete_keys')
self.assertEqual(len(test_notifier.NOTIFICATIONS), 0)
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.delete_keys,
req, '777', body)
self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
def test_qos_specs_delete_keys_badkey(self):
self.stubs.Set(qos_specs, 'delete_keys',
return_qos_specs_delete_keys)
req = fakes.HTTPRequest.blank('/v2/fake/qos-specs/666/delete_keys')
body = {"keys": ['foo', 'zoo']}
self.assertEqual(len(test_notifier.NOTIFICATIONS), 0)
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.delete_keys,
req, '666', body)
self.assertEqual(len(test_notifier.NOTIFICATIONS), 1)
def test_create(self):
self.stubs.Set(qos_specs, 'create',
return_qos_specs_create)

16
cinder/tests/db/test_qos_specs.py
View File

@ -136,6 +136,22 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase):
self.assertRaises(exception.QoSSpecsNotFound, db.qos_specs_get,
self.ctxt, specs_id)
def test_qos_specs_item_delete(self):
name = str(int(time.time()))
value = dict(consumer='front-end',
foo='Foo', bar='Bar')
specs_id = self._create_qos_specs(name, value)
del value['consumer']
del value['foo']
expected = {'name': name,
'id': specs_id,
'consumer': 'front-end',
'specs': value}
db.qos_specs_item_delete(self.ctxt, specs_id, 'foo')
specs = db.qos_specs_get_by_name(self.ctxt, name)
self.assertDictMatch(specs, expected)
def test_associate_type_with_qos(self):
self.assertRaises(exception.VolumeTypeNotFound,
db.volume_type_qos_associate,

49
cinder/tests/test_qos_specs.py
View File

@ -145,6 +145,47 @@ class QoSSpecsTestCase(test.TestCase):
# able to delete in-use qos specs if force=True
qos_specs.delete(self.ctxt, 'InUse', force=True)
def test_delete_keys(self):
def fake_db_qos_delete_key(context, id, key):
if key == 'NotFound':
raise exception.QoSSpecsKeyNotFound(specs_id=id,
specs_key=key)
else:
pass
def fake_qos_specs_get(context, id):
if id == 'NotFound':
raise exception.QoSSpecsNotFound(specs_id=id)
else:
pass
value = dict(consumer='front-end',
foo='Foo', bar='Bar', zoo='tiger')
specs_id = self._create_qos_specs('QoSName', value)
qos_specs.delete_keys(self.ctxt, specs_id, ['foo', 'bar'])
del value['consumer']
del value['foo']
del value['bar']
expected = {'name': 'QoSName',
'id': specs_id,
'consumer': 'front-end',
'specs': value}
specs = qos_specs.get_qos_specs(self.ctxt, specs_id)
self.assertDictMatch(expected, specs)
self.stubs.Set(qos_specs, 'get_qos_specs', fake_qos_specs_get)
self.stubs.Set(db, 'qos_specs_item_delete', fake_db_qos_delete_key)
self.assertRaises(exception.InvalidQoSSpecs,
qos_specs.delete_keys, self.ctxt, None, [])
self.assertRaises(exception.QoSSpecsNotFound,
qos_specs.delete_keys, self.ctxt, 'NotFound', [])
self.assertRaises(exception.QoSSpecsKeyNotFound,
qos_specs.delete_keys, self.ctxt,
'Found', ['NotFound'])
self.assertRaises(exception.QoSSpecsKeyNotFound,
qos_specs.delete_keys, self.ctxt, 'Found',
['foo', 'bar', 'NotFound'])
def test_get_associations(self):
def fake_db_associate_get(context, id):
if id == 'Trouble':
@ -259,6 +300,12 @@ class QoSSpecsTestCase(test.TestCase):
raise db_exc.DBError()
pass
def fake_qos_specs_get(context, id):
if id == 'NotFound':
raise exception.QoSSpecsNotFound(specs_id=id)
else:
pass
type1_ref = volume_types.create(self.ctxt, 'TypeName1')
type2_ref = volume_types.create(self.ctxt, 'TypeName2')
specs_id = self._create_qos_specs('QoSName')
@ -276,6 +323,8 @@ class QoSSpecsTestCase(test.TestCase):
self.stubs.Set(db, 'qos_specs_disassociate_all',
fake_db_disassociate_all)
self.stubs.Set(qos_specs, 'get_qos_specs',
fake_qos_specs_get)
self.assertRaises(exception.QoSSpecsDisassociateFailed,
qos_specs.disassociate_all,
self.ctxt, 'Trouble')

33
cinder/volume/qos_specs.py
View File

@ -124,16 +124,28 @@ def delete(context, qos_specs_id, force=False):
if qos_specs_id is None:
msg = _("id cannot be None")
raise exception.InvalidQoSSpecs(reason=msg)
else:
# check if there is any entity associated with this
# qos specs.
res = db.qos_specs_associations_get(context, qos_specs_id)
if res and not force:
raise exception.QoSSpecsInUse(specs_id=qos_specs_id)
elif force:
# remove all association
disassociate_all(context, qos_specs_id)
db.qos_specs_delete(context, qos_specs_id)
# check if there is any entity associated with this qos specs
res = db.qos_specs_associations_get(context, qos_specs_id)
if res and not force:
raise exception.QoSSpecsInUse(specs_id=qos_specs_id)
elif res and force:
# remove all association
db.qos_specs_disassociate_all(context, qos_specs_id)
db.qos_specs_delete(context, qos_specs_id)
def delete_keys(context, qos_specs_id, keys):
"""Marks specified key of target qos specs as deleted."""
if qos_specs_id is None:
msg = _("id cannot be None")
raise exception.InvalidQoSSpecs(reason=msg)
# make sure qos_specs_id is valid
get_qos_specs(context, qos_specs_id)
for key in keys:
db.qos_specs_item_delete(context, qos_specs_id, key)
def get_associations(context, specs_id):
@ -209,6 +221,7 @@ def disassociate_qos_specs(context, specs_id, type_id):
def disassociate_all(context, specs_id):
"""Disassociate qos_specs from all entities."""
try:
get_qos_specs(context, specs_id)
db.qos_specs_disassociate_all(context, specs_id)
except db_exc.DBError as e:
LOG.exception(_('DB error: %s') % e)