From ea6954f72fb4eb13d4333e4ed7fc01735aba6368 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Thu, 6 Jul 2017 16:32:05 -0400 Subject: [PATCH] Use python import machinery for inventory code Change Idb7780f55e4a1fd77dd76becbf67c1ccbf220db7 restructured the python inventory generation code so that it would be possible to install it with pip. This change removes (most) of the import path hacks and switches to using a pip-installed version of osa_toolkit. Of note, the path hacks are left in place for the dynamic_inventory.py file for now, as that file is really more of an endpoint, but is tested. Also, the bootstrap-ansible.sh script was modified to install the code; this is unnecessary with the tox environments because the tox directive 'usedevelop=True' does that already. Production environments still need this, though. Finally, to maintain usability when called directly, the interpreter for dynamic_inventory.py was updated from `/usr/bin/env` to `/opt/ansible-runime/python`. This change ensures that in a full deployment the user is using the exact same code paths whether Ansible invokes the script, or it is called directly. This also means that using the script locally on a development machine, it must be invoked as an argument to Python, unless the ansible-runtime directory exists. Change-Id: Iafa573b1b144f98528d5e0aceb3f36e9de2a22a2 --- doc/source/contributor/inventory.rst | 3 ++ osa_toolkit/filesystem.py | 2 +- osa_toolkit/generate.py | 6 ++-- playbooks/inventory/dynamic_inventory.py | 14 ++++++---- ...ntory-python-imports-c967b20e64b43758.yaml | 6 ++++ scripts/bootstrap-ansible.sh | 4 +++ scripts/gen-config.py | 7 ++--- scripts/inventory-manage.py | 8 +----- tests/test_dictutils.py | 9 +----- tests/test_filesystem.py | 8 ++---- tests/test_inventory.py | 28 +++++++++---------- tests/test_ip.py | 9 +----- tests/test_manage.py | 6 +--- 13 files changed, 47 insertions(+), 63 deletions(-) create mode 100644 releasenotes/notes/inventory-python-imports-c967b20e64b43758.yaml diff --git a/doc/source/contributor/inventory.rst b/doc/source/contributor/inventory.rst index 58f0521190..e545f3d0e6 100644 --- a/doc/source/contributor/inventory.rst +++ b/doc/source/contributor/inventory.rst @@ -31,6 +31,9 @@ The command can also be run manually as follows: This invocation is useful when testing changes to the dynamic inventory script. +.. note:: When running the ``dynamic_inventory.py`` script on a local + development machine, use ``python dynamic_inventory.py`` instead. + Inputs ^^^^^^ diff --git a/osa_toolkit/filesystem.py b/osa_toolkit/filesystem.py index b7742db70b..099e57d2b2 100644 --- a/osa_toolkit/filesystem.py +++ b/osa_toolkit/filesystem.py @@ -21,10 +21,10 @@ import datetime import json import logging import os +from osa_toolkit import dictutils as du import tarfile import yaml -import dictutils as du logger = logging.getLogger('osa-inventory') diff --git a/osa_toolkit/generate.py b/osa_toolkit/generate.py index 5bb966f405..c3cef7601d 100755 --- a/osa_toolkit/generate.py +++ b/osa_toolkit/generate.py @@ -15,15 +15,15 @@ # # (c) 2014, Kevin Carter -import ip import json import logging import netaddr +from osa_toolkit import dictutils as du +from osa_toolkit import filesystem as filesys +from osa_toolkit import ip import uuid import warnings -import dictutils as du -import filesystem as filesys logger = logging.getLogger('osa-inventory') diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 5eee369411..c5eb545aee 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/opt/ansible-runtime/bin/python # Copyright 2014, Rackspace US, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,11 +19,13 @@ import argparse import os import sys -current_path = os.path.abspath(os.path.dirname(os.path.realpath(__file__))) -lib_path = os.path.join(current_path, '..', '..', 'osa_toolkit') -sys.path.append(lib_path) - -import generate +try: + from osa_toolkit import generate +except ImportError: + current_path = os.path.abspath(os.path.dirname(os.path.realpath(__file__))) + lib_path = os.path.join(current_path, '..', '..', 'osa_toolkit') + sys.path.append(lib_path) + from osa_toolkit import generate # Function kept in order to use relative pathing for the env.d directory diff --git a/releasenotes/notes/inventory-python-imports-c967b20e64b43758.yaml b/releasenotes/notes/inventory-python-imports-c967b20e64b43758.yaml new file mode 100644 index 0000000000..495c949163 --- /dev/null +++ b/releasenotes/notes/inventory-python-imports-c967b20e64b43758.yaml @@ -0,0 +1,6 @@ +--- +other: + - The inventory generation code has been switched to use standard Python + packaging tools. For most, this should not be a visible change. However, + running the dynamic inventory script on a local development environment + should now be called via ``python dynamic_inventory.py``. diff --git a/scripts/bootstrap-ansible.sh b/scripts/bootstrap-ansible.sh index ae4f2e5bf1..ff98ad8083 100755 --- a/scripts/bootstrap-ansible.sh +++ b/scripts/bootstrap-ansible.sh @@ -80,6 +80,7 @@ esac PIP_OPTS="" if [ -n "$HTTPS_PROXY" ]; then PIP_OPTS="--proxy $HTTPS_PROXY" + elif [ -n "$HTTP_PROXY" ]; then PIP_OPTS="--proxy $HTTP_PROXY" fi @@ -141,6 +142,9 @@ PIP_OPTS+=" --constraint ${UPPER_CONSTRAINTS_FILE}" ${PIP_COMMAND} install ${PIP_OPTS} -r requirements.txt ${ANSIBLE_PACKAGE} \ || ${PIP_COMMAND} install --isolated ${PIP_OPTS} -r requirements.txt ${ANSIBLE_PACKAGE} +# Install our osa_toolkit code from the current checkout +$PIP_COMMAND install -e . + # Ensure that Ansible binaries run from the venv pushd /opt/ansible-runtime/bin for ansible_bin in $(ls -1 ansible*); do diff --git a/scripts/gen-config.py b/scripts/gen-config.py index dab9d06b09..f2aea94e05 100755 --- a/scripts/gen-config.py +++ b/scripts/gen-config.py @@ -19,11 +19,7 @@ import argparse import os import sys -cwd = os.path.abspath(os.path.dirname(__file__)) -import_path = os.path.join(cwd, '..', 'osa_toolkit') -sys.path.append(import_path) - -import tools +from osa_toolki import tools def args(arg_list): @@ -58,6 +54,7 @@ def args(arg_list): if __name__ == "__main__": script_args = args(sys.argv[1:]) + config = tools.make_example_config( script_args['base'], script_args['conf_dir'] diff --git a/scripts/inventory-manage.py b/scripts/inventory-manage.py index 9caa571ee0..be70a25f2e 100755 --- a/scripts/inventory-manage.py +++ b/scripts/inventory-manage.py @@ -23,14 +23,8 @@ # to manage.py in order to facilitate importing of the python code # This file remains for backwards compatibility -import os -import sys -cwd = os.path.abspath(os.path.dirname(__file__)) -import_path = os.path.join(cwd, '..', 'osa_toolkit') -sys.path.append(import_path) - -import manage +from osa_toolkit import manage if __name__ == "__main__": manage.main() diff --git a/tests/test_dictutils.py b/tests/test_dictutils.py index 7568ab1b6e..0fb5a30017 100644 --- a/tests/test_dictutils.py +++ b/tests/test_dictutils.py @@ -12,16 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. -import os -from os import path -import sys import unittest -LIB_DIR = 'osa_toolkit' - -sys.path.append(path.join(os.getcwd(), LIB_DIR)) - -import dictutils as du +from osa_toolkit import dictutils as du class TestMergeDictUnit(unittest.TestCase): diff --git a/tests/test_filesystem.py b/tests/test_filesystem.py index 6a656fa511..71e3cea429 100644 --- a/tests/test_filesystem.py +++ b/tests/test_filesystem.py @@ -24,12 +24,10 @@ from test_inventory import get_inventory from test_inventory import make_config INV_DIR = 'playbooks/inventory' -LIB_DIR = 'osa_toolkit' -sys.path.append(path.join(os.getcwd(), LIB_DIR)) sys.path.append(path.join(os.getcwd(), INV_DIR)) -import filesystem as fs +from osa_toolkit import filesystem as fs TARGET_DIR = path.join(os.getcwd(), 'tests', 'inventory') USER_CONFIG_FILE = path.join(TARGET_DIR, 'openstack_user_config.yml') @@ -50,14 +48,14 @@ class TestMultipleRuns(unittest.TestCase): def test_creating_backup_file(self): inventory_file_path = os.path.join(TARGET_DIR, 'openstack_inventory.json') - get_backup_name_path = 'filesystem._get_backup_name' + get_backup_name_path = 'osa_toolkit.filesystem._get_backup_name' backup_name = 'openstack_inventory.json-20160531_171804.json' tar_file = mock.MagicMock() tar_file.__enter__.return_value = tar_file # run make backup with faked tarfiles and date - with mock.patch('filesystem.tarfile.open') as tar_open: + with mock.patch('osa_toolkit.filesystem.tarfile.open') as tar_open: tar_open.return_value = tar_file with mock.patch(get_backup_name_path) as backup_mock: backup_mock.return_value = backup_name diff --git a/tests/test_inventory.py b/tests/test_inventory.py index 2aa50bd25b..03ef66a636 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -26,16 +26,14 @@ import warnings import yaml INV_DIR = 'playbooks/inventory' -LIB_DIR = 'osa_toolkit' -sys.path.append(path.join(os.getcwd(), LIB_DIR)) sys.path.append(path.join(os.getcwd(), INV_DIR)) -import dictutils +from osa_toolkit import dictutils import dynamic_inventory -import filesystem as fs -import generate as di -import tools +from osa_toolkit import filesystem as fs +from osa_toolkit import generate as di +from osa_toolkit import tools TARGET_DIR = path.join(os.getcwd(), 'tests', 'inventory') BASE_ENV_DIR = INV_DIR @@ -483,8 +481,8 @@ class TestIps(unittest.TestCase): self.longMessage = True self.env = fs.load_environment(BASE_ENV_DIR, {}) - @mock.patch('filesystem.load_environment') - @mock.patch('filesystem.load_user_configuration') + @mock.patch('osa_toolkit.filesystem.load_environment') + @mock.patch('osa_toolkit.filesystem.load_user_configuration') def test_duplicates(self, mock_load_config, mock_load_env): """Test that no duplicate IPs are made on any network.""" @@ -1182,8 +1180,8 @@ class TestNetworkEntry(unittest.TestCase): class TestDebugLogging(unittest.TestCase): - @mock.patch('generate.logging') - @mock.patch('generate.logger') + @mock.patch('osa_toolkit.generate.logging') + @mock.patch('osa_toolkit.generate.logger') def test_logging_enabled(self, mock_logger, mock_logging): # Shadow the real value so tests don't complain about it mock_logging.DEBUG = 10 @@ -1194,8 +1192,8 @@ class TestDebugLogging(unittest.TestCase): self.assertTrue(mock_logger.info.called) self.assertTrue(mock_logger.debug.called) - @mock.patch('generate.logging') - @mock.patch('generate.logger') + @mock.patch('osa_toolkit.generate.logging') + @mock.patch('osa_toolkit.generate.logger') def test_logging_disabled(self, mock_logger, mock_logging): get_inventory(extra_args={"debug": False}) @@ -1239,7 +1237,7 @@ class TestLxcHosts(TestConfigCheckBase): inventory['lxc_hosts']['hosts'].append('compute1') faked_path = INV_DIR - with mock.patch('filesystem.load_inventory') as inv_mock: + with mock.patch('osa_toolkit.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 @@ -1255,7 +1253,7 @@ class TestLxcHosts(TestConfigCheckBase): self.assertNotIn('lxc_hosts', inventory.keys()) faked_path = INV_DIR - with mock.patch('filesystem.load_inventory') as inv_mock: + with mock.patch('osa_toolkit.filesystem.load_inventory') as inv_mock: inv_mock.return_value = (inventory, faked_path) new_inventory = get_inventory() @@ -1372,7 +1370,7 @@ class TestInventoryGroupConstraints(unittest.TestCase): 'load_user_configuration': mock.DEFAULT } - with mock.patch.multiple('filesystem', **kwargs) as mocks: + with mock.patch.multiple('osa_toolkit.filesystem', **kwargs) as mocks: mocks['load_environment'].return_value = env mocks['load_user_configuration'].return_value = config diff --git a/tests/test_ip.py b/tests/test_ip.py index b80011c743..7b60fa6ee1 100644 --- a/tests/test_ip.py +++ b/tests/test_ip.py @@ -12,16 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. -import os -from os import path -import sys import unittest -LIB_DIR = path.join(os.getcwd(), 'osa_toolkit') - -sys.path.append(LIB_DIR) - -import ip +from osa_toolkit import ip class TestIPManager(unittest.TestCase): diff --git a/tests/test_manage.py b/tests/test_manage.py index aaeea1232f..584b618b77 100644 --- a/tests/test_manage.py +++ b/tests/test_manage.py @@ -14,16 +14,12 @@ import os from os import path -import sys import test_inventory import unittest -MANAGE_DIR = path.join(os.getcwd(), 'osa_toolkit') TARGET_DIR = path.join(os.getcwd(), 'tests', 'inventory') -sys.path.append(MANAGE_DIR) - -import manage as mi +from osa_toolkit import manage as mi def setUpModule():