diff --git a/cinder/api/contrib/qos_specs_manage.py b/cinder/api/contrib/qos_specs_manage.py index 558418990ca..43aba89a050 100644 --- a/cinder/api/contrib/qos_specs_manage.py +++ b/cinder/api/contrib/qos_specs_manage.py @@ -88,10 +88,10 @@ class QoSSpecsController(wsgi.Controller): self.validate_string_length(name, 'name', min_length=1, max_length=255, remove_whitespaces=True) name = name.strip() - + # Remove name from 'specs' since passing it in as separate param + del specs['name'] try: - qos_specs.create(context, name, specs) - spec = qos_specs.get_qos_specs_by_name(context, name) + spec = qos_specs.create(context, name, specs) notifier_info = dict(name=name, specs=specs) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.create', diff --git a/cinder/api/views/qos_specs.py b/cinder/api/views/qos_specs.py index ad49f8fff22..794228278bf 100644 --- a/cinder/api/views/qos_specs.py +++ b/cinder/api/views/qos_specs.py @@ -31,19 +31,20 @@ class ViewBuilder(common.ViewBuilder): def summary(self, request, qos_spec): """Generic, non-detailed view of a qos_specs.""" - return { - 'qos_specs': qos_spec, - 'links': self._get_links(request, - qos_spec['id']), - } + return self.detail(request, qos_spec) def detail(self, request, qos_spec): """Detailed view of a single qos_spec.""" # TODO(zhiteng) Add associations to detailed view return { - 'qos_specs': qos_spec, + 'qos_specs': { + 'id': qos_spec.id, + 'name': qos_spec.name, + 'consumer': qos_spec.consumer, + 'specs': qos_spec.specs + }, 'links': self._get_links(request, - qos_spec['id']), + qos_spec.id), } def associations(self, request, associates): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 884c6f9b920..41c0b3fdeb7 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2857,6 +2857,11 @@ def volume_types_get_by_name_or_id(context, volume_type_list): @require_admin_context def volume_type_qos_associations_get(context, qos_specs_id, inactive=False): read_deleted = "yes" if inactive else "no" + # Raise QoSSpecsNotFound if no specs found + if not resource_exists(context, + models.QualityOfServiceSpecs, + qos_specs_id): + raise exception.QoSSpecsNotFound(specs_id=qos_specs_id) return model_query(context, models.VolumeTypes, read_deleted=read_deleted). \ filter_by(qos_specs_id=qos_specs_id).all() @@ -3117,15 +3122,14 @@ def qos_specs_create(context, values): :param values dictionary that contains specifications for QoS e.g. {'name': 'Name', - 'qos_specs': { - 'consumer': 'front-end', + 'consumer': 'front-end', + 'specs': { 'total_iops_sec': 1000, 'total_bytes_sec': 1024000 } } """ specs_id = str(uuid.uuid4()) - session = get_session() with session.begin(): try: @@ -3145,8 +3149,18 @@ def qos_specs_create(context, values): specs_root.update(root) specs_root.save(session=session) + # Save 'consumer' value directly as it will not be in + # values['specs'] and so we avoid modifying/copying passed in dict + consumer = {'key': 'consumer', + 'value': values['consumer'], + 'specs_id': specs_id, + 'id': six.text_type(uuid.uuid4())} + cons_entry = models.QualityOfServiceSpecs() + cons_entry.update(consumer) + cons_entry.save(session=session) + # Insert all specification entries for QoS specs - for k, v in values['qos_specs'].items(): + for k, v in values.get('specs', {}).items(): item = dict(key=k, value=v, specs_id=specs_id) item['id'] = str(uuid.uuid4()) spec_entry = models.QualityOfServiceSpecs() @@ -3213,12 +3227,10 @@ def _dict_with_qos_specs(rows): result = [] for row in rows: if row['key'] == 'QoS_Specs_Name': - member = {} - member['name'] = row['value'] - member.update(dict(id=row['id'])) + member = {'name': row['value'], 'id': row['id']} if row.specs: spec_dict = _dict_with_children_specs(row.specs) - member.update(dict(consumer=spec_dict['consumer'])) + member['consumer'] = spec_dict['consumer'] del spec_dict['consumer'] member.update(dict(specs=spec_dict)) result.append(member) @@ -3228,7 +3240,6 @@ def _dict_with_qos_specs(rows): @require_admin_context def qos_specs_get(context, qos_specs_id, inactive=False): rows = _qos_specs_get_ref(context, qos_specs_id, None, inactive) - return _dict_with_qos_specs(rows)[0] @@ -3321,8 +3332,6 @@ def qos_specs_associations_get(context, qos_specs_id): extend qos specs association to other entities, such as volumes, sometime in future. """ - # Raise QoSSpecsNotFound if no specs found - _qos_specs_get_ref(context, qos_specs_id, None) return volume_type_qos_associations_get(context, qos_specs_id) @@ -3355,7 +3364,6 @@ def qos_specs_disassociate_all(context, qos_specs_id): def qos_specs_item_delete(context, qos_specs_id, key): 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). \ @@ -3396,7 +3404,7 @@ def _qos_specs_get_item(context, qos_specs_id, key, session=None): @handle_db_data_error @require_admin_context -def qos_specs_update(context, qos_specs_id, specs): +def qos_specs_update(context, qos_specs_id, updates): """Make updates to an existing qos specs. Perform add, update or delete key/values to a qos specs. @@ -3406,6 +3414,13 @@ def qos_specs_update(context, qos_specs_id, specs): with session.begin(): # make sure qos specs exists _qos_specs_get_ref(context, qos_specs_id, session) + specs = updates.get('specs', {}) + + if 'consumer' in updates: + # Massage consumer to the right place for DB and copy specs + # before updating so we don't modify dict for caller + specs = specs.copy() + specs['consumer'] = updates['consumer'] spec_ref = None for key in specs.keys(): try: @@ -4745,6 +4760,7 @@ def _get_get_method(model): GET_EXCEPTIONS = { models.ConsistencyGroup: consistencygroup_get, models.VolumeTypes: _volume_type_get_full, + models.QualityOfServiceSpecs: qos_specs_get, } if model in GET_EXCEPTIONS: diff --git a/cinder/objects/__init__.py b/cinder/objects/__init__.py index 10367843ae1..4795734d48f 100644 --- a/cinder/objects/__init__.py +++ b/cinder/objects/__init__.py @@ -27,6 +27,7 @@ def register_all(): __import__('cinder.objects.backup') __import__('cinder.objects.cgsnapshot') __import__('cinder.objects.consistencygroup') + __import__('cinder.objects.qos_specs') __import__('cinder.objects.service') __import__('cinder.objects.snapshot') __import__('cinder.objects.volume') diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 76a5de2b2db..7807c367efc 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -100,6 +100,9 @@ OBJ_VERSIONS.add('1.2', {'Backup': '1.4', 'BackupImport': '1.4'}) OBJ_VERSIONS.add('1.3', {'Service': '1.3'}) OBJ_VERSIONS.add('1.4', {'Snapshot': '1.1'}) OBJ_VERSIONS.add('1.5', {'VolumeType': '1.1'}) +OBJ_VERSIONS.add('1.6', {'QualityOfServiceSpecs': '1.0', + 'QualityOfServiceSpecsList': '1.0', + 'VolumeType': '1.2'}) class CinderObjectRegistry(base.VersionedObjectRegistry): diff --git a/cinder/objects/fields.py b/cinder/objects/fields.py index 9f8121f44e2..fade039ff95 100644 --- a/cinder/objects/fields.py +++ b/cinder/objects/fields.py @@ -104,3 +104,19 @@ class SnapshotStatus(Enum): class SnapshotStatusField(BaseEnumField): AUTO_TYPE = SnapshotStatus() + + +class QoSConsumerValues(Enum): + BACK_END = 'back-end' + FRONT_END = 'front-end' + BOTH = 'both' + + ALL = (BACK_END, FRONT_END, BOTH) + + def __init__(self): + super(QoSConsumerValues, self).__init__( + valid_values=QoSConsumerValues.ALL) + + +class QoSConsumerField(BaseEnumField): + AUTO_TYPE = QoSConsumerValues() diff --git a/cinder/objects/qos_specs.py b/cinder/objects/qos_specs.py new file mode 100644 index 00000000000..ee4711dff9a --- /dev/null +++ b/cinder/objects/qos_specs.py @@ -0,0 +1,193 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_db import exception as db_exc +from oslo_log import log as logging + +from cinder import db +from cinder import exception +from cinder.i18n import _, _LE +from cinder import objects +from cinder.objects import base +from cinder.objects import fields as c_fields +from oslo_versionedobjects import fields + +LOG = logging.getLogger(__name__) + + +@base.CinderObjectRegistry.register +class QualityOfServiceSpecs(base.CinderPersistentObject, + base.CinderObject, + base.CinderObjectDictCompat, + base.CinderComparableObject): + # Version + # 1.0: Initial version + VERSION = "1.0" + + OPTIONAL_FIELDS = ['volume_types'] + + fields = { + 'id': fields.UUIDField(), + 'name': fields.StringField(), + 'consumer': c_fields.QoSConsumerField( + default=c_fields.QoSConsumerValues.BACK_END), + 'specs': fields.DictOfNullableStringsField(nullable=True), + 'volume_types': fields.ObjectField('VolumeTypeList', nullable=True), + } + + def __init__(self, *args, **kwargs): + super(QualityOfServiceSpecs, self).__init__(*args, **kwargs) + self._init_specs = {} + + def __setattr__(self, name, value): + try: + super(QualityOfServiceSpecs, self).__setattr__(name, value) + except ValueError: + if name == 'consumer': + # Give more descriptive error message for invalid 'consumer' + msg = (_("Valid consumer of QoS specs are: %s") % + c_fields.QoSConsumerField()) + raise exception.InvalidQoSSpecs(reason=msg) + else: + raise + + def obj_reset_changes(self, fields=None, recursive=False): + super(QualityOfServiceSpecs, self).obj_reset_changes(fields, recursive) + if fields is None or 'specs' in fields: + self._init_specs = self.specs.copy() if self.specs else {} + + def obj_what_changed(self): + changes = super(QualityOfServiceSpecs, self).obj_what_changed() + + # Do comparison of what's in the dict vs. reference to the specs object + if self.obj_attr_is_set('id'): + if self.specs != self._init_specs: + changes.add('specs') + else: + # If both dicts are equal don't consider anything gets changed + if 'specs' in changes: + changes.remove('specs') + + return changes + + def obj_get_changes(self): + changes = super(QualityOfServiceSpecs, self).obj_get_changes() + if 'specs' in changes: + # For specs, we only want what has changed in the dictionary, + # because otherwise we'll individually overwrite the DB value for + # every key in 'specs' even if it hasn't changed + specs_changes = {} + for key, val in self.specs.items(): + if val != self._init_specs.get(key): + specs_changes[key] = val + changes['specs'] = specs_changes + + specs_keys_removed = (set(self._init_specs.keys()) - + set(self.specs.keys())) + if specs_keys_removed: + # Special key notifying which specs keys have been deleted + changes['specs_keys_removed'] = specs_keys_removed + + return changes + + def obj_load_attr(self, attrname): + if attrname not in QualityOfServiceSpecs.OPTIONAL_FIELDS: + raise exception.ObjectActionError( + action='obj_load_attr', + reason=_('attribute %s not lazy-loadable') % attrname) + if not self._context: + raise exception.OrphanedObjectError(method='obj_load_attr', + objtype=self.obj_name()) + + if attrname == 'volume_types': + self.volume_types = objects.VolumeTypeList.get_all_types_for_qos( + self._context, self.id) + + @staticmethod + def _from_db_object(context, qos_spec, db_qos_spec): + for name, field in qos_spec.fields.items(): + if name not in QualityOfServiceSpecs.OPTIONAL_FIELDS: + value = db_qos_spec.get(name) + # 'specs' could be null if only a consumer is given, so make + # it an empty dict instead of None + if not value and isinstance(field, fields.DictOfStringsField): + value = {} + setattr(qos_spec, name, value) + + qos_spec._context = context + qos_spec.obj_reset_changes() + return qos_spec + + def create(self): + if self.obj_attr_is_set('id'): + raise exception.ObjectActionError(action='create', + reason='already created') + updates = self.cinder_obj_get_changes() + + try: + create_ret = db.qos_specs_create(self._context, updates) + except db_exc.DBDataError: + msg = _('Error writing field to database') + LOG.exception(msg) + raise exception.Invalid(msg) + except db_exc.DBError: + LOG.exception(_LE('DB error occured when creating QoS specs.')) + raise exception.QoSSpecsCreateFailed(name=self.name, + qos_specs=self.specs) + # Save ID with the object + updates['id'] = create_ret['id'] + self._from_db_object(self._context, self, updates) + + def save(self): + updates = self.cinder_obj_get_changes() + if updates: + if 'specs_keys_removed' in updates.keys(): + for specs_key_to_remove in updates['specs_keys_removed']: + db.qos_specs_item_delete( + self._context, self.id, specs_key_to_remove) + del updates['specs_keys_removed'] + db.qos_specs_update(self._context, self.id, updates) + + self.obj_reset_changes() + + def destroy(self, force=False): + """Deletes the QoS spec. + + :param force: when force is True, all volume_type mappings for this QoS + are deleted. When force is False and volume_type + mappings still exist, a QoSSpecsInUse exception is thrown + """ + if self.volume_types: + if not force: + raise exception.QoSSpecsInUse(specs_id=self.id) + else: + # remove all association + db.qos_specs_disassociate_all(self._context, self.id) + db.qos_specs_delete(self._context, self.id) + + +@base.CinderObjectRegistry.register +class QualityOfServiceSpecsList(base.ObjectListBase, base.CinderObject): + VERSION = '1.0' + + fields = { + 'objects': fields.ListOfObjectsField('QualityOfServiceSpecs'), + } + child_versions = { + '1.0': '1.0', + } + + @classmethod + def get_all(cls, context, *args, **kwargs): + specs = db.qos_specs_get_all(context, *args, **kwargs) + return base.obj_make_list(context, cls(context), + objects.QualityOfServiceSpecs, specs) diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index 7536ba1c845..d9828f81ec7 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -15,6 +15,7 @@ from oslo_utils import versionutils from oslo_versionedobjects import fields +from cinder import db from cinder import exception from cinder.i18n import _ from cinder import objects @@ -22,7 +23,7 @@ from cinder.objects import base from cinder.volume import volume_types -OPTIONAL_FIELDS = ['extra_specs', 'projects'] +OPTIONAL_FIELDS = ['extra_specs', 'projects', 'qos_specs'] @base.CinderObjectRegistry.register @@ -30,7 +31,8 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, base.CinderObjectDictCompat, base.CinderComparableObject): # Version 1.0: Initial version # Version 1.1: Changed extra_specs to DictOfNullableStringsField - VERSION = '1.1' + # Version 1.2: Added qos_specs + VERSION = '1.2' fields = { 'id': fields.UUIDField(), @@ -39,6 +41,8 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, 'is_public': fields.BooleanField(default=True, nullable=True), 'projects': fields.ListOfStringsField(nullable=True), 'extra_specs': fields.DictOfNullableStringsField(nullable=True), + 'qos_specs': fields.ObjectField('QualityOfServiceSpecs', + nullable=True), } def obj_make_compatible(self, primitive, target_version): @@ -82,7 +86,12 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, type.extra_specs = specs if 'projects' in expected_attrs: type.projects = db_type.get('projects', []) - + if 'qos_specs' in expected_attrs: + qos_specs = objects.QualityOfServiceSpecs(context) + qos_specs._from_db_object(context, + qos_specs, + type['qos_specs']) + type.qos_specs = qos_specs type._context = context type.obj_reset_changes() return type @@ -130,3 +139,9 @@ class VolumeTypeList(base.ObjectListBase, base.CinderObject): return base.obj_make_list(context, cls(context), objects.VolumeType, types.values(), expected_attrs=expected_attrs) + + @classmethod + def get_all_types_for_qos(self, context, qos_id): + types = db.qos_specs_associations_get(context, qos_id) + return base.obj_make_list(context, self(context), objects.VolumeType, + types) diff --git a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py index 1b8cd12b3ab..fab818b3a9a 100644 --- a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py +++ b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py @@ -21,6 +21,7 @@ from cinder.api.contrib import qos_specs_manage from cinder import context from cinder import db from cinder import exception +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake @@ -37,7 +38,7 @@ def stub_qos_specs(id): "key4": "value4", "key5": "value5"} res.update(dict(specs=specs)) - return res + return objects.QualityOfServiceSpecs(**res) def stub_qos_associates(id): @@ -97,14 +98,11 @@ def return_qos_specs_create(context, name, specs): raise exception.QoSSpecsCreateFailed(name=id, qos_specs=specs) elif name == 'qos_spec_%s' % fake.INVALID_ID: raise exception.InvalidQoSSpecs(reason=name) - pass - -def return_qos_specs_get_by_name(context, name): - if name == 'qos_spec_%s' % fake.WILL_NOT_BE_FOUND_ID: - raise exception.QoSSpecsNotFound(specs_id=name) - - return stub_qos_specs(name.split("_")[2]) + return objects.QualityOfServiceSpecs(name=name, + specs=specs, + consumer='back-end', + id=fake.QOS_SPEC_ID) def return_get_qos_associations(context, id): @@ -149,8 +147,8 @@ class QoSSpecManageApiTest(test.TestCase): specs = dict(name=name, qos_specs=values) else: specs = {'name': name, - 'qos_specs': { - 'consumer': 'back-end', + 'consumer': 'back-end', + 'specs': { 'key1': 'value1', 'key2': 'value2'}} return db.qos_specs_create(self.ctxt, specs)['id'] @@ -389,11 +387,8 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.create', side_effect=return_qos_specs_create) - @mock.patch('cinder.volume.qos_specs.get_qos_specs_by_name', - side_effect=return_qos_specs_get_by_name) @mock.patch('cinder.api.openstack.wsgi.Controller.validate_string_length') - def test_create(self, mock_validate, mock_qos_get_specs, - mock_qos_spec_create): + def test_create(self, mock_validate, mock_qos_spec_create): body = {"qos_specs": {"name": "qos_specs_%s" % fake.QOS_SPEC_ID, "key1": "value1"}} @@ -423,9 +418,7 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.create', side_effect=return_qos_specs_create) - @mock.patch('cinder.volume.qos_specs.get_qos_specs_by_name', - side_effect=return_qos_specs_get_by_name) - def test_create_conflict(self, mock_qos_get_specs, mock_qos_spec_create): + def test_create_conflict(self, mock_qos_spec_create): body = {"qos_specs": {"name": 'qos_spec_%s' % fake.ALREADY_EXISTS_ID, "key1": "value1"}} req = fakes.HTTPRequest.blank('/v2/%s/qos-specs' % fake.PROJECT_ID) @@ -438,9 +431,7 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.create', side_effect=return_qos_specs_create) - @mock.patch('cinder.volume.qos_specs.get_qos_specs_by_name', - side_effect=return_qos_specs_get_by_name) - def test_create_failed(self, mock_qos_get_specs, mock_qos_spec_create): + def test_create_failed(self, mock_qos_spec_create): body = {"qos_specs": {"name": 'qos_spec_%s' % fake.ACTION_FAILED_ID, "key1": "value1"}} req = fakes.HTTPRequest.blank('/v2/%s/qos-specs' % fake.PROJECT_ID) diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index b78d478774f..ffd9a0450a6 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -573,13 +573,13 @@ class VolumeRetypeActionsTest(test.TestCase): self.retype_mocks['reserve'].side_effect = exc self._retype_volume_exec(413, vol_type_new.id, vol.id) - def _retype_volume_qos(self, vol_status, consumer, expected_status, + def _retype_volume_qos(self, vol_status, consumer_pass, expected_status, same_qos=False, has_qos=True, has_type=True): admin_ctxt = context.get_admin_context() if has_qos: qos_old = utils.create_qos(admin_ctxt, self, name='old', - qos_specs={'consumer': consumer})['id'] + consumer=consumer_pass)['id'] else: qos_old = None @@ -588,7 +588,7 @@ class VolumeRetypeActionsTest(test.TestCase): else: qos_new = utils.create_qos(admin_ctxt, self, name='new', - qos_specs={'consumer': consumer})['id'] + consumer=consumer_pass)['id'] if has_type: vol_type_old = utils.create_volume_type(admin_ctxt, self, diff --git a/cinder/tests/unit/db/test_qos_specs.py b/cinder/tests/unit/db/test_qos_specs.py index 48ab1b8f9f2..79596df9ce0 100644 --- a/cinder/tests/unit/db/test_qos_specs.py +++ b/cinder/tests/unit/db/test_qos_specs.py @@ -40,16 +40,14 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): project_id=fake.PROJECT_ID, is_admin=True) - def _create_qos_specs(self, name, values=None): + def _create_qos_specs(self, name, consumer='back-end', values=None): """Create a transfer object.""" - if values: - specs = dict(name=name, qos_specs=values) - else: - specs = {'name': name, - 'qos_specs': { - 'consumer': 'back-end', - 'key1': 'value1', - 'key2': 'value2'}} + if values is None: + values = {'key1': 'value1', 'key2': 'value2'} + + specs = {'name': name, + 'consumer': consumer, + 'specs': values} return db.qos_specs_create(self.ctxt, specs)['id'] def test_qos_specs_create(self): @@ -66,84 +64,66 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): self.assertEqual(specs_id, query_id) def test_qos_specs_get(self): - value = dict(consumer='front-end', - key1='foo', key2='bar') - specs_id = self._create_qos_specs('Name1', value) + qos_spec = {'name': 'Name1', + 'consumer': 'front-end', + 'specs': {'key1': 'foo', 'key2': 'bar'}} + specs_id = self._create_qos_specs(qos_spec['name'], + qos_spec['consumer'], + qos_spec['specs']) fake_id = fake.WILL_NOT_BE_FOUND_ID self.assertRaises(exception.QoSSpecsNotFound, db.qos_specs_get, self.ctxt, fake_id) - specs = db.qos_specs_get(self.ctxt, specs_id) - expected = dict(name='Name1', id=specs_id, consumer='front-end') - del value['consumer'] - expected.update(dict(specs=value)) - self.assertDictMatch(expected, specs) + specs_returned = db.qos_specs_get(self.ctxt, specs_id) + qos_spec['id'] = specs_id + self.assertDictMatch(qos_spec, specs_returned) def test_qos_specs_get_all(self): - value1 = dict(consumer='front-end', - key1='v1', key2='v2') - value2 = dict(consumer='back-end', - key3='v3', key4='v4') - value3 = dict(consumer='back-end', - key5='v5', key6='v6') + qos_list = [ + {'name': 'Name1', + 'consumer': 'front-end', + 'specs': {'key1': 'v1', 'key2': 'v2'}}, + {'name': 'Name2', + 'consumer': 'back-end', + 'specs': {'key1': 'v3', 'key2': 'v4'}}, + {'name': 'Name3', + 'consumer': 'back-end', + 'specs': {'key1': 'v5', 'key2': 'v6'}}] - spec_id1 = self._create_qos_specs('Name1', value1) - spec_id2 = self._create_qos_specs('Name2', value2) - spec_id3 = self._create_qos_specs('Name3', value3) + for qos in qos_list: + qos['id'] = self._create_qos_specs(qos['name'], + qos['consumer'], + qos['specs']) - specs = db.qos_specs_get_all(self.ctxt) - self.assertEqual(3, len(specs), + specs_list_returned = db.qos_specs_get_all(self.ctxt) + self.assertEqual(len(qos_list), len(specs_list_returned), "Unexpected number of qos specs records") - expected1 = dict(name='Name1', id=spec_id1, consumer='front-end') - expected2 = dict(name='Name2', id=spec_id2, consumer='back-end') - expected3 = dict(name='Name3', id=spec_id3, consumer='back-end') - del value1['consumer'] - del value2['consumer'] - del value3['consumer'] - expected1.update(dict(specs=value1)) - expected2.update(dict(specs=value2)) - expected3.update(dict(specs=value3)) - self.assertIn(expected1, specs) - self.assertIn(expected2, specs) - self.assertIn(expected3, specs) - - def test_qos_specs_get_by_name(self): - name = str(int(time.time())) - value = dict(consumer='front-end', - foo='Foo', bar='Bar') - specs_id = self._create_qos_specs(name, value) - specs = db.qos_specs_get_by_name(self.ctxt, name) - del value['consumer'] - expected = {'name': name, - 'id': specs_id, - 'consumer': 'front-end', - 'specs': value} - self.assertDictMatch(expected, specs) + for expected_qos in qos_list: + self.assertIn(expected_qos, specs_list_returned) def test_qos_specs_delete(self): name = str(int(time.time())) specs_id = self._create_qos_specs(name) db.qos_specs_delete(self.ctxt, specs_id) - self.assertRaises(exception.QoSSpecsNotFound, db.qos_specs_get, + 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) + value = dict(foo='Foo', bar='Bar') + specs_id = self._create_qos_specs(name, 'front-end', 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) + specs = db.qos_specs_get(self.ctxt, specs_id) self.assertDictMatch(expected, specs) def test_associate_type_with_qos(self): @@ -214,7 +194,8 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): def test_qos_specs_update(self): name = 'FakeName' specs_id = self._create_qos_specs(name) - value = dict(key2='new_value2', key3='value3') + value = {'consumer': 'both', + 'specs': {'key2': 'new_value2', 'key3': 'value3'}} self.assertRaises(exception.QoSSpecsNotFound, db.qos_specs_update, self.ctxt, fake.WILL_NOT_BE_FOUND_ID, value) @@ -222,3 +203,4 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): specs = db.qos_specs_get(self.ctxt, specs_id) self.assertEqual('new_value2', specs['specs']['key2']) self.assertEqual('value3', specs['specs']['key3']) + self.assertEqual('both', specs['consumer']) diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 2272176a048..a44f62e23ef 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -30,6 +30,8 @@ object_data = { 'CGSnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'ConsistencyGroup': '1.2-ff7638e03ae7a3bb7a43a6c5c4d0c94a', 'ConsistencyGroupList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'QualityOfServiceSpecs': '1.0-0b212e0a86ee99092229874e03207fe8', + 'QualityOfServiceSpecsList': '1.0-1b54e51ad0fc1f3a8878f5010e7e16dc', 'Service': '1.3-d7c1e133791c9d766596a0528fc9a12f', 'ServiceList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'Snapshot': '1.1-37966f7141646eb29e9ad5298ff2ca8a', @@ -38,7 +40,7 @@ object_data = { 'VolumeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'VolumeAttachment': '1.0-b30dacf62b2030dd83d8a1603f1064ff', 'VolumeAttachmentList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', - 'VolumeType': '1.1-6673dd9ce7c27e9c85279afb20833877', + 'VolumeType': '1.2-02ecb0baac87528d041f4ddd95b95579', 'VolumeTypeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', } diff --git a/cinder/tests/unit/objects/test_qos.py b/cinder/tests/unit/objects/test_qos.py new file mode 100644 index 00000000000..eba7e399a3b --- /dev/null +++ b/cinder/tests/unit/objects/test_qos.py @@ -0,0 +1,116 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock + +from cinder.db.sqlalchemy import models +from cinder import exception +from cinder import objects +from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import objects as test_objects + +fake_qos = {'consumer': 'front-end', + 'id': fake.OBJECT_ID, + 'name': 'qos_name', + 'specs': {'key1': 'val1', 'key2': 'val2'}} + +fake_qos_no_id = fake_qos.copy() +del fake_qos_no_id['id'] + + +class TestQos(test_objects.BaseObjectsTestCase): + @mock.patch('cinder.db.get_by_id', return_value=fake_qos) + def test_get_by_id(self, qos_get): + qos_object = objects.QualityOfServiceSpecs.get_by_id( + self.context, fake.OBJECT_ID) + self._compare(self, fake_qos, qos_object) + qos_get.assert_called_once_with( + self.context, models.QualityOfServiceSpecs, fake.OBJECT_ID) + + @mock.patch('cinder.db.qos_specs_create', + return_value={'name': 'qos_name', 'id': fake.OBJECT_ID}) + def test_create(self, qos_fake_create): + qos_object = objects.QualityOfServiceSpecs( + self.context, **fake_qos_no_id) + qos_object.create() + self._compare(self, fake_qos, qos_object) + + # Fail to create a second time + self.assertRaises(exception.ObjectActionError, qos_object.create) + + self.assertEqual(1, len(qos_fake_create.mock_calls)) + + @mock.patch('cinder.db.qos_specs_item_delete') + @mock.patch('cinder.db.qos_specs_update') + def test_save(self, qos_fake_update, qos_fake_delete): + qos_dict = fake_qos.copy() + qos_dict['specs']['key_to_remove1'] = 'val' + qos_dict['specs']['key_to_remove2'] = 'val' + qos_object = objects.QualityOfServiceSpecs._from_db_object( + self.context, objects.QualityOfServiceSpecs(), qos_dict) + + qos_object.specs['key1'] = 'val1' + qos_object.save() + # No values have changed so no updates should be made + self.assertFalse(qos_fake_update.called) + + qos_object.consumer = 'back-end' + qos_object.specs['key1'] = 'val2' + qos_object.specs['new_key'] = 'val3' + + del qos_object.specs['key_to_remove1'] + del qos_object.specs['key_to_remove2'] + qos_object.save() + qos_fake_update.assert_called_once_with( + self.context, fake.OBJECT_ID, + {'specs': {'key1': 'val2', 'new_key': 'val3'}, + 'consumer': 'back-end'}) + qos_fake_delete.assert_has_calls([ + mock.call(self.context, fake.OBJECT_ID, 'key_to_remove1'), + mock.call(self.context, fake.OBJECT_ID, 'key_to_remove2')]) + + @mock.patch('cinder.objects.VolumeTypeList.get_all_types_for_qos', + return_value=None) + @mock.patch('cinder.db.qos_specs_delete') + def test_destroy_no_vol_types(self, qos_fake_delete, fake_get_vol_types): + qos_object = objects.QualityOfServiceSpecs._from_db_object( + self.context, objects.QualityOfServiceSpecs(), fake_qos) + qos_object.destroy() + + qos_fake_delete.assert_called_once_with(self.context, fake_qos['id']) + + @mock.patch('cinder.db.qos_specs_delete') + @mock.patch('cinder.db.qos_specs_disassociate_all') + @mock.patch('cinder.objects.VolumeTypeList.get_all_types_for_qos') + def test_destroy_with_vol_types(self, fake_get_vol_types, + qos_fake_disassociate, qos_fake_delete): + qos_object = objects.QualityOfServiceSpecs._from_db_object( + self.context, objects.QualityOfServiceSpecs(), fake_qos) + fake_get_vol_types.return_value = objects.VolumeTypeList( + objects=[objects.VolumeType(id=fake.VOLUME_TYPE_ID)]) + self.assertRaises(exception.QoSSpecsInUse, qos_object.destroy) + + qos_object.destroy(force=True) + qos_fake_delete.assert_called_once_with(self.context, fake_qos['id']) + qos_fake_disassociate.assert_called_once_with( + self.context, fake_qos['id']) + + @mock.patch('cinder.objects.VolumeTypeList.get_all_types_for_qos', + return_value=None) + @mock.patch('cinder.db.get_by_id', return_value=fake_qos) + def test_get_volume_type(self, fake_get_by_id, fake_get_vol_types): + qos_object = objects.QualityOfServiceSpecs.get_by_id( + self.context, fake.OBJECT_ID) + self.assertFalse(fake_get_vol_types.called) + # Access lazy-loadable attribute + qos_object.volume_types + self.assertTrue(fake_get_vol_types.called) diff --git a/cinder/tests/unit/test_qos_specs.py b/cinder/tests/unit/test_qos_specs.py index ef4048dcac5..81835e381fb 100644 --- a/cinder/tests/unit/test_qos_specs.py +++ b/cinder/tests/unit/test_qos_specs.py @@ -17,6 +17,8 @@ Unit Tests for qos specs internal API """ +import mock +import six import time from oslo_db import exception as db_exc @@ -25,6 +27,7 @@ from cinder import context from cinder import db from cinder import exception from cinder import test +from cinder.tests.unit import fake_constants as fake from cinder.volume import qos_specs from cinder.volume import volume_types @@ -38,22 +41,33 @@ def fake_db_qos_specs_create(context, values): pass +def fake_db_get_vol_type(vol_type_number=1): + return {'name': 'type-' + six.text_type(vol_type_number), + 'id': fake.QOS_SPEC_ID, + 'updated_at': None, + 'created_at': None, + 'deleted_at': None, + 'description': 'desc', + 'deleted': False, + 'is_public': True, + 'projects': None, + 'extra_specs': None} + + class QoSSpecsTestCase(test.TestCase): """Test cases for qos specs code.""" def setUp(self): super(QoSSpecsTestCase, self).setUp() self.ctxt = context.get_admin_context() - def _create_qos_specs(self, name, values=None): + def _create_qos_specs(self, name, consumer='back-end', values=None): """Create a transfer object.""" - if values: - specs = dict(name=name, qos_specs=values) - else: - specs = {'name': name, - 'qos_specs': { - 'consumer': 'back-end', - 'key1': 'value1', - 'key2': 'value2'}} + if values is None: + values = {'key1': 'value1', 'key2': 'value2'} + + specs = {'name': name, + 'consumer': consumer, + 'specs': values} return db.qos_specs_create(self.ctxt, specs)['id'] def test_create(self): @@ -61,27 +75,31 @@ class QoSSpecsTestCase(test.TestCase): 'key2': 'value2', 'key3': 'value3'} ref = qos_specs.create(self.ctxt, 'FakeName', input) - specs = qos_specs.get_qos_specs(self.ctxt, ref['id']) - expected = (dict(consumer='back-end')) - expected.update(dict(id=ref['id'])) - expected.update(dict(name='FakeName')) - del input['consumer'] - expected.update(dict(specs=input)) - self.assertDictMatch(expected, specs) - - self.stubs.Set(db, 'qos_specs_create', - fake_db_qos_specs_create) + specs_obj = qos_specs.get_qos_specs(self.ctxt, ref['id']) + specs_obj_dic = {'consumer': specs_obj['consumer'], + 'id': specs_obj['id'], + 'name': specs_obj['name'], + 'specs': specs_obj['specs']} + expected = {'consumer': 'back-end', + 'id': ref['id'], + 'name': 'FakeName', + 'specs': input} + self.assertDictMatch(expected, + specs_obj_dic) # qos specs must have unique name self.assertRaises(exception.QoSSpecsExists, - qos_specs.create, self.ctxt, 'DupQoSName', input) + qos_specs.create, self.ctxt, 'FakeName', input) - input.update({'consumer': 'FakeConsumer'}) # consumer must be one of: front-end, back-end, both + input['consumer'] = 'fake' self.assertRaises(exception.InvalidQoSSpecs, qos_specs.create, self.ctxt, 'QoSName', input) del input['consumer'] + + self.stubs.Set(db, 'qos_specs_create', + fake_db_qos_specs_create) # able to catch DBError self.assertRaises(exception.QoSSpecsCreateFailed, qos_specs.create, self.ctxt, 'FailQoSName', input) @@ -90,39 +108,45 @@ class QoSSpecsTestCase(test.TestCase): def fake_db_update(context, specs_id, values): raise db_exc.DBError() - input = {'key1': 'value1', - 'consumer': 'WrongPlace'} - # consumer must be one of: front-end, back-end, both - self.assertRaises(exception.InvalidQoSSpecs, - qos_specs.update, self.ctxt, 'fake_id', input) + qos = {'consumer': 'back-end', + 'specs': {'key1': 'value1'}} - input['consumer'] = 'front-end' # qos specs must exists self.assertRaises(exception.QoSSpecsNotFound, - qos_specs.update, self.ctxt, 'fake_id', input) + qos_specs.update, self.ctxt, 'fake_id', qos) + + specs_id = self._create_qos_specs('Name', + qos['consumer'], + qos['specs']) - specs_id = self._create_qos_specs('Name', input) qos_specs.update(self.ctxt, specs_id, - {'key1': 'newvalue1', - 'key2': 'value2'}) + {'key1': 'newvalue1', 'key2': 'value2'}) + specs = qos_specs.get_qos_specs(self.ctxt, specs_id) self.assertEqual('newvalue1', specs['specs']['key1']) self.assertEqual('value2', specs['specs']['key2']) + # consumer must be one of: front-end, back-end, both + self.assertRaises(exception.InvalidQoSSpecs, + qos_specs.update, self.ctxt, specs_id, + {'consumer': 'not-real'}) + self.stubs.Set(db, 'qos_specs_update', fake_db_update) self.assertRaises(exception.QoSSpecsUpdateFailed, - qos_specs.update, self.ctxt, 'fake_id', input) + qos_specs.update, self.ctxt, specs_id, {'key': + 'new_key'}) def test_delete(self): + qos_id = self._create_qos_specs('my_qos') + def fake_db_associations_get(context, id): - if id == 'InUse': - return True - else: - return False + vol_types = [] + if id == qos_id: + vol_types = [fake_db_get_vol_type(id)] + return vol_types def fake_db_delete(context, id): - if id == 'NotFound': - raise exception.QoSSpecsNotFound(specs_id=id) + pass def fake_disassociate_all(context, id): pass @@ -137,9 +161,13 @@ class QoSSpecsTestCase(test.TestCase): self.assertRaises(exception.QoSSpecsNotFound, qos_specs.delete, self.ctxt, 'NotFound') self.assertRaises(exception.QoSSpecsInUse, - qos_specs.delete, self.ctxt, 'InUse') + qos_specs.delete, self.ctxt, qos_id) # able to delete in-use qos specs if force=True - qos_specs.delete(self.ctxt, 'InUse', force=True) + qos_specs.delete(self.ctxt, qos_id, force=True) + + # Can delete without forcing when no volume types + qos_id_with_no_vol_types = self._create_qos_specs('no_vol_types') + qos_specs.delete(self.ctxt, qos_id_with_no_vol_types, force=False) def test_delete_keys(self): def fake_db_qos_delete_key(context, id, key): @@ -155,21 +183,25 @@ class QoSSpecsTestCase(test.TestCase): else: pass - value = dict(consumer='front-end', - foo='Foo', bar='Bar', zoo='tiger') - specs_id = self._create_qos_specs('QoSName', value) + value = {'foo': 'Foo', 'bar': 'Bar', 'zoo': 'tiger'} + name = 'QoSName' + consumer = 'front-end' + specs_id = self._create_qos_specs(name, consumer, value) qos_specs.delete_keys(self.ctxt, specs_id, ['foo', 'bar']) - del value['consumer'] + del value['foo'] del value['bar'] - expected = {'name': 'QoSName', + expected = {'name': name, 'id': specs_id, - 'consumer': 'front-end', + 'consumer': consumer, 'specs': value} specs = qos_specs.get_qos_specs(self.ctxt, specs_id) - self.assertDictMatch(expected, specs) + specs_dic = {'consumer': specs['consumer'], + 'id': specs['id'], + 'name': specs['name'], + 'specs': specs['specs']} + self.assertDictMatch(expected, specs_dic) - 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, []) @@ -177,29 +209,25 @@ class QoSSpecsTestCase(test.TestCase): qos_specs.delete_keys, self.ctxt, 'NotFound', []) self.assertRaises(exception.QoSSpecsKeyNotFound, qos_specs.delete_keys, self.ctxt, - 'Found', ['NotFound']) + specs_id, ['NotFound']) self.assertRaises(exception.QoSSpecsKeyNotFound, - qos_specs.delete_keys, self.ctxt, 'Found', + qos_specs.delete_keys, self.ctxt, specs_id, ['foo', 'bar', 'NotFound']) - def test_get_associations(self): - def fake_db_associate_get(context, id): - if id == 'Trouble': - raise db_exc.DBError() - return [{'name': 'type-1', 'id': 'id-1'}, - {'name': 'type-2', 'id': 'id-2'}] + @mock.patch.object(db, 'qos_specs_associations_get') + def test_get_associations(self, mock_qos_specs_associations_get): + vol_types = [fake_db_get_vol_type(x) for x in range(2)] - self.stubs.Set(db, 'qos_specs_associations_get', - fake_db_associate_get) - expected1 = {'association_type': 'volume_type', - 'name': 'type-1', - 'id': 'id-1'} - expected2 = {'association_type': 'volume_type', - 'name': 'type-2', - 'id': 'id-2'} - res = qos_specs.get_associations(self.ctxt, 'specs-id') - self.assertIn(expected1, res) - self.assertIn(expected2, res) + mock_qos_specs_associations_get.return_value = vol_types + specs_id = self._create_qos_specs('new_spec') + res = qos_specs.get_associations(self.ctxt, specs_id) + for vol_type in vol_types: + expected_type = { + 'association_type': 'volume_type', + 'id': vol_type['id'], + 'name': vol_type['name'] + } + self.assertIn(expected_type, res) self.assertRaises(exception.CinderException, qos_specs.get_associations, self.ctxt, @@ -254,18 +282,8 @@ class QoSSpecsTestCase(test.TestCase): self.ctxt, 'specs-id', 'Invalid') def test_disassociate_qos_specs(self): - def fake_qos_specs_get(context, id): - if id == 'NotFound': - raise exception.QoSSpecsNotFound(specs_id=id) - else: - pass - def fake_db_disassociate(context, id, type_id): - if id == 'Trouble': - raise db_exc.DBError() - elif type_id == 'NotFound': - raise exception.VolumeTypeNotFound(volume_type_id=type_id) - pass + raise db_exc.DBError() type_ref = volume_types.create(self.ctxt, 'TypeName') specs_id = self._create_qos_specs('QoSName') @@ -279,16 +297,19 @@ class QoSSpecsTestCase(test.TestCase): res = qos_specs.get_associations(self.ctxt, specs_id) self.assertEqual(0, len(res)) - self.stubs.Set(db, 'qos_specs_disassociate', - fake_db_disassociate) - self.stubs.Set(qos_specs, 'get_qos_specs', - fake_qos_specs_get) self.assertRaises(exception.VolumeTypeNotFound, qos_specs.disassociate_qos_specs, - self.ctxt, 'specs-id', 'NotFound') + self.ctxt, specs_id, 'NotFound') + + # Verify we can disassociate specs from volume_type even if they are + # not associated with no error + qos_specs.disassociate_qos_specs(self.ctxt, specs_id, type_ref['id']) + qos_specs.associate_qos_with_type(self.ctxt, specs_id, type_ref['id']) + self.stubs.Set(db, 'qos_specs_disassociate', + fake_db_disassociate) self.assertRaises(exception.QoSSpecsDisassociateFailed, qos_specs.disassociate_qos_specs, - self.ctxt, 'Trouble', 'id') + self.ctxt, specs_id, type_ref['id']) def test_disassociate_all(self): def fake_db_disassociate_all(context, id): @@ -326,57 +347,51 @@ class QoSSpecsTestCase(test.TestCase): self.ctxt, 'Trouble') def test_get_all_specs(self): - input = {'key1': 'value1', - 'key2': 'value2', - 'key3': 'value3', - 'consumer': 'both'} - specs_id1 = self._create_qos_specs('Specs1', input) - input.update({'key4': 'value4'}) - specs_id2 = self._create_qos_specs('Specs2', input) + qos_specs_list = [{'name': 'Specs1', + 'created_at': None, + 'updated_at': None, + 'deleted_at': None, + 'deleted': None, + 'consumer': 'both', + 'specs': {'key1': 'value1', + 'key2': 'value2', + 'key3': 'value3'}}, + {'name': 'Specs2', + 'created_at': None, + 'updated_at': None, + 'deleted_at': None, + 'deleted': None, + 'consumer': 'both', + 'specs': {'key1': 'value1', + 'key2': 'value2', + 'key3': 'value3', + 'key4': 'value4'}}] + + for qos_specs_dict in qos_specs_list: + qos_specs_id = self._create_qos_specs( + qos_specs_dict['name'], + qos_specs_dict['consumer'], + qos_specs_dict['specs']) + qos_specs_dict['id'] = qos_specs_id - expected1 = { - 'id': specs_id1, - 'name': 'Specs1', - 'consumer': 'both', - 'specs': {'key1': 'value1', - 'key2': 'value2', - 'key3': 'value3'}} - expected2 = { - 'id': specs_id2, - 'name': 'Specs2', - 'consumer': 'both', - 'specs': {'key1': 'value1', - 'key2': 'value2', - 'key3': 'value3', - 'key4': 'value4'}} res = qos_specs.get_all_specs(self.ctxt) - self.assertEqual(2, len(res)) - self.assertIn(expected1, res) - self.assertIn(expected2, res) + self.assertEqual(len(qos_specs_list), len(res)) + + qos_res_simple_dict = [] + # Need to make list of dictionaries instead of VOs for assertIn to work + for qos in res: + qos_res_simple_dict.append( + qos.obj_to_primitive()['versioned_object.data']) + for qos_spec in qos_specs_list: + self.assertIn(qos_spec, qos_res_simple_dict) def test_get_qos_specs(self): one_time_value = str(int(time.time())) - input = {'key1': one_time_value, + specs = {'key1': one_time_value, 'key2': 'value2', - 'key3': 'value3', - 'consumer': 'both'} - id = self._create_qos_specs('Specs1', input) - specs = qos_specs.get_qos_specs(self.ctxt, id) + 'key3': 'value3'} + qos_id = self._create_qos_specs('Specs1', 'both', specs) + specs = qos_specs.get_qos_specs(self.ctxt, qos_id) self.assertEqual(one_time_value, specs['specs']['key1']) - self.assertRaises(exception.InvalidQoSSpecs, qos_specs.get_qos_specs, self.ctxt, None) - - def test_get_qos_specs_by_name(self): - one_time_value = str(int(time.time())) - input = {'key1': one_time_value, - 'key2': 'value2', - 'key3': 'value3', - 'consumer': 'back-end'} - self._create_qos_specs(one_time_value, input) - specs = qos_specs.get_qos_specs_by_name(self.ctxt, - one_time_value) - self.assertEqual(one_time_value, specs['specs']['key1']) - - self.assertRaises(exception.InvalidQoSSpecs, - qos_specs.get_qos_specs_by_name, self.ctxt, None) diff --git a/cinder/volume/qos_specs.py b/cinder/volume/qos_specs.py index 99f09dcc1d7..85fea67c7e4 100644 --- a/cinder/volume/qos_specs.py +++ b/cinder/volume/qos_specs.py @@ -22,6 +22,7 @@ from oslo_log import log as logging from cinder import context from cinder import db from cinder import exception +from cinder import objects from cinder.i18n import _, _LE, _LW from cinder.volume import volume_types @@ -31,38 +32,6 @@ LOG = logging.getLogger(__name__) CONTROL_LOCATION = ['front-end', 'back-end', 'both'] -def _verify_prepare_qos_specs(specs, create=True): - """Check if 'consumer' value in qos specs is valid. - - Verify 'consumer' value in qos_specs is valid, raise - exception if not. Assign default value to 'consumer', which - is 'back-end' if input is empty. - - :params create a flag indicate if specs being verified is - for create. If it's false, that means specs is for update, - so that there's no need to add 'consumer' if that wasn't in - specs. - """ - - # Check control location, if it's missing in input, assign default - # control location: 'front-end' - if not specs: - specs = {} - # remove 'name' since we will handle that elsewhere. - if specs.get('name', None): - del specs['name'] - try: - if specs['consumer'] not in CONTROL_LOCATION: - msg = _("Valid consumer of QoS specs are: %s") % CONTROL_LOCATION - raise exception.InvalidQoSSpecs(reason=msg) - except KeyError: - # Default consumer is back-end, i.e Cinder volume service - if create: - specs['consumer'] = 'back-end' - - return specs - - def create(context, name, specs=None): """Creates qos_specs. @@ -71,23 +40,19 @@ def create(context, name, specs=None): 'total_iops_sec': 1000, 'total_bytes_sec': 1024000} """ - _verify_prepare_qos_specs(specs) + consumer = specs.get('consumer') + if consumer: + # If we need to modify specs, copy so we don't cause unintended + # consequences for the caller + specs = specs.copy() + del specs['consumer'] - values = dict(name=name, qos_specs=specs) + values = dict(name=name, consumer=consumer, specs=specs) LOG.debug("Dict for qos_specs: %s", values) - - try: - qos_specs_ref = db.qos_specs_create(context, values) - except db_exc.DBDataError: - msg = _('Error writing field to database') - LOG.exception(msg) - raise exception.Invalid(msg) - except db_exc.DBError: - LOG.exception(_LE('DB error:')) - raise exception.QoSSpecsCreateFailed(name=name, - qos_specs=specs) - return qos_specs_ref + qos_spec = objects.QualityOfServiceSpecs(context, **values) + qos_spec.create() + return qos_spec def update(context, qos_specs_id, specs): @@ -99,17 +64,29 @@ def update(context, qos_specs_id, specs): 'total_iops_sec': 500, 'total_bytes_sec': 512000,} """ - # need to verify specs in case 'consumer' is passed - _verify_prepare_qos_specs(specs, create=False) LOG.debug('qos_specs.update(): specs %s' % specs) + try: - res = db.qos_specs_update(context, qos_specs_id, specs) + qos_spec = objects.QualityOfServiceSpecs.get_by_id(context, + qos_specs_id) + + if 'consumer' in specs: + qos_spec.consumer = specs['consumer'] + # If we need to modify specs, copy so we don't cause unintended + # consequences for the caller + specs = specs.copy() + del specs['consumer'] + + # Update any values in specs dict + qos_spec.specs.update(specs) + + qos_spec.save() except db_exc.DBError: LOG.exception(_LE('DB error:')) raise exception.QoSSpecsUpdateFailed(specs_id=qos_specs_id, qos_specs=specs) - return res + return qos_spec def delete(context, qos_specs_id, force=False): @@ -126,15 +103,10 @@ def delete(context, qos_specs_id, force=False): msg = _("id cannot be None") raise exception.InvalidQoSSpecs(reason=msg) - # 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) + qos_spec = objects.QualityOfServiceSpecs.get_by_id( + context, qos_specs_id) - db.qos_specs_delete(context, qos_specs_id) + qos_spec.destroy(force) def delete_keys(context, qos_specs_id, keys): @@ -143,30 +115,42 @@ def delete_keys(context, qos_specs_id, keys): 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) + qos_spec = objects.QualityOfServiceSpecs.get_by_id(context, qos_specs_id) + + # Previous behavior continued to delete keys until it hit first unset one, + # so for now will mimic that. In the future it would be useful to have all + # or nothing deletion of keys (or at least delete all set keys), + # especially since order of keys from CLI to API is not preserved currently + try: + for key in keys: + try: + del qos_spec.specs[key] + except KeyError: + raise exception.QoSSpecsKeyNotFound( + specs_key=key, specs_id=qos_specs_id) + finally: + qos_spec.save() -def get_associations(context, specs_id): +def get_associations(context, qos_specs_id): """Get all associations of given qos specs.""" try: - # query returns a list of volume types associated with qos specs - associates = db.qos_specs_associations_get(context, specs_id) + qos_spec = objects.QualityOfServiceSpecs.get_by_id( + context, qos_specs_id) except db_exc.DBError: LOG.exception(_LE('DB error:')) msg = _('Failed to get all associations of ' - 'qos specs %s') % specs_id + 'qos specs %s') % qos_specs_id LOG.warning(msg) raise exception.CinderException(message=msg) result = [] - for vol_type in associates: - member = dict(association_type='volume_type') - member.update(dict(name=vol_type['name'])) - member.update(dict(id=vol_type['id'])) - result.append(member) + for vol_type in qos_spec.volume_types: + result.append({ + 'association_type': 'volume_type', + 'name': vol_type.name, + 'id': vol_type.id + }) return result @@ -234,28 +218,18 @@ def disassociate_all(context, specs_id): def get_all_specs(context, filters=None, marker=None, limit=None, offset=None, sort_keys=None, sort_dirs=None): """Get all non-deleted qos specs.""" - qos_specs = db.qos_specs_get_all(context, filters=filters, marker=marker, - limit=limit, offset=offset, - sort_keys=sort_keys, sort_dirs=sort_dirs) - return qos_specs + return objects.QualityOfServiceSpecsList.get_all( + context, filters=filters, marker=marker, limit=limit, offset=offset, + sort_keys=sort_keys, sort_dirs=sort_dirs) -def get_qos_specs(ctxt, id): +def get_qos_specs(ctxt, spec_id): """Retrieves single qos specs by id.""" - if id is None: + if spec_id is None: msg = _("id cannot be None") raise exception.InvalidQoSSpecs(reason=msg) if ctxt is None: ctxt = context.get_admin_context() - return db.qos_specs_get(ctxt, id) - - -def get_qos_specs_by_name(context, name): - """Retrieves single qos specs by name.""" - if name is None: - msg = _("name cannot be None") - raise exception.InvalidQoSSpecs(reason=msg) - - return db.qos_specs_get_by_name(context, name) + return objects.QualityOfServiceSpecs.get_by_id(ctxt, spec_id) diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py index cb68c779768..161317dd50e 100644 --- a/cinder/volume/volume_types.py +++ b/cinder/volume/volume_types.py @@ -209,6 +209,7 @@ def get_volume_type_encryption(context, volume_type_id): def get_volume_type_qos_specs(volume_type_id): + """Get all qos specs for given volume type.""" ctxt = context.get_admin_context() res = db.volume_type_qos_specs_get(ctxt, volume_type_id) diff --git a/tools/lintstack.py b/tools/lintstack.py index 5f8cdcf5ee3..6def20f1498 100755 --- a/tools/lintstack.py +++ b/tools/lintstack.py @@ -87,6 +87,8 @@ objects_ignore_messages = [ "Module 'cinder.objects' has no 'CGSnapshotList' member", "Module 'cinder.objects' has no 'ConsistencyGroup' member", "Module 'cinder.objects' has no 'ConsistencyGroupList' member", + "Module 'cinder.objects' has no 'QualityOfServiceSpecs' member", + "Module 'cinder.objects' has no 'QualityOfServiceSpecsList' member", "Module 'cinder.objects' has no 'Service' member", "Module 'cinder.objects' has no 'ServiceList' member", "Module 'cinder.objects' has no 'Snapshot' member",