Split main to smaller functions

* make main more readable by splitting the code to smaller functions like
set_logging, read_deployer_input, set_options.
* --use-test-accounts argument was renamed to --test-accounts and it accepts
a path to accounts.yaml file.
* also comments and docstrings were added.

Change-Id: I06e97d153753b41dba2244ee145606f6c37eae67

config_tempest/config_tempest.py

@@ -68,7 +68,6 @@ from tempest.lib.services.network import networks_client
 from tempest.lib.services.volume.v2 import services_client
 
 LOG = logging.getLogger(__name__)
-LOG_FORMAT = '%(asctime)s - %(name)s - %(levelname)s - %(message)s'
 
 # Get the current tempest workspace path
 TEMPEST_WORKSPACE = os.getcwd()
@@ -119,44 +118,67 @@ SERVICE_EXTENSION_KEY = {
 }
 
 
-def main():
+def set_logging(debug, verbose):
-    args = parse_arguments()
+    """Set logging based on the arguments.
-    args.remove = parse_values_to_remove(args.remove)
-    logging.basicConfig(format=LOG_FORMAT)
 
-    if args.debug:
+    :type debug: Boolean
+    :type verbose: Boolean
+    """
+    log_format = '%(asctime)s - %(name)s - %(levelname)s - %(message)s'
+    logging.basicConfig(format=log_format)
+    if debug:
         LOG.setLevel(logging.DEBUG)
-    elif args.verbose:
+    elif verbose:
         LOG.setLevel(logging.INFO)
 
-    conf = TempestConf()
+
+def read_deployer_input(deployer_input_file, conf):
+    """Read deployer-input file and set values in conf accordingly.
+
+    :param deployer_input_file: Path to the deployer inut file
+    :type deployer_input_file: String
+    :param conf: TempestConf object
+    """
+    LOG.info("Adding options from deployer-input file '%s'",
+             deployer_input_file)
+    deployer_input = ConfigParser.SafeConfigParser()
+    deployer_input.read(deployer_input_file)
+    for section in deployer_input.sections():
+        # There are no deployer input options in DEFAULT
+        for (key, value) in deployer_input.items(section):
+            conf.set(section, key, value, priority=True)
+
+
+def set_options(conf, deployer_input, non_admin, overrides=[],
+                test_accounts=None, cloud_creds=None):
+    """Set options in conf provided by different source.
+
+    1. read the default values in default-overrides file
+    2. read a file provided by --deployer-input argument
+    3. set values from client's config (os-client-config support) if provided
+    4. set overrides - may override values which were set in the steps above
+
+    :param conf: TempestConf object
+    :param deployer_input: Path to the deployer inut file
+    :type deployer_input: string
+    :type non_admin: boolean
+    :param overrides: list of tuples: [(section, key, value)]
+    :type overrides: list
+    :param test_accounts: Path to the accounts.yaml file
+    :type test_accounts: string
+    :param cloud_creds: Cloud credentials from client's config
+    :type cloud_creds: dict
+    """
     if os.path.isfile(DEFAULTS_FILE):
         LOG.info("Reading defaults from file '%s'", DEFAULTS_FILE)
         conf.read(DEFAULTS_FILE)
-    if args.deployer_input and os.path.isfile(args.deployer_input):
+
-        LOG.info("Adding options from deployer-input file '%s'",
+    if deployer_input and os.path.isfile(deployer_input):
-                 args.deployer_input)
+        read_deployer_input(deployer_input, conf)
-        deployer_input = ConfigParser.SafeConfigParser()
+
-        deployer_input.read(args.deployer_input)
+    if non_admin:
-        for section in deployer_input.sections():
+        # non admin, so delete auth admin values which were set
-            # There are no deployer input options in DEFAULT
+        # from default-overides.conf file
-            for (key, value) in deployer_input.items(section):
-                conf.set(section, key, value, priority=True)
-    # get and set auth data from client's config (os-client-config support)
-    set_cloud_config_values(conf, args)
-    # set overrides - vales specified in CLI
-    for section, key, value in args.overrides:
-        conf.set(section, key, value, priority=True)
-    uri = conf.get("identity", "uri")
-    api_version = 2
-    if "v3" in uri:
-        api_version = 3
-        conf.set("identity", "auth_version", "v3")
-        conf.set("identity", "uri_v3", uri)
-    else:
-        # TODO(arxcruz) make a check if v3 is enabled
-        conf.set("identity", "uri_v3", uri.replace("v2.0", "v3"))
-    if args.non_admin:
         conf.set("auth", "admin_username", "")
         conf.set("auth", "admin_project_name", "")
         conf.set("auth", "admin_password", "")
