From 59aaac0cb5cf935d5d5e9ee899ecfd25b67923a1 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Wed, 1 Feb 2017 13:28:58 -0500 Subject: [PATCH] Only insert container hosts into lxc_hosts This change introduces a set of functions for dynamically populating the lxc_hosts function on each inventory run. Any previous membership is overwritten each time. The output is written to the openstack_inventory.json still in order to provide parity with the values provided to the ansible executable on stdout. Previous code was fairly naive, inserting hosts into lxc_hosts when they were marked as `is_metal`. This would add hosts, such as Ceph, that were on metal but had no LXC containers. A previous attempt at fixing this changed the _build_container_hosts function to provide more information about the container build process, then used that data in the _append_host_containers function to populate the lxc_hosts group. However, this approach failed due to limited information in each pass of the loop - if a node was an AIO, it might be erroneously removed from the lxc_hosts group because a container wasn't built on a given pass, and due to ordering, that pass may be the last one of the loop. To get around such problems, this code instead processes the inventory in whole, after all containers have been made. Population into the group is determined according to whether or not a given host's `physical_host` hostvar matches the host name. Change-Id: I9f3336f77cd0ef05fe1c7edeaf7defc6d93c3111 Closes-Bug: #1660996 --- lib/generate.py | 50 ++++++++++++++++--- ...lxc_hosts_membership-1f73f3114638c09e.yaml | 7 +++ tests/test_inventory.py | 34 +++++++++++++ 3 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/fix_lxc_hosts_membership-1f73f3114638c09e.yaml 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):