From 74c62da3b5076015563b9f8fe269818546db66cb Mon Sep 17 00:00:00 2001 From: Bence Romsics Date: Fri, 6 Jul 2018 15:25:06 +0200 Subject: [PATCH] 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 | 21 +++++-- .../tests/unit/clients/test_placement.py | 55 +++++++++++++++++-- ...ional-rp-generations-44d1f1055d5496be.yaml | 12 ++++ 3 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/placement-client-optional-rp-generations-44d1f1055d5496be.yaml diff --git a/neutron_lib/clients/placement.py b/neutron_lib/clients/placement.py index 5e36119e6..e50a3bbfe 100644 --- a/neutron_lib/clients/placement.py +++ b/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, diff --git a/neutron_lib/tests/unit/clients/test_placement.py b/neutron_lib/tests/unit/clients/test_placement.py index 182209de9..42bfae5dd 100644 --- a/neutron_lib/tests/unit/clients/test_placement.py +++ b/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] diff --git a/releasenotes/notes/placement-client-optional-rp-generations-44d1f1055d5496be.yaml b/releasenotes/notes/placement-client-optional-rp-generations-44d1f1055d5496be.yaml new file mode 100644 index 000000000..a6dad4d31 --- /dev/null +++ b/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.