From 4d1df7b0e75260c96164fd5a21347f5ce18c7a8b Mon Sep 17 00:00:00 2001 From: Jiri Podivin Date: Wed, 5 May 2021 11:18:27 +0200 Subject: [PATCH] New optional param for log path As we now have a mechanism for falling back to a default directory for logs and artifacts, we can allow the customers to specify their own. This patch add the argument to the CLI along with minimum necessary information about the usage. Signed-off-by: Jiri Podivin Change-Id: Id31a7ce2dfc4d9a132fea7d600e8beb8adc0739b (cherry picked from commit 7876cab41586157a1b5e8fd426deb16d79567a52) --- validations_libs/cli/run.py | 8 +++++ validations_libs/tests/cli/test_run.py | 48 ++++++++++++++++++-------- validations_libs/utils.py | 6 ++-- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/validations_libs/cli/run.py b/validations_libs/cli/run.py index ca4ca5b9..765cb744 100644 --- a/validations_libs/cli/run.py +++ b/validations_libs/cli/run.py @@ -53,6 +53,13 @@ class Run(BaseCommand): help=("Path where the ansible roles, library " "and plugins are located.")) + parser.add_argument( + '--validation-log-dir', + dest='validation_log_dir', + default=constants.VALIDATIONS_LOG_BASEDIR, + help=( + "Path where the log files and artifacts will be located. ")) + parser.add_argument('--inventory', '-i', type=str, default="localhost", help="Path of the Ansible inventory.") @@ -169,6 +176,7 @@ class Run(BaseCommand): python_interpreter=parsed_args.python_interpreter, quiet=quiet_mode, ssh_user=parsed_args.ssh_user, + log_path=parsed_args.validation_log_dir ) except RuntimeError as e: raise RuntimeError(e) diff --git a/validations_libs/tests/cli/test_run.py b/validations_libs/tests/cli/test_run.py index 370c0229..650da240 100644 --- a/validations_libs/tests/cli/test_run.py +++ b/validations_libs/tests/cli/test_run.py @@ -58,13 +58,15 @@ class TestRun(BaseCommand): self.assertRaises(Exception, self.check_parser, self.cmd, arglist, verifylist) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('validations_libs.cli.common.print_dict') @mock.patch('getpass.getuser', return_value='doe') @mock.patch('validations_libs.validation_actions.ValidationActions.' 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) - def test_run_command_extra_vars(self, mock_run, mock_user, mock_print): + def test_run_command_extra_vars(self, mock_run, mock_user, mock_print, + mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -76,7 +78,8 @@ class TestRun(BaseCommand): 'extra_env_vars': None, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-vars', 'key=value'] @@ -87,6 +90,7 @@ class TestRun(BaseCommand): self.cmd.take_action(parsed_args) mock_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('validations_libs.cli.common.print_dict') @mock.patch('getpass.getuser', return_value='doe') @@ -94,7 +98,7 @@ class TestRun(BaseCommand): 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) def test_run_command_extra_vars_twice(self, mock_run, mock_user, - mock_print): + mock_print, mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -106,7 +110,8 @@ class TestRun(BaseCommand): 'extra_env_vars': None, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-vars', 'key=value1', @@ -128,6 +133,7 @@ class TestRun(BaseCommand): self.assertRaises(Exception, self.check_parser, self.cmd, arglist, verifylist) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('yaml.safe_load', return_value={'key': 'value'}) @mock.patch('six.moves.builtins.open') @mock.patch('getpass.getuser', @@ -136,7 +142,8 @@ class TestRun(BaseCommand): 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) def test_run_command_extra_vars_file(self, mock_run, mock_user, mock_open, - mock_yaml): + mock_yaml, mock_log_dir): + run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -148,7 +155,8 @@ class TestRun(BaseCommand): 'extra_env_vars': None, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-vars-file', '/foo/vars.yaml'] @@ -159,12 +167,13 @@ class TestRun(BaseCommand): self.cmd.take_action(parsed_args) mock_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('getpass.getuser', return_value='doe') @mock.patch('validations_libs.validation_actions.ValidationActions.' 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) - def test_run_command_extra_env_vars(self, mock_run, mock_user): + def test_run_command_extra_env_vars(self, mock_run, mock_user, mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -176,7 +185,8 @@ class TestRun(BaseCommand): 'extra_env_vars': {'key': 'value'}, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-env-vars', 'key=value'] @@ -187,6 +197,7 @@ class TestRun(BaseCommand): self.cmd.take_action(parsed_args) mock_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('getpass.getuser', return_value='doe') @mock.patch('validations_libs.validation_actions.ValidationActions.' @@ -194,10 +205,13 @@ class TestRun(BaseCommand): return_value=fakes.FAKE_SUCCESS_RUN) def test_run_command_extra_env_vars_with_custom_callback(self, mock_run, - mock_user): + mock_user, + mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, + 'log_path': mock_log_dir, + 'quiet': False, 'group': [], 'extra_vars': None, 'validations_dir': '/usr/share/ansible/validation-playbooks', @@ -217,12 +231,13 @@ class TestRun(BaseCommand): self.cmd.take_action(parsed_args) mock_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('getpass.getuser', return_value='doe') @mock.patch('validations_libs.validation_actions.ValidationActions.' 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) - def test_run_command_extra_env_vars_twice(self, mock_run, mock_user): + def test_run_command_extra_env_vars_twice(self, mock_run, mock_user, mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -234,7 +249,8 @@ class TestRun(BaseCommand): 'extra_env_vars': {'key': 'value2'}, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-env-vars', 'key=value1', @@ -246,13 +262,16 @@ class TestRun(BaseCommand): self.cmd.take_action(parsed_args) mock_run.assert_called_with(**run_called_args) + @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR') @mock.patch('getpass.getuser', return_value='doe') @mock.patch('validations_libs.validation_actions.ValidationActions.' 'run_validations', return_value=fakes.FAKE_SUCCESS_RUN) - def test_run_command_extra_env_vars_and_extra_vars(self, mock_run, - mock_user): + def test_run_command_extra_env_vars_and_extra_vars(self, + mock_run, + mock_user, + mock_log_dir): run_called_args = { 'inventory': 'localhost', 'limit_hosts': None, @@ -264,7 +283,8 @@ class TestRun(BaseCommand): 'extra_env_vars': {'key2': 'value2'}, 'python_interpreter': sys.executable, 'quiet': True, - 'ssh_user': 'doe'} + 'ssh_user': 'doe', + 'log_path': mock_log_dir} arglist = ['--validation', 'foo', '--extra-vars', 'key=value', diff --git a/validations_libs/utils.py b/validations_libs/utils.py index a27bf005..6308a589 100644 --- a/validations_libs/utils.py +++ b/validations_libs/utils.py @@ -49,7 +49,7 @@ def create_log_dir(log_path=constants.VALIDATIONS_LOG_BASEDIR): try: if os.path.exists(log_path): if os.access(log_path, os.W_OK): - return log_path + return os.path.abspath(log_path) else: LOG.error( ( @@ -76,7 +76,7 @@ def create_log_dir(log_path=constants.VALIDATIONS_LOG_BASEDIR): log_path=log_path)) os.makedirs(log_path) - return log_path + return os.path.abspath(log_path) except (OSError, PermissionError) as error: LOG.error( ( @@ -117,7 +117,7 @@ def create_artifacts_dir(log_path=constants.VALIDATIONS_LOG_BASEDIR, current_time()) try: os.makedirs(validation_artifacts_dir) - return validation_uuid, validation_artifacts_dir + return validation_uuid, os.path.abspath(validation_artifacts_dir) except (OSError, PermissionError): LOG.exception( (