From cc7bd498fa8c7879ffa64b5361636ad410b7dbe4 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Tue, 21 Jun 2016 19:54:23 -0400 Subject: [PATCH] Change USED_IPS variable to a Python set USED_IPS being a list did not convey it's intention of limiting IP addresses to only one instance. Using the set data type does this, and removes the need for iterating over the USED_IP array from our own code. Other lists within the code that use the 'append_if' function were not changed here because the set datatype does not get written as a list in YAML, which may be undesirable. It may be completely possible to cast sets as lists on output, but this patch is largely about clarifying intent around USED_IPS so the set-to-list dump logic wasn't explored. Also, the duplicate IP tests were left in place due to the fact that the IP addresses are populated into a queue structure separately, which may still have bugs that don't belong to Python or netaddr. In the future, it may be desirable to modify the IP-handling logic to completely use netaddr's specialized classes. Change-Id: I22312321738d1f1001984d4c81f7fd24b7a1f1c8 --- playbooks/inventory/dynamic_inventory.py | 14 +++++++------- tests/test_inventory.py | 5 +++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 35ec8513aa..5d668dc42d 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -29,7 +29,7 @@ import uuid import yaml -USED_IPS = [] +USED_IPS = set() INVENTORY_SKEL = { '_meta': { 'hostvars': {} @@ -130,7 +130,7 @@ def get_ip_address(name, ip_q): while ip_addr in USED_IPS: ip_addr = ip_q.get(timeout=1) else: - append_if(array=USED_IPS, item=ip_addr) + USED_IPS.add(ip_addr) return str(ip_addr) except AttributeError: return None @@ -151,7 +151,7 @@ def _load_ip_q(cidr, ip_q): str(netaddr.IPNetwork(cidr).network), str(netaddr.IPNetwork(cidr).broadcast) ] - USED_IPS.extend(base_exclude) + USED_IPS.update(base_exclude) for ip in random.sample(_all_ips, len(_all_ips)): if ip not in USED_IPS: ip_q.put(ip) @@ -443,7 +443,7 @@ def user_defined_setup(config, inventory): for _k, _v in _value['host_vars'].items(): hvs[_key][_k] = _v - append_if(array=USED_IPS, item=_value['ip']) + USED_IPS.add(_value['ip']) append_if(array=inventory[key]['hosts'], item=_key) @@ -803,9 +803,9 @@ def _set_used_ips(user_defined_config, inventory): split_ip[-1] ) ) - USED_IPS.extend([str(i) for i in ip_range]) + USED_IPS.update([str(i) for i in ip_range]) else: - append_if(array=USED_IPS, item=split_ip[0]) + USED_IPS.add(split_ip[0]) # Find all used IP addresses and ensure that they are not used again for host_entry in inventory['_meta']['hostvars'].values(): @@ -813,7 +813,7 @@ def _set_used_ips(user_defined_config, inventory): for network_entry in networks.values(): address = network_entry.get('address') if address: - append_if(array=USED_IPS, item=address) + USED_IPS.add(address) def _ensure_inventory_uptodate(inventory, container_skel): diff --git a/tests/test_inventory.py b/tests/test_inventory.py index d7d3adfd5e..409145a450 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -321,7 +321,8 @@ class TestIps(unittest.TestCase): # tearDown is ineffective for this loop, so clean the USED_IPs # on each run inventory = None - di.USED_IPS = [] + di.USED_IPS = set() + inventory = get_inventory() ips = collections.defaultdict(int) hostvars = inventory['_meta']['hostvars'] @@ -349,7 +350,7 @@ class TestIps(unittest.TestCase): def tearDown(self): # Since the get_ip_address function touches USED_IPS, # and USED_IPS is currently a global var, make sure we clean it out - di.USED_IPS = [] + di.USED_IPS = set() class TestConfigChecks(unittest.TestCase):