@@ -170,53 +192,23 @@ def main():
         # Moved to auth
         conf.set("identity", "admin_password", "")
         conf.set("auth", "use_dynamic_credentials", "False")
-    if args.use_test_accounts:
+
+    # get and set auth data from client's config
+    if cloud_creds:
+        set_cloud_config_values(non_admin, cloud_creds, conf)
+
+    if test_accounts:
         # new way for running using accounts file
         conf.set("auth", "use_dynamic_credentials", "False")
-        conf.set("auth", "test_accounts_file", "etc/accounts.yaml")
+        conf.set("auth", "test_accounts_file",
-    clients = ClientManager(conf, not args.non_admin)
+                 os.path.abspath(test_accounts))
-    swift_discover = conf.get_defaulted('object-storage-feature-enabled',
-                                        'discoverability')
-    services = api_discovery.discover(
-        clients.auth_provider,
-        clients.identity_region,
-        object_store_discovery=conf.get_bool_value(swift_discover),
-        api_version=api_version,
-        disable_ssl_certificate_validation=conf.get_defaulted(
-            'identity',
-            'disable_ssl_certificate_validation'
-        )
-    )
-    if args.create and not args.use_test_accounts:
-        create_tempest_users(clients.tenants, clients.roles, clients.users,
-                             conf, services)
-    create_tempest_flavors(clients.flavors, conf, args.create)
-    create_tempest_images(clients.images, conf, args.image, args.create,
-                          args.image_disk_format)
-    has_neutron = "network" in services
 
-    LOG.info("Setting up network")
+    # set overrides - values specified in CLI
-    LOG.debug("Is neutron present: {0}".format(has_neutron))
+    for section, key, value in overrides:
-    create_tempest_networks(clients, conf, has_neutron, args.network_id)
+        conf.set(section, key, value, priority=True)
-
-    configure_discovered_services(conf, services)
-    check_volume_backup_service(clients.volume_service, conf, services)
-    check_ceilometer_service(clients.service_client, conf, services)
-    configure_boto(conf, services)
-    configure_keystone_feature_flags(conf, services)
-    configure_horizon(conf)
-
-    # remove all unwanted values if were specified
-    if args.remove != {}:
-        LOG.info("Removing configuration: %s", str(args.remove))
-        conf.remove_values(args)
-    LOG.info("Creating configuration file %s", os.path.abspath(args.out))
-    with open(args.out, 'w') as f:
-        conf.write(f)
 
 
 def parse_arguments():
-    # TODO(tkammer): add mutual exclusion groups
     cloud_config = os_client_config.OpenStackConfig()
     parser = argparse.ArgumentParser(__doc__)
     cloud_config.register_argparse_arguments(parser, sys.argv)
@@ -244,8 +236,8 @@ def parse_arguments():
                         help='Print more information about the execution')
     parser.add_argument('--non-admin', action='store_true', default=False,
                         help='Run without admin creds')
-    parser.add_argument('--use-test-accounts', action='store_true',
+    parser.add_argument('--test-accounts', default=None, metavar='PATH',
-                        default=False, help='Use accounts from accounts.yaml')
+                        help='Use accounts from accounts.yaml')
     parser.add_argument('--image-disk-format', default=DEFAULT_IMAGE_FORMAT,
                         help="""a format of an image to be uploaded to glance.
                                 Default is '%s'""" % DEFAULT_IMAGE_FORMAT)
@@ -270,6 +262,7 @@ def parse_arguments():
                         " together, since creating" " resources requires"
                         " admin rights")
     args.overrides = parse_overrides(args.overrides)
+    args.remove = parse_values_to_remove(args.remove)
     cloud = cloud_config.get_one_cloud(argparse=args)
     return cloud
 
