From 3216f078d481d2b48b90b63c2d93082c86b9809d Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Tue, 27 Feb 2018 12:29:11 +0000 Subject: [PATCH] placement: generation in provider aggregate APIs Placement API microversion 1.19 enhances the payloads for the `GET /resource_providers/{uuid}/aggregates` response and the `PUT /resource_providers/{uuid}/aggregates` request and response to be identical, and to include the ``resource_provider_generation``. As with other generation-aware APIs, if the ``resource_provider_generation`` specified in the `PUT` request does not match the generation known by the server, a 409 Conflict error is returned. Change-Id: I86416e35da1798cdf039b42c9ed7629f0f9c75fc blueprint: placement-aggregate-generation --- .../openstack/placement/handlers/aggregate.py | 44 +++++++++-- nova/api/openstack/placement/microversion.py | 2 + .../placement/objects/resource_provider.py | 16 +++- .../placement/rest_api_version_history.rst | 10 +++ .../openstack/placement/schemas/aggregate.py | 22 +++++- .../placement/gabbits/aggregate.yaml | 76 ++++++++++++++----- .../placement/gabbits/microversion.yaml | 4 +- .../gabbits/resource-provider-aggregates.yaml | 16 +++- .../resource-provider-resources-query.yaml | 4 +- .../placement/gabbits/shared-resources.yaml | 10 ++- placement-api-ref/source/aggregates.inc | 61 +++++++++++---- placement-api-ref/source/parameters.yaml | 3 + .../aggregates/get-aggregates-1.19.json | 7 ++ .../aggregates/update-aggregates-1.19.json | 7 ++ .../update-aggregates-request-1.19.json | 7 ++ ...aggregate-generation-9dad79fb0356fcc0.yaml | 10 +++ 16 files changed, 247 insertions(+), 52 deletions(-) create mode 100644 placement-api-ref/source/samples/aggregates/get-aggregates-1.19.json create mode 100644 placement-api-ref/source/samples/aggregates/update-aggregates-1.19.json create mode 100644 placement-api-ref/source/samples/aggregates/update-aggregates-request-1.19.json create mode 100644 releasenotes/notes/placement-aggregate-generation-9dad79fb0356fcc0.yaml diff --git a/nova/api/openstack/placement/handlers/aggregate.py b/nova/api/openstack/placement/handlers/aggregate.py index fdd73cb52d1e..04764fd35482 100644 --- a/nova/api/openstack/placement/handlers/aggregate.py +++ b/nova/api/openstack/placement/handlers/aggregate.py @@ -11,23 +11,33 @@ # under the License. """Aggregate handlers for Placement API.""" +from oslo_db import exception as db_exc from oslo_serialization import jsonutils from oslo_utils import encodeutils from oslo_utils import timeutils +import webob from nova.api.openstack.placement import microversion from nova.api.openstack.placement.objects import resource_provider as rp_obj from nova.api.openstack.placement.schemas import aggregate as schema from nova.api.openstack.placement import util from nova.api.openstack.placement import wsgi_wrapper +from nova import exception +from nova.i18n import _ -def _send_aggregates(req, aggregate_uuids): +_INCLUDE_GENERATION_VERSION = (1, 19) + + +def _send_aggregates(req, resource_provider, aggregate_uuids): want_version = req.environ[microversion.MICROVERSION_ENVIRON] response = req.response response.status = 200 + payload = _serialize_aggregates(aggregate_uuids) + if want_version.matches(min_version=_INCLUDE_GENERATION_VERSION): + payload['resource_provider_generation'] = resource_provider.generation response.body = encodeutils.to_utf8( - jsonutils.dumps(_serialize_aggregates(aggregate_uuids))) + jsonutils.dumps(payload)) response.content_type = 'application/json' if want_version.matches((1, 15)): req.response.cache_control = 'no-cache' @@ -60,7 +70,7 @@ def get_aggregates(req): context, uuid) aggregate_uuids = resource_provider.get_aggregates() - return _send_aggregates(req, aggregate_uuids) + return _send_aggregates(req, resource_provider, aggregate_uuids) @wsgi_wrapper.PlacementWsgify @@ -68,10 +78,32 @@ def get_aggregates(req): @microversion.version_handler('1.1') def set_aggregates(req): context = req.environ['placement.context'] + want_version = req.environ[microversion.MICROVERSION_ENVIRON] + consider_generation = want_version.matches( + min_version=_INCLUDE_GENERATION_VERSION) + put_schema = schema.PUT_AGGREGATES_SCHEMA_V1_1 + if consider_generation: + put_schema = schema.PUT_AGGREGATES_SCHEMA_V1_19 uuid = util.wsgi_path_item(req.environ, 'uuid') resource_provider = rp_obj.ResourceProvider.get_by_uuid( context, uuid) - aggregate_uuids = util.extract_json(req.body, schema.PUT_AGGREGATES_SCHEMA) - resource_provider.set_aggregates(aggregate_uuids) + data = util.extract_json(req.body, put_schema) + if consider_generation: + # Check for generation conflict + rp_gen = data['resource_provider_generation'] + if resource_provider.generation != rp_gen: + raise webob.exc.HTTPConflict( + _("Resource provider's generation already changed. Please " + "update the generation and try again.")) + aggregate_uuids = data['aggregates'] + else: + aggregate_uuids = data + try: + resource_provider.set_aggregates( + aggregate_uuids, increment_generation=consider_generation) + except (exception.ConcurrentUpdateDetected, + db_exc.DBDuplicateEntry) as exc: + raise webob.exc.HTTPConflict( + _('Update conflict: %(error)s') % {'error': exc}) - return _send_aggregates(req, aggregate_uuids) + return _send_aggregates(req, resource_provider, aggregate_uuids) diff --git a/nova/api/openstack/placement/microversion.py b/nova/api/openstack/placement/microversion.py index 83d18e2c8ee9..ae6b73b31aaa 100644 --- a/nova/api/openstack/placement/microversion.py +++ b/nova/api/openstack/placement/microversion.py @@ -61,6 +61,8 @@ VERSIONS = [ '1.17', # Add 'required' query parameter to GET /allocation_candidates and # return traits in the provider summary. '1.18', # Support ?required= queryparam on GET /resource_providers + '1.19', # Include generation and conflict detection in provider aggregates + # APIs ] diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index b3bc36614b7e..b9106b653134 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -432,7 +432,9 @@ def _get_aggregates_by_provider_id(context, rp_id): @db_api.api_context_manager.writer -def _set_aggregates(context, rp_id, provided_aggregates): +def _set_aggregates(context, resource_provider, provided_aggregates, + increment_generation=False): + rp_id = resource_provider.id # When aggregate uuids are persisted no validation is done # to ensure that they refer to something that has meaning # elsewhere. It is assumed that code which makes use of the @@ -489,6 +491,10 @@ def _set_aggregates(context, rp_id, provided_aggregates): select_agg_id) context.session.execute(insert_aggregates) + if increment_generation: + resource_provider.generation = _increment_provider_generation( + context, resource_provider) + @db_api.api_context_manager.reader def _get_traits_by_provider_id(context, rp_id): @@ -757,13 +763,17 @@ class ResourceProvider(base.VersionedObject, base.TimestampedObject): """Get the aggregate uuids associated with this resource provider.""" return _get_aggregates_by_provider_id(self._context, self.id) - def set_aggregates(self, aggregate_uuids): + def set_aggregates(self, aggregate_uuids, increment_generation=False): """Set the aggregate uuids associated with this resource provider. If an aggregate does not exist, one will be created using the provided uuid. + + The resource provider generation is incremented if and only if the + increment_generation parameter is True. """ - _set_aggregates(self._context, self.id, aggregate_uuids) + _set_aggregates(self._context, self, aggregate_uuids, + increment_generation=increment_generation) def set_traits(self, traits): """Replaces the set of traits associated with the resource provider diff --git a/nova/api/openstack/placement/rest_api_version_history.rst b/nova/api/openstack/placement/rest_api_version_history.rst index b6be104d0e49..21208f4f1a07 100644 --- a/nova/api/openstack/placement/rest_api_version_history.rst +++ b/nova/api/openstack/placement/rest_api_version_history.rst @@ -233,3 +233,13 @@ based on other query parameters. Trait names which are empty, do not exist, or are otherwise invalid will result in a 400 error. + +1.19 Include generation and conflict detection in provider aggregates APIs +-------------------------------------------------------------------------- + +Enhance the payloads for the `GET /resource_providers/{uuid}/aggregates` +response and the `PUT /resource_providers/{uuid}/aggregates` request and +response to be identical, and to include the ``resource_provider_generation``. +As with other generation-aware APIs, if the ``resource_provider_generation`` +specified in the `PUT` request does not match the generation known by the +server, a 409 Conflict error is returned. diff --git a/nova/api/openstack/placement/schemas/aggregate.py b/nova/api/openstack/placement/schemas/aggregate.py index 3478d7249e7e..dc5d94921665 100644 --- a/nova/api/openstack/placement/schemas/aggregate.py +++ b/nova/api/openstack/placement/schemas/aggregate.py @@ -10,9 +10,10 @@ # License for the specific language governing permissions and limitations # under the License. """Aggregate schemas for Placement API.""" +import copy -PUT_AGGREGATES_SCHEMA = { +_AGGREGATES_LIST_SCHEMA = { "type": "array", "items": { "type": "string", @@ -20,3 +21,22 @@ PUT_AGGREGATES_SCHEMA = { }, "uniqueItems": True } + + +PUT_AGGREGATES_SCHEMA_V1_1 = copy.deepcopy(_AGGREGATES_LIST_SCHEMA) + + +PUT_AGGREGATES_SCHEMA_V1_19 = { + "type": "object", + "properties": { + "aggregates": copy.deepcopy(_AGGREGATES_LIST_SCHEMA), + "resource_provider_generation": { + "type": "integer", + } + }, + "required": [ + "aggregates", + "resource_provider_generation", + ], + "additionalProperties": False, +} diff --git a/nova/tests/functional/api/openstack/placement/gabbits/aggregate.yaml b/nova/tests/functional/api/openstack/placement/gabbits/aggregate.yaml index b831e8b76c07..74fced612576 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/aggregate.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/aggregate.yaml @@ -57,22 +57,39 @@ tests: response_json_paths: $.errors[0].title: Not Found -- name: put same aggregates twice - PUT: $LAST_URL - data: - - *agg_1 - - *agg_1 - status: 400 - response_strings: - - has non-unique elements - response_json_paths: - $.errors[0].title: Bad Request - -- name: put some aggregates +- name: put some aggregates - old payload and new microversion PUT: $LAST_URL data: - *agg_1 - *agg_2 + status: 400 + response_strings: + - JSON does not validate + response_json_paths: + $.errors[0].title: Bad Request + +- name: put some aggregates - new payload and old microversion + PUT: $LAST_URL + request_headers: + openstack-api-version: placement 1.18 + data: + resource_provider_generation: 0 + aggregates: + - *agg_1 + - *agg_2 + status: 400 + response_strings: + - JSON does not validate + response_json_paths: + $.errors[0].title: Bad Request + +- name: put some aggregates - new payload and new microversion + PUT: $LAST_URL + data: + resource_provider_generation: 0 + aggregates: + - *agg_1 + - *agg_2 status: 200 response_headers: content-type: /application/json/ @@ -82,6 +99,7 @@ tests: response_json_paths: $.aggregates[0]: *agg_1 $.aggregates[1]: *agg_2 + $.resource_provider_generation: 1 - name: get those aggregates GET: $LAST_URL @@ -92,9 +110,18 @@ tests: response_json_paths: $.aggregates.`len`: 2 +- name: clear those aggregates - generation conflict + PUT: $LAST_URL + data: + resource_provider_generation: 0 + aggregates: [] + status: 409 + - name: clear those aggregates PUT: $LAST_URL - data: [] + data: + resource_provider_generation: 1 + aggregates: [] status: 200 response_json_paths: $.aggregates: [] @@ -113,7 +140,7 @@ tests: response_json_paths: $.errors[0].title: Bad Request -- name: put invalid json not array +- name: put invalid json no generation PUT: $LAST_URL data: aggregates: @@ -128,11 +155,26 @@ tests: - name: put invalid json not uuids PUT: $LAST_URL data: - - harry - - sally + aggregates: + - harry + - sally + resource_provider_generation: 2 status: 400 response_strings: - - JSON does not validate + - "is not a 'uuid'" + response_json_paths: + $.errors[0].title: Bad Request + +- name: put same aggregates twice + PUT: $LAST_URL + data: + aggregates: + - *agg_1 + - *agg_1 + resource_provider_generation: 2 + status: 400 + response_strings: + - has non-unique elements response_json_paths: $.errors[0].title: Bad Request diff --git a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml index 7bafe881e47f..a8bb64b83338 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml @@ -39,13 +39,13 @@ tests: response_json_paths: $.errors[0].title: Not Acceptable -- name: latest microversion is 1.18 +- name: latest microversion is 1.19 GET: / request_headers: openstack-api-version: placement latest response_headers: vary: /OpenStack-API-Version/ - openstack-api-version: placement 1.18 + openstack-api-version: placement 1.19 - name: other accept header bad version GET: / diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml index b0bd3e67e0ac..087f359c95ff 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-aggregates.yaml @@ -34,7 +34,9 @@ tests: - name: associate an aggregate with rp1 PUT: /resource_providers/893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/aggregates data: - - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + aggregates: + - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + resource_provider_generation: 0 status: 200 - name: get by aggregates one result @@ -58,7 +60,9 @@ tests: - name: associate an aggregate with rp2 PUT: /resource_providers/5202c48f-c960-4eec-bde3-89c4f22a17b9/aggregates data: - - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + aggregates: + - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + resource_provider_generation: 0 status: 200 - name: get by aggregates two result @@ -71,7 +75,9 @@ tests: - name: associate another aggregate with rp2 PUT: /resource_providers/5202c48f-c960-4eec-bde3-89c4f22a17b9/aggregates data: - - 99652f11-9f77-46b9-80b7-4b1989be9f8c + aggregates: + - 99652f11-9f77-46b9-80b7-4b1989be9f8c + resource_provider_generation: 1 status: 200 - name: get by both aggregates two @@ -83,7 +89,9 @@ tests: - name: clear aggregates on rp1 PUT: /resource_providers/893337e9-1e55-49f0-bcfe-6a2f16fbf2f7/aggregates - data: [] + data: + aggregates: [] + resource_provider_generation: 1 status: 200 - name: get by both aggregates one diff --git a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-resources-query.yaml b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-resources-query.yaml index f80c1753a18d..e873e810709f 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-resources-query.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/resource-provider-resources-query.yaml @@ -146,7 +146,9 @@ tests: - name: associate an aggregate with rp1 PUT: /resource_providers/$ENVIRON['RP_UUID']/aggregates data: - - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + aggregates: + - 83a3d69d-8920-48e2-8914-cadfd8fa2f91 + resource_provider_generation: $HISTORY['list resource providers providing disk and vcpu resources'].$RESPONSE['$.resource_providers[0].generation'] status: 200 - name: get by aggregates with resources diff --git a/nova/tests/functional/api/openstack/placement/gabbits/shared-resources.yaml b/nova/tests/functional/api/openstack/placement/gabbits/shared-resources.yaml index 9bfd8b904d07..4517288bae6a 100644 --- a/nova/tests/functional/api/openstack/placement/gabbits/shared-resources.yaml +++ b/nova/tests/functional/api/openstack/placement/gabbits/shared-resources.yaml @@ -92,12 +92,16 @@ tests: - name: aggregate shared PUT: /resource_providers/d450bd39-3b01-4355-9ea1-594f96594cf1/aggregates data: - - f3dc0f36-97d4-4daf-be0c-d71466da9c85 + aggregates: + - f3dc0f36-97d4-4daf-be0c-d71466da9c85 + resource_provider_generation: 1 - name: aggregate cn1 PUT: /resource_providers/8d830468-6395-46b0-b56a-f934a1d60bbe/aggregates data: - - f3dc0f36-97d4-4daf-be0c-d71466da9c85 + aggregates: + - f3dc0f36-97d4-4daf-be0c-d71466da9c85 + resource_provider_generation: 1 # no shared trait - name: get resources no shared @@ -114,7 +118,7 @@ tests: - name: set trait shared PUT: /resource_providers/d450bd39-3b01-4355-9ea1-594f96594cf1/traits data: - resource_provider_generation: 1 + resource_provider_generation: 2 traits: - MISC_SHARES_VIA_AGGREGATE diff --git a/placement-api-ref/source/aggregates.inc b/placement-api-ref/source/aggregates.inc index dfbde76914cc..18efa06fdfb0 100644 --- a/placement-api-ref/source/aggregates.inc +++ b/placement-api-ref/source/aggregates.inc @@ -58,7 +58,8 @@ identified by `{uuid}`. Normal Response Codes: 200 -Error response codes: itemNotFound(404) +Error response codes: itemNotFound(404) if the provider does not exist. (If the +provider has no aggregates, the result is 200 with an empty aggregate list.) Request ------- @@ -67,19 +68,33 @@ Request - uuid: resource_provider_uuid_path -Response --------- +Response (microversions 1.1 - 1.18) +----------------------------------- .. rest_parameters:: parameters.yaml - aggregates: aggregates -Response Example ----------------- +Response Example (microversions 1.1 - 1.18) +------------------------------------------- .. literalinclude:: ./samples/aggregates/get-aggregates.json :language: javascript +Response (microversions 1.19 - ) +-------------------------------- + +.. rest_parameters:: parameters.yaml + + - aggregates: aggregates + - resource_provider_generation: resource_provider_generation_v1_19 + +Response Example (microversions 1.19 - ) +---------------------------------------- + +.. literalinclude:: ./samples/aggregates/get-aggregates-1.19.json + :language: javascript + Update resource provider aggregates =================================== @@ -89,31 +104,47 @@ Associate a list of aggregates with the resource provider identified by `{uuid}` Normal Response Codes: 200 -Error response codes: badRequest(400), itemNotFound(404) +Error response codes: badRequest(400), itemNotFound(404), conflict(409) -Request -------- +Request (microversion 1.1 - 1.18) +--------------------------------- .. rest_parameters:: parameters.yaml - uuid: resource_provider_uuid_path - aggregates: aggregates -Request example ---------------- +Request example (microversion 1.1 - 1.18) +----------------------------------------- .. literalinclude:: ./samples/aggregates/update-aggregates-request.json :language: javascript -Response --------- +Request (microversion 1.19 - ) +--------------------------------- + +.. rest_parameters:: parameters.yaml + + - uuid: resource_provider_uuid_path + - aggregates: aggregates + - resource_provider_generation: resource_provider_generation_v1_19 + +Request example (microversion 1.19 - ) +----------------------------------------- + +.. literalinclude:: ./samples/aggregates/update-aggregates-request-1.19.json + :language: javascript + +Response (microversion 1.19 - ) +---------------------------------- .. rest_parameters:: parameters.yaml - aggregates: aggregates + - resource_provider_generation: resource_provider_generation_v1_19 -Response Example ----------------- +Response Example (microversion 1.19 - ) +------------------------------------------ -.. literalinclude:: ./samples/aggregates/update-aggregates.json +.. literalinclude:: ./samples/aggregates/update-aggregates-1.19.json :language: javascript diff --git a/placement-api-ref/source/parameters.yaml b/placement-api-ref/source/parameters.yaml index bbc33e2bc0f8..393487908fe5 100644 --- a/placement-api-ref/source/parameters.yaml +++ b/placement-api-ref/source/parameters.yaml @@ -310,6 +310,9 @@ resource_provider_generation_optional: concurrent resource provider updates. The value is ignored; it is present to preserve symmetry between read and write representations. +resource_provider_generation_v1_19: + <<: *resource_provider_generation + min_version: 1.19 resource_provider_links: type: array in: body diff --git a/placement-api-ref/source/samples/aggregates/get-aggregates-1.19.json b/placement-api-ref/source/samples/aggregates/get-aggregates-1.19.json new file mode 100644 index 000000000000..fd1d7433d33b --- /dev/null +++ b/placement-api-ref/source/samples/aggregates/get-aggregates-1.19.json @@ -0,0 +1,7 @@ +{ + "aggregates": [ + "42896e0d-205d-4fe3-bd1e-100924931787", + "5e08ea53-c4c6-448e-9334-ac4953de3cfa" + ], + "resource_provider_generation": 8 +} diff --git a/placement-api-ref/source/samples/aggregates/update-aggregates-1.19.json b/placement-api-ref/source/samples/aggregates/update-aggregates-1.19.json new file mode 100644 index 000000000000..2980c426c742 --- /dev/null +++ b/placement-api-ref/source/samples/aggregates/update-aggregates-1.19.json @@ -0,0 +1,7 @@ +{ + "aggregates": [ + "42896e0d-205d-4fe3-bd1e-100924931787", + "5e08ea53-c4c6-448e-9334-ac4953de3cfa" + ], + "resource_provider_generation": 9 +} diff --git a/placement-api-ref/source/samples/aggregates/update-aggregates-request-1.19.json b/placement-api-ref/source/samples/aggregates/update-aggregates-request-1.19.json new file mode 100644 index 000000000000..2980c426c742 --- /dev/null +++ b/placement-api-ref/source/samples/aggregates/update-aggregates-request-1.19.json @@ -0,0 +1,7 @@ +{ + "aggregates": [ + "42896e0d-205d-4fe3-bd1e-100924931787", + "5e08ea53-c4c6-448e-9334-ac4953de3cfa" + ], + "resource_provider_generation": 9 +} diff --git a/releasenotes/notes/placement-aggregate-generation-9dad79fb0356fcc0.yaml b/releasenotes/notes/placement-aggregate-generation-9dad79fb0356fcc0.yaml new file mode 100644 index 000000000000..3c614e93aa62 --- /dev/null +++ b/releasenotes/notes/placement-aggregate-generation-9dad79fb0356fcc0.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Placement API microversion 1.19 enhances the payloads for the + `GET /resource_providers/{uuid}/aggregates` response and the + `PUT /resource_providers/{uuid}/aggregates` request and response to be + identical, and to include the ``resource_provider_generation``. As with + other generation-aware APIs, if the ``resource_provider_generation`` + specified in the `PUT` request does not match the generation known by the + server, a 409 Conflict error is returned.