Merge pull request #1048 from twiest/prod

Sync master -> Prod

3 files changed, 711 insertions, 166 deletions

diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py
index fc16d9ceb..1da49c807 100644
--- a/utils/test/cli_installer_tests.py
+++ b/utils/test/cli_installer_tests.py
@@ -5,12 +5,10 @@
 import copy
 import os
 import ConfigParser
-import yaml
 
 import ooinstall.cli_installer as cli
 
-from click.testing import CliRunner
-from test.oo_config_tests import OOInstallFixture
+from test.fixture import OOCliFixture, SAMPLE_CONFIG, build_input, read_yaml
 from mock import patch
 
 
@@ -41,8 +39,67 @@ MOCK_FACTS = {
     },
 }
 
-# Substitute in a product name before use:
-SAMPLE_CONFIG = """
+MOCK_FACTS_QUICKHA = {
+    '10.0.0.1': {
+        'common': {
+            'ip': '10.0.0.1',
+            'public_ip': '10.0.0.1',
+            'hostname': 'master-private.example.com',
+            'public_hostname': 'master.example.com'
+        }
+    },
+    '10.0.0.2': {
+        'common': {
+            'ip': '10.0.0.2',
+            'public_ip': '10.0.0.2',
+            'hostname': 'node1-private.example.com',
+            'public_hostname': 'node1.example.com'
+        }
+    },
+    '10.0.0.3': {
+        'common': {
+            'ip': '10.0.0.3',
+            'public_ip': '10.0.0.3',
+            'hostname': 'node2-private.example.com',
+            'public_hostname': 'node2.example.com'
+        }
+    },
+    '10.0.0.4': {
+        'common': {
+            'ip': '10.0.0.4',
+            'public_ip': '10.0.0.4',
+            'hostname': 'proxy-private.example.com',
+            'public_hostname': 'proxy.example.com'
+        }
+    },
+}
+
+# Missing connect_to on some hosts:
+BAD_CONFIG = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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
+  - 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: 10.0.0.3
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+"""
+
+QUICKHA_CONFIG = """
 variant: %s
 ansible_ssh_user: root
 hosts:
@@ -58,6 +115,7 @@ hosts:
     hostname: node1-private.example.com
     public_ip: 24.222.0.2
     public_hostname: node1.example.com
+    master: true
     node: true
   - connect_to: 10.0.0.3
     ip: 10.0.0.3
@@ -65,109 +123,145 @@ hosts:
     public_ip: 24.222.0.3
     public_hostname: node2.example.com
     node: true
+    master: true
+  - connect_to: 10.0.0.4
+    ip: 10.0.0.4
+    hostname: node3-private.example.com
+    public_ip: 24.222.0.4
+    public_hostname: node3.example.com
+    node: true
+  - connect_to: 10.0.0.5
+    ip: 10.0.0.5
+    hostname: proxy-private.example.com
+    public_ip: 24.222.0.5
+    public_hostname: proxy.example.com
+    master_lb: true
 """
 
+QUICKHA_2_MASTER_CONFIG = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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: 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
+    master: true
+    node: true
+  - connect_to: 10.0.0.4
+    ip: 10.0.0.4
+    hostname: node3-private.example.com
+    public_ip: 24.222.0.4
+    public_hostname: node3.example.com
+    node: true
+  - connect_to: 10.0.0.5
+    ip: 10.0.0.5
+    hostname: proxy-private.example.com
+    public_ip: 24.222.0.5
+    public_hostname: proxy.example.com
+    master_lb: true
+"""
 
-class OOCliFixture(OOInstallFixture):
-
-    def setUp(self):
-        OOInstallFixture.setUp(self)
-        self.runner = CliRunner()
-
-        # Add any arguments you would like to test here, the defaults ensure
-        # we only do unattended invocations here, and using temporary files/dirs.
-        self.cli_args = ["-a", self.work_dir]
-
-    def run_cli(self):
-        return self.runner.invoke(cli.cli, self.cli_args)
-
-    def assert_result(self, result, exit_code):
-        if result.exception is not None or result.exit_code != exit_code:
-            print "Unexpected result from CLI execution"
-            print "Exit code: %s" % result.exit_code
-            print "Exception: %s" % result.exception
-            print result.exc_info
-            import traceback
-            traceback.print_exception(*result.exc_info)
-            print "Output:\n%s" % result.output
-            self.fail("Exception during CLI execution")
-
-    def _read_yaml(self, config_file_path):
-        f = open(config_file_path, 'r')
-        config = yaml.safe_load(f.read())
-        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)
+QUICKHA_CONFIG_REUSED_LB = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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: 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
+    master: true
+    node: true
+    master_lb: true
+  - connect_to: 10.0.0.3
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+    master: true
+"""
 
