From a947b57094f2fd5da6d54e1c98b2aa947bb37914 Mon Sep 17 00:00:00 2001 From: Kevin Carter Date: Thu, 9 Apr 2020 12:18:20 -0500 Subject: [PATCH] Add the ability to load vars from files This change will provide the ability to load extra vars from files, instead of having to pass options through the CLI parser. By loading vars from file we can ensure options are made more safe and better handled, especially in cases when a given option may be massive, as is the case with `parameter_defaults`. > A new argument has been added to the ansible playbook runner which will allow us to pass options into the method that will be stored in an extravars file, which is then dynamically loaded by ansible-runner. Information on extravars files can be seen here: [0]. > A test has been added to exercise the new extravars file capability. [0]: https://ansible-runner.readthedocs.io/en/latest/intro.html#env-extravars Closes-Bug: #1871338 Change-Id: I9675e587abf3f07e91319a40620a8f4c67fbf97b Signed-off-by: Kevin Carter --- tripleoclient/tests/test_utils.py | 30 +++++++++++++++++++++++ tripleoclient/utils.py | 31 +++++++++++++++++------- tripleoclient/v1/overcloud_deploy.py | 4 ++- tripleoclient/v1/overcloud_parameters.py | 4 ++- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index bd4374dea..142f24a81 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -215,6 +215,36 @@ class TestRunAnsiblePlaybook(TestCase): mock.call().__enter__().write('job_timeout: 2520\n'), # 42m * 60 mock_open.mock_calls) + @mock.patch('six.moves.builtins.open') + @mock.patch('tripleoclient.utils.makedirs') + @mock.patch('os.path.exists', side_effect=(False, True, True)) + def test_run_with_extravar_file(self, mock_exists, mock_mkdir, mock_open): + ansible_runner.ArtifactLoader = mock.MagicMock() + ansible_runner.Runner.run = mock.MagicMock(return_value=('', 0)) + ansible_runner.runner_config = mock.MagicMock() + utils.run_ansible_playbook( + playbook='existing.yaml', + inventory='localhost,', + workdir='/tmp', + extra_vars_file={ + 'foo': 'bar', + 'things': { + 'more': 'options' + }, + 'num': 42 + } + ) + self.assertIn( + mock.call('/tmp/env/extravars', 'w'), + mock_open.mock_calls + ) + self.assertIn( + mock.call().__enter__().write( + 'foo: bar\nnum: 42\nthings:\n more: options\n' + ), + mock_open.mock_calls + ) + class TestRunCommandAndLog(TestCase): def setUp(self): diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 2a7cf3b02..3214da2a8 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -227,11 +227,12 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None, ssh_user='root', key=None, module_path=None, limit_hosts=None, tags=None, skip_tags=None, verbosity=0, quiet=False, extra_vars=None, - plan='overcloud', gathering_policy='smart', - extra_env_variables=None, parallel_run=False, - callback_whitelist=None, ansible_cfg=None, - ansible_timeout=30, reproduce_command=False, - fail_on_rc=True, timeout=None): + extra_vars_file=None, plan='overcloud', + gathering_policy='smart', extra_env_variables=None, + parallel_run=False, callback_whitelist=None, + ansible_cfg=None, ansible_timeout=30, + reproduce_command=False, fail_on_rc=True, + timeout=None): """Simple wrapper for ansible-playbook. :param playbook: Playbook filename. @@ -287,6 +288,10 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None, path of a JSON or YAML file type. :type extra_vars: Either a Dict or the absolute path of JSON or YAML + :param extra_vars_file: Set additional ansible variables using an + extravar file. + :type extra_vars_file: Dictionary + :param plan: Plan name (Defaults to "overcloud"). :type plan: String @@ -360,18 +365,26 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None, if not playbook_dir: playbook_dir = workdir + # Ensure that the ansible-runner env exists + runner_env = os.path.join(workdir, 'env') + makedirs(runner_env) + + if extra_vars_file: + runner_extra_vars = os.path.join(runner_env, 'extravars') + with open(runner_extra_vars, 'w') as f: + f.write(yaml.safe_dump(extra_vars_file, default_flow_style=False)) + if timeout and timeout > 0: - makedirs(os.path.join(workdir, 'env')) - settings_file = os.path.join(workdir, 'env/settings') + settings_file = os.path.join(runner_env, 'settings') timeout_value = timeout * 60 if os.path.exists(settings_file): - with open(os.path.join(workdir, 'env/settings'), 'r') as f: + with open(settings_file, 'r') as f: settings_object = yaml.safe_load(f.read()) settings_object['job_timeout'] = timeout_value else: settings_object = {'job_timeout': timeout_value} - with open(os.path.join(workdir, 'env/settings'), 'w') as f: + with open(settings_file, 'w') as f: f.write(yaml.safe_dump(settings_object, default_flow_style=False)) if isinstance(playbook, (list, set)): diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 51d62dc5f..06f405ab3 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -308,7 +308,9 @@ class DeployOvercloud(command.Command): playbook_dir=constants.ANSIBLE_TRIPLEO_PLAYBOOKS, verbosity=utils.playbook_verbosity(self=self), extra_vars={ - "container": container_name, + "container": container_name + }, + extra_vars_file={ "parameters": params } ) diff --git a/tripleoclient/v1/overcloud_parameters.py b/tripleoclient/v1/overcloud_parameters.py index 95a314e19..9fccdd96e 100644 --- a/tripleoclient/v1/overcloud_parameters.py +++ b/tripleoclient/v1/overcloud_parameters.py @@ -62,7 +62,9 @@ class SetParameters(command.Command): playbook_dir=constants.ANSIBLE_TRIPLEO_PLAYBOOKS, verbosity=utils.playbook_verbosity(self=self), extra_vars={ - "container": parsed_args.name, + "container": parsed_args.name + }, + extra_vars_file={ "parameters": params } )