diff options
| -rw-r--r-- | callback_plugins/openshift_quick_installer.py | 37 | ||||
| -rw-r--r-- | utils/src/ooinstall/cli_installer.py | 17 | ||||
| -rw-r--r-- | utils/src/ooinstall/oo_config.py | 1 | ||||
| -rw-r--r-- | utils/src/ooinstall/openshift_ansible.py | 3 | ||||
| -rw-r--r-- | 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 <http://www.gnu.org/licenses/>. -""" -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') | 
