Factor up (most) CONF value in clients.Manager

Factor up CONF dependencies from the client manager class, so
that the part of it independent from CONF and unstable interfaces
can be moved to the manager base class, and be useful to external
consumers (such as plugins).

An helper function is defined in config.py to help extract from
configuration the parameters relevant for service client __init__.
This is used for Tempest own service clients and will be used by
plugins as well.

Partially-implements: bp client-manager-refactor

Change-Id: I19cdcf6f84144f6b1b0223a41a52f5b6e4b8fdd1

releasenotes/notes/service_client_config-8a1d7b4de769c633.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - A new helper method `service_client_config` has been added
+    to the stable module config.py that returns extracts from
+    configuration into a dictionary the configuration settings
+    relevant for the initisialisation of a service client.

tempest/clients.py

@@ -21,6 +21,7 @@ from tempest.common import negative_rest_client
 from tempest import config
 from tempest import exceptions
 from tempest.lib import auth
+from tempest.lib import exceptions as lib_exc
 from tempest.lib.services import compute
 from tempest.lib.services import image
 from tempest.lib.services import network
@@ -39,15 +40,10 @@ LOG = logging.getLogger(__name__)
 class Manager(service_clients.ServiceClients):
     """Top level manager for OpenStack tempest clients"""
 
-    default_params = {
-        'disable_ssl_certificate_validation':
-            CONF.identity.disable_ssl_certificate_validation,
-        'ca_certs': CONF.identity.ca_certificates_file,
-        'trace_requests': CONF.debug.trace_requests
-    }
+    default_params = config.service_client_config()
 
-    # NOTE: Tempest uses timeout values of compute API if project specific
-    # timeout values don't exist.
+    # TODO(andreaf) This is only used by data_processing and baremetal clients,
+    # and should be removed once they are out of Tempest
     default_params_with_timeout_values = {
         'build_interval': CONF.compute.build_interval,
         'build_timeout': CONF.compute.build_timeout
@@ -65,7 +61,11 @@ class Manager(service_clients.ServiceClients):
         _, identity_uri = get_auth_provider_class(credentials)
         super(Manager, self).__init__(
             credentials=credentials, identity_uri=identity_uri, scope=scope,
-            region=CONF.identity.region, **self.default_params)
+            region=CONF.identity.region,
+            client_parameters=self._prepare_configuration())
+        # TODO(andreaf) When clients are initialised without the right
+        # parameters available, the calls below will trigger a KeyError.
+        # We should catch that and raise a better error.
         self._set_compute_clients()
         self._set_identity_clients()
         self._set_volume_clients()
@@ -96,15 +96,38 @@ class Manager(service_clients.ServiceClients):
         self.negative_client = negative_rest_client.NegativeRestClient(
             self.auth_provider, service, **self.default_params)
 
+    def _prepare_configuration(self):
+        """Map values from CONF into Manager parameters
+
+        This uses `config.service_client_config` for all services to collect
+        most configuration items needed to init the clients.
+        """
+        # NOTE(andreaf) Configuration items will be passed in future patches
+        # into ClientFactory objects, but for now we update all the
+        # _set_*_client methods to consume them so we can verify that the
+        # configuration collected is correct
+
+        configuration = {}
+
+        # Setup the parameters for all Tempest services.
+        # NOTE(andreaf) Since client.py is an internal module of Tempest,
+        # it doesn't have to consider plugin configuration.
+        for service in service_clients.tempest_modules():
+            try:
+                # NOTE(andreaf) Use the unversioned service name to fetch
+                # the configuration since configuration is not versioned.
+                service_for_config = service.split('.')[0]
+                if service_for_config not in configuration:
+                    configuration[service_for_config] = (
+                        config.service_client_config(service_for_config))
+            except lib_exc.UnknownServiceClient:
+                LOG.warn(
+                    'Could not load configuration for service %s' % service)
+
+        return configuration
+
     def _set_network_clients(self):
