From 1dfff7e52d7e8a9bfd5fc465b7f641ac196b206c Mon Sep 17 00:00:00 2001 From: zhangbailin Date: Thu, 23 Nov 2017 16:48:23 +0800 Subject: [PATCH] Add enough notification for QoS In order to meet the needs of users, add 'created_at' and 'updated_at' in logs. The following interfaces are modified: create, update, delete, delete_keys, associations (List all associations of given QoS specs.), associate (Associate a QoS specs with a volume type.), disassociate (Disassociate a QoS specs from a volume type.), disassociate_all (Disassociate a QoS specs from all volume types.). Users can search log information from different angles according to the notification information. In addition, add an interface that converts the datetime format to date in utils. Closes-Bug: #1569150 Change-Id: I7c05330f108989bf2e8b4b2a2ba8b8428eeee2e7 --- cinder/api/contrib/qos_specs_manage.py | 45 ++++++++++++---- cinder/db/sqlalchemy/api.py | 21 ++++++-- .../unit/api/contrib/test_qos_specs_manage.py | 51 +++++++++++++++---- cinder/tests/unit/db/test_qos_specs.py | 7 ++- cinder/tests/unit/test_qos_specs.py | 6 ++- cinder/tests/unit/test_volume_types.py | 7 ++- cinder/utils.py | 15 ++++++ 7 files changed, 123 insertions(+), 29 deletions(-) diff --git a/cinder/api/contrib/qos_specs_manage.py b/cinder/api/contrib/qos_specs_manage.py index 917cacb3716..828112292dd 100644 --- a/cinder/api/contrib/qos_specs_manage.py +++ b/cinder/api/contrib/qos_specs_manage.py @@ -16,6 +16,7 @@ """The QoS specs extension""" from oslo_log import log as logging +from oslo_utils import timeutils import six from six.moves import http_client import webob @@ -90,7 +91,9 @@ class QoSSpecsController(wsgi.Controller): try: spec = qos_specs.create(context, name, specs) - notifier_info = dict(name=name, specs=specs) + notifier_info = dict(name=name, + created_at=spec.created_at, + specs=specs) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.create', notifier_info) @@ -119,12 +122,16 @@ class QoSSpecsController(wsgi.Controller): def update(self, req, id, body=None): context = req.environ['cinder.context'] context.authorize(policy.UPDATE_POLICY) - self.assert_valid_body(body, 'qos_specs') specs = body['qos_specs'] try: + spec = qos_specs.get_qos_specs(context, id) + qos_specs.update(context, id, specs) - notifier_info = dict(id=id, specs=specs) + notifier_info = dict(id=id, + created_at=spec.created_at, + updated_at=timeutils.utcnow(), + specs=specs) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.update', notifier_info) @@ -164,10 +171,13 @@ class QoSSpecsController(wsgi.Controller): force = utils.get_bool_param('force', req.params) LOG.debug("Delete qos_spec: %(id)s, force: %(force)s", {'id': id, 'force': force}) - try: + spec = qos_specs.get_qos_specs(context, id) + qos_specs.delete(context, id, force) - notifier_info = dict(id=id) + notifier_info = dict(id=id, + created_at=spec.created_at, + deleted_at=timeutils.utcnow()) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.delete', notifier_info) @@ -206,7 +216,10 @@ class QoSSpecsController(wsgi.Controller): try: qos_specs.delete_keys(context, id, keys) - notifier_info = dict(id=id) + spec = qos_specs.get_qos_specs(context, id) + notifier_info = dict(id=id, + created_at=spec.created_at, + updated_at=spec.updated_at) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.delete_keys', notifier_info) except exception.NotFound as err: @@ -227,8 +240,11 @@ class QoSSpecsController(wsgi.Controller): LOG.debug("Get associations for qos_spec id: %s", id) try: + spec = qos_specs.get_qos_specs(context, id) + associates = qos_specs.get_associations(context, id) - notifier_info = dict(id=id) + notifier_info = dict(id=id, + created_at=spec.created_at) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.associations', notifier_info) @@ -267,8 +283,11 @@ class QoSSpecsController(wsgi.Controller): {'id': id, 'type_id': type_id}) try: + spec = qos_specs.get_qos_specs(context, id) + qos_specs.associate_qos_with_type(context, id, type_id) - notifier_info = dict(id=id, type_id=type_id) + notifier_info = dict(id=id, type_id=type_id, + created_at=spec.created_at) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.associate', notifier_info) @@ -316,8 +335,11 @@ class QoSSpecsController(wsgi.Controller): {'id': id, 'type_id': type_id}) try: + spec = qos_specs.get_qos_specs(context, id) + qos_specs.disassociate_qos_specs(context, id, type_id) - notifier_info = dict(id=id, type_id=type_id) + notifier_info = dict(id=id, type_id=type_id, + created_at=spec.created_at) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.disassociate', notifier_info) @@ -346,8 +368,11 @@ class QoSSpecsController(wsgi.Controller): LOG.debug("Disassociate qos_spec: %s from all.", id) try: + spec = qos_specs.get_qos_specs(context, id) + qos_specs.disassociate_all(context, id) - notifier_info = dict(id=id) + notifier_info = dict(id=id, + created_at=spec.created_at) rpc.get_notifier('QoSSpecs').info(context, 'qos_specs.disassociate_all', notifier_info) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index eed502364f4..f877d2b8227 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -61,7 +61,6 @@ from cinder.objects import fields from cinder import utils from cinder.volume import utils as vol_utils - CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -4547,11 +4546,21 @@ def _qos_specs_get_all_ref(context, qos_specs_id, session=None, def _dict_with_children_specs(specs): """Convert specs list to a dict.""" result = {} + update_time = None for spec in specs: # Skip deleted keys if not spec['deleted']: + # Add update time to specs list, in order to get the keyword + # 'updated_at' in specs info when printing logs. + if not update_time and spec['updated_at']: + update_time = spec['updated_at'] + elif update_time and spec['updated_at']: + if (update_time - + spec['updated_at']).total_seconds() < 0: + update_time = spec['updated_at'] result.update({spec['key']: spec['value']}) - + if update_time: + result.update({'updated_at': update_time}) return result @@ -4566,10 +4575,15 @@ def _dict_with_qos_specs(rows): result = [] for row in rows: if row['key'] == 'QoS_Specs_Name': - member = {'name': row['value'], 'id': row['id']} + # Add create time for member, in order to get the keyword + # 'created_at' in the specs info when printing logs. + member = {'name': row['value'], 'id': row['id'], + 'created_at': row['created_at']} if row.specs: spec_dict = _dict_with_children_specs(row.specs) member['consumer'] = spec_dict.pop('consumer') + if spec_dict.get('updated_at'): + member['updated_at'] = spec_dict.pop('updated_at') member.update(dict(specs=spec_dict)) result.append(member) return result @@ -4787,7 +4801,6 @@ def qos_specs_update(context, qos_specs_id, updates): return specs - #################### 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 e2bfc819d5c..116473bfc8d 100644 --- a/cinder/tests/unit/api/contrib/test_qos_specs_manage.py +++ b/cinder/tests/unit/api/contrib/test_qos_specs_manage.py @@ -39,6 +39,8 @@ def stub_qos_specs(id): "key4": "value4", "key5": "value5"} res.update(dict(specs=specs)) + res.update(dict(created_at='2017-12-13T02:37:54Z')) + res.update(dict(updated_at='2017-12-13T02:38:58Z')) return objects.QualityOfServiceSpecs(**res) @@ -102,6 +104,8 @@ def return_qos_specs_create(context, name, specs): return objects.QualityOfServiceSpecs(name=name, specs=specs, + created_at='2017-12-13T02:37:54Z', + updated_at='2017-12-13T02:38:58Z', consumer='back-end', id=fake.QOS_SPEC_ID) @@ -330,9 +334,12 @@ class QoSSpecManageApiTest(test.TestCase): self.controller.delete, req, fake.QOS_SPEC_ID) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.delete_keys', side_effect=return_qos_specs_delete_keys) - def test_qos_specs_delete_keys(self, mock_qos_delete_keys): + def test_qos_specs_delete_keys(self, mock_qos_delete_keys, + mock_get_qos): body = {"keys": ['bar', 'zoo']} req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s/delete_keys' % (fake.PROJECT_ID, fake.IN_USE_ID), @@ -355,9 +362,12 @@ class QoSSpecManageApiTest(test.TestCase): req, fake.WILL_NOT_BE_FOUND_ID, body) self.assertEqual(1, self.notifier.get_notification_count()) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.delete_keys', side_effect=return_qos_specs_delete_keys) - def test_qos_specs_delete_keys_badkey(self, mock_qos_specs_delete): + def test_qos_specs_delete_keys_badkey(self, mock_qos_specs_delete, + mock_get_qos): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s/delete_keys' % (fake.PROJECT_ID, fake.IN_USE_ID), use_admin_context=True) @@ -370,7 +380,10 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.delete_keys', side_effect=return_qos_specs_delete_keys) - def test_qos_specs_delete_keys_get_notifier(self, mock_qos_delete_keys): + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) + def test_qos_specs_delete_keys_get_notifier(self, mock_get_qos_specs, + mock_qos_delete_keys): body = {"keys": ['bar', 'zoo']} req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s/delete_keys' % (fake.PROJECT_ID, fake.IN_USE_ID), @@ -467,7 +480,9 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.update', side_effect=return_qos_specs_update) - def test_update(self, mock_qos_update): + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) + def test_update(self, mock_get_qos, mock_qos_update): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.QOS_SPEC_ID), use_admin_context=True) @@ -479,7 +494,9 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.update', side_effect=return_qos_specs_update) - def test_update_not_found(self, mock_qos_update): + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) + def test_update_not_found(self, mock_get_qos_specs, mock_qos_update): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.WILL_NOT_BE_FOUND_ID), @@ -491,9 +508,11 @@ class QoSSpecManageApiTest(test.TestCase): req, fake.WILL_NOT_BE_FOUND_ID, body) self.assertEqual(1, self.notifier.get_notification_count()) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.update', side_effect=return_qos_specs_update) - def test_update_invalid_input(self, mock_qos_update): + def test_update_invalid_input(self, mock_qos_update, mock_get_qos): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.INVALID_ID), use_admin_context=True) @@ -504,10 +523,12 @@ class QoSSpecManageApiTest(test.TestCase): req, fake.INVALID_ID, body) self.assertEqual(1, self.notifier.get_notification_count()) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) @ddt.data({'qos_specs': {'key1': ['value1']}}, {'qos_specs': {1: 'value1'}} ) - def test_update_non_string_key_or_value(self, body): + def test_update_non_string_key_or_value(self, body, mock_get_qos): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.UUID1), use_admin_context=True) @@ -516,9 +537,11 @@ class QoSSpecManageApiTest(test.TestCase): req, fake.UUID1, body) self.assertEqual(1, self.notifier.get_notification_count()) + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.update', side_effect=return_qos_specs_update) - def test_update_failed(self, mock_qos_update): + def test_update_failed(self, mock_qos_update, mock_get_qos): req = fakes.HTTPRequest.blank('/v2/%s/qos-specs/%s' % (fake.PROJECT_ID, fake.UPDATE_FAILED_ID), @@ -543,7 +566,9 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.get_associations', side_effect=return_get_qos_associations) - def test_get_associations(self, mock_get_assciations): + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) + def test_get_associations(self, mock_get_qos, mock_get_assciations): req = fakes.HTTPRequest.blank( '/v2/%s/qos-specs/%s/associations' % ( fake.PROJECT_ID, fake.QOS_SPEC_ID), use_admin_context=True) @@ -567,7 +592,10 @@ class QoSSpecManageApiTest(test.TestCase): @mock.patch('cinder.volume.qos_specs.get_associations', side_effect=return_get_qos_associations) - def test_get_associations_failed(self, mock_get_associations): + @mock.patch('cinder.volume.qos_specs.get_qos_specs', + side_effect=return_qos_specs_get_qos_specs) + def test_get_associations_failed(self, mock_get_qos, + mock_get_associations): req = fakes.HTTPRequest.blank( '/v2/%s/qos-specs/%s/associations' % ( fake.PROJECT_ID, fake.RAISE_ID), use_admin_context=True) @@ -706,7 +734,8 @@ class QoSSpecManageApiTest(test.TestCase): side_effect=return_qos_specs_get_qos_specs) @mock.patch('cinder.volume.qos_specs.disassociate_all', side_effect=return_disassociate_all) - def test_disassociate_all_not_found(self, mock_disassociate, mock_get): + def test_disassociate_all_not_found(self, mock_disassociate, + mock_get_qos): req = fakes.HTTPRequest.blank( '/v2/%s/qos-specs/%s/disassociate_all' % ( fake.PROJECT_ID, fake.WILL_NOT_BE_FOUND_ID), diff --git a/cinder/tests/unit/db/test_qos_specs.py b/cinder/tests/unit/db/test_qos_specs.py index dcaff931fed..a93784e308d 100644 --- a/cinder/tests/unit/db/test_qos_specs.py +++ b/cinder/tests/unit/db/test_qos_specs.py @@ -76,6 +76,7 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): db.qos_specs_get, self.ctxt, fake_id) specs_returned = db.qos_specs_get(self.ctxt, specs_id) + qos_spec['created_at'] = specs_returned['created_at'] qos_spec['id'] = specs_id self.assertDictEqual(qos_spec, specs_returned) @@ -91,10 +92,12 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): 'consumer': 'back-end', 'specs': {'key1': 'v5', 'key2': 'v6'}}] - for qos in qos_list: + for index, qos in enumerate(qos_list): qos['id'] = self._create_qos_specs(qos['name'], qos['consumer'], qos['specs']) + specs = db.qos_specs_get(self.ctxt, qos['id']) + qos_list[index]['created_at'] = specs['created_at'] specs_list_returned = db.qos_specs_get_all(self.ctxt) self.assertEqual(len(qos_list), len(specs_list_returned), @@ -124,6 +127,7 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): 'specs': value} db.qos_specs_item_delete(self.ctxt, specs_id, 'foo') specs = db.qos_specs_get(self.ctxt, specs_id) + expected['created_at'] = specs['created_at'] self.assertDictEqual(expected, specs) def test_associate_type_with_qos(self): @@ -201,6 +205,7 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase): self.ctxt, fake.WILL_NOT_BE_FOUND_ID, value) db.qos_specs_update(self.ctxt, specs_id, value) specs = db.qos_specs_get(self.ctxt, specs_id) + value['created_at'] = specs['created_at'] self.assertEqual('new_value2', specs['specs']['key2']) self.assertEqual('value3', specs['specs']['key3']) self.assertEqual('both', specs['consumer']) diff --git a/cinder/tests/unit/test_qos_specs.py b/cinder/tests/unit/test_qos_specs.py index 596a8d656b5..2722756793e 100644 --- a/cinder/tests/unit/test_qos_specs.py +++ b/cinder/tests/unit/test_qos_specs.py @@ -29,6 +29,7 @@ from cinder import db from cinder import exception from cinder import test from cinder.tests.unit import fake_constants as fake +from cinder import utils from cinder.volume import qos_specs from cinder.volume import volume_types @@ -366,12 +367,15 @@ class QoSSpecsTestCase(test.TestCase): 'key3': 'value3', 'key4': 'value4'}}] - for qos_specs_dict in qos_specs_list: + for index, qos_specs_dict in enumerate(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 + specs = db.qos_specs_get(self.ctxt, qos_specs_id) + qos_specs_list[index]['created_at'] = utils.time_format( + specs['created_at']) res = qos_specs.get_all_specs(self.ctxt) self.assertEqual(len(qos_specs_list), len(res)) diff --git a/cinder/tests/unit/test_volume_types.py b/cinder/tests/unit/test_volume_types.py index 3c2b518f82c..5a4ab8ff336 100644 --- a/cinder/tests/unit/test_volume_types.py +++ b/cinder/tests/unit/test_volume_types.py @@ -361,6 +361,8 @@ class VolumeTypeTestCase(test.TestCase): 'k2': 'v2', 'k3': 'v3'}}} res = volume_types.get_volume_type_qos_specs(type_ref['id']) + specs = db.qos_specs_get(self.ctxt, qos_ref['id']) + expected['qos_specs']['created_at'] = specs['created_at'] self.assertDictEqual(expected, res) def test_volume_types_diff(self): @@ -382,7 +384,7 @@ class VolumeTypeTestCase(test.TestCase): self.assertEqual(('val1', 'val0'), diff['extra_specs']['key1']) # qos_ref 1 and 2 have the same specs, while 3 has different - qos_keyvals1 = {'k1': 'v1', 'k2': 'v2', 'k3': 'v3'} + qos_keyvals1 = {'k1': 'v1', 'k2': 'v2', 'k3': 'v3', 'created_at': 'v4'} qos_keyvals2 = {'k1': 'v0', 'k2': 'v2', 'k3': 'v3'} qos_ref1 = qos_specs.create(self.ctxt, 'qos-specs-1', qos_keyvals1) qos_ref2 = qos_specs.create(self.ctxt, 'qos-specs-2', qos_keyvals1) @@ -437,7 +439,8 @@ class VolumeTypeTestCase(test.TestCase): self.assertEqual({'consumer': (None, 'back-end'), 'k1': (None, 'v1'), 'k2': (None, 'v2'), - 'k3': (None, 'v3')}, diff['qos_specs']) + 'k3': (None, 'v3'), + 'created_at': (None, 'v4')}, diff['qos_specs']) self.assertEqual({'cipher': (None, 'c1'), 'control_location': (None, 'front-end'), 'deleted': (None, False), diff --git a/cinder/utils.py b/cinder/utils.py index 3e305e8a654..37b28c9e5ef 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -271,6 +271,21 @@ def last_completed_audit_period(unit=None): return (begin, end) +def time_format(at=None): + """Format datetime string to date. + + :param at: Type is datetime.datetime (example + 'datetime.datetime(2017, 12, 24, 22, 11, 32, 6086)') + :returns: Format date (example '2017-12-24T22:11:32Z'). + """ + if not at: + at = timeutils.utcnow() + date_string = at.strftime("%Y-%m-%dT%H:%M:%S") + tz = at.tzname(None) if at.tzinfo else 'UTC' + date_string += ('Z' if tz == 'UTC' else tz) + return date_string + + def is_none_string(val): """Check if a string represents a None value.""" if not isinstance(val, six.string_types):