From 2b48637b6790139c363f0c62977bfa585a2bc1d9 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 4 May 2018 10:09:21 -0500 Subject: [PATCH] Use openstack.config directly for config shade depends on openstacksdk already. Let's go ahead and start consuming it directly rather than through the os-client-config compat layer. We have to leave the os-client-config depend for the legacy client functions. Change-Id: Ibe2176c16e29960e9cc2b569325e41d1a4e297a8 --- lower-constraints.txt | 2 +- requirements.txt | 2 +- shade/__init__.py | 8 +-- shade/inventory.py | 9 ++-- shade/openstackcloud.py | 70 ++++++++++++------------- shade/tests/functional/base.py | 2 +- shade/tests/unit/base.py | 4 +- shade/tests/unit/test_inventory.py | 24 ++++----- shade/tests/unit/test_shade_operator.py | 33 ++++-------- 9 files changed, 69 insertions(+), 85 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 02662922a..e16995536 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -19,7 +19,7 @@ mock==2.0.0 mox3==0.20.0 munch==2.2.0 netifaces==0.10.6 -openstacksdk==0.11.2 +openstacksdk==0.13.0 os-client-config==1.28.0 os-service-types==1.2.0 oslotest==3.2.0 diff --git a/requirements.txt b/requirements.txt index 4416c5b4d..0e970cc1e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,4 +4,4 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 os-client-config>=1.28.0 # Apache-2.0 -openstacksdk>=0.11.2 # Apache-2.0 +openstacksdk>=0.13.0 # Apache-2.0 diff --git a/shade/__init__.py b/shade/__init__.py index ae61b31ae..8e300ddd3 100644 --- a/shade/__init__.py +++ b/shade/__init__.py @@ -16,7 +16,7 @@ import logging import warnings import keystoneauth1.exceptions -import os_client_config +from openstack.config import loader import pbr.version import requestsexceptions @@ -34,11 +34,7 @@ if requestsexceptions.SubjectAltNameWarning: def _get_openstack_config(app_name=None, app_version=None): # Protect against older versions of os-client-config that don't expose this - try: - return os_client_config.OpenStackConfig( - app_name=app_name, app_version=app_version) - except Exception: - return os_client_config.OpenStackConfig() + return loader.OpenStackConfig(app_name=app_name, app_version=app_version) def simple_logging(debug=False, http_debug=False): diff --git a/shade/inventory.py b/shade/inventory.py index 2490e93bf..1559ba08e 100644 --- a/shade/inventory.py +++ b/shade/inventory.py @@ -14,7 +14,8 @@ import functools -import os_client_config +from openstack import exceptions +from openstack.config import loader import shade from shade import _utils @@ -31,8 +32,8 @@ class OpenStackInventory(object): use_direct_get=False): if config_files is None: config_files = [] - config = os_client_config.config.OpenStackConfig( - config_files=os_client_config.config.CONFIG_FILES + config_files) + config = loader.OpenStackConfig( + config_files=loader.CONFIG_FILES + config_files) self.extra_config = config.get_extra_config( config_key, config_defaults) @@ -47,7 +48,7 @@ class OpenStackInventory(object): shade.OpenStackCloud( cloud_config=config.get_one_cloud(cloud)) ] - except os_client_config.exceptions.OpenStackConfigException as e: + except exceptions.ConfigException as e: raise shade.OpenStackCloudException(e) if private: diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index cdbf2c057..71caa75c5 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -21,7 +21,6 @@ import iso8601 import json import jsonpatch import operator -import os_client_config.defaults import six import threading import time @@ -35,7 +34,7 @@ from six.moves import urllib import keystoneauth1.exceptions import keystoneauth1.session import os -import os_client_config +from openstack.config import loader import shade from shade.exc import * # noqa @@ -121,7 +120,7 @@ class OpenStackCloud( string. Optional, defaults to None. :param app_version: Version of the application to be appended to the user-agent string. Optional, defaults to None. - :param CloudConfig cloud_config: Cloud config object from os-client-config + :param CloudRegion cloud_config: Cloud config object from openstack.config In the future, this will be the only way to pass in cloud configuration, but is being phased in currently. @@ -140,24 +139,23 @@ class OpenStackCloud( self.log = _log.setup_logging('shade') if not cloud_config: - config = os_client_config.OpenStackConfig( + config = loader.OpenStackConfig( app_name=app_name, app_version=app_version) - cloud_config = config.get_one_cloud(**kwargs) + cloud_config = config.get_one(**kwargs) + cloud_region = cloud_config - self.name = cloud_config.name - self.auth = cloud_config.get_auth_args() - self.region_name = cloud_config.region_name - self.default_interface = cloud_config.get_interface() - self.private = cloud_config.config.get('private', False) - self.api_timeout = cloud_config.config['api_timeout'] - self.image_api_use_tasks = cloud_config.config['image_api_use_tasks'] - self.secgroup_source = cloud_config.config['secgroup_source'] - self.force_ipv4 = cloud_config.force_ipv4 + self.name = cloud_region.name + self.auth = cloud_region.get_auth_args() + self.region_name = cloud_region.region_name + self.default_interface = cloud_region.get_interface() + self.private = cloud_region.config.get('private', False) + self.api_timeout = cloud_region.config['api_timeout'] + self.image_api_use_tasks = cloud_region.config['image_api_use_tasks'] + self.secgroup_source = cloud_region.config['secgroup_source'] + self.force_ipv4 = cloud_region.force_ipv4 self.strict_mode = strict - # TODO(mordred) When os-client-config adds a "get_client_settings()" - # method to CloudConfig - remove this. - self._extra_config = cloud_config._openstack_config.get_extra_config( + self._extra_config = cloud_region.get_client_config( 'shade', { 'get_flavor_extra_specs': True, }) @@ -168,14 +166,14 @@ class OpenStackCloud( self.manager = task_manager.TaskManager( name=':'.join([self.name, self.region_name]), client=self) - self._external_ipv4_names = cloud_config.get_external_ipv4_networks() - self._internal_ipv4_names = cloud_config.get_internal_ipv4_networks() - self._external_ipv6_names = cloud_config.get_external_ipv6_networks() - self._internal_ipv6_names = cloud_config.get_internal_ipv6_networks() - self._nat_destination = cloud_config.get_nat_destination() - self._default_network = cloud_config.get_default_network() + self._external_ipv4_names = cloud_region.get_external_ipv4_networks() + self._internal_ipv4_names = cloud_region.get_internal_ipv4_networks() + self._external_ipv6_names = cloud_region.get_external_ipv6_networks() + self._internal_ipv6_names = cloud_region.get_internal_ipv6_networks() + self._nat_destination = cloud_region.get_nat_destination() + self._default_network = cloud_region.get_default_network() - self._floating_ip_source = cloud_config.config.get( + self._floating_ip_source = cloud_region.config.get( 'floating_ip_source') if self._floating_ip_source: if self._floating_ip_source.lower() == 'none': @@ -183,16 +181,16 @@ class OpenStackCloud( else: self._floating_ip_source = self._floating_ip_source.lower() - self._use_external_network = cloud_config.config.get( + self._use_external_network = cloud_region.config.get( 'use_external_network', True) - self._use_internal_network = cloud_config.config.get( + self._use_internal_network = cloud_region.config.get( 'use_internal_network', True) # Work around older TaskManager objects that don't have submit_task if not hasattr(self.manager, 'submit_task'): self.manager.submit_task = self.manager.submitTask - (self.verify, self.cert) = cloud_config.get_requests_verify_args() + (self.verify, self.cert) = cloud_region.get_requests_verify_args() # Turn off urllib3 warnings about insecure certs if we have # explicitly configured requests to tell it we do not want # cert verification @@ -226,9 +224,9 @@ class OpenStackCloud( self._networks_lock = threading.Lock() self._reset_network_caches() - cache_expiration_time = int(cloud_config.get_cache_expiration_time()) - cache_class = cloud_config.get_cache_class() - cache_arguments = cloud_config.get_cache_arguments() + cache_expiration_time = int(cloud_region.get_cache_expiration_time()) + cache_class = cloud_region.get_cache_class() + cache_arguments = cloud_region.get_cache_arguments() self._resource_caches = {} @@ -236,7 +234,7 @@ class OpenStackCloud( self.cache_enabled = True self._cache = self._make_cache( cache_class, cache_expiration_time, cache_arguments) - expirations = cloud_config.get_cache_expiration() + expirations = cloud_region.get_cache_expirations() for expire_key in expirations.keys(): # Only build caches for things we have list operations for if getattr( @@ -278,11 +276,11 @@ class OpenStackCloud( # If server expiration time is set explicitly, use that. Otherwise # fall back to whatever it was before - self._SERVER_AGE = cloud_config.get_cache_resource_expiration( + self._SERVER_AGE = cloud_region.get_cache_resource_expiration( 'server', self._SERVER_AGE) - self._PORT_AGE = cloud_config.get_cache_resource_expiration( + self._PORT_AGE = cloud_region.get_cache_resource_expiration( 'port', self._PORT_AGE) - self._FLOAT_AGE = cloud_config.get_cache_resource_expiration( + self._FLOAT_AGE = cloud_region.get_cache_resource_expiration( 'floating_ip', self._FLOAT_AGE) self._container_cache = dict() @@ -296,7 +294,7 @@ class OpenStackCloud( self._local_ipv6 = ( _utils.localhost_supports_ipv6() if not self.force_ipv4 else False) - self.cloud_config = cloud_config + self.cloud_config = cloud_region def connect_as(self, **kwargs): """Make a new OpenStackCloud object with new auth context. @@ -324,7 +322,7 @@ class OpenStackCloud( if self.cloud_config._openstack_config: config = self.cloud_config._openstack_config else: - config = os_client_config.OpenStackConfig( + config = loader.OpenStackConfig( app_name=self.cloud_config._app_name, app_version=self.cloud_config._app_version, load_yaml_config=False) diff --git a/shade/tests/functional/base.py b/shade/tests/functional/base.py index 6e6ed159b..2f7c0a0f9 100644 --- a/shade/tests/functional/base.py +++ b/shade/tests/functional/base.py @@ -12,7 +12,7 @@ import os -import os_client_config as occ +import openstack.config as occ import shade from shade.tests import base diff --git a/shade/tests/unit/base.py b/shade/tests/unit/base.py index 61a356661..edcd48d0f 100644 --- a/shade/tests/unit/base.py +++ b/shade/tests/unit/base.py @@ -20,7 +20,7 @@ import uuid import fixtures import mock import os -import os_client_config as occ +import openstack.config as occ from requests import structures from requests_mock.contrib import fixture as rm_fixture from six.moves import urllib @@ -140,7 +140,7 @@ class TestCase(BaseTestCase): super(TestCase, self).setUp(cloud_config_fixture=cloud_config_fixture) self.session_fixture = self.useFixture(fixtures.MonkeyPatch( - 'os_client_config.cloud_config.CloudConfig.get_session', + 'openstack.config.cloud_region.CloudRegion.get_session', mock.Mock())) diff --git a/shade/tests/unit/test_inventory.py b/shade/tests/unit/test_inventory.py index 84846b231..c07b304df 100644 --- a/shade/tests/unit/test_inventory.py +++ b/shade/tests/unit/test_inventory.py @@ -12,9 +12,9 @@ import mock -import os_client_config +from openstack.config import loader -from os_client_config import exceptions as occ_exc +from openstack import exceptions as os_exc from shade import exc from shade import inventory @@ -27,7 +27,7 @@ class TestInventory(base.TestCase): def setUp(self): super(TestInventory, self).setUp() - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test__init(self, mock_cloud, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] @@ -35,13 +35,13 @@ class TestInventory(base.TestCase): inv = inventory.OpenStackInventory() mock_config.assert_called_once_with( - config_files=os_client_config.config.CONFIG_FILES + config_files=loader.CONFIG_FILES ) self.assertIsInstance(inv.clouds, list) self.assertEqual(1, len(inv.clouds)) self.assertTrue(mock_config.return_value.get_all_clouds.called) - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test__init_one_cloud(self, mock_cloud, mock_config): mock_config.return_value.get_one_cloud.return_value = [{}] @@ -49,7 +49,7 @@ class TestInventory(base.TestCase): inv = inventory.OpenStackInventory(cloud='supercloud') mock_config.assert_called_once_with( - config_files=os_client_config.config.CONFIG_FILES + config_files=loader.CONFIG_FILES ) self.assertIsInstance(inv.clouds, list) self.assertEqual(1, len(inv.clouds)) @@ -57,7 +57,7 @@ class TestInventory(base.TestCase): mock_config.return_value.get_one_cloud.assert_called_once_with( 'supercloud') - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test__raise_exception_on_no_cloud(self, mock_cloud, mock_config): """ @@ -65,7 +65,7 @@ class TestInventory(base.TestCase): shade exception is emitted. """ mock_config.return_value.get_one_cloud.side_effect = ( - occ_exc.OpenStackConfigException() + os_exc.ConfigException() ) self.assertRaises(exc.OpenStackCloudException, inventory.OpenStackInventory, @@ -73,7 +73,7 @@ class TestInventory(base.TestCase): mock_config.return_value.get_one_cloud.assert_called_once_with( 'supercloud') - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test_list_hosts(self, mock_cloud, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] @@ -92,7 +92,7 @@ class TestInventory(base.TestCase): self.assertFalse(inv.clouds[0].get_openstack_vars.called) self.assertEqual([server], ret) - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test_list_hosts_no_detail(self, mock_cloud, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] @@ -111,7 +111,7 @@ class TestInventory(base.TestCase): inv.clouds[0].list_servers.assert_called_once_with(detailed=False) self.assertFalse(inv.clouds[0].get_openstack_vars.called) - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test_search_hosts(self, mock_cloud, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] @@ -127,7 +127,7 @@ class TestInventory(base.TestCase): ret = inv.search_hosts('server_id') self.assertEqual([server], ret) - @mock.patch("os_client_config.config.OpenStackConfig") + @mock.patch("openstack.config.loader.OpenStackConfig") @mock.patch("shade.OpenStackCloud") def test_get_host(self, mock_cloud, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] diff --git a/shade/tests/unit/test_shade_operator.py b/shade/tests/unit/test_shade_operator.py index 4e2df23eb..e6a9ce10e 100644 --- a/shade/tests/unit/test_shade_operator.py +++ b/shade/tests/unit/test_shade_operator.py @@ -10,15 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneauth1 import plugin as ksa_plugin - -from distutils import version as du_version import mock import munch import testtools -import os_client_config as occ -from os_client_config import cloud_config +from openstack.config import cloud_region import shade from shade import exc from shade.tests import fakes @@ -55,13 +51,13 @@ class TestShadeOperator(base.RequestsMockTestCase): self.assertEqual('22', self.op_cloud.get_image_id('22')) self.assertEqual('22', self.op_cloud.get_image_id('22 name')) - @mock.patch.object(cloud_config.CloudConfig, 'get_endpoint') + @mock.patch.object(cloud_region.CloudRegion, 'get_endpoint') def test_get_session_endpoint_provided(self, fake_get_endpoint): fake_get_endpoint.return_value = 'http://fake.url' self.assertEqual( 'http://fake.url', self.op_cloud.get_session_endpoint('image')) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_get_session_endpoint_session(self, get_session_mock): session_mock = mock.Mock() session_mock.get_endpoint.return_value = 'http://fake.url' @@ -69,7 +65,7 @@ class TestShadeOperator(base.RequestsMockTestCase): self.assertEqual( 'http://fake.url', self.op_cloud.get_session_endpoint('image')) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_get_session_endpoint_exception(self, get_session_mock): class FakeException(Exception): pass @@ -87,7 +83,7 @@ class TestShadeOperator(base.RequestsMockTestCase): " No service"): self.op_cloud.get_session_endpoint("image") - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_get_session_endpoint_unavailable(self, get_session_mock): session_mock = mock.Mock() session_mock.get_endpoint.return_value = None @@ -95,32 +91,25 @@ class TestShadeOperator(base.RequestsMockTestCase): image_endpoint = self.op_cloud.get_session_endpoint("image") self.assertIsNone(image_endpoint) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_get_session_endpoint_identity(self, get_session_mock): session_mock = mock.Mock() get_session_mock.return_value = session_mock self.op_cloud.get_session_endpoint('identity') - # occ > 1.26.0 fixes keystoneclient construction. Unfortunately, it - # breaks our mocking of what keystoneclient does here. Since we're - # close to just getting rid of ksc anyway, just put in a version match - occ_version = du_version.StrictVersion(occ.__version__) - if occ_version > du_version.StrictVersion('1.26.0'): - kwargs = dict( - interface='public', region_name='RegionOne', - service_name=None, service_type='identity') - else: - kwargs = dict(interface=ksa_plugin.AUTH_INTERFACE) + kwargs = dict( + interface='public', region_name='RegionOne', + service_name=None, service_type='identity') session_mock.get_endpoint.assert_called_with(**kwargs) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_has_service_no(self, get_session_mock): session_mock = mock.Mock() session_mock.get_endpoint.return_value = None get_session_mock.return_value = session_mock self.assertFalse(self.op_cloud.has_service("image")) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') + @mock.patch.object(cloud_region.CloudRegion, 'get_session') def test_has_service_yes(self, get_session_mock): session_mock = mock.Mock() session_mock.get_endpoint.return_value = 'http://fake.url'