diff --git a/validations_libs/constants.py b/validations_libs/constants.py index a96e119d..c1ebd566 100644 --- a/validations_libs/constants.py +++ b/validations_libs/constants.py @@ -24,9 +24,7 @@ VALIDATION_GROUPS = ['no-op', 'prep', 'post'] -VALIDATIONS_LOG_BASEDIR = ('/var/log/validations' - if os.path.exists('/var/log/validations') else - os.getcwd()) +VALIDATIONS_LOG_BASEDIR = os.path.join(os.environ.get('HOME'), 'validations') VALIDATION_ANSIBLE_ARTIFACT_PATH = '{}/artifacts/'.format( VALIDATIONS_LOG_BASEDIR) diff --git a/validations_libs/tests/test_utils.py b/validations_libs/tests/test_utils.py index 655c602f..45b3eff7 100644 --- a/validations_libs/tests/test_utils.py +++ b/validations_libs/tests/test_utils.py @@ -47,18 +47,6 @@ class TestUtils(TestCase): utils.get_validations_data, validation) - @mock.patch('validations_libs.utils.current_time', - return_value='2020-04-02T06:58:20.352272Z') - @mock.patch('os.makedirs') - @mock.patch('uuid.uuid4', return_value='1234') - def test_create_artifacts_dir(self, mock_uuid, mock_makedirs, - mock_datetime): - uuid, dir_path = utils.create_artifacts_dir(dir_path='/tmp/foo', - prefix='ntp') - self.assertEqual(uuid, '1234') - self.assertEqual(dir_path, - '/tmp/foo/1234_ntp_2020-04-02T06:58:20.352272Z') - @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK) @mock.patch('six.moves.builtins.open') @mock.patch('glob.glob') diff --git a/validations_libs/tests/test_validation_actions.py b/validations_libs/tests/test_validation_actions.py index 58d7bf3c..ff0355f3 100644 --- a/validations_libs/tests/test_validation_actions.py +++ b/validations_libs/tests/test_validation_actions.py @@ -45,11 +45,9 @@ class TestValidationActions(TestCase): 'My Validation Two Name', ['prep', 'pre-introspection'])])) - @mock.patch('validations_libs.utils.create_artifacts_dir', - return_value=('1234', '/tmp/')) @mock.patch('validations_libs.utils.get_validations_playbook', return_value=['/tmp/foo/fake.yaml']) - def test_validation_skip_validation(self, mock_validation_play, mock_tmp): + def test_validation_skip_validation(self, mock_validation_play): playbook = ['fake.yaml'] inventory = 'tmp/inventory.yaml' @@ -66,16 +64,26 @@ class TestValidationActions(TestCase): limit_hosts=None) self.assertEqual(run_return, []) + @mock.patch('validations_libs.utils.current_time', + return_value='time') + @mock.patch('validations_libs.utils.uuid.uuid4', + return_value='123') + @mock.patch('validations_libs.utils.os.makedirs') + @mock.patch('validations_libs.utils.os.path.exists', + return_value=True) @mock.patch('validations_libs.utils.get_validations_playbook', return_value=['/tmp/foo/fake.yaml']) @mock.patch('validations_libs.ansible.Ansible.run') - @mock.patch('validations_libs.utils.create_artifacts_dir', - return_value=('1234', '/tmp/')) - def test_validation_skip_on_specific_host(self, mock_tmp, mock_ansible_run, - mock_validation_play): + def test_validation_skip_on_specific_host(self, mock_ansible_run, + mock_validation_play, + mock_exists, + mock_makedirs, + mock_uuid, + mock_time): + mock_ansible_run.return_value = ('fake.yaml', 0, 'successful') run_called_args = { - 'workdir': '/tmp/', + 'workdir': '/var/log/validations/artifacts/123_fake.yaml_time', 'playbook': '/tmp/foo/fake.yaml', 'base_dir': '/usr/share/ansible/', 'playbook_dir': '/tmp/foo', @@ -86,11 +94,11 @@ class TestValidationActions(TestCase): 'quiet': True, 'extra_vars': None, 'limit_hosts': '!cloud1', - 'ansible_artifact_path': '/tmp/', 'extra_env_variables': None, 'ansible_cfg': None, 'gathering_policy': 'explicit', - 'log_path': None, + 'ansible_artifact_path': '/var/log/validations/artifacts/123_fake.yaml_time', + 'log_path': '/var/log/validations', 'run_async': False, 'python_interpreter': None, 'ssh_user': None @@ -106,22 +114,33 @@ class TestValidationActions(TestCase): run = ValidationActions() run_return = run.run_validations(playbook, inventory, + log_path='/var/log/validations', validations_dir='/tmp/foo', skip_list=skip_list, limit_hosts='!cloud1', ) mock_ansible_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.utils.current_time', + return_value='time') + @mock.patch('validations_libs.utils.uuid.uuid4', + return_value='123') + @mock.patch('validations_libs.utils.os.makedirs') + @mock.patch('validations_libs.utils.os.path.exists', + return_value=True) @mock.patch('validations_libs.utils.get_validations_playbook', return_value=['/tmp/foo/fake.yaml']) @mock.patch('validations_libs.ansible.Ansible.run') - @mock.patch('validations_libs.utils.create_artifacts_dir', - return_value=('1234', '/tmp/')) - def test_validation_skip_with_limit_host(self, mock_tmp, mock_ansible_run, - mock_validation_play): + def test_validation_skip_with_limit_host(self, mock_ansible_run, + mock_validation_play, + mock_exists, + mock_makedirs, + mock_uuid, + mock_time): + mock_ansible_run.return_value = ('fake.yaml', 0, 'successful') run_called_args = { - 'workdir': '/tmp/', + 'workdir': '/var/log/validations/artifacts/123_fake.yaml_time', 'playbook': '/tmp/foo/fake.yaml', 'base_dir': '/usr/share/ansible/', 'playbook_dir': '/tmp/foo', @@ -135,8 +154,8 @@ class TestValidationActions(TestCase): 'extra_env_variables': None, 'ansible_cfg': None, 'gathering_policy': 'explicit', - 'ansible_artifact_path': '/tmp/', - 'log_path': None, + 'ansible_artifact_path': '/var/log/validations/artifacts/123_fake.yaml_time', + 'log_path': '/var/log/validations', 'run_async': False, 'python_interpreter': None, 'ssh_user': None @@ -152,40 +171,31 @@ class TestValidationActions(TestCase): run = ValidationActions() run_return = run.run_validations(playbook, inventory, + log_path='/var/log/validations', validations_dir='/tmp/foo', skip_list=skip_list, limit_hosts='cloud,cloud1,!cloud2') + mock_ansible_run.assert_called_with(**run_called_args) - @mock.patch('validations_libs.validation_logs.ValidationLogs.get_results') + @mock.patch('validations_libs.validation_actions.ValidationLogs.get_results', + side_effect=fakes.FAKE_SUCCESS_RUN) @mock.patch('validations_libs.utils.parse_all_validations_on_disk') @mock.patch('validations_libs.ansible.Ansible.run') - @mock.patch('validations_libs.utils.create_artifacts_dir', - return_value=('1234', '/tmp/')) - def test_validation_run_success(self, mock_tmp, mock_ansible_run, - mock_validation_dir, mock_results): + def test_validation_run_success(self, mock_ansible_run, + mock_validation_dir, + mock_results): + mock_validation_dir.return_value = [{ 'description': 'My Validation One Description', 'groups': ['prep', 'pre-deployment'], 'id': 'foo', 'name': 'My Validition One Name', 'parameters': {}}] + mock_ansible_run.return_value = ('foo.yaml', 0, 'successful') - mock_results.return_value = [{'Duration': '0:00:01.761', - 'Host_Group': 'overcloud', - 'Status': 'PASSED', - 'Status_by_Host': 'subnode-1,PASSED', - 'UUID': 'foo', - 'Unreachable_Hosts': '', - 'Validations': 'ntp'}] - expected_run_return = [{'Duration': '0:00:01.761', - 'Host_Group': 'overcloud', - 'Status': 'PASSED', - 'Status_by_Host': 'subnode-1,PASSED', - 'UUID': 'foo', - 'Unreachable_Hosts': '', - 'Validations': 'ntp'}] + expected_run_return = fakes.FAKE_SUCCESS_RUN[0] playbook = ['fake.yaml'] inventory = 'tmp/inventory.yaml' @@ -209,17 +219,18 @@ class TestValidationActions(TestCase): @mock.patch('validations_libs.validation_logs.ValidationLogs.get_results') @mock.patch('validations_libs.utils.parse_all_validations_on_disk') @mock.patch('validations_libs.ansible.Ansible.run') - @mock.patch('validations_libs.utils.create_artifacts_dir', - return_value=('1234', '/tmp/')) - def test_validation_run_failed(self, mock_tmp, mock_ansible_run, + def test_validation_run_failed(self, mock_ansible_run, mock_validation_dir, mock_results): + mock_validation_dir.return_value = [{ 'description': 'My Validation One Description', 'groups': ['prep', 'pre-deployment'], 'id': 'foo', 'name': 'My Validition One Name', 'parameters': {}}] + mock_ansible_run.return_value = ('foo.yaml', 0, 'failed') + mock_results.return_value = [{'Duration': '0:00:01.761', 'Host_Group': 'overcloud', 'Status': 'PASSED', @@ -227,6 +238,7 @@ class TestValidationActions(TestCase): 'UUID': 'foo', 'Unreachable_Hosts': '', 'Validations': 'ntp'}] + expected_run_return = [{'Duration': '0:00:01.761', 'Host_Group': 'overcloud', 'Status': 'PASSED', diff --git a/validations_libs/utils.py b/validations_libs/utils.py index dd23c159..d12cb494 100644 --- a/validations_libs/utils.py +++ b/validations_libs/utils.py @@ -32,25 +32,51 @@ def current_time(): return '%sZ' % datetime.datetime.utcnow().isoformat() -def create_artifacts_dir(dir_path=None, prefix=None): - """Create Ansible artifacts directory +def create_log_dir(log_path=constants.VALIDATIONS_LOG_BASEDIR): + """Create default validations log dir. + Log the failure if encountering OSError or PermissionError. + :raises: RuntimeError + """ + try: + if os.path.exists(log_path): + if os.access(log_path, os.W_OK): + return log_path + else: + create_log_dir(constants.VALIDATIONS_LOG_BASEDIR) + else: + os.makedirs(log_path) + return log_path + except (OSError, PermissionError): + LOG.error( + ( + "Error while creating the log directory. " + "Please check the access rights for: '{}'" + ).format(log_path) + ) + # Fallback in default path if log_path != from constants path + if log_path != constants.VALIDATIONS_LOG_BASEDIR: + create_log_dir(constants.VALIDATIONS_LOG_BASEDIR) + raise RuntimeError() - :param dir_path: Directory asbolute path - :type dir_path: `string` + +def create_artifacts_dir(log_path=constants.VALIDATIONS_LOG_BASEDIR, + prefix=None): + """Create Ansible artifacts directory + :param log_path: Directory asbolute path + :type log_path: `string` :param prefix: Playbook name :type prefix: `string` :return: The UUID of the validation and the absolute Path of the log file :rtype: `string`, `string` """ - dir_path = (dir_path if dir_path else - constants.VALIDATION_ANSIBLE_ARTIFACT_PATH) + artifact_dir = os.path.join(log_path, 'artifacts') validation_uuid = str(uuid.uuid4()) - log_dir = "{}/{}_{}_{}".format(dir_path, validation_uuid, + log_dir = "{}/{}_{}_{}".format(artifact_dir, validation_uuid, (prefix if prefix else ''), current_time()) try: os.makedirs(log_dir) return validation_uuid, log_dir - except OSError: + except (OSError, PermissionError): LOG.exception( ( "Error while creating Ansible artifacts log file." diff --git a/validations_libs/validation_actions.py b/validations_libs/validation_actions.py index b7738c93..17528ac9 100644 --- a/validations_libs/validation_actions.py +++ b/validations_libs/validation_actions.py @@ -208,7 +208,8 @@ class ValidationActions(object): extra_env_vars=None, ansible_cfg=None, quiet=True, workdir=None, limit_hosts=None, run_async=False, base_dir=constants.DEFAULT_VALIDATIONS_BASEDIR, - log_path=None, python_interpreter=None, + log_path=constants.VALIDATIONS_LOG_BASEDIR, + python_interpreter=None, skip_list=None, callback_whitelist=None, output_callback='validation_stdout', @@ -248,6 +249,8 @@ class ValidationActions(object): ``constants.DEFAULT_VALIDATIONS_BASEDIR``) :type base_dir: ``string`` :param log_path: The absolute path of the validations logs directory + (Defaults to + ``constants.VALIDATIONS_LOG_BASEDIR``) :type log_path: ``string`` :param python_interpreter: Path to the Python interpreter to be used for module execution on remote targets, @@ -333,6 +336,7 @@ class ValidationActions(object): else: raise RuntimeError("No validations found") + log_path = v_utils.create_log_dir(log_path) self.log.debug('Running the validations with Ansible') results = [] for playbook in playbooks: @@ -343,7 +347,7 @@ class ValidationActions(object): limit_hosts) if _play: validation_uuid, artifacts_dir = v_utils.create_artifacts_dir( - dir_path=log_path, prefix=os.path.basename(playbook)) + log_path=log_path, prefix=os.path.basename(playbook)) run_ansible = v_ansible(validation_uuid) _playbook, _rc, _status = run_ansible.run( workdir=artifacts_dir, @@ -378,7 +382,7 @@ class ValidationActions(object): return results # Return log results uuid = [id['UUID'] for id in results] - vlog = ValidationLogs() + vlog = ValidationLogs(log_path) return vlog.get_results(uuid) def group_information(self, groups):