@@ -277,8 +270,9 @@ def parse_arguments():
 def parse_values_to_remove(options):
     """Manual parsing of remove arguments.
 
-    :options list of arguments following --remove argument
+    :param options: list of arguments following --remove argument
-    :returns dict containing key paths with values to be removed
+    :return: dictionary containing key paths with values to be removed
+    :rtype: dict
     EXAMPLE: {'identity.username': [myname],
               'identity-feature-enabled.api_extensions': [http, https]}
     """
@@ -300,7 +294,10 @@ def parse_values_to_remove(options):
 def parse_overrides(overrides):
     """Manual parsing of positional arguments.
 
-    TODO(mkollaro) find a way to do it in argparse
+    :param overrides: list of section.keys and values to override, example:
+               ['section.key', 'value', 'section.key', 'value']
+    :return: list of tuples, example: [('section', 'key', 'value'), ...]
+    :rtype: list
     """
     if len(overrides) % 2 != 0:
         raise Exception("An odd number of override options was found. The"
@@ -320,36 +317,35 @@ def parse_overrides(overrides):
     return new_overrides
 
 
-def set_cloud_config_values(conf, args):
+def set_cloud_config_values(non_admin, cloud_creds, conf):
     """Set values from client's cloud config file.
 
-    If the cloud config files was provided, set admin and non-admin credentials
+    Set admin and non-admin credentials and uri from cloud credentials.
-    and uri.
     Note: the values may be later overriden by values specified in CLI.
 
-    :conf TempestConf object
+    :type non_admin: Boolean
-    :args parsed arguments including client config values
+    :param cloud_creds: auth data from os-client-config
+    :type cloud_creds: dict
+    :param conf: TempestConf object
     """
-    cloud_creds = args.config.get('auth')
+    try:
-    if cloud_creds:
+        if non_admin:
-        try:
+            conf.set('identity', 'username', cloud_creds['username'])
-            if args.non_admin:
+            conf.set('identity',
-                conf.set('identity', 'username', cloud_creds['username'])
+                     'tenant_name',
-                conf.set('identity',
+                     cloud_creds['project_name'])
-                         'tenant_name',
+            conf.set('identity', 'password', cloud_creds['password'])
-                         cloud_creds['project_name'])
+        else:
-                conf.set('identity', 'password', cloud_creds['password'])
+            conf.set('identity', 'admin_username', cloud_creds['username'])
-            else:
+            conf.set('identity',
-                conf.set('identity', 'admin_username', cloud_creds['username'])
+                     'admin_tenant_name',
-                conf.set('identity',
+                     cloud_creds['project_name'])
-                         'admin_tenant_name',
+            conf.set('identity', 'admin_password', cloud_creds['password'])
-                         cloud_creds['project_name'])
+        conf.set('identity', 'uri', cloud_creds['auth_url'])
-                conf.set('identity', 'admin_password', cloud_creds['password'])
-            conf.set('identity', 'uri', cloud_creds['auth_url'])
 
-        except cfg.NoSuchOptError:
+    except cfg.NoSuchOptError:
-            LOG.warning(
+        LOG.warning(
-                'Could not load some identity options from cloud config file')
+            'Could not load some identity options from cloud config file')
 
 
 class ProjectsClient(object):
@@ -1114,5 +1110,65 @@ def _find_image(client, image_id, image_name):
         return None
 
 
+def main():
+    args = parse_arguments()
+    set_logging(args.debug, args.verbose)
+
+    conf = TempestConf()
+    cloud_creds = args.config.get('auth')
+    set_options(conf, args.deployer_input, args.non_admin,
+                args.overrides, args.test_accounts, cloud_creds)
+
+    uri = conf.get("identity", "uri")
+    api_version = 2
+    if "v3" in uri:
+        api_version = 3
+        conf.set("identity", "auth_version", "v3")
+        conf.set("identity", "uri_v3", uri)
+    else:
+        # TODO(arxcruz) make a check if v3 is enabled
+        conf.set("identity", "uri_v3", uri.replace("v2.0", "v3"))
+
+    clients = ClientManager(conf, not args.non_admin)
+    swift_discover = conf.get_defaulted('object-storage-feature-enabled',
+                                        'discoverability')
+    services = api_discovery.discover(
+        clients.auth_provider,
+        clients.identity_region,
+        object_store_discovery=conf.get_bool_value(swift_discover),
+        api_version=api_version,
+        disable_ssl_certificate_validation=conf.get_defaulted(
+            'identity',
+            'disable_ssl_certificate_validation'
+        )
+    )
+    if args.create and args.test_accounts is None:
+        create_tempest_users(clients.tenants, clients.roles, clients.users,
+                             conf, services)
+    create_tempest_flavors(clients.flavors, conf, args.create)
+    create_tempest_images(clients.images, conf, args.image, args.create,
+                          args.image_disk_format)
+    has_neutron = "network" in services
+
+    LOG.info("Setting up network")
+    LOG.debug("Is neutron present: {0}".format(has_neutron))
+    create_tempest_networks(clients, conf, has_neutron, args.network_id)
+
+    configure_discovered_services(conf, services)
+    check_volume_backup_service(clients.volume_service, conf, services)
+    check_ceilometer_service(clients.service_client, conf, services)
+    configure_boto(conf, services)
+    configure_keystone_feature_flags(conf, services)
+    configure_horizon(conf)
+
+    # remove all unwanted values if were specified
+    if args.remove != {}:
+        LOG.info("Removing configuration: %s", str(args.remove))
+        conf.remove_values(args)
+    LOG.info("Creating configuration file %s", os.path.abspath(args.out))
+    with open(args.out, 'w') as f:
+        conf.write(f)
+
+
 if __name__ == "__main__":
     main()

config_tempest/tests/test_config_tempest.py

@@ -165,45 +165,38 @@ class TestOsClientConfigSupport(BaseConfigTempestTest):
                      'get_project_by_name')
         self.useFixture(MonkeyPatch(func2mock, mock_function))
 