-        params = {
-            'service': CONF.network.catalog_type,
-            'region': CONF.network.region or CONF.identity.region,
-            'endpoint_type': CONF.network.endpoint_type,
-            'build_interval': CONF.network.build_interval,
-            'build_timeout': CONF.network.build_timeout
-        }
-        params.update(self.default_params)
+        params = self.parameters['network']
         self.network_agents_client = network.AgentsClient(
             self.auth_provider, **params)
         self.network_extensions_client = network.ExtensionsClient(
@@ -135,20 +158,13 @@ class Manager(service_clients.ServiceClients):
             self.auth_provider, **params)
 
     def _set_image_clients(self):
-        params = {
-            'service': CONF.image.catalog_type,
-            'region': CONF.image.region or CONF.identity.region,
-            'endpoint_type': CONF.image.endpoint_type,
-            'build_interval': CONF.image.build_interval,
-            'build_timeout': CONF.image.build_timeout
-        }
-        params.update(self.default_params)
-
         if CONF.service_available.glance:
+            params = self.parameters['image']
             self.image_client = image.v1.ImagesClient(
                 self.auth_provider, **params)
             self.image_member_client = image.v1.ImageMembersClient(
                 self.auth_provider, **params)
+
             self.image_client_v2 = image.v2.ImagesClient(
                 self.auth_provider, **params)
             self.image_member_client_v2 = image.v2.ImageMembersClient(
@@ -161,14 +177,7 @@ class Manager(service_clients.ServiceClients):
                 self.auth_provider, **params)
 
     def _set_compute_clients(self):
-        params = {
-            'service': CONF.compute.catalog_type,
-            'region': CONF.compute.region or CONF.identity.region,
-            'endpoint_type': CONF.compute.endpoint_type,
-            'build_interval': CONF.compute.build_interval,
-            'build_timeout': CONF.compute.build_timeout
-        }
-        params.update(self.default_params)
+        params = self.parameters['compute']
 
         self.agents_client = compute.AgentsClient(self.auth_provider, **params)
         self.compute_networks_client = compute.NetworksClient(
@@ -234,10 +243,11 @@ class Manager(service_clients.ServiceClients):
         # NOTE: The following client needs special timeout values because
         # the API is a proxy for the other component.
         params_volume = copy.deepcopy(params)
-        params_volume.update({
-            'build_interval': CONF.volume.build_interval,
-            'build_timeout': CONF.volume.build_timeout
-        })
+        # Optional parameters
+        for _key in ('build_interval', 'build_timeout'):
+            _value = self.parameters['volume'].get(_key)
+            if _value:
+                params_volume[_key] = _value
         self.volumes_extensions_client = compute.VolumesClient(
             self.auth_provider, **params_volume)
         self.compute_versions_client = compute.VersionsClient(
@@ -246,14 +256,10 @@ class Manager(service_clients.ServiceClients):
             self.auth_provider, **params_volume)
 
     def _set_identity_clients(self):
-        params = {
-            'service': CONF.identity.catalog_type,
-            'region': CONF.identity.region
-        }
-        params.update(self.default_params_with_timeout_values)
+        params = self.parameters['identity']
 
         # Clients below use the admin endpoint type of Keystone API v2
-        params_v2_admin = params.copy()
+        params_v2_admin = copy.copy(params)
         params_v2_admin['endpoint_type'] = CONF.identity.v2_admin_endpoint_type
         self.endpoints_client = identity.v2.EndpointsClient(self.auth_provider,
                                                             **params_v2_admin)
@@ -269,7 +275,7 @@ class Manager(service_clients.ServiceClients):
             self.auth_provider, **params_v2_admin)
 
         # Clients below use the public endpoint type of Keystone API v2
