Merge "Only insert container hosts into lxc_hosts" into stable/newton

playbooks/inventory/dynamic_inventory.py

@@ -249,6 +249,7 @@ def _build_container_hosts(container_affinity, container_hosts, type_and_name,
                 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']
@@ -268,11 +269,14 @@ def _build_container_hosts(container_affinity, container_hosts, type_and_name,
 
                 logger.debug("Adding container %s to %s",
                              container_host_name, container_host_type)
+
                 append_if(
                     array=inventory[container_host_type]["hosts"],
                     item=container_host_name
                 )
+
                 append_if(array=container_hosts, item=container_host_name)
+
             else:
                 if host_type not in hostvars:
                     hostvars[host_type] = {}
@@ -415,8 +419,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
-        append_if(array=inventory['lxc_hosts']['hosts'],
-                  item=host_type)
 
         # Get any set host options
         host_options = config[physical_host_type][host_type]
@@ -814,6 +816,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 = append_if(array=host_nodes, item=physical_host)
+            if appended:
+                logger.debug("%s added to lxc_hosts group", physical_host)
+    return host_nodes
+
 
 def find_config_path(user_config_path=None):
     """Return the path to the user configuration files.

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.

tests/test_inventory.py

@@ -1224,6 +1224,38 @@ 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')
+
+        with mock.patch('dynamic_inventory.get_inventory') as inv_mock:
+            inv_mock.return_value = inventory
+            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())
+
+        with mock.patch('dynamic_inventory.get_inventory') as inv_mock:
+            inv_mock.return_value = inventory
+            new_inventory = get_inventory()
+
+        self.assertEqual(original_lxc_hosts, new_inventory['lxc_hosts'])
+
 
 if __name__ == '__main__':
     unittest.main()