Merge pull request #912 from twiest/master

sync master -> prod branch

3 files changed, 198 insertions, 41 deletions

diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index 6cdc19f20..d9d3dd80c 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -71,7 +71,7 @@ def delete_hosts(hosts):
                 click.echo("\"{}\" doesn't coorespond to any valid input.".format(del_idx))
     return hosts, None
 
-def collect_hosts():
+def collect_hosts(master_set=False):
     """
         Collect host information from user. This will later be filled in using
         ansible.
@@ -108,8 +108,10 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen
                                       value_proc=validate_prompt_hostname)
 
         host_props['connect_to'] = hostname_or_ip
-
-        host_props['master'] = click.confirm('Will this host be an OpenShift Master?')
+        if not master_set:
+            is_master = click.confirm('Will this host be an OpenShift Master?')
+            host_props['master'] = is_master
+            master_set = True
         host_props['node'] = True
 
         #TODO: Reenable this option once container installs are out of tech preview
@@ -188,7 +190,7 @@ Notes:
     facts_confirmed = click.confirm("Do the above facts look correct?")
     if not facts_confirmed:
         message = """
-Edit %s with the desired values and rerun atomic-openshift-installer with --unattended .
+Edit %s with the desired values and run `atomic-openshift-installer --unattended install` to restart the install.
 """ % oo_cfg.config_path
         click.echo(message)
         # Make sure we actually write out the config file.
@@ -308,7 +310,7 @@ def collect_new_nodes():
 Add new nodes here
     """
     click.echo(message)
-    return collect_hosts()
+    return collect_hosts(True)
 
 def get_installed_hosts(hosts, callback_facts):
     installed_hosts = []
@@ -336,7 +338,9 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose):
             if not unattended:
                 click.echo('By default the installer only adds new nodes to an installed environment.')
                 response = click.prompt('Do you want to (1) only add additional nodes or ' \
-                                        '(2) perform a clean install?', type=int)
+                                        '(2) reinstall the existing hosts ' \
+                                        'potentially erasing any custom changes?',
+                                        type=int)
                 # TODO: this should be reworked with error handling.
                 # Click can certainly do this for us.
                 # This should be refactored as soon as we add a 3rd option.
@@ -429,8 +433,10 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose):
 # 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):
     """
-    The main click CLI module. Responsible for handling most common CLI options,
-    assigning any defaults and adding to the context for the sub-commands.
+    atomic-openshift-installer makes the process for installing OSE or AEP easier by interactively gathering the data needed to run on each host.
+    It can also be run in unattended mode if provided with a configuration file.
+
+    Further reading: https://docs.openshift.com/enterprise/latest/install_config/install/quick_install.html
     """
     ctx.obj = {}
     ctx.obj['unattended'] = unattended
diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py
index e4c808e85..372f27bda 100644
--- a/utils/src/ooinstall/openshift_ansible.py
+++ b/utils/src/ooinstall/openshift_ansible.py
@@ -82,7 +82,7 @@ def write_host(host, inventory, scheduleable=True):
     if installer_host in [host.connect_to, host.hostname, host.public_hostname]:
         facts += ' ansible_connection=local'
         if os.geteuid() != 0:
-            no_pwd_sudo = subprocess.call(['sudo', '-v', '-n'])
+            no_pwd_sudo = subprocess.call(['sudo', '-n', 'echo', 'openshift'])
             if no_pwd_sudo == 1:
                 print 'The atomic-openshift-installer requires sudo access without a password.'
                 sys.exit(1)
diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py
index d58539b18..fc16d9ceb 100644
--- a/utils/test/cli_installer_tests.py
+++ b/utils/test/cli_installer_tests.py
@@ -46,20 +46,20 @@ SAMPLE_CONFIG = """
 variant: %s
 ansible_ssh_user: root
 hosts:
-  - connect_to: master-private.example.com
+  - connect_to: 10.0.0.1
     ip: 10.0.0.1
     hostname: master-private.example.com
     public_ip: 24.222.0.1
     public_hostname: master.example.com
     master: true
     node: true
-  - connect_to: node1-private.example.com
+  - connect_to: 10.0.0.2
     ip: 10.0.0.2
     hostname: node1-private.example.com
     public_ip: 24.222.0.2
     public_hostname: node1.example.com
     node: true
-  - connect_to: node2-private.example.com
+  - connect_to: 10.0.0.3
     ip: 10.0.0.3
     hostname: node2-private.example.com
     public_ip: 24.222.0.3
@@ -98,6 +98,76 @@ class OOCliFixture(OOInstallFixture):
         f.close()
         return config
 
+    def _verify_load_facts(self, load_facts_mock):
+        """ Check that we ran load facts with expected inputs. """
+        load_facts_args = load_facts_mock.call_args[0]
+        self.assertEquals(os.path.join(self.work_dir, ".ansible/hosts"),
+            load_facts_args[0])
+        self.assertEquals(os.path.join(self.work_dir,
+            "playbooks/byo/openshift_facts.yml"), load_facts_args[1])
+        env_vars = load_facts_args[2]
+        self.assertEquals(os.path.join(self.work_dir,
+            '.ansible/callback_facts.yaml'),
+            env_vars['OO_INSTALL_CALLBACK_FACTS_YAML'])
+        self.assertEqual('/tmp/ansible.log', env_vars['ANSIBLE_LOG_PATH'])
+
+    def _verify_run_playbook(self, run_playbook_mock, exp_hosts_len, exp_hosts_to_run_on_len):
+        """ Check that we ran playbook with expected inputs. """
+        hosts = run_playbook_mock.call_args[0][0]
+        hosts_to_run_on = run_playbook_mock.call_args[0][1]
+        self.assertEquals(exp_hosts_len, len(hosts))
+        self.assertEquals(exp_hosts_to_run_on_len, len(hosts_to_run_on))
+
+    def _verify_config_hosts(self, written_config, host_count):
+        print written_config['hosts']
+        self.assertEquals(host_count, len(written_config['hosts']))
+        for h in written_config['hosts']:
+            self.assertTrue(h['node'])
+            self.assertTrue('ip' in h)
+            self.assertTrue('hostname' in h)
+            self.assertTrue('public_ip' in h)
+            self.assertTrue('public_hostname' in h)
+
+    #pylint: disable=too-many-arguments
+    def _verify_get_hosts_to_run_on(self, mock_facts, load_facts_mock,
+                                    run_playbook_mock, cli_input,
+                                    exp_hosts_len=None, exp_hosts_to_run_on_len=None,
+                                    force=None):
+        """
+        Tests cli_installer.py:get_hosts_to_run_on.  That method has quite a
+        few subtle branches in the logic.  The goal with this method is simply
+        to handle all the messy stuff here and allow the main test cases to be
+        easily read.  The basic idea is to modify mock_facts to return a
+        version indicating OpenShift is already installed on particular hosts.
+        """
+        load_facts_mock.return_value = (mock_facts, 0)
+        run_playbook_mock.return_value = 0
+
+        if cli_input:
+            self.cli_args.append("install")
+            result = self.runner.invoke(cli.cli,
+                                        self.cli_args,
+                                        input=cli_input)
+        else:
+            config_file = self.write_config(os.path.join(self.work_dir,
+                'ooinstall.conf'), SAMPLE_CONFIG % 'openshift-enterprise')
+
+            self.cli_args.extend(["-c", config_file, "install"])
+            if force:
+                self.cli_args.append("--force")
+            result = self.runner.invoke(cli.cli, self.cli_args)
+            written_config = self._read_yaml(config_file)
+            self._verify_config_hosts(written_config, exp_hosts_len)
+
+        self.assert_result(result, 0)
+        self._verify_load_facts(load_facts_mock)
+        self._verify_run_playbook(run_playbook_mock, exp_hosts_len, exp_hosts_to_run_on_len)
+
+        # Make sure we ran on the expected masters and nodes:
+        hosts = run_playbook_mock.call_args[0][0]
+        hosts_to_run_on = run_playbook_mock.call_args[0][1]
+        self.assertEquals(exp_hosts_len, len(hosts))
+        self.assertEquals(exp_hosts_to_run_on_len, len(hosts_to_run_on))
 
 class UnattendedCliTests(OOCliFixture):
 
@@ -105,6 +175,92 @@ class UnattendedCliTests(OOCliFixture):
         OOCliFixture.setUp(self)
         self.cli_args.append("-u")
 
+    # unattended with config file and all installed hosts (without --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on1(self, load_facts_mock, run_playbook_mock):
+        mock_facts = copy.deepcopy(MOCK_FACTS)
+        mock_facts['10.0.0.1']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.2']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.3']['common']['version'] = "3.0.0"
+
+        load_facts_mock.return_value = (mock_facts, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), SAMPLE_CONFIG % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+
+        if result.exception is None or result.exit_code != 1:
+            print "Exit code: %s" % result.exit_code
+            self.fail("Unexpected CLI return")
+
+    # unattended with config file and all installed hosts (with --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on2(self, load_facts_mock, run_playbook_mock):
+        mock_facts = copy.deepcopy(MOCK_FACTS)
+        mock_facts['10.0.0.1']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.2']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.3']['common']['version'] = "3.0.0"
+        self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock, run_playbook_mock,
+                                         cli_input=None,
+                                         exp_hosts_len=3,
+                                         exp_hosts_to_run_on_len=3,
+                                         force=True)
+
+    # unattended with config file and no installed hosts (without --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on3(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS, 0)
+        run_playbook_mock.return_value = 0
+        self._verify_get_hosts_to_run_on(MOCK_FACTS, load_facts_mock, run_playbook_mock,
+                                         cli_input=None,
+                                         exp_hosts_len=3,
+                                         exp_hosts_to_run_on_len=3,
+                                         force=False)
+
+    # unattended with config file and no installed hosts (with --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on4(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS, 0)
+        run_playbook_mock.return_value = 0
+        self._verify_get_hosts_to_run_on(MOCK_FACTS, load_facts_mock, run_playbook_mock,
+                                         cli_input=None,
+                                         exp_hosts_len=3,
+                                         exp_hosts_to_run_on_len=3,
+                                         force=True)
+
+    # unattended with config file and some installed some uninstalled hosts (without --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on5(self, load_facts_mock, run_playbook_mock):
+        mock_facts = copy.deepcopy(MOCK_FACTS)
+        mock_facts['10.0.0.1']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.2']['common']['version'] = "3.0.0"
+        self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock, run_playbook_mock,
+                                         cli_input=None,
+                                         exp_hosts_len=3,
+                                         exp_hosts_to_run_on_len=2,
+                                         force=False)
+
+    # unattended with config file and some installed some uninstalled hosts (with --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on6(self, load_facts_mock, run_playbook_mock):
+        mock_facts = copy.deepcopy(MOCK_FACTS)
+        mock_facts['10.0.0.1']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.2']['common']['version'] = "3.0.0"
+        self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock, run_playbook_mock,
+                                         cli_input=None,
+                                         exp_hosts_len=3,
+                                         exp_hosts_to_run_on_len=3,
+                                         force=True)
+
     @patch('ooinstall.openshift_ansible.run_main_playbook')
     @patch('ooinstall.openshift_ansible.load_system_facts')
     def test_cfg_full_run(self, load_facts_mock, run_playbook_mock):
@@ -363,35 +519,6 @@ class AttendedCliTests(OOCliFixture):
 
         return '\n'.join(inputs)
 
-    def _verify_load_facts(self, load_facts_mock):
-        """ Check that we ran load facts with expected inputs. """
-        load_facts_args = load_facts_mock.call_args[0]
-        self.assertEquals(os.path.join(self.work_dir, ".ansible/hosts"),
-            load_facts_args[0])
-        self.assertEquals(os.path.join(self.work_dir,
-            "playbooks/byo/openshift_facts.yml"), load_facts_args[1])
-        env_vars = load_facts_args[2]
-        self.assertEquals(os.path.join(self.work_dir,
-            '.ansible/callback_facts.yaml'),
-            env_vars['OO_INSTALL_CALLBACK_FACTS_YAML'])
-        self.assertEqual('/tmp/ansible.log', env_vars['ANSIBLE_LOG_PATH'])
-
-    def _verify_run_playbook(self, run_playbook_mock, exp_hosts_len, exp_hosts_to_run_on_len):
-        """ Check that we ran playbook with expected inputs. """
-        hosts = run_playbook_mock.call_args[0][0]
-        hosts_to_run_on = run_playbook_mock.call_args[0][1]
-        self.assertEquals(exp_hosts_len, len(hosts))
-        self.assertEquals(exp_hosts_to_run_on_len, len(hosts_to_run_on))
-
-    def _verify_config_hosts(self, written_config, host_count):
-        self.assertEquals(host_count, len(written_config['hosts']))
-        for h in written_config['hosts']:
-            self.assertTrue(h['node'])
-            self.assertTrue('ip' in h)
-            self.assertTrue('hostname' in h)
-            self.assertTrue('public_ip' in h)
-            self.assertTrue('public_hostname' in h)
-
     @patch('ooinstall.openshift_ansible.run_main_playbook')
     @patch('ooinstall.openshift_ansible.load_system_facts')
     def test_full_run(self, load_facts_mock, run_playbook_mock):
@@ -416,6 +543,7 @@ class AttendedCliTests(OOCliFixture):
         written_config = self._read_yaml(self.config_file)
         self._verify_config_hosts(written_config, 3)
 
+    # interactive with config file and some installed some uninstalled hosts
     @patch('ooinstall.openshift_ansible.run_main_playbook')
     @patch('ooinstall.openshift_ansible.load_system_facts')
     def test_add_nodes(self, load_facts_mock, run_playbook_mock):
@@ -472,6 +600,29 @@ class AttendedCliTests(OOCliFixture):
         written_config = self._read_yaml(config_file)
         self._verify_config_hosts(written_config, 3)
 
+    #interactive with config file and all installed hosts
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_get_hosts_to_run_on(self, load_facts_mock, run_playbook_mock):
+        mock_facts = copy.deepcopy(MOCK_FACTS)
+        mock_facts['10.0.0.1']['common']['version'] = "3.0.0"
+        mock_facts['10.0.0.2']['common']['version'] = "3.0.0"
+
+        cli_input = self._build_input(hosts=[
+            ('10.0.0.1', True),
+            ],
+                                      add_nodes=[('10.0.0.2', False)],
+                                      ssh_user='root',
+                                      variant_num=1,
+                                      confirm_facts='y')
+
+        self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock,
+                                         run_playbook_mock,
+                                         cli_input,
+                                         exp_hosts_len=2,
+                                         exp_hosts_to_run_on_len=2,
+                                         force=False)
+
 # TODO: test with config file, attended add node
 # TODO: test with config file, attended new node already in config file
 # TODO: test with config file, attended new node already in config file, plus manually added nodes