openstack/placement
Code Issues Proposed changes

Fix placement allocate while over-capacity

Browse Source 
There is a bug in the placement over-capacity check that prevents
updating existing allocations if a resource provider is over capacity,
even if that change makes no change to the amount of resources that
are allocated, or even if the allocation makes the situation better.
This can happen when the reserved or allocation_ratio is changed on
a provider with existing allocations. The bug prevents escaping this
situation by deallocating resources unless a single atomic allocation
can be performed to result in an under-capacity outcome. That is not
something consumers like Nova can orchestrate and thus operators get
stuck in a situation where they must un-reserve some resource, or
temporarily increase the allocation_ratio in order to be able to
escape. That could result in a worsening situation.

This change makes us allow allocation changes if they improve or
otherwise do not worsen an over-capacity situation, which is clearly
what operators expect based on the multiple bugs opened against Nova
for this issue.

Closes-Bug: #2104040
Related-Bug: #1924123
Related-Bug: #1941892
Related-Bug: #1943191
Change-Id: If7264d3ce679b6f9b604a3bcd2a417cc175e3793
This commit is contained in:
Dan Smith
2025-03-24 12:24:35 -07:00
parent 356ff48bb8
commit 5ac4c70b8a
2 changed files with 114 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
placement/objects/allocation.py
+85 -26
View File
@@ -70,7 +70,42 @@ def _delete_allocations_by_ids(ctx, alloc_ids):
ctx.session.execute(del_sql)
def _check_capacity_exceeded(ctx, allocs):
def _provider_usage_query(provider_ids, rc_ids):
"""Generate a query fragment to grab provider usage per class."""
usage = sa.select(
_ALLOC_TBL.c.resource_provider_id,
_ALLOC_TBL.c.resource_class_id,
sql.func.sum(_ALLOC_TBL.c.used).label('used'),
)
usage = usage.where(
sa.and_(_ALLOC_TBL.c.resource_class_id.in_(rc_ids),
_ALLOC_TBL.c.resource_provider_id.in_(provider_ids)))
usage = usage.group_by(_ALLOC_TBL.c.resource_provider_id,
_ALLOC_TBL.c.resource_class_id)
return usage
def _provider_usage_summary(ctx, provider_ids, rcs):
"""Generate a summary dict of usages for a given list of providers.
Returns a dict of {rp_id: {rc1: n, rc2: m, ...}}
:param provider_ids: List of resource provider ids
:param rcs: List of resource class names
"""
usage = _provider_usage_query(provider_ids,
[ctx.rc_cache.id_from_string(rc)
for rc in rcs])
records = ctx.session.execute(usage)
summary = collections.defaultdict(dict)
for record in records:
rp_summary = summary[record.resource_provider_id]
rp_summary[record.resource_class_id] = record.used
return summary
def _check_capacity_exceeded(ctx, allocs, prev_usage):
"""Checks to see if the supplied allocation records would result in any of
the inventories involved having their capacity exceeded.
@@ -87,6 +122,8 @@ def _check_capacity_exceeded(ctx, allocs):
:param ctx: `placement.context.RequestContext` that has an oslo_db
Session
:param allocs: List of `Allocation` objects to check
:param prev_usage: Dict of providers' previous usage counts before this
operation
"""
# The SQL generated below looks like this:
# SELECT
@@ -119,17 +156,7 @@ def _check_capacity_exceeded(ctx, allocs):
for a in allocs])
provider_uuids = set([a.resource_provider.uuid for a in allocs])
provider_ids = set([a.resource_provider.id for a in allocs])
usage = sa.select(
_ALLOC_TBL.c.resource_provider_id,
_ALLOC_TBL.c.resource_class_id,
sql.func.sum(_ALLOC_TBL.c.used).label('used'),
)
usage = usage.where(
sa.and_(_ALLOC_TBL.c.resource_class_id.in_(rc_ids),
_ALLOC_TBL.c.resource_provider_id.in_(provider_ids)))
usage = usage.group_by(_ALLOC_TBL.c.resource_provider_id,
_ALLOC_TBL.c.resource_class_id)
usage = usage.subquery(name='usage')
usage = _provider_usage_query(provider_ids, rc_ids).subquery(name='usage')
inv_join = sql.join(
_RP_TBL, _INV_TBL,
@@ -224,19 +251,46 @@ def _check_capacity_exceeded(ctx, allocs):
# usage.used can be returned as None
used = usage.used or 0
capacity = (usage.total - usage.reserved) * allocation_ratio
amount_needed_all = rp_resource_class_sum[rp_uuid][rc_id]
if (capacity < (used + amount_needed) or
capacity < (used + rp_resource_class_sum[rp_uuid][rc_id])):
LOG.warning(
"Over capacity for %(rc)s on resource provider %(rp)s. "
"Needed: %(needed)s, Used: %(used)s, Capacity: %(cap)s",
{'rc': alloc.resource_class,
'rp': rp_uuid,
'needed': amount_needed,
'used': used,
'cap': capacity})
raise exception.InvalidAllocationCapacityExceeded(
resource_class=alloc.resource_class,
resource_provider=rp_uuid)
capacity < (used + amount_needed_all)):
# If we get here, this RP is over capacity. That's a problem,
# except we allow it *if* the current set of allocations is
# making the situation better (i.e. resulting in less or the same
# amount of over-capacity).
try:
prev_used = prev_usage[alloc.resource_provider.id][rc_id]
except KeyError:
prev_used = None
params = {
'rc': alloc.resource_class,
'rp': rp_uuid,
'needed': amount_needed,
'needed_all': used + amount_needed_all,
'used': used,
'prev_used': prev_used,
'cap': capacity,
}
if prev_used is not None and (
used + amount_needed_all <= prev_used):
# The situation is no worse than it was, so no reason to
# refuse the change.
LOG.warning(
"Over capacity for %(rc)s on resource provider "
"%(rp)s but allocation of %(needed)s reduces "
"existing overage from %(prev_used)s to %(needed_all)s",
params)
else:
# The situation is worse than it was, so refuse.
LOG.warning(
"Over capacity for %(rc)s on resource provider %(rp)s. "
"Needed: %(needed)s, Used: %(used)s, Capacity: %(cap)s",
params)
raise exception.InvalidAllocationCapacityExceeded(
resource_class=alloc.resource_class,
resource_provider=rp_uuid)
return res_providers
@@ -335,8 +389,13 @@ def _set_allocations(context, allocs):
"""
# First delete any existing allocations for any consumers. This
# provides a clean slate for the consumers mentioned in the list of
# allocations being manipulated.
# allocations being manipulated. Before we do, grab a snapshot of the
# current usage for any of the affected providers and classes.
consumer_ids = set(alloc.consumer.uuid for alloc in allocs)
prev_usage = _provider_usage_summary(
context,
[alloc.resource_provider.id for alloc in allocs],
[alloc.resource_class for alloc in allocs])
for consumer_id in consumer_ids:
_delete_allocations_for_consumer(context, consumer_id)
@@ -360,7 +419,7 @@ def _set_allocations(context, allocs):
# _check_capacity_exceeded will raise a ResourceClassNotFound # if any
# allocation is using a resource class that does not exist.
visited_consumers = {}
visited_rps = _check_capacity_exceeded(context, allocs)
visited_rps = _check_capacity_exceeded(context, allocs, prev_usage)
for alloc in allocs:
if alloc.consumer.id not in visited_consumers:
visited_consumers[alloc.consumer.id] = alloc.consumer
placement/tests/functional/db/test_allocation.py
+29 -9
View File
@@ -599,7 +599,9 @@ class TestAllocationListCreateDelete(tb.PlacementDbBaseTestCase):
self.ctx, empty_rp)
self.assertEqual(0, len(allocations))
def _test_change_allocation_over_capacity(self, pre_inv, post_inv):
@mock.patch('placement.objects.allocation.LOG')
def _test_change_allocation_over_capacity(self, pre_inv, post_inv,
mock_log):
"""Test allocation changes while a provider is over capacity."""
# Create a provider with some over-committed inventory
@@ -632,29 +634,47 @@ class TestAllocationListCreateDelete(tb.PlacementDbBaseTestCase):
# Attempt to reduce the over-capacity situation by reducing one of the
# consumers' allocation (from 7 to 6)
# FIXME(danms): This is reproducing bug #2104040, which should be
# fixed as this is improving the over-capacity situation
alloc_post = alloc_obj.Allocation(consumer=consumers[0],
resource_provider=provider,
resource_class=orc.VCPU,
used=6)
self.assertRaises(exception.InvalidAllocationCapacityExceeded,
alloc_obj.replace_all, self.ctx, [alloc_post])
alloc_obj.replace_all(self.ctx, [alloc_post])
log_line = (mock_log.warning.call_args_list[0][0][0] %
mock_log.warning.call_args_list[0][0][1])
self.assertEqual(
log_line,
('Over capacity for VCPU on resource provider %s but allocation of'
' 6 reduces existing overage from 14 to 13') % provider.uuid)
# Attempt to reduce the over-capacity issue by reducing both
# allocations to 6 (one is still at 7, one is already at 6).
# Make sure that the check works as expected even with multiple
# providers (i.e the per-transaction sum is what matters) and
# especially when the first is the one that drops the count.
# FIXME(danms): This is reproducing bug #2104040, which should be
# fixed as this is improving the over-capacity situation
allocs_post = [alloc_obj.Allocation(consumer=consumer,
resource_provider=provider,
resource_class=orc.VCPU,
used=6)
for consumer in consumers]
self.assertRaises(exception.InvalidAllocationCapacityExceeded,
alloc_obj.replace_all, self.ctx, allocs_post)
alloc_obj.replace_all(self.ctx, allocs_post)
# Make sure we can swap inventory to another consumer without any
# improvement (but no further worsening either).
alt_consumer = consumer_obj.Consumer(self.ctx, uuid=uuidsentinel.inst3,
user=self.user_obj,
project=self.project_obj)
alt_consumer.create()
consumers.append(alt_consumer)
allocs_post = [alloc_obj.Allocation(consumer=consumer,
resource_provider=provider,
resource_class=orc.VCPU,
used=(6 if consumer != consumers[0]
else 0))
for consumer in consumers]
alloc_obj.replace_all(self.ctx, allocs_post)
# First consumer no longer has an allocation, so remove it from our
# list
del consumers[0]
# Make sure that we can bring the provider out of over-capacity by
# reducing multiple allocations at once.