From 76f1cd09097dc28c9efe7e2476534fb8a9cfede5 Mon Sep 17 00:00:00 2001 From: Tim Bielawa Date: Mon, 26 Sep 2016 12:03:36 -0700 Subject: 'fix' unittests by removing the users ability to specify an ansible config --- callback_plugins/openshift_quick_installer.py | 37 +++--- utils/src/ooinstall/cli_installer.py | 17 +-- utils/src/ooinstall/oo_config.py | 1 - utils/src/ooinstall/openshift_ansible.py | 3 +- utils/test/cli_installer_tests.py | 166 ++++++++++++++------------ 5 files changed, 119 insertions(+), 105 deletions(-) diff --git a/callback_plugins/openshift_quick_installer.py b/callback_plugins/openshift_quick_installer.py index f6cc43cf9..7923a9a80 100644 --- a/callback_plugins/openshift_quick_installer.py +++ b/callback_plugins/openshift_quick_installer.py @@ -1,3 +1,5 @@ +# pylint: disable=invalid-name,protected-access,import-error,line-too-long + # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by # the Free Software Foundation, either version 3 of the License, or @@ -11,8 +13,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -""" -This file is a stdout callback plugin for the OpenShift Quick +"""This file is a stdout callback plugin for the OpenShift Quick Installer. The purpose of this callback plugin is to reduce the amount of produced output for customers and enable simpler progress checking. @@ -24,13 +25,21 @@ What's different: * The Tasks and Handlers in each play (and included roles) are printed as a series of .'s following the play progress line. +* Many of these methods include copy and paste code from the upstream + default.py callback. We do that to give us control over the stdout + output while allowing Ansible to handle the file logging + normally. The biggest changes here are that we are manually setting + `log_only` to True in the Display.display method and we redefine the + Display.banner method locally so we can set log_only on that call as + well. + """ from __future__ import (absolute_import, print_function) import imp import os import sys - +from ansible import constants as C ANSIBLE_PATH = imp.find_module('ansible')[1] DEFAULT_PATH = os.path.join(ANSIBLE_PATH, 'plugins/callback/default.py') DEFAULT_MODULE = imp.load_source( @@ -44,7 +53,6 @@ try: except ImportError: # < ansible 2.1 BASECLASS = DEFAULT_MODULE.CallbackModule -from ansible import constants as C reload(sys) sys.setdefaultencoding('utf-8') @@ -63,9 +71,11 @@ class CallbackModule(DEFAULT_MODULE.CallbackModule): plays_total_ran = 0 def banner(self, msg, color=None): - ''' - Prints a header-looking line with stars taking up to 80 columns + '''Prints a header-looking line with stars taking up to 80 columns of width (3 columns, minimum) + + Overrides the upstream banner method so that display is called + with log_only=True ''' msg = msg.strip() star_len = (79 - len(msg)) @@ -88,9 +98,8 @@ class CallbackModule(DEFAULT_MODULE.CallbackModule): We could print the number of tasks here as well by using `play.get_tasks()` but that is not accurate when a play includes a -role. Only the tasks directly assigned to a play are directly exposed -in the `play` object. - +role. Only the tasks directly assigned to a play are exposed in the +`play` object. """ self.plays_total_ran += 1 print("") @@ -187,14 +196,14 @@ The only thing we change here is adding `log_only=True` to the self._process_items(result) else: - if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and not '_ansible_verbose_override' in result._result: + if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and '_ansible_verbose_override' not in result._result: msg += " => %s" % (self._dump_results(result._result),) self._display.display(msg, color=color, log_only=True) self._handle_warnings(result._result) def v2_runner_item_on_ok(self, result): - """Print out task results for you're iterating""" + """Print out task results for items you're iterating over""" delegated_vars = result._result.get('_ansible_delegated_vars', None) if result._task.action in ('include', 'include_role'): return @@ -212,7 +221,7 @@ The only thing we change here is adding `log_only=True` to the msg += " => (item=%s)" % (self._get_item(result._result),) - if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and not '_ansible_verbose_override' in result._result: + if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and '_ansible_verbose_override' not in result._result: msg += " => %s" % self._dump_results(result._result) self._display.display(msg, color=color, log_only=True) @@ -220,7 +229,7 @@ The only thing we change here is adding `log_only=True` to the """Print out task results when an item is skipped""" if C.DISPLAY_SKIPPED_HOSTS: msg = "skipping: [%s] => (item=%s) " % (result._host.get_name(), self._get_item(result._result)) - if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and not '_ansible_verbose_override' in result._result: + if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and '_ansible_verbose_override' not in result._result: msg += " => %s" % self._dump_results(result._result) self._display.display(msg, color=C.COLOR_SKIP, log_only=True) @@ -231,7 +240,7 @@ The only thing we change here is adding `log_only=True` to the self._process_items(result) else: msg = "skipping: [%s]" % result._host.get_name() - if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and not '_ansible_verbose_override' in result._result: + if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and '_ansible_verbose_override' not in result._result: msg += " => %s" % self._dump_results(result._result) self._display.display(msg, color=C.COLOR_SKIP, log_only=True) diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 81dda2ea4..674d1cee4 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -816,12 +816,6 @@ def set_infra_nodes(hosts): # callback=validate_ansible_dir, default=DEFAULT_PLAYBOOK_DIR, envvar='OO_ANSIBLE_PLAYBOOK_DIRECTORY') -@click.option('--ansible-config', - type=click.Path(file_okay=True, - dir_okay=False, - writable=True, - readable=True), - default=None) @click.option('--ansible-log-path', type=click.Path(file_okay=True, dir_okay=False, @@ -837,7 +831,7 @@ def set_infra_nodes(hosts): # pylint: disable=too-many-arguments # pylint: disable=line-too-long # Main CLI entrypoint, not much we can do about too many arguments. -def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_config, ansible_log_path, verbose, debug): +def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_log_path, verbose, debug): """ atomic-openshift-installer makes the process for installing OSE or AEP easier by interactively gathering the data needed to run on each host. @@ -856,7 +850,6 @@ def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_conf ctx.obj = {} ctx.obj['unattended'] = unattended ctx.obj['configuration'] = configuration - ctx.obj['ansible_config'] = ansible_config ctx.obj['ansible_log_path'] = ansible_log_path ctx.obj['verbose'] = verbose @@ -877,14 +870,12 @@ def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_conf oo_cfg.ansible_playbook_directory = ansible_playbook_directory ctx.obj['ansible_playbook_directory'] = ansible_playbook_directory - if ctx.obj['ansible_config']: - oo_cfg.settings['ansible_config'] = ctx.obj['ansible_config'] - elif 'ansible_config' not in oo_cfg.settings and \ - os.path.exists(DEFAULT_ANSIBLE_CONFIG): + if os.path.exists(DEFAULT_ANSIBLE_CONFIG): # If we're installed by RPM this file should exist and we can use it as our default: oo_cfg.settings['ansible_config'] = DEFAULT_ANSIBLE_CONFIG - oo_cfg.settings['ansible_quiet_config'] = QUIET_ANSIBLE_CONFIG + if os.path.exists(QUIET_ANSIBLE_CONFIG): + oo_cfg.settings['ansible_quiet_config'] = QUIET_ANSIBLE_CONFIG oo_cfg.settings['ansible_log_path'] = ctx.obj['ansible_log_path'] diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index ce0ae8deb..8c5f3396b 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -12,7 +12,6 @@ installer_log = logging.getLogger('installer') CONFIG_PERSIST_SETTINGS = [ 'ansible_ssh_user', 'ansible_callback_facts_yaml', - 'ansible_config', 'ansible_inventory_path', 'ansible_log_path', 'deployment', diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 79c60af82..eb1e61a60 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -286,9 +286,10 @@ def run_main_playbook(inventory_file, hosts, hosts_to_run_on, verbose=False): facts_env = os.environ.copy() if 'ansible_log_path' in CFG.settings: facts_env['ANSIBLE_LOG_PATH'] = CFG.settings['ansible_log_path'] + + # override the ansible config for our main playbook run if 'ansible_quiet_config' in CFG.settings: facts_env['ANSIBLE_CONFIG'] = CFG.settings['ansible_quiet_config'] - # facts_env["ANSIBLE_CALLBACK_PLUGINS"] = CFG.settings['ansible_plugins_directory'] return run_ansible(main_playbook_path, inventory_file, facts_env, verbose) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 6d9d443ff..34392777b 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -599,82 +599,96 @@ class UnattendedCliTests(OOCliFixture): self.assertEquals('openshift-enterprise', inventory.get('OSEv3:vars', 'deployment_type')) - @patch('ooinstall.openshift_ansible.run_ansible') - @patch('ooinstall.openshift_ansible.load_system_facts') - def test_no_ansible_config_specified(self, load_facts_mock, run_ansible_mock): - load_facts_mock.return_value = (MOCK_FACTS, 0) - run_ansible_mock.return_value = 0 - - config = SAMPLE_CONFIG % 'openshift-enterprise' - - self._ansible_config_test(load_facts_mock, run_ansible_mock, - config, None, None) - - @patch('ooinstall.openshift_ansible.run_ansible') - @patch('ooinstall.openshift_ansible.load_system_facts') - def test_ansible_config_specified_cli(self, load_facts_mock, run_ansible_mock): - load_facts_mock.return_value = (MOCK_FACTS, 0) - run_ansible_mock.return_value = 0 - - config = SAMPLE_CONFIG % 'openshift-enterprise' - ansible_config = os.path.join(self.work_dir, 'ansible.cfg') - - self._ansible_config_test(load_facts_mock, run_ansible_mock, - config, ansible_config, ansible_config) - - @patch('ooinstall.openshift_ansible.run_ansible') - @patch('ooinstall.openshift_ansible.load_system_facts') - def test_ansible_config_specified_in_installer_config(self, - load_facts_mock, run_ansible_mock): - - load_facts_mock.return_value = (MOCK_FACTS, 0) - run_ansible_mock.return_value = 0 - - ansible_config = os.path.join(self.work_dir, 'ansible.cfg') - config = SAMPLE_CONFIG % 'openshift-enterprise' - config = "%s\nansible_config: %s" % (config, ansible_config) - self._ansible_config_test(load_facts_mock, run_ansible_mock, - config, None, ansible_config) - - #pylint: disable=too-many-arguments - # This method allows for drastically simpler tests to write, and the args - # are all useful. - def _ansible_config_test(self, load_facts_mock, run_ansible_mock, - installer_config, ansible_config_cli=None, expected_result=None): - """ - Utility method for testing the ways you can specify the ansible config. - """ - - load_facts_mock.return_value = (MOCK_FACTS, 0) - run_ansible_mock.return_value = 0 - - config_file = self.write_config(os.path.join(self.work_dir, - 'ooinstall.conf'), installer_config) - - self.cli_args.extend(["-c", config_file]) - if ansible_config_cli: - self.cli_args.extend(["--ansible-config", ansible_config_cli]) - self.cli_args.append("install") - result = self.runner.invoke(cli.cli, self.cli_args) - self.assert_result(result, 0) - - # Test the env vars for facts playbook: - facts_env_vars = load_facts_mock.call_args[0][2] - if expected_result: - self.assertEquals(expected_result, facts_env_vars['ANSIBLE_CONFIG']) - else: - # If user running test has rpm installed, this might be set to default: - self.assertTrue('ANSIBLE_CONFIG' not in facts_env_vars or - facts_env_vars['ANSIBLE_CONFIG'] == cli.DEFAULT_ANSIBLE_CONFIG) - - # Test the env vars for main playbook: - env_vars = run_ansible_mock.call_args[0][2] - if expected_result: - self.assertEquals(expected_result, env_vars['ANSIBLE_CONFIG']) - else: - # If user running test has rpm installed, this might be set to default: - self.assertTrue('ANSIBLE_CONFIG' not in env_vars or - env_vars['ANSIBLE_CONFIG'] == cli.DEFAULT_ANSIBLE_CONFIG) + # 2016-09-26 - tbielawa - COMMENTING OUT these tests FOR NOW while + # we wait to see if anyone notices that we took away their ability + # to set the ansible_config parameter in the command line options + # and in the installer config file. + # + # We have removed the ability to set the ansible config file + # manually so that our new quieter output mode is the default and + # only output mode. + # + # RE: https://trello.com/c/DSwwizwP - atomic-openshift-install + # should only output relevant information. + + # @patch('ooinstall.openshift_ansible.run_ansible') + # @patch('ooinstall.openshift_ansible.load_system_facts') + # def test_no_ansible_config_specified(self, load_facts_mock, run_ansible_mock): + # load_facts_mock.return_value = (MOCK_FACTS, 0) + # run_ansible_mock.return_value = 0 + + # config = SAMPLE_CONFIG % 'openshift-enterprise' + + # self._ansible_config_test(load_facts_mock, run_ansible_mock, + # config, None, None) + + # @patch('ooinstall.openshift_ansible.run_ansible') + # @patch('ooinstall.openshift_ansible.load_system_facts') + # def test_ansible_config_specified_cli(self, load_facts_mock, run_ansible_mock): + # load_facts_mock.return_value = (MOCK_FACTS, 0) + # run_ansible_mock.return_value = 0 + + # config = SAMPLE_CONFIG % 'openshift-enterprise' + # ansible_config = os.path.join(self.work_dir, 'ansible.cfg') + + # self._ansible_config_test(load_facts_mock, run_ansible_mock, + # config, ansible_config, ansible_config) + + # @patch('ooinstall.openshift_ansible.run_ansible') + # @patch('ooinstall.openshift_ansible.load_system_facts') + # def test_ansible_config_specified_in_installer_config(self, + # load_facts_mock, run_ansible_mock): + + # load_facts_mock.return_value = (MOCK_FACTS, 0) + # run_ansible_mock.return_value = 0 + + # ansible_config = os.path.join(self.work_dir, 'ansible.cfg') + # config = SAMPLE_CONFIG % 'openshift-enterprise' + # config = "%s\nansible_config: %s" % (config, ansible_config) + # self._ansible_config_test(load_facts_mock, run_ansible_mock, + # config, None, ansible_config) + + # #pylint: disable=too-many-arguments + # # This method allows for drastically simpler tests to write, and the args + # # are all useful. + # def _ansible_config_test(self, load_facts_mock, run_ansible_mock, + # installer_config, ansible_config_cli=None, expected_result=None): + # """ + # Utility method for testing the ways you can specify the ansible config. + # """ + + # load_facts_mock.return_value = (MOCK_FACTS, 0) + # run_ansible_mock.return_value = 0 + + # config_file = self.write_config(os.path.join(self.work_dir, + # 'ooinstall.conf'), installer_config) + + # self.cli_args.extend(["-c", config_file]) + # if ansible_config_cli: + # self.cli_args.extend(["--ansible-config", ansible_config_cli]) + # self.cli_args.append("install") + # result = self.runner.invoke(cli.cli, self.cli_args) + # self.assert_result(result, 0) + + # # Test the env vars for facts playbook: + # facts_env_vars = load_facts_mock.call_args[0][2] + # if expected_result: + # self.assertEquals(expected_result, facts_env_vars['ANSIBLE_CONFIG']) + # else: + # # If user running test has rpm installed, this might be set to default: + # self.assertTrue('ANSIBLE_CONFIG' not in facts_env_vars or + # facts_env_vars['ANSIBLE_CONFIG'] == cli.DEFAULT_ANSIBLE_CONFIG) + + # # Test the env vars for main playbook: + # env_vars = run_ansible_mock.call_args[0][2] + # if expected_result: + # self.assertEquals(expected_result, env_vars['ANSIBLE_CONFIG']) + # else: + # # If user running test has rpm installed, this might be set to default: + # # + # # By default we will use the quiet config + # self.assertTrue('ANSIBLE_CONFIG' not in env_vars or + # env_vars['ANSIBLE_CONFIG'] == cli.QUIET_ANSIBLE_CONFIG) # unattended with bad config file and no installed hosts (without --force) @patch('ooinstall.openshift_ansible.run_main_playbook') -- cgit v1.2.3