-        params_v2_public = params.copy()
+        params_v2_public = copy.copy(params)
         params_v2_public['endpoint_type'] = (
             CONF.identity.v2_public_endpoint_type)
         self.identity_public_client = identity.v2.IdentityClient(
@@ -279,8 +285,9 @@ class Manager(service_clients.ServiceClients):
         self.users_public_client = identity.v2.UsersClient(
             self.auth_provider, **params_v2_public)
 
-        # Clients below use the endpoint type of Keystone API v3
-        params_v3 = params.copy()
+        # Clients below use the endpoint type of Keystone API v3, which is set
+        # in endpoint_type
+        params_v3 = copy.copy(params)
         params_v3['endpoint_type'] = CONF.identity.v3_endpoint_type
         self.domains_client = identity.v3.DomainsClient(self.auth_provider,
                                                         **params_v3)
@@ -326,14 +333,8 @@ class Manager(service_clients.ServiceClients):
                 raise exceptions.InvalidConfiguration(msg)
 
     def _set_volume_clients(self):
-        params = {
-            'service': CONF.volume.catalog_type,
-            'region': CONF.volume.region or CONF.identity.region,
-            'endpoint_type': CONF.volume.endpoint_type,
-            'build_interval': CONF.volume.build_interval,
-            'build_timeout': CONF.volume.build_timeout
-        }
-        params.update(self.default_params)
+        # Mandatory parameters (always defined)
+        params = self.parameters['volume']
 
         self.volume_qos_client = volume.v1.QosSpecsClient(self.auth_provider,
                                                           **params)
@@ -381,12 +382,8 @@ class Manager(service_clients.ServiceClients):
             volume.v2.AvailabilityZoneClient(self.auth_provider, **params)
 
     def _set_object_storage_clients(self):
-        params = {
-            'service': CONF.object_storage.catalog_type,
-            'region': CONF.object_storage.region or CONF.identity.region,
-            'endpoint_type': CONF.object_storage.endpoint_type
-        }
-        params.update(self.default_params_with_timeout_values)
+        # Mandatory parameters (always defined)
+        params = self.parameters['object-storage']
 
         self.account_client = object_storage.AccountClient(self.auth_provider,
                                                            **params)
@@ -404,12 +401,8 @@ def get_auth_provider_class(credentials):
 
 
 def get_auth_provider(credentials, pre_auth=False, scope='project'):
-    default_params = {
-        'disable_ssl_certificate_validation':
-            CONF.identity.disable_ssl_certificate_validation,
-        'ca_certs': CONF.identity.ca_certificates_file,
-        'trace_requests': CONF.debug.trace_requests
-    }
+    # kwargs for auth provider match the common ones used by service clients
+    default_params = config.service_client_config()
     if credentials is None:
         raise exceptions.InvalidCredentials(
             'Credentials must be specified')

tempest/config.py

@@ -25,6 +25,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 import testtools
 
+from tempest.lib import exceptions
 from tempest.test_discover import plugins
 
 
@@ -1345,3 +1346,84 @@ def skip_if_config(*args):
             return f(self, *func_args, **func_kwargs)
         return wrapper
     return decorator
+
+
+def service_client_config(service_client_name=None):
+    """Return a dict with the parameters to init service clients
+
+    Extracts from CONF the settings specific to the service_client_name and
+    api_version, and formats them as dict ready to be passed to the service
+    clients __init__:
+
+        * `region` (default to identity)
+        * `catalog_type`
+        * `endpoint_type`
+        * `build_timeout` (object-storage and identity default to compute)
+        * `build_interval` (object-storage and identity default to compute)
+
+    The following common settings are always returned, even if
+    `service_client_name` is None:
+
+        * `disable_ssl_certificate_validation`
+        * `ca_certs`
+        * `trace_requests`
+
+    The dict returned by this does not fit a few service clients:
+
+        * The endpoint type is not returned for identity client, since it takes
+          three different values for v2 admin, v2 public and v3
+        * The `ServersClient` from compute accepts an optional
+          `enable_instance_password` parameter, which is not returned.
+        * The `VolumesClient` for both v1 and v2 volume accept an optional
+          `default_volume_size` parameter, which is not returned.
+        * The `TokenClient` and `V3TokenClient` have a very different
+          interface, only auth_url is needed for them.
+
+    :param service_client_name: str Name of the service. Supported values are
+        'compute', 'identity', 'image', 'network', 'object-storage', 'volume'
+    :return: dictionary of __init__ parameters for the service clients
+    :rtype: dict
+    """
+    _parameters = {
+        'disable_ssl_certificate_validation':
+            CONF.identity.disable_ssl_certificate_validation,
+        'ca_certs': CONF.identity.ca_certificates_file,
+        'trace_requests': CONF.debug.trace_requests
+    }
+
+    if service_client_name is None:
+        return _parameters
+
+    # Get the group of options first, by normalising the service_group_name
+    # Services with a '-' in the name have an '_' in the option group name
+    config_group = service_client_name.replace('-', '_')
+    # NOTE(andreaf) Check if the config group exists. This allows for this
+    # helper to be used for settings from registered plugins as well
+    try:
+        options = getattr(CONF, config_group)
+    except cfg.NoSuchOptError:
+        # Option group not defined
+        raise exceptions.UnknownServiceClient(services=service_client_name)
+    # Set endpoint_type
+    # Identity uses different settings depending on API version, so do not
+    # return the endpoint at all.
+    if service_client_name != 'identity':
+        _parameters['endpoint_type'] = getattr(options, 'endpoint_type')
+    # Set build_*
+    # Object storage and identity groups do not have conf settings for
+    # build_* parameters, and we default to compute in any case
+    for setting in ['build_timeout', 'build_interval']:
+        if not hasattr(options, setting) or not getattr(options, setting):
+            _parameters[setting] = getattr(CONF.compute, setting)
+        else:
+            _parameters[setting] = getattr(options, setting)
+    # Set region
+    # If a service client does not define region or region is not set
+    # default to the identity region
+    if not hasattr(options, 'region') or not getattr(options, 'region'):
+        _parameters['region'] = CONF.identity.region
+    else:
+        _parameters['region'] = getattr(options, 'region')
+    # Set service
+    _parameters['service'] = getattr(options, 'catalog_type')
+    return _parameters

tempest/lib/exceptions.py

@@ -225,3 +225,7 @@ class SSHExecCommandFailed(TempestException):
     message = ("Command '%(command)s', exit status: %(exit_status)d, "
                "stderr:\n%(stderr)s\n"
                "stdout:\n%(stdout)s")
+
+
+class UnknownServiceClient(TempestException):
+    message = "Service clients named %(services)s are not known"

tempest/service_clients.py

@@ -18,6 +18,22 @@ from tempest.lib import auth
 from tempest.lib import exceptions
 
 
+def tempest_modules():
+    """List of service client modules available in Tempest.
+
+    Provides a list of service modules available Tempest.
+    """
+    return set(['compute', 'identity.v2', 'identity.v3', 'image.v1',
+                'image.v2', 'network', 'object-storage', 'volume.v1',
+                'volume.v2', 'volume.v3'])
+
+
+def available_modules():
+    """List of service client modules available in Tempest and plugins"""
+    # TODO(andreaf) For now this returns only tempest_modules
+    return tempest_modules()
+
+
 class ServiceClients(object):
     """Service client provider class
 
@@ -30,7 +46,8 @@ class ServiceClients(object):
 
         >>> from tempest import service_clients
         >>> johndoe = cred_provider.get_creds_by_role(['johndoe'])
-        >>> johndoe_clients = service_clients.ServiceClients(johndoe)
+        >>> johndoe_clients = service_clients.ServiceClients(johndoe,
+        >>>                                                  identity_uri)
         >>> johndoe_servers = johndoe_clients.servers_client.list_servers()
 
     """
@@ -40,17 +57,45 @@ class ServiceClients(object):
     # initialises this class using values from tempest CONF object. The wrapper
     # class should only be used by tests hosted in Tempest.
 
-    def __init__(self, credentials, identity_uri, region=None,
-                 scope='project', disable_ssl_certificate_validation=True,
-                 ca_certs=None, trace_requests=''):
+    def __init__(self, credentials, identity_uri, region=None, scope='project',
+                 disable_ssl_certificate_validation=True, ca_certs=None,
+                 trace_requests='', client_parameters=None):
         """Service Clients provider
 
         Instantiate a `ServiceClients` object, from a set of credentials and an
         identity URI. The identity version is inferred from the credentials
         object. Optionally auth scope can be provided.
+
+        A few parameters can be given a value which is applied as default
+        for all service clients: region, dscv, ca_certs, trace_requests.
+
         Parameters dscv, ca_certs and trace_requests all apply to the auth
         provider as well as any service clients provided by this manager.
 
+        Any other client parameter must be set via client_parameters.
+        The list of available parameters is defined in the service clients
+        interfaces. For reference, most clients will accept 'region',
+        'service', 'endpoint_type', 'build_timeout' and 'build_interval', which
+        are all inherited from RestClient.
+
+        The `config` module in Tempest exposes an helper function
+        `service_client_config` that can be used to extract from configuration
+        a dictionary ready to be injected in kwargs.
+
+        Exceptions are:
+        - Token clients for 'identity' have a very different interface
+        - Volume client for 'volume' accepts 'default_volume_size'
+        - Servers client from 'compute' accepts 'enable_instance_password'
+
+        Examples:
+
+            >>> identity_params = config.service_client_config('identity')
+            >>> params = {
+            >>>     'identity': identity_params,
+            >>>     'compute': {'region': 'region2'}}
+            >>> manager = lib_manager.Manager(
+            >>>     my_creds, identity_uri, client_parameters=params)
+
         :param credentials: An instance of `auth.Credentials`
         :param identity_uri: URI of the identity API. This should be a
                              mandatory parameter, and it will so soon.
@@ -60,12 +105,26 @@ class ServiceClients(object):
                                                   service clients.
         :param ca_certs Applies to auth and to all service clients.
         :param trace_requests Applies to auth and to all service clients.
+        :param client_parameters Dictionary with parameters for service
+            clients. Keys of the dictionary are the service client service
+            name, as declared in `service_clients.available_modules()` except
+            for the version. Values are dictionaries of parameters that are
+            going to be passed to all clients in the service client module.
+
+        Examples:
+
+            >>> params_service_x = {'param_name': 'param_value'}
+            >>> client_parameters = { 'service_x': params_service_x }
+
+            >>> params_service_y = config.service_client_config('service_y')
+            >>> client_parameters['service_y'] = params_service_y
+
         """
         self.credentials = credentials
         self.identity_uri = identity_uri
         if not identity_uri:
             raise exceptions.InvalidCredentials(
-                'Manager requires a non-empty identity_uri.')
+                'ServiceClients requires a non-empty identity_uri.')
         self.region = region
         # Check if passed or default credentials are valid
         if not self.credentials.is_valid():
