diff --git a/cinder/api/contrib/hosts.py b/cinder/api/contrib/hosts.py index 00ce08ae5d3..a09eff62464 100644 --- a/cinder/api/contrib/hosts.py +++ b/cinder/api/contrib/hosts.py @@ -99,7 +99,8 @@ def _list_hosts(req, service=None): """Returns a summary list of hosts.""" curr_time = timeutils.utcnow(with_timezone=True) context = req.environ['cinder.context'] - services = objects.ServiceList.get_all(context, False) + filters = {'disabled': False} + services = objects.ServiceList.get_all(context, filters) zone = '' if 'zone' in req.GET: zone = req.GET['zone'] diff --git a/cinder/api/contrib/services.py b/cinder/api/contrib/services.py index ccb75f5ef10..1adbbfcdeec 100644 --- a/cinder/api/contrib/services.py +++ b/cinder/api/contrib/services.py @@ -82,27 +82,20 @@ class ServiceController(wsgi.Controller): authorize(context, action='index') detailed = self.ext_mgr.is_loaded('os-extended-services') now = timeutils.utcnow(with_timezone=True) - services = objects.ServiceList.get_all(context) - host = '' + filters = {} + if 'host' in req.GET: - host = req.GET['host'] - service = '' - if 'service' in req.GET: - service = req.GET['service'] + filters['host'] = req.GET['host'] + if 'binary' in req.GET: + filters['binary'] = req.GET['binary'] + elif 'service' in req.GET: + filters['binary'] = req.GET['service'] versionutils.report_deprecated_feature(LOG, _( "Query by service parameter is deprecated. " "Please use binary parameter instead.")) - binary = '' - if 'binary' in req.GET: - binary = req.GET['binary'] - if host: - services = [s for s in services if s.host == host] - # NOTE(uni): deprecating service request key, binary takes precedence - binary_key = binary or service - if binary_key: - services = [s for s in services if s.binary == binary_key] + services = objects.ServiceList.get_all(context, filters) svcs = [] for svc in services: diff --git a/cinder/db/api.py b/cinder/db/api.py index cf651e796e6..abde7c80242 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -106,9 +106,9 @@ def service_get_by_host_and_topic(context, host, topic): return IMPL.service_get_by_host_and_topic(context, host, topic) -def service_get_all(context, disabled=None): +def service_get_all(context, filters=None): """Get all services.""" - return IMPL.service_get_all(context, disabled) + return IMPL.service_get_all(context, filters) def service_get_all_by_topic(context, topic, disabled=None): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index b2afe3c959c..a5620fa09b2 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -373,11 +373,22 @@ def service_get(context, service_id): @require_admin_context -def service_get_all(context, disabled=None): +def service_get_all(context, filters=None): + if filters and not is_valid_model_filters(models.Service, filters): + return [] + query = model_query(context, models.Service) - if disabled is not None: - query = query.filter_by(disabled=disabled) + 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 + + if filters: + query = query.filter_by(**filters) return query.all() diff --git a/cinder/tests/unit/api/contrib/test_hosts.py b/cinder/tests/unit/api/contrib/test_hosts.py index 4c91351257e..f1e54d1a868 100644 --- a/cinder/tests/unit/api/contrib/test_hosts.py +++ b/cinder/tests/unit/api/contrib/test_hosts.py @@ -70,7 +70,7 @@ def stub_utcnow(with_timezone=False): return datetime.datetime(2013, 7, 3, 0, 0, 2, tzinfo=tzinfo) -def stub_service_get_all(self, req): +def stub_service_get_all(context, filters=None): return SERVICE_LIST diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index 5cd9c1ebb6e..0af69a09d4a 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -132,7 +132,13 @@ class FakeRequestWithHostBinary(object): def fake_service_get_all(context, filters=None): - return fake_services_list + 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): diff --git a/cinder/tests/unit/objects/test_service.py b/cinder/tests/unit/objects/test_service.py index e7b3eec321a..57586d8d1a9 100644 --- a/cinder/tests/unit/objects/test_service.py +++ b/cinder/tests/unit/objects/test_service.py @@ -142,8 +142,9 @@ class TestServiceList(test_objects.BaseObjectsTestCase): db_service = fake_service.fake_db_service() service_get_all.return_value = [db_service] - services = objects.ServiceList.get_all(self.context, 'foo') - service_get_all.assert_called_once_with(self.context, 'foo') + 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) self.assertEqual(1, len(services)) TestService._compare(self, db_service, services[0]) diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 0b0c4cf59c5..c1165f9a22e 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -178,18 +178,25 @@ class DBAPIServiceTestCase(BaseTest): def test_service_get_all(self): values = [ - {'host': 'host1', 'topic': 'topic1'}, - {'host': 'host2', 'topic': 'topic2'}, + {'host': 'host1', 'binary': 'b1'}, + {'host': 'host1@ceph', 'binary': 'b2'}, + {'host': 'host2', 'binary': 'b2'}, {'disabled': True} ] services = [self._create_service(vals) for vals in values] + disabled_services = [services[-1]] non_disabled_services = services[:-1] - + expected = services[:2] + expected_bin = services[1:3] compares = [ - (services, db.service_get_all(self.ctxt)), - (disabled_services, db.service_get_all(self.ctxt, True)), - (non_disabled_services, db.service_get_all(self.ctxt, False)) + (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})), + (non_disabled_services, db.service_get_all(self.ctxt, + {'disabled': False})), ] for comp in compares: self._assertEqualListsOfObjects(*comp)