From f0a9959a7fd98b091a17a29544eacdbd6dd37337 Mon Sep 17 00:00:00 2001 From: Lajos Katona Date: Tue, 20 Mar 2018 14:00:29 +0100 Subject: [PATCH] Fix placement calls in placement client Based on the placement api-ref fix the already implemented placement API calls in the placement client of neutron-lib. Fixed/changed parts: - Add version header to create_resource_provider - Change create_inventory to update_resource_provider_inventories, as the http request is a PUT not a POST, and add exception to handle the case when the rp is not found. - Change update_inventory to update_resource_provider_inventory, to make the method name in sync with placement API ref. - Fix the failing unit tests, and cover the negative case for update_resource_provider_inventories. Change-Id: I336c57e734f91795038c5ba9074970d1f5522da2 Related-Bug: #1578989 --- neutron_lib/clients/placement.py | 77 ++++++++++++---- neutron_lib/exceptions/placement.py | 10 +++ .../tests/unit/clients/test_placement.py | 88 +++++++++++++------ neutron_lib/tests/unit/test_fixture.py | 4 +- ..._client_method_names_b26bb71425f42db3.yaml | 19 ++++ 5 files changed, 150 insertions(+), 48 deletions(-) create mode 100644 releasenotes/notes/change_placement_client_method_names_b26bb71425f42db3.yaml diff --git a/neutron_lib/clients/placement.py b/neutron_lib/clients/placement.py index 4dee2cdd6..1a15809db 100644 --- a/neutron_lib/clients/placement.py +++ b/neutron_lib/clients/placement.py @@ -30,6 +30,7 @@ LOG = logging.getLogger(__name__) API_VERSION_REQUEST_HEADER = 'OpenStack-API-Version' PLACEMENT_API_WITH_MEMBER_OF = 'placement 1.3' PLACEMENT_API_WITH_NESTED_RESOURCES = 'placement 1.14' +PLACEMENT_API_LATEST_SUPPORTED = PLACEMENT_API_WITH_NESTED_RESOURCES def _check_placement_api_available(f): @@ -71,7 +72,7 @@ class PlacementAPIClient(object): """Client class for placement ReST API.""" def __init__(self, conf, - openstack_api_version=PLACEMENT_API_WITH_NESTED_RESOURCES): + openstack_api_version=PLACEMENT_API_LATEST_SUPPORTED): self._openstack_api_version = openstack_api_version self._target_version = _get_version(openstack_api_version) self._conf = conf @@ -115,7 +116,9 @@ class PlacementAPIClient(object): (required) and the uuid (required). """ url = '/resource_providers' - self._post(url, resource_provider) + self._post(url, resource_provider, + headers={API_VERSION_REQUEST_HEADER: + self._openstack_api_version}) @_check_placement_api_available def delete_resource_provider(self, resource_provider_uuid): @@ -193,17 +196,46 @@ class PlacementAPIClient(object): **filters).json() @_check_placement_api_available - def create_inventory(self, resource_provider_uuid, inventory): - """Create an inventory. + def update_resource_provider_inventories( + self, resource_provider_uuid, inventories, + resource_provider_generation): + """Update resource provider inventories. :param resource_provider_uuid: UUID of the resource provider. - :param inventory: The inventory. A dict with resource_class (required), - total (required), reserved (required), min_unit - (required), max_unit (required), step_size - (required) and allocation_ratio (required). + :param inventories: The inventories. A dict in the format (see: + Placement API ref: https://goo.gl/F22mtk) + {resource_class(required): + {allocation_ratio(required): + total(required): + max_unit(required): + min_unit(required): + reserved(required): + step_size(required): + }} + :param resource_provider_generation: The generation of the resource + provider. + :raises PlacementResourceProviderNotFound: if the resource provider + is not found. + :raises PlacementResourceProviderGenerationConflict: if the generation + of the resource + provider does not + match with the + server side. """ url = '/resource_providers/%s/inventories' % resource_provider_uuid - self._post(url, inventory) + body = { + 'resource_provider_generation': resource_provider_generation, + 'inventories': inventories + } + try: + return self._put(url, body).json() + except ks_exc.NotFound: + raise n_exc.PlacementResourceProviderNotFound( + resource_provider=resource_provider_uuid) + except ks_exc.Conflict: + raise n_exc.PlacementResourceProviderGenerationConflict( + resource_provider=resource_provider_uuid, + generation=resource_provider_generation) @_check_placement_api_available def get_inventory(self, resource_provider_uuid, resource_class): @@ -231,21 +263,30 @@ class PlacementAPIClient(object): raise @_check_placement_api_available - def update_inventory(self, resource_provider_uuid, inventory, - resource_class): - """Update an inventory. + def update_resource_provider_inventory( + self, resource_provider_uuid, inventory, resource_class, + resource_provider_generation): + """Update resource provider inventory. :param resource_provider_uuid: UUID of the resource provider. - :param inventory: The inventory, in a dictionary. - :param resource_class: The resource class of the inventory to update. - :raises PlacementInventoryUpdateConflict: For failure to update - inventory due to outdated - resource_provider_generation. + :param inventory: The inventory to be updated for the resource class. + :param resource_class: The name of the resource class. + :param resource_provider_generation: The generation of the resource + provider. + :raises PlacementResourceNotFound: If the resource provider or the + resource class is not found. + :raises PlacementInventoryUpdateConflict: If the resource provider + generation does not match + with the server side. """ url = '/resource_providers/%s/inventories/%s' % ( resource_provider_uuid, resource_class) + inventory['resource_provider_generation'] = \ + resource_provider_generation try: - self._put(url, inventory) + return self._put(url, inventory).json() + except ks_exc.NotFound as e: + raise n_exc.PlacementResourceNotFound(url=e.url) except ks_exc.Conflict: raise n_exc.PlacementInventoryUpdateConflict( resource_provider=resource_provider_uuid, diff --git a/neutron_lib/exceptions/placement.py b/neutron_lib/exceptions/placement.py index 05acc3b6e..f1989c963 100644 --- a/neutron_lib/exceptions/placement.py +++ b/neutron_lib/exceptions/placement.py @@ -20,10 +20,20 @@ class PlacementEndpointNotFound(exceptions.NotFound): message = _("Placement API endpoint not found.") +class PlacementResourceNotFound(exceptions.NotFound): + message = _("Placement resource not found on url: %(url)s.") + + class PlacementResourceProviderNotFound(exceptions.NotFound): message = _("Placement resource provider not found %(resource_provider)s.") +class PlacementResourceProviderGenerationConflict(exceptions.Conflict): + message = _("Placement resource provider generation does not match with " + "the server side for resource provider: " + "%(resource_provider)s with generation %(generation)s.") + + class PlacementInventoryNotFound(exceptions.NotFound): message = _("Placement inventory not found for resource provider " "%(resource_provider)s, resource class %(resource_class)s.") diff --git a/neutron_lib/tests/unit/clients/test_placement.py b/neutron_lib/tests/unit/clients/test_placement.py index b31f306ae..55dac9f69 100644 --- a/neutron_lib/tests/unit/clients/test_placement.py +++ b/neutron_lib/tests/unit/clients/test_placement.py @@ -24,7 +24,13 @@ from neutron_lib.tests import _base as base RESOURCE_PROVIDER_UUID = uuidutils.generate_uuid() +RESOURCE_PROVIDER_GENERATION = 1 RESOURCE_CLASS_NAME = 'resource_class_name' +INVENTORY = { + RESOURCE_CLASS_NAME: { + 'total': 42 + } +} class TestPlacementAPIClient(base.BaseTestCase): @@ -34,17 +40,20 @@ class TestPlacementAPIClient(base.BaseTestCase): config = mock.Mock() config.region_name = 'region_name' self.openstack_api_version = ( - placement.PLACEMENT_API_WITH_NESTED_RESOURCES) + placement.PLACEMENT_API_LATEST_SUPPORTED) self.placement_api_client = placement.PlacementAPIClient( config, self.openstack_api_version) self.placement_fixture = self.useFixture( fixture.PlacementAPIClientFixture(self.placement_api_client)) + self.headers = {'OpenStack-API-Version': self.openstack_api_version} def test_create_resource_provider(self): self.placement_api_client.create_resource_provider( RESOURCE_PROVIDER_UUID) self.placement_fixture.mock_post.assert_called_once_with( - '/resource_providers', RESOURCE_PROVIDER_UUID) + '/resource_providers', + RESOURCE_PROVIDER_UUID, + headers=self.headers) def test_delete_resource_provider(self): self.placement_api_client.delete_resource_provider( @@ -53,11 +62,10 @@ class TestPlacementAPIClient(base.BaseTestCase): '/resource_providers/%s' % RESOURCE_PROVIDER_UUID) def test_get_resource_provider(self): - headers = {'OpenStack-API-Version': self.openstack_api_version} self.placement_api_client.get_resource_provider(RESOURCE_PROVIDER_UUID) self.placement_fixture.mock_get.assert_called_once_with( '/resource_providers/%s' % RESOURCE_PROVIDER_UUID, - headers=headers) + headers=self.headers) def test_get_resource_provider_no_resource_provider(self): self.placement_fixture.mock_get.side_effect = ks_exc.NotFound() @@ -66,24 +74,23 @@ class TestPlacementAPIClient(base.BaseTestCase): RESOURCE_PROVIDER_UUID) def test_list_resource_providers(self): - headers = {'OpenStack-API-Version': self.openstack_api_version} filter_1 = {'name': 'name1', 'in_tree': 'tree1_uuid'} self.placement_api_client.list_resource_providers(**filter_1) self.placement_fixture.mock_get.assert_called_once_with( - '/resource_providers', headers=headers, **filter_1) + '/resource_providers', headers=self.headers, **filter_1) filter_2 = {'member_of': ['aggregate_uuid'], 'uuid': 'uuid_1', 'resources': {'r_class1': 'value1'}} self.placement_fixture.mock_get.reset_mock() self.placement_api_client.list_resource_providers(**filter_2) self.placement_fixture.mock_get.assert_called_once_with( - '/resource_providers', headers=headers, **filter_2) + '/resource_providers', headers=self.headers, **filter_2) filter_1.update(filter_2) self.placement_fixture.mock_get.reset_mock() self.placement_api_client.list_resource_providers(**filter_1) self.placement_fixture.mock_get.assert_called_once_with( - '/resource_providers', headers=headers, **filter_1) + '/resource_providers', headers=self.headers, **filter_1) def test_list_resource_providers_placement_api_version_too_low(self): self.placement_api_client._target_version = (1, 1) @@ -96,13 +103,24 @@ class TestPlacementAPIClient(base.BaseTestCase): self.placement_api_client.list_resource_providers, in_tree='tree1_uuid') - def test_create_inventory(self): - inventory = mock.ANY - self.placement_api_client.create_inventory(RESOURCE_PROVIDER_UUID, - inventory) - self.placement_fixture.mock_post.assert_called_once_with( + def test_update_resource_provider_inventories(self): + expected_body = { + 'resource_provider_generation': RESOURCE_PROVIDER_GENERATION, + 'inventories': INVENTORY + } + self.placement_api_client.update_resource_provider_inventories( + RESOURCE_PROVIDER_UUID, INVENTORY, RESOURCE_PROVIDER_GENERATION) + self.placement_fixture.mock_put.assert_called_once_with( '/resource_providers/%s/inventories' % RESOURCE_PROVIDER_UUID, - inventory) + expected_body) + + def test_update_resource_provider_inventories_no_rp(self): + self.placement_fixture.mock_put.side_effect = ks_exc.NotFound() + + self.assertRaises( + n_exc.PlacementResourceProviderNotFound, + self.placement_api_client.update_resource_provider_inventories, + RESOURCE_PROVIDER_UUID, INVENTORY, RESOURCE_PROVIDER_GENERATION) def test_get_inventory(self): self.placement_api_client.get_inventory(RESOURCE_PROVIDER_UUID, @@ -136,38 +154,50 @@ class TestPlacementAPIClient(base.BaseTestCase): self.placement_api_client.get_inventory, RESOURCE_PROVIDER_UUID, RESOURCE_CLASS_NAME) - def test_update_inventory(self): - inventory = mock.ANY - self.placement_api_client.update_inventory( - RESOURCE_PROVIDER_UUID, inventory, RESOURCE_CLASS_NAME) + def test_update_resource_provider_inventory(self): + expected_body = { + 'resource_provider_generation': RESOURCE_PROVIDER_GENERATION, + } + expected_body.update(INVENTORY) + self.placement_api_client.update_resource_provider_inventory( + RESOURCE_PROVIDER_UUID, INVENTORY, RESOURCE_CLASS_NAME, + resource_provider_generation=1) self.placement_fixture.mock_put.assert_called_once_with( '/resource_providers/%(rp_uuid)s/inventories/%(rc_name)s' % {'rp_uuid': RESOURCE_PROVIDER_UUID, 'rc_name': RESOURCE_CLASS_NAME}, - inventory) + expected_body) - def test_update_inventory_conflict_exception(self): - inventory = mock.ANY + def test_update_resource_inventory_inventory_conflict_exception(self): self.placement_fixture.mock_put.side_effect = ks_exc.Conflict() - self.assertRaises(n_exc.PlacementInventoryUpdateConflict, - self.placement_api_client.update_inventory, - RESOURCE_PROVIDER_UUID, inventory, - RESOURCE_CLASS_NAME) + self.assertRaises( + n_exc.PlacementInventoryUpdateConflict, + self.placement_api_client.update_resource_provider_inventory, + RESOURCE_PROVIDER_UUID, INVENTORY, + RESOURCE_CLASS_NAME, resource_provider_generation=1) + + def test_update_resource_provider_inventory_not_found(self): + # Test the resource provider not found case + self.placement_fixture.mock_put.side_effect = ks_exc.NotFound( + details="No resource provider with uuid") + self.assertRaises( + n_exc.PlacementResourceNotFound, + self.placement_api_client.update_resource_provider_inventory, + RESOURCE_PROVIDER_UUID, INVENTORY, + RESOURCE_CLASS_NAME, RESOURCE_PROVIDER_GENERATION) def test_associate_aggregates(self): - headers = {'OpenStack-API-Version': self.openstack_api_version} self.placement_api_client.associate_aggregates(RESOURCE_PROVIDER_UUID, mock.ANY) self.placement_fixture.mock_put.assert_called_once_with( '/resource_providers/%s/aggregates' % RESOURCE_PROVIDER_UUID, - mock.ANY, headers=headers) + mock.ANY, headers=self.headers) def test_list_aggregates(self): - headers = {'OpenStack-API-Version': self.openstack_api_version} self.placement_api_client.list_aggregates(RESOURCE_PROVIDER_UUID) self.placement_fixture.mock_get.assert_called_once_with( '/resource_providers/%s/aggregates' % RESOURCE_PROVIDER_UUID, - headers=headers) + headers=self.headers) def test_list_aggregates_no_resource_provider(self): self.placement_fixture.mock_get.side_effect = ks_exc.NotFound() diff --git a/neutron_lib/tests/unit/test_fixture.py b/neutron_lib/tests/unit/test_fixture.py index 69ba987cf..e4ca0b74e 100644 --- a/neutron_lib/tests/unit/test_fixture.py +++ b/neutron_lib/tests/unit/test_fixture.py @@ -139,8 +139,10 @@ class PlacementAPIClientFixtureTestCase(base.BaseTestCase): p_fixture.mock_post.assert_called_once() def test_put(self): + inventory = {'total': 42} p_client, p_fixture = self._create_client_and_fixture() - p_client.update_inventory('resource', mock.ANY, 'class_name') + p_client.update_resource_provider_inventory('resource', inventory, + 'class_name', 1) p_fixture.mock_put.assert_called_once() def test_delete(self): diff --git a/releasenotes/notes/change_placement_client_method_names_b26bb71425f42db3.yaml b/releasenotes/notes/change_placement_client_method_names_b26bb71425f42db3.yaml new file mode 100644 index 000000000..e9dafa57e --- /dev/null +++ b/releasenotes/notes/change_placement_client_method_names_b26bb71425f42db3.yaml @@ -0,0 +1,19 @@ +--- +prelude: > + Change create_inventory in placement client to + update_resource_provider_inventories and update_inventory to + update_resource_provider_inventory +issues: + - Placement API has no POST method for creating resource provider inventories + but instead has PUT to update the inventories of a resource provider. + - Placement API has method to update the inventory for a given + resource_provider. +fixes: + - Change the method name create_inventory in clients/placement.py to + update_resource_provider_inventories as that represents what is on the + placement side. + - Change the POST call to /resource_providers/{uuid}/inventories to + PUT. + - Change the method name update_inventory in clients/placement.py to + update_resource_provider_inventory as that represents that the method + updates the inventory of a resource_provider.