@@ -88,3 +147,32 @@ class ServiceClients(object):
             self.credentials, self.identity_uri, scope=scope,
             disable_ssl_certificate_validation=self.dscv,
             ca_certs=self.ca_certs, trace_requests=self.trace_requests)
+        # Setup some defaults for client parameters of registered services
+        client_parameters = client_parameters or {}
+        self.parameters = {}
+        # Parameters are provided for unversioned services
+        unversioned_services = set(
+            [x.split('.')[0] for x in available_modules()])
+        for service in unversioned_services:
+            self.parameters[service] = self._setup_parameters(
+                client_parameters.pop(service, {}))
+        # Check that no client parameters was supplied for unregistered clients
+        if client_parameters:
+            raise exceptions.UnknownServiceClient(
+                services=list(client_parameters.keys()))
+
+    def _setup_parameters(self, parameters):
+        """Setup default values for client parameters
+
+        Region by default is the region passed as an __init__ parameter.
+        Checks that no parameter for an unknown service is provided.
+        """
+        _parameters = {}
+        # Use region from __init__
+        if self.region:
+            _parameters['region'] = self.region
+        # Update defaults with specified parameters
+        _parameters.update(parameters)
+        # If any parameter is left, parameters for an unknown service were
+        # provided as input. Fail rather than ignore silently.
+        return _parameters

