From effb2a7d65afa545d6fd4d55b6b4647d2df77816 Mon Sep 17 00:00:00 2001 From: Brian Curtin Date: Tue, 3 Nov 2015 14:36:10 -0600 Subject: [PATCH] Refactor os-client-config usage in from_config This change allows the from_config factory function to better support two angles from which it will most commonly be used. 1. When SDK is being used from within OSC, OSC will have obtained configuration data from OCC and is better served by passing it directly into SDK rather than SDK additionally accessing that data. 2. Direct users of the SDK, such as customer applications, are less likely to have dealt directly with OCC and are better served by just passing in the name of the configuration they want to run with. Closes-Bug:1513920 Change-Id: I06f871b822e4b8b0cc8cc8db9e7ccff91f918e5a --- .../users/guides/connect_from_config.rst | 21 ++--- examples/connect.py | 8 +- openstack/connection.py | 32 +++++--- openstack/tests/functional/base.py | 9 +- .../tests/functional/image/v2/test_image.py | 6 +- openstack/tests/unit/test_connection.py | 82 +++++++++++++++++++ 6 files changed, 122 insertions(+), 36 deletions(-) diff --git a/doc/source/users/guides/connect_from_config.rst b/doc/source/users/guides/connect_from_config.rst index c06ffedb..e7bb51de 100644 --- a/doc/source/users/guides/connect_from_config.rst +++ b/doc/source/users/guides/connect_from_config.rst @@ -31,8 +31,13 @@ locations: * ~/.config/openstack * /etc/openstack -call :py:func:`~openstack.connection.from_config` with an object that has -the name of the cloud configuration to use. +call :py:func:`~openstack.connection.from_config`. The ``from_config`` +function takes three optional arguments, such as **cloud_name**, +which allows you to specify one set of cloud credentials in your +``clouds.yaml`` file. Additionally, **cloud_config** and **options** +allow you to pass in configiration data you may have already received +from ``os-client-config``, as well as additional options that the +``os-client-config`` library may need. .. literalinclude:: ../examples/connect.py :pyobject: Opts @@ -51,16 +56,8 @@ absolute path of a file.:: export OS_CLIENT_CONFIG_FILE=/path/to/my/config/my-clouds.yaml -and call :py:func:`~openstack.connection.from_config` with an object that has -the name of the cloud configuration to use. - -.. literalinclude:: ../examples/connect.py - :pyobject: Opts - -.. literalinclude:: ../examples/connect.py - :pyobject: create_connection_from_config - -.. note:: To enable logging, set ``debug=True`` in the ``Opts`` object. +and call :py:func:`~openstack.connection.from_config` with the **cloud_name** +of the cloud configuration to use, . .. Create Connection From Environment Variables -------------------------------------------- diff --git a/examples/connect.py b/examples/connect.py index dc45b531..b5714041 100644 --- a/examples/connect.py +++ b/examples/connect.py @@ -35,8 +35,8 @@ TEST_CLOUD = os.getenv('OS_TEST_CLOUD', 'test_cloud') class Opts(object): - def __init__(self, test_cloud='test_cloud', debug=False): - self.cloud = test_cloud + def __init__(self, cloud_name='test_cloud', debug=False): + self.cloud = cloud_name self.debug = debug @@ -46,7 +46,7 @@ def _get_resource_value(resource_key, default): except KeyError: return default -opts = Opts(test_cloud=TEST_CLOUD) +opts = Opts(cloud_name=TEST_CLOUD) occ = os_client_config.OpenStackConfig() cloud = occ.get_one_cloud(opts.cloud, argparse=opts) @@ -62,7 +62,7 @@ PRIVATE_KEYPAIR_FILE = _get_resource_value('private_keypair_file', def create_connection_from_config(): - return connection.from_config(opts) + return connection.from_config(cloud_config=cloud, options=opts) def create_connection(auth_url, region, project_name, username, password): diff --git a/openstack/connection.py b/openstack/connection.py index b0a8916e..7613c218 100644 --- a/openstack/connection.py +++ b/openstack/connection.py @@ -72,17 +72,27 @@ from openstack import utils _logger = logging.getLogger(__name__) -def from_config(opts): - """Create a connection from a configuration. +def from_config(cloud_name=None, cloud_config=None, options=None): + """Create a Connection using os-client-config - Create a :class:`~openstack.connection.Connection` from a configuration - similar to a os-client-config CloudConfig. - - :param opts: An options class like the :class:`~argparse.Namespace` class. + :param str cloud_name: Use the `cloud_name` configuration details when + creating the Connection instance. + :param cloud_config: An instance of + `os_client_config.config.OpenStackConfig` + as returned from the os-client-config library. + If no `config` is provided, + `os_client_config.OpenStackConfig` will be called, + and the provided `cloud_name` will be used in + determining which cloud's configuration details + will be used in creation of the + `Connection` instance. + :param options: An argparse Namespace object; allows direct passing + in of argparse options to be added to the cloud config. + This value is passed to the `argparse` argument of + `os_client_config.config.OpenStackConfig.get_one_cloud`. :rtype: :class:`~openstack.connection.Connection` """ - # TODO(thowe): I proposed that service name defaults to None in OCC defaults = {} prof = profile.Profile() @@ -92,17 +102,15 @@ def from_config(opts): # TODO(thowe): default is 2 which turns into v2 which doesn't work # this stuff needs to be fixed where we keep version and path separated. defaults['network_api_version'] = 'v2.0' - - # Get the cloud_config - occ = os_client_config.OpenStackConfig(override_defaults=defaults) - cloud_config = occ.get_one_cloud(opts.cloud, argparse=opts) + if cloud_config is None: + occ = os_client_config.OpenStackConfig(override_defaults=defaults) + cloud_config = occ.get_one_cloud(cloud=cloud_name, argparse=options) if cloud_config.debug: utils.enable_logging(True, stream=sys.stdout) # TODO(mordred) we need to add service_type setting to openstacksdk. # Some clouds have type overridden as well as name. - prof = profile.Profile() services = [service.service_type for service in prof.get_services()] for service in cloud_config.get_services(): if service in services: diff --git a/openstack/tests/functional/base.py b/openstack/tests/functional/base.py index c9765307..e37081fe 100644 --- a/openstack/tests/functional/base.py +++ b/openstack/tests/functional/base.py @@ -19,6 +19,9 @@ from openstack import exceptions from openstack import service_filter +CLOUD_NAME = os.getenv('OS_CLOUD', 'test_cloud') + + def requires_service(**kwargs): """Check whether a service is available for this test @@ -50,14 +53,10 @@ def requires_service(**kwargs): class BaseFunctionalTest(unittest.TestCase): - class Opts(object): - def __init__(self): - self.cloud = os.getenv('OS_CLOUD', 'test_cloud') @classmethod def setUpClass(cls): - opts = cls.Opts() - cls.conn = connection.from_config(opts) + cls.conn = connection.from_config(cloud_name=CLOUD_NAME) @classmethod def assertIs(cls, expected, actual): diff --git a/openstack/tests/functional/image/v2/test_image.py b/openstack/tests/functional/image/v2/test_image.py index 3dd320a9..b4ba4e91 100644 --- a/openstack/tests/functional/image/v2/test_image.py +++ b/openstack/tests/functional/image/v2/test_image.py @@ -18,15 +18,15 @@ TEST_IMAGE_NAME = 'Test Image' class TestImage(base.BaseFunctionalTest): - class ImageOpts(base.BaseFunctionalTest.Opts): + class ImageOpts(object): def __init__(self): - super(TestImage.ImageOpts, self).__init__() self.image_api_version = '2' @classmethod def setUpClass(cls): opts = cls.ImageOpts() - cls.conn = connection.from_config(opts) + cls.conn = connection.from_config(cloud_name=base.CLOUD_NAME, + options=opts) cls.img = cls.conn.image.upload_image( name=TEST_IMAGE_NAME, diff --git a/openstack/tests/unit/test_connection.py b/openstack/tests/unit/test_connection.py index f7523625..a3eb0abc 100644 --- a/openstack/tests/unit/test_connection.py +++ b/openstack/tests/unit/test_connection.py @@ -10,7 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import tempfile + import mock +import os_client_config from openstack.auth.identity import v2 from openstack import connection @@ -19,6 +23,24 @@ from openstack.tests.unit import base from openstack import transport +CONFIG_AUTH_URL = "http://127.0.0.1:5000/v2.0" +CONFIG_USERNAME = "BozoTheClown" +CONFIG_PASSWORD = "TopSecret" +CONFIG_PROJECT = "TheGrandPrizeGame" + +CLOUD_CONFIG = """ +clouds: + sample: + region_name: RegionOne + auth: + auth_url: {auth_url} + username: {username} + password: {password} + project_name: {project} +""".format(auth_url=CONFIG_AUTH_URL, username=CONFIG_USERNAME, + password=CONFIG_PASSWORD, project=CONFIG_PROJECT) + + class TestConnection(base.TestCase): def setUp(self): super(TestConnection, self).setUp() @@ -85,3 +107,63 @@ class TestConnection(base.TestCase): conn = connection.Connection(authenticator=self.auth, user_agent=user_agent) self.assertTrue(conn.transport._user_agent.startswith(user_agent)) + + def _prepare_test_config(self): + # Create a temporary directory where our test config will live + # and insert it into the search path via OS_CLIENT_CONFIG_FILE. + # NOTE: If OCC stops popping OS_C_C_F off of os.environ, this + # will need to change to respect that. It currently works between + # tests because the environment variable is always wiped by OCC itself. + config_dir = tempfile.mkdtemp() + config_path = os.path.join(config_dir, "clouds.yaml") + + with open(config_path, "w") as conf: + conf.write(CLOUD_CONFIG) + + os.environ["OS_CLIENT_CONFIG_FILE"] = config_path + + def test_from_config_given_data(self): + self._prepare_test_config() + + data = os_client_config.OpenStackConfig().get_one_cloud("sample") + + sot = connection.from_config(cloud_config=data) + + self.assertEqual(CONFIG_USERNAME, + sot.authenticator.auth_plugin.username) + self.assertEqual(CONFIG_PASSWORD, + sot.authenticator.auth_plugin.password) + self.assertEqual(CONFIG_AUTH_URL, + sot.authenticator.auth_plugin.auth_url) + self.assertEqual(CONFIG_PROJECT, + sot.authenticator.auth_plugin.tenant_name) + + def test_from_config_given_name(self): + self._prepare_test_config() + + sot = connection.from_config(cloud_name="sample") + + self.assertEqual(CONFIG_USERNAME, + sot.authenticator.auth_plugin.username) + self.assertEqual(CONFIG_PASSWORD, + sot.authenticator.auth_plugin.password) + self.assertEqual(CONFIG_AUTH_URL, + sot.authenticator.auth_plugin.auth_url) + self.assertEqual(CONFIG_PROJECT, + sot.authenticator.auth_plugin.tenant_name) + + def test_from_config_given_options(self): + self._prepare_test_config() + + version = "100" + + class Opts(object): + compute_api_version = version + + sot = connection.from_config(cloud_name="sample", options=Opts) + + pref = sot.session.profile.get_preference("compute") + + # NOTE: Along the way, the `v` prefix gets added so we can build + # up URLs with it. + self.assertEqual("v" + version, pref.version)