openstack/openstack-ansible
Code Issues Proposed changes

Inventory refactor SystemExit uses removed

Browse Source 
The use of SystemExit exceptions in what is effectively library code
has been eliminated. A new exception is introduced and it is handled
with a translation to a SystemExit within generate.py when
appropriate.

When attempting to load an existing inventory, a tuple is returned
instead of simply the inventory. The tuple includes the path used to
load the inventory, or an appropriate path. This facilitates saving
to the correct place with save_inventory later.

Use of dir_find within generate.py was also eliminated and the logic
around path to configuration should be simpler now.

One debug logging message was adjusted to only run when the desired
file is actually loaded, where it was always trying to log a message
even if not file was found previously.

The manage.py file had imports updated to replicate the pattern used
in generate.py

Change-Id: I9caecfa09517a508882ae491df9e3d2da9386012
Parent-Id: I577cdbf4aadfcce846412edd7e2a394c257c0243
This commit is contained in:
Steve Lewis 2016-11-21 16:36:14 -08:00
parent ee33b159c1
commit 5cde716959
5 changed files with 61 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
lib/filesystem.py
View File

@ -31,6 +31,20 @@ logger = logging.getLogger('osa-inventory')
INVENTORY_FILENAME = 'openstack_inventory.json'
class MissingDataSource(Exception):
def __init__(self, *sources):
self.sources = sources
error_msg = "Could not read data sources: '{sources}'."
self.message = error_msg.format(sources=self.sources)
def __str__(self):
return self.message
def __repr__(self):
return self.message
def _get_search_paths(preferred_path=None, suffix=None):
"""Return a list of search paths, including the standard location
@ -56,15 +70,17 @@ def _get_search_paths(preferred_path=None, suffix=None):
def file_find(filename, preferred_path=None, raise_if_missing=True):
"""Return the path to an existing file, or False if no file is found.
If no file is found and raise_if_missing is True, the system will exit.
If no file is found and raise_if_missing is True, MissingDataSource
will be raised.
The file lookup will be done in the following directories:
* ``preferred_path`` [Optional]
* ``/etc/openstack_deploy/``
:param filename: ``str`` Name of the file to find
:param preferred_path: ``str`` Additional directory to look in FIRST
:param raise_if_missing: ``bool`` Should a SystemExit be raised if the file
is not found
:param raise_if_missing: ``bool`` Should a MissingDataSource be raised if
the file is not found
"""
search_paths = _get_search_paths(preferred_path, suffix=filename)
@ -75,7 +91,7 @@ def file_find(filename, preferred_path=None, raise_if_missing=True):
# The file was not found
if raise_if_missing:
raise SystemExit('No file found at: {}'.format(search_paths))
raise MissingDataSource(search_paths)
else:
return False
@ -92,8 +108,8 @@ def dir_find(preferred_path=None, suffix=None, raise_if_missing=True):
:param preferred_path: ``str`` Additional directory to look in FIRST
:param suffix: ``str`` Name of a subdirectory to find under standard paths
:param raise_if_missing: ``bool`` Should a SystemExit be raised if the
directory is not found.
:param raise_if_missing: ``bool`` Should a MissingDataSource be raised if
the directory is not found.
"""
search_paths = _get_search_paths(preferred_path, suffix)
@ -103,7 +119,7 @@ def dir_find(preferred_path=None, suffix=None, raise_if_missing=True):
# The directory was not found
if raise_if_missing:
raise SystemExit('No directory found at:{}'.format(search_paths))
raise MissingDataSource(search_paths)
else:
return False
@ -185,8 +201,8 @@ def load_from_json(filename, preferred_path=None, raise_if_missing=True):
:param filename: ``str`` Name of the file to read from
:param preferred_path: ``str`` Path to the json file to try FIRST
:param raise_if_missing: ``bool`` Should a SystemExit be raised if the file
is not found
:param raise_if_missing: ``bool`` Should a MissingDataSource be raised if
the file is not found
:return ``(dict, str)`` Dictionary describing the JSON file contents or
False, and the fully resolved file name loaded or None
"""
@ -208,19 +224,26 @@ def load_inventory(preferred_path=None, default_inv=None):
:param preferred_path: ``str`` Path to the inventory directory to try FIRST
:param default_inv: ``dict`` Default inventory skeleton
:return: ``dict`` A dictionary found or ``default_inv``
:return: ``(dict, str)`` Dictionary describing the JSON file contents or
``default_inv``, and the directory from which the inventory was loaded
or should have been loaded from.
"""
inventory, file_loaded = load_from_json(INVENTORY_FILENAME, preferred_path,
raise_if_missing=False)
if file_loaded is not False:
load_path = os.path.dirname(file_loaded)
else:
load_path = dir_find(preferred_path)
if inventory is not False:
logger.debug("Loaded existing inventory from {}".format(file_loaded))
_make_backup(preferred_path, file_loaded)
_make_backup(load_path, file_loaded)
else:
logger.debug("No existing inventory, created fresh skeleton.")
inventory = copy.deepcopy(default_inv)
return inventory
return inventory, load_path
def save_inventory(inventory_json, save_path):
@ -251,11 +274,12 @@ def load_environment(config_path, environment):
if env_plugins is not False:
_extra_config(user_defined_config=environment, base_dir=env_plugins)
logger.debug("Loaded environment from {}".format(config_path))
logger.debug("Loaded environment from {}".format(config_path))
return environment
def load_user_configuration(config_path):
def load_user_configuration(config_path=None):
"""Create a user configuration dictionary from config files
:param config_path: ``str`` path where the configuration files are kept
@ -264,8 +288,10 @@ def load_user_configuration(config_path):
user_defined_config = dict()
# Load the user defined configuration file
user_config_file = os.path.join(config_path, 'openstack_user_config.yml')
if os.path.isfile(user_config_file):
user_config_file = file_find('openstack_user_config.yml',
preferred_path=config_path,
raise_if_missing=False)
if user_config_file is not False:
with open(user_config_file, 'rb') as f:
user_defined_config.update(yaml.safe_load(f.read()) or {})
@ -276,10 +302,8 @@ def load_user_configuration(config_path):
# Exit if no user_config was found and loaded
if not user_defined_config:
raise SystemExit(
'No user config loaded\n'
'No openstack_user_config files are available in either \n{}'
'\nor \n{}/conf.d directory'.format(config_path, config_path)
)
raise MissingDataSource(_get_search_paths(config_path) +
_get_search_paths(config_path, 'conf.d'))
logger.debug("User configuration loaded from: {}".format(user_config_file))
return user_defined_config

14
lib/generate.py
View File

@ -962,16 +962,18 @@ def main(config=None, check=False, debug=False, environment=None, **kwargs):
if debug:
_prepare_debug_logger()
# Get the path to the user configuration files
config_path = filesys.dir_find(preferred_path=config)
try:
user_defined_config = filesys.load_user_configuration(config)
except filesys.MissingDataSource as ex:
raise SystemExit(ex)
user_defined_config = filesys.load_user_configuration(config_path)
base_env_dir = environment
base_env = filesys.load_environment(base_env_dir, {})
environment = filesys.load_environment(config_path, base_env)
environment = filesys.load_environment(config, base_env)
# Load existing inventory file if found
dynamic_inventory = filesys.load_inventory(config_path, INVENTORY_SKEL)
dynamic_inventory, inv_path = filesys.load_inventory(config,
INVENTORY_SKEL)
# Save the users container cidr as a group variable
cidr_networks = user_defined_config.get('cidr_networks')
@ -1042,6 +1044,6 @@ def main(config=None, check=False, debug=False, environment=None, **kwargs):
logger.debug("%d hosts found.", num_hosts)
# Save new dynamic inventory
filesys.save_inventory(dynamic_inventory_json, config_path)
filesys.save_inventory(dynamic_inventory_json, inv_path)
return dynamic_inventory_json

13
lib/manage.py
View File

@ -22,9 +22,8 @@ import argparse
import json
import prettytable
from dictutils import recursive_dict_removal
from filesystem import load_from_json
from filesystem import save_inventory
import dictutils as du
import filesystem as filesys
def args():
@ -307,7 +306,7 @@ def main():
user_args = args()
# Get the contents of the system inventory
inventory, filename = load_from_json(user_args['file'])
inventory, filename = filesys.load_from_json(user_args['file'])
# Make a table with hosts in the left column and details about each in the
# columns to the right
@ -326,12 +325,12 @@ def main():
elif user_args['clear_ips'] is True:
remove_ip_addresses(inventory)
inventory_json = json.dumps(inventory, indent=2)
save_inventory(inventory_json, filename)
filesys.save_inventory(inventory_json, filename)
print('Success. . .')
else:
recursive_dict_removal(inventory, user_args['remove_item'])
du.recursive_dict_removal(inventory, user_args['remove_item'])
inventory_json = json.dumps(inventory, indent=2)
save_inventory(inventory_json, filename)
filesys.save_inventory(inventory_json, filename)
print('Success. . .')
if __name__ == "__main__":

2
tests/test_filesystem.py
View File

@ -75,7 +75,7 @@ class TestMultipleRuns(unittest.TestCase):
# Generate the initial inventory files
get_inventory(clean=False)
inv = fs.load_inventory(TARGET_DIR)
inv, path = fs.load_inventory(TARGET_DIR)
self.assertIsInstance(inv, dict)
self.assertIn('_meta', inv)
# This test is basically just making sure we get more than

2
tests/test_inventory.py
View File

@ -991,7 +991,7 @@ class TestOverridingEnvIntegration(OverridingEnvBase):
self.user_defined_config = get_config()
# Inventory is necessary since keys are assumed present
self.inv = fs.load_inventory(TARGET_DIR, di.INVENTORY_SKEL)
self.inv, path = fs.load_inventory(TARGET_DIR, di.INVENTORY_SKEL)
def skel_setup(self):
self.environment = fs.load_environment(TARGET_DIR, self.base_env)