Placement client: optional RP generations

The Placement API has the concept of resource provider generations to
enable the co-ordination of multiple API clients, so they can avoid
accidentally overwriting each other's updates.

However when we know that a single client writes certain resource
providers then the generation handling becomes an unnecessary extra
API round-trip. Ideally the Placement API would give us a chance to
express our ignorance about multi-client coordination (accepting that
a single client can overwrite its own previous updates).

Until the Placement API gives us such a feature we implement it in
the client lib, by making 'resource_provider_generation' parameters
optional.  This is inferior to the API doing the same because we may
still see GenerationConflict exceptions even when we said
method(..., resource_provider_generation=None).

Later changes for the minimum guaranteed bandwidth
feature need 'resource_provider_generation' to be
optional for 'update_resource_provider_traits' and
'update_resource_provider_inventories'. For the sake of consistency
we do the same to 'update_resource_provider_inventory'.

Change-Id: I4312c3f8819f7c5b74b68f6fa0507e42094eab79
Partial-Bug: #1578989
See-Also: https://review.openstack.org/502306 (nova spec)
See-Also: https://review.openstack.org/508149 (neutron spec)

neutron_lib/clients/placement.py

@@ -254,7 +254,7 @@ class PlacementAPIClient(object):
     @_check_placement_api_available
     def update_resource_provider_inventories(
             self, resource_provider_uuid, inventories,
-            resource_provider_generation):
+            resource_provider_generation=None):
         """Update resource provider inventories.
 
         :param resource_provider_uuid: UUID of the resource provider.
@@ -269,7 +269,7 @@ class PlacementAPIClient(object):
                             step_size(required):
                             }}
         :param resource_provider_generation: The generation of the resource
-                                             provider.
+                                             provider. Optional.
         :raises PlacementResourceProviderNotFound: if the resource provider
                                                    is not found.
         :raises PlacementResourceProviderGenerationConflict: if the generation
@@ -278,6 +278,9 @@ class PlacementAPIClient(object):
                                                              match with the
                                                              server side.
         """
+        if resource_provider_generation is None:
+            resource_provider_generation = self.get_resource_provider(
+                resource_provider_uuid=resource_provider_uuid)['generation']
         url = '/resource_providers/%s/inventories' % resource_provider_uuid
         body = {
             'resource_provider_generation': resource_provider_generation,
@@ -363,20 +366,23 @@ class PlacementAPIClient(object):
     @_check_placement_api_available
     def update_resource_provider_inventory(
             self, resource_provider_uuid, inventory, resource_class,
-            resource_provider_generation):
+            resource_provider_generation=None):
         """Update resource provider inventory.
 
         :param resource_provider_uuid: UUID of the resource provider.
         :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.
+                                             provider. Optional.
         :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.
         """
+        if resource_provider_generation is None:
+            resource_provider_generation = self.get_resource_provider(
+                resource_provider_uuid=resource_provider_uuid)['generation']
         url = '/resource_providers/%s/inventories/%s' % (
             resource_provider_uuid, resource_class)
         inventory['resource_provider_generation'] = \
@@ -460,19 +466,22 @@ class PlacementAPIClient(object):
     @_check_placement_api_available
     def update_resource_provider_traits(
             self, resource_provider_uuid, traits,
-            resource_provider_generation):
+            resource_provider_generation=None):
         """Update resource provider traits
 
         :param resource_provider_uuid: UUID of the resource provider for which
                                        to set the traits
         :param traits: a list of traits.
         :param resource_provider_generation: The generation of the resource
-                                             provider.
+                                             provider. Optional.
         :raises PlacementResourceProviderNotFound: If the resource provider
                                                    is not found.
         :raises PlacementTraitNotFound: If any of the specified traits are not
                                         valid.
         """
+        if resource_provider_generation is None:
+            resource_provider_generation = self.get_resource_provider(
+                resource_provider_uuid=resource_provider_uuid)['generation']
         url = '/resource_providers/%s/traits' % (resource_provider_uuid)
         body = {
             'resource_provider_generation': resource_provider_generation,

neutron_lib/tests/unit/clients/test_placement.py

@@ -142,7 +142,7 @@ class TestPlacementAPIClient(base.BaseTestCase):
             self.placement_api_client.list_resource_providers,
             in_tree='tree1_uuid')
 
-    def test_update_resource_provider_inventories(self):
+    def test_update_resource_provider_inventories_generation(self):
         expected_body = {
             'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
             'inventories': INVENTORY
@@ -153,6 +153,20 @@ class TestPlacementAPIClient(base.BaseTestCase):
             '/resource_providers/%s/inventories' % RESOURCE_PROVIDER_UUID,
             expected_body)
 
