From 8399965ed095ff7518adbec1ddabdd86bf411223 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 19 Apr 2016 17:58:43 -0400 Subject: [PATCH] Check for two IP addresses assigned to same host When parsing the openstack_user_config.yml file, the code would not account for user mistakes when multiple IP addresses were specified for the same hostname. When multiple IP addresses were specified, the last one parsed would be written to the inventory. This change instead throws a runtime error when this situation arises, so that the invalid config cannot be written. The tox.ini configuration is modified to make sure that the insert order on the configuration dictionary is the same on every run of the tests. Were this not set, the insertion order may well change dependon on the hash seed, which would cause test failures because the assertions would not match. An OrderedDict is also used to ensure platform differences don't affect testing order. The behavior of this class shouldn't differ from normal dictionaries in a way that invalidates the test cases. Change-Id: I7c724b1dd668a8372bf2dafaf3461e0a3cb1a557 --- playbooks/inventory/dynamic_inventory.py | 46 +++++++++++++++- ...ultiple-ips-for-host-f27cb1f1e878640d.yaml | 4 ++ tests/test_inventory.py | 54 +++++++++++++++++++ tox.ini | 4 ++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/multiple-ips-for-host-f27cb1f1e878640d.yaml diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 54372221cb..92e34d3b64 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -68,6 +68,24 @@ class MultipleHostsWithOneIPError(Exception): return self.message +class MultipleIpForHostError(Exception): + def __init__(self, hostname, current_ip, new_ip): + self.hostname = hostname + self.current_ip = current_ip + self.new_ip = new_ip + + # Sort the IPs for our error message so we're always consistent. + ips = [current_ip, new_ip] + ips.sort() + + error_msg = "Host {hostname} has both {ips[0]} and {ips[1]} assigned" + + self.message = error_msg.format(hostname=hostname, ips=ips) + + def __str__(self): + return self.message + + def args(): """Setup argument Parsing.""" parser = argparse.ArgumentParser( @@ -389,8 +407,8 @@ def user_defined_setup(config, inventory): return for _key, _value in value.iteritems(): - if _key not in inventory['_meta']['hostvars']: - inventory['_meta']['hostvars'][_key] = {} + if _key not in hvs: + hvs[_key] = {} hvs[_key].update({ 'ansible_ssh_host': _value['ip'], @@ -894,6 +912,28 @@ def _check_same_ip_to_multiple_host(config): raise MultipleHostsWithOneIPError(*info) +def _check_multiple_ips_to_host(config): + """Check for multiple IPs assigned to a single hostname + + :param: config: ``dict`` User provided configuration + """ + + # Extract only the dictionaries in the host groups. + host_ip_map = {} + for groupnames, group in config.items(): + if '_hosts' in groupnames: + for hostname, entries in group.items(): + if hostname not in host_ip_map: + host_ip_map[hostname] = entries['ip'] + else: + current_ip = host_ip_map[hostname] + new_ip = entries['ip'] + if not current_ip == new_ip: + raise MultipleIpForHostError(hostname, current_ip, + new_ip) + return True + + def _check_config_settings(cidr_networks, config, container_skel): """check preciseness of config settings @@ -938,6 +978,8 @@ def _check_config_settings(cidr_networks, config, container_skel): # look for same ip address assigned to different hosts _check_same_ip_to_multiple_host(config) + _check_multiple_ips_to_host(config) + def load_environment(config_path): """Create an environment dictionary from config files diff --git a/releasenotes/notes/multiple-ips-for-host-f27cb1f1e878640d.yaml b/releasenotes/notes/multiple-ips-for-host-f27cb1f1e878640d.yaml new file mode 100644 index 0000000000..934b87d798 --- /dev/null +++ b/releasenotes/notes/multiple-ips-for-host-f27cb1f1e878640d.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Assigning multiple IP addresses to the same host name will now result in + an inventory error before running any playbooks. diff --git a/tests/test_inventory.py b/tests/test_inventory.py index 51f7fbf0c7..7fecb0d81c 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -390,6 +390,11 @@ class TestConfigChecks(unittest.TestCase): user_defined_config[group][new_hostname] = old_hostname_settings self.write_config() + def set_new_ip(self, user_defined_config, group, hostname, ip): + # Sets an IP address for a specified host. + user_defined_config[group][hostname]['ip'] = ip + self.write_config() + def test_provider_networks_check(self): # create config file without provider networks self.delete_config_key(self.user_defined_config, 'provider_networks') @@ -440,6 +445,55 @@ class TestConfigChecks(unittest.TestCase): "assign same ip to both hosts") self.assertEqual(context.exception.message, expectedLog) + def test_one_host_two_ips_externally(self): + # haproxy chosen because it was last in the config file as of + # writing + self.set_new_ip(self.user_defined_config, 'haproxy_hosts', 'aio1', + '172.29.236.101') + with self.assertRaises(di.MultipleIpForHostError) as context: + get_inventory() + expectedLog = ("Host aio1 has both 172.29.236.100 and 172.29.236.101 " + "assigned") + self.assertEqual(context.exception.message, expectedLog) + + def test_two_ips(self): + # Use an OrderedDict to be certain our testing order is preserved + # Even with the same hash seed, different OSes get different results, + # eg. local OS X vs gate's Linux + config = collections.OrderedDict() + config['infra_hosts'] = { + 'host1': { + 'ip': '192.168.1.1' + } + } + config['compute_hosts'] = { + 'host1': { + 'ip': '192.168.1.2' + } + } + + with self.assertRaises(di.MultipleIpForHostError) as context: + di._check_multiple_ips_to_host(config) + self.assertEqual(context.exception.current_ip, '192.168.1.1') + self.assertEqual(context.exception.new_ip, '192.168.1.2') + self.assertEqual(context.exception.hostname, 'host1') + + def test_correct_hostname_ip_map(self): + config = { + 'infra_hosts': { + 'host1': { + 'ip': '192.168.1.1' + } + }, + 'compute_hosts': { + 'host2': { + 'ip': '192.168.1.2' + } + }, + } + ret = di._check_multiple_ips_to_host(config) + self.assertTrue(ret) + def tearDown(self): if self.config_changed: # get back our initial user config file diff --git a/tox.ini b/tox.ini index 404f02b5c0..82907576ab 100644 --- a/tox.ini +++ b/tox.ini @@ -90,6 +90,10 @@ commands = {toxinidir}/playbooks/*.yml" [testenv:inventory] +# Use a fixed seed since some inventory tests rely on specific ordering +setenv = + {[testenv]setenv} + PYTHONHASHSEED = 100 commands = coverage erase coverage run {toxinidir}/tests/test_inventory.py