tempest/tests/fake_config.py

@@ -57,3 +57,57 @@ class FakePrivate(config.TempestConfigPrivate):
     def __init__(self, parse_conf=True, config_path=None):
         self._set_attrs()
         self.lock_path = cfg.CONF.oslo_concurrency.lock_path
+
+fake_service1_group = cfg.OptGroup(name='fake-service1', title='Fake service1')
+
+FakeService1Group = [
+    cfg.StrOpt('catalog_type', default='fake-service1'),
+    cfg.StrOpt('endpoint_type', default='faketype'),
+    cfg.StrOpt('region', default='fake_region'),
+    cfg.IntOpt('build_timeout', default=99),
+    cfg.IntOpt('build_interval', default=9)]
+
+fake_service2_group = cfg.OptGroup(name='fake-service2', title='Fake service2')
+
+FakeService2Group = [
+    cfg.StrOpt('catalog_type', default='fake-service2'),
+    cfg.StrOpt('endpoint_type', default='faketype')]
+
+
+class ServiceClientsConfigFixture(conf_fixture.Config):
+
+    def __init__(self):
+        cfg.CONF([], default_config_files=[])
+        config._opts.append((fake_service1_group, FakeService1Group))
+        config._opts.append((fake_service2_group, FakeService2Group))
+        config.register_opts()
+        super(ServiceClientsConfigFixture, self).__init__()
+
+    def setUp(self):
+        super(ServiceClientsConfigFixture, self).setUp()
+        # Debug default values
+        self.conf.set_default('trace_requests', 'fake_module', 'debug')
+        # Identity default values
+        self.conf.set_default('disable_ssl_certificate_validation', True,
+                              group='identity')
+        self.conf.set_default('ca_certificates_file', '/fake/certificates',
+                              group='identity')
+        self.conf.set_default('region', 'fake_region', 'identity')
+        # Identity endpoints
+        self.conf.set_default('v3_endpoint_type', 'fake_v3_uri', 'identity')
+        self.conf.set_default('v2_public_endpoint_type', 'fake_v2_public_uri',
+                              'identity')
+        self.conf.set_default('v2_admin_endpoint_type', 'fake_v2_admin_uri',
+                              'identity')
+        # Compute default values
+        self.conf.set_default('build_interval', 88, group='compute')
+        self.conf.set_default('build_timeout', 8, group='compute')
+
+
+class ServiceClientsFakePrivate(config.TempestConfigPrivate):
+    def __init__(self, parse_conf=True, config_path=None):
+        self._set_attrs()
+        self.fake_service1 = cfg.CONF['fake-service1']
+        self.fake_service2 = cfg.CONF['fake-service2']
+        print('Services registered')
+        self.lock_path = cfg.CONF.oslo_concurrency.lock_path

