Check for configured groups in the environment

When defining a configuration, it's possible that the groups defined in
the openstack_user_config.yml file might not match those defined by the
env.d contents. This isn't strictly a problem - in most cases it's
ignored and will simply result in extra data. However, if keys in the
configuration and the environment are mismatched, it could result in
variable data not being mapped correctly in the resultant inventory.

This patchset extends the -c/--check parameter to compare the configured
groups against the environment's groups, and issue a warning if there is
a mismatch, which should help deployers diagnose some configuration
issues.

The warnings were not added to standard run operations due to the amount
of output generated on stdout by the JSON dump.

Change-Id: I7a7e52b31909e963873063e43e28a5bb9a6963fd

doc/source/developer-docs/inventory.rst

@@ -124,6 +124,9 @@ Using the ``--check`` flag when running ``dynamic_inventory.py`` will run the
 inventory build process and look for known errors, but not write any files to
 disk.
 
+If any groups defined in the ``openstack_user_config.yml`` or ``conf.d`` files
+are not found in the environment, a warning will be raised.
+
 This check does not do YAML syntax validation, though it will fail if there
 are unparseable errors.
 

lib/generate.py

@@ -25,6 +25,7 @@ import Queue
 import random
 import tarfile
 import uuid
+import warnings
 import yaml
 
 logger = logging.getLogger('osa-inventory')
@@ -1057,6 +1058,34 @@ def _check_config_settings(cidr_networks, config, container_skel):
     _check_lxc_hosts(config)
 
 
+def _check_all_conf_groups_present(config, environment):
+    """Verifies that all groups defined in the config are in the environment
+
+    If a group is in config but not the environment, a warning will be raised.
+    Multiple warnings can be raised, and the return value will be set to False.
+
+    If all groups found are in the environment, the function returns True
+
+    :param config: ``dict`` user's provided configuration
+    :param environment: ``dict`` group membership mapping
+    :rtype: bool, True if all groups are in environment, False otherwise
+    """
+    excludes = ('global_overrides', 'cidr_networks', 'used_ips')
+    config_groups = [k for k in config.keys() if k not in excludes]
+    env_groups = environment['physical_skel'].keys()
+
+    retval = True
+
+    for group in config_groups:
+        if group not in env_groups:
+            msg = ("Group %s was found in configuration but "
+                   "not the environment." % group)
+            warnings.warn(msg)
+
+            retval = False
+    return retval
+
+
 def load_environment(config_path, environment):
     """Create an environment dictionary from config files
 
@@ -1228,6 +1257,7 @@ def main(config=None, check=False, debug=False, environment=None, **kwargs):
     )
 
     if check:
+        if _check_all_conf_groups_present(user_defined_config, environment):
             return 'Configuration ok!'
 
     # Generate a list of all hosts and their used IP addresses

releasenotes/notes/inventory-check-groups-1cc245cdcbb999df.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - The ``--check`` parameter for ``dynamic_inventory.py`` will now raise
+    warnings if there are any groups defined in the user configuration that
+    are not also found in the environment definition.

tests/test_inventory.py

@@ -10,6 +10,7 @@ from os import path
 import Queue
 import sys
 import unittest
+import warnings
 import yaml
 
 INV_DIR = 'playbooks/inventory'
@@ -1202,5 +1203,59 @@ class TestLxcHosts(TestConfigCheckBase):
             get_inventory()
 
 
+class TestConfigMatchesEnvironment(unittest.TestCase):
+    def setUp(self):
+        self.env = di.load_environment(BASE_ENV_DIR, {})
+
+    def test_matching_keys(self):
+        config = get_config()
+
+        result = di._check_all_conf_groups_present(config, self.env)
+        self.assertTrue(result)
+
+    def test_failed_match(self):
+        bad_config = get_config()
+        bad_config['bogus_key'] = []
+
+        result = di._check_all_conf_groups_present(bad_config, self.env)
+        self.assertFalse(result)
+
+    def test_extra_config_key_warning(self):
+        bad_config = get_config()
+        bad_config['bogus_key'] = []
+        with warnings.catch_warnings(record=True) as wl:
+            di._check_all_conf_groups_present(bad_config, self.env)
+            self.assertEqual(1, len(wl))
+            self.assertTrue('bogus_key' in str(wl[0].message))
+
+    def test_multiple_extra_keys(self):
+        bad_config = get_config()
+        bad_config['bogus_key1'] = []
+        bad_config['bogus_key2'] = []
+
+        with warnings.catch_warnings(record=True) as wl:
+            di._check_all_conf_groups_present(bad_config, self.env)
+            self.assertEqual(2, len(wl))
+            warn_msgs = [str(warn.message) for warn in wl]
+            warn_msgs.sort()
+            self.assertTrue('bogus_key1' in warn_msgs[0])
+            self.assertTrue('bogus_key2' in warn_msgs[1])
+
+    def test_confirm_exclusions(self):
+        """Ensure the the excluded keys in the function are present."""
+        config = get_config()
+        excluded_keys = ('global_overrides', 'cidr_networks', 'used_ips')
+
+        for key in excluded_keys:
+            config[key] = 'sentinel value'
+
+        with warnings.catch_warnings(record=True) as wl:
+            di._check_all_conf_groups_present(config, self.env)
+            self.assertEqual(0, len(wl))
+
+        for key in excluded_keys:
+            self.assertIn(key, config.keys())
+
+
 if __name__ == '__main__':
     unittest.main()