Consider root id is None in the database case
There are cases where ``root_provider_id`` of a resource provider is set to NULL just after it is upgraded to the Rocky release. In such cases getting allocation candidates raises a Keyerror. This patch fixes that bug for cases there is no sharing or nested providers in play. Change-Id: I9639d852078c95de506110f24d3f35e7cf5e361e Closes-Bug:#1799892
This commit is contained in:
Tetsuro Nakamura
Eric Fried
parent
ea26392239
commit
cdbedac920
|
|
@ -733,9 +733,18 @@ def _provider_ids_from_rp_ids(context, rp_ids):
|
|||
me.c.parent_provider_id == parent.c.id)
|
||||
sel = sa.select(cols).select_from(me_to_parent)
|
||||
sel = sel.where(me.c.id.in_(rp_ids))
|
||||
return {
|
||||
r[0]: ProviderIds(**dict(r)) for r in context.session.execute(sel)
|
||||
}
|
||||
|
||||
ret = {}
|
||||
for r in context.session.execute(sel):
|
||||
# Use its id/uuid for the root id/uuid if the root id/uuid is None
|
||||
# TODO(tetsuro): Remove this to when we are sure all root_provider_id
|
||||
# values are NOT NULL
|
||||
d = dict(r)
|
||||
if d['root_id'] is None:
|
||||
d['root_id'] = d['id']
|
||||
d['root_uuid'] = d['uuid']
|
||||
ret[d['id']] = ProviderIds(**d)
|
||||
return ret
|
||||
|
||||
|
||||
def _provider_ids_from_uuid(context, uuid):
|
||||
|
|
@ -2732,13 +2741,15 @@ def _get_usages_by_provider_tree(ctx, root_ids):
|
|||
# FROM allocations
|
||||
# JOIN resource_providers
|
||||
# ON allocations.resource_provider_id = resource_providers.id
|
||||
# AND resource_providers.root_provider_id IN($root_ids)
|
||||
# AND (resource_providers.root_provider_id IN($root_ids)
|
||||
# OR resource_providers.id IN($root_ids))
|
||||
# GROUP BY resource_provider_id, resource_class_id
|
||||
# )
|
||||
# AS usages
|
||||
# AS usage
|
||||
# ON inv.resource_provider_id = usage.resource_provider_id
|
||||
# AND inv.resource_class_id = usage.resource_class_id
|
||||
# WHERE rp.root_provider_id IN ($root_ids)
|
||||
# WHERE (rp.root_provider_id IN ($root_ids)
|
||||
# OR resource_providers.id IN($root_ids))
|
||||
rpt = sa.alias(_RP_TBL, name="rp")
|
||||
inv = sa.alias(_INV_TBL, name="inv")
|
||||
# Build our derived table (subquery in the FROM clause) that sums used
|
||||
|
|
@ -2746,7 +2757,12 @@ def _get_usages_by_provider_tree(ctx, root_ids):
|
|||
derived_alloc_to_rp = sa.join(
|
||||
_ALLOC_TBL, _RP_TBL,
|
||||
sa.and_(_ALLOC_TBL.c.resource_provider_id == _RP_TBL.c.id,
|
||||
_RP_TBL.c.root_provider_id.in_(root_ids)))
|
||||
# TODO(tetsuro): Remove this OR condition when all
|
||||
# root_provider_id values are NOT NULL
|
||||
sa.or_(_RP_TBL.c.root_provider_id.in_(root_ids),
|
||||
_RP_TBL.c.id.in_(root_ids))
|
||||
)
|
||||
)
|
||||
usage = sa.alias(
|
||||
sa.select([
|
||||
_ALLOC_TBL.c.resource_provider_id,
|
||||
|
|
@ -2778,7 +2794,14 @@ def _get_usages_by_provider_tree(ctx, root_ids):
|
|||
inv.c.allocation_ratio,
|
||||
inv.c.max_unit,
|
||||
usage.c.used,
|
||||
]).select_from(usage_join).where(rpt.c.root_provider_id.in_(root_ids))
|
||||
]).select_from(usage_join).where(
|
||||
# TODO(tetsuro): Remove this or condition when all
|
||||
# root_provider_id values are NOT NULL
|
||||
sa.or_(
|
||||
rpt.c.root_provider_id.in_(root_ids),
|
||||
rpt.c.id.in_(root_ids)
|
||||
)
|
||||
)
|
||||
return ctx.session.execute(query).fetchall()
|
||||
|
||||
|
||||
|
|
@ -3052,7 +3075,16 @@ def _get_providers_with_resource(ctx, rc_id, amount):
|
|||
sel = sel.where(sa.and_(*where_conds))
|
||||
res = ctx.session.execute(sel).fetchall()
|
||||
res = set((r[0], r[1]) for r in res)
|
||||
return res
|
||||
# TODO(tetsuro): Bug#1799892: We could have old providers with no root
|
||||
# provider set and they haven't undergone a data migration yet,
|
||||
# so we need to set the root_id explicitly here. We remove
|
||||
# this and when all root_provider_id values are NOT NULL
|
||||
ret = []
|
||||
for rp_tuple in res:
|
||||
rp_id = rp_tuple[0]
|
||||
root_id = rp_id if rp_tuple[1] is None else rp_tuple[1]
|
||||
ret.append((rp_id, root_id))
|
||||
return ret
|
||||
|
||||
|
||||
@db_api.placement_context_manager.reader
|
||||
|
|
|
|||
|
|
@ -467,6 +467,9 @@ class AllocationCandidatesTestCase(tb.PlacementDbBaseTestCase):
|
|||
)
|
||||
conn.execute(ins_rptbl)
|
||||
|
||||
# This is needed for _validate_allocation_requests() at the end
|
||||
self.rp_uuid_to_name[uuids.rp1] = 'cn1'
|
||||
|
||||
# Add VCPU(resource_class_id=0) inventory to the provider.
|
||||
ins_invtbl = inv_tbl.insert().values(
|
||||
id=1,
|
||||
|
|
@ -491,31 +494,7 @@ class AllocationCandidatesTestCase(tb.PlacementDbBaseTestCase):
|
|||
)
|
||||
conn.execute(ins_alloctbl)
|
||||
|
||||
# TODO(tetsuro): Bug#1799892: Fix this not to raise the KeyError
|
||||
# alloc_cands = self._get_allocation_candidates(
|
||||
# {'': placement_lib.RequestGroup(
|
||||
# use_same_provider=False,
|
||||
# resources={
|
||||
# fields.ResourceClass.VCPU: 1
|
||||
# }
|
||||
# )}
|
||||
# )
|
||||
#
|
||||
# expected = [
|
||||
# [('cn1', fields.ResourceClass.VCPU, 1)]
|
||||
# ]
|
||||
# self._validate_allocation_requests(expected, alloc_cands)
|
||||
#
|
||||
# expected = {
|
||||
# 'cn1': set([
|
||||
# (fields.ResourceClass.VCPU, 8, 4)
|
||||
# ]),
|
||||
# }
|
||||
# self._validate_provider_summary_resources(expected, alloc_cands)
|
||||
|
||||
self.assertRaises(
|
||||
KeyError,
|
||||
self._get_allocation_candidates,
|
||||
alloc_cands = self._get_allocation_candidates(
|
||||
{'': placement_lib.RequestGroup(
|
||||
use_same_provider=False,
|
||||
resources={
|
||||
|
|
@ -524,6 +503,53 @@ class AllocationCandidatesTestCase(tb.PlacementDbBaseTestCase):
|
|||
)}
|
||||
)
|
||||
|
||||
expected = [
|
||||
[('cn1', fields.ResourceClass.VCPU, 1)]
|
||||
]
|
||||
self._validate_allocation_requests(expected, alloc_cands)
|
||||
|
||||
expected = {
|
||||
'cn1': set([
|
||||
(fields.ResourceClass.VCPU, 8, 4)
|
||||
]),
|
||||
}
|
||||
self._validate_provider_summary_resources(expected, alloc_cands)
|
||||
|
||||
# NOTE(tetsuro): Getting allocation candidates goes through a
|
||||
# different path when sharing/nested providers exist, let's test
|
||||
# that case and the path creating a new sharing provider.
|
||||
# We omit the direct database insertion of 'ss1' here since 'cn1',
|
||||
# which has no root id in the database, is the actual target of the
|
||||
# following test.
|
||||
ss1 = self._create_provider('ss1', uuids.agg1)
|
||||
tb.set_traits(ss1, "MISC_SHARES_VIA_AGGREGATE")
|
||||
tb.add_inventory(ss1, fields.ResourceClass.VCPU, 8)
|
||||
|
||||
alloc_cands = self._get_allocation_candidates(
|
||||
{'': placement_lib.RequestGroup(
|
||||
use_same_provider=False,
|
||||
resources={
|
||||
fields.ResourceClass.VCPU: 1
|
||||
}
|
||||
)}
|
||||
)
|
||||
|
||||
expected = [
|
||||
[('cn1', fields.ResourceClass.VCPU, 1)],
|
||||
[('ss1', fields.ResourceClass.VCPU, 1)]
|
||||
]
|
||||
self._validate_allocation_requests(expected, alloc_cands)
|
||||
|
||||
expected = {
|
||||
'cn1': set([
|
||||
(fields.ResourceClass.VCPU, 8, 4)
|
||||
]),
|
||||
'ss1': set([
|
||||
(fields.ResourceClass.VCPU, 8, 0)
|
||||
]),
|
||||
}
|
||||
self._validate_provider_summary_resources(expected, alloc_cands)
|
||||
|
||||
def test_all_local(self):
|
||||
"""Create some resource providers that can satisfy the request for
|
||||
resources with local (non-shared) resources and verify that the
|
||||
|
|
|
|||
Loading…
Reference in New Issue