openstack
/
nova
Code Issues Proposed changes

Adapter raise_exc=False by default 

With the exception of a single spot in the code, every request we were
making through keystoneauth1 Adapter/Session was using raise_exc=False -
i.e. don't raise exceptions on >=400; we process the Response ourselves.

As of keystoneauth1 3.9.0 it is possible to pass a raise_exc kwarg into
the Adapter upon creation, setting that as the default for all requests
on that Adapter.  With this patch, we set raise_exc=False on all Adapter
creations and only override it in the one spot where we were actually
expecting and processing failures as exceptions.

Change-Id: I477afdfce746d405adf77134f57670563fd575e1
This commit is contained in:
Eric Fried 2018-06-22 09:40:33 -05:00
parent 68caa292ce
commit af4d8d3932
10 changed files with 51 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nova/api/openstack/identity.py
View File

@ -38,7 +38,7 @@ def verify_project_id(context, project_id):
explanation=_("Project ID %s is not a valid project.") %
project_id)
try:
resp = adap.get('/projects/%s' % project_id, raise_exc=False)
resp = adap.get('/projects/%s' % project_id)
except kse.EndpointNotFound:
LOG.error(
"Keystone identity service version 3.0 was not found. This might "

4
nova/api/openstack/placement/direct.py
View File

@ -69,12 +69,10 @@ class PlacementDirect(interceptor.RequestsInterceptor):
# TODO(efried): See below
if latest_microversion:
headers['OpenStack-API-Version'] = 'placement latest'
# TODO(efried): Set raise_exc globally when
# https://review.openstack.org/#/c/574784/ is released.
self.adapter = adapter.Adapter(
session.Session(auth=None, session=request_session,
additional_headers=headers),
service_type='placement')
service_type='placement', raise_exc=False)
# TODO(efried): Figure out why this isn't working:
# default_microversion='latest' if latest_microversion else None)
self._mocked_endpoint = mock.patch(

2
nova/cmd/status.py
View File

@ -193,7 +193,7 @@ class UpgradeCommands(object):
"""
client = utils.get_ksa_adapter('placement')
return client.get(path).json()
return client.get(path, raise_exc=True).json()
def _check_placement(self):
"""Checks to see if the placement API is ready for scheduling.

12
nova/scheduler/client/report.py
View File

@ -276,8 +276,7 @@ class SchedulerReportClient(object):
def get(self, url, version=None, global_request_id=None):
headers = ({request_id.INBOUND_HEADER: global_request_id}
if global_request_id else {})
return self._client.get(url, raise_exc=False, microversion=version,
headers=headers)
return self._client.get(url, microversion=version, headers=headers)
def post(self, url, data, version=None, global_request_id=None):
headers = ({request_id.INBOUND_HEADER: global_request_id}
@ -286,8 +285,8 @@ class SchedulerReportClient(object):
# media type to application/json for us. Placement API is
# more sensitive to this than other APIs in the OpenStack
# ecosystem.
return self._client.post(url, json=data, raise_exc=False,
microversion=version, headers=headers)
return self._client.post(url, json=data, microversion=version,
headers=headers)
def put(self, url, data, version=None, global_request_id=None):
# NOTE(sdague): using json= instead of data= sets the
@ -299,13 +298,12 @@ class SchedulerReportClient(object):
global_request_id} if global_request_id else {}}
if data is not None:
kwargs['json'] = data
return self._client.put(url, raise_exc=False, **kwargs)
return self._client.put(url, **kwargs)
def delete(self, url, version=None, global_request_id=None):
headers = ({request_id.INBOUND_HEADER: global_request_id}
if global_request_id else {})
return self._client.delete(url, raise_exc=False, microversion=version,
headers=headers)
return self._client.delete(url, microversion=version, headers=headers)
@safe_connect
def get_allocation_candidates(self, context, resources):

15
nova/tests/fixtures.py
View File

