From a5a88e41af40578dc0ab18810ac0c02646c29904 Mon Sep 17 00:00:00 2001 From: Vladimir Masarik Date: Fri, 10 Sep 2021 21:21:22 +0200 Subject: [PATCH] Fix: adding new ips with inventory builder (#7577) (#7583) * Fix: adding new ips with inventory builder (#7577) * moved conflig loading logic to after checking whether the config should be loaded, and added check for whether the config should be loaded * added check for removing nodes from config if the user wants to remove a node, we need to load the config * Fix tox errors --- contrib/inventory_builder/inventory.py | 75 +++-- .../inventory_builder/tests/test_inventory.py | 276 ++++++++++++++++-- 2 files changed, 306 insertions(+), 45 deletions(-) diff --git a/contrib/inventory_builder/inventory.py b/contrib/inventory_builder/inventory.py index 042de28e6..106f9eeea 100644 --- a/contrib/inventory_builder/inventory.py +++ b/contrib/inventory_builder/inventory.py @@ -48,7 +48,7 @@ ROLES = ['all', 'kube_control_plane', 'kube_node', 'etcd', 'k8s_cluster', 'calico_rr'] PROTECTED_NAMES = ROLES AVAILABLE_COMMANDS = ['help', 'print_cfg', 'print_ips', 'print_hostnames', - 'load'] + 'load', 'add'] _boolean_states = {'1': True, 'yes': True, 'true': True, 'on': True, '0': False, 'no': False, 'false': False, 'off': False} yaml = YAML() @@ -82,22 +82,35 @@ class KubesprayInventory(object): def __init__(self, changed_hosts=None, config_file=None): self.config_file = config_file self.yaml_config = {} - if self.config_file: + loadPreviousConfig = False + # See whether there are any commands to process + if changed_hosts and changed_hosts[0] in AVAILABLE_COMMANDS: + if changed_hosts[0] == "add": + loadPreviousConfig = True + changed_hosts = changed_hosts[1:] + else: + self.parse_command(changed_hosts[0], changed_hosts[1:]) + sys.exit(0) + + # If the user wants to remove a node, we need to load the config anyway + if changed_hosts and changed_hosts[0][0] == "-": + loadPreviousConfig = True + + if self.config_file and loadPreviousConfig: # Load previous YAML file try: self.hosts_file = open(config_file, 'r') - self.yaml_config = yaml.load_all(self.hosts_file) - except OSError: - pass - - if changed_hosts and changed_hosts[0] in AVAILABLE_COMMANDS: - self.parse_command(changed_hosts[0], changed_hosts[1:]) - sys.exit(0) + self.yaml_config = yaml.load(self.hosts_file) + except OSError as e: + # I am assuming we are catching "cannot open file" exceptions + print(e) + sys.exit(1) self.ensure_required_groups(ROLES) if changed_hosts: changed_hosts = self.range2ips(changed_hosts) - self.hosts = self.build_hostnames(changed_hosts) + self.hosts = self.build_hostnames(changed_hosts, + loadPreviousConfig) self.purge_invalid_hosts(self.hosts.keys(), PROTECTED_NAMES) self.set_all(self.hosts) self.set_k8s_cluster() @@ -158,17 +171,29 @@ class KubesprayInventory(object): except IndexError: raise ValueError("Host name must end in an integer") - def build_hostnames(self, changed_hosts): + # Keeps already specified hosts, + # and adds or removes the hosts provided as an argument + def build_hostnames(self, changed_hosts, loadPreviousConfig=False): existing_hosts = OrderedDict() highest_host_id = 0 - try: - for host in self.yaml_config['all']['hosts']: - existing_hosts[host] = self.yaml_config['all']['hosts'][host] - host_id = self.get_host_id(host) - if host_id > highest_host_id: - highest_host_id = host_id - except Exception: - pass + # Load already existing hosts from the YAML + if loadPreviousConfig: + try: + for host in self.yaml_config['all']['hosts']: + # Read configuration of an existing host + hostConfig = self.yaml_config['all']['hosts'][host] + existing_hosts[host] = hostConfig + # If the existing host seems + # to have been created automatically, detect its ID + if host.startswith(HOST_PREFIX): + host_id = self.get_host_id(host) + if host_id > highest_host_id: + highest_host_id = host_id + except Exception as e: + # I am assuming we are catching automatically + # created hosts without IDs + print(e) + sys.exit(1) # FIXME(mattymo): Fix condition where delete then add reuses highest id next_host_id = highest_host_id + 1 @@ -176,6 +201,7 @@ class KubesprayInventory(object): all_hosts = existing_hosts.copy() for host in changed_hosts: + # Delete the host from config the hostname/IP has a "-" prefix if host[0] == "-": realhost = host[1:] if self.exists_hostname(all_hosts, realhost): @@ -184,6 +210,8 @@ class KubesprayInventory(object): elif self.exists_ip(all_hosts, realhost): self.debug("Marked {0} for deletion.".format(realhost)) self.delete_host_by_ip(all_hosts, realhost) + # Host/Argument starts with a digit, + # then we assume its an IP address elif host[0].isdigit(): if ',' in host: ip, access_ip = host.split(',') @@ -203,11 +231,15 @@ class KubesprayInventory(object): next_host = subprocess.check_output(cmd, shell=True) next_host = next_host.strip().decode('ascii') else: + # Generates a hostname because we have only an IP address next_host = "{0}{1}".format(HOST_PREFIX, next_host_id) next_host_id += 1 + # Uses automatically generated node name + # in case we dont provide it. all_hosts[next_host] = {'ansible_host': access_ip, 'ip': ip, 'access_ip': access_ip} + # Host/Argument starts with a letter, then we assume its a hostname elif host[0].isalpha(): if ',' in host: try: @@ -226,6 +258,7 @@ class KubesprayInventory(object): 'access_ip': access_ip} return all_hosts + # Expand IP ranges into individual addresses def range2ips(self, hosts): reworked_hosts = [] @@ -394,9 +427,11 @@ help - Display this message print_cfg - Write inventory file to stdout print_ips - Write a space-delimited list of IPs from "all" group print_hostnames - Write a space-delimited list of Hostnames from "all" group +add - Adds specified hosts into an already existing inventory Advanced usage: -Add another host after initial creation: inventory.py 10.10.1.5 +Create new or overwrite old inventory file: inventory.py 10.10.1.5 +Add another host after initial creation: inventory.py add 10.10.1.6 Add range of hosts: inventory.py 10.10.1.3-10.10.1.5 Add hosts with different ip and access ip: inventory.py 10.0.0.1,192.168.10.1 10.0.0.2,192.168.10.2 10.0.0.3,192.168.10.3 Add hosts with a specific hostname, ip, and optional access ip: first,10.0.0.1,192.168.10.1 second,10.0.0.2 last,10.0.0.3 diff --git a/contrib/inventory_builder/tests/test_inventory.py b/contrib/inventory_builder/tests/test_inventory.py index f9aa40bc1..4596f3396 100644 --- a/contrib/inventory_builder/tests/test_inventory.py +++ b/contrib/inventory_builder/tests/test_inventory.py @@ -67,23 +67,14 @@ class TestInventory(unittest.TestCase): self.assertRaisesRegex(ValueError, "Host name must end in an", self.inv.get_host_id, hostname) - def test_build_hostnames_add_one(self): - changed_hosts = ['10.90.0.2'] - expected = OrderedDict([('node1', - {'ansible_host': '10.90.0.2', - 'ip': '10.90.0.2', - 'access_ip': '10.90.0.2'})]) - result = self.inv.build_hostnames(changed_hosts) - self.assertEqual(expected, result) - def test_build_hostnames_add_duplicate(self): changed_hosts = ['10.90.0.2'] - expected = OrderedDict([('node1', + expected = OrderedDict([('node3', {'ansible_host': '10.90.0.2', 'ip': '10.90.0.2', 'access_ip': '10.90.0.2'})]) self.inv.yaml_config['all']['hosts'] = expected - result = self.inv.build_hostnames(changed_hosts) + result = self.inv.build_hostnames(changed_hosts, True) self.assertEqual(expected, result) def test_build_hostnames_add_two(self): @@ -99,6 +90,30 @@ class TestInventory(unittest.TestCase): result = self.inv.build_hostnames(changed_hosts) self.assertEqual(expected, result) + def test_build_hostnames_add_three(self): + changed_hosts = ['10.90.0.2', '10.90.0.3', '10.90.0.4'] + expected = OrderedDict([ + ('node1', {'ansible_host': '10.90.0.2', + 'ip': '10.90.0.2', + 'access_ip': '10.90.0.2'}), + ('node2', {'ansible_host': '10.90.0.3', + 'ip': '10.90.0.3', + 'access_ip': '10.90.0.3'}), + ('node3', {'ansible_host': '10.90.0.4', + 'ip': '10.90.0.4', + 'access_ip': '10.90.0.4'})]) + result = self.inv.build_hostnames(changed_hosts) + self.assertEqual(expected, result) + + def test_build_hostnames_add_one(self): + changed_hosts = ['10.90.0.2'] + expected = OrderedDict([('node1', + {'ansible_host': '10.90.0.2', + 'ip': '10.90.0.2', + 'access_ip': '10.90.0.2'})]) + result = self.inv.build_hostnames(changed_hosts) + self.assertEqual(expected, result) + def test_build_hostnames_delete_first(self): changed_hosts = ['-10.90.0.2'] existing_hosts = OrderedDict([ @@ -113,7 +128,24 @@ class TestInventory(unittest.TestCase): ('node2', {'ansible_host': '10.90.0.3', 'ip': '10.90.0.3', 'access_ip': '10.90.0.3'})]) - result = self.inv.build_hostnames(changed_hosts) + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + def test_build_hostnames_delete_by_hostname(self): + changed_hosts = ['-node1'] + existing_hosts = OrderedDict([ + ('node1', {'ansible_host': '10.90.0.2', + 'ip': '10.90.0.2', + 'access_ip': '10.90.0.2'}), + ('node2', {'ansible_host': '10.90.0.3', + 'ip': '10.90.0.3', + 'access_ip': '10.90.0.3'})]) + self.inv.yaml_config['all']['hosts'] = existing_hosts + expected = OrderedDict([ + ('node2', {'ansible_host': '10.90.0.3', + 'ip': '10.90.0.3', + 'access_ip': '10.90.0.3'})]) + result = self.inv.build_hostnames(changed_hosts, True) self.assertEqual(expected, result) def test_exists_hostname_positive(self): @@ -313,7 +345,7 @@ class TestInventory(unittest.TestCase): self.assertRaisesRegex(Exception, "Range of ip_addresses isn't valid", self.inv.range2ips, host_range) - def test_build_hostnames_different_ips_add_one(self): + def test_build_hostnames_create_with_one_different_ips(self): changed_hosts = ['10.90.0.2,192.168.0.2'] expected = OrderedDict([('node1', {'ansible_host': '192.168.0.2', @@ -322,17 +354,7 @@ class TestInventory(unittest.TestCase): result = self.inv.build_hostnames(changed_hosts) self.assertEqual(expected, result) - def test_build_hostnames_different_ips_add_duplicate(self): - changed_hosts = ['10.90.0.2,192.168.0.2'] - expected = OrderedDict([('node1', - {'ansible_host': '192.168.0.2', - 'ip': '10.90.0.2', - 'access_ip': '192.168.0.2'})]) - self.inv.yaml_config['all']['hosts'] = expected - result = self.inv.build_hostnames(changed_hosts) - self.assertEqual(expected, result) - - def test_build_hostnames_different_ips_add_two(self): + def test_build_hostnames_create_with_two_different_ips(self): changed_hosts = ['10.90.0.2,192.168.0.2', '10.90.0.3,192.168.0.3'] expected = OrderedDict([ ('node1', {'ansible_host': '192.168.0.2', @@ -341,6 +363,210 @@ class TestInventory(unittest.TestCase): ('node2', {'ansible_host': '192.168.0.3', 'ip': '10.90.0.3', 'access_ip': '192.168.0.3'})]) - self.inv.yaml_config['all']['hosts'] = OrderedDict() result = self.inv.build_hostnames(changed_hosts) self.assertEqual(expected, result) + + def test_build_hostnames_create_with_three_different_ips(self): + changed_hosts = ['10.90.0.2,192.168.0.2', + '10.90.0.3,192.168.0.3', + '10.90.0.4,192.168.0.4'] + expected = OrderedDict([ + ('node1', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node2', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node3', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + result = self.inv.build_hostnames(changed_hosts) + self.assertEqual(expected, result) + + def test_build_hostnames_overwrite_one_with_different_ips(self): + changed_hosts = ['10.90.0.2,192.168.0.2'] + expected = OrderedDict([('node1', + {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'})]) + existing = OrderedDict([('node5', + {'ansible_host': '192.168.0.5', + 'ip': '10.90.0.5', + 'access_ip': '192.168.0.5'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts) + self.assertEqual(expected, result) + + def test_build_hostnames_overwrite_three_with_different_ips(self): + changed_hosts = ['10.90.0.2,192.168.0.2'] + expected = OrderedDict([('node1', + {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'})]) + existing = OrderedDict([ + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'}), + ('node5', {'ansible_host': '192.168.0.5', + 'ip': '10.90.0.5', + 'access_ip': '192.168.0.5'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts) + self.assertEqual(expected, result) + + def test_build_hostnames_different_ips_add_duplicate(self): + changed_hosts = ['10.90.0.2,192.168.0.2'] + expected = OrderedDict([('node3', + {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'})]) + existing = expected + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + def test_build_hostnames_add_two_different_ips_into_one_existing(self): + changed_hosts = ['10.90.0.3,192.168.0.3', '10.90.0.4,192.168.0.4'] + expected = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + + existing = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + def test_build_hostnames_add_two_different_ips_into_two_existing(self): + changed_hosts = ['10.90.0.4,192.168.0.4', '10.90.0.5,192.168.0.5'] + expected = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'}), + ('node5', {'ansible_host': '192.168.0.5', + 'ip': '10.90.0.5', + 'access_ip': '192.168.0.5'})]) + + existing = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + def test_build_hostnames_add_two_different_ips_into_three_existing(self): + changed_hosts = ['10.90.0.5,192.168.0.5', '10.90.0.6,192.168.0.6'] + expected = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'}), + ('node5', {'ansible_host': '192.168.0.5', + 'ip': '10.90.0.5', + 'access_ip': '192.168.0.5'}), + ('node6', {'ansible_host': '192.168.0.6', + 'ip': '10.90.0.6', + 'access_ip': '192.168.0.6'})]) + + existing = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + # Add two IP addresses into a config that has + # three already defined IP addresses. One of the IP addresses + # is a duplicate. + def test_build_hostnames_add_two_duplicate_one_overlap(self): + changed_hosts = ['10.90.0.4,192.168.0.4', '10.90.0.5,192.168.0.5'] + expected = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'}), + ('node5', {'ansible_host': '192.168.0.5', + 'ip': '10.90.0.5', + 'access_ip': '192.168.0.5'})]) + + existing = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result) + + # Add two duplicate IP addresses into a config that has + # three already defined IP addresses + def test_build_hostnames_add_two_duplicate_two_overlap(self): + changed_hosts = ['10.90.0.3,192.168.0.3', '10.90.0.4,192.168.0.4'] + expected = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + + existing = OrderedDict([ + ('node2', {'ansible_host': '192.168.0.2', + 'ip': '10.90.0.2', + 'access_ip': '192.168.0.2'}), + ('node3', {'ansible_host': '192.168.0.3', + 'ip': '10.90.0.3', + 'access_ip': '192.168.0.3'}), + ('node4', {'ansible_host': '192.168.0.4', + 'ip': '10.90.0.4', + 'access_ip': '192.168.0.4'})]) + self.inv.yaml_config['all']['hosts'] = existing + result = self.inv.build_hostnames(changed_hosts, True) + self.assertEqual(expected, result)