From 91378741dcacc3891194c91055a5154809f3a0e6 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Mon, 11 Jul 2016 14:26:46 -0400 Subject: [PATCH] Allow empty container dicts in env overrides When specifying an empty dictionary for a container, the component should be skipped rather than being deployed. This change ensures that the relevant keys are present in a container's dictionary before trying to add it to the hierarchy. As an example: container_skel: nova_api_metadata_container: {} Previously, if 'contains' or 'belongs_to' were missing keys, the script would raise a KeyError. Both 'contains' and 'belongs_to' keys can comprise of empty lists rather than having them completely missing as another option for overrides. The test classes for the overrides were split into base, unit, and integration classes. Simply subclassing the integration tests from the existing unit test class caused the test_* methods to be duplicated. Thus, the supporting code for both was moved into a base class to reduce code and execution duplication. Change-Id: Iebecaaa0e1c0296d5289c050c6f7fc7ca6e71bef Closes-Bug: #1600628 --- playbooks/inventory/dynamic_inventory.py | 23 ++++--- tests/test_inventory.py | 87 +++++++++++++++++++++++- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 8a59a265d9..8865c37b4f 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -686,16 +686,19 @@ def container_skel_load(container_skel, inventory, config): :param config: ``dict`` User defined information """ for key, value in container_skel.iteritems(): - for assignment in value['contains']: - for container_type in value['belongs_to']: - _add_container_hosts( - assignment, - config, - key, - container_type, - inventory, - value.get('properties') - ) + contains_in = value.get('contains', False) + belongs_to_in = value.get('belongs_to', False) + if contains_in or belongs_to_in: + for assignment in value['contains']: + for container_type in value['belongs_to']: + _add_container_hosts( + assignment, + config, + key, + container_type, + inventory, + value.get('properties') + ) else: cidr_networks = config.get('cidr_networks') provider_queues = {} diff --git a/tests/test_inventory.py b/tests/test_inventory.py index ca89ed4ead..15eddd53cb 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -801,7 +801,7 @@ class TestEnsureInventoryUptoDate(unittest.TestCase): self.inv = None -class TestOverridingEnvVars(unittest.TestCase): +class OverridingEnvBase(unittest.TestCase): def setUp(self): self.base_env = di.load_environment(BASE_ENV_DIR, {}) @@ -816,6 +816,13 @@ class TestOverridingEnvVars(unittest.TestCase): with open(path.join(self.override_path, 'cinder.yml'), 'w') as f: f.write(yaml.safe_dump(self.cinder_config)) + def tearDown(self): + os.remove(path.join(self.override_path, 'cinder.yml')) + os.rmdir(self.override_path) + + +class TestOverridingEnvVars(OverridingEnvBase): + def test_cinder_metal_override(self): vol = self.cinder_config['container_skel']['cinder_volumes_container'] vol['properties']['is_metal'] = False @@ -880,9 +887,83 @@ class TestOverridingEnvVars(unittest.TestCase): self.assertEqual(test_vol['belongs_to'], []) + +class TestOverridingEnvIntegration(OverridingEnvBase): + def setUp(self): + super(TestOverridingEnvIntegration, self).setUp() + self.user_defined_config = {} + with open(USER_CONFIG_FILE, 'rb') as f: + self.user_defined_config.update(yaml.safe_load(f.read())) + + # Inventory is necessary since keys are assumed present + self.inv = di.get_inventory(TARGET_DIR, '') + + def skel_setup(self): + self.environment = di.load_environment(TARGET_DIR, self.base_env) + + di.skel_setup(self.environment, self.inv) + + di.skel_load( + self.environment.get('physical_skel'), + self.inv + ) + + def test_emptying_container_integration(self): + self.cinder_config = {} + self.cinder_config['container_skel'] = {'cinder_volumes_container': {}} + + self.write_override_env() + self.skel_setup() + + di.container_skel_load( + self.environment.get('container_skel'), + self.inv, + self.user_defined_config + ) + + test_vol = self.base_env['container_skel']['cinder_volumes_container'] + + self.assertNotIn('belongs_to', test_vol) + self.assertNotIn('contains', test_vol) + + def test_empty_contains(self): + vol = self.cinder_config['container_skel']['cinder_volumes_container'] + vol['contains'] = [] + + self.write_override_env() + self.skel_setup() + + di.container_skel_load( + self.environment.get('container_skel'), + self.inv, + self.user_defined_config + ) + + test_vol = self.base_env['container_skel']['cinder_volumes_container'] + + self.assertEqual(test_vol['contains'], []) + + def test_empty_belongs_to(self): + vol = self.cinder_config['container_skel']['cinder_volumes_container'] + vol['belongs_to'] = [] + + self.write_override_env() + self.skel_setup() + + di.container_skel_load( + self.environment.get('container_skel'), + self.inv, + self.user_defined_config + ) + + test_vol = self.base_env['container_skel']['cinder_volumes_container'] + + self.assertEqual(test_vol['belongs_to'], []) + def tearDown(self): - os.remove(path.join(self.override_path, 'cinder.yml')) - os.rmdir(self.override_path) + super(TestOverridingEnvIntegration, self).tearDown() + self.user_defined_config = None + self.inv = None if __name__ == '__main__': unittest.main()