@ -25,6 +25,7 @@ import os
import warnings
import fixtures
from keystoneauth1 import adapter as ka
from keystoneauth1 import session as ks
import mock
from oslo_concurrency import lockutils
@ -1738,7 +1739,7 @@ class PlacementFixture(placement.PlacementFixture):
'keystoneauth1.session.TCPKeepAliveAdapter.init_poolmanager',
adapters.HTTPAdapter.init_poolmanager))
self._client = ks.Session(auth=None)
self._client = ka.Adapter(ks.Session(auth=None), raise_exc=False)
# NOTE(sbauza): We need to mock the scheduler report client because
# we need to fake Keystone by directly calling the endpoint instead
# of looking up the service catalog, like we did for the OSAPIFixture.
@ -1776,8 +1777,7 @@ class PlacementFixture(placement.PlacementFixture):
return self._client.get(
url,
endpoint_override=self.endpoint,
headers=headers,
raise_exc=False)
headers=headers)
def _fake_post(self, *args, **kwargs):
(url, data) = args[1:]
@ -1793,8 +1793,7 @@ class PlacementFixture(placement.PlacementFixture):
return self._client.post(
url, json=data,
endpoint_override=self.endpoint,
headers=headers,
raise_exc=False)
headers=headers)
def _fake_put(self, *args, **kwargs):
(url, data) = args[1:]
@ -1810,8 +1809,7 @@ class PlacementFixture(placement.PlacementFixture):
return self._client.put(
url, json=data,
endpoint_override=self.endpoint,
headers=headers,
raise_exc=False)
headers=headers)
def _fake_delete(self, *args, **kwargs):
(url,) = args[1:]
@ -1821,8 +1819,7 @@ class PlacementFixture(placement.PlacementFixture):
return self._client.delete(
url,
endpoint_override=self.endpoint,
headers={'x-auth-token': self.token},
raise_exc=False)
headers={'x-auth-token': self.token})
class UnHelperfulClientChannel(privsep_daemon._ClientChannel):

10
nova/tests/functional/api/openstack/placement/test_direct.py
View File

