diff --git a/lib/generate.py b/lib/generate.py index a46ceb93d5..564c20b08e 100755 --- a/lib/generate.py +++ b/lib/generate.py @@ -175,6 +175,7 @@ def _build_container_hosts(container_affinity, container_hosts, type_and_name, du.append_if(array=container_list, item=i) existing_count = len(list(set(container_list))) + if existing_count < container_affinity: hostvars = inventory['_meta']['hostvars'] container_mapping = inventory[container_type]['children'] @@ -196,6 +197,7 @@ def _build_container_hosts(container_affinity, container_hosts, type_and_name, array=inventory[container_host_type]["hosts"], item=container_host_name ) + if appended: logger.debug("Added container %s to %s", container_host_name, container_host_type) @@ -346,10 +348,6 @@ def _add_container_hosts(assignment, config, container_name, container_type, # If host_type is not in config do not append containers to it if host_type not in config[physical_host_type]: continue - appended = du.append_if(array=inventory['lxc_hosts']['hosts'], - item=host_type) - if appended: - logger.debug("%s added to lxc_hosts group", host_type) # Get any set host options host_options = config[physical_host_type][host_type] @@ -671,9 +669,7 @@ def container_skel_load(container_skel, inventory, config): :param config: ``dict`` User defined information """ logger.debug("Loading container skeleton") - if 'lxc_hosts' not in inventory.keys(): - logger.debug("Created lxc_hosts group.") - inventory['lxc_hosts'] = {'hosts': []} + for key, value in container_skel.iteritems(): contains_in = value.get('contains', False) belongs_to_in = value.get('belongs_to', False) @@ -734,6 +730,46 @@ def container_skel_load(container_skel, inventory, config): static_routes=p_net.get('static_routes') ) + populate_lxc_hosts(inventory) + + +def populate_lxc_hosts(inventory): + """Insert nodes hosting LXC containers into the lxc_hosts group + + The inventory dictionary passed in to this function will be mutated. + + :param inventory: The dictionary containing the Ansible inventory + """ + host_nodes = _find_lxc_hosts(inventory) + inventory['lxc_hosts'] = {'hosts': host_nodes} + logger.debug("Created lxc_hosts group.") + + +def _find_lxc_hosts(inventory): + """Build the lxc_hosts dynamic group + + Inspect the generated inventory for nodes that host LXC containers. + Return a list of those that match for insertion into the inventory. + Populate the 'lxc_hosts' group with any node that matches. + + This and the populate_lxc_hosts function are split in order to be less + coupled and more testable. + + :param inventory: The dictionary containing the Ansible inventory + :returns: List of hostnames that are LXC hosts + :rtype: list + """ + host_nodes = [] + for host, hostvars in inventory['_meta']['hostvars'].items(): + physical_host = hostvars.get('physical_host', None) + + # We want this node's "parent", so append the physical host + if not host == physical_host: + appended = du.append_if(array=host_nodes, item=physical_host) + if appended: + logger.debug("%s added to lxc_hosts group", physical_host) + return host_nodes + def _ensure_inventory_uptodate(inventory, container_skel): """Update inventory if needed. diff --git a/releasenotes/notes/fix_lxc_hosts_membership-1f73f3114638c09e.yaml b/releasenotes/notes/fix_lxc_hosts_membership-1f73f3114638c09e.yaml new file mode 100644 index 0000000000..084219f6b0 --- /dev/null +++ b/releasenotes/notes/fix_lxc_hosts_membership-1f73f3114638c09e.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - Metal hosts were being inserted into the ``lxc_hosts`` + group, even if they had no containers (Bug 1660996). + This is now corrected for newly configured hosts. In addition, any + hosts that did not belong in ``lxc_hosts`` will be removed on the + next inventory run or playbook call. diff --git a/tests/test_inventory.py b/tests/test_inventory.py index 3a89bdde0f..491582aa8f 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -1213,6 +1213,40 @@ class TestLxcHosts(TestConfigCheckBase): with self.assertRaises(di.LxcHostsDefined): get_inventory() + def test_host_without_containers(self): + self.add_host('compute_hosts', 'compute1', '172.29.236.102') + inventory = get_inventory() + self.assertNotIn('compute1', inventory['lxc_hosts']['hosts']) + + def test_cleaning_bad_hosts(self): + self.add_host('compute_hosts', 'compute1', '172.29.236.102') + inventory = get_inventory() + # insert compute1 into lxc_hosts, which mimicks bug behavior + inventory['lxc_hosts']['hosts'].append('compute1') + faked_path = INV_DIR + + with mock.patch('filesystem.load_inventory') as inv_mock: + inv_mock.return_value = (inventory, faked_path) + new_inventory = get_inventory() + # host should no longer be in lxc_hosts + + self.assertNotIn('compute1', new_inventory['lxc_hosts']['hosts']) + + def test_emptying_lxc_hosts(self): + """If lxc_hosts is deleted between runs, it should re-populate""" + + inventory = get_inventory() + original_lxc_hosts = inventory.pop('lxc_hosts') + + self.assertNotIn('lxc_hosts', inventory.keys()) + + faked_path = INV_DIR + with mock.patch('filesystem.load_inventory') as inv_mock: + inv_mock.return_value = (inventory, faked_path) + new_inventory = get_inventory() + + self.assertEqual(original_lxc_hosts, new_inventory['lxc_hosts']) + class TestConfigMatchesEnvironment(unittest.TestCase): def setUp(self):