diff options
| -rw-r--r-- | utils/src/ooinstall/cli_installer.py | 41 | ||||
| -rw-r--r-- | utils/test/cli_installer_tests.py | 80 | ||||
| -rw-r--r-- | utils/test/fixture.py | 17 |
3 files changed, 112 insertions, 26 deletions
diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 28b72a7dc..9a447406f 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -14,7 +14,6 @@ from ooinstall.variants import find_variant, get_variant_version_combos DEFAULT_ANSIBLE_CONFIG = '/usr/share/atomic-openshift-utils/ansible.cfg' DEFAULT_PLAYBOOK_DIR = '/usr/share/ansible/openshift-ansible/' -MIN_MASTERS_FOR_HA = 3 def validate_ansible_dir(path): if not path: @@ -126,7 +125,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen host_props['master'] = True num_masters += 1 - if num_masters >= MIN_MASTERS_FOR_HA or version == '3.0': + if version == '3.0': masters_set = True host_props['node'] = True @@ -147,10 +146,13 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen if print_summary: print_host_summary(hosts) - if num_masters <= 1 or num_masters >= MIN_MASTERS_FOR_HA: + # If we have one master, this is enough for an all-in-one deployment, + # thus we can start asking if you wish to proceed. Otherwise we assume + # you must. + if masters_set or num_masters != 2: more_hosts = click.confirm('Do you want to add additional hosts?') - if num_masters > 1: + if num_masters >= 3: collect_master_lb(hosts) return hosts @@ -166,8 +168,26 @@ def print_host_summary(hosts): click.echo('OpenShift Nodes: %s' % len(nodes)) for host in nodes: click.echo(' %s' % host.connect_to) - click.echo('Additional Masters required for HA: %s' % - max(MIN_MASTERS_FOR_HA - len(masters), 0)) + + click.echo("") + + if len(masters) == 1: + click.echo("NOTE: Add a total of 3 or more Masters to perform an HA" + " installation.") + elif len(masters) == 2: + min_masters_message = """ +WARNING: A minimum of 3 masters are required to perform an HA installation. +Please add one more to proceed. +""" + click.echo(min_masters_message) + elif len(masters) >= 3: + ha_message = """ +NOTE: Multiple Masters specified, this will be an HA deployment with a separate +etcd cluster. You will be prompted to provide the FQDN of a load balancer once +finished entering hosts. +""" + click.echo(ha_message) + click.echo('') @@ -290,15 +310,20 @@ Edit %s with the desired values and run `atomic-openshift-installer --unattended def check_hosts_config(oo_cfg, unattended): click.clear() masters = [host for host in oo_cfg.hosts if host.master] + + if len(masters) == 2: + click.echo("A minimum of 3 Masters are required for HA deployments.") + sys.exit(1) + if len(masters) > 1: master_lb = [host for host in oo_cfg.hosts if host.master_lb] if len(master_lb) > 1: click.echo('More than one Master load balancer specified. Only one is allowed.') - sys.exit(0) + sys.exit(1) elif len(master_lb) == 1: if master_lb[0].master or master_lb[0].node: click.echo('The Master load balancer is configured as a master or node. Please correct this.') - sys.exit(0) + sys.exit(1) # Check for another host with same connect_to? else: message = """ diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index a0fc95c33..c12b4d564 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -123,10 +123,49 @@ 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 """ @@ -156,6 +195,7 @@ hosts: public_ip: 24.222.0.3 public_hostname: node2.example.com node: true + master: true """ QUICKHA_CONFIG_NO_LB = """ @@ -182,6 +222,7 @@ hosts: public_ip: 24.222.0.3 public_hostname: node2.example.com node: true + master: true """ class UnattendedCliTests(OOCliFixture): @@ -497,7 +538,7 @@ class UnattendedCliTests(OOCliFixture): self.assertTrue("You must specify either an ip or hostname" in result.output) - #unattended with two masters, one node, and haproxy + #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): @@ -514,10 +555,27 @@ class UnattendedCliTests(OOCliFixture): # 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(4, len(hosts)) - self.assertEquals(4, len(hosts_to_run_on)) + 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) - #unattended with two masters, one node, but no load balancer specified: + # 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): @@ -541,7 +599,7 @@ class UnattendedCliTests(OOCliFixture): self.assertEquals(3, len(hosts)) self.assertEquals(3, len(hosts_to_run_on)) - #unattended with two masters, one node, and one of the masters reused as load balancer: + #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): @@ -555,7 +613,7 @@ class UnattendedCliTests(OOCliFixture): result = self.runner.invoke(cli.cli, self.cli_args) # This is not a valid configuration: - self.assert_result(result, 0) + self.assert_result(result, 1) class AttendedCliTests(OOCliFixture): @@ -690,8 +748,8 @@ class AttendedCliTests(OOCliFixture): 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)], + ('10.0.0.3', True), + ('10.0.0.4', False)], ssh_user='root', variant_num=1, confirm_facts='y', @@ -713,10 +771,10 @@ class AttendedCliTests(OOCliFixture): inventory.get('nodes', '10.0.0.1 openshift_schedulable')) self.assertEquals('False', inventory.get('nodes', '10.0.0.2 openshift_schedulable')) - self.assertEquals(None, - inventory.get('nodes', '10.0.0.3')) self.assertEquals('False', - inventory.get('nodes', '10.0.0.4 openshift_schedulable')) + 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'))) diff --git a/utils/test/fixture.py b/utils/test/fixture.py index ab1c3f12e..14baec6b3 100644 --- a/utils/test/fixture.py +++ b/utils/test/fixture.py @@ -54,7 +54,7 @@ class OOCliFixture(OOInstallFixture): 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: + if result.exit_code != exit_code: print "Unexpected result from CLI execution" print "Exit code: %s" % result.exit_code print "Exception: %s" % result.exception @@ -163,7 +163,6 @@ def build_input(ssh_user=None, hosts=None, variant_num=None, num_masters = 0 if hosts: i = 0 - min_masters_for_ha = 3 for (host, is_master) in hosts: inputs.append(host) if is_master: @@ -172,11 +171,15 @@ def build_input(ssh_user=None, hosts=None, variant_num=None, else: inputs.append('n') #inputs.append('rpm') - if i < len(hosts) - 1: - if num_masters <= 1 or num_masters >= min_masters_for_ha: - inputs.append('y') # Add more hosts - else: - inputs.append('n') # Done adding hosts + # 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: |