tempest/tests/test_config.py

@@ -0,0 +1,89 @@
+# Copyright (c) 2016 Hewlett-Packard Enterprise Development Company, L.P.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+
+import testtools
+
+from tempest import config
+from tempest.lib import exceptions
+from tempest.tests import base
+from tempest.tests import fake_config
+
+
+class TestServiceClientConfig(base.TestCase):
+
+    expected_common_params = set(['disable_ssl_certificate_validation',
+                                  'ca_certs', 'trace_requests'])
+    expected_extra_params = set(['service', 'endpoint_type', 'region',
+                                 'build_timeout', 'build_interval'])
+
+    def setUp(self):
+        super(TestServiceClientConfig, self).setUp()
+        self.useFixture(fake_config.ServiceClientsConfigFixture())
+        self.patchobject(config, 'CONF',
+                         fake_config.ServiceClientsFakePrivate())
+        self.CONF = config.CONF
+
+    def test_service_client_config_no_service(self):
+        params = config.service_client_config()
+        for param_name in self.expected_common_params:
+            self.assertIn(param_name, params)
+        for param_name in self.expected_extra_params:
+            self.assertNotIn(param_name, params)
+        self.assertEqual(
+            self.CONF.identity.disable_ssl_certificate_validation,
+            params['disable_ssl_certificate_validation'])
+        self.assertEqual(self.CONF.identity.ca_certificates_file,
+                         params['ca_certs'])
+        self.assertEqual(self.CONF.debug.trace_requests,
+                         params['trace_requests'])
+
+    def test_service_client_config_service_all(self):
+        params = config.service_client_config(
+            service_client_name='fake-service1')
+        for param_name in self.expected_common_params:
+            self.assertIn(param_name, params)
+        for param_name in self.expected_extra_params:
+            self.assertIn(param_name, params)
+        self.assertEqual(self.CONF.fake_service1.catalog_type,
+                         params['service'])
+        self.assertEqual(self.CONF.fake_service1.endpoint_type,
+                         params['endpoint_type'])
+        self.assertEqual(self.CONF.fake_service1.region, params['region'])
+        self.assertEqual(self.CONF.fake_service1.build_timeout,
+                         params['build_timeout'])
+        self.assertEqual(self.CONF.fake_service1.build_interval,
+                         params['build_interval'])
+
+    def test_service_client_config_service_minimal(self):
+        params = config.service_client_config(
+            service_client_name='fake-service2')
+        for param_name in self.expected_common_params:
+            self.assertIn(param_name, params)
+        for param_name in self.expected_extra_params:
+            self.assertIn(param_name, params)
+        self.assertEqual(self.CONF.fake_service2.catalog_type,
+                         params['service'])
+        self.assertEqual(self.CONF.fake_service2.endpoint_type,
+                         params['endpoint_type'])
+        self.assertEqual(self.CONF.identity.region, params['region'])
+        self.assertEqual(self.CONF.compute.build_timeout,
+                         params['build_timeout'])
+        self.assertEqual(self.CONF.compute.build_interval,
+                         params['build_interval'])
+
+    def test_service_client_config_service_unknown(self):
+        unknown_service = 'unknown_service'
+        with testtools.ExpectedException(exceptions.UnknownServiceClient,
+                                         '.*' + unknown_service + '.*'):
+            config.service_client_config(service_client_name=unknown_service)

