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

@@ -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):
+    def update_resource_provider_inventories(
-        """Create an inventory.
+            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),
+        :param inventories: The inventories. A dict in the format (see:
-                          total (required), reserved (required), min_unit
+                            Placement API ref: https://goo.gl/F22mtk)
-                          (required), max_unit (required), step_size
+                            {resource_class(required):
-                          (required) and allocation_ratio (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,
+    def update_resource_provider_inventory(
-                         resource_class):
+            self, resource_provider_uuid, inventory, resource_class,
-        """Update an inventory.
+            resource_provider_generation):
+        """Update resource provider inventory.
 
         :param resource_provider_uuid: UUID of the resource provider.
-        :param inventory: The inventory, in a dictionary.
+        :param inventory: The inventory to be updated for the resource class.
-        :param resource_class: The resource class of the inventory to update.
+        :param resource_class: The name of the resource class.
-        :raises PlacementInventoryUpdateConflict: For failure to update
+        :param resource_provider_generation: The generation of the resource
-                                                  inventory due to outdated
+                                             provider.
-                                                  resource_provider_generation.
+        :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,

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.")

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):
+    def test_update_resource_provider_inventories(self):
-        inventory = mock.ANY
+        expected_body = {
-        self.placement_api_client.create_inventory(RESOURCE_PROVIDER_UUID,
+            'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
-                                                   inventory)
+            'inventories': INVENTORY
-        self.placement_fixture.mock_post.assert_called_once_with(
+        }
+        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):
+    def test_update_resource_provider_inventory(self):
-        inventory = mock.ANY
+        expected_body = {
-        self.placement_api_client.update_inventory(
+            'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
-            RESOURCE_PROVIDER_UUID, inventory, RESOURCE_CLASS_NAME)
+        }
+        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):
+    def test_update_resource_inventory_inventory_conflict_exception(self):
-        inventory = mock.ANY
         self.placement_fixture.mock_put.side_effect = ks_exc.Conflict()
-        self.assertRaises(n_exc.PlacementInventoryUpdateConflict,
+        self.assertRaises(
-                          self.placement_api_client.update_inventory,
+            n_exc.PlacementInventoryUpdateConflict,
-                          RESOURCE_PROVIDER_UUID, inventory,
+            self.placement_api_client.update_resource_provider_inventory,
-                          RESOURCE_CLASS_NAME)
+            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()

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):

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.