From 8ac92aa946a985a20d279312d041fd66e09a9fbd Mon Sep 17 00:00:00 2001 From: Yang Youseok Date: Thu, 19 Apr 2018 16:26:40 +0900 Subject: [PATCH] Select service_provider on the basis of service_module In case of service_provider, selection according to service type is necessary. Currently there was no filtering logic, so if there are two or more service plugins using service_provider, there is a duplicate service_provider. New argument 'svc_type' is added to ProviderConfiguration so that only the service_provider matching the service type is shown. From caller side of ProviderConfiguration, one should specify 'svc_type' since ProviderConfiguration class have new 'svc_type' argument to find service provider. Although netron code base using ProviderConfiguration changed, existed code out of newtron tree should be also modified following the change becuase if not, there would be duplicated entries problem currently appeared. But there is no difference without 'svc_type' argument because matching is effective only when the argument is specified. A new test case added in test_get_service_providers() in neutron/tests/unit/extensions/test_servicetype.py which does not have any filter options. Without this patch, this test case would be failed having duplicated results. Change-Id: I6ad9897dd174b45c7f2315699d25d38d4c060abc Closes-Bug: #1763627 --- doc/source/contributor/contribute.rst | 2 +- .../l3_router/service_providers/driver_controller.py | 3 ++- neutron/services/provider_configuration.py | 8 +++++--- neutron/tests/unit/extensions/test_servicetype.py | 11 +++++++++-- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/doc/source/contributor/contribute.rst b/doc/source/contributor/contribute.rst index 36b39a9c22d..89d1c776063 100644 --- a/doc/source/contributor/contribute.rst +++ b/doc/source/contributor/contribute.rst @@ -554,7 +554,7 @@ your code:: service_type_manager = servicetype_db.ServiceTypeManager.get_instance() service_type_manager.add_provider_configuration( YOUR_SERVICE_TYPE, - pconf.ProviderConfiguration(YOUR_SERVICE_MODULE)) + pconf.ProviderConfiguration(YOUR_SERVICE_MODULE, YOUR_SERVICE_TYPE)) This is typically required when you instantiate your service plugin class. diff --git a/neutron/services/l3_router/service_providers/driver_controller.py b/neutron/services/l3_router/service_providers/driver_controller.py index b572b6e3985..a3cd9f0ba41 100644 --- a/neutron/services/l3_router/service_providers/driver_controller.py +++ b/neutron/services/l3_router/service_providers/driver_controller.py @@ -207,7 +207,8 @@ class _LegacyPlusProviderConfiguration( # loads up ha, dvr, and single_node service providers automatically. # If an operator has setup explicit values that conflict with these, # the operator defined values will take priority. - super(_LegacyPlusProviderConfiguration, self).__init__() + super(_LegacyPlusProviderConfiguration, self).__init__( + svc_type=plugin_constants.L3) for name, driver in (('dvrha', 'dvrha.DvrHaDriver'), ('dvr', 'dvr.DvrDriver'), ('ha', 'ha.HaDriver'), ('single_node', 'single_node.SingleNodeDriver')): diff --git a/neutron/services/provider_configuration.py b/neutron/services/provider_configuration.py index 97b42006eca..1361d41ec50 100644 --- a/neutron/services/provider_configuration.py +++ b/neutron/services/provider_configuration.py @@ -156,7 +156,7 @@ def get_provider_driver_class(driver, namespace=SERVICE_PROVIDERS): return new_driver -def parse_service_provider_opt(service_module='neutron'): +def parse_service_provider_opt(service_module='neutron', service_type=None): """Parse service definition opts and returns result.""" def validate_name(name): @@ -175,6 +175,8 @@ def parse_service_provider_opt(service_module='neutron'): split = prov_def.split(':') try: svc_type, name, driver = split[:3] + if service_type and service_type != svc_type: + continue except ValueError: raise n_exc.Invalid(_("Invalid service provider format")) validate_name(name) @@ -215,9 +217,9 @@ class ServiceProviderAlreadyAssociated(n_exc.Conflict): class ProviderConfiguration(object): - def __init__(self, svc_module='neutron'): + def __init__(self, svc_module='neutron', svc_type=None): self.providers = {} - for prov in parse_service_provider_opt(svc_module): + for prov in parse_service_provider_opt(svc_module, svc_type): self.add_provider(prov) def _ensure_driver_unique(self, driver): diff --git a/neutron/tests/unit/extensions/test_servicetype.py b/neutron/tests/unit/extensions/test_servicetype.py index e51fec0f45a..a17bbb76931 100644 --- a/neutron/tests/unit/extensions/test_servicetype.py +++ b/neutron/tests/unit/extensions/test_servicetype.py @@ -54,8 +54,10 @@ class ServiceTypeManagerTestCase(testlib_api.SqlTestCase): st_db.ServiceTypeManager._instance = None self.manager = st_db.ServiceTypeManager.get_instance() for provider in service_providers: + service_type = provider.split(':')[0] self.manager.add_provider_configuration( - provider.split(':')[0], provconf.ProviderConfiguration()) + service_type, provconf.ProviderConfiguration( + svc_type=service_type)) def test_service_provider_driver_not_unique(self): self._set_override([constants.LOADBALANCER + ':lbaas:driver']) @@ -75,6 +77,9 @@ class ServiceTypeManagerTestCase(testlib_api.SqlTestCase): constants.FIREWALL + ':fwaas:driver_path2']) ctx = context.get_admin_context() + res = self.manager.get_service_providers(ctx) + self.assertEqual(2, len(res)) + res = self.manager.get_service_providers( ctx, filters=dict(service_type=[constants.LOADBALANCER]) @@ -235,8 +240,10 @@ class ServiceTypeManagerExtTestCase(ServiceTypeExtensionTestCaseBase): st_db.ServiceTypeManager._instance = None self.manager = st_db.ServiceTypeManager.get_instance() for provider in service_providers: + service_type = provider.split(':')[0] self.manager.add_provider_configuration( - provider.split(':')[0], provconf.ProviderConfiguration()) + service_type, provconf.ProviderConfiguration( + svc_type=service_type)) super(ServiceTypeManagerExtTestCase, self).setUp() def _list_service_providers(self):