From 037950923abd5188977e825d994f14111b0e6a89 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 16 Nov 2015 12:03:47 -0400 Subject: Fix tests on systems with openshift-ansible rpms installed. --- utils/src/ooinstall/cli_installer.py | 3 ++- utils/test/cli_installer_tests.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 3c3f45c3b..3b5dbaf0e 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -458,7 +458,8 @@ def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_conf if ctx.obj['ansible_config']: oo_cfg.settings['ansible_config'] = ctx.obj['ansible_config'] - elif os.path.exists(DEFAULT_ANSIBLE_CONFIG): + elif 'ansible_config' not in oo_cfg.settings and \ + 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 diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index fc16d9ceb..baadad358 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -284,7 +284,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] @@ -450,14 +452,18 @@ 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) class AttendedCliTests(OOCliFixture): -- cgit v1.2.3 From 77e9ebdc9c576015c886ed31ae26b693a9eace91 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 16 Nov 2015 12:19:01 -0400 Subject: Default to installing OSE 3.1 instead of 3.0. --- utils/src/ooinstall/cli_installer.py | 1 + utils/src/ooinstall/variants.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 3b5dbaf0e..daffe4aea 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -205,6 +205,7 @@ def get_variant_and_version(): message = "%s\n(%s) %s %s" % (message, i, variant.description, version.name) i = i + 1 + message = "%s\n" % message click.echo(message) response = click.prompt("Choose a variant from above: ", default=1) diff --git a/utils/src/ooinstall/variants.py b/utils/src/ooinstall/variants.py index 3bb61dddb..571025543 100644 --- a/utils/src/ooinstall/variants.py +++ b/utils/src/ooinstall/variants.py @@ -30,14 +30,14 @@ class Variant(object): self.versions = versions def latest_version(self): - return self.versions[-1] + return self.versions[0] # WARNING: Keep the versions ordered, most recent last: OSE = Variant('openshift-enterprise', 'OpenShift Enterprise', [ - Version('3.0', 'enterprise'), - Version('3.1', 'openshift-enterprise') + Version('3.1', 'openshift-enterprise'), + Version('3.0', 'enterprise') ] ) -- cgit v1.2.3 From c6b7ba8b0017af0875bd912ad0e83fb3d5879591 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 16 Nov 2015 12:53:03 -0400 Subject: Pylint fix for long line in cli docstring. --- utils/src/ooinstall/cli_installer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index daffe4aea..a19c19c8f 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -432,7 +432,8 @@ 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): """ - atomic-openshift-installer makes the process for installing OSE or AEP easier by interactively gathering the data needed to run on each host. + 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 -- cgit v1.2.3 From 11b3651e62b1b6aa458da574fe3948b86359ca80 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Tue, 17 Nov 2015 13:36:43 -0500 Subject: atomic-openshift-installer: Correct single master case Correct the case where the first host entered is not a master. --- utils/src/ooinstall/cli_installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index d9d3dd80c..f34255234 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -111,7 +111,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen if not master_set: is_master = click.confirm('Will this host be an OpenShift Master?') host_props['master'] = is_master - master_set = True + master_set = is_master host_props['node'] = True #TODO: Reenable this option once container installs are out of tech preview -- cgit v1.2.3 From 32db6b68266ab0c44260be878c6eaf64a9d495dc Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Tue, 17 Nov 2015 09:17:49 -0500 Subject: atomic-openshift-installer: pylint fixes A few fixes to keep pylint happy. --- utils/src/ooinstall/cli_installer.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index a1632ed0c..94f7afdc9 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -337,7 +337,8 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose): # This check has to happen before we start removing hosts later in this method if not force: if not unattended: - click.echo('By default the installer only adds new nodes to an installed environment.') + 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) reinstall the existing hosts ' \ 'potentially erasing any custom changes?', @@ -374,8 +375,8 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose): else: if unattended: if not force: - click.echo('Installed environment detected and no additional nodes specified: ' \ - 'aborting. If you want a fresh install, use ' \ + click.echo('Installed environment detected and no additional ' \ + 'nodes specified: aborting. If you want a fresh install, use ' \ '`atomic-openshift-installer install --force`') sys.exit(1) else: @@ -389,8 +390,8 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose): click.echo('Gathering information from hosts...') callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, verbose) if error: - click.echo("There was a problem fetching the required information. " \ - "See {} for details.".format(oo_cfg.settings['ansible_log_path'])) + click.echo("There was a problem fetching the required information. See " \ + "{} for details.".format(oo_cfg.settings['ansible_log_path'])) sys.exit(1) else: pass # proceeding as normal should do a clean install @@ -431,6 +432,7 @@ def get_hosts_to_run_on(oo_cfg, callback_facts, unattended, force, verbose): @click.option('-v', '--verbose', is_flag=True, default=False) #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): """ -- cgit v1.2.3 From c32d7eb88d30cbb131a9b8562d51e7a5930b79f4 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 13 Nov 2015 10:23:10 -0500 Subject: atomic-openshift-installer: connect_to error handling Catch the exception that happens when connect_to isn't specified in installer.cfg.yaml --- utils/src/ooinstall/cli_installer.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index a1632ed0c..d38c0fd09 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -8,6 +8,7 @@ import re import sys from ooinstall import openshift_ansible from ooinstall import OOConfig +from ooinstall.oo_config import OOConfigInvalidHostError from ooinstall.oo_config import Host from ooinstall.variants import find_variant, get_variant_version_combos @@ -447,7 +448,11 @@ def cli(ctx, unattended, configuration, ansible_playbook_directory, ansible_conf ctx.obj['ansible_log_path'] = ansible_log_path ctx.obj['verbose'] = verbose - oo_cfg = OOConfig(ctx.obj['configuration']) + try: + oo_cfg = OOConfig(ctx.obj['configuration']) + except OOConfigInvalidHostError as e: + click.echo(e) + sys.exit(1) # If no playbook dir on the CLI, check the config: if not ansible_playbook_directory: -- cgit v1.2.3 From 7a5cc821c30abc6f1fce374e2a360bb9406f71f1 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 14:22:10 -0500 Subject: Add some tests for a bad config --- utils/test/cli_installer_tests.py | 38 ++++++++++++++++++++++++++++++++++++++ utils/test/oo_config_tests.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index baadad358..e12b4a264 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -67,6 +67,29 @@ hosts: node: true """ +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 +""" class OOCliFixture(OOInstallFixture): @@ -465,6 +488,21 @@ class UnattendedCliTests(OOCliFixture): 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"]) + self.runner.invoke(cli.cli, self.cli_args) + + # proving here that we didn't generate an exception from the bad config + assert True class AttendedCliTests(OOCliFixture): 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) -- cgit v1.2.3 From a72243eda9ad0fb066c405a8171d41bfd8a16ecf Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 14:34:42 -0500 Subject: Check the end result on bad config file --- utils/test/cli_installer_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index e12b4a264..40a2f844d 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -499,10 +499,10 @@ class UnattendedCliTests(OOCliFixture): 'ooinstall.conf'), BAD_CONFIG % 'openshift-enterprise') self.cli_args.extend(["-c", config_file, "install"]) - self.runner.invoke(cli.cli, self.cli_args) + result = self.runner.invoke(cli.cli, self.cli_args) - # proving here that we didn't generate an exception from the bad config - assert True + assert result.exit_code == 1 + assert result.output == "You must specify either and 'ip' or 'hostname' to connect to.\n" class AttendedCliTests(OOCliFixture): -- cgit v1.2.3 From 0ee19018001ed6a0a578aa5b7b75e096d6c014d6 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Wed, 18 Nov 2015 10:59:04 -0500 Subject: atomic-openshift-installer: HA for quick installer This adds the ability to quickly set up a multi-master environment. --- utils/src/ooinstall/cli_installer.py | 82 ++++++++++++++++++++++++++++---- utils/src/ooinstall/oo_config.py | 9 +++- utils/src/ooinstall/openshift_ansible.py | 36 ++++++++++++-- utils/test/cli_installer_tests.py | 1 + 4 files changed, 111 insertions(+), 17 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 0b3af8829..1b4a67259 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -72,7 +72,7 @@ def delete_hosts(hosts): click.echo("\"{}\" doesn't coorespond to any valid input.".format(del_idx)) return hosts, None -def collect_hosts(master_set=False): +def collect_hosts(masters_set=False): """ Collect host information from user. This will later be filled in using ansible. @@ -102,17 +102,20 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts = [] more_hosts = True + num_masters = 0 while more_hosts: host_props = {} - hostname_or_ip = click.prompt('Enter hostname or IP address:', - default='', - value_proc=validate_prompt_hostname) - - host_props['connect_to'] = hostname_or_ip - if not master_set: - is_master = click.confirm('Will this host be an OpenShift Master?') - host_props['master'] = is_master - master_set = is_master + host_props['connect_to'] = click.prompt('Enter hostname or IP address:', + default='', + value_proc=validate_prompt_hostname) + + if not masters_set: + if click.confirm('Will this host be an OpenShift Master?'): + host_props['master'] = True + num_masters += 1 + if num_masters >= 3: + masters_set = True + hosts.append(collect_ha_proxy()) host_props['node'] = True #TODO: Reenable this option once container installs are out of tech preview @@ -132,6 +135,29 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen more_hosts = click.confirm('Do you want to add additional hosts?') return hosts +def collect_ha_proxy(): + """ + Get an HA proxy from the user + """ + message = """ +Setting up High Availability Masters requires a load balancing solution. +Please provide a host that will be configured as a proxy. This can either be +an existing load balancer configured to balance all masters on port 8443 or a +new host that will have HAProxy installed on it. +""" + click.echo(message) + host_props = {} + host_props['connect_to'] = click.prompt('Enter hostname or IP address:', + default='', + value_proc=validate_prompt_hostname) + host_props['run_on'] = click.confirm('Is this a clean host you want to install HAProxy on?') + host_props['master'] = False + host_props['node'] = False + host_props['ha_proxy'] = True + ha_proxy = Host(**host_props) + + return ha_proxy + def confirm_hosts_facts(oo_cfg, callback_facts): hosts = oo_cfg.hosts click.clear() @@ -199,6 +225,40 @@ Edit %s with the desired values and run `atomic-openshift-installer --unattended sys.exit(0) return default_facts + + +def check_hosts_config(oo_cfg): + click.clear() + masters = [host for host in oo_cfg.hosts if host.master] + if len(masters) > 1: + ha_proxy = [host for host in oo_cfg.hosts if host.ha_proxy] + click.echo(ha_proxy) + if len(ha_proxy) > 1: + click.echo('More than one HAProxy specified. Only one proxy is allowed.') + sys.exit(0) + elif len(ha_proxy) == 1: + if ha_proxy[0].master or ha_proxy[0].node: + click.echo('HAProxy is configured as a master or node. Please correct this.') + sys.exit(0) + else: + message = """ +No HAProxy given in config. Either specify one or provide a load balancing solution +of your choice to balance the master API (port 8443) on all master hosts. + +https://docs.openshift.org/latest/install_config/install/advanced_install.html#multiple-masters +""" + confirm_continue(message) + + nodes = [host.node for host in oo_cfg.hosts] + if len(masters) == len(nodes): + message = """ +No dedicated Nodes specified. By default, colocated Masters have their Nodes set to unscheduleable. +Would you like to label the colocated masters as scheduleable? +""" + confirm_continue(message) + + return + def get_variant_and_version(): message = "\nWhich variant would you like to install?\n\n" @@ -555,6 +615,8 @@ def install(ctx, force): else: oo_cfg = get_missing_info_from_user(oo_cfg) + check_hosts_config(oo_cfg) + click.echo('Gathering information from hosts...') callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, verbose) diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index 9c97e6e93..6b4f36204 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -36,19 +36,24 @@ class Host(object): self.public_ip = kwargs.get('public_ip', None) self.public_hostname = kwargs.get('public_hostname', None) self.connect_to = kwargs.get('connect_to', None) + self.run_on = kwargs.get('run_on', None) # Should this host run as an OpenShift master: self.master = kwargs.get('master', False) # Should this host run as an OpenShift node: self.node = kwargs.get('node', False) + + # Should this host run as an HAProxy: + self.ha_proxy = kwargs.get('ha_proxy', False) + self.containerized = kwargs.get('containerized', False) if self.connect_to is None: raise OOConfigInvalidHostError("You must specify either and 'ip' " \ "or 'hostname' to connect to.") - if self.master is False and self.node is False: + if self.master is False and self.node is False and self.ha_proxy is False: raise OOConfigInvalidHostError( "You must specify each host as either a master or a node.") @@ -62,7 +67,7 @@ class Host(object): """ Used when exporting to yaml. """ d = {} for prop in ['ip', 'hostname', 'public_ip', 'public_hostname', - 'master', 'node', 'containerized', 'connect_to']: + 'master', 'node', 'ha_proxy', 'containerized', 'connect_to', 'run_on']: # If the property is defined (not None or False), export it: if getattr(self, prop): d[prop] = getattr(self, prop) diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 372f27bda..ec97c4144 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -17,14 +17,31 @@ def set_config(cfg): def generate_inventory(hosts): global CFG + masters = [host for host in hosts if host.master] + nodes = [host for host in hosts if host.node] + proxy = next((host for host in hosts if host.ha_proxy), None) + multiple_masters = len(masters) > 1 base_inventory_path = CFG.settings['ansible_inventory_path'] base_inventory = open(base_inventory_path, 'w') - base_inventory.write('\n[OSEv3:children]\nmasters\nnodes\n') + + base_inventory.write('\n[OSEv3:children]\n') + base_inventory.write('masters\n') + base_inventory.write('nodes\n') + if multiple_masters: + base_inventory.write('etcd\n') + if getattr(proxy, 'run_on', False): + base_inventory.write('lb\n') + base_inventory.write('\n[OSEv3:vars]\n') base_inventory.write('ansible_ssh_user={}\n'.format(CFG.settings['ansible_ssh_user'])) if CFG.settings['ansible_ssh_user'] != 'root': base_inventory.write('ansible_become=true\n') + if multiple_masters: + base_inventory.write('openshift_master_cluster_method=native\n') + base_inventory.write("openshift_master_cluster_hostname={}\n".format(proxy.hostname)) + base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) + # Find the correct deployment type for ansible: ver = find_variant(CFG.settings['variant'], @@ -45,19 +62,28 @@ def generate_inventory(hosts): "'enabled': 1, 'gpgcheck': 0}}]\n".format(os.environ['OO_INSTALL_PUDDLE_REPO'])) base_inventory.write('\n[masters]\n') - masters = (host for host in hosts if host.master) for master in masters: write_host(master, base_inventory) + + if len(masters) > 1: + base_inventory.write('\n[etcd]\n') + for master in masters: + write_host(master, base_inventory) + base_inventory.write('\n[nodes]\n') - nodes = (host for host in hosts if host.node) for node in nodes: # TODO: Until the Master can run the SDN itself we have to configure the Masters # as Nodes too. scheduleable = True # If there's only one Node and it's also a Master we want it to be scheduleable: - if node in masters and len(masters) != 1: + if node in masters and len(masters) != len(nodes): scheduleable = False write_host(node, base_inventory, scheduleable) + + if getattr(proxy, 'run_on', False): + base_inventory.write('\n[lb]\n') + write_host(proxy, base_inventory) + base_inventory.close() return base_inventory_path @@ -118,6 +144,7 @@ def default_facts(hosts, verbose=False): facts_env = os.environ.copy() facts_env["OO_INSTALL_CALLBACK_FACTS_YAML"] = CFG.settings['ansible_callback_facts_yaml'] facts_env["ANSIBLE_CALLBACK_PLUGINS"] = CFG.settings['ansible_plugins_directory'] + facts_env["OPENSHIFT_MASTER_CLUSTER_METHOD"] = 'native' if 'ansible_log_path' in CFG.settings: facts_env["ANSIBLE_LOG_PATH"] = CFG.settings['ansible_log_path'] if 'ansible_config' in CFG.settings: @@ -176,4 +203,3 @@ def run_upgrade_playbook(verbose=False): if 'ansible_config' in CFG.settings: facts_env['ANSIBLE_CONFIG'] = CFG.settings['ansible_config'] return run_ansible(playbook, inventory_file, facts_env, verbose) - diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 40a2f844d..61bc77445 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -613,6 +613,7 @@ class AttendedCliTests(OOCliFixture): result = self.runner.invoke(cli.cli, self.cli_args, input=cli_input) + print result self.assert_result(result, 0) self._verify_load_facts(load_facts_mock) -- cgit v1.2.3 From da2eddb79784ddcff75cbb71a4d7cc77b3e86386 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 10:09:24 -0500 Subject: Add interactive test --- utils/test/cli_installer_tests.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 61bc77445..b5ef87cce 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -668,6 +668,33 @@ class AttendedCliTests(OOCliFixture): exp_hosts_to_run_on_len=2, force=False) + #interactive multimaster + @patch('ooinstall.openshift_ansible.run_main_playbook') + @patch('ooinstall.openshift_ansible.load_system_facts') + def test_quick_ha(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=[ + ('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') + 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, 3, 3) + + written_config = self._read_yaml(self.config_file) + self._verify_config_hosts(written_config, 3) + + return # 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 -- cgit v1.2.3 From d446b2fe458444a88ceffd1d1966d6814b5185b5 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 10:41:42 -0500 Subject: Enforce 1 or 3 masters --- utils/src/ooinstall/cli_installer.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 1b4a67259..fce8f9b22 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -113,9 +113,12 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen if click.confirm('Will this host be an OpenShift Master?'): host_props['master'] = True num_masters += 1 + + if num_masters > 1: + hosts.append(collect_ha_proxy()) + if num_masters >= 3: masters_set = True - hosts.append(collect_ha_proxy()) host_props['node'] = True #TODO: Reenable this option once container installs are out of tech preview @@ -132,7 +135,8 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts.append(host) - more_hosts = click.confirm('Do you want to add additional hosts?') + if num_masters <= 1 or num_masters >= 3: + more_hosts = click.confirm('Do you want to add additional hosts?') return hosts def collect_ha_proxy(): @@ -144,6 +148,8 @@ Setting up High Availability Masters requires a load balancing solution. Please provide a host that will be configured as a proxy. This can either be an existing load balancer configured to balance all masters on port 8443 or a new host that will have HAProxy installed on it. + +This will also require you to set a third master. """ click.echo(message) host_props = {} -- cgit v1.2.3 From 296d1283202fe92de3045eab8bbc60db192f3a46 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 10:56:19 -0500 Subject: Breakup inventory writing --- utils/src/ooinstall/openshift_ansible.py | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index ec97c4144..ff674153d 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -25,23 +25,9 @@ def generate_inventory(hosts): base_inventory_path = CFG.settings['ansible_inventory_path'] base_inventory = open(base_inventory_path, 'w') - base_inventory.write('\n[OSEv3:children]\n') - base_inventory.write('masters\n') - base_inventory.write('nodes\n') - if multiple_masters: - base_inventory.write('etcd\n') - if getattr(proxy, 'run_on', False): - base_inventory.write('lb\n') - - base_inventory.write('\n[OSEv3:vars]\n') - base_inventory.write('ansible_ssh_user={}\n'.format(CFG.settings['ansible_ssh_user'])) - if CFG.settings['ansible_ssh_user'] != 'root': - base_inventory.write('ansible_become=true\n') - if multiple_masters: - base_inventory.write('openshift_master_cluster_method=native\n') - base_inventory.write("openshift_master_cluster_hostname={}\n".format(proxy.hostname)) - base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) + write_inventory_children(base_inventory, multiple_masters, proxy) + write_inventory_vars(base_inventory, multiple_masters, proxy) # Find the correct deployment type for ansible: ver = find_variant(CFG.settings['variant'], @@ -87,6 +73,28 @@ def generate_inventory(hosts): base_inventory.close() return base_inventory_path +def write_inventory_children(base_inventory, multiple_masters, proxy): + global CFG + + base_inventory.write('\n[OSEv3:children]\n') + base_inventory.write('masters\n') + base_inventory.write('nodes\n') + if multiple_masters: + base_inventory.write('etcd\n') + if getattr(proxy, 'run_on', False): + base_inventory.write('lb\n') + +def write_inventory_vars(base_inventory, multiple_masters, proxy): + global CFG + base_inventory.write('\n[OSEv3:vars]\n') + base_inventory.write('ansible_ssh_user={}\n'.format(CFG.settings['ansible_ssh_user'])) + if CFG.settings['ansible_ssh_user'] != 'root': + base_inventory.write('ansible_become=true\n') + if multiple_masters: + base_inventory.write('openshift_master_cluster_method=native\n') + base_inventory.write("openshift_master_cluster_hostname={}\n".format(proxy.hostname)) + base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) + def write_host(host, inventory, scheduleable=True): global CFG -- cgit v1.2.3 From 992d147e722795be98bcb1a8b890c66035ab6c49 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 15:13:51 -0500 Subject: cli_installer_tests: Add test for unattended quick HA --- utils/test/cli_installer_tests.py | 101 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index b5ef87cce..87aafe782 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -41,6 +41,41 @@ MOCK_FACTS = { }, } +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' + } + }, +} + # Substitute in a product name before use: SAMPLE_CONFIG = """ variant: %s @@ -91,6 +126,38 @@ hosts: node: true """ +QUICKHA_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.3 + ip: 10.0.0.3 + hostname: node2-private.example.com + public_ip: 24.222.0.3 + public_hostname: node2.example.com + node: true + - connect_to: 10.0.0.4 + ip: 10.0.0.4 + hostname: proxy-private.example.com + public_ip: 24.222.0.4 + public_hostname: proxy.example.com + ha_proxy: true +""" + class OOCliFixture(OOInstallFixture): def setUp(self): @@ -504,6 +571,40 @@ class UnattendedCliTests(OOCliFixture): assert result.exit_code == 1 assert result.output == "You must specify either and 'ip' or 'hostname' to connect to.\n" + #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_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) + + 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']) + # 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] + hosts_to_run_on = run_playbook_mock.call_args[0][1] + self.assertEquals(4, len(hosts)) + self.assertEquals(4, len(hosts_to_run_on)) + class AttendedCliTests(OOCliFixture): def setUp(self): -- cgit v1.2.3 From 16e373e9c71f929e3eaf5d747e1f1ad9057c0184 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 15:34:35 -0500 Subject: atomic-openshift-installer: Reverse version and host collection Reverse the order we ask two questions: What variant the user wants to install and which hosts to install on. This lets us avoid asking for multiple masters for 3.0 installs. --- utils/src/ooinstall/cli_installer.py | 18 ++++++++++-------- utils/test/cli_installer_tests.py | 6 +++--- 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index fce8f9b22..01093379f 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -72,7 +72,7 @@ def delete_hosts(hosts): click.echo("\"{}\" doesn't coorespond to any valid input.".format(del_idx)) return hosts, None -def collect_hosts(masters_set=False): +def collect_hosts(version=None, masters_set=False): """ Collect host information from user. This will later be filled in using ansible. @@ -117,7 +117,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen if num_masters > 1: hosts.append(collect_ha_proxy()) - if num_masters >= 3: + if num_masters >= 3 or version == '3.0': masters_set = True host_props['node'] = True @@ -265,7 +265,7 @@ Would you like to label the colocated masters as scheduleable? return -def get_variant_and_version(): +def get_variant_and_version(multi_master=False): message = "\nWhich variant would you like to install?\n\n" i = 1 @@ -277,6 +277,8 @@ def get_variant_and_version(): message = "%s\n" % message click.echo(message) + if multi_master: + click.echo('NOTE: 3.0 installations are not') response = click.prompt("Choose a variant from above: ", default=1) product, version = combos[response - 1] @@ -358,16 +360,16 @@ https://docs.openshift.com/enterprise/latest/admin_guide/install/prerequisites.h oo_cfg.settings['ansible_ssh_user'] = get_ansible_ssh_user() click.clear() - if not oo_cfg.hosts: - oo_cfg.hosts = collect_hosts() - click.clear() - if oo_cfg.settings.get('variant', '') == '': variant, version = get_variant_and_version() oo_cfg.settings['variant'] = variant.name oo_cfg.settings['variant_version'] = version.name click.clear() + if not oo_cfg.hosts: + oo_cfg.hosts = collect_hosts(version=oo_cfg.settings['variant_version']) + click.clear() + return oo_cfg @@ -378,7 +380,7 @@ def collect_new_nodes(): Add new nodes here """ click.echo(message) - return collect_hosts(True) + return collect_hosts(masters_set=True) def get_installed_hosts(hosts, callback_facts): installed_hosts = [] diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 87aafe782..9cb44404c 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -628,6 +628,9 @@ class AttendedCliTests(OOCliFixture): if ssh_user: inputs.append(ssh_user) + if variant_num: + inputs.append(str(variant_num)) # Choose variant + version + if hosts: i = 0 for (host, is_master) in hosts: @@ -640,9 +643,6 @@ class AttendedCliTests(OOCliFixture): 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 -- cgit v1.2.3 From ac6bc198d469315e7fec2452211c13d37abca795 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Fri, 20 Nov 2015 15:58:05 -0500 Subject: atomic-openshift-installer: Rename ha_proxy Rename ha_proxy variables and methods to 'master_lb' to better future-proof things. --- utils/src/ooinstall/cli_installer.py | 20 ++++++++++---------- utils/src/ooinstall/oo_config.py | 6 +++--- utils/src/ooinstall/openshift_ansible.py | 2 +- utils/test/cli_installer_tests.py | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 01093379f..ac9d884d9 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -115,7 +115,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen num_masters += 1 if num_masters > 1: - hosts.append(collect_ha_proxy()) + hosts.append(collect_master_lb()) if num_masters >= 3 or version == '3.0': masters_set = True @@ -139,7 +139,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen more_hosts = click.confirm('Do you want to add additional hosts?') return hosts -def collect_ha_proxy(): +def collect_master_lb(): """ Get an HA proxy from the user """ @@ -159,10 +159,10 @@ This will also require you to set a third master. host_props['run_on'] = click.confirm('Is this a clean host you want to install HAProxy on?') host_props['master'] = False host_props['node'] = False - host_props['ha_proxy'] = True - ha_proxy = Host(**host_props) + host_props['master_lb'] = True + master_lb = Host(**host_props) - return ha_proxy + return master_lb def confirm_hosts_facts(oo_cfg, callback_facts): hosts = oo_cfg.hosts @@ -237,13 +237,13 @@ def check_hosts_config(oo_cfg): click.clear() masters = [host for host in oo_cfg.hosts if host.master] if len(masters) > 1: - ha_proxy = [host for host in oo_cfg.hosts if host.ha_proxy] - click.echo(ha_proxy) - if len(ha_proxy) > 1: + master_lb = [host for host in oo_cfg.hosts if host.master_lb] + click.echo(master_lb) + if len(master_lb) > 1: click.echo('More than one HAProxy specified. Only one proxy is allowed.') sys.exit(0) - elif len(ha_proxy) == 1: - if ha_proxy[0].master or ha_proxy[0].node: + elif len(master_lb) == 1: + if master_lb[0].master or master_lb[0].node: click.echo('HAProxy is configured as a master or node. Please correct this.') sys.exit(0) else: diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index 6b4f36204..243a75b09 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -45,7 +45,7 @@ class Host(object): self.node = kwargs.get('node', False) # Should this host run as an HAProxy: - self.ha_proxy = kwargs.get('ha_proxy', False) + self.master_lb = kwargs.get('master_lb', False) self.containerized = kwargs.get('containerized', False) @@ -53,7 +53,7 @@ class Host(object): raise OOConfigInvalidHostError("You must specify either and 'ip' " \ "or 'hostname' to connect to.") - if self.master is False and self.node is False and self.ha_proxy is False: + if self.master is False and self.node is False and self.master_lb is False: raise OOConfigInvalidHostError( "You must specify each host as either a master or a node.") @@ -67,7 +67,7 @@ class Host(object): """ Used when exporting to yaml. """ d = {} for prop in ['ip', 'hostname', 'public_ip', 'public_hostname', - 'master', 'node', 'ha_proxy', 'containerized', 'connect_to', 'run_on']: + 'master', 'node', 'master_lb', 'containerized', 'connect_to', 'run_on']: # If the property is defined (not None or False), export it: if getattr(self, prop): d[prop] = getattr(self, prop) diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index ff674153d..86c707b17 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -19,7 +19,7 @@ def generate_inventory(hosts): global CFG masters = [host for host in hosts if host.master] nodes = [host for host in hosts if host.node] - proxy = next((host for host in hosts if host.ha_proxy), None) + proxy = next((host for host in hosts if host.master_lb), None) multiple_masters = len(masters) > 1 base_inventory_path = CFG.settings['ansible_inventory_path'] diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 9cb44404c..ad00af76f 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -155,7 +155,7 @@ hosts: hostname: proxy-private.example.com public_ip: 24.222.0.4 public_hostname: proxy.example.com - ha_proxy: true + master_lb: true """ class OOCliFixture(OOInstallFixture): -- cgit v1.2.3 From c148283d1731d08fbfd4446af88450c5983c7b7a Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Mon, 23 Nov 2015 11:56:27 -0500 Subject: Handling preconfigured load balancers The preconfigured load balancers, previously denoted by having 'run_on' set to false, cannot have their facts gathered which results in a stack trace. Later when we write out the inventory we have to fake out the hostname and just use 'connect_to'. We're likely going to have the concept of other types of "plug-in" hosts where we don't run ansible. We should make sure we abstract this properly so it's easy to add additional types of hosts. Also in the commit: - Renamed 'run_on' to 'preconfigured' and inverted the logic as needed - Output tally of Masters and Nodes as well as remaining Masters required for HA - Minor rewording in a few places - Currently only prompting for the load balancer after all other hosts have been entered - Removed spurious echo --- utils/src/ooinstall/cli_installer.py | 34 ++++++++++++++++++++++---------- utils/src/ooinstall/oo_config.py | 4 ++-- utils/src/ooinstall/openshift_ansible.py | 17 +++++++++++++--- 3 files changed, 40 insertions(+), 15 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index ac9d884d9..9abea0683 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -79,6 +79,7 @@ def collect_hosts(version=None, masters_set=False): Returns: a list of host information collected from the user """ + min_masters_for_ha = 3 click.clear() click.echo('***Host Configuration***') message = """ @@ -114,10 +115,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen host_props['master'] = True num_masters += 1 - if num_masters > 1: - hosts.append(collect_master_lb()) - - if num_masters >= 3 or version == '3.0': + if num_masters >= min_masters_for_ha or version == '3.0': masters_set = True host_props['node'] = True @@ -135,8 +133,18 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts.append(host) - if num_masters <= 1 or num_masters >= 3: + click.echo('') + click.echo('Current Masters: {}'.format(num_masters)) + click.echo('Current Nodes: {}'.format(len(hosts))) + click.echo('Additional Masters required for HA: {}'.format(max(min_masters_for_ha - num_masters, 0))) + click.echo('') + + if num_masters <= 1 or num_masters >= min_masters_for_ha: more_hosts = click.confirm('Do you want to add additional hosts?') + + if num_masters > 1: + hosts.append(collect_master_lb()) + return hosts def collect_master_lb(): @@ -149,14 +157,19 @@ Please provide a host that will be configured as a proxy. This can either be an existing load balancer configured to balance all masters on port 8443 or a new host that will have HAProxy installed on it. -This will also require you to set a third master. +If the host provided does is not yet configured a reference haproxy load +balancer will be installed. It's important to note that while the rest of the +environment will be fault tolerant this reference load balancer will not be. +It can be replaced post-installation with a load balancer with the same +hostname. """ click.echo(message) host_props = {} host_props['connect_to'] = click.prompt('Enter hostname or IP address:', default='', value_proc=validate_prompt_hostname) - host_props['run_on'] = click.confirm('Is this a clean host you want to install HAProxy on?') + install_haproxy = click.confirm('Should the reference haproxy load balancer be installed on this host?') + host_props['preconfigured'] = not install_haproxy host_props['master'] = False host_props['node'] = False host_props['master_lb'] = True @@ -201,6 +214,8 @@ Notes: default_facts_lines = [] default_facts = {} for h in hosts: + if h.preconfigured == True: + continue default_facts[h.connect_to] = {} h.ip = callback_facts[h.connect_to]["common"]["ip"] h.public_ip = callback_facts[h.connect_to]["common"]["public_ip"] @@ -238,13 +253,12 @@ def check_hosts_config(oo_cfg): masters = [host for host in oo_cfg.hosts if host.master] if len(masters) > 1: master_lb = [host for host in oo_cfg.hosts if host.master_lb] - click.echo(master_lb) if len(master_lb) > 1: - click.echo('More than one HAProxy specified. Only one proxy is allowed.') + click.echo('More than one Master load balancer specified. Only one is allowed.') sys.exit(0) elif len(master_lb) == 1: if master_lb[0].master or master_lb[0].node: - click.echo('HAProxy is configured as a master or node. Please correct this.') + click.echo('The Master load balancer is configured as a master or node. Please correct this.') sys.exit(0) else: message = """ diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index 243a75b09..b6f0cdce3 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -36,7 +36,7 @@ class Host(object): self.public_ip = kwargs.get('public_ip', None) self.public_hostname = kwargs.get('public_hostname', None) self.connect_to = kwargs.get('connect_to', None) - self.run_on = kwargs.get('run_on', None) + self.preconfigured = kwargs.get('preconfigured', None) # Should this host run as an OpenShift master: self.master = kwargs.get('master', False) @@ -67,7 +67,7 @@ class Host(object): """ Used when exporting to yaml. """ d = {} for prop in ['ip', 'hostname', 'public_ip', 'public_hostname', - 'master', 'node', 'master_lb', 'containerized', 'connect_to', 'run_on']: + 'master', 'node', 'master_lb', 'containerized', 'connect_to', 'preconfigured']: # If the property is defined (not None or False), export it: if getattr(self, prop): d[prop] = getattr(self, prop) diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 86c707b17..ed1ba2c77 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -19,7 +19,7 @@ def generate_inventory(hosts): global CFG masters = [host for host in hosts if host.master] nodes = [host for host in hosts if host.node] - proxy = next((host for host in hosts if host.master_lb), None) + proxy = determine_proxy_configuration(hosts) multiple_masters = len(masters) > 1 base_inventory_path = CFG.settings['ansible_inventory_path'] @@ -66,13 +66,24 @@ def generate_inventory(hosts): scheduleable = False write_host(node, base_inventory, scheduleable) - if getattr(proxy, 'run_on', False): + if not getattr(proxy, 'preconfigured', True): base_inventory.write('\n[lb]\n') write_host(proxy, base_inventory) base_inventory.close() return base_inventory_path +def determine_proxy_configuration(hosts): + proxy = next((host for host in hosts if host.master_lb), None) + if proxy: + if proxy.hostname == None: + proxy.hostname = proxy.connect_to + proxy.public_hostname = proxy.connect_to + print('asd09o') + return proxy + + return None + def write_inventory_children(base_inventory, multiple_masters, proxy): global CFG @@ -81,7 +92,7 @@ def write_inventory_children(base_inventory, multiple_masters, proxy): base_inventory.write('nodes\n') if multiple_masters: base_inventory.write('etcd\n') - if getattr(proxy, 'run_on', False): + if not getattr(proxy, 'preconfigured', True): base_inventory.write('lb\n') def write_inventory_vars(base_inventory, multiple_masters, proxy): -- cgit v1.2.3 From 54ba429f37454b7ed7564ae53fe266da9f955ba4 Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Mon, 23 Nov 2015 18:08:26 -0500 Subject: atomic-openshift-installer: Fix lint issue --- utils/src/ooinstall/openshift_ansible.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index ed1ba2c77..d098a5593 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -79,7 +79,7 @@ def determine_proxy_configuration(hosts): if proxy.hostname == None: proxy.hostname = proxy.connect_to proxy.public_hostname = proxy.connect_to - print('asd09o') + print 'asd09o' return proxy return None -- cgit v1.2.3 From 6a988e92ff6d63229cecf9b2d571ce114af820d5 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 08:32:46 -0500 Subject: Removing a debug line --- utils/src/ooinstall/openshift_ansible.py | 1 - 1 file changed, 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index d098a5593..66b9ead93 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -79,7 +79,6 @@ def determine_proxy_configuration(hosts): if proxy.hostname == None: proxy.hostname = proxy.connect_to proxy.public_hostname = proxy.connect_to - print 'asd09o' return proxy return None -- cgit v1.2.3 From 1c326a03caf06e388ecb9a2c3d140445f60005ba Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 09:48:42 -0500 Subject: Fixing tests for quick_ha Also: * minor rewording of the text that informs the admin about scheduleable masters. --- utils/src/ooinstall/cli_installer.py | 5 +++-- utils/test/cli_installer_tests.py | 38 +++++++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 13 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 9abea0683..3eee0c32b 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -272,8 +272,9 @@ https://docs.openshift.org/latest/install_config/install/advanced_install.html#m nodes = [host.node for host in oo_cfg.hosts] if len(masters) == len(nodes): message = """ -No dedicated Nodes specified. By default, colocated Masters have their Nodes set to unscheduleable. -Would you like to label the colocated masters as scheduleable? +No dedicated Nodes specified. By default, colocated Masters have their Nodes +set to unscheduleable. Continuing at this point will lable all nodes as +scheduleable. """ confirm_continue(message) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index ad00af76f..9d87b6607 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -212,11 +212,12 @@ class OOCliFixture(OOInstallFixture): 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) + if 'preconfigured' not in h: + self.assertTrue(h['node']) + self.assertTrue('ip' in h) + self.assertTrue('public_ip' in h) #pylint: disable=too-many-arguments def _verify_get_hosts_to_run_on(self, mock_facts, load_facts_mock, @@ -615,7 +616,8 @@ class AttendedCliTests(OOCliFixture): #pylint: disable=too-many-arguments def _build_input(self, ssh_user=None, hosts=None, variant_num=None, - add_nodes=None, confirm_facts=None): + add_nodes=None, confirm_facts=None, scheduleable_masters_ok=None, + master_lb=None): """ Builds a CLI input string with newline characters to simulate the full run. @@ -633,23 +635,35 @@ class AttendedCliTests(OOCliFixture): if hosts: i = 0 + num_masters = 0 + min_masters_for_ha = 3 for (host, is_master) in hosts: inputs.append(host) - inputs.append('y' if is_master else 'n') + if is_master: + inputs.append('y') + num_masters += 1 + else: + inputs.append('n') #inputs.append('rpm') if i < len(hosts) - 1: - inputs.append('y') # Add more hosts + if num_masters <= 1 or num_masters >= min_masters_for_ha: + inputs.append('y') # Add more hosts else: inputs.append('n') # Done adding hosts i += 1 + if master_lb: + inputs.append(master_lb[0]) + inputs.append('y' if master_lb[1] else 'n') + # TODO: support option 2, fresh install if add_nodes: + if scheduleable_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('y' if is_master else 'n') #inputs.append('rpm') if i < len(add_nodes) - 1: inputs.append('y') # Add more hosts @@ -760,6 +774,7 @@ class AttendedCliTests(OOCliFixture): add_nodes=[('10.0.0.2', False)], ssh_user='root', variant_num=1, + scheduleable_masters_ok=True, confirm_facts='y') self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock, @@ -773,7 +788,7 @@ class AttendedCliTests(OOCliFixture): @patch('ooinstall.openshift_ansible.run_main_playbook') @patch('ooinstall.openshift_ansible.load_system_facts') def test_quick_ha(self, load_facts_mock, run_playbook_mock): - load_facts_mock.return_value = (MOCK_FACTS, 0) + load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0) run_playbook_mock.return_value = 0 cli_input = self._build_input(hosts=[ @@ -783,17 +798,18 @@ class AttendedCliTests(OOCliFixture): ('10.0.0.4', True)], ssh_user='root', variant_num=1, - confirm_facts='y') + 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, 3, 3) + self._verify_run_playbook(run_playbook_mock, 5, 5) written_config = self._read_yaml(self.config_file) - self._verify_config_hosts(written_config, 3) + self._verify_config_hosts(written_config, 5) return # TODO: test with config file, attended add node -- cgit v1.2.3 From ba47106f21fd18c64c3a3bb89980a3e857fbd034 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 10:34:51 -0500 Subject: Avoid printing the master and node totals in the add-a-node scenario --- utils/src/ooinstall/cli_installer.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 3eee0c32b..812a42795 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -72,7 +72,7 @@ def delete_hosts(hosts): click.echo("\"{}\" doesn't coorespond to any valid input.".format(del_idx)) return hosts, None -def collect_hosts(version=None, masters_set=False): +def collect_hosts(version=None, masters_set=False, print_summary=True): """ Collect host information from user. This will later be filled in using ansible. @@ -133,11 +133,12 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts.append(host) - click.echo('') - click.echo('Current Masters: {}'.format(num_masters)) - click.echo('Current Nodes: {}'.format(len(hosts))) - click.echo('Additional Masters required for HA: {}'.format(max(min_masters_for_ha - num_masters, 0))) - click.echo('') + if print_summary: + click.echo('') + click.echo('Current Masters: {}'.format(num_masters)) + click.echo('Current Nodes: {}'.format(len(hosts))) + click.echo('Additional Masters required for HA: {}'.format(max(min_masters_for_ha - num_masters, 0))) + click.echo('') if num_masters <= 1 or num_masters >= min_masters_for_ha: more_hosts = click.confirm('Do you want to add additional hosts?') @@ -395,7 +396,7 @@ def collect_new_nodes(): Add new nodes here """ click.echo(message) - return collect_hosts(masters_set=True) + return collect_hosts(masters_set=True, print_summary=False) def get_installed_hosts(hosts, callback_facts): installed_hosts = [] -- cgit v1.2.3 From dbb557ec080cf3e17cfb5a09371fa89b8eb7023d Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 11:45:48 -0500 Subject: Bug 1284991 - "atomic-openshift-installer uninstall" error when configuration file is missing. --- utils/src/ooinstall/cli_installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 0b3af8829..84092a774 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -487,7 +487,7 @@ def uninstall(ctx): verbose = ctx.obj['verbose'] if len(oo_cfg.hosts) == 0: - click.echo("No hosts defined in: %s" % oo_cfg['configuration']) + click.echo("No hosts defined in: %s" % oo_cfg.config_path) sys.exit(1) click.echo("OpenShift will be uninstalled from the following hosts:\n") -- cgit v1.2.3 From 05f63f022d1a42da6b5a34ae58c1eda745757b7c Mon Sep 17 00:00:00 2001 From: Jason DeTiberus Date: Tue, 24 Nov 2015 15:57:06 -0500 Subject: fixes for installer wrapper scaleup --- utils/src/ooinstall/openshift_ansible.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 372f27bda..e9402dfb1 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -130,10 +130,10 @@ def run_main_playbook(hosts, hosts_to_run_on, verbose=False): inventory_file = generate_inventory(hosts_to_run_on) if len(hosts_to_run_on) != len(hosts): main_playbook_path = os.path.join(CFG.ansible_playbook_directory, - 'playbooks/common/openshift-cluster/scaleup.yml') + 'playbooks/byo/openshift-cluster/scaleup.yml') else: main_playbook_path = os.path.join(CFG.ansible_playbook_directory, - 'playbooks/byo/config.yml') + 'playbooks/byo/openshift-cluster/config.yml') facts_env = os.environ.copy() if 'ansible_log_path' in CFG.settings: facts_env['ANSIBLE_LOG_PATH'] = CFG.settings['ansible_log_path'] -- cgit v1.2.3 From ed650557aef9bb1d18b755ffdf891fcb26bb20cb Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 15:05:27 -0500 Subject: Properly setting scheduleability for HA Master scenarios If the only Nodes we have are also on Masters we set the scheduleable. --- utils/src/ooinstall/cli_installer.py | 4 +- utils/src/ooinstall/openshift_ansible.py | 22 ++++--- utils/test/cli_installer_tests.py | 101 ++++++++++++++++++++++++++++++- 3 files changed, 114 insertions(+), 13 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 812a42795..c62461ad3 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -270,11 +270,11 @@ https://docs.openshift.org/latest/install_config/install/advanced_install.html#m """ confirm_continue(message) - nodes = [host.node for host in oo_cfg.hosts] + nodes = [host for host in oo_cfg.hosts if host.node] if len(masters) == len(nodes): message = """ No dedicated Nodes specified. By default, colocated Masters have their Nodes -set to unscheduleable. Continuing at this point will lable all nodes as +set to unscheduleable. Continuing at this point will label all nodes as scheduleable. """ confirm_continue(message) diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 66b9ead93..75125084c 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -57,14 +57,20 @@ def generate_inventory(hosts): write_host(master, base_inventory) base_inventory.write('\n[nodes]\n') - for node in nodes: - # TODO: Until the Master can run the SDN itself we have to configure the Masters - # as Nodes too. - scheduleable = True - # If there's only one Node and it's also a Master we want it to be scheduleable: - if node in masters and len(masters) != len(nodes): - scheduleable = False - write_host(node, base_inventory, scheduleable) + + # TODO: It would be much better to calculate the scheduleability elsewhere + # and store it on the Node object. + if set(nodes) == set(masters): + for node in nodes: + write_host(node, base_inventory) + else: + for node in nodes: + # TODO: Until the Master can run the SDN itself we have to configure the Masters + # as Nodes too. + scheduleable = True + if node in masters: + scheduleable = False + write_host(node, base_inventory, scheduleable) if not getattr(proxy, 'preconfigured', True): base_inventory.write('\n[lb]\n') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 9d87b6607..d78153dd9 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -633,9 +633,9 @@ class AttendedCliTests(OOCliFixture): if variant_num: inputs.append(str(variant_num)) # Choose variant + version + num_masters = 0 if hosts: i = 0 - num_masters = 0 min_masters_for_ha = 3 for (host, is_master) in hosts: inputs.append(host) @@ -671,6 +671,13 @@ class AttendedCliTests(OOCliFixture): 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 @@ -702,6 +709,15 @@ class AttendedCliTests(OOCliFixture): written_config = self._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_scheduleable')) + 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') @@ -784,10 +800,10 @@ class AttendedCliTests(OOCliFixture): exp_hosts_to_run_on_len=2, force=False) - #interactive multimaster + #interactive multimaster: one more node than master @patch('ooinstall.openshift_ansible.run_main_playbook') @patch('ooinstall.openshift_ansible.load_system_facts') - def test_quick_ha(self, load_facts_mock, run_playbook_mock): + def test_quick_ha1(self, load_facts_mock, run_playbook_mock): load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0) run_playbook_mock.return_value = 0 @@ -811,7 +827,86 @@ class AttendedCliTests(OOCliFixture): written_config = self._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_scheduleable')) + self.assertEquals('False', + inventory.get('nodes', '10.0.0.2 openshift_scheduleable')) + self.assertEquals(None, + inventory.get('nodes', '10.0.0.3')) + self.assertEquals('False', + inventory.get('nodes', '10.0.0.4 openshift_scheduleable')) + + return + + #interactive multimaster: equal number masters and nodes + @patch('ooinstall.openshift_ansible.run_main_playbook') + @patch('ooinstall.openshift_ansible.load_system_facts') + def test_quick_ha2(self, load_facts_mock, run_playbook_mock): + load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 0) + run_playbook_mock.return_value = 0 + + cli_input = self._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 = self._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(None, + inventory.get('nodes', '10.0.0.1')) + self.assertEquals(None, + inventory.get('nodes', '10.0.0.2')) + self.assertEquals(None, + inventory.get('nodes', '10.0.0.3')) + return + + #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 = self._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 = self._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(None, + inventory.get('nodes', '10.0.0.1')) + + return + # 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 -- cgit v1.2.3 From 7b85e1dfa5b96246807c7987a75a1d1f5478de36 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 16:54:17 -0500 Subject: Silencing pylint branch errors for now for the atomic-openshift-installer harness --- utils/test/cli_installer_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index d78153dd9..c951b6580 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -614,7 +614,7 @@ 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 + #pylint: disable=too-many-arguments,too-many-branches def _build_input(self, ssh_user=None, hosts=None, variant_num=None, add_nodes=None, confirm_facts=None, scheduleable_masters_ok=None, master_lb=None): -- cgit v1.2.3 From 80c166ab60c4608ac83afb865f76b4206d593818 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Wed, 25 Nov 2015 15:17:38 -0400 Subject: Explicitly set schedulable when masters == nodes. When the masters are the only nodes in play, we need to explicitly set schedulable to True due to logic in openshift_facts.py which assumes that if the node is also a master, schedulable should be false. --- utils/src/ooinstall/openshift_ansible.py | 18 +++++++++++++----- utils/test/cli_installer_tests.py | 32 ++++++++++++-------------------- 2 files changed, 25 insertions(+), 25 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 9afc9a644..84e4db61d 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -62,12 +62,12 @@ def generate_inventory(hosts): # and store it on the Node object. if set(nodes) == set(masters): for node in nodes: - write_host(node, base_inventory) + write_host(node, base_inventory, True) else: for node in nodes: # TODO: Until the Master can run the SDN itself we have to configure the Masters # as Nodes too. - scheduleable = True + scheduleable = None if node in masters: scheduleable = False write_host(node, base_inventory, scheduleable) @@ -112,7 +112,7 @@ def write_inventory_vars(base_inventory, multiple_masters, proxy): base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) -def write_host(host, inventory, scheduleable=True): +def write_host(host, inventory, scheduleable=None): global CFG facts = '' @@ -126,8 +126,16 @@ def write_host(host, inventory, scheduleable=True): facts += ' openshift_public_hostname={}'.format(host.public_hostname) # TODO: For not write_host is handles both master and nodes. # Technically only nodes will ever need this. - if not scheduleable: - facts += ' openshift_scheduleable=False' + + # Distinguish between three states, no schedulability specified (use default), + # explicitly set to True, or explicitly set to False: + if scheduleable is None: + pass + elif scheduleable: + facts += ' openshift_schedulable=True' + elif not scheduleable: + facts += ' openshift_schedulable=False' + installer_host = socket.gethostname() if installer_host in [host.connect_to, host.hostname, host.public_hostname]: facts += ' ansible_connection=local' diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index c951b6580..8e9c2d698 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -209,7 +209,6 @@ class OOCliFixture(OOInstallFixture): 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('hostname' in h) @@ -712,7 +711,7 @@ class AttendedCliTests(OOCliFixture): 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_scheduleable')) + inventory.get('nodes', '10.0.0.1 openshift_schedulable')) self.assertEquals(None, inventory.get('nodes', '10.0.0.2')) self.assertEquals(None, @@ -744,7 +743,6 @@ class AttendedCliTests(OOCliFixture): result = self.runner.invoke(cli.cli, self.cli_args, input=cli_input) - print result self.assert_result(result, 0) self._verify_load_facts(load_facts_mock) @@ -830,15 +828,13 @@ class AttendedCliTests(OOCliFixture): 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_scheduleable')) + inventory.get('nodes', '10.0.0.1 openshift_schedulable')) self.assertEquals('False', - inventory.get('nodes', '10.0.0.2 openshift_scheduleable')) + 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_scheduleable')) - - return + inventory.get('nodes', '10.0.0.4 openshift_schedulable')) #interactive multimaster: equal number masters and nodes @patch('ooinstall.openshift_ansible.run_main_playbook') @@ -868,14 +864,12 @@ class AttendedCliTests(OOCliFixture): inventory = ConfigParser.ConfigParser(allow_no_value=True) inventory.read(os.path.join(self.work_dir, '.ansible/hosts')) - self.assertEquals(None, - inventory.get('nodes', '10.0.0.1')) - self.assertEquals(None, - inventory.get('nodes', '10.0.0.2')) - self.assertEquals(None, - inventory.get('nodes', '10.0.0.3')) - - return + 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 all-in-one @patch('ooinstall.openshift_ansible.run_main_playbook') @@ -902,10 +896,8 @@ class AttendedCliTests(OOCliFixture): inventory = ConfigParser.ConfigParser(allow_no_value=True) inventory.read(os.path.join(self.work_dir, '.ansible/hosts')) - self.assertEquals(None, - inventory.get('nodes', '10.0.0.1')) - - return + 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 -- cgit v1.2.3 From 139a82349040983a4af7ebd7e09e32a96bc00667 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 10:40:05 -0400 Subject: Block re-use of master/node as load balancer in attended install. Code was present to catch this in unattended installs but was looking for a host record with both master/node and master_lb set to true, but in the attended installs we were adding a separate host record with the same connect_to. Attended tests can now optionally specify multiple "attempted" strings for the master_lb specification, we'll try to input each if multiple are specified. Cleanup some empty defaults and error messages as well. --- utils/src/ooinstall/cli_installer.py | 35 +++++++++++++++++++++++---------- utils/src/ooinstall/oo_config.py | 4 ++-- utils/test/cli_installer_tests.py | 38 ++++++++++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 18 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index d7c06745e..a016a5597 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -106,8 +106,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen num_masters = 0 while more_hosts: host_props = {} - host_props['connect_to'] = click.prompt('Enter hostname or IP address:', - default='', + host_props['connect_to'] = click.prompt('Enter hostname or IP address', value_proc=validate_prompt_hostname) if not masters_set: @@ -144,13 +143,17 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen more_hosts = click.confirm('Do you want to add additional hosts?') if num_masters > 1: - hosts.append(collect_master_lb()) + collect_master_lb(hosts) return hosts -def collect_master_lb(): +def collect_master_lb(hosts): """ - Get an HA proxy from the user + Get a valid load balancer from the user and append it to the list of + hosts. + + Ensure user does not specify a system already used as a master/node as + this is an invalid configuration. """ message = """ Setting up High Availability Masters requires a load balancing solution. @@ -166,17 +169,28 @@ hostname. """ click.echo(message) host_props = {} - host_props['connect_to'] = click.prompt('Enter hostname or IP address:', - default='', - value_proc=validate_prompt_hostname) + + # Using an embedded function here so we have access to the hosts list: + def validate_prompt_lb(hostname): + # Run the standard hostname check first: + hostname = validate_prompt_hostname(hostname) + + # Make sure this host wasn't already specified: + for host in hosts: + if host.connect_to == hostname and (host.master or host.node): + raise click.BadParameter('Cannot re-use "%s" as a load balancer, ' + 'please specify a separate host' % hostname) + return hostname + + host_props['connect_to'] = click.prompt('Enter hostname or IP address', + value_proc=validate_prompt_lb) install_haproxy = click.confirm('Should the reference haproxy load balancer be installed on this host?') host_props['preconfigured'] = not install_haproxy host_props['master'] = False host_props['node'] = False host_props['master_lb'] = True master_lb = Host(**host_props) - - return master_lb + hosts.append(master_lb) def confirm_hosts_facts(oo_cfg, callback_facts): hosts = oo_cfg.hosts @@ -261,6 +275,7 @@ def check_hosts_config(oo_cfg): 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) + # Check for another host with same connect_to? else: message = """ No HAProxy given in config. Either specify one or provide a load balancing solution diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index b6f0cdce3..37aaf9197 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -50,8 +50,8 @@ class Host(object): self.containerized = kwargs.get('containerized', False) if self.connect_to is None: - raise OOConfigInvalidHostError("You must specify either and 'ip' " \ - "or 'hostname' to connect to.") + raise OOConfigInvalidHostError("You must specify either an ip " \ + "or hostname as 'connect_to'") if self.master is False and self.node is False and self.master_lb is False: raise OOConfigInvalidHostError( diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 8e9c2d698..2891cefcc 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -568,8 +568,9 @@ class UnattendedCliTests(OOCliFixture): self.cli_args.extend(["-c", config_file, "install"]) result = self.runner.invoke(cli.cli, self.cli_args) - assert result.exit_code == 1 - assert result.output == "You must specify either and 'ip' or 'hostname' to connect to.\n" + self.assertEquals(1, result.exit_code) + self.assertTrue("You must specify either an ip or hostname" + in result.output) #unattended with two masters, one node, and haproxy @patch('ooinstall.openshift_ansible.run_main_playbook') @@ -651,8 +652,12 @@ class AttendedCliTests(OOCliFixture): 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: - inputs.append(master_lb[0]) + if type(master_lb[0]) is list or type(master_lb[0]) is 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 @@ -801,7 +806,7 @@ class AttendedCliTests(OOCliFixture): #interactive multimaster: one more node than master @patch('ooinstall.openshift_ansible.run_main_playbook') @patch('ooinstall.openshift_ansible.load_system_facts') - def test_quick_ha1(self, load_facts_mock, run_playbook_mock): + 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 @@ -836,10 +841,10 @@ class AttendedCliTests(OOCliFixture): self.assertEquals('False', inventory.get('nodes', '10.0.0.4 openshift_schedulable')) - #interactive multimaster: equal number masters and nodes + #interactive multimaster: identical masters and nodes @patch('ooinstall.openshift_ansible.run_main_playbook') @patch('ooinstall.openshift_ansible.load_system_facts') - def test_quick_ha2(self, load_facts_mock, run_playbook_mock): + 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 @@ -871,6 +876,27 @@ class AttendedCliTests(OOCliFixture): 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 = self._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') -- cgit v1.2.3 From e6054d0ef7d07f370496dac82975b17dc3dfce70 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 11:55:32 -0400 Subject: Don't prompt to continue during unattended installs. --- utils/src/ooinstall/cli_installer.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index a016a5597..acfb5065b 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -263,7 +263,7 @@ Edit %s with the desired values and run `atomic-openshift-installer --unattended -def check_hosts_config(oo_cfg): +def check_hosts_config(oo_cfg, unattended): click.clear() masters = [host for host in oo_cfg.hosts if host.master] if len(masters) > 1: @@ -283,7 +283,8 @@ of your choice to balance the master API (port 8443) on all master hosts. https://docs.openshift.org/latest/install_config/install/advanced_install.html#multiple-masters """ - confirm_continue(message) + if not unattended: + confirm_continue(message) nodes = [host for host in oo_cfg.hosts if host.node] if len(masters) == len(nodes): @@ -292,7 +293,8 @@ No dedicated Nodes specified. By default, colocated Masters have their Nodes set to unscheduleable. Continuing at this point will label all nodes as scheduleable. """ - confirm_continue(message) + if not unattended: + confirm_continue(message) return @@ -654,7 +656,7 @@ def install(ctx, force): else: oo_cfg = get_missing_info_from_user(oo_cfg) - check_hosts_config(oo_cfg) + check_hosts_config(oo_cfg, ctx.obj['unattended']) click.echo('Gathering information from hosts...') callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, -- cgit v1.2.3 From 8a0153e992938a9b3441faca77305be1bb42f4d2 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 12:20:25 -0400 Subject: Test unattended HA quick install. Checking behavior when there is no LB specified, and when the user attempts to re-use a master or node as their LB. --- utils/src/ooinstall/cli_installer.py | 23 ++++--- utils/src/ooinstall/openshift_ansible.py | 2 +- utils/test/cli_installer_tests.py | 109 +++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 9 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index acfb5065b..82f695fca 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -278,22 +278,28 @@ def check_hosts_config(oo_cfg, unattended): # Check for another host with same connect_to? else: message = """ -No HAProxy given in config. Either specify one or provide a load balancing solution -of your choice to balance the master API (port 8443) on all master hosts. +WARNING: No master load balancer specified in config. If you proceed you will +need to provide a load balancing solution of your choice to balance the +API (port 8443) on all master hosts. https://docs.openshift.org/latest/install_config/install/advanced_install.html#multiple-masters """ - if not unattended: + if unattended: + click.echo(message) + else: confirm_continue(message) nodes = [host for host in oo_cfg.hosts if host.node] + # TODO: This looks a little unsafe, maybe look for dedicated nodes only: if len(masters) == len(nodes): message = """ -No dedicated Nodes specified. By default, colocated Masters have their Nodes -set to unscheduleable. Continuing at this point will label all nodes as -scheduleable. +WARNING: No dedicated Nodes specified. By default, colocated Masters have +their Nodes set to unscheduleable. If you proceed all nodes will be labelled +as schedulable. """ - if not unattended: + if unattended: + click.echo(message) + else: confirm_continue(message) return @@ -318,7 +324,8 @@ def get_variant_and_version(multi_master=False): return product, version def confirm_continue(message): - click.echo(message) + if message: + click.echo(message) click.confirm("Are you ready to continue?", default=False, abort=True) return diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 84e4db61d..866590c49 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -106,7 +106,7 @@ def write_inventory_vars(base_inventory, multiple_masters, proxy): base_inventory.write('ansible_ssh_user={}\n'.format(CFG.settings['ansible_ssh_user'])) if CFG.settings['ansible_ssh_user'] != 'root': base_inventory.write('ansible_become=true\n') - if multiple_masters: + if multiple_masters and proxy is not None: base_inventory.write('openshift_master_cluster_method=native\n') base_inventory.write("openshift_master_cluster_hostname={}\n".format(proxy.hostname)) base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 2891cefcc..b1ab7b283 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -102,6 +102,7 @@ hosts: node: true """ +# Missing connect_to on some hosts: BAD_CONFIG = """ variant: %s ansible_ssh_user: root @@ -158,6 +159,59 @@ hosts: master_lb: true """ +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 +""" + +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 +""" + class OOCliFixture(OOInstallFixture): def setUp(self): @@ -606,6 +660,61 @@ class UnattendedCliTests(OOCliFixture): self.assertEquals(4, len(hosts)) self.assertEquals(4, len(hosts_to_run_on)) + #unattended with two 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) + + # We consider this a valid outcome but lets make sure the warning + # was displayed: + self.assert_result(result, 0) + self.assertTrue('No master load balancer specified in config' in result.output) + + 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']) + # 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] + hosts_to_run_on = run_playbook_mock.call_args[0][1] + 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: + @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, 0) + + class AttendedCliTests(OOCliFixture): def setUp(self): -- cgit v1.2.3 From 5d2167cd565e97157f81996c872540bb6515d205 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 12:22:34 -0400 Subject: Trim assertions in HA testing. We're asserting the same things in loading facts over and over, which is not what these tests are really intended to catch. This behavior is tested elsewhere. --- utils/test/cli_installer_tests.py | 28 ---------------------------- 1 file changed, 28 deletions(-) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index b1ab7b283..d6cee3cc2 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -640,20 +640,6 @@ class UnattendedCliTests(OOCliFixture): result = self.runner.invoke(cli.cli, self.cli_args) self.assert_result(result, 0) - 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']) - # 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] hosts_to_run_on = run_playbook_mock.call_args[0][1] @@ -678,20 +664,6 @@ class UnattendedCliTests(OOCliFixture): self.assert_result(result, 0) self.assertTrue('No master load balancer specified in config' in result.output) - 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']) - # 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] hosts_to_run_on = run_playbook_mock.call_args[0][1] -- cgit v1.2.3 From 8305a07e547330397f7ce0bad6a38d0f9c6242ea Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 12:25:05 -0400 Subject: Pylint touchups. --- utils/test/cli_installer_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index d6cee3cc2..20dd57463 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -1,6 +1,6 @@ # TODO: Temporarily disabled due to importing old code into openshift-ansible # repo. We will work on these over time. -# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name +# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,too-many-lines import copy import os @@ -735,7 +735,7 @@ class AttendedCliTests(OOCliFixture): # You can pass a single master_lb or a list if you intend for one to get rejected: if master_lb: - if type(master_lb[0]) is list or type(master_lb[0]) is tuple: + if isinstance(master_lb[0], list) or isinstance(master_lb[0], tuple): inputs.extend(master_lb[0]) else: inputs.append(master_lb[0]) -- cgit v1.2.3 From 38c76bfacfa1c7b7ff09f0259143385306e9a778 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 26 Nov 2015 15:32:46 -0400 Subject: Breakout a test fixture to reduce module size. --- utils/test/cli_installer_tests.py | 240 +++----------------------------------- utils/test/fixture.py | 218 ++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 222 deletions(-) create mode 100644 utils/test/fixture.py (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 20dd57463..3cf704cd5 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -1,16 +1,14 @@ # TODO: Temporarily disabled due to importing old code into openshift-ansible # repo. We will work on these over time. -# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,too-many-lines +# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name 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 @@ -76,32 +74,6 @@ MOCK_FACTS_QUICKHA = { }, } -# 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 -""" - # Missing connect_to on some hosts: BAD_CONFIG = """ variant: %s @@ -212,107 +184,6 @@ hosts: node: 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): - self.assertEquals(host_count, len(written_config['hosts'])) - for h in written_config['hosts']: - self.assertTrue('hostname' in h) - self.assertTrue('public_hostname' in h) - if 'preconfigured' not in h: - self.assertTrue(h['node']) - self.assertTrue('ip' in h) - self.assertTrue('public_ip' 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): def setUp(self): @@ -491,7 +362,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, @@ -520,7 +391,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: @@ -695,88 +566,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,too-many-branches - def _build_input(self, ssh_user=None, hosts=None, variant_num=None, - add_nodes=None, confirm_facts=None, scheduleable_masters_ok=None, - master_lb=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 variant_num: - inputs.append(str(variant_num)) # Choose variant + version - - num_masters = 0 - if hosts: - i = 0 - min_masters_for_ha = 3 - 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') - 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 - 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 scheduleable_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) - @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)], @@ -791,7 +587,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(self.config_file) + written_config = read_yaml(self.config_file) self._verify_config_hosts(written_config, 3) inventory = ConfigParser.ConfigParser(allow_no_value=True) @@ -817,7 +613,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), ], @@ -834,7 +630,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') @@ -846,7 +642,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, @@ -857,7 +653,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 @@ -868,7 +664,7 @@ 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)], @@ -891,7 +687,7 @@ class AttendedCliTests(OOCliFixture): load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 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', True), ('10.0.0.3', False), @@ -908,7 +704,7 @@ class AttendedCliTests(OOCliFixture): self._verify_load_facts(load_facts_mock) self._verify_run_playbook(run_playbook_mock, 5, 5) - written_config = self._read_yaml(self.config_file) + written_config = read_yaml(self.config_file) self._verify_config_hosts(written_config, 5) inventory = ConfigParser.ConfigParser(allow_no_value=True) @@ -929,7 +725,7 @@ class AttendedCliTests(OOCliFixture): load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 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', True), ('10.0.0.3', True)], @@ -945,7 +741,7 @@ class AttendedCliTests(OOCliFixture): self._verify_load_facts(load_facts_mock) self._verify_run_playbook(run_playbook_mock, 4, 4) - written_config = self._read_yaml(self.config_file) + written_config = read_yaml(self.config_file) self._verify_config_hosts(written_config, 4) inventory = ConfigParser.ConfigParser(allow_no_value=True) @@ -964,7 +760,7 @@ class AttendedCliTests(OOCliFixture): load_facts_mock.return_value = (MOCK_FACTS_QUICKHA, 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', True), ('10.0.0.3', False), @@ -985,7 +781,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)], ssh_user='root', variant_num=1, @@ -998,7 +794,7 @@ class AttendedCliTests(OOCliFixture): self._verify_load_facts(load_facts_mock) self._verify_run_playbook(run_playbook_mock, 1, 1) - written_config = self._read_yaml(self.config_file) + written_config = read_yaml(self.config_file) self._verify_config_hosts(written_config, 1) inventory = ConfigParser.ConfigParser(allow_no_value=True) diff --git a/utils/test/fixture.py b/utils/test/fixture.py new file mode 100644 index 000000000..ab1c3f12e --- /dev/null +++ b/utils/test/fixture.py @@ -0,0 +1,218 @@ +# 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.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 _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, scheduleable_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 + min_masters_for_ha = 3 + 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') + 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 + 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 scheduleable_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) + -- cgit v1.2.3 From e838b2d7d56dcbe70263c61e1e3146854f9115ae Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 27 Nov 2015 10:08:08 -0400 Subject: Assert etcd section written for HA installs. --- utils/test/cli_installer_tests.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'utils') diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 3cf704cd5..a0fc95c33 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -718,6 +718,9 @@ class AttendedCliTests(OOCliFixture): self.assertEquals('False', inventory.get('nodes', '10.0.0.4 openshift_schedulable')) + 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') -- cgit v1.2.3 From d4f56d9e00596e0268bec8926bf1faeaffeefae7 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 27 Nov 2015 10:27:48 -0400 Subject: Text improvements for host specification. --- utils/src/ooinstall/cli_installer.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 82f695fca..ec9ccb565 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -81,19 +81,31 @@ def collect_hosts(version=None, masters_set=False, print_summary=True): """ min_masters_for_ha = 3 click.clear() - click.echo('***Host Configuration***') + click.echo('*** Host Configuration ***') message = """ -The OpenShift Master serves the API and web console. It also coordinates the -jobs that have to run across the environment. It can even run the datastore. -For wizard based installations the database will be embedded. It's possible to -change this later using etcd from Red Hat Enterprise Linux 7. +You must now specify the hosts that will compose your OpenShift cluster. + +Please enter an IP or hostname to connect to for each system in the cluster. +You will then be prompted to identify what role you would like this system to +serve in the cluster. + +OpenShift Masters serve the API and web console and coordinate the jobs to run +across the environment. If desired you can specify multiple Master systems for +an HA deployment, in which case you will be prompted to identify a *separate* +system to act as the load balancer for your cluster after all Masters and Nodes +are defined. + +If only one Master is specified, an etcd instance embedded within the OpenShift +Master service will be used as the datastore. This can be later replaced with a +separate etcd instance if desired. If multiple Masters are specified, a +separate etcd cluster will be configured with each Master serving as a member. Any Masters configured as part of this installation process will also be configured as Nodes. This is so that the Master will be able to proxy to Pods from the API. By default this Node will be unscheduleable but this can be changed after installation with 'oadm manage-node'. -The OpenShift Node provides the runtime environments for containers. It will +OpenShift Nodes provide the runtime environments for containers. They will host the required services to be managed by the Master. http://docs.openshift.com/enterprise/latest/architecture/infrastructure_components/kubernetes_infrastructure.html#master @@ -415,7 +427,7 @@ https://docs.openshift.com/enterprise/latest/admin_guide/install/prerequisites.h def collect_new_nodes(): click.clear() - click.echo('***New Node Configuration***') + click.echo('*** New Node Configuration ***') message = """ Add new nodes here """ -- cgit v1.2.3 From 23674ef59589b884d399c5c0c57f30f2baf5b89e Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 27 Nov 2015 10:38:23 -0400 Subject: Print a system summary after adding each. --- utils/src/ooinstall/cli_installer.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index ec9ccb565..56bb75187 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -14,6 +14,7 @@ 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: @@ -79,7 +80,6 @@ def collect_hosts(version=None, masters_set=False, print_summary=True): Returns: a list of host information collected from the user """ - min_masters_for_ha = 3 click.clear() click.echo('*** Host Configuration ***') message = """ @@ -126,7 +126,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 num_masters >= MIN_MASTERS_FOR_HA or version == '3.0': masters_set = True host_props['node'] = True @@ -145,13 +145,9 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts.append(host) if print_summary: - click.echo('') - click.echo('Current Masters: {}'.format(num_masters)) - click.echo('Current Nodes: {}'.format(len(hosts))) - click.echo('Additional Masters required for HA: {}'.format(max(min_masters_for_ha - num_masters, 0))) - click.echo('') + print_host_summary(hosts) - if num_masters <= 1 or num_masters >= min_masters_for_ha: + if num_masters <= 1 or num_masters >= MIN_MASTERS_FOR_HA: more_hosts = click.confirm('Do you want to add additional hosts?') if num_masters > 1: @@ -159,6 +155,22 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen return hosts + +def print_host_summary(hosts): + masters = [host for host in hosts if host.master] + nodes = [host for host in hosts if host.node] + click.echo('') + click.echo('Current Masters: %s' % len(masters)) + for host in masters: + click.echo(' %s' % host.connect_to) + click.echo('Current 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('') + + def collect_master_lb(hosts): """ Get a valid load balancer from the user and append it to the list of -- cgit v1.2.3 From 94ce712e8e0ebeec4b0c27c6b9b26f03a26a1fd5 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 27 Nov 2015 10:47:35 -0400 Subject: Improved output when re-running after editing config. --- utils/src/ooinstall/cli_installer.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 56bb75187..28b72a7dc 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -160,10 +160,10 @@ def print_host_summary(hosts): masters = [host for host in hosts if host.master] nodes = [host for host in hosts if host.node] click.echo('') - click.echo('Current Masters: %s' % len(masters)) + click.echo('OpenShift Masters: %s' % len(masters)) for host in masters: click.echo(' %s' % host.connect_to) - click.echo('Current Nodes: %s' % len(nodes)) + 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' % @@ -690,6 +690,7 @@ def install(ctx, force): check_hosts_config(oo_cfg, ctx.obj['unattended']) click.echo('Gathering information from hosts...') + print_host_summary(oo_cfg.hosts) callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, verbose) if error: @@ -714,8 +715,8 @@ def install(ctx, force): click.echo('Ready to run installation process.') message = """ -If changes are needed to the values recorded by the installer please update {}. -""".format(oo_cfg.config_path) +If changes are needed please edit the config file above and re-run. +""" if not ctx.obj['unattended']: confirm_continue(message) -- cgit v1.2.3 From 385ca96f5aaf8987820a5c7a25349ab7bedf9318 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 24 Nov 2015 17:35:43 -0500 Subject: Fixing 'unscheduleable' typo --- utils/src/ooinstall/cli_installer.py | 6 +++--- utils/src/ooinstall/openshift_ansible.py | 12 ++++++------ utils/test/cli_installer_tests.py | 14 +++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index d7c06745e..0b38f706c 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -90,7 +90,7 @@ change this later using etcd from Red Hat Enterprise Linux 7. Any Masters configured as part of this installation process will also be configured as Nodes. This is so that the Master will be able to proxy to Pods -from the API. By default this Node will be unscheduleable but this can be changed +from the API. By default this Node will be unschedulable but this can be changed after installation with 'oadm manage-node'. The OpenShift Node provides the runtime environments for containers. It will @@ -274,8 +274,8 @@ https://docs.openshift.org/latest/install_config/install/advanced_install.html#m if len(masters) == len(nodes): message = """ No dedicated Nodes specified. By default, colocated Masters have their Nodes -set to unscheduleable. Continuing at this point will label all nodes as -scheduleable. +set to unschedulable. Continuing at this point will label all nodes as +schedulable. """ confirm_continue(message) diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 9afc9a644..4aa60922d 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -67,10 +67,10 @@ def generate_inventory(hosts): for node in nodes: # TODO: Until the Master can run the SDN itself we have to configure the Masters # as Nodes too. - scheduleable = True + schedulable = True if node in masters: - scheduleable = False - write_host(node, base_inventory, scheduleable) + schedulable = False + write_host(node, base_inventory, schedulable) if not getattr(proxy, 'preconfigured', True): base_inventory.write('\n[lb]\n') @@ -112,7 +112,7 @@ def write_inventory_vars(base_inventory, multiple_masters, proxy): base_inventory.write("openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname)) -def write_host(host, inventory, scheduleable=True): +def write_host(host, inventory, schedulable=True): global CFG facts = '' @@ -126,8 +126,8 @@ def write_host(host, inventory, scheduleable=True): facts += ' openshift_public_hostname={}'.format(host.public_hostname) # TODO: For not write_host is handles both master and nodes. # Technically only nodes will ever need this. - if not scheduleable: - facts += ' openshift_scheduleable=False' + if not schedulable: + facts += ' openshift_schedulable=False' installer_host = socket.gethostname() if installer_host in [host.connect_to, host.hostname, host.public_hostname]: facts += ' ansible_connection=local' diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index c951b6580..90b6b15a3 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -616,7 +616,7 @@ class AttendedCliTests(OOCliFixture): #pylint: disable=too-many-arguments,too-many-branches def _build_input(self, ssh_user=None, hosts=None, variant_num=None, - add_nodes=None, confirm_facts=None, scheduleable_masters_ok=None, + add_nodes=None, confirm_facts=None, schedulable_masters_ok=None, master_lb=None): """ Builds a CLI input string with newline characters to simulate @@ -658,7 +658,7 @@ class AttendedCliTests(OOCliFixture): # TODO: support option 2, fresh install if add_nodes: - if scheduleable_masters_ok: + if schedulable_masters_ok: inputs.append('y') inputs.append('1') # Add more nodes i = 0 @@ -712,7 +712,7 @@ class AttendedCliTests(OOCliFixture): 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_scheduleable')) + inventory.get('nodes', '10.0.0.1 openshift_schedulable')) self.assertEquals(None, inventory.get('nodes', '10.0.0.2')) self.assertEquals(None, @@ -790,7 +790,7 @@ class AttendedCliTests(OOCliFixture): add_nodes=[('10.0.0.2', False)], ssh_user='root', variant_num=1, - scheduleable_masters_ok=True, + schedulable_masters_ok=True, confirm_facts='y') self._verify_get_hosts_to_run_on(mock_facts, load_facts_mock, @@ -830,13 +830,13 @@ class AttendedCliTests(OOCliFixture): 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_scheduleable')) + inventory.get('nodes', '10.0.0.1 openshift_schedulable')) self.assertEquals('False', - inventory.get('nodes', '10.0.0.2 openshift_scheduleable')) + 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_scheduleable')) + inventory.get('nodes', '10.0.0.4 openshift_schedulable')) return -- cgit v1.2.3 From 4872c5e1d298cba9901e2ecf207580b7d4fdece7 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Fri, 27 Nov 2015 11:31:35 -0400 Subject: Adjust requirement for 3 masters for HA deployments. If only 2 masters are specified, consider this a configuration error if running an unattended install, and prevent it completely if running an attended install. (continues to prompt for hosts until you have at least 3) Because this condition cannot be entered in the interactive install, we can't really write a test for this negative case. --- utils/src/ooinstall/cli_installer.py | 41 ++++++++++++++---- utils/test/cli_installer_tests.py | 80 +++++++++++++++++++++++++++++++----- utils/test/fixture.py | 17 ++++---- 3 files changed, 112 insertions(+), 26 deletions(-) (limited to 'utils') 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: -- cgit v1.2.3 From cdd9aa7543f869aab5914fb816a66ed0debb0930 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 30 Nov 2015 15:20:54 -0400 Subject: Error out if no load balancer specified. --- utils/src/ooinstall/cli_installer.py | 24 ++++++++++-------------- utils/test/cli_installer_tests.py | 11 ++--------- 2 files changed, 12 insertions(+), 23 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 9a447406f..249a8a7d9 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -201,11 +201,11 @@ def collect_master_lb(hosts): """ message = """ Setting up High Availability Masters requires a load balancing solution. -Please provide a host that will be configured as a proxy. This can either be -an existing load balancer configured to balance all masters on port 8443 or a -new host that will have HAProxy installed on it. +Please provide a the FQDN of a host that will be configured as a proxy. This +can be either an existing load balancer configured to balance all masters on +port 8443 or a new host that will have HAProxy installed on it. -If the host provided does is not yet configured a reference haproxy load +If the host provided does is not yet configured, a reference haproxy load balancer will be installed. It's important to note that while the rest of the environment will be fault tolerant this reference load balancer will not be. It can be replaced post-installation with a load balancer with the same @@ -318,25 +318,21 @@ def check_hosts_config(oo_cfg, unattended): 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.') + click.echo('ERROR: More than one Master load balancer specified. Only one is allowed.') 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.') + click.echo('ERROR: The Master load balancer is configured as a master or node. Please correct this.') sys.exit(1) - # Check for another host with same connect_to? else: message = """ -WARNING: No master load balancer specified in config. If you proceed you will -need to provide a load balancing solution of your choice to balance the -API (port 8443) on all master hosts. +ERROR: No master load balancer specified in config. You must provide the FQDN +of a load balancer to balance the API (port 8443) on all Master hosts. https://docs.openshift.org/latest/install_config/install/advanced_install.html#multiple-masters """ - if unattended: - click.echo(message) - else: - confirm_continue(message) + click.echo(message) + sys.exit(1) nodes = [host for host in oo_cfg.hosts if host.node] # TODO: This looks a little unsafe, maybe look for dedicated nodes only: diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index c12b4d564..ad76cc3e9 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -588,17 +588,10 @@ class UnattendedCliTests(OOCliFixture): self.cli_args.extend(["-c", config_file, "install"]) result = self.runner.invoke(cli.cli, self.cli_args) - # We consider this a valid outcome but lets make sure the warning - # was displayed: - self.assert_result(result, 0) + # This is not a valid input: + self.assert_result(result, 1) self.assertTrue('No master load balancer specified in config' in result.output) - # 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(3, len(hosts)) - self.assertEquals(3, len(hosts_to_run_on)) - #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') -- cgit v1.2.3 From 4f6794ea6b3b7eb7383a74cb35e8ce6d445675c8 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 30 Nov 2015 15:30:02 -0400 Subject: Suggest dedicated nodes for an HA deployment. --- utils/src/ooinstall/cli_installer.py | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 249a8a7d9..8f9d82f43 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -188,6 +188,15 @@ finished entering hosts. """ click.echo(ha_message) + dedicated_nodes_message = """ +WARNING: Dedicated Nodes are recommended for an HA deployment. If no dedicated +Nodes are specified, each configured Master will be marked as a schedulable +Node. +""" + dedicated_nodes = [host for host in hosts if host.node and not host.master] + if len(dedicated_nodes) == 0: + click.echo(dedicated_nodes_message) + click.echo('') -- cgit v1.2.3 From e796856c804cd7090ed45aba7838436e7fdc9580 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Mon, 30 Nov 2015 15:56:12 -0400 Subject: Fix bug when warning on no dedicated nodes. --- utils/src/ooinstall/cli_installer.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 8f9d82f43..f8dfe2feb 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -343,9 +343,8 @@ https://docs.openshift.org/latest/install_config/install/advanced_install.html#m click.echo(message) sys.exit(1) - nodes = [host for host in oo_cfg.hosts if host.node] - # TODO: This looks a little unsafe, maybe look for dedicated nodes only: - if len(masters) == len(nodes): + dedicated_nodes = [host for host in oo_cfg.hosts if host.node and not host.master] + if len(dedicated_nodes) == 0: message = """ WARNING: No dedicated Nodes specified. By default, colocated Masters have their Nodes set to unscheduleable. If you proceed all nodes will be labelled -- cgit v1.2.3 From 098833f05200258259e2fdb59084c07a2472fac0 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Tue, 1 Dec 2015 09:43:52 -0400 Subject: Cleanup more schedulable typos. --- utils/src/ooinstall/openshift_ansible.py | 2 +- utils/test/fixture.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index c5257f1db..e36116cc9 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -58,7 +58,7 @@ def generate_inventory(hosts): base_inventory.write('\n[nodes]\n') - # TODO: It would be much better to calculate the scheduleability elsewhere + # TODO: It would be much better to calculate the schedulability elsewhere # and store it on the Node object. if set(nodes) == set(masters): for node in nodes: diff --git a/utils/test/fixture.py b/utils/test/fixture.py index 14baec6b3..90bd9e1ef 100644 --- a/utils/test/fixture.py +++ b/utils/test/fixture.py @@ -140,7 +140,7 @@ class OOCliFixture(OOInstallFixture): #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, scheduleable_masters_ok=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 @@ -192,7 +192,7 @@ def build_input(ssh_user=None, hosts=None, variant_num=None, # TODO: support option 2, fresh install if add_nodes: - if scheduleable_masters_ok: + if schedulable_masters_ok: inputs.append('y') inputs.append('1') # Add more nodes i = 0 -- cgit v1.2.3 From 1b770dff59abc23e93eecfb76c4b10dc9e2dafe2 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Tue, 1 Dec 2015 10:06:48 -0400 Subject: Add warning for HA deployments with < 3 dedicated nodes. --- utils/src/ooinstall/cli_installer.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index ee962c21a..31e8cb147 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -172,30 +172,34 @@ def print_host_summary(hosts): click.echo("") if len(masters) == 1: - click.echo("NOTE: Add a total of 3 or more Masters to perform an HA" - " installation.") + ha_hint_message = """ +NOTE: Add a total of 3 or more Masters to perform an HA installation.""" + click.echo(ha_hint_message) 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. -""" +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. -""" +finished entering hosts.""" click.echo(ha_message) dedicated_nodes_message = """ WARNING: Dedicated Nodes are recommended for an HA deployment. If no dedicated Nodes are specified, each configured Master will be marked as a schedulable -Node. -""" +Node.""" + + min_ha_nodes_message = """ +WARNING: A minimum of 3 dedicated Nodes are recommended for an HA +deployment.""" dedicated_nodes = [host for host in hosts if host.node and not host.master] if len(dedicated_nodes) == 0: click.echo(dedicated_nodes_message) + elif len(dedicated_nodes) < 3: + click.echo(min_ha_nodes_message) click.echo('') -- cgit v1.2.3 From d7ff5b10a3de3f7966148d9e08c0468ef3d6a7f0 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Tue, 1 Dec 2015 10:41:43 -0400 Subject: Improved installation summary. Displays each host and the roles it will play based on the current configuration. As the configuration grows the summary will adapt to indicate embedded vs separte etcd, scheduled vs unscheduled nodes, etc. --- utils/src/ooinstall/cli_installer.py | 57 ++++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 12 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 31e8cb147..dbe3f6c32 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -144,7 +144,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen hosts.append(host) if print_summary: - print_host_summary(hosts) + print_installation_summary(hosts) # 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 @@ -158,18 +158,26 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen return hosts -def print_host_summary(hosts): +def print_installation_summary(hosts): + """ + Displays a summary of all hosts configured thus far, and what role each + will play. + + Shows total nodes/masters, hints for performing/modifying the deployment + with additional setup, warnings for invalid or sub-optimal configurations. + """ + click.clear() + click.echo('*** Installation Summary ***\n') + click.echo('Hosts:') + for host in hosts: + print_host_summary(hosts, host) + masters = [host for host in hosts if host.master] nodes = [host for host in hosts if host.node] + dedicated_nodes = [host for host in hosts if host.node and not host.master] click.echo('') - click.echo('OpenShift Masters: %s' % len(masters)) - for host in masters: - click.echo(' %s' % host.connect_to) - click.echo('OpenShift Nodes: %s' % len(nodes)) - for host in nodes: - click.echo(' %s' % host.connect_to) - - click.echo("") + click.echo('Total OpenShift Masters: %s' % len(masters)) + click.echo('Total OpenShift Nodes: %s' % len(nodes)) if len(masters) == 1: ha_hint_message = """ @@ -195,7 +203,6 @@ Node.""" min_ha_nodes_message = """ WARNING: A minimum of 3 dedicated Nodes are recommended for an HA deployment.""" - dedicated_nodes = [host for host in hosts if host.node and not host.master] if len(dedicated_nodes) == 0: click.echo(dedicated_nodes_message) elif len(dedicated_nodes) < 3: @@ -204,6 +211,32 @@ deployment.""" click.echo('') +def print_host_summary(all_hosts, host): + description_tokens = [] + masters = [ahost for ahost in all_hosts if ahost.master] + nodes = [ahost for ahost in all_hosts if ahost.node] + click.echo("- %s" % host.connect_to) + if host.master: + click.echo(" - OpenShift Master") + if host.node: + if not host.master: + click.echo(" - OpenShift Node (Dedicated)") + elif host.master and len(masters) == len(nodes): + click.echo(" - OpenShift Node") + else: + click.echo(" - OpenShift Node (Unscheduled)") + if host.master_lb: + if host.preconfigured: + click.echo(" - Load Balancer (Preconfigured)") + else: + click.echo(" - Load Balancer (HAProxy)") + if host.master: + if len(masters) > 1: + click.echo(" - Etcd Member") + else: + click.echo(" - Etcd (Embedded)") + + def collect_master_lb(hosts): """ Get a valid load balancer from the user and append it to the list of @@ -723,7 +756,7 @@ def install(ctx, force): check_hosts_config(oo_cfg, ctx.obj['unattended']) click.echo('Gathering information from hosts...') - print_host_summary(oo_cfg.hosts) + print_installation_summary(oo_cfg.hosts) callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, verbose) if error: -- cgit v1.2.3 From bd43109412c1477fde8152db7e84d73c857d544f Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Tue, 1 Dec 2015 15:09:10 -0400 Subject: Centralize etcd/schedulability logic for each host. --- utils/src/ooinstall/cli_installer.py | 9 +++------ utils/src/ooinstall/oo_config.py | 26 ++++++++++++++++++++++++++ utils/src/ooinstall/openshift_ansible.py | 21 ++++++++------------- 3 files changed, 37 insertions(+), 19 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index dbe3f6c32..8cabe5431 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -212,16 +212,13 @@ deployment.""" def print_host_summary(all_hosts, host): - description_tokens = [] - masters = [ahost for ahost in all_hosts if ahost.master] - nodes = [ahost for ahost in all_hosts if ahost.node] click.echo("- %s" % host.connect_to) if host.master: click.echo(" - OpenShift Master") if host.node: - if not host.master: + if host.is_dedicated_node(): click.echo(" - OpenShift Node (Dedicated)") - elif host.master and len(masters) == len(nodes): + elif host.is_schedulable_node(all_hosts): click.echo(" - OpenShift Node") else: click.echo(" - OpenShift Node (Unscheduled)") @@ -231,7 +228,7 @@ def print_host_summary(all_hosts, host): else: click.echo(" - Load Balancer (HAProxy)") if host.master: - if len(masters) > 1: + if host.is_etcd_member(all_hosts): click.echo(" - Etcd Member") else: click.echo(" - Etcd (Embedded)") diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index 37aaf9197..1be85bc1d 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -73,6 +73,32 @@ class Host(object): d[prop] = getattr(self, prop) return d + def is_etcd_member(self, all_hosts): + """ Will this host be a member of a standalone etcd cluster. """ + if not self.master: + return False + masters = [host for host in all_hosts if host.master] + if len(masters) > 1: + return True + return False + + def is_dedicated_node(self): + """ Will this host be a dedicated node. (not a master) """ + return self.node and not self.master + + def is_schedulable_node(self, all_hosts): + """ Will this host be a node marked as schedulable. """ + if not self.node: + return False + if not self.master: + return True + + masters = [host for host in all_hosts if host.master] + nodes = [host for host in all_hosts if host.node] + if len(masters) == len(nodes): + return True + return False + class OOConfig(object): default_dir = os.path.normpath( diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index e36116cc9..17196a813 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -58,19 +58,14 @@ def generate_inventory(hosts): base_inventory.write('\n[nodes]\n') - # TODO: It would be much better to calculate the schedulability elsewhere - # and store it on the Node object. - if set(nodes) == set(masters): - for node in nodes: - write_host(node, base_inventory, True) - else: - for node in nodes: - # TODO: Until the Master can run the SDN itself we have to configure the Masters - # as Nodes too. - schedulable = None - if node in masters: - schedulable = False - write_host(node, base_inventory, schedulable) + for node in nodes: + # Let the fact defaults decide if we're not a master: + schedulable = None + + # If the node is also a master, we must explicitly set schedulablity: + if node.master: + schedulable = node.is_schedulable_node(hosts) + write_host(node, base_inventory, schedulable) if not getattr(proxy, 'preconfigured', True): base_inventory.write('\n[lb]\n') -- cgit v1.2.3 From af009a7a51d7b6f5799a14c452cc7db92727135e Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Wed, 18 Nov 2015 16:19:19 -0600 Subject: Fedora changes: - ansible bootstrap playbook for Fedora 23+ - add conditionals to handle yum vs dnf - add Fedora OpenShift COPR - update BYO host README for repo configs and fedora bootstrap Fix typo in etcd README, remove unnecessary parens in openshift_node main.yml rebase on master, update package cache refresh handler for yum vs dnf Fix typo in etcd README, remove unnecessary parens in openshift_node main.yml --- utils/site_assets/oo-install-bootstrap.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/site_assets/oo-install-bootstrap.sh b/utils/site_assets/oo-install-bootstrap.sh index e1b2cec90..3847c029a 100755 --- a/utils/site_assets/oo-install-bootstrap.sh +++ b/utils/site_assets/oo-install-bootstrap.sh @@ -9,6 +9,13 @@ cmdlnargs="$@" : ${OO_INSTALL_LOG:=${TMPDIR}/INSTALLPKGNAME.log} [[ $TMPDIR != */ ]] && TMPDIR="${TMPDIR}/" +if rpm -q dnf; +then + PKG_MGR="dnf" +else + PKG_MGR="yum" +fi + if [ $OO_INSTALL_CONTEXT != 'origin_vm' ] then clear @@ -18,7 +25,7 @@ if [ -e /etc/redhat-release ] then for i in python python-virtualenv openssh-clients gcc do - rpm -q $i >/dev/null 2>&1 || { echo >&2 "Missing installation dependency detected. Please run \"yum install ${i}\"."; exit 1; } + rpm -q $i >/dev/null 2>&1 || { echo >&2 "Missing installation dependency detected. Please run \"${PKG_MGR} install ${i}\"."; exit 1; } done fi for i in python virtualenv ssh gcc -- cgit v1.2.3 From 62ab67626448edfbf70fd862de0324a8c7252a13 Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 8 Dec 2015 10:12:32 -0500 Subject: Bug 1287977 - Incorrect check output from atomic-openshift-installer when working with preconfigured load balancer --- utils/src/ooinstall/oo_config.py | 10 +++++-- utils/test/cli_installer_tests.py | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index 1be85bc1d..031b82bc1 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -14,7 +14,8 @@ PERSIST_SETTINGS = [ 'variant_version', 'version', ] -REQUIRED_FACTS = ['ip', 'public_ip', 'hostname', 'public_hostname'] +DEFAULT_REQUIRED_FACTS = ['ip', 'public_ip', 'hostname', 'public_hostname'] +PRECONFIGURED_REQUIRED_FACTS = ['hostname', 'public_hostname'] class OOConfigFileError(Exception): @@ -208,7 +209,12 @@ class OOConfig(object): for host in self.hosts: missing_facts = [] - for required_fact in REQUIRED_FACTS: + if host.preconfigured: + required_facts = PRECONFIGURED_REQUIRED_FACTS + else: + required_facts = DEFAULT_REQUIRED_FACTS + + for required_fact in required_facts: if not getattr(host, required_fact): missing_facts.append(required_fact) if len(missing_facts) > 0: diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index d028bf472..1da49c807 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -225,6 +225,44 @@ hosts: master: true """ +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): def setUp(self): @@ -608,6 +646,25 @@ class UnattendedCliTests(OOCliFixture): # 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): -- cgit v1.2.3 From 0056156b4b31d10d15034536b280564be38ec04b Mon Sep 17 00:00:00 2001 From: Brenton Leanhardt Date: Tue, 8 Dec 2015 10:32:06 -0500 Subject: Improving output when gathering facts --- utils/src/ooinstall/cli_installer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'utils') diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index 8cabe5431..dc88cb1ad 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -752,8 +752,8 @@ def install(ctx, force): check_hosts_config(oo_cfg, ctx.obj['unattended']) - click.echo('Gathering information from hosts...') print_installation_summary(oo_cfg.hosts) + click.echo('Gathering information from hosts...') callback_facts, error = openshift_ansible.default_facts(oo_cfg.hosts, verbose) if error: -- cgit v1.2.3 From b69a8724b38583a808f99c73cfed2d635353bf4c Mon Sep 17 00:00:00 2001 From: Samuel Munilla Date: Tue, 8 Dec 2015 14:56:37 -0500 Subject: atomic-openshift-installer: Error handling on yaml loading This addresses the stack trace that has been plaguing recent demos. In the case of an error with callback_facts.yaml the program output is much clearer and a course of action is suggested. --- utils/src/ooinstall/openshift_ansible.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'utils') diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py index 17196a813..fd2cd7fbd 100644 --- a/utils/src/ooinstall/openshift_ansible.py +++ b/utils/src/ooinstall/openshift_ansible.py @@ -157,9 +157,15 @@ def load_system_facts(inventory_file, os_facts_path, env_vars, verbose=False): status = subprocess.call(args, env=env_vars, stdout=FNULL) if not status == 0: return [], 1 - callback_facts_file = open(CFG.settings['ansible_callback_facts_yaml'], 'r') - callback_facts = yaml.load(callback_facts_file) - callback_facts_file.close() + + with open(CFG.settings['ansible_callback_facts_yaml'], 'r') as callback_facts_file: + try: + callback_facts = yaml.safe_load(callback_facts_file) + except yaml.YAMLError, exc: + print "Error in {}".format(CFG.settings['ansible_callback_facts_yaml']), exc + print "Try deleting and rerunning the atomic-openshift-installer" + sys.exit(1) + return callback_facts, 0 -- cgit v1.2.3