From 20918c04a13eefca2fa62f5bc2a5a6e719923e32 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Mon, 26 Mar 2018 14:54:52 -0500 Subject: [PATCH] Get rid of 406 paths in report client Since it is now [1] mandatory to upgrade placement before nova services, there is no need for fallback code of the form: resp = placement.call(url, version=NEWISH) if resp.status_code == 406: resp = placement.call(url, version=OLDER) This change set removes instances of this pattern from the scheduler report client. [1] https://review.openstack.org/556631 Change-Id: Ic0a8395399030ddaad93132ad0532dfb410596e0 --- nova/scheduler/client/report.py | 52 +++----------- .../unit/scheduler/client/test_report.py | 70 ------------------- 2 files changed, 8 insertions(+), 114 deletions(-) diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index d3ba15223e0b..30300b2d9f96 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -482,38 +482,16 @@ class SchedulerReportClient(object): return [] aggs = ','.join(agg_uuids) - url = "/resource_providers?member_of=in:" + aggs - # First try microversion 1.18 with the `required` queryparam. - resp = self.get( - url + '&required=' + os_traits.MISC_SHARES_VIA_AGGREGATE, - version='1.18', global_request_id=context.global_id) + url = "/resource_providers?member_of=in:%s&required=%s" % ( + aggs, os_traits.MISC_SHARES_VIA_AGGREGATE) + resp = self.get(url, version='1.18', + global_request_id=context.global_id) if resp.status_code == 200: return resp.json()['resource_providers'] - if resp.status_code == 406: - # TODO(efried): Remove this branch when placement minimum is 1.18 - # Fall back to less efficient algorithm - we have to get traits - # for every provider in the aggregate to filter out non-sharing. - resp = self.get( - url, version='1.3', global_request_id=context.global_id) - if resp.status_code == 200: - rps = [] - for rp in resp.json()['resource_providers']: - traits = self._get_provider_traits(context, rp['uuid']) - if os_traits.MISC_SHARES_VIA_AGGREGATE in traits: - rps.append(rp) - return rps - # In this error case, the word 'sharing' isn't appropriate. - msg = _("[%(placement_req_id)s] Failed to retrieve resource " - "providers associated with the following aggregates from " - "placement API: %(aggs)s. Got %(status_code)d: " - "%(err_text)s.") - else: - msg = _("[%(placement_req_id)s] Failed to retrieve sharing " - "resource providers associated with the following " - "aggregates from placement API: %(aggs)s. Got " - "%(status_code)d: %(err_text)s.") - + msg = _("[%(placement_req_id)s] Failed to retrieve sharing resource " + "providers associated with the following aggregates from " + "placement API: %(aggs)s. Got %(status_code)d: %(err_text)s.") args = { 'aggs': aggs, 'status_code': resp.status_code, @@ -583,15 +561,6 @@ class SchedulerReportClient(object): version=POST_RPS_RETURNS_PAYLOAD_API_VERSION, global_request_id=context.global_id) - # TODO(efried): Remove this block when minimum placement - # version always returns new provider payload. - if resp.status_code == 406: - # Bug #1746075 cont'd: Otherwise, use the "silent" version and - # retrieve the newly-created provider via GET. - resp = self.post( - url, payload, version=NESTED_PROVIDER_API_VERSION, - global_request_id=context.global_id) - placement_req_id = get_placement_request_id(resp) if resp: @@ -604,12 +573,7 @@ class SchedulerReportClient(object): 'placement_req_id': placement_req_id, } LOG.info(msg, args) - if resp.status_code == 200: - return resp.json() - else: - # TODO(efried): Remove this branch when minimum placement - # version always returns new provider payload. - return self._get_resource_provider(context, uuid) + return resp.json() # TODO(efried): Push error codes from placement, and use 'em. name_conflict = 'Conflicting resource provider name:' diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index 5bf3108bef51..8cf85ca2a6a5 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/nova/tests/unit/scheduler/client/test_report.py @@ -1703,52 +1703,6 @@ class TestProviderOperations(SchedulerReportClientTestCase): headers={'X-Openstack-Request-Id': self.context.global_id}) self.assertEqual(rpjson, result) - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - def test_get_sharing_providers_old(self, mock_get_traits): - # Run _get_sharing_providers() through the pre-1.18 code path - resp_mock = mock.Mock(status_code=200) - rpjson = [ - { - 'uuid': uuids.compute_node, - 'name': 'compute_host', - 'generation': 42, - 'parent_provider_uuid': None, - 'root_provider_uuid': None, - 'links': [], - }, - { - 'uuid': uuids.sharing, - 'name': 'storage_provider', - 'generation': 42, - 'parent_provider_uuid': None, - 'root_provider_uuid': None, - 'links': [], - }, - ] - resp_mock.json.return_value = {'resource_providers': rpjson} - # Simulate 1.18 not supported - self.ks_adap_mock.get.side_effect = ( - mock.Mock(status_code=406), resp_mock) - - mock_get_traits.side_effect = [ - set(['MISC_SHARES_VIA_AGGREGATE', 'CUSTOM_FOO']), - set(['CUSTOM_BAR']), - ] - result = self.client._get_sharing_providers( - self.context, [uuids.agg1, uuids.agg2]) - - expected_url2 = ('/resource_providers?member_of=in:' + - ','.join((uuids.agg1, uuids.agg2))) - expected_url1 = (expected_url2 + '&required=MISC_SHARES_VIA_AGGREGATE') - call_kwargs = dict( - raise_exc=False, - headers={'X-Openstack-Request-Id': self.context.global_id}) - self.ks_adap_mock.get.assert_has_calls(( - mock.call(expected_url1, microversion='1.18', **call_kwargs), - mock.call(expected_url2, microversion='1.3', **call_kwargs))) - self.assertEqual(rpjson[:1], result) - def test_get_sharing_providers_emptylist(self): self.assertEqual( [], self.client._get_sharing_providers(self.context, [])) @@ -1834,30 +1788,6 @@ class TestProviderOperations(SchedulerReportClientTestCase): self.assertEqual('req-' + uuids.request_id, logging_mock.call_args[0][1]['placement_req_id']) - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_resource_provider') - def test_create_resource_provider_legacy(self, mock_get): - """Test that _create_resource_provider() sends a dict of resource - provider information without a parent provider UUID. - """ - uuid = uuids.compute_node - name = 'computehost' - resp406_mock = mock.Mock(status_code=406) - resp201_mock = mock.Mock(status_code=201) - self.ks_adap_mock.post.side_effect = (resp406_mock, resp201_mock) - - self.assertEqual( - mock_get.return_value, - self.client._create_resource_provider(self.context, uuid, name)) - - self.ks_adap_mock.post.assert_has_calls([ - mock.call( - '/resource_providers', microversion=mver, - json={'uuid': uuid, 'name': name}, raise_exc=False, - headers={'X-Openstack-Request-Id': self.context.global_id}) - for mver in ('1.20', '1.14')]) - mock_get.assert_called_once_with(self.context, uuid) - def test_create_resource_provider(self): """Test that _create_resource_provider() sends a dict of resource provider information without a parent provider UUID.