tempest/tests/test_service_clients.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations under
 # the License.
 
+import fixtures
 import testtools
 
 from tempest.lib import auth
@@ -23,7 +24,13 @@ from tempest.tests.lib import fake_credentials
 
 class TestServiceClients(base.TestCase):
 
-    def test__init__creds_v2_uri(self):
+    def setUp(self):
+        super(TestServiceClients, self).setUp()
+        self.useFixture(fixtures.MockPatch(
+            'tempest.service_clients.tempest_modules',
+            return_value=set(['fake_service1', 'fake_service2'])))
+
+    def test___init___creds_v2_uri(self):
         # Verify that no API request is made, since no mock
         # is required to run the test successfully
         creds = fake_credentials.FakeKeystoneV2Credentials()
@@ -32,7 +39,7 @@ class TestServiceClients(base.TestCase):
         self.assertIsInstance(_manager.auth_provider,
                               auth.KeystoneV2AuthProvider)
 
-    def test__init__creds_v3_uri(self):
+    def test___init___creds_v3_uri(self):
         # Verify that no API request is made, since no mock
         # is required to run the test successfully
         creds = fake_credentials.FakeKeystoneV3Credentials()
@@ -41,22 +48,79 @@ class TestServiceClients(base.TestCase):
         self.assertIsInstance(_manager.auth_provider,
                               auth.KeystoneV3AuthProvider)
 
