pass endpoint interface to http client
The 'interface' argument was being ignored so that the HTTP client was always using the public endpoint for Ironic. This fixes it so that the 'interface' argument is taken into consideration. There's also no need to explicitly set the interface to 'publicURL' because that's already the default in keystoneauth. Change-Id: I610836e5038774621690aca88b2aee25670f0262 story: 2005118 task: 29802
This commit is contained in:
Guang Yee
Eric Fried
parent
fdba8ed994
commit
d6eea403cb
@ -100,14 +100,22 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
||||
session = session_loader.load_from_options(auth=auth_plugin,
|
||||
**session_opts)
|
||||
|
||||
# Make sure we also pass the endpoint interface to the HTTP client.
|
||||
# NOTE(gyee): we are supposed to be using valid_interfaces as interface
|
||||
# is deprecated.
|
||||
interface = kwargs.get('interface')
|
||||
|
||||
endpoint = kwargs.get('endpoint')
|
||||
if not endpoint:
|
||||
try:
|
||||
# endpoint will be used to get hostname
|
||||
# and port that will be used for API version caching.
|
||||
# NOTE(gyee): KSA defaults interface to 'public' if it is
|
||||
# empty or None so there's no need to set it to publicURL
|
||||
# explicitly.
|
||||
endpoint = session.get_endpoint(
|
||||
service_type=kwargs.get('service_type') or 'baremetal',
|
||||
interface=kwargs.get('interface') or 'publicURL',
|
||||
interface=interface,
|
||||
region_name=kwargs.get('region_name')
|
||||
)
|
||||
except Exception as e:
|
||||
@ -120,7 +128,8 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None,
|
||||
'max_retries': max_retries,
|
||||
'retry_interval': retry_interval,
|
||||
'session': session,
|
||||
'endpoint_override': endpoint
|
||||
'endpoint_override': endpoint,
|
||||
'interface': interface
|
||||
}
|
||||
|
||||
return Client(api_version, **ironicclient_kwargs)
|
||||
|
||||
@ -32,7 +32,8 @@ class ClientTest(utils.BaseTestCase):
|
||||
@mock.patch.object(kaloading, 'get_plugin_loader', autospec=True)
|
||||
def _test_get_client(self, mock_ks_loader, mock_ks_session,
|
||||
mock_retrieve_data, warn_mock, version=None,
|
||||
auth='password', warn_mock_call_count=0, **kwargs):
|
||||
auth='password', warn_mock_call_count=0,
|
||||
expected_interface=None, **kwargs):
|
||||
session = mock_ks_session.return_value.load_from_options.return_value
|
||||
session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123'
|
||||
|
||||
@ -60,7 +61,7 @@ class ClientTest(utils.BaseTestCase):
|
||||
if not {'endpoint', 'ironic_url'}.intersection(kwargs):
|
||||
session.get_endpoint.assert_called_once_with(
|
||||
service_type=kwargs.get('service_type') or 'baremetal',
|
||||
interface=kwargs.get('interface') or 'publicURL',
|
||||
interface=expected_interface,
|
||||
region_name=kwargs.get('region_name'))
|
||||
if 'os_ironic_api_version' in kwargs:
|
||||
# NOTE(TheJulia): This does not test the negotiation logic
|
||||
@ -76,6 +77,15 @@ class ClientTest(utils.BaseTestCase):
|
||||
port='6385')
|
||||
self.assertEqual(version or v1.DEFAULT_VER,
|
||||
client.http_client.os_ironic_api_version)
|
||||
|
||||
# make sure the interface is conveyed to the client
|
||||
if expected_interface is not None:
|
||||
self.assertEqual(expected_interface,
|
||||
client.http_client.interface)
|
||||
if kwargs.get('os_endpoint_type'):
|
||||
self.assertEqual(kwargs['os_endpoint_type'],
|
||||
client.http_client.interface)
|
||||
|
||||
return client
|
||||
|
||||
def test_get_client_only_ironic_url(self):
|
||||
@ -128,6 +138,32 @@ class ClientTest(utils.BaseTestCase):
|
||||
}
|
||||
self._test_get_client(warn_mock_call_count=4, **kwargs)
|
||||
|
||||
def test_get_client_and_endpoint_type(self):
|
||||
kwargs = {
|
||||
'os_project_name': 'PROJECT_NAME',
|
||||
'os_username': 'USERNAME',
|
||||
'os_password': 'PASSWORD',
|
||||
'os_auth_url': 'http://localhost:35357/v2.0',
|
||||
'os_auth_token': '',
|
||||
'os_service_type': '',
|
||||
'os_endpoint_type': 'adminURL'
|
||||
}
|
||||
self._test_get_client(warn_mock_call_count=5,
|
||||
expected_interface='adminURL', **kwargs)
|
||||
|
||||
def test_get_client_and_interface(self):
|
||||
kwargs = {
|
||||
'os_project_name': 'PROJECT_NAME',
|
||||
'os_username': 'USERNAME',
|
||||
'os_password': 'PASSWORD',
|
||||
'os_auth_url': 'http://localhost:35357/v2.0',
|
||||
'os_auth_token': '',
|
||||
'os_service_type': '',
|
||||
'interface': 'internal'
|
||||
}
|
||||
self._test_get_client(warn_mock_call_count=4,
|
||||
expected_interface='internal', **kwargs)
|
||||
|
||||
def test_get_client_with_region_no_auth_token(self):
|
||||
kwargs = {
|
||||
'os_project_name': 'PROJECT_NAME',
|
||||
@ -214,7 +250,7 @@ class ClientTest(utils.BaseTestCase):
|
||||
}
|
||||
iroclient.get_client('1', **kwargs)
|
||||
session.get_endpoint.assert_called_once_with(service_type='baremetal',
|
||||
interface='publicURL',
|
||||
interface=None,
|
||||
region_name=None)
|
||||
|
||||
def test_get_client_incorrect_session_passed(self):
|
||||
@ -229,7 +265,7 @@ class ClientTest(utils.BaseTestCase):
|
||||
@mock.patch.object(kaloading.session, 'Session', autospec=True)
|
||||
def _test_loader_arguments_passed_correctly(
|
||||
self, mock_ks_session, passed_kwargs, expected_kwargs,
|
||||
loader_class):
|
||||
loader_class, expected_interface=None):
|
||||
session = mock_ks_session.return_value.load_from_options.return_value
|
||||
session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123'
|
||||
|
||||
@ -246,9 +282,8 @@ class ClientTest(utils.BaseTestCase):
|
||||
auth=mock.ANY, **session_opts)
|
||||
if 'ironic_url' not in passed_kwargs:
|
||||
service_type = passed_kwargs.get('service_type') or 'baremetal'
|
||||
interface = passed_kwargs.get('interface') or 'publicURL'
|
||||
session.get_endpoint.assert_called_once_with(
|
||||
service_type=service_type, interface=interface,
|
||||
service_type=service_type, interface=expected_interface,
|
||||
region_name=passed_kwargs.get('region_name'))
|
||||
|
||||
def test_loader_arguments_admin_token(self):
|
||||
@ -282,6 +317,24 @@ class ClientTest(utils.BaseTestCase):
|
||||
loader_class=identity.Token
|
||||
)
|
||||
|
||||
def test_loader_arguments_interface(self):
|
||||
passed_kwargs = {
|
||||
'os_auth_url': 'http://localhost:35357/v3',
|
||||
'os_region_name': 'REGIONONE',
|
||||
'os_auth_token': 'USER_AUTH_TOKEN',
|
||||
'os_project_name': 'admin',
|
||||
'interface': 'internal'
|
||||
}
|
||||
expected_kwargs = {
|
||||
'auth_url': 'http://localhost:35357/v3',
|
||||
'project_name': 'admin',
|
||||
'token': 'USER_AUTH_TOKEN'
|
||||
}
|
||||
self._test_loader_arguments_passed_correctly(
|
||||
passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs,
|
||||
loader_class=identity.Token, expected_interface='internal'
|
||||
)
|
||||
|
||||
def test_loader_arguments_password_tenant_name(self):
|
||||
passed_kwargs = {
|
||||
'os_auth_url': 'http://localhost:35357/v3',
|
||||
@ -344,7 +397,8 @@ class ClientTest(utils.BaseTestCase):
|
||||
'max_retries': None,
|
||||
'retry_interval': None,
|
||||
'session': session,
|
||||
'endpoint_override': 'http://ironic.example.org:6385/'}
|
||||
'endpoint_override': 'http://ironic.example.org:6385/',
|
||||
'interface': None}
|
||||
)
|
||||
self.assertFalse(session.get_endpoint.called)
|
||||
|
||||
@ -368,7 +422,8 @@ class ClientTest(utils.BaseTestCase):
|
||||
'max_retries': None,
|
||||
'retry_interval': None,
|
||||
'session': session,
|
||||
'endpoint_override': session.get_endpoint.return_value}
|
||||
'endpoint_override': session.get_endpoint.return_value,
|
||||
'interface': None}
|
||||
)
|
||||
|
||||
@mock.patch.object(iroclient, 'Client', autospec=True)
|
||||
@ -385,7 +440,8 @@ class ClientTest(utils.BaseTestCase):
|
||||
'max_retries': None,
|
||||
'retry_interval': None,
|
||||
'session': session,
|
||||
'endpoint_override': session.get_endpoint.return_value}
|
||||
'endpoint_override': session.get_endpoint.return_value,
|
||||
'interface': None}
|
||||
)
|
||||
self.assertFalse(mock_ks_session.called)
|
||||
|
||||
|
||||
@ -0,0 +1,7 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
The ``interface`` argument was being ignored so that the HTTP client was
|
||||
always using the public endpoint for Ironic. This fixes it so that the
|
||||
``interface`` argument is taken into consideration. See
|
||||
`story 2005118 <https://storyboard.openstack.org/#!/story/2005118>`_.
|
||||
Loading…
Reference in New Issue
Block a user