+    def test_update_resource_provider_inventories_no_generation(self):
+        expected_body = {
+            'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
+            'inventories': INVENTORY
+        }
+        with mock.patch.object(
+                self.placement_api_client, 'get_resource_provider',
+                return_value={'generation': RESOURCE_PROVIDER_GENERATION}):
+            self.placement_api_client.update_resource_provider_inventories(
+                RESOURCE_PROVIDER_UUID, INVENTORY)
+        self.placement_fixture.mock_put.assert_called_once_with(
+            '/resource_providers/%s/inventories' % RESOURCE_PROVIDER_UUID,
+            expected_body)
+
     def test_update_resource_provider_inventories_no_rp(self):
         self.placement_fixture.mock_put.side_effect = ks_exc.NotFound()
 
@@ -214,7 +228,7 @@ class TestPlacementAPIClient(base.BaseTestCase):
                           self.placement_api_client.get_inventory,
                           RESOURCE_PROVIDER_UUID, RESOURCE_CLASS_NAME)
 
-    def test_update_resource_provider_inventory(self):
+    def test_update_resource_provider_inventory_generation(self):
         expected_body = {
             'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
         }
@@ -228,6 +242,22 @@ class TestPlacementAPIClient(base.BaseTestCase):
              'rc_name': RESOURCE_CLASS_NAME},
             expected_body)
 
+    def test_update_resource_provider_inventory_no_generation(self):
+        expected_body = {
+            'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
+        }
+        expected_body.update(INVENTORY)
+        with mock.patch.object(
+                self.placement_api_client, 'get_resource_provider',
+                return_value={'generation': RESOURCE_PROVIDER_GENERATION}):
+            self.placement_api_client.update_resource_provider_inventory(
+                RESOURCE_PROVIDER_UUID, INVENTORY, RESOURCE_CLASS_NAME)
+        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},
+            expected_body)
+
     def test_update_resource_inventory_inventory_conflict_exception(self):
         self.placement_fixture.mock_put.side_effect = ks_exc.Conflict()
         self.assertRaises(
@@ -286,14 +316,29 @@ class TestPlacementAPIClient(base.BaseTestCase):
         self.placement_fixture.mock_put.assert_called_once_with(
             '/traits/%s' % TRAIT_NAME, None)
 
-    def test_update_resource_provider_traits(self):
+    def test_update_resource_provider_traits_generation(self):
         traits = [TRAIT_NAME]
         self.placement_api_client.update_resource_provider_traits(
-            RESOURCE_PROVIDER_UUID, traits, resource_provider_generation=0)
+            RESOURCE_PROVIDER_UUID, traits,
+            resource_provider_generation=RESOURCE_PROVIDER_GENERATION)
         self.placement_fixture.mock_put.assert_called_once_with(
             '/resource_providers/%(rp_uuid)s/traits' %
             {'rp_uuid': RESOURCE_PROVIDER_UUID},
-            {'resource_provider_generation': 0, 'traits': traits})
+            {'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
+             'traits': traits})
+
+    def test_update_resource_provider_traits_no_generation(self):
+        traits = [TRAIT_NAME]
+        with mock.patch.object(
+                self.placement_api_client, 'get_resource_provider',
+                return_value={'generation': RESOURCE_PROVIDER_GENERATION}):
+            self.placement_api_client.update_resource_provider_traits(
+                RESOURCE_PROVIDER_UUID, traits)
+        self.placement_fixture.mock_put.assert_called_once_with(
+            '/resource_providers/%(rp_uuid)s/traits' %
+            {'rp_uuid': RESOURCE_PROVIDER_UUID},
+            {'resource_provider_generation': RESOURCE_PROVIDER_GENERATION,
+             'traits': traits})
 
     def test_update_resource_provider_traits_no_rp(self):
         traits = [TRAIT_NAME]

releasenotes/notes/placement-client-optional-rp-generations-44d1f1055d5496be.yaml

@@ -0,0 +1,12 @@
+---
+other:
+  - |
+    The ``resource_provider_generation`` parameters of the following
+    methods of ``PlacementAPIClient`` are now optional:
+    ``update_resource_provider_inventories``,
+    ``update_resource_provider_inventory`` and
+    ``update_resource_provider_traits``.
+    You may call the methods without this parameter or pass ``None``
+    with the meaning to ignore resource provider generations. That is the
+    client will (in quick succession) get the object and update it supplying
+    the same generation.