placement: delete auto-created consumers on fail
When we fail to create allocations for new consumers (either when
issuing a PUT /allocations/{new_consumer_uuid} or a POST /allocations
where the payload includes a new consumer UUID), we need to ensure that
we delete the Consumer object and underlying record in the consumers
table that gets auto-created before calling AllocationList.create_all().
This auto-created consumer record is what is used to compare things like
consumer generation in later calls to PUT|POST /allocations, and this
phantom consumer record was causing confusion when normal retries (for
things like 409 Conflict due to concurrent provider or inventory
updates) would be rejected stating that the expected consumer generation
was 0 and not null (null being the sentinel that indicates the caller is
expecting the consumer is a new consumer).
Change-Id: If37ef318bd5482a2d19928002c6f1fa24932946f
Closes-bug: #1779725
Closes-bug: #1778576
This commit is contained in:
Jay Pipes
parent
194e842c30
commit
c641981826
|
|
@ -155,22 +155,21 @@ def _serialize_allocations_for_resource_provider(allocations,
|
|||
# but having it in this file seems wrong, however, since it uses
|
||||
# _new_allocations it's being left here for now. We need a place for shared
|
||||
# handler code, but util.py is already too big and too diverse.
|
||||
def create_allocation_list(context, data, want_version):
|
||||
def create_allocation_list(context, data, consumers):
|
||||
"""Create an AllocationList based on provided data.
|
||||
|
||||
:param context: The placement context.
|
||||
:param data: A dictionary of multiple allocations by consumer uuid.
|
||||
:param want_version: The desired microversion, which controls how
|
||||
consumer generations are handled.
|
||||
:param consumers: A dictionary, keyed by consumer UUID, of Consumer objects
|
||||
:return: An AllocationList.
|
||||
:raises: `webob.exc.HTTPBadRequest` if a resource provider included in the
|
||||
allocations does not exist.
|
||||
"""
|
||||
allocation_objects = []
|
||||
|
||||
for consumer_uuid in data:
|
||||
project_id = data[consumer_uuid]['project_id']
|
||||
user_id = data[consumer_uuid]['user_id']
|
||||
allocations = data[consumer_uuid]['allocations']
|
||||
consumer_generation = data[consumer_uuid].get('consumer_generation')
|
||||
consumer = consumers[consumer_uuid]
|
||||
if allocations:
|
||||
rp_objs = _resource_providers_by_uuid(context, allocations.keys())
|
||||
for resource_provider_uuid in allocations:
|
||||
|
|
@ -178,12 +177,8 @@ def create_allocation_list(context, data, want_version):
|
|||
resources = allocations[resource_provider_uuid]['resources']
|
||||
new_allocations = _new_allocations(context,
|
||||
resource_provider,
|
||||
consumer_uuid,
|
||||
resources,
|
||||
project_id,
|
||||
user_id,
|
||||
consumer_generation,
|
||||
want_version)
|
||||
consumer,
|
||||
resources)
|
||||
allocation_objects.extend(new_allocations)
|
||||
else:
|
||||
# The allocations are empty, which means wipe them out.
|
||||
|
|
@ -295,28 +290,17 @@ def _resource_providers_by_uuid(ctx, rp_uuids):
|
|||
return res
|
||||
|
||||
|
||||
def _new_allocations(context, resource_provider, consumer_uuid,
|
||||
resources, project_id, user_id, consumer_generation,
|
||||
want_version):
|
||||
def _new_allocations(context, resource_provider, consumer, resources):
|
||||
"""Create new allocation objects for a set of resources
|
||||
|
||||
Returns a list of Allocation objects.
|
||||
Returns a list of Allocation objects
|
||||
|
||||
:param context: The placement context.
|
||||
:param resource_provider: The resource provider that has the resources.
|
||||
:param consumer_uuid: The uuid of the consumer of the resources.
|
||||
:param consumer: The Consumer object consuming the resources.
|
||||
:param resources: A dict of resource classes and values.
|
||||
:param project_id: The project consuming the resources.
|
||||
:param user_id: The user consuming the resources.
|
||||
:param consumer_generation: The generation supplied by the user when
|
||||
PUT/POST'ing allocations. May be None if
|
||||
the microversion is <1.28
|
||||
:param want_version: The microversion object from the context.
|
||||
"""
|
||||
allocations = []
|
||||
consumer = util.ensure_consumer(
|
||||
context, consumer_uuid, project_id, user_id, consumer_generation,
|
||||
want_version)
|
||||
for resource_class in resources:
|
||||
allocation = rp_obj.Allocation(
|
||||
resource_provider=resource_provider,
|
||||
|
|
@ -327,6 +311,21 @@ def _new_allocations(context, resource_provider, consumer_uuid,
|
|||
return allocations
|
||||
|
||||
|
||||
def _delete_consumers(consumers):
|
||||
"""Helper function that deletes any consumer object supplied to it
|
||||
|
||||
:param consumers: iterable of Consumer objects to delete
|
||||
"""
|
||||
for consumer in consumers:
|
||||
try:
|
||||
consumer.delete()
|
||||
LOG.debug("Deleted auto-created consumer with consumer UUID "
|
||||
"%s after failed allocation", consumer.uuid)
|
||||
except Exception as err:
|
||||
LOG.warning("Got an exception when deleting auto-created "
|
||||
"consumer with UUID %s: %s", consumer.uuid, err)
|
||||
|
||||
|
||||
def _set_allocations_for_consumer(req, schema):
|
||||
context = req.environ['placement.context']
|
||||
context.can(policies.ALLOC_UPDATE)
|
||||
|
|
@ -346,9 +345,12 @@ def _set_allocations_for_consumer(req, schema):
|
|||
}
|
||||
allocation_data = allocations_dict
|
||||
|
||||
# If the body includes an allocation for a resource provider
|
||||
# that does not exist, raise a 400.
|
||||
allocation_objects = []
|
||||
# Consumer object saved in case we need to delete the auto-created consumer
|
||||
# record
|
||||
consumer = None
|
||||
# Whether we created a new consumer record
|
||||
created_new_consumer = False
|
||||
if not allocation_data:
|
||||
# The allocations are empty, which means wipe them out. Internal
|
||||
# to the allocation object this is signalled by a used value of 0.
|
||||
|
|
@ -366,25 +368,35 @@ def _set_allocations_for_consumer(req, schema):
|
|||
allocation.used = 0
|
||||
allocation_objects.append(allocation)
|
||||
else:
|
||||
# If the body includes an allocation for a resource provider
|
||||
# that does not exist, raise a 400.
|
||||
rp_objs = _resource_providers_by_uuid(context, allocation_data.keys())
|
||||
consumer, created_new_consumer = util.ensure_consumer(
|
||||
context, consumer_uuid, data.get('project_id'),
|
||||
data.get('user_id'), data.get('consumer_generation'),
|
||||
want_version)
|
||||
for resource_provider_uuid, allocation in allocation_data.items():
|
||||
resource_provider = rp_objs[resource_provider_uuid]
|
||||
new_allocations = _new_allocations(context,
|
||||
resource_provider,
|
||||
consumer_uuid,
|
||||
allocation['resources'],
|
||||
data.get('project_id'),
|
||||
data.get('user_id'),
|
||||
data.get('consumer_generation'),
|
||||
want_version)
|
||||
consumer,
|
||||
allocation['resources'])
|
||||
allocation_objects.extend(new_allocations)
|
||||
|
||||
allocations = rp_obj.AllocationList(
|
||||
context, objects=allocation_objects)
|
||||
|
||||
def _create_allocations(alloc_list):
|
||||
try:
|
||||
alloc_list.create_all()
|
||||
LOG.debug("Successfully wrote allocations %s", alloc_list)
|
||||
except Exception:
|
||||
if created_new_consumer:
|
||||
_delete_consumers([consumer])
|
||||
raise
|
||||
|
||||
try:
|
||||
allocations.create_all()
|
||||
LOG.debug("Successfully wrote allocations %s", allocations)
|
||||
_create_allocations(allocations)
|
||||
# InvalidInventory is a parent for several exceptions that
|
||||
# indicate either that Inventory is not present, or that
|
||||
# capacity limits have been exceeded.
|
||||
|
|
@ -447,15 +459,45 @@ def set_allocations(req):
|
|||
want_schema = schema.POST_ALLOCATIONS_V1_28
|
||||
data = util.extract_json(req.body, want_schema)
|
||||
|
||||
# First, ensure that all consumers referenced in the payload actually
|
||||
# exist. And if not, create them. Keep a record of auto-created consumers
|
||||
# so we can clean them up if the end allocation create_all() fails.
|
||||
consumers = {} # dict of Consumer objects, keyed by consumer UUID
|
||||
new_consumers_created = []
|
||||
for consumer_uuid in data:
|
||||
project_id = data[consumer_uuid]['project_id']
|
||||
user_id = data[consumer_uuid]['user_id']
|
||||
consumer_generation = data[consumer_uuid].get('consumer_generation')
|
||||
try:
|
||||
consumer, new_consumer_created = util.ensure_consumer(
|
||||
context, consumer_uuid, project_id, user_id,
|
||||
consumer_generation, want_version)
|
||||
if new_consumer_created:
|
||||
new_consumers_created.append(consumer)
|
||||
consumers[consumer_uuid] = consumer
|
||||
except Exception:
|
||||
# If any errors (for instance, a consumer generation conflict)
|
||||
# occur when ensuring consumer records above, make sure we delete
|
||||
# any auto-created consumers.
|
||||
_delete_consumers(new_consumers_created)
|
||||
raise
|
||||
|
||||
# Create a sequence of allocation objects to be used in one
|
||||
# AllocationList.create_all() call, which will mean all the changes
|
||||
# happen within a single transaction and with resource provider
|
||||
# and consumer generations (if applicable) check all in one go.
|
||||
allocations = create_allocation_list(context, data, want_version)
|
||||
allocations = create_allocation_list(context, data, consumers)
|
||||
|
||||
def _create_allocations(alloc_list):
|
||||
try:
|
||||
alloc_list.create_all()
|
||||
LOG.debug("Successfully wrote allocations %s", alloc_list)
|
||||
except Exception:
|
||||
_delete_consumers(new_consumers_created)
|
||||
raise
|
||||
|
||||
try:
|
||||
allocations.create_all()
|
||||
LOG.debug("Successfully wrote allocations %s", allocations)
|
||||
_create_allocations(allocations)
|
||||
except exception.NotFound as exc:
|
||||
raise webob.exc.HTTPBadRequest(
|
||||
_("Unable to allocate inventory %(error)s") % {'error': exc})
|
||||
|
|
|
|||
|
|
@ -130,6 +130,17 @@ def _increment_consumer_generation(ctx, consumer):
|
|||
return new_generation
|
||||
|
||||
|
||||
@db_api.placement_context_manager.writer
|
||||
def _delete_consumer(ctx, consumer):
|
||||
"""Deletes the supplied consumer.
|
||||
|
||||
:param ctx: `nova.context.RequestContext` that contains an oslo_db Session
|
||||
:param consumer: `Consumer` whose generation should be updated.
|
||||
"""
|
||||
del_stmt = CONSUMER_TBL.delete().where(CONSUMER_TBL.c.id == consumer.id)
|
||||
ctx.session.execute(del_stmt)
|
||||
|
||||
|
||||
@base.VersionedObjectRegistry.register_if(False)
|
||||
class Consumer(base.VersionedObject, base.TimestampedObject):
|
||||
|
||||
|
|
@ -193,3 +204,6 @@ class Consumer(base.VersionedObject, base.TimestampedObject):
|
|||
consumer)
|
||||
"""
|
||||
self.generation = _increment_consumer_generation(self._context, self)
|
||||
|
||||
def delete(self):
|
||||
_delete_consumer(self._context, self)
|
||||
|
|
|
|||
|
|
@ -582,7 +582,9 @@ def ensure_consumer(ctx, consumer_uuid, project_id, user_id,
|
|||
"""Ensures there are records in the consumers, projects and users table for
|
||||
the supplied external identifiers.
|
||||
|
||||
Returns a populated Consumer object containing Project and User sub-objects
|
||||
Returns a tuple containing the populated Consumer object containing Project
|
||||
and User sub-objects and a boolean indicating whether a new Consumer object
|
||||
was created (as opposed to an existing consumer record retrieved)
|
||||
|
||||
:param ctx: The request context.
|
||||
:param consumer_uuid: The uuid of the consumer of the resources.
|
||||
|
|
@ -594,6 +596,7 @@ def ensure_consumer(ctx, consumer_uuid, project_id, user_id,
|
|||
:raises webob.exc.HTTPConflict if consumer generation is required and there
|
||||
was a mismatch
|
||||
"""
|
||||
created_new_consumer = False
|
||||
requires_consumer_generation = want_version.matches((1, 28))
|
||||
if project_id is None:
|
||||
project_id = CONF.placement.incomplete_consumer_project_id
|
||||
|
|
@ -647,7 +650,8 @@ def ensure_consumer(ctx, consumer_uuid, project_id, user_id,
|
|||
consumer = consumer_obj.Consumer(
|
||||
ctx, uuid=consumer_uuid, project=proj, user=user)
|
||||
consumer.create()
|
||||
created_new_consumer = True
|
||||
except exception.ConsumerExists:
|
||||
# No worries, another thread created this user already
|
||||
consumer = consumer_obj.Consumer.get_by_uuid(ctx, consumer_uuid)
|
||||
return consumer
|
||||
return consumer, created_new_consumer
|
||||
|
|
|
|||
|
|
@ -231,10 +231,7 @@ tests:
|
|||
$.allocation_requests[0].allocations['$ENVIRON["RP_UUID"]'].resources.VCPU: 1
|
||||
$.allocation_requests[0].allocations['8aa83304-4b6d-4a23-b954-06d8b36b206a'].resources.DISK_GB: 200
|
||||
|
||||
# This fails because of bug 1778576
|
||||
# https://bugs.launchpad.net/nova/+bug/1778576
|
||||
- name: put that allocation to new consumer
|
||||
xfail: True
|
||||
PUT: /allocations/55555555-5555-5555-5555-555555555555
|
||||
data:
|
||||
allocations: $RESPONSE['$.allocation_requests[0].allocations']
|
||||
|
|
|
|||
|
|
@ -55,8 +55,10 @@ tests:
|
|||
$:
|
||||
allocations: {}
|
||||
|
||||
- name: retry allocations new consumer, wrong gen
|
||||
desc: consumer generation is wrong below to make test pass
|
||||
# The failure to allocate above should have deleted the auto-created consumer,
|
||||
# so when we retry the allocation here, we should be able to use the
|
||||
# appropriate null generation to indicate this is a new consumer
|
||||
- name: retry allocations new consumer, still null gen
|
||||
PUT: /allocations/88888888-8888-8888-8888-888888888888
|
||||
data:
|
||||
allocations:
|
||||
|
|
@ -65,8 +67,5 @@ tests:
|
|||
VCPU: 1
|
||||
project_id: $ENVIRON['PROJECT_ID']
|
||||
user_id: $ENVIRON['USER_ID']
|
||||
# The API suggests we should have null here
|
||||
# consumer_generation: null
|
||||
# but the error response says 0, so use it to make test pass
|
||||
consumer_generation: 0
|
||||
consumer_generation: null
|
||||
status: 204
|
||||
|
|
|
|||
|
|
@ -108,6 +108,31 @@ tests:
|
|||
user_id: $ENVIRON['USER_ID']
|
||||
status: 204
|
||||
|
||||
- name: get allocations for instance consumer
|
||||
GET: /allocations/$ENVIRON['INSTANCE_UUID']
|
||||
request_headers:
|
||||
# We want to inspect the consumer generations...
|
||||
openstack-api-version: placement 1.28
|
||||
response_json_paths:
|
||||
$.allocations["$HISTORY['rp compute02'].$RESPONSE['uuid']"].resources[MEMORY_MB]: 1024
|
||||
$.allocations["$HISTORY['rp compute02'].$RESPONSE['uuid']"].resources[VCPU]: 2
|
||||
$.allocations["$HISTORY['rp storage01'].$RESPONSE['uuid']"].resources[DISK_GB]: 5
|
||||
$.consumer_generation: 1
|
||||
$.project_id: $ENVIRON['PROJECT_ID']
|
||||
$.user_id: $ENVIRON['USER_ID']
|
||||
|
||||
- name: get allocations for migration consumer
|
||||
GET: /allocations/$ENVIRON['MIGRATION_UUID']
|
||||
request_headers:
|
||||
# We want to inspect the consumer generations...
|
||||
openstack-api-version: placement 1.28
|
||||
response_json_paths:
|
||||
$.allocations["$HISTORY['rp compute01'].$RESPONSE['uuid']"].resources[MEMORY_MB]: 1024
|
||||
$.allocations["$HISTORY['rp compute01'].$RESPONSE['uuid']"].resources[VCPU]: 2
|
||||
$.consumer_generation: 1
|
||||
$.project_id: $ENVIRON['PROJECT_ID']
|
||||
$.user_id: $ENVIRON['USER_ID']
|
||||
|
||||
- name: confirm usages
|
||||
GET: /usages?project_id=$ENVIRON['PROJECT_ID']
|
||||
response_json_paths:
|
||||
|
|
|
|||
Loading…
Reference in New Issue