diff --git a/cinder/db/api.py b/cinder/db/api.py index 7ee9b905a31..234269a67b6 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -99,34 +99,27 @@ def service_destroy(context, service_id): return IMPL.service_destroy(context, service_id) -def service_get(context, service_id): - """Get a service or raise if it does not exist.""" - return IMPL.service_get(context, service_id) +def service_get(context, service_id=None, **filters): + """Get a service that matches the criteria. + + A possible filter is is_up=True and it will filter nodes that are down. + + :param service_id: Id of the service. + :param filters: Filters for the query in the form of key/value. + + :raise ServiceNotFound: If service doesn't exist. + """ + return IMPL.service_get(context, service_id, **filters) -def service_get_by_host_and_topic(context, host, topic): - """Get a service by host it's on and topic it listens to.""" - return IMPL.service_get_by_host_and_topic(context, host, topic) +def service_get_all(context, **filters): + """Get all services that match the criteria. + A possible filter is is_up=True and it will filter nodes that are down. -def service_get_all(context, filters=None): - """Get all services.""" - return IMPL.service_get_all(context, filters) - - -def service_get_all_by_topic(context, topic, disabled=None): - """Get all services for a given topic.""" - return IMPL.service_get_all_by_topic(context, topic, disabled=disabled) - - -def service_get_all_by_binary(context, binary, disabled=None): - """Get all services for a given binary.""" - return IMPL.service_get_all_by_binary(context, binary, disabled) - - -def service_get_by_args(context, host, binary): - """Get the state of a service by node name and binary.""" - return IMPL.service_get_by_args(context, host, binary) + :param filters: Filters for the query in the form of key/value arguments. + """ + return IMPL.service_get_all(context, **filters) def service_create(context, values): @@ -138,7 +131,6 @@ def service_update(context, service_id, values): """Set the given properties on an service and update it. Raises NotFound if service does not exist. - """ return IMPL.service_update(context, service_id, values) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index dce988c47a7..bd82ae6850f 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -362,105 +362,84 @@ QUOTA_SYNC_FUNCTIONS = { ################### -@require_admin_context -def service_destroy(context, service_id): - session = get_session() - with session.begin(): - service_ref = _service_get(context, service_id, session=session) - service_ref.delete(session=session) +def _clean_filters(filters): + return {k: v for k, v in filters.items() if v is not None} -@require_admin_context -def _service_get(context, service_id, session=None): - result = model_query( - context, - models.Service, - session=session).\ - filter_by(id=service_id).\ - first() - if not result: - raise exception.ServiceNotFound(service_id=service_id) - - return result - - -@require_admin_context -def service_get(context, service_id): - return _service_get(context, service_id) - - -@require_admin_context -def service_get_all(context, filters=None): +def _service_query(context, session=None, read_deleted='no', host=None, + is_up=None, **filters): + filters = _clean_filters(filters) if filters and not is_valid_model_filters(models.Service, filters): - return [] + return None - query = model_query(context, models.Service) + query = model_query(context, models.Service, session=session, + read_deleted=read_deleted) + + # Host is a particular case of filter, because we must retrieve not only + # exact matches (single backend configuration), but also match hosts that + # have the backend defined (multi backend configuration). + if host: + host_attr = models.Service.host + # Mysql is not doing case sensitive filtering, so we force it + if CONF.database.connection.startswith('mysql:'): + conditions = or_(host_attr == func.binary(host), + host_attr.op('LIKE BINARY')(host + '@%')) + else: + conditions = or_(host_attr == host, + host_attr.op('LIKE')(host + '@%')) + query = query.filter(conditions) if filters: - try: - host = filters.pop('host') - host_attr = models.Service.host - conditions = or_(host_attr == - host, host_attr.op('LIKE')(host + '@%')) - query = query.filter(conditions) - except KeyError: - pass - query = query.filter_by(**filters) - return query.all() + if is_up is not None: + date_limit = (timeutils.utcnow() - + dt.timedelta(seconds=CONF.service_down_time)) + svc = models.Service + filter_ = or_( + and_(svc.created_at.isnot(None), svc.created_at >= date_limit), + and_(svc.updated_at.isnot(None), svc.updated_at >= date_limit)) + query = query.filter(filter_ == is_up) + + return query @require_admin_context -def service_get_all_by_topic(context, topic, disabled=None): - query = model_query( - context, models.Service, read_deleted="no").\ - filter_by(topic=topic) - - if disabled is not None: - query = query.filter_by(disabled=disabled) - - return query.all() +def service_destroy(context, service_id): + query = _service_query(context, id=service_id) + if not query.update(models.Service.delete_values()): + raise exception.ServiceNotFound(service_id=service_id) @require_admin_context -def service_get_all_by_binary(context, binary, disabled=None): - query = model_query( - context, models.Service, read_deleted="no").filter_by(binary=binary) +def service_get(context, service_id=None, **filters): + """Get a service that matches the criteria. - if disabled is not None: - query = query.filter_by(disabled=disabled) + A possible filter is is_up=True and it will filter nodes that are down. - return query.all() + :param service_id: Id of the service. + :param filters: Filters for the query in the form of key/value. + :raise ServiceNotFound: If service doesn't exist. + """ + query = _service_query(context, id=service_id, **filters) + service = None if not query else query.first() + if not service: + serv_id = service_id or filters.get('topic') or filters.get('binary') + raise exception.ServiceNotFound(service_id=serv_id, + host=filters.get('host')) + return service @require_admin_context -def service_get_by_host_and_topic(context, host, topic): - result = model_query( - context, models.Service, read_deleted="no").\ - filter_by(disabled=False).\ - filter_by(host=host).\ - filter_by(topic=topic).\ - first() - if not result: - raise exception.ServiceNotFound(service_id=topic, - host=host) - return result +def service_get_all(context, **filters): + """Get all services that match the criteria. + A possible filter is is_up=True and it will filter nodes that are down. -@require_admin_context -def service_get_by_args(context, host, binary): - results = model_query(context, models.Service).\ - filter_by(host=host).\ - filter_by(binary=binary).\ - all() - - for result in results: - if host == result['host']: - return result - - raise exception.ServiceNotFound(service_id=binary, - host=host) + :param filters: Filters for the query in the form of key/value. + """ + query = _service_query(context, **filters) + return [] if not query else query.all() @require_admin_context @@ -479,14 +458,15 @@ def service_create(context, values): @require_admin_context @_retry_on_deadlock def service_update(context, service_id, values): - session = get_session() - with session.begin(): - service_ref = _service_get(context, service_id, session=session) - if ('disabled' in values): - service_ref['modified_at'] = timeutils.utcnow() - service_ref['updated_at'] = literal_column('updated_at') - service_ref.update(values) - return service_ref + if 'disabled' in values: + values = values.copy() + values['modified_at'] = values.get('modified_at', timeutils.utcnow()) + values['updated_at'] = values.get('updated_at', + literal_column('updated_at')) + query = _service_query(context, id=service_id) + result = query.update(values) + if not result: + raise exception.ServiceNotFound(service_id=service_id) ################### diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index d7879cf980d..8edfcd2e770 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -45,10 +45,14 @@ class CinderBase(models.TimestampMixin, deleted = Column(Boolean, default=False) metadata = None + @staticmethod + def delete_values(): + return {'deleted': True, + 'deleted_at': timeutils.utcnow()} + def delete(self, session): """Delete this object.""" - self.deleted = True - self.deleted_at = timeutils.utcnow() + self.update(self.delete_values()) self.save(session=session) diff --git a/cinder/objects/service.py b/cinder/objects/service.py index d64f21be697..c1e7dce466c 100644 --- a/cinder/objects/service.py +++ b/cinder/objects/service.py @@ -70,12 +70,13 @@ class Service(base.CinderPersistentObject, base.CinderObject, @classmethod def get_by_host_and_topic(cls, context, host, topic): - db_service = db.service_get_by_host_and_topic(context, host, topic) + db_service = db.service_get(context, disabled=False, host=host, + topic=topic) return cls._from_db_object(context, cls(context), db_service) @classmethod def get_by_args(cls, context, host, binary_key): - db_service = db.service_get_by_args(context, host, binary_key) + db_service = db.service_get(context, host=host, binary=binary_key) return cls._from_db_object(context, cls(context), db_service) def create(self): @@ -139,20 +140,19 @@ class ServiceList(base.ObjectListBase, base.CinderObject): @classmethod def get_all(cls, context, filters=None): - services = db.service_get_all(context, filters) + services = db.service_get_all(context, **(filters or {})) return base.obj_make_list(context, cls(context), objects.Service, services) @classmethod def get_all_by_topic(cls, context, topic, disabled=None): - services = db.service_get_all_by_topic(context, topic, - disabled=disabled) + services = db.service_get_all(context, topic=topic, disabled=disabled) return base.obj_make_list(context, cls(context), objects.Service, services) @classmethod def get_all_by_binary(cls, context, binary, disabled=None): - services = db.service_get_all_by_binary(context, binary, - disabled=disabled) + services = db.service_get_all(context, binary=binary, + disabled=disabled) return base.obj_make_list(context, cls(context), objects.Service, services) diff --git a/cinder/test.py b/cinder/test.py index b1ce45c4b92..a17c792cdde 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -321,6 +321,13 @@ class TestCase(testtools.TestCase): self.addCleanup(patcher.stop) return new_attr + def patch(self, path, *args, **kwargs): + """Use python mock to mock a path with automatic cleanup.""" + patcher = mock.patch(path, *args, **kwargs) + result = patcher.start() + self.addCleanup(patcher.stop) + return result + # Useful assertions def assertDictMatch(self, d1, d2, approx_equal=False, tolerance=0.001): """Assert two dicts are equivalent. diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py index 680668b1750..409a740cd60 100644 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py @@ -659,11 +659,12 @@ class AdminActionsTest(BaseAdminTest): vac.validate_update, {'status': 'creating'}) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.backup.rpcapi.BackupAPI.delete_backup', mock.Mock()) + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.backup.api.API._check_support_to_force_delete') def _force_delete_backup_util(self, test_status, mock_check_support, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + mock_service_get_all): + mock_service_get_all.return_value = [ {'availability_zone': "az1", 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] # admin context diff --git a/cinder/tests/unit/api/contrib/test_backups.py b/cinder/tests/unit/api/contrib/test_backups.py index 17fc97d7d58..e4a02d82b60 100644 --- a/cinder/tests/unit/api/contrib/test_backups.py +++ b/cinder/tests/unit/api/contrib/test_backups.py @@ -57,6 +57,8 @@ class BackupsAPITestCase(test.TestCase): self.user_context = context.RequestContext( fake.USER_ID, fake.PROJECT_ID, auth_token=True) self.controller = backups.BackupsController() + self.patch('cinder.objects.service.Service._get_minimum_version', + return_value=None) @staticmethod def _create_backup(volume_id=fake.VOLUME_ID, @@ -462,12 +464,12 @@ class BackupsAPITestCase(test.TestCase): fake_auth_context=self.user_context)) self.assertEqual(400, res.status_int) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_create_backup_json(self, mock_validate, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -491,15 +493,17 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['backup']) - self.assertTrue(_mock_service_get_all_by_topic.called) + _mock_service_get_all.assert_called_once_with(mock.ANY, + disabled=False, + topic='cinder-backup') self.assertTrue(mock_validate.called) db.volume_destroy(context.get_admin_context(), volume_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_backup_inuse_no_force(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -528,9 +532,9 @@ class BackupsAPITestCase(test.TestCase): db.volume_destroy(context.get_admin_context(), volume_id) - @mock.patch('cinder.db.service_get_all_by_topic') - def test_create_backup_inuse_force(self, _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + @mock.patch('cinder.db.service_get_all') + def test_create_backup_inuse_force(self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -557,17 +561,19 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['backup']) - self.assertTrue(_mock_service_get_all_by_topic.called) + _mock_service_get_all.assert_called_once_with(mock.ANY, + disabled=False, + topic='cinder-backup') db.backup_destroy(context.get_admin_context(), backup_id) db.volume_destroy(context.get_admin_context(), volume_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_create_backup_snapshot_json(self, mock_validate, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -591,7 +597,9 @@ class BackupsAPITestCase(test.TestCase): res_dict = jsonutils.loads(res.body) self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['backup']) - self.assertTrue(_mock_service_get_all_by_topic.called) + _mock_service_get_all.assert_called_once_with(mock.ANY, + disabled=False, + topic='cinder-backup') self.assertTrue(mock_validate.called) db.volume_destroy(context.get_admin_context(), volume_id) @@ -705,14 +713,14 @@ class BackupsAPITestCase(test.TestCase): req, body) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') @ddt.data(False, True) def test_create_backup_delta(self, backup_from_snapshot, mock_validate, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -746,7 +754,9 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertIn('id', res_dict['backup']) - self.assertTrue(_mock_service_get_all_by_topic.called) + _mock_service_get_all.assert_called_once_with(mock.ANY, + disabled=False, + topic='cinder-backup') self.assertTrue(mock_validate.called) db.backup_destroy(context.get_admin_context(), backup_id) @@ -754,10 +764,10 @@ class BackupsAPITestCase(test.TestCase): snapshot.destroy() db.volume_destroy(context.get_admin_context(), volume_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_incremental_backup_invalid_status( - self, _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -869,12 +879,12 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(400, res.status_int) self.assertEqual(400, res_dict['badRequest']['code']) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_backup_WithOUT_enabled_backup_service( self, - _mock_service_get_all_by_topic): + _mock_service_get_all): # need an enabled backup service available - _mock_service_get_all_by_topic.return_value = [] + _mock_service_get_all.return_value = [] volume_id = utils.create_volume(self.context, size=2).id req = webob.Request.blank('/v2/%s/backups' % fake.PROJECT_ID) @@ -901,10 +911,10 @@ class BackupsAPITestCase(test.TestCase): volume = self.volume_api.get(context.get_admin_context(), volume_id) self.assertEqual('available', volume['status']) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_incremental_backup_invalid_no_full( - self, _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'fake_az', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] @@ -934,8 +944,8 @@ class BackupsAPITestCase(test.TestCase): db.volume_destroy(context.get_admin_context(), volume_id) - @mock.patch('cinder.db.service_get_all_by_topic') - def test_is_backup_service_enabled(self, _mock_service_get_all_by_topic): + @mock.patch('cinder.db.service_get_all') + def test_is_backup_service_enabled(self, _mock_service_get_all): testhost = 'test_host' alt_host = 'strange_host' @@ -960,12 +970,12 @@ class BackupsAPITestCase(test.TestCase): 'disabled': 0, 'updated_at': timeutils.utcnow()}] # Setup mock to run through the following service cases - _mock_service_get_all_by_topic.side_effect = [empty_service, - host_not_match, - az_not_match, - disabled_service, - dead_service, - multi_services] + _mock_service_get_all.side_effect = [empty_service, + host_not_match, + az_not_match, + disabled_service, + dead_service, + multi_services] volume_id = utils.create_volume(self.context, size=2, host=testhost).id @@ -1006,10 +1016,9 @@ class BackupsAPITestCase(test.TestCase): volume['availability_zone'], testhost)) - @mock.patch('cinder.db.service_get_all_by_topic') - def test_get_available_backup_service(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + @mock.patch('cinder.db.service_get_all') + def test_get_available_backup_service(self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost1', 'disabled': 0, 'updated_at': timeutils.utcnow()}, {'availability_zone': 'az2', 'host': 'testhost2', @@ -1026,10 +1035,10 @@ class BackupsAPITestCase(test.TestCase): 'testhost4', 'az1') self.assertEqual('testhost1', actual_host) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_get_available_backup_service_with_same_host( - self, _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost1', 'disabled': 0, 'updated_at': timeutils.utcnow()}, {'availability_zone': 'az2', 'host': 'testhost2', @@ -1045,10 +1054,9 @@ class BackupsAPITestCase(test.TestCase): self.backup_api._get_available_backup_service_host, 'testhost4', 'az1') - @mock.patch('cinder.db.service_get_all_by_topic') - def test_delete_backup_available( - self, _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + @mock.patch('cinder.db.service_get_all') + def test_delete_backup_available(self, _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE) @@ -1065,10 +1073,10 @@ class BackupsAPITestCase(test.TestCase): db.backup_destroy(context.get_admin_context(), backup_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_delete_delta_backup(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] backup_id = self._create_backup(status=fields.BackupStatus.AVAILABLE) @@ -1088,10 +1096,10 @@ class BackupsAPITestCase(test.TestCase): db.backup_destroy(context.get_admin_context(), delta_id) db.backup_destroy(context.get_admin_context(), backup_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_delete_backup_error(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] backup_id = self._create_backup(status=fields.BackupStatus.ERROR) @@ -1141,10 +1149,10 @@ class BackupsAPITestCase(test.TestCase): db.backup_destroy(context.get_admin_context(), backup_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_delete_backup_with_InvalidBackup2(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] volume_id = utils.create_volume(self.context, size=5).id @@ -1170,10 +1178,10 @@ class BackupsAPITestCase(test.TestCase): db.backup_destroy(context.get_admin_context(), delta_backup_id) db.backup_destroy(context.get_admin_context(), backup_id) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_delete_backup_service_down(self, - _mock_service_get_all_by_topic): - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all): + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': '1775-04-19 05:00:00'}] backup_id = self._create_backup(status='available') @@ -1256,18 +1264,17 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual("Missing required element 'restore' in request body.", res_dict['badRequest']['message']) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.volume.api.API.create') def test_restore_backup_volume_id_unspecified( - self, _mock_volume_api_create, - _mock_service_get_all_by_topic): + self, _mock_volume_api_create, _mock_service_get_all): # intercept volume creation to ensure created volume # has status of available def fake_volume_api_create(context, size, name, description): volume_id = utils.create_volume(self.context, size=size).id return db.volume_get(context, volume_id) - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] _mock_volume_api_create.side_effect = fake_volume_api_create @@ -1288,11 +1295,11 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(202, res.status_int) self.assertEqual(backup_id, res_dict['restore']['backup_id']) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.volume.api.API.create') def test_restore_backup_name_specified(self, _mock_volume_api_create, - _mock_service_get_all_by_topic): + _mock_service_get_all): # Intercept volume creation to ensure created volume # has status of available def fake_volume_api_create(context, size, name, description): @@ -1301,7 +1308,7 @@ class BackupsAPITestCase(test.TestCase): return db.volume_get(context, volume_id) _mock_volume_api_create.side_effect = fake_volume_api_create - _mock_service_get_all_by_topic.return_value = [ + _mock_service_get_all.return_value = [ {'availability_zone': 'az1', 'host': 'testhost', 'disabled': 0, 'updated_at': timeutils.utcnow()}] diff --git a/cinder/tests/unit/api/contrib/test_hosts.py b/cinder/tests/unit/api/contrib/test_hosts.py index f482a172d17..19fbf3b6856 100644 --- a/cinder/tests/unit/api/contrib/test_hosts.py +++ b/cinder/tests/unit/api/contrib/test_hosts.py @@ -21,7 +21,6 @@ import webob.exc from cinder.api.contrib import hosts as os_hosts from cinder import context -from cinder import db from cinder import test @@ -69,10 +68,6 @@ def stub_utcnow(with_timezone=False): return datetime.datetime(2013, 7, 3, 0, 0, 2, tzinfo=tzinfo) -def stub_service_get_all(context, filters=None): - return SERVICE_LIST - - class FakeRequest(object): environ = {'cinder.context': context.get_admin_context()} GET = {} @@ -90,9 +85,9 @@ class HostTestCase(test.TestCase): super(HostTestCase, self).setUp() self.controller = os_hosts.HostController() self.req = FakeRequest() - self.stubs.Set(db, 'service_get_all', - stub_service_get_all) - self.stubs.Set(timeutils, 'utcnow', stub_utcnow) + self.patch('cinder.db.service_get_all', autospec=True, + return_value=SERVICE_LIST) + self.mock_object(timeutils, 'utcnow', stub_utcnow) def _test_host_update(self, host, key, val, expected_value): body = {key: val} diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index 9c330060a15..1e77d5325d3 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -17,15 +17,13 @@ import datetime from iso8601 import iso8601 -from oslo_utils import timeutils +import mock import webob.exc from cinder.api.contrib import services from cinder.api import extensions from cinder import context -from cinder import db from cinder import exception -from cinder import policy from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake @@ -132,21 +130,24 @@ class FakeRequestWithHostBinary(object): GET = {"host": "host1", "binary": "cinder-volume"} -def fake_service_get_all(context, filters=None): - filters = filters or {} - host = filters.get('host') - binary = filters.get('binary') - return [s for s in fake_services_list - if (not host or s['host'] == host or - s['host'].startswith(host + '@')) - and (not binary or s['binary'] == binary)] - - -def fake_service_get_by_host_binary(context, host, binary): +def fake_service_get_all(context, **filters): + result = [] + host = filters.pop('host', None) for service in fake_services_list: - if service['host'] == host and service['binary'] == binary: - return service - return None + if (host and service['host'] != host and + not service['host'].startswith(host + '@')): + continue + + if all(v is None or service.get(k) == v for k, v in filters.items()): + result.append(service) + return result + + +def fake_service_get(context, service_id=None, **filters): + result = fake_service_get_all(context, id=service_id, **filters) + if not result: + raise exception.ServiceNotFound(service_id=service_id) + return result[0] def fake_service_get_by_id(value): @@ -174,18 +175,16 @@ def fake_utcnow(with_timezone=False): return datetime.datetime(2012, 10, 29, 13, 42, 11, tzinfo=tzinfo) +@mock.patch('cinder.db.service_get_all', fake_service_get_all) +@mock.patch('cinder.db.service_get', fake_service_get) +@mock.patch('oslo_utils.timeutils.utcnow', fake_utcnow) +@mock.patch('cinder.db.sqlalchemy.api.service_update', fake_service_update) +@mock.patch('cinder.policy.enforce', fake_policy_enforce) class ServicesTest(test.TestCase): def setUp(self): super(ServicesTest, self).setUp() - self.stubs.Set(db, "service_get_all", fake_service_get_all) - self.stubs.Set(timeutils, "utcnow", fake_utcnow) - self.stubs.Set(db, "service_get_by_args", - fake_service_get_by_host_binary) - self.stubs.Set(db, "service_update", fake_service_update) - self.stubs.Set(policy, "enforce", fake_policy_enforce) - self.context = context.get_admin_context() self.ext_mgr = extensions.ExtensionManager() self.ext_mgr.extensions = {} diff --git a/cinder/tests/unit/api/contrib/test_snapshot_manage.py b/cinder/tests/unit/api/contrib/test_snapshot_manage.py index 0bdd8bd447d..59d84ffec66 100644 --- a/cinder/tests/unit/api/contrib/test_snapshot_manage.py +++ b/cinder/tests/unit/api/contrib/test_snapshot_manage.py @@ -109,7 +109,7 @@ class SnapshotManageTest(test.TestCase): @mock.patch('cinder.volume.rpcapi.VolumeAPI.manage_existing_snapshot') @mock.patch('cinder.volume.api.API.create_snapshot_in_db') - @mock.patch('cinder.db.service_get_by_args') + @mock.patch('cinder.db.service_get') def test_manage_snapshot_ok(self, mock_db, mock_create_snapshot, mock_rpcapi): """Test successful manage volume execution. @@ -126,11 +126,9 @@ class SnapshotManageTest(test.TestCase): res = self._get_resp_post(body) self.assertEqual(202, res.status_int, res) - # Check the db.service_get_by_host_and_topic was called with correct - # arguments. - self.assertEqual(1, mock_db.call_count) - args = mock_db.call_args[0] - self.assertEqual('fake_host', args[1]) + # Check the db.service_get was called with correct arguments. + mock_db.assert_called_once_with( + mock.ANY, host='fake_host', binary='cinder-volume') # Check the create_snapshot_in_db was called with correct arguments. self.assertEqual(1, mock_create_snapshot.call_count) diff --git a/cinder/tests/unit/api/contrib/test_volume_manage.py b/cinder/tests/unit/api/contrib/test_volume_manage.py index 22f67595d29..6d475aae291 100644 --- a/cinder/tests/unit/api/contrib/test_volume_manage.py +++ b/cinder/tests/unit/api/contrib/test_volume_manage.py @@ -40,13 +40,13 @@ def app(): return mapper -def db_service_get_by_host_and_topic(context, host, topic): - """Replacement for db.service_get_by_host_and_topic. +def service_get_by_host_and_topic(context, host, topic): + """Replacement for Service.service_get_by_host_and_topic. - We stub the db.service_get_by_host_and_topic method to return something - for a specific host, and raise an exception for anything else. We don't - use the returned data (the code under test just use the call to check for - existence of a host, so the content returned doesn't matter. + We mock the Service.service_get_by_host_and_topic method to return + something for a specific host, and raise an exception for anything else. + We don't use the returned data (the code under test just use the call to + check for existence of a host, so the content returned doesn't matter. """ if host == 'host_ok': return {} @@ -126,8 +126,8 @@ def api_get_manageable_volumes(*args, **kwargs): return vols -@mock.patch('cinder.db.service_get_by_host_and_topic', - db_service_get_by_host_and_topic) +@mock.patch('cinder.objects.service.Service.get_by_host_and_topic', + service_get_by_host_and_topic) @mock.patch('cinder.volume.volume_types.get_volume_type_by_name', vt_get_volume_type_by_name) @mock.patch('cinder.volume.volume_types.get_volume_type', diff --git a/cinder/tests/unit/api/v1/stubs.py b/cinder/tests/unit/api/v1/stubs.py index 80a4db480cd..628d3f1368d 100644 --- a/cinder/tests/unit/api/v1/stubs.py +++ b/cinder/tests/unit/api/v1/stubs.py @@ -190,7 +190,7 @@ def stub_snapshot_update(self, context, *args, **param): pass -def stub_service_get_all_by_topic(context, topic, disabled=None): +def stub_service_get_all(context, **filters): return [{'availability_zone': "zone1:host1", "disabled": 0}] diff --git a/cinder/tests/unit/api/v1/test_volume_metadata.py b/cinder/tests/unit/api/v1/test_volume_metadata.py index 31fcc32a42b..bf049b10031 100644 --- a/cinder/tests/unit/api/v1/test_volume_metadata.py +++ b/cinder/tests/unit/api/v1/test_volume_metadata.py @@ -145,8 +145,10 @@ class volumeMetaDataTest(test.TestCase): self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) self.stubs.Set(cinder.db, 'volume_metadata_get', return_volume_metadata) - self.stubs.Set(cinder.db, 'service_get_all_by_topic', - stubs.stub_service_get_all_by_topic) + self.patch( + 'cinder.db.service_get_all', autospec=True, + return_value=stubs.stub_service_get_all(None)) + self.ext_mgr = extensions.ExtensionManager() self.ext_mgr.extensions = {} self.volume_controller = volumes.VolumeController(self.ext_mgr) diff --git a/cinder/tests/unit/api/v1/test_volumes.py b/cinder/tests/unit/api/v1/test_volumes.py index f941759d78a..9b86723a15f 100644 --- a/cinder/tests/unit/api/v1/test_volumes.py +++ b/cinder/tests/unit/api/v1/test_volumes.py @@ -48,8 +48,9 @@ class VolumeApiTest(test.TestCase): self.controller = volumes.VolumeController(self.ext_mgr) self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all) - self.stubs.Set(db, 'service_get_all_by_topic', - stubs.stub_service_get_all_by_topic) + self.patch( + 'cinder.db.service_get_all', autospec=True, + return_value=stubs.stub_service_get_all(None)) self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete) def test_volume_create(self): diff --git a/cinder/tests/unit/api/v2/stubs.py b/cinder/tests/unit/api/v2/stubs.py index daa90ab2576..dd19f7d8322 100644 --- a/cinder/tests/unit/api/v2/stubs.py +++ b/cinder/tests/unit/api/v2/stubs.py @@ -220,6 +220,10 @@ def stub_snapshot_update(self, context, *args, **param): pass +def stub_service_get_all(*args, **kwargs): + return [{'availability_zone': "zone1:host1", "disabled": 0}] + + def stub_service_get_all_by_topic(context, topic, disabled=None): return [{'availability_zone': "zone1:host1", "disabled": 0}] diff --git a/cinder/tests/unit/api/v2/test_volume_metadata.py b/cinder/tests/unit/api/v2/test_volume_metadata.py index 80e9c85d346..a8feb1bb860 100644 --- a/cinder/tests/unit/api/v2/test_volume_metadata.py +++ b/cinder/tests/unit/api/v2/test_volume_metadata.py @@ -132,8 +132,9 @@ class volumeMetaDataTest(test.TestCase): self.stubs.Set(volume.api.API, 'get', get_volume) self.stubs.Set(db, 'volume_metadata_get', return_volume_metadata) - self.stubs.Set(db, 'service_get_all_by_topic', - stubs.stub_service_get_all_by_topic) + self.patch( + 'cinder.db.service_get_all', autospec=True, + return_value=stubs.stub_service_get_all_by_topic(None, None)) self.stubs.Set(self.volume_api, 'update_volume_metadata', fake_update_volume_metadata) diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 903c16ed826..acdd242053d 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -59,8 +59,9 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all) self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete) - self.stubs.Set(db, 'service_get_all_by_topic', - stubs.stub_service_get_all_by_topic) + self.patch( + 'cinder.db.service_get_all', autospec=True, + return_value=stubs.stub_service_get_all_by_topic(None, None)) self.maxDiff = None self.ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True) diff --git a/cinder/tests/unit/objects/test_service.py b/cinder/tests/unit/objects/test_service.py index 57586d8d1a9..e0e97ca9f5e 100644 --- a/cinder/tests/unit/objects/test_service.py +++ b/cinder/tests/unit/objects/test_service.py @@ -30,25 +30,25 @@ class TestService(test_objects.BaseObjectsTestCase): self._compare(self, db_service, service) service_get.assert_called_once_with(self.context, 1) - @mock.patch('cinder.db.service_get_by_host_and_topic') - def test_get_by_host_and_topic(self, service_get_by_host_and_topic): + @mock.patch('cinder.db.service_get') + def test_get_by_host_and_topic(self, service_get): db_service = fake_service.fake_db_service() - service_get_by_host_and_topic.return_value = db_service + service_get.return_value = db_service service = objects.Service.get_by_host_and_topic( self.context, 'fake-host', 'fake-topic') self._compare(self, db_service, service) - service_get_by_host_and_topic.assert_called_once_with( - self.context, 'fake-host', 'fake-topic') + service_get.assert_called_once_with( + self.context, disabled=False, host='fake-host', topic='fake-topic') - @mock.patch('cinder.db.service_get_by_args') - def test_get_by_args(self, service_get_by_args): + @mock.patch('cinder.db.service_get') + def test_get_by_args(self, service_get): db_service = fake_service.fake_db_service() - service_get_by_args.return_value = db_service + service_get.return_value = db_service service = objects.Service.get_by_args( self.context, 'fake-host', 'fake-key') self._compare(self, db_service, service) - service_get_by_args.assert_called_once_with( - self.context, 'fake-host', 'fake-key') + service_get.assert_called_once_with( + self.context, host='fake-host', binary='fake-key') @mock.patch('cinder.db.service_create') def test_create(self, service_create): @@ -102,21 +102,21 @@ class TestService(test_objects.BaseObjectsTestCase): call_bool, mock.call(self.context, 123)]) - @mock.patch('cinder.db.service_get_all_by_binary') + @mock.patch('cinder.db.service_get_all') def _test_get_minimum_version(self, services_update, expected, - service_get_all_by_binary): + service_get_all): services = [fake_service.fake_db_service(**s) for s in services_update] - service_get_all_by_binary.return_value = services + service_get_all.return_value = services min_rpc = objects.Service.get_minimum_rpc_version(self.context, 'foo') self.assertEqual(expected[0], min_rpc) min_obj = objects.Service.get_minimum_obj_version(self.context, 'foo') self.assertEqual(expected[1], min_obj) - service_get_all_by_binary.assert_has_calls( - [mock.call(self.context, 'foo', disabled=None)] * 2) + service_get_all.assert_has_calls( + [mock.call(self.context, binary='foo', disabled=None)] * 2) - @mock.patch('cinder.db.service_get_all_by_binary') - def test_get_minimum_version(self, service_get_all_by_binary): + @mock.patch('cinder.db.service_get_all') + def test_get_minimum_version(self, service_get_all): services_update = [ {'rpc_current_version': '1.0', 'object_current_version': '1.3'}, {'rpc_current_version': '1.1', 'object_current_version': '1.2'}, @@ -125,8 +125,8 @@ class TestService(test_objects.BaseObjectsTestCase): expected = ('1.0', '1.2') self._test_get_minimum_version(services_update, expected) - @mock.patch('cinder.db.service_get_all_by_binary') - def test_get_minimum_version_liberty(self, service_get_all_by_binary): + @mock.patch('cinder.db.service_get_all') + def test_get_minimum_version_liberty(self, service_get_all): services_update = [ {'rpc_current_version': '1.0', 'object_current_version': '1.3'}, {'rpc_current_version': '1.1', 'object_current_version': None}, @@ -144,30 +144,30 @@ class TestServiceList(test_objects.BaseObjectsTestCase): filters = {'host': 'host', 'binary': 'foo', 'disabled': False} services = objects.ServiceList.get_all(self.context, filters) - service_get_all.assert_called_once_with(self.context, filters) + service_get_all.assert_called_once_with(self.context, **filters) self.assertEqual(1, len(services)) TestService._compare(self, db_service, services[0]) - @mock.patch('cinder.db.service_get_all_by_topic') - def test_get_all_by_topic(self, service_get_all_by_topic): + @mock.patch('cinder.db.service_get_all') + def test_get_all_by_topic(self, service_get_all): db_service = fake_service.fake_db_service() - service_get_all_by_topic.return_value = [db_service] + service_get_all.return_value = [db_service] services = objects.ServiceList.get_all_by_topic( self.context, 'foo', 'bar') - service_get_all_by_topic.assert_called_once_with( - self.context, 'foo', disabled='bar') + service_get_all.assert_called_once_with( + self.context, topic='foo', disabled='bar') self.assertEqual(1, len(services)) TestService._compare(self, db_service, services[0]) - @mock.patch('cinder.db.service_get_all_by_binary') - def test_get_all_by_binary(self, service_get_all_by_binary): + @mock.patch('cinder.db.service_get_all') + def test_get_all_by_binary(self, service_get_all): db_service = fake_service.fake_db_service() - service_get_all_by_binary.return_value = [db_service] + service_get_all.return_value = [db_service] services = objects.ServiceList.get_all_by_binary( self.context, 'foo', 'bar') - service_get_all_by_binary.assert_called_once_with( - self.context, 'foo', disabled='bar') + service_get_all.assert_called_once_with( + self.context, binary='foo', disabled='bar') self.assertEqual(1, len(services)) TestService._compare(self, db_service, services[0]) diff --git a/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py b/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py index f01c9e8bb5f..e1ca2451f8b 100644 --- a/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py +++ b/cinder/tests/unit/scheduler/test_allocated_capacity_weigher.py @@ -43,14 +43,14 @@ class AllocatedCapacityWeigherTestCase(test.TestCase): [weights.capacity.AllocatedCapacityWeigher], hosts, weight_properties)[0] - @mock.patch('cinder.db.sqlalchemy.api.service_get_all_by_topic') - def _get_all_hosts(self, _mock_service_get_all_by_topic, disabled=False): + @mock.patch('cinder.db.sqlalchemy.api.service_get_all') + def _get_all_hosts(self, _mock_service_get_all, disabled=False): ctxt = context.get_admin_context() - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic, + fakes.mock_host_manager_db_calls(_mock_service_get_all, disabled=disabled) host_states = self.host_manager.get_all_host_states(ctxt) - _mock_service_get_all_by_topic.assert_called_once_with( - ctxt, CONF.volume_topic, disabled=disabled) + _mock_service_get_all.assert_called_once_with( + ctxt, topic=CONF.volume_topic, disabled=disabled) return host_states def test_default_of_spreading_first(self): diff --git a/cinder/tests/unit/scheduler/test_capacity_weigher.py b/cinder/tests/unit/scheduler/test_capacity_weigher.py index fb6826a655e..c762abb03e5 100644 --- a/cinder/tests/unit/scheduler/test_capacity_weigher.py +++ b/cinder/tests/unit/scheduler/test_capacity_weigher.py @@ -43,14 +43,14 @@ class CapacityWeigherTestCase(test.TestCase): hosts, weight_properties) - @mock.patch('cinder.db.sqlalchemy.api.service_get_all_by_topic') - def _get_all_hosts(self, _mock_service_get_all_by_topic, disabled=False): + @mock.patch('cinder.db.sqlalchemy.api.service_get_all') + def _get_all_hosts(self, _mock_service_get_all, disabled=False): ctxt = context.get_admin_context() - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic, + fakes.mock_host_manager_db_calls(_mock_service_get_all, disabled=disabled) host_states = self.host_manager.get_all_host_states(ctxt) - _mock_service_get_all_by_topic.assert_called_once_with( - ctxt, CONF.volume_topic, disabled=disabled) + _mock_service_get_all.assert_called_once_with( + ctxt, topic=CONF.volume_topic, disabled=disabled) return host_states # If thin_provisioning_support = False, use the following formula: diff --git a/cinder/tests/unit/scheduler/test_filter_scheduler.py b/cinder/tests/unit/scheduler/test_filter_scheduler.py index c1e3cd3df92..b1c896d0925 100644 --- a/cinder/tests/unit/scheduler/test_filter_scheduler.py +++ b/cinder/tests/unit/scheduler/test_filter_scheduler.py @@ -50,16 +50,16 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): sched.schedule_create_consistencygroup, fake_context, 'faki-id1', request_spec_list, {}) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_schedule_consistencygroup(self, - _mock_service_get_all_by_topic): + _mock_service_get_all): # Make sure _schedule_group() can find host successfully. sched = fakes.FakeFilterScheduler() sched.host_manager = fakes.FakeHostManager() fake_context = context.RequestContext('user', 'project', is_admin=True) - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) + fakes.mock_host_manager_db_calls(_mock_service_get_all) specs = {'capabilities:consistencygroup_support': ' True'} request_spec = {'volume_properties': {'project_id': 1, @@ -75,12 +75,12 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec_list, {}) self.assertIsNotNone(weighed_host.obj) - self.assertTrue(_mock_service_get_all_by_topic.called) + self.assertTrue(_mock_service_get_all.called) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_schedule_consistencygroup_no_cg_support_in_extra_specs( self, - _mock_service_get_all_by_topic): + _mock_service_get_all): # Make sure _schedule_group() can find host successfully even # when consistencygroup_support is not specified in volume type's # extra specs @@ -89,7 +89,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context = context.RequestContext('user', 'project', is_admin=True) - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) + fakes.mock_host_manager_db_calls(_mock_service_get_all) request_spec = {'volume_properties': {'project_id': 1, 'size': 0}, @@ -104,7 +104,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec_list, {}) self.assertIsNotNone(weighed_host.obj) - self.assertTrue(_mock_service_get_all_by_topic.called) + self.assertTrue(_mock_service_get_all.called) def test_create_volume_no_hosts(self): # Ensure empty hosts/child_zones result in NoValidHosts exception. @@ -174,8 +174,8 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context, request_spec, {}) self.assertTrue(self.was_admin) - @mock.patch('cinder.db.service_get_all_by_topic') - def test_schedule_happy_day(self, _mock_service_get_all_by_topic): + @mock.patch('cinder.db.service_get_all') + def test_schedule_happy_day(self, _mock_service_get_all): # Make sure there's nothing glaringly wrong with _schedule() # by doing a happy day pass through. sched = fakes.FakeFilterScheduler() @@ -183,16 +183,16 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): fake_context = context.RequestContext('user', 'project', is_admin=True) - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic) + fakes.mock_host_manager_db_calls(_mock_service_get_all) request_spec = {'volume_type': {'name': 'LVM_iSCSI'}, 'volume_properties': {'project_id': 1, 'size': 1}} weighed_host = sched._schedule(fake_context, request_spec, {}) self.assertIsNotNone(weighed_host.obj) - self.assertTrue(_mock_service_get_all_by_topic.called) + self.assertTrue(_mock_service_get_all.called) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_volume_clear_host_different_with_cg(self, _mock_service_get_all): # Ensure we clear those hosts whose backend is not same as @@ -208,7 +208,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): weighed_host = sched._schedule(fake_context, request_spec, {}) self.assertIsNone(weighed_host) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_create_volume_host_same_as_cg(self, _mock_service_get_all): # Ensure we don't clear the host whose backend is same as # consistencygroup's backend. @@ -338,7 +338,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): return (sched, fake_context) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_host_passes_filters_happy_day(self, _mock_service_get_topic): """Do a successful pass through of with host_passes_filters().""" sched, ctx = self._host_passes_filters_setup( @@ -352,7 +352,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): self.assertEqual('host1', utils.extract_host(ret_host.host)) self.assertTrue(_mock_service_get_topic.called) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_host_passes_filters_default_pool_happy_day( self, _mock_service_get_topic): """Do a successful pass through of with host_passes_filters().""" @@ -367,7 +367,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): self.assertEqual('host5', utils.extract_host(ret_host.host)) self.assertTrue(_mock_service_get_topic.called) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_host_passes_filters_no_capacity(self, _mock_service_get_topic): """Fail the host due to insufficient capacity.""" sched, ctx = self._host_passes_filters_setup( @@ -381,7 +381,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): ctx, 'host1#lvm1', request_spec, {}) self.assertTrue(_mock_service_get_topic.called) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_retype_policy_never_migrate_pass(self, _mock_service_get_topic): # Retype should pass if current host passes filters and # policy=never. host4 doesn't have enough space to hold an additional @@ -401,7 +401,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): migration_policy='never') self.assertEqual('host4', utils.extract_host(host_state.host)) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_retype_with_pool_policy_never_migrate_pass( self, _mock_service_get_topic): # Retype should pass if current host passes filters and @@ -422,7 +422,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): migration_policy='never') self.assertEqual('host3#lvm3', host_state.host) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_retype_policy_never_migrate_fail(self, _mock_service_get_topic): # Retype should fail if current host doesn't pass filters and # policy=never. @@ -439,7 +439,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): request_spec, filter_properties={}, migration_policy='never') - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_retype_policy_demand_migrate_pass(self, _mock_service_get_topic): # Retype should pass if current host fails filters but another host # is suitable when policy=on-demand. @@ -457,7 +457,7 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): migration_policy='on-demand') self.assertEqual('host1', utils.extract_host(host_state.host)) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') def test_retype_policy_demand_migrate_fail(self, _mock_service_get_topic): # Retype should fail if current host doesn't pass filters and # no other suitable candidates exist even if policy=on-demand. diff --git a/cinder/tests/unit/scheduler/test_host_manager.py b/cinder/tests/unit/scheduler/test_host_manager.py index f1f99e8e29c..9c45d7839b7 100644 --- a/cinder/tests/unit/scheduler/test_host_manager.py +++ b/cinder/tests/unit/scheduler/test_host_manager.py @@ -121,8 +121,8 @@ class HostManagerTestCase(test.TestCase): self.assertDictMatch(expected, service_states) @mock.patch('cinder.utils.service_is_up') - @mock.patch('cinder.db.service_get_all_by_topic') - def test_has_all_capabilities(self, _mock_service_get_all_by_topic, + @mock.patch('cinder.db.service_get_all') + def test_has_all_capabilities(self, _mock_service_get_all, _mock_service_is_up): _mock_service_is_up.return_value = True services = [ @@ -133,8 +133,8 @@ class HostManagerTestCase(test.TestCase): dict(id=3, host='host3', topic='volume', disabled=False, availability_zone='zone1', updated_at=timeutils.utcnow()), ] - _mock_service_get_all_by_topic.return_value = services - # Create host_manager again to let db.service_get_all_by_topic mock run + _mock_service_get_all.return_value = services + # Create host_manager again to let db.service_get_all mock run self.host_manager = host_manager.HostManager() self.assertFalse(self.host_manager.has_all_capabilities()) @@ -153,12 +153,12 @@ class HostManagerTestCase(test.TestCase): host3_volume_capabs) self.assertTrue(self.host_manager.has_all_capabilities()) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.utils.service_is_up') @mock.patch('oslo_utils.timeutils.utcnow') def test_update_and_get_pools(self, _mock_utcnow, _mock_service_is_up, - _mock_service_get_all_by_topic): + _mock_service_get_all): """Test interaction between update and get_pools This test verifies that each time that get_pools is called it gets the @@ -182,7 +182,7 @@ class HostManagerTestCase(test.TestCase): timestamp=None, reserved_percentage=0), } - _mock_service_get_all_by_topic.return_value = services + _mock_service_get_all.return_value = services _mock_service_is_up.return_value = True _mock_warning = mock.Mock() host_manager.LOG.warn = _mock_warning @@ -206,10 +206,10 @@ class HostManagerTestCase(test.TestCase): self.assertEqual(1, len(res)) self.assertEqual(dates[2], res[0]['capabilities']['timestamp']) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.utils.service_is_up') def test_get_all_host_states(self, _mock_service_is_up, - _mock_service_get_all_by_topic): + _mock_service_get_all): context = 'fake_context' topic = CONF.volume_topic @@ -257,16 +257,16 @@ class HostManagerTestCase(test.TestCase): # First test: service_is_up is always True, host5 is disabled, # host4 has no capabilities self.host_manager.service_states = service_states - _mock_service_get_all_by_topic.return_value = services + _mock_service_get_all.return_value = services _mock_service_is_up.return_value = True _mock_warning = mock.Mock() host_manager.LOG.warning = _mock_warning # Get all states self.host_manager.get_all_host_states(context) - _mock_service_get_all_by_topic.assert_called_with(context, - topic, - disabled=False) + _mock_service_get_all.assert_called_with(context, + disabled=False, + topic=topic) expected = [] for service in service_objs: expected.append(mock.call(service)) @@ -284,14 +284,14 @@ class HostManagerTestCase(test.TestCase): # Second test: Now service_is_up returns False for host3 _mock_service_is_up.reset_mock() _mock_service_is_up.side_effect = [True, True, False, True] - _mock_service_get_all_by_topic.reset_mock() + _mock_service_get_all.reset_mock() _mock_warning.reset_mock() # Get all states, make sure host 3 is reported as down self.host_manager.get_all_host_states(context) - _mock_service_get_all_by_topic.assert_called_with(context, - topic, - disabled=False) + _mock_service_get_all.assert_called_with(context, + disabled=False, + topic=topic) self.assertEqual(expected, _mock_service_is_up.call_args_list) self.assertGreater(_mock_warning.call_count, 0) @@ -306,10 +306,10 @@ class HostManagerTestCase(test.TestCase): test_service.TestService._compare(self, volume_node, host_state_map[host].service) - @mock.patch('cinder.db.service_get_all_by_topic') + @mock.patch('cinder.db.service_get_all') @mock.patch('cinder.utils.service_is_up') def test_get_pools(self, _mock_service_is_up, - _mock_service_get_all_by_topic): + _mock_service_get_all): context = 'fake_context' services = [ @@ -336,7 +336,7 @@ class HostManagerTestCase(test.TestCase): provisioned_capacity_gb=9300), } - _mock_service_get_all_by_topic.return_value = services + _mock_service_get_all.return_value = services _mock_service_is_up.return_value = True _mock_warning = mock.Mock() host_manager.LOG.warn = _mock_warning diff --git a/cinder/tests/unit/scheduler/test_volume_number_weigher.py b/cinder/tests/unit/scheduler/test_volume_number_weigher.py index 5512470c7fe..9982cdcde9d 100644 --- a/cinder/tests/unit/scheduler/test_volume_number_weigher.py +++ b/cinder/tests/unit/scheduler/test_volume_number_weigher.py @@ -69,14 +69,14 @@ class VolumeNumberWeigherTestCase(test.TestCase): hosts, weight_properties)[0] - @mock.patch('cinder.db.sqlalchemy.api.service_get_all_by_topic') - def _get_all_hosts(self, _mock_service_get_all_by_topic, disabled=False): + @mock.patch('cinder.db.sqlalchemy.api.service_get_all') + def _get_all_hosts(self, _mock_service_get_all, disabled=False): ctxt = context.get_admin_context() - fakes.mock_host_manager_db_calls(_mock_service_get_all_by_topic, + fakes.mock_host_manager_db_calls(_mock_service_get_all, disabled=disabled) host_states = self.host_manager.get_all_host_states(ctxt) - _mock_service_get_all_by_topic.assert_called_once_with( - ctxt, CONF.volume_topic, disabled=disabled) + _mock_service_get_all.assert_called_once_with( + ctxt, topic=CONF.volume_topic, disabled=disabled) return host_states def test_volume_number_weight_multiplier1(self): diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index 7de6375dd11..13fd5feb307 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -439,7 +439,7 @@ class TestCinderManageCmd(test.TestCase): host_cmds.list() get_admin_context.assert_called_once_with() - service_get_all.assert_called_once_with(mock.sentinel.ctxt, None) + service_get_all.assert_called_once_with(mock.sentinel.ctxt) self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.db.service_get_all') @@ -462,7 +462,7 @@ class TestCinderManageCmd(test.TestCase): host_cmds.list(zone='fake-az1') get_admin_context.assert_called_once_with() - service_get_all.assert_called_once_with(mock.sentinel.ctxt, None) + service_get_all.assert_called_once_with(mock.sentinel.ctxt) self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.objects.base.CinderObjectSerializer') @@ -747,7 +747,7 @@ class TestCinderManageCmd(test.TestCase): self.assertEqual(expected_out, fake_out.getvalue()) get_admin_context.assert_called_with() - service_get_all.assert_called_with(ctxt, None) + service_get_all.assert_called_with(ctxt) def test_service_commands_list(self): service = {'binary': 'cinder-binary', @@ -858,8 +858,7 @@ class TestCinderManageCmd(test.TestCase): self.assertEqual(2, exit) @mock.patch('cinder.db.service_destroy') - @mock.patch('cinder.db.service_get_by_args', - return_value = {'id': '12'}) + @mock.patch('cinder.db.service_get', return_value = {'id': '12'}) def test_remove_service_success(self, mock_get_by_args, mock_service_destroy): service_commands = cinder_manage.ServiceCommands() diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 7e60bcad1bf..d2132699904 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -18,6 +18,7 @@ import datetime import enum import mock +from oslo_config import cfg from oslo_utils import uuidutils import six @@ -33,6 +34,7 @@ from cinder import test from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils +CONF = cfg.CONF THREE = 3 THREE_HUNDREDS = 300 ONE_HUNDREDS = 100 @@ -127,7 +129,12 @@ class DBAPIServiceTestCase(BaseTest): def _create_service(self, values): v = self._get_base_values() v.update(values) - return db.service_create(self.ctxt, v) + service = db.service_create(self.ctxt, v) + # We need to read the contents from the DB if we have set updated_at + # or created_at fields + if 'updated_at' in values or 'created_at' in values: + service = db.service_get(self.ctxt, service.id) + return service def test_service_create(self): service = self._create_service({}) @@ -142,8 +149,9 @@ class DBAPIServiceTestCase(BaseTest): db.service_destroy(self.ctxt, service1['id']) self.assertRaises(exception.ServiceNotFound, db.service_get, self.ctxt, service1['id']) - self._assertEqualObjects(db.service_get(self.ctxt, service2['id']), - service2) + self._assertEqualObjects( + service2, + db.service_get(self.ctxt, service2['id'])) def test_service_update(self): service = self._create_service({}) @@ -175,33 +183,35 @@ class DBAPIServiceTestCase(BaseTest): def test_service_get_by_host_and_topic(self): service1 = self._create_service({'host': 'host1', 'topic': 'topic1'}) - real_service1 = db.service_get_by_host_and_topic(self.ctxt, - host='host1', - topic='topic1') + real_service1 = db.service_get(self.ctxt, host='host1', topic='topic1') self._assertEqualObjects(service1, real_service1) def test_service_get_all(self): + expired = (datetime.datetime.utcnow() + - datetime.timedelta(seconds=CONF.service_down_time + 1)) values = [ - {'host': 'host1', 'binary': 'b1'}, + {'host': 'host1', 'binary': 'b1', 'created_at': expired}, {'host': 'host1@ceph', 'binary': 'b2'}, {'host': 'host2', 'binary': 'b2'}, - {'disabled': True} + {'disabled': True, 'created_at': expired, 'updated_at': expired} ] services = [self._create_service(vals) for vals in values] - disabled_services = [services[-1]] + disabled_services = services[-1:] non_disabled_services = services[:-1] + up_services = services[1:3] + down_services = [services[0], services[3]] expected = services[:2] expected_bin = services[1:3] compares = [ - (services, db.service_get_all(self.ctxt, {})), (services, db.service_get_all(self.ctxt)), - (expected, db.service_get_all(self.ctxt, {'host': 'host1'})), - (expected_bin, db.service_get_all(self.ctxt, {'binary': 'b2'})), - (disabled_services, db.service_get_all(self.ctxt, - {'disabled': True})), + (expected, db.service_get_all(self.ctxt, host='host1')), + (expected_bin, db.service_get_all(self.ctxt, binary='b2')), + (disabled_services, db.service_get_all(self.ctxt, disabled=True)), (non_disabled_services, db.service_get_all(self.ctxt, - {'disabled': False})), + disabled=False)), + (up_services, db.service_get_all(self.ctxt, is_up=True)), + (down_services, db.service_get_all(self.ctxt, is_up=False)), ] for comp in compares: self._assertEqualListsOfObjects(*comp) @@ -215,7 +225,7 @@ class DBAPIServiceTestCase(BaseTest): ] services = [self._create_service(vals) for vals in values] expected = services[:3] - real = db.service_get_all_by_topic(self.ctxt, 't1') + real = db.service_get_all(self.ctxt, topic='t1') self._assertEqualListsOfObjects(expected, real) def test_service_get_all_by_binary(self): @@ -227,7 +237,7 @@ class DBAPIServiceTestCase(BaseTest): ] services = [self._create_service(vals) for vals in values] expected = services[:3] - real = db.service_get_all_by_binary(self.ctxt, 'b1') + real = db.service_get_all(self.ctxt, binary='b1') self._assertEqualListsOfObjects(expected, real) def test_service_get_by_args(self): @@ -237,58 +247,30 @@ class DBAPIServiceTestCase(BaseTest): ] services = [self._create_service(vals) for vals in values] - service1 = db.service_get_by_args(self.ctxt, 'host1', 'a') + service1 = db.service_get(self.ctxt, host='host1', binary='a') self._assertEqualObjects(services[0], service1) - service2 = db.service_get_by_args(self.ctxt, 'host2', 'b') + service2 = db.service_get(self.ctxt, host='host2', binary='b') self._assertEqualObjects(services[1], service2) def test_service_get_by_args_not_found_exception(self): self.assertRaises(exception.ServiceNotFound, - db.service_get_by_args, - self.ctxt, 'non-exists-host', 'a') + db.service_get, + self.ctxt, host='non-exists-host', binary='a') - @mock.patch('cinder.db.sqlalchemy.api.model_query') - def test_service_get_by_args_with_case_insensitive(self, model_query): - class case_insensitive_filter(object): - def __init__(self, records): - self.records = records + @mock.patch('sqlalchemy.orm.query.Query.filter_by') + def test_service_get_by_args_with_case_insensitive(self, filter_by): + CONF.set_default('connection', 'mysql://', 'database') + db.service_get(self.ctxt, host='host', binary='a') - def filter_by(self, **kwargs): - ret = mock.Mock() - ret.all = mock.Mock() - - results = [] - for record in self.records: - for key, value in kwargs.items(): - if record[key].lower() != value.lower(): - break - else: - results.append(record) - - ret.filter_by = case_insensitive_filter(results).filter_by - ret.all.return_value = results - return ret - - values = [ - {'host': 'host', 'binary': 'a'}, - {'host': 'HOST', 'binary': 'a'} - ] - services = [self._create_service(vals) for vals in values] - - query = mock.Mock() - query.filter_by = case_insensitive_filter(services).filter_by - model_query.return_value = query - - service1 = db.service_get_by_args(self.ctxt, 'host', 'a') - self._assertEqualObjects(services[0], service1) - - service2 = db.service_get_by_args(self.ctxt, 'HOST', 'a') - self._assertEqualObjects(services[1], service2) - - self.assertRaises(exception.ServiceNotFound, - db.service_get_by_args, - self.ctxt, 'Host', 'a') + self.assertNotEqual(0, filter_by.call_count) + self.assertEqual(1, filter_by.return_value.filter.call_count) + or_op = filter_by.return_value.filter.call_args[0][0].clauses[0] + self.assertIsInstance(or_op, + sqlalchemy_api.sql.elements.BinaryExpression) + binary_op = or_op.right + self.assertIsInstance(binary_op, sqlalchemy_api.sql.functions.Function) + self.assertEqual('binary', binary_op.name) class DBAPIVolumeTestCase(BaseTest): diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py index c41bbf2c2a8..8ce0b306962 100644 --- a/cinder/tests/unit/test_service.py +++ b/cinder/tests/unit/test_service.py @@ -128,6 +128,12 @@ class ServiceTestCase(test.TestCase): self.host = 'foo' self.binary = 'cinder-fake' self.topic = 'fake' + self.service_ref = {'host': self.host, + 'binary': self.binary, + 'topic': self.topic, + 'report_count': 0, + 'availability_zone': 'nova', + 'id': 1} def test_create(self): # NOTE(vish): Create was moved out of mock replay to make sure that @@ -143,17 +149,13 @@ class ServiceTestCase(test.TestCase): # Check that the entry has been really created in the DB objects.Service.get_by_id(context.get_admin_context(), app.service_id) - def test_report_state_newly_disconnected(self): - service_ref = {'host': self.host, - 'binary': self.binary, - 'topic': self.topic, - 'report_count': 0, - 'availability_zone': 'nova', - 'id': 1} + @mock.patch.object(objects.service.Service, 'get_by_args') + @mock.patch.object(objects.service.Service, 'get_by_id') + def test_report_state_newly_disconnected(self, get_by_id, get_by_args): + get_by_args.side_effect = exception.NotFound() + get_by_id.side_effect = db_exc.DBConnectionError() with mock.patch.object(objects.service, 'db') as mock_db: - mock_db.service_get_by_args.side_effect = exception.NotFound() - mock_db.service_create.return_value = service_ref - mock_db.service_get.side_effect = db_exc.DBConnectionError() + mock_db.service_create.return_value = self.service_ref serv = service.Service( self.host, @@ -166,17 +168,13 @@ class ServiceTestCase(test.TestCase): self.assertTrue(serv.model_disconnected) self.assertFalse(mock_db.service_update.called) - def test_report_state_disconnected_DBError(self): - service_ref = {'host': self.host, - 'binary': self.binary, - 'topic': self.topic, - 'report_count': 0, - 'availability_zone': 'nova', - 'id': 1} + @mock.patch.object(objects.service.Service, 'get_by_args') + @mock.patch.object(objects.service.Service, 'get_by_id') + def test_report_state_disconnected_DBError(self, get_by_id, get_by_args): + get_by_args.side_effect = exception.NotFound() + get_by_id.side_effect = db_exc.DBError() with mock.patch.object(objects.service, 'db') as mock_db: - mock_db.service_get_by_args.side_effect = exception.NotFound() - mock_db.service_create.return_value = service_ref - mock_db.service_get.side_effect = db_exc.DBError() + mock_db.service_create.return_value = self.service_ref serv = service.Service( self.host, @@ -189,41 +187,27 @@ class ServiceTestCase(test.TestCase): self.assertTrue(serv.model_disconnected) self.assertFalse(mock_db.service_update.called) - def test_report_state_newly_connected(self): - service_ref = {'host': self.host, - 'binary': self.binary, - 'topic': self.topic, - 'report_count': 0, - 'availability_zone': 'nova', - 'id': 1} - with mock.patch.object(objects.service, 'db') as mock_db,\ - mock.patch('cinder.db.sqlalchemy.api.get_by_id') as get_by_id: - mock_db.service_get_by_args.side_effect = exception.NotFound() - mock_db.service_create.return_value = service_ref - get_by_id.return_value = service_ref + @mock.patch('cinder.db.sqlalchemy.api.service_update') + @mock.patch('cinder.db.sqlalchemy.api.service_get') + def test_report_state_newly_connected(self, get_by_id, service_update): + get_by_id.return_value = self.service_ref - serv = service.Service( - self.host, - self.binary, - self.topic, - 'cinder.tests.unit.test_service.FakeManager' - ) - serv.start() - serv.model_disconnected = True - serv.report_state() + serv = service.Service( + self.host, + self.binary, + self.topic, + 'cinder.tests.unit.test_service.FakeManager' + ) + serv.start() + serv.model_disconnected = True + serv.report_state() - self.assertFalse(serv.model_disconnected) - self.assertTrue(mock_db.service_update.called) + self.assertFalse(serv.model_disconnected) + self.assertTrue(service_update.called) def test_report_state_manager_not_working(self): - service_ref = {'host': self.host, - 'binary': self.binary, - 'topic': self.topic, - 'report_count': 0, - 'availability_zone': 'nova', - 'id': 1} with mock.patch('cinder.db') as mock_db: - mock_db.service_get.return_value = service_ref + mock_db.service_get.return_value = self.service_ref serv = service.Service( self.host, diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index d1583eb0a4e..e003c262e9e 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -240,105 +240,71 @@ class BaseVolumeTestCase(test.TestCase): class AvailabilityZoneTestCase(BaseVolumeTestCase): + def setUp(self): + super(AvailabilityZoneTestCase, self).setUp() + self.get_all = self.patch( + 'cinder.db.service_get_all', autospec=True, + return_value = [{'availability_zone': 'a', 'disabled': False}]) + def test_list_availability_zones_cached(self): - volume_api = cinder.volume.api.API() - with mock.patch.object(volume_api.db, - 'service_get_all_by_topic') as get_all: - get_all.return_value = [ - { - 'availability_zone': 'a', - 'disabled': False, - }, - ] - azs = volume_api.list_availability_zones(enable_cache=True) - self.assertEqual([{"name": 'a', 'available': True}], list(azs)) - self.assertIsNotNone(volume_api.availability_zones_last_fetched) - self.assertTrue(get_all.called) - volume_api.list_availability_zones(enable_cache=True) - self.assertEqual(1, get_all.call_count) + azs = self.volume_api.list_availability_zones(enable_cache=True) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNotNone(self.volume_api.availability_zones_last_fetched) + self.assertTrue(self.get_all.called) + self.volume_api.list_availability_zones(enable_cache=True) + self.assertEqual(1, self.get_all.call_count) def test_list_availability_zones_no_cached(self): - volume_api = cinder.volume.api.API() - with mock.patch.object(volume_api.db, - 'service_get_all_by_topic') as get_all: - get_all.return_value = [ - { - 'availability_zone': 'a', - 'disabled': False, - }, - ] - azs = volume_api.list_availability_zones(enable_cache=False) - self.assertEqual([{"name": 'a', 'available': True}], list(azs)) - self.assertIsNone(volume_api.availability_zones_last_fetched) + azs = self.volume_api.list_availability_zones(enable_cache=False) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNone(self.volume_api.availability_zones_last_fetched) - with mock.patch.object(volume_api.db, - 'service_get_all_by_topic') as get_all: - get_all.return_value = [ - { - 'availability_zone': 'a', - 'disabled': True, - }, - ] - azs = volume_api.list_availability_zones(enable_cache=False) - self.assertEqual([{"name": 'a', 'available': False}], list(azs)) - self.assertIsNone(volume_api.availability_zones_last_fetched) + self.get_all.return_value[0]['disabled'] = True + azs = self.volume_api.list_availability_zones(enable_cache=False) + self.assertEqual([{"name": 'a', 'available': False}], list(azs)) + self.assertIsNone(self.volume_api.availability_zones_last_fetched) def test_list_availability_zones_refetched(self): timeutils.set_time_override() self.addCleanup(timeutils.clear_time_override) - volume_api = cinder.volume.api.API() - with mock.patch.object(volume_api.db, - 'service_get_all_by_topic') as get_all: - get_all.return_value = [ - { - 'availability_zone': 'a', - 'disabled': False, - }, - ] - azs = volume_api.list_availability_zones(enable_cache=True) - self.assertEqual([{"name": 'a', 'available': True}], list(azs)) - self.assertIsNotNone(volume_api.availability_zones_last_fetched) - last_fetched = volume_api.availability_zones_last_fetched - self.assertTrue(get_all.called) - volume_api.list_availability_zones(enable_cache=True) - self.assertEqual(1, get_all.call_count) + azs = self.volume_api.list_availability_zones(enable_cache=True) + self.assertEqual([{"name": 'a', 'available': True}], list(azs)) + self.assertIsNotNone(self.volume_api.availability_zones_last_fetched) + last_fetched = self.volume_api.availability_zones_last_fetched + self.assertTrue(self.get_all.called) + self.volume_api.list_availability_zones(enable_cache=True) + self.assertEqual(1, self.get_all.call_count) - # The default cache time is 3600, push past that... - timeutils.advance_time_seconds(3800) - get_all.return_value = [ - { - 'availability_zone': 'a', - 'disabled': False, - }, - { - 'availability_zone': 'b', - 'disabled': False, - }, - ] - azs = volume_api.list_availability_zones(enable_cache=True) - azs = sorted([n['name'] for n in azs]) - self.assertEqual(['a', 'b'], azs) - self.assertEqual(2, get_all.call_count) - self.assertGreater(volume_api.availability_zones_last_fetched, - last_fetched) + # The default cache time is 3600, push past that... + timeutils.advance_time_seconds(3800) + self.get_all.return_value = [ + { + 'availability_zone': 'a', + 'disabled': False, + }, + { + 'availability_zone': 'b', + 'disabled': False, + }, + ] + azs = self.volume_api.list_availability_zones(enable_cache=True) + azs = sorted([n['name'] for n in azs]) + self.assertEqual(['a', 'b'], azs) + self.assertEqual(2, self.get_all.call_count) + self.assertGreater(self.volume_api.availability_zones_last_fetched, + last_fetched) def test_list_availability_zones_enabled_service(self): - services = [ + def sort_func(obj): + return obj['name'] + + self.get_all.return_value = [ {'availability_zone': 'ping', 'disabled': 0}, {'availability_zone': 'ping', 'disabled': 1}, {'availability_zone': 'pong', 'disabled': 0}, {'availability_zone': 'pung', 'disabled': 1}, ] - def stub_service_get_all_by_topic(*args, **kwargs): - return services - - self.stubs.Set(db, 'service_get_all_by_topic', - stub_service_get_all_by_topic) - - def sort_func(obj): - return obj['name'] - volume_api = cinder.volume.api.API() azs = volume_api.list_availability_zones() azs = sorted(azs, key=sort_func) @@ -1493,8 +1459,7 @@ class VolumeTestCase(BaseVolumeTestCase): snapshot_obj = fake_snapshot.fake_snapshot_obj(self.context, **snapshot) - with mock.patch.object(db, - 'service_get_all_by_topic') as mock_get_service, \ + with mock.patch('cinder.db.service_get_all') as mock_get_service, \ mock.patch.object(volume_api, 'list_availability_zones') as mock_get_azs: mock_get_service.return_value = [{'host': 'foo'}] @@ -5356,7 +5321,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'failover_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_failover_host(self, mock_db_args, mock_db_update, mock_failover): """Test replication failover_host.""" @@ -5371,7 +5336,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'failover_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_failover_host_unexpected_status(self, mock_db_args, mock_db_update, mock_failover): @@ -5389,7 +5354,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'freeze_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_freeze_host(self, mock_db_args, mock_db_update, mock_freeze): """Test replication freeze_host.""" @@ -5404,7 +5369,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'freeze_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_freeze_host_unexpected_status(self, mock_db_args, mock_db_update, mock_freeze): @@ -5422,7 +5387,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'thaw_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_thaw_host(self, mock_db_args, mock_db_update, mock_thaw): """Test replication thaw_host.""" @@ -5438,7 +5403,7 @@ class ReplicationTestCase(BaseVolumeTestCase): @mock.patch.object(volume_rpcapi.VolumeAPI, 'thaw_host') @mock.patch.object(cinder.db, 'conditional_update') - @mock.patch.object(cinder.db, 'service_get_by_args') + @mock.patch.object(cinder.db, 'service_get') def test_thaw_host_unexpected_status(self, mock_db_args, mock_db_update, mock_thaw):