@ -66,10 +66,7 @@ class TestDirect(test.NoDBTestCase):
def test_json_validation_happens(self):
data = {'name': 'fake', 'cowsay': 'moo'}
with direct.PlacementDirect(CONF) as client:
# TODO(efried): Set raise_exc globally when
# https://review.openstack.org/#/c/574784/ is released.
resp = client.post('/resource_providers', json=data,
raise_exc=False)
resp = client.post('/resource_providers', json=data)
self.assertFalse(resp)
self.assertEqual(400, resp.status_code)
@ -84,13 +81,12 @@ class TestDirect(test.NoDBTestCase):
# attempt to create child
data = {'name': 'child', 'parent_provider_uuid': uuidsentinel.p_rp}
# no microversion, 400
resp = client.post('/resource_providers', json=data,
raise_exc=False)
resp = client.post('/resource_providers', json=data)
self.assertFalse(resp)
self.assertEqual(400, resp.status_code)
# low microversion, 400
resp = client.post('/resource_providers', json=data,
raise_exc=False, microversion='1.13')
microversion='1.13')
self.assertFalse(resp)
self.assertEqual(400, resp.status_code)
resp = client.post('/resource_providers', json=data,

61
nova/tests/unit/scheduler/client/test_report.py
View File

@ -382,7 +382,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=expected_payload,
raise_exc=False,
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertTrue(res)
@ -421,7 +420,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=expected_payload,
raise_exc=False,
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertTrue(res)
@ -489,7 +487,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=mock.ANY,
raise_exc=False,
headers={'X-Openstack-Request-Id': self.context.global_id})
# We have to pull the json body from the mock call_args to validate
# it separately otherwise hash seed issues get in the way.
@ -581,7 +578,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=mock.ANY,
raise_exc=False,
headers={'X-Openstack-Request-Id': self.context.global_id})
# We have to pull the allocations from the json body from the
# mock call_args to validate it separately otherwise hash seed
@ -657,7 +653,7 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['project_id'] = project_id
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=mock.ANY, raise_exc=False,
expected_url, microversion='1.12', json=mock.ANY,
headers={'X-Openstack-Request-Id': self.context.global_id})
# We have to pull the json body from the mock call_args to validate
# it separately otherwise hash seed issues get in the way.
@ -743,7 +739,7 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['project_id'] = project_id
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=mock.ANY, raise_exc=False,
expected_url, microversion='1.12', json=mock.ANY,
headers={'X-Openstack-Request-Id': self.context.global_id})
# We have to pull the json body from the mock call_args to validate
# it separately otherwise hash seed issues get in the way.
@ -797,7 +793,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
# identical since we're retrying the same HTTP request
expected_calls = [
mock.call(expected_url, microversion='1.12', json=expected_payload,
raise_exc=False,
headers={'X-Openstack-Request-Id':
self.context.global_id})] * 2
self.assertEqual(len(expected_calls),
@ -843,7 +838,6 @@ class TestPutAllocations(SchedulerReportClientTestCase):
expected_payload['user_id'] = user_id
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.12', json=expected_payload,
raise_exc=False,
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertFalse(res)
@ -911,7 +905,7 @@ class TestPutAllocations(SchedulerReportClientTestCase):
key=sort_by_uuid)
self.assertEqual(expected_allocations, actual_allocations)
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.10', json=mock.ANY, raise_exc=False,
expected_url, microversion='1.10', json=mock.ANY,
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertTrue(res)
@ -993,7 +987,7 @@ class TestPutAllocations(SchedulerReportClientTestCase):
key=sort_by_uuid)
self.assertEqual(expected_allocations, actual_allocations)
self.ks_adap_mock.put.assert_called_once_with(
expected_url, microversion='1.10', json=mock.ANY, raise_exc=False,
expected_url, microversion='1.10', json=mock.ANY,
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertTrue(res)
@ -1502,7 +1496,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/allocation_candidates?%s' % parse.urlencode(
expected_query)
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.25',
expected_url, microversion='1.25',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(mock.sentinel.alloc_reqs, alloc_reqs)
self.assertEqual(mock.sentinel.p_sums, p_sums)
@ -1542,7 +1536,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_query)
self.assertEqual(mock.sentinel.alloc_reqs, alloc_reqs)
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.25',
expected_url, microversion='1.25',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(mock.sentinel.p_sums, p_sums)
@ -1564,7 +1558,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
res = self.client.get_allocation_candidates(self.context, resources)
self.ks_adap_mock.get.assert_called_once_with(
mock.ANY, raise_exc=False, microversion='1.25',
mock.ANY, microversion='1.25',
headers={'X-Openstack-Request-Id': self.context.global_id})
url = self.ks_adap_mock.get.call_args[0][0]
split_url = parse.urlsplit(url)
@ -1597,7 +1591,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
)
expected_url = '/resource_providers/' + uuid
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.14',
expected_url, microversion='1.14',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(expected_provider_dict, result)
@ -1612,7 +1606,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/resource_providers/' + uuid
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.14',
expected_url, microversion='1.14',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertIsNone(result)
@ -1633,7 +1627,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/resource_providers/' + uuid
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.14',
expected_url, microversion='1.14',
headers={'X-Openstack-Request-Id': self.context.global_id})
# A 503 Service Unavailable should trigger an error log that
# includes the placement request id and return None
@ -1672,7 +1666,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
','.join((uuids.agg1, uuids.agg2)) +
'&required=MISC_SHARES_VIA_AGGREGATE')
self.ks_adap_mock.get.assert_called_once_with(
expected_url, microversion='1.18', raise_exc=False,
expected_url, microversion='1.18',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(rpjson, result)
@ -1698,7 +1692,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = ('/resource_providers?member_of=in:' + uuid +
'&required=MISC_SHARES_VIA_AGGREGATE')
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.18',
expected_url, microversion='1.18',
headers={'X-Openstack-Request-Id': self.context.global_id})
# A 503 Service Unavailable should trigger an error log that
# includes the placement request id
@ -1733,7 +1727,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/resource_providers?in_tree=' + root
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.14',
expected_url, microversion='1.14',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(rpjson, result)
@ -1753,7 +1747,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/resource_providers?in_tree=' + uuid
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.14',
expected_url, microversion='1.14',
headers={'X-Openstack-Request-Id': self.context.global_id})
# A 503 Service Unavailable should trigger an error log that includes
# the placement request id
@ -1781,8 +1775,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
expected_url = '/resource_providers'
self.ks_adap_mock.post.assert_called_once_with(
expected_url, json=expected_payload, raise_exc=False,
microversion='1.20',
expected_url, json=expected_payload, microversion='1.20',
headers={'X-Openstack-Request-Id': self.context.global_id})
def test_create_resource_provider_with_parent(self):
@ -1812,8 +1805,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
}
expected_url = '/resource_providers'
self.ks_adap_mock.post.assert_called_once_with(
expected_url, json=expected_payload, raise_exc=False,
microversion='1.20',
expected_url, json=expected_payload, microversion='1.20',
headers={'X-Openstack-Request-Id': self.context.global_id})
@mock.patch.object(report.LOG, 'info')
@ -1843,8 +1835,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
}
expected_url = '/resource_providers'
self.ks_adap_mock.post.assert_called_once_with(
expected_url, json=expected_payload, raise_exc=False,
microversion='1.20',
expected_url, json=expected_payload, microversion='1.20',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(mock.sentinel.get_rp, result)
# The 409 response will produce a message to the info log.
@ -1884,8 +1875,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
}
expected_url = '/resource_providers'
self.ks_adap_mock.post.assert_called_once_with(
expected_url, json=expected_payload, raise_exc=False,
microversion='1.20',
expected_url, json=expected_payload, microversion='1.20',
headers={'X-Openstack-Request-Id': self.context.global_id})
# A 503 Service Unavailable should log an error that
# includes the placement request id and
@ -1901,7 +1891,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
url = '/resource_providers/%s/aggregates' % uuids.foo
self.client.put(url, [])
self.ks_adap_mock.put.assert_called_once_with(
url, json=[], raise_exc=False, microversion=None, headers={})
url, json=[], microversion=None, headers={})
def test_delete_provider(self):
delete_mock = fake_requests.FakeResponse(None)
@ -1918,8 +1908,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.ks_adap_mock.delete.assert_called_once_with(
'/resource_providers/' + uuids.root,
headers={'X-Openstack-Request-Id': 'gri'}, microversion=None,
raise_exc=False)
headers={'X-Openstack-Request-Id': 'gri'}, microversion=None)
self.assertFalse(self.client._provider_tree.exists(uuids.root))
self.assertNotIn(uuids.root, self.client._association_refresh_time)
@ -1936,7 +1925,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.assertRaises(exc, self.client._delete_provider, uuids.root)
self.ks_adap_mock.delete.assert_called_once_with(
'/resource_providers/' + uuids.root, microversion=None,
headers={}, raise_exc=False)
headers={})
self.ks_adap_mock.delete.reset_mock()
@ -1957,7 +1946,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
self.ks_adap_mock.put.assert_called_once_with(
'/resource_providers/%s/aggregates' % uuids.rp, json=aggs,
raise_exc=False, microversion='1.1',
microversion='1.1',
headers={'X-Openstack-Request-Id': self.context.global_id})
# Cache was updated
self.assertEqual(set(aggs),
@ -1991,7 +1980,7 @@ class TestAggregates(SchedulerReportClientTestCase):
expected_url = '/resource_providers/' + uuid + '/aggregates'
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.1',
expected_url, microversion='1.1',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertEqual(set(aggs), result)
@ -2013,7 +2002,7 @@ class TestAggregates(SchedulerReportClientTestCase):
expected_url = '/resource_providers/' + uuid + '/aggregates'
self.ks_adap_mock.get.assert_called_once_with(
expected_url, raise_exc=False, microversion='1.1',
expected_url, microversion='1.1',
headers={'X-Openstack-Request-Id': self.context.global_id})
self.assertTrue(log_mock.called)
self.assertEqual(uuids.request_id,
@ -2023,7 +2012,7 @@ class TestAggregates(SchedulerReportClientTestCase):
class TestTraits(SchedulerReportClientTestCase):
trait_api_kwargs = {'raise_exc': False, 'microversion': '1.6'}
trait_api_kwargs = {'microversion': '1.6'}
def test_get_provider_traits_found(self):
uuid = uuids.compute_node

3
nova/tests/unit/test_identity.py
View File

@ -56,8 +56,7 @@ class IdentityValidationTest(test.NoDBTestCase):
self.mock_get_adap.assert_called_once_with(
'identity', ksa_auth=mock.ANY,
min_version=(3, 0), max_version=(3, 'latest'))
self.mock_adap.get.assert_called_once_with(
'/projects/foo', raise_exc=False)
self.mock_adap.get.assert_called_once_with('/projects/foo')
def test_good_id(self):
"""Test response 200.

6
nova/tests/unit/test_utils.py
View File

@ -1241,7 +1241,7 @@ class GetKSAAdapterTestCase(test.NoDBTestCase):
# load_adapter* called with what we passed in (and the right group)
self.load_adap.assert_called_once_with(
utils.CONF, 'glance', session='sess', auth='auth',
min_version='min', max_version='max')
min_version='min', max_version='max', raise_exc=False)
def test_auth_from_session(self):
self.sess.auth = 'auth'
@ -1255,7 +1255,7 @@ class GetKSAAdapterTestCase(test.NoDBTestCase):
# load_adapter* called with the auth from the session
self.load_adap.assert_called_once_with(
utils.CONF, 'ironic', session=self.sess, auth='auth',
min_version=None, max_version=None)
min_version=None, max_version=None, raise_exc=False)
def test_load_auth_and_session(self):
ret = utils.get_ksa_adapter('volumev3')
@ -1269,7 +1269,7 @@ class GetKSAAdapterTestCase(test.NoDBTestCase):
# load_adapter* called with the loaded auth & session
self.load_adap.assert_called_once_with(
utils.CONF, 'cinder', session=self.sess, auth=self.auth,
min_version=None, max_version=None)
min_version=None, max_version=None, raise_exc=False)
class GetEndpointTestCase(test.NoDBTestCase):

6
nova/utils.py
View File

@ -1175,6 +1175,10 @@ def get_ksa_adapter(service_type, ksa_auth=None, ksa_session=None,
used, ksa auth and/or session options may also be required, or the relevant
parameter supplied.
A raise_exc=False adapter is returned, meaning responses >=400 return the
Response object rather than raising an exception. This behavior can be
overridden on a per-request basis by setting raise_exc=True.
:param service_type: String name of the service type for which the Adapter
is to be constructed.
:param ksa_auth: A keystoneauth1 auth plugin. If not specified, we attempt
@ -1221,7 +1225,7 @@ def get_ksa_adapter(service_type, ksa_auth=None, ksa_session=None,
return ks_loading.load_adapter_from_conf_options(
CONF, confgrp, session=ksa_session, auth=ksa_auth,
min_version=min_version, max_version=max_version)
min_version=min_version, max_version=max_version, raise_exc=False)
def get_endpoint(ksa_adapter):