reshaper: Look up provider if not in inventories

Per the referenced bug, we weren't accounting for the scenario where a
reshape operation was removing *all* inventories for a provider (which
could be fairly common). With this fix, we do a three-stage lookup of
the provider object: If it's not in the inventories, we look in the
allocations; if it's not in the allocations, we look it up in the
database.

Change-Id: I594bb64f87c61b7ffd39c19e0fd42c4c087a3a11
Closes-Bug: #1783130
This commit is contained in:
Eric Fried 2018-07-23 12:10:12 -05:00
parent 4d525b4ec1
commit d13e5a5011
2 changed files with 42 additions and 19 deletions

View File

@ -4160,8 +4160,11 @@ def reshape(ctx, inventories, allocations):
# in this transaction. We keep a cache of these because as we perform the
# various operations on the providers, their generations increment and we
# want to "inject" the changed resource provider objects into the
# AllocationList's objects before calling AllocationList.replace_all()
affected_providers = {}
# AllocationList's objects before calling AllocationList.replace_all().
# We start with the providers in the allocation objects, but only use one
# if we don't find it in the inventories.
affected_providers = {alloc.resource_provider.uuid: alloc.resource_provider
for alloc in allocations}
# We have to do the inventory changes in two steps because:
# - we can't delete inventories with allocations; and
# - we can't create allocations on nonexistent inventories.
@ -4177,7 +4180,29 @@ def reshape(ctx, inventories, allocations):
for rp_uuid, new_inv_list in inventories.items():
LOG.debug("reshaping: *interim* inventory replacement for provider %s",
rp_uuid)
rp = new_inv_list[0].resource_provider
# We need the resource provider object.
if new_inv_list:
# If the inventory list is nonempty, grab it from the first entry.
# This overrides (and will replace, below) an object that was
# prepopulated from the allocations.
rp = new_inv_list[0].resource_provider
elif rp_uuid in affected_providers:
# No inventory object; second choice is from an allocation object.
rp = affected_providers[rp_uuid]
else:
# No inventory or allocations for this provider - which makes sense
# when we're "clearing out" a provider. Look it up.
rp = ResourceProvider.get_by_uuid(ctx, rp_uuid)
# Update the cache. This may be replacing an entry that came from
# allocations, or adding a new entry from inventories and/or db lookup.
affected_providers[rp_uuid] = rp
# Optimization: If the new inventory is empty, the below would be
# replacing it with itself (and incrementing the generation)
# unnecessarily.
if not new_inv_list:
continue
# A dict, keyed by resource class, of the Inventory objects. We start
# with the original inventory list.
inv_by_rc = {inv.resource_class: inv for inv in
@ -4189,7 +4214,6 @@ def reshape(ctx, inventories, allocations):
inv_by_rc[inv.resource_class] = inv
# Set the interim inventory structure.
rp.set_inventory(InventoryList(objects=list(inv_by_rc.values())))
affected_providers[rp_uuid] = rp
# NOTE(jaypipes): The above inventory replacements will have
# incremented the resource provider generations, so we need to look in
@ -4208,8 +4232,4 @@ def reshape(ctx, inventories, allocations):
for rp_uuid, new_inv_list in inventories.items():
LOG.debug("reshaping: *final* inventory replacement for provider %s",
rp_uuid)
# TODO(efried): If we wanted this to be more efficient, we could keep
# track of providers for which all inventories are being deleted in the
# above loop and just do those and skip the rest, since they're already
# in their final form.
new_inv_list[0].resource_provider.set_inventory(new_inv_list)
affected_providers[rp_uuid].set_inventory(new_inv_list)

View File

@ -529,8 +529,6 @@ tests:
# the allocations under two new consumers. This is an artificial test to
# exercise new consumer creation.
- name: consolidate inventory and allocations
# TODO(efried): bug https://bugs.launchpad.net/nova/+bug/1783130
xfail: true
POST: /reshaper
data:
inventories:
@ -547,8 +545,6 @@ tests:
max_unit: 8
$ENVIRON['ALT_RP_UUID']:
resource_provider_generation: 3
# bug https://bugs.launchpad.net/nova/+bug/1783130
# means that this will cause an IndexError.
inventories: {}
allocations:
$ENVIRON['CONSUMER_0']:
@ -576,12 +572,22 @@ tests:
project_id: $ENVIRON['PROJECT_ID']
user_id: $ENVIRON['ALT_USER_ID']
consumer_generation: null
$ENVIRON['CONSUMER_ID']:
allocations: {}
project_id: $ENVIRON['PROJECT_ID']
user_id: $ENVIRON['USER_ID']
consumer_generation: 2
$ENVIRON['ALT_CONSUMER_ID']:
allocations:
$ENVIRON['RP_UUID']:
resources:
DISK_GB: 20
project_id: $ENVIRON['PROJECT_ID']
user_id: $ENVIRON['ALT_USER_ID']
consumer_generation: 2
status: 204
- name: get usages on parent after move back
# TODO(efried): bug https://bugs.launchpad.net/nova/+bug/1783130
# Fails because 'consolidate inventory and allocations' above fails.
xfail: true
GET: /resource_providers/$ENVIRON['RP_UUID']/usages
response_json_paths:
$.usages:
@ -590,9 +596,6 @@ tests:
$.resource_provider_generation: 11
- name: get usages on child after move back
# TODO(efried): bug https://bugs.launchpad.net/nova/+bug/1783130
# Fails because 'consolidate inventory and allocations' above fails.
xfail: true
GET: /resource_providers/$ENVIRON['ALT_RP_UUID']/usages
response_json_paths:
$.usages: {}