From e0b58924a7aa97acbace36654c5652c14827b3be Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Fri, 7 Oct 2016 15:56:47 -0400 Subject: [PATCH] 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 | 3 + lib/generate.py | 32 ++++++++++- ...ventory-check-groups-1cc245cdcbb999df.yaml | 5 ++ tests/test_inventory.py | 55 +++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/inventory-check-groups-1cc245cdcbb999df.yaml diff --git a/doc/source/developer-docs/inventory.rst b/doc/source/developer-docs/inventory.rst index 6080363987..634ea3369c 100644 --- a/doc/source/developer-docs/inventory.rst +++ b/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. diff --git a/lib/generate.py b/lib/generate.py index ccf0edb993..d2b440089c 100755 --- a/lib/generate.py +++ b/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,7 +1257,8 @@ def main(config=None, check=False, debug=False, environment=None, **kwargs): ) if check: - return 'Configuration ok!' + if _check_all_conf_groups_present(user_defined_config, environment): + return 'Configuration ok!' # Generate a list of all hosts and their used IP addresses hostnames_ips = {} diff --git a/releasenotes/notes/inventory-check-groups-1cc245cdcbb999df.yaml b/releasenotes/notes/inventory-check-groups-1cc245cdcbb999df.yaml new file mode 100644 index 0000000000..8371fb13dd --- /dev/null +++ b/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. diff --git a/tests/test_inventory.py b/tests/test_inventory.py index 0a95e35d6a..97c33878fc 100644 --- a/tests/test_inventory.py +++ b/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()