-        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)
+QUICKHA_CONFIG_NO_LB = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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: 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
+    master: true
+    node: true
+  - connect_to: 10.0.0.3
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+    master: true
+"""
 
-        # 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))
+QUICKHA_CONFIG_PRECONFIGURED_LB = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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: 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
+    master: true
+    node: true
+  - connect_to: 10.0.0.3
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+    master: true
+  - connect_to: 10.0.0.4
+    ip: 10.0.0.4
+    hostname: node3-private.example.com
+    public_ip: 24.222.0.4
+    public_hostname: node3.example.com
+    node: true
+  - connect_to: proxy-private.example.com
+    hostname: proxy-private.example.com
+    public_hostname: proxy.example.com
+    master_lb: true
+    preconfigured: true
+"""
 
 class UnattendedCliTests(OOCliFixture):
 
@@ -284,7 +378,9 @@ class UnattendedCliTests(OOCliFixture):
             '.ansible/callback_facts.yaml'),
             env_vars['OO_INSTALL_CALLBACK_FACTS_YAML'])
         self.assertEqual('/tmp/ansible.log', env_vars['ANSIBLE_LOG_PATH'])
-        self.assertTrue('ANSIBLE_CONFIG' not in env_vars)
+        # 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)
 
         # Make sure we ran on the expected masters and nodes:
         hosts = run_playbook_mock.call_args[0][0]
@@ -345,7 +441,7 @@ class UnattendedCliTests(OOCliFixture):
         result = self.runner.invoke(cli.cli, self.cli_args)
         self.assert_result(result, 0)
 
-        written_config = self._read_yaml(config_file)
+        written_config = read_yaml(config_file)
 
         self.assertEquals('openshift-enterprise', written_config['variant'])
         # We didn't specify a version so the latest should have been assumed,
@@ -374,7 +470,7 @@ class UnattendedCliTests(OOCliFixture):
         result = self.runner.invoke(cli.cli, self.cli_args)
         self.assert_result(result, 0)
 
-        written_config = self._read_yaml(config_file)
+        written_config = read_yaml(config_file)
 
         self.assertEquals('openshift-enterprise', written_config['variant'])
         # Make sure our older version was preserved:
@@ -450,15 +546,125 @@ class UnattendedCliTests(OOCliFixture):
         if expected_result:
             self.assertEquals(expected_result, facts_env_vars['ANSIBLE_CONFIG'])
         else:
-            self.assertFalse('ANSIBLE_CONFIG' in facts_env_vars)
+            # 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:
-            self.assertFalse('ANSIBLE_CONFIG' in env_vars)
+            # 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)
 
+    # unattended with bad config file and no installed hosts (without --force)
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_bad_config(self, load_facts_mock, run_playbook_mock):
+        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'), BAD_CONFIG % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+
+        self.assertEquals(1, result.exit_code)
+        self.assertTrue("You must specify either an ip or hostname"
+            in result.output)
+
+    #unattended with three masters, one node, and haproxy
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_quick_ha_full_run(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), QUICKHA_CONFIG % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+        self.assert_result(result, 0)
+
+        # 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(5, len(hosts))
+        self.assertEquals(5, len(hosts_to_run_on))
+
+    #unattended with two masters, one node, and haproxy
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_quick_ha_only_2_masters(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), QUICKHA_2_MASTER_CONFIG % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+
+        # This is an invalid config:
+        self.assert_result(result, 1)
+        self.assertTrue("A minimum of 3 Masters are required" in result.output)
+
+    #unattended with three masters, one node, but no load balancer specified:
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_quick_ha_no_lb(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), QUICKHA_CONFIG_NO_LB % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+
+        # This is not a valid input:
+        self.assert_result(result, 1)
+        self.assertTrue('No master load balancer specified in config' in result.output)
+
+    #unattended with three masters, one node, and one of the masters reused as load balancer:
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_quick_ha_reused_lb(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), QUICKHA_CONFIG_REUSED_LB % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+
+        # This is not a valid configuration:
+        self.assert_result(result, 1)
+
+    #unattended with preconfigured lb
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_quick_ha_preconfigured_lb(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        config_file = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), QUICKHA_CONFIG_PRECONFIGURED_LB % 'openshift-enterprise')
+
+        self.cli_args.extend(["-c", config_file, "install"])
+        result = self.runner.invoke(cli.cli, self.cli_args)
+        self.assert_result(result, 0)
+
+        # 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(5, len(hosts))
+        self.assertEquals(5, len(hosts_to_run_on))
 
 class AttendedCliTests(OOCliFixture):
 