-    def _obtain_client_config_data(self, mock_args, admin):
+    def _obtain_client_config_data(self, non_admin):
         cloud_args = {
             'username': 'cloud_user',
             'password': 'cloud_pass',
             'project_name': 'cloud_project',
             'auth_url': 'http://auth.url.com/'
         }
-        mock_function = mock.Mock(return_value=cloud_args)
-        func2mock = 'os_client_config.cloud_config.CloudConfig.config.get'
-        self.useFixture(MonkeyPatch(func2mock, mock_function))
         # create an empty conf
         conf = tool.TempestConf()
         conf.set('identity', 'uri', cloud_args['auth_url'], priority=True)
         # call the function and check if data were obtained properly
-        tool.set_cloud_config_values(conf, mock_args)
+        tool.set_cloud_config_values(non_admin, cloud_args, conf)
-        if admin:
+        if non_admin:
-            self.assertEqual(cloud_args['username'],
-                             conf.get('identity', 'admin_username'))
-            self.assertEqual(cloud_args['password'],
-                             conf.get('identity', 'admin_password'))
-            self.assertEqual(cloud_args['project_name'],
-                             conf.get('identity', 'admin_tenant_name'))
-        else:
             self.assertEqual(cloud_args['username'],
                              conf.get('identity', 'username'))
             self.assertEqual(cloud_args['password'],
                              conf.get('identity', 'password'))
             self.assertEqual(cloud_args['project_name'],
                              conf.get('identity', 'tenant_name'))
+        else:
+            self.assertEqual(cloud_args['username'],
+                             conf.get('identity', 'admin_username'))
+            self.assertEqual(cloud_args['password'],
+                             conf.get('identity', 'admin_password'))
+            self.assertEqual(cloud_args['project_name'],
+                             conf.get('identity', 'admin_tenant_name'))
 
-    @mock.patch('os_client_config.cloud_config.CloudConfig',
+    def test_init_manager_client_config(self):
-                **{'non_admin': True})
+        self._obtain_client_config_data(True)
-    def test_init_manager_client_config(self, mock_args):
-        self._obtain_client_config_data(mock_args, False)
 
-    @mock.patch('os_client_config.cloud_config.CloudConfig',
+    def test_init_manager_client_config_as_admin(self):
-                **{'non_admin': False})
+        self._obtain_client_config_data(False)
-    def test_init_manager_client_config_as_admin(self, mock_args):
-        self._obtain_client_config_data(mock_args, True)
 
     @mock.patch('os_client_config.cloud_config.CloudConfig')
     def test_init_manager_client_config_get_default(self, mock_args):