openstack/openstack-ansible
Code Issues Proposed changes

Change USED_IPS variable to a Python set

Browse Source 
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
This commit is contained in:
Nolan Brubaker 2016-06-21 19:54:23 -04:00
parent 582f983b4c
commit cc7bd498fa
2 changed files with 10 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
playbooks/inventory/dynamic_inventory.py
View File

@ -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):

5
tests/test_inventory.py
View File

@ -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):