From 06dc06c53240340afbc7dbc5daf4b5b3dee5d95a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Mu=C3=B1oz?= Date: Fri, 30 Jun 2023 15:49:22 +0200 Subject: [PATCH] Added cli params preference over config file. For this, we refactored the CIVConfig class and changed the behavior to allow combined options (a config file and some params to override what is already present in the config). Also, if the config file does not exist, it will always be created with default values and then updated with the values provided via cli params. --- cloud-image-val.py | 55 ++++++++--------- lib/config_lib.py | 96 ++++++++++++++++++++++-------- main/cloud_image_validator.py | 34 ++++++----- test/test_cloud_image_validator.py | 20 +++---- 4 files changed, 123 insertions(+), 82 deletions(-) diff --git a/cloud-image-val.py b/cloud-image-val.py index f8686776..e0f20a2c 100644 --- a/cloud-image-val.py +++ b/cloud-image-val.py @@ -1,5 +1,4 @@ import os -import json from pprint import pprint from argparse import ArgumentParser, RawTextHelpFormatter @@ -11,11 +10,14 @@ parser.add_argument('-r', '--resources-file', help='Path to the resources JSON file that contains the Cloud provider and the images to use.\n' - 'See cloud/sample/resources_.json to know about the expected file structure.') + 'See cloud/sample/resources_.json to know about the expected file structure.', + default=None) parser.add_argument('-o', '--output-file', - help='Output file path of the resultant Junit XML test report and others') + help='Output file path of the resultant Junit XML test report and others', + default=None) parser.add_argument('-t', '--test-filter', - help='Use this option to filter tests execution by test name') + help='Use this option to filter tests execution by test name', + default=None) parser.add_argument('-m', '--include-markers', help='Use this option to specify which tests to run that match a pytest markers expression.\n' 'The only marker currently supported is "pub" (see pytest.ini for more details)\n' @@ -26,23 +28,23 @@ '--> https://doc.pytest.org/en/latest/example/markers.html', default=None) parser.add_argument('-p', '--parallel', - help='Use this option to enable parallel test execution mode. Default is DISABLED', - default=False, - action='store_true') + help='Use this option to enable parallel test execution mode.', + action='store_true', + default=None) parser.add_argument('-d', '--debug', - help='Use this option to enable debugging mode. Default is DISABLED', - default=False, - action='store_true') + help='Use this option to enable debugging mode.', + action='store_true', + default=None) parser.add_argument('-s', '--stop-cleanup', help='Use this option to enable stop cleanup process until a key is pressed. \n' - 'Helpful when you need to connect through ssh to an instance. Default is DISABLED', - default=False, - action='store_true') + 'Helpful when you need to connect through ssh to an instance.', + action='store_true', + default=None) parser.add_argument('-e', '--environment', - help='Use this option to set what invironment CIV is going to run on.\n' - 'This can change CIV bahaviour like how "-s" works. this option can be\n' - 'set to "automated" or "local". Default is "local"', - default="local") + help='Use this option to set what environment CIV is going to run on.\n' + 'This can change CIV behaviour like how "-s" works. this option can be\n' + 'set to "automated" or "local".', + default=None) parser.add_argument('-c', '--config-file', help='Use this option to pass CLI options through a config file.\n' 'This config should be in yaml format, examples can be found in the README', @@ -62,23 +64,14 @@ os.environ['PYTHONPATH'] = ':'.join( [f'{os.path.dirname(__file__)}', os.environ['PYTHONPATH']]) - if args.config_file: - civ_config = CIVConfig(args.config_file) - civ_config.validate() - config = civ_config.get_config() - else: - assert (args.resources_file is not None), 'ERROR: Please provide a resources file' - assert (args.output_file is not None), 'ERROR: Please provide an output path' + config_manager = CIVConfig(args) - if args.tags: - args.tags = json.loads(args.tags) + config_manager.update_config() + config_manager.validate_config() - civ_config = CIVConfig() - args.config_file = civ_config.config_path - civ_config.write_config(args) - config = civ_config.get_config() + config = config_manager.get_config() - if config["debug"]: + if config['debug']: console_lib.print_divider('Config') pprint(config) diff --git a/lib/config_lib.py b/lib/config_lib.py index b861dceb..ea009331 100644 --- a/lib/config_lib.py +++ b/lib/config_lib.py @@ -1,27 +1,74 @@ +import os.path + import yaml -class CIVConfig(): - def __init__(self, config_path='/tmp/civ_config.yml'): - self.config_path = config_path +class CIVConfig: + config_path = '/tmp/civ_config.yaml' + command_line_args = {} + + config_file_arg_name = 'config_file' + + def __init__(self, args=None): + if args.config_file is not None: + self.config_path = args.config_file - def validate(self): + self.command_line_args = args.__dict__ + + def validate_config(self): with open(self.config_path) as config_file: try: config = yaml.safe_load(config_file) except Exception as e: - print(f'ERROR: loading the config yaml failed, please check the sintax.\n{e}') - exit() + print('ERROR: Failed to load the config yaml, please check the syntax.') + print(e) + exit(1) assert 'resources_file' in config.keys(), 'ERROR: Please provide a resources file' assert 'output_file' in config.keys(), 'ERROR: Please provide an output path' - self.set_defaults(config) - - def write_config(self, args): - args = args.__dict__ + def write_config(self, config_to_write): with open(self.config_path, 'w+') as config_file: - yaml.dump(args, config_file) + yaml.dump(config_to_write, config_file) + + def update_config(self): + if os.path.exists(self.config_path): + config = self.get_config() + else: + config = self.get_default_config() + + if len(self.command_line_args) == 1 and self.config_file_arg_name in self.command_line_args: + return + + self.command_line_args.pop(self.config_file_arg_name) + + for arg_name, arg_value in self.command_line_args.items(): + if arg_name not in config: + config[arg_name] = arg_value + + if arg_value == config[arg_name] or arg_value is None: + continue + + print(f'DEBUG: Overriding "{arg_name}" config item...') + + if arg_name == 'tags': + config[arg_name] = self.get_tags_dict_from_command_line_arg_value(arg_value) + continue + + config[arg_name] = arg_value + + self.write_config(config) + + def get_tags_dict_from_command_line_arg_value(self, tags_arg_value): + tags_dict = {} + + tags_list = tags_arg_value.split(',') + + for t in tags_list: + tag_data = t.split(':') + tags_dict[tag_data[0].strip()] = tag_data[1].strip() + + return tags_dict def get_config(self): with open(self.config_path) as config_file: @@ -29,18 +76,17 @@ def get_config(self): return config - def set_defaults(self, config): - config_defaults = {'environment': 'local', - 'tags': None, - 'debug': False, - 'include_markers': None, - 'parallel': False, - 'stop_cleanup': False, - 'test_filter': None} + def get_default_config(self): + config_defaults = { + 'resources_file': None, + 'output_file': None, + 'environment': 'local', + 'tags': None, + 'debug': False, + 'include_markers': None, + 'parallel': False, + 'stop_cleanup': None, + 'test_filter': None, + } - for default in config_defaults: - if default not in config: - config[default] = config_defaults[default] - - with open(self.config_path, 'w+') as config_file: - yaml.dump(config, config_file) + return config_defaults diff --git a/main/cloud_image_validator.py b/main/cloud_image_validator.py index 491b8134..69006605 100644 --- a/main/cloud_image_validator.py +++ b/main/cloud_image_validator.py @@ -19,6 +19,8 @@ class CloudImageValidator: infra_controller = None infra_configurator = None + infra_error_exit_code = 100 + def __init__(self, config): self.config = config @@ -37,23 +39,24 @@ def main(self): console_lib.print_divider('Running tests') wait_status = self.run_tests_in_all_instances(instances) + exit_code = wait_status >> 8 except Exception as e: print(e) - exit_code = 1 + exit_code = self.infra_error_exit_code finally: - if self.config["stop_cleanup"]: - if self.config["environment"] == "local": + if self.config['stop_cleanup']: + if self.config['environment'] == 'local': self.print_ssh_commands_for_instances(instances) input('Press ENTER to proceed with cleanup:') - elif self.config["environment"] == "automated": + elif self.config["environment"] == 'automated': console_lib.print_divider('Skipping cleanup') return exit_code else: print('ERROR: --environment parameter should be either "local" or "automated"') - exit() + exit_code = self.infra_error_exit_code console_lib.print_divider('Cleanup') self.cleanup() @@ -74,20 +77,20 @@ def initialize_infrastructure(self): ssh_lib.generate_ssh_key_pair(self.ssh_identity_file) self.infra_configurator = TerraformConfigurator(ssh_key_path=self.ssh_pub_key_file, - resources_path=self.config["resources_file"], + resources_path=self.config['resources_file'], config=self.config) self.infra_configurator.configure_from_resources_json() - if self.config["debug"]: + if self.config['debug']: self.infra_configurator.print_configuration() - return TerraformController(self.infra_configurator, self.config["debug"]) + return TerraformController(self.infra_configurator, self.config['debug']) def deploy_infrastructure(self): self.infra_controller.create_infra() instances = self.infra_controller.get_instances() - if self.config["debug"]: + if self.config['debug']: pprint(instances) self._write_instances_to_json(instances) @@ -106,19 +109,18 @@ def run_tests_in_all_instances(self, instances): runner = SuiteRunner(cloud_provider=self.infra_configurator.cloud_name, instances=instances, ssh_config=self.ssh_config_file, - parallel=self.config["parallel"], - debug=self.config["debug"]) + parallel=self.config['parallel'], + debug=self.config['debug']) - return runner.run_tests(self.config["output_file"], - self.config["test_filter"], - self.config["include_markers"]) + return runner.run_tests(self.config['output_file'], + self.config['test_filter'], + self.config['include_markers']) def cleanup(self): self.infra_controller.destroy_infra() - if not self.config["debug"]: + if not self.config['debug']: os.remove(self.ssh_identity_file) os.remove(self.ssh_pub_key_file) os.remove(self.ssh_config_file) os.remove(self.instances_json) - os.remove(self.config["config_file"]) diff --git a/test/test_cloud_image_validator.py b/test/test_cloud_image_validator.py index 4a46f8ad..fd91607b 100644 --- a/test/test_cloud_image_validator.py +++ b/test/test_cloud_image_validator.py @@ -8,14 +8,14 @@ class TestCloudImageValidator: - test_config = {"resources_file": '/fake/test/resources_file.json', - "output_file": '/fake/test/output_file.xml', - "test_filter": 'test_test_name', - "include_markers": 'pub', - "parallel": True, - "debug": True, - "stop_cleanup": False, - "config_file": "/fake/test/config_file.yml" + test_config = {'resources_file': '/fake/test/resources_file.json', + 'output_file': '/fake/test/output_file.xml', + 'test_filter': 'test_test_name', + 'include_markers': 'pub', + 'parallel': True, + 'debug': True, + 'stop_cleanup': False, + 'config_file': '/tmp/test_config_file.yml', } test_instances = { 'instance-1': {'public_dns': 'value_1', 'username': 'value_2'}, @@ -57,6 +57,7 @@ def test_main(self, mocker, validator): # Assert assert result == exit_code_test + assert mock_print_divider.call_args_list == [ mocker.call('Deploying infrastructure'), mocker.call('Preparing environment'), @@ -151,6 +152,5 @@ def test_destroy_infrastructure(self, mocker, validator): mocker.call(validator.ssh_identity_file), mocker.call(validator.ssh_pub_key_file), mocker.call(validator.ssh_config_file), - mocker.call(validator.instances_json), - mocker.call(validator.config["config_file"]) + mocker.call(validator.instances_json) ]