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

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.

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

@@ -1215,6 +1215,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):