-    def test__init__base_creds_uri(self):
+    def test___init___base_creds_uri(self):
         creds = fake_credentials.FakeCredentials()
         uri = 'fake_uri'
         with testtools.ExpectedException(exceptions.InvalidCredentials):
             service_clients.ServiceClients(creds, identity_uri=uri)
 
-    def test__init__invalid_creds_uri(self):
+    def test___init___invalid_creds_uri(self):
         creds = fake_credentials.FakeKeystoneV2Credentials()
         delattr(creds, 'username')
         uri = 'fake_uri'
         with testtools.ExpectedException(exceptions.InvalidCredentials):
             service_clients.ServiceClients(creds, identity_uri=uri)
 
-    def test__init__creds_uri_none(self):
+    def test___init___creds_uri_none(self):
         creds = fake_credentials.FakeKeystoneV2Credentials()
-        msg = "Invalid Credentials\nDetails: Manager requires a non-empty"
+        msg = ("Invalid Credentials\nDetails: ServiceClients requires a "
+               "non-empty")
         with testtools.ExpectedException(exceptions.InvalidCredentials,
                                          value_re=msg):
             service_clients.ServiceClients(creds, None)
+
+    def test___init___creds_uri_params(self):
+        creds = fake_credentials.FakeKeystoneV2Credentials()
+        expeted_params = {'fake_param1': 'fake_value1',
+                          'fake_param2': 'fake_value2'}
+        params = {'fake_service1': expeted_params}
+        uri = 'fake_uri'
+        _manager = service_clients.ServiceClients(creds, identity_uri=uri,
+                                                  client_parameters=params)
+        self.assertIn('fake_service1', _manager.parameters.keys())
+        for _key in expeted_params.keys():
+            self.assertIn(_key, _manager.parameters['fake_service1'].keys())
+            self.assertEqual(expeted_params[_key],
+                             _manager.parameters['fake_service1'].get(_key))
+
+    def test___init___creds_uri_params_unknown_services(self):
+        creds = fake_credentials.FakeKeystoneV2Credentials()
+        fake_params = {'fake_param1': 'fake_value1'}
+        params = {'unknown_service1': fake_params,
+                  'unknown_service2': fake_params}
+        uri = 'fake_uri'
+        msg = "(?=.*{0})(?=.*{1})".format(*list(params.keys()))
+        with testtools.ExpectedException(
+                exceptions.UnknownServiceClient, value_re=msg):
+            service_clients.ServiceClients(creds, identity_uri=uri,
+                                           client_parameters=params)
+
+    def _get_manager(self, init_region='fake_region'):
+        # Get a manager to invoke _setup_parameters on
+        creds = fake_credentials.FakeKeystoneV2Credentials()
+        return service_clients.ServiceClients(creds, identity_uri='fake_uri',
+                                              region=init_region)
+
+    def test__setup_parameters_none_no_region(self):
+        kwargs = {}
+        _manager = self._get_manager(init_region=None)
+        _params = _manager._setup_parameters(kwargs)
+        self.assertNotIn('region', _params)
+
+    def test__setup_parameters_none(self):
+        kwargs = {}
+        _manager = self._get_manager()
+        _params = _manager._setup_parameters(kwargs)
+        self.assertIn('region', _params)
+        self.assertEqual('fake_region', _params['region'])
+
+    def test__setup_parameters_all(self):
+        expected_params = {'region': 'fake_region1',
+                           'catalog_type': 'fake_service2_mod',
+                           'fake_param1': 'fake_value1',
+                           'fake_param2': 'fake_value2'}
+        _manager = self._get_manager()
+        _params = _manager._setup_parameters(expected_params)
+        for _key in _params.keys():
+            self.assertEqual(expected_params[_key],
+                             _params[_key])