@@ -468,64 +674,13 @@ class AttendedCliTests(OOCliFixture):
         self.config_file = os.path.join(self.work_dir, 'config.yml')
         self.cli_args.extend(["-c", self.config_file])
 
-    #pylint: disable=too-many-arguments
-    def _build_input(self, ssh_user=None, hosts=None, variant_num=None,
-        add_nodes=None, confirm_facts=None):
-        """
-        Builds a CLI input string with newline characters to simulate
-        the full run.
-        This gives us only one place to update when the input prompts change.
-        """
-
-        inputs = [
-            'y',  # let's proceed
-        ]
-        if ssh_user:
-            inputs.append(ssh_user)
-
-        if hosts:
-            i = 0
-            for (host, is_master) in hosts:
-                inputs.append(host)
-                inputs.append('y' if is_master else 'n')
-                #inputs.append('rpm')
-                if i < len(hosts) - 1:
-                    inputs.append('y')  # Add more hosts
-                else:
-                    inputs.append('n')  # Done adding hosts
-                i += 1
-
-        if variant_num:
-            inputs.append(str(variant_num))  # Choose variant + version
-
-        # TODO: support option 2, fresh install
-        if add_nodes:
-            inputs.append('1')  # Add more nodes
-            i = 0
-            for (host, is_master) in add_nodes:
-                inputs.append(host)
-                inputs.append('y' if is_master else 'n')
-                #inputs.append('rpm')
-                if i < len(add_nodes) - 1:
-                    inputs.append('y')  # Add more hosts
-                else:
-                    inputs.append('n')  # Done adding hosts
-                i += 1
-
-        inputs.extend([
-            confirm_facts,
-            'y',  # lets do this
-        ])
-
-        return '\n'.join(inputs)
-
     @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):
         load_facts_mock.return_value = (MOCK_FACTS, 0)
         run_playbook_mock.return_value = 0
 
-        cli_input = self._build_input(hosts=[
+        cli_input = build_input(hosts=[
             ('10.0.0.1', True),
             ('10.0.0.2', False),
             ('10.0.0.3', False)],
@@ -540,9 +695,18 @@ class AttendedCliTests(OOCliFixture):
         self._verify_load_facts(load_facts_mock)
         self._verify_run_playbook(run_playbook_mock, 3, 3)
 
-        written_config = self._read_yaml(self.config_file)
+        written_config = read_yaml(self.config_file)
         self._verify_config_hosts(written_config, 3)
 
+        inventory = ConfigParser.ConfigParser(allow_no_value=True)
+        inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
+        self.assertEquals('False',
+            inventory.get('nodes', '10.0.0.1  openshift_schedulable'))
+        self.assertEquals(None,
+            inventory.get('nodes', '10.0.0.2'))
+        self.assertEquals(None,
+            inventory.get('nodes', '10.0.0.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')
@@ -557,7 +721,7 @@ class AttendedCliTests(OOCliFixture):
         load_facts_mock.return_value = (mock_facts, 0)
         run_playbook_mock.return_value = 0
 
-        cli_input = self._build_input(hosts=[
+        cli_input = build_input(hosts=[
             ('10.0.0.1', True),
             ('10.0.0.2', False),
             ],
@@ -574,7 +738,7 @@ class AttendedCliTests(OOCliFixture):
         self._verify_load_facts(load_facts_mock)
         self._verify_run_playbook(run_playbook_mock, 3, 2)
 
-        written_config = self._read_yaml(self.config_file)
+        written_config = read_yaml(self.config_file)
         self._verify_config_hosts(written_config, 3)
 
     @patch('ooinstall.openshift_ansible.run_main_playbook')
@@ -586,7 +750,7 @@ class AttendedCliTests(OOCliFixture):
         config_file = self.write_config(os.path.join(self.work_dir,
                                                      'ooinstall.conf'),
                                         SAMPLE_CONFIG % 'openshift-enterprise')
-        cli_input = self._build_input(confirm_facts='y')
+        cli_input = build_input(confirm_facts='y')
         self.cli_args.extend(["-c", config_file])
         self.cli_args.append("install")
         result = self.runner.invoke(cli.cli,
@@ -597,7 +761,7 @@ class AttendedCliTests(OOCliFixture):
         self._verify_load_facts(load_facts_mock)
         self._verify_run_playbook(run_playbook_mock, 3, 3)
 
-        written_config = self._read_yaml(config_file)
+        written_config = read_yaml(config_file)
         self._verify_config_hosts(written_config, 3)
 
     #interactive with config file and all installed hosts
@@ -608,12 +772,13 @@ class AttendedCliTests(OOCliFixture):
         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=[
+        cli_input = build_input(hosts=[
             ('10.0.0.1', True),
             ],
                                       add_nodes=[('10.0.0.2', False)],
                                       ssh_user='root',
                                       variant_num=1,
+                                      schedulable_masters_ok=True,
                                       confirm_facts='y')
 
         self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock,
@@ -623,6 +788,131 @@ class AttendedCliTests(OOCliFixture):
                                          exp_hosts_to_run_on_len=2,
                                          force=False)
 
+    #interactive multimaster: one more node than master
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_ha_dedicated_node(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        cli_input = build_input(hosts=[
+            ('10.0.0.1', True),
+            ('10.0.0.2', True),
+            ('10.0.0.3', True),
+            ('10.0.0.4', False)],
+                                      ssh_user='root',
+                                      variant_num=1,
+                                      confirm_facts='y',
+                                      master_lb=('10.0.0.5', False))
+        self.cli_args.append("install")
+        result = self.runner.invoke(cli.cli, self.cli_args,
+            input=cli_input)
+        self.assert_result(result, 0)
+
+        self._verify_load_facts(load_facts_mock)
+        self._verify_run_playbook(run_playbook_mock, 5, 5)
+
+        written_config = read_yaml(self.config_file)
+        self._verify_config_hosts(written_config, 5)
+
+        inventory = ConfigParser.ConfigParser(allow_no_value=True)
+        inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
+        self.assertEquals('False',
+            inventory.get('nodes', '10.0.0.1  openshift_schedulable'))
+        self.assertEquals('False',
+            inventory.get('nodes', '10.0.0.2  openshift_schedulable'))
+        self.assertEquals('False',
+            inventory.get('nodes', '10.0.0.3  openshift_schedulable'))
+        self.assertEquals(None,
+            inventory.get('nodes', '10.0.0.4'))
+
+        self.assertTrue(inventory.has_section('etcd'))
+        self.assertEquals(3, len(inventory.items('etcd')))
+
+    #interactive multimaster: identical masters and nodes
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_ha_no_dedicated_nodes(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        cli_input = build_input(hosts=[
+            ('10.0.0.1', True),
+            ('10.0.0.2', True),
+            ('10.0.0.3', True)],
+                                      ssh_user='root',
+                                      variant_num=1,
+                                      confirm_facts='y',
+                                      master_lb=('10.0.0.5', False))
+        self.cli_args.append("install")
+        result = self.runner.invoke(cli.cli, self.cli_args,
+            input=cli_input)
+        self.assert_result(result, 0)
+
+        self._verify_load_facts(load_facts_mock)
+        self._verify_run_playbook(run_playbook_mock, 4, 4)
+
+        written_config = read_yaml(self.config_file)
+        self._verify_config_hosts(written_config, 4)
+
+        inventory = ConfigParser.ConfigParser(allow_no_value=True)
+        inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
+        self.assertEquals('True',
+            inventory.get('nodes', '10.0.0.1  openshift_schedulable'))
+        self.assertEquals('True',
+            inventory.get('nodes', '10.0.0.2  openshift_schedulable'))
+        self.assertEquals('True',
+            inventory.get('nodes', '10.0.0.3  openshift_schedulable'))
+
+    #interactive multimaster: attempting to use a master as the load balancer should fail:
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_ha_reuse_master_as_lb(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0)
+        run_playbook_mock.return_value = 0
+
+        cli_input = build_input(hosts=[
+                                      ('10.0.0.1', True),
+                                      ('10.0.0.2', True),
+                                      ('10.0.0.3', False),
+                                      ('10.0.0.4', True)],
+                                      ssh_user='root',
+                                      variant_num=1,
+                                      confirm_facts='y',
+                                      master_lb=(['10.0.0.2', '10.0.0.5'], False))
+        self.cli_args.append("install")
+        result = self.runner.invoke(cli.cli, self.cli_args,
+            input=cli_input)
+        self.assert_result(result, 0)
+
+    #interactive all-in-one
+    @patch('ooinstall.openshift_ansible.run_main_playbook')
+    @patch('ooinstall.openshift_ansible.load_system_facts')
+    def test_all_in_one(self, load_facts_mock, run_playbook_mock):
+        load_facts_mock.return_value = (MOCK_FACTS, 0)
+        run_playbook_mock.return_value = 0
+
+        cli_input = build_input(hosts=[
+            ('10.0.0.1', True)],
+                                      ssh_user='root',
+                                      variant_num=1,
+                                      confirm_facts='y')
+        self.cli_args.append("install")
+        result = self.runner.invoke(cli.cli, self.cli_args,
+            input=cli_input)
+        self.assert_result(result, 0)
+
+        self._verify_load_facts(load_facts_mock)
+        self._verify_run_playbook(run_playbook_mock, 1, 1)
+
+        written_config = read_yaml(self.config_file)
+        self._verify_config_hosts(written_config, 1)
+
+        inventory = ConfigParser.ConfigParser(allow_no_value=True)
+        inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
+        self.assertEquals('True',
+            inventory.get('nodes', '10.0.0.1  openshift_schedulable'))
+
 # 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
diff --git a/utils/test/fixture.py b/utils/test/fixture.py
new file mode 100644
index 000000000..90bd9e1ef
--- /dev/null
+++ b/utils/test/fixture.py
@@ -0,0 +1,221 @@
+# pylint: disable=missing-docstring
+import os
+import yaml
+
+import ooinstall.cli_installer as cli
+
+from test.oo_config_tests import OOInstallFixture
+from click.testing import CliRunner
+
+# Substitute in a product name before use:
+SAMPLE_CONFIG = """
+variant: %s
+ansible_ssh_user: root
+hosts:
+  - 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: 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: 10.0.0.3
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+"""
+
+def read_yaml(config_file_path):
+    cfg_f = open(config_file_path, 'r')
+    config = yaml.safe_load(cfg_f.read())
+    cfg_f.close()
+    return config
+
+
+class OOCliFixture(OOInstallFixture):
+
+    def setUp(self):
+        OOInstallFixture.setUp(self)
+        self.runner = CliRunner()
+
+        # Add any arguments you would like to test here, the defaults ensure
+        # we only do unattended invocations here, and using temporary files/dirs.
+        self.cli_args = ["-a", self.work_dir]
+
+    def run_cli(self):
+        return self.runner.invoke(cli.cli, self.cli_args)
+
+    def assert_result(self, result, exit_code):
+        if result.exit_code != exit_code:
+            print "Unexpected result from CLI execution"
+            print "Exit code: %s" % result.exit_code
+            print "Exception: %s" % result.exception
+            print result.exc_info
+            import traceback
+            traceback.print_exception(*result.exc_info)
+            print "Output:\n%s" % result.output
+            self.fail("Exception during CLI execution")
+
+    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 host in written_config['hosts']:
+            self.assertTrue('hostname' in host)
+            self.assertTrue('public_hostname' in host)
+            if 'preconfigured' not in host:
+                self.assertTrue(host['node'])
+                self.assertTrue('ip' in host)
+                self.assertTrue('public_ip' in host)
+
+    #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 = 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))
+
+
+#pylint: disable=too-many-arguments,too-many-branches
+def build_input(ssh_user=None, hosts=None, variant_num=None,
+                add_nodes=None, confirm_facts=None, schedulable_masters_ok=None,
+                master_lb=None):
+    """
+    Build an input string simulating a user entering values in an interactive
+    attended install.
+
+    This is intended to give us one place to update when the CLI prompts change.
+    We should aim to keep this dependent on optional keyword arguments with
+    sensible defaults to keep things from getting too fragile.
+    """
+
+    inputs = [
+        'y',  # let's proceed
+    ]
+    if ssh_user:
+        inputs.append(ssh_user)
+
+    if variant_num:
+        inputs.append(str(variant_num))  # Choose variant + version
+
+    num_masters = 0
+    if hosts:
+        i = 0
+        for (host, is_master) in hosts:
+            inputs.append(host)
+            if is_master:
+                inputs.append('y')
+                num_masters += 1
+            else:
+                inputs.append('n')
+            #inputs.append('rpm')
+            # We should not be prompted to add more hosts if we're currently at
+            # 2 masters, this is an invalid HA configuration, so this question
+            # will not be asked, and the user must enter the next host:
+            if num_masters != 2:
+                if i < len(hosts) - 1:
+                    if num_masters >= 1:
+                        inputs.append('y')  # Add more hosts
+                else:
+                    inputs.append('n')  # Done adding hosts
+            i += 1
+
+    # You can pass a single master_lb or a list if you intend for one to get rejected:
+    if master_lb:
+        if isinstance(master_lb[0], list) or isinstance(master_lb[0], tuple):
+            inputs.extend(master_lb[0])
+        else:
+            inputs.append(master_lb[0])
+        inputs.append('y' if master_lb[1] else 'n')
+
+    # TODO: support option 2, fresh install
+    if add_nodes:
+        if schedulable_masters_ok:
+            inputs.append('y')
+        inputs.append('1')  # Add more nodes
+        i = 0
+        for (host, is_master) in add_nodes:
+            inputs.append(host)
+            #inputs.append('rpm')
+            if i < len(add_nodes) - 1:
+                inputs.append('y')  # Add more hosts
+            else:
+                inputs.append('n')  # Done adding hosts
+            i += 1
+
+    if add_nodes is None:
+        total_hosts = hosts
+    else:
+        total_hosts = hosts + add_nodes
+    if total_hosts is not None and num_masters == len(total_hosts):
+        inputs.append('y')
+
+    inputs.extend([
+        confirm_facts,
+        'y',  # lets do this
+    ])
+
+    return '\n'.join(inputs)
+
diff --git a/utils/test/oo_config_tests.py b/utils/test/oo_config_tests.py
index 0dd4a30e9..9f5f8e244 100644
--- a/utils/test/oo_config_tests.py
+++ b/utils/test/oo_config_tests.py
@@ -73,6 +73,29 @@ hosts:
     node: true
 """
 
+CONFIG_BAD = """
+variant: openshift-enterprise
+ansible_ssh_user: root
+hosts:
+  - connect_to: master-private.example.com
+    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
+  - 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
+    ip: 10.0.0.3
+    hostname: node2-private.example.com
+    public_ip: 24.222.0.3
+    public_hostname: node2.example.com
+    node: true
+"""
 
 class OOInstallFixture(unittest.TestCase):
 
@@ -161,6 +184,17 @@ class OOConfigTests(OOInstallFixture):
         self.assertEquals('openshift-enterprise', ooconfig.settings['variant'])
         self.assertEquals('v1', ooconfig.settings['version'])
 
+    def test_load_bad_config(self):
+
+        cfg_path = self.write_config(os.path.join(self.work_dir,
+            'ooinstall.conf'), CONFIG_BAD)
+        try:
+            OOConfig(cfg_path)
+            assert False
+        except OOConfigInvalidHostError:
+            assert True
+
+
     def test_load_complete_facts(self):
         cfg_path = self.write_config(os.path.join(self.work_dir,
             'ooinstall.conf'), SAMPLE_CONFIG)