openstack/placement
Code Issues Proposed changes

Don't use OVO for Inventory and InventoryList

Browse Source 
There are some more substantial changes here, including a few
things what may need to move earlier in the statck.

The standard stuff: Inventory and InventoryList are now classical
Python objects.

* Neither make use of a self._context, so that is removed, cascading
  changes through some tests.

* From early on Inventory used obj_set_defaults to, uh, set defaults.
  Now keyword args in __init__ are handling that.

* However a test which was checking for out of bounds values when
  creating Inventory objects has been removed because it can no
  longer work. I feel this is safe because we have jsonschema
  on the HTTP API and HTTP is the interface.

* Timestamp handling (in placement.util) needs some tweaks to
  do with objects that were never loaded from the database. The
  comments there should indicate what's going on.

There is quite a bit of duplication shared with other *List classes.
That cleanup will come.

Change-Id: I1a58810e73010ccefb80967d12f283e0f7007205
This commit is contained in:
Chris Dent
2019-02-13 23:42:14 +00:00
parent dc4c711029
commit 75555e7e2c
6 changed files with 55 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
placement/objects/resource_provider.py
View File

@@ -1627,20 +1627,25 @@ class ResourceProviderList(base.ObjectListBase, base.VersionedObject):
ResourceProvider, resource_providers)
@base.VersionedObjectRegistry.register_if(False)
class Inventory(base.VersionedObject, base.TimestampedObject):
class Inventory(object):
fields = {
'id': fields.IntegerField(read_only=True),
'resource_provider': fields.ObjectField('ResourceProvider'),
'resource_class': fields.StringField(read_only=True),
'total': fields.NonNegativeIntegerField(),
'reserved': fields.NonNegativeIntegerField(default=0),
'min_unit': fields.NonNegativeIntegerField(default=1),
'max_unit': fields.NonNegativeIntegerField(default=1),
'step_size': fields.NonNegativeIntegerField(default=1),
'allocation_ratio': fields.NonNegativeFloatField(default=1.0),
}
# kwargs included because some constructors pass resource_class_id
# but it is not used.
def __init__(self, id=None, resource_provider=None, resource_class=None,
total=None, reserved=0, min_unit=1, max_unit=1, step_size=1,
allocation_ratio=1.0, updated_at=None, created_at=None,
**kwargs):
self.id = id
self.resource_provider = resource_provider
self.resource_class = resource_class
self.total = total
self.reserved = reserved
self.min_unit = min_unit
self.max_unit = max_unit
self.step_size = step_size
self.allocation_ratio = allocation_ratio
self.updated_at = updated_at
self.created_at = created_at
@property
def capacity(self):
@@ -1668,12 +1673,17 @@ def _get_inventory_by_provider_id(ctx, rp_id):
return [dict(r) for r in ctx.session.execute(sel)]
@base.VersionedObjectRegistry.register_if(False)
class InventoryList(base.ObjectListBase, base.VersionedObject):
class InventoryList(object):
fields = {
'objects': fields.ListOfObjectsField('Inventory'),
}
def __init__(self, objects=None):
self.objects = objects or []
def __len__(self):
"""List length is a proxy for truthiness."""
return len(self.objects)
def __getitem__(self, index):
return self.objects[index]
def find(self, res_class):
"""Return the inventory record from the list of Inventory records that
@@ -1701,13 +1711,13 @@ class InventoryList(base.ObjectListBase, base.VersionedObject):
# constructor as-is
objs = [
Inventory(
context, resource_provider=rp,
resource_provider=rp,
resource_class=_RC_CACHE.string_from_id(
rec['resource_class_id']),
**rec)
for rec in db_inv
]
inv_list = cls(context, objects=objs)
inv_list = cls(objects=objs)
return inv_list

4
placement/tests/functional/db/test_base.py
View File

@@ -43,7 +43,6 @@ def add_inventory(rp, rc, total, **kwargs):
kwargs.setdefault('max_unit', total)
inv = rp_obj.Inventory(rp._context, resource_provider=rp,
resource_class=rc, total=total, **kwargs)
inv.obj_set_defaults()
rp.add_inventory(inv)
return inv
@@ -118,8 +117,7 @@ class PlacementDbBaseTestCase(base.TestCase):
def _make_allocation(self, inv_dict, alloc_dict):
alloc_dict = copy.copy(alloc_dict)
rp = self._create_provider('allocation_resource_provider')
disk_inv = rp_obj.Inventory(context=self.ctx,
resource_provider=rp, **inv_dict)
disk_inv = rp_obj.Inventory(resource_provider=rp, **inv_dict)
inv_list = rp_obj.InventoryList(objects=[disk_inv])
rp.set_inventory(inv_list)
consumer_id = alloc_dict.pop('consumer_id')

36
placement/tests/functional/db/test_reshape.py
View File

@@ -104,32 +104,32 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
# storage provider.
after_inventories = {
# cn1 keeps the RAM only
cn1: rp_obj.InventoryList(self.ctx, objects=[
cn1: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1,
resource_provider=cn1,
resource_class='MEMORY_MB', total=32768, reserved=0,
max_unit=32768, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
# each NUMA node gets half of the CPUs
cn1_numa0: rp_obj.InventoryList(self.ctx, objects=[
cn1_numa0: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1_numa0,
resource_provider=cn1_numa0,
resource_class='VCPU', total=8, reserved=0,
max_unit=8, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
cn1_numa1: rp_obj.InventoryList(self.ctx, objects=[
cn1_numa1: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1_numa1,
resource_provider=cn1_numa1,
resource_class='VCPU', total=8, reserved=0,
max_unit=8, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
# The sharing provider gets a bunch of disk
ss: rp_obj.InventoryList(self.ctx, objects=[
ss: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=ss,
resource_provider=ss,
resource_class='DISK_GB', total=100000, reserved=0,
max_unit=1000, min_unit=1, step_size=1,
allocation_ratio=1.0),
@@ -283,32 +283,32 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
# storage provider.
after_inventories = {
# cn1 keeps the RAM only
cn1: rp_obj.InventoryList(self.ctx, objects=[
cn1: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1,
resource_provider=cn1,
resource_class='MEMORY_MB', total=32768, reserved=0,
max_unit=32768, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
# each NUMA node gets half of the CPUs
cn1_numa0: rp_obj.InventoryList(self.ctx, objects=[
cn1_numa0: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1_numa0,
resource_provider=cn1_numa0,
resource_class='VCPU', total=8, reserved=0,
max_unit=8, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
cn1_numa1: rp_obj.InventoryList(self.ctx, objects=[
cn1_numa1: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=cn1_numa1,
resource_provider=cn1_numa1,
resource_class='VCPU', total=8, reserved=0,
max_unit=8, min_unit=1, step_size=1,
allocation_ratio=1.0),
]),
# The sharing provider gets a bunch of disk
ss: rp_obj.InventoryList(self.ctx, objects=[
ss: rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=ss,
resource_provider=ss,
resource_class='DISK_GB', total=100000, reserved=0,
max_unit=1000, min_unit=1, step_size=1,
allocation_ratio=1.0),
@@ -341,9 +341,9 @@ class ReshapeTestCase(tb.PlacementDbBaseTestCase):
# generation was validated and the actual call to reshape()
ss_threadB = rp_obj.ResourceProvider.get_by_uuid(self.ctx, ss.uuid)
# Reduce the amount of storage to 2000, from 100000.
new_ss_inv = rp_obj.InventoryList(self.ctx, objects=[
new_ss_inv = rp_obj.InventoryList(objects=[
rp_obj.Inventory(
self.ctx, resource_provider=ss_threadB,
resource_provider=ss_threadB,
resource_class='DISK_GB', total=2000, reserved=0,
max_unit=1000, min_unit=1, step_size=1,
allocation_ratio=1.0)])

6
placement/tests/functional/db/test_resource_provider.py
View File

@@ -773,7 +773,6 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase):
disk_inv = rp_obj.Inventory(resource_provider=rp,
resource_class='DISK_GB',
total=2048)
disk_inv.obj_set_defaults()
error = self.assertRaises(exception.NotFound, rp.update_inventory,
disk_inv)
self.assertIn('No inventory of class DISK_GB found',
@@ -791,7 +790,6 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase):
disk_inv = rp_obj.Inventory(
resource_provider=rp,
resource_class=orc.DISK_GB, total=new_total)
disk_inv.obj_set_defaults()
rp.update_inventory(disk_inv)
usages = rp_obj.UsageList.get_all_by_resource_provider_uuid(
@@ -1455,12 +1453,10 @@ class TestAllocationListCreateDelete(tb.PlacementDbBaseTestCase):
inv1 = rp_obj.Inventory(resource_provider=rp1,
resource_class=rp1_class,
total=1024, max_unit=max_unit)
inv1.obj_set_defaults()
rp1.set_inventory(rp_obj.InventoryList(objects=[inv1]))
inv2 = rp_obj.Inventory(resource_provider=rp2,
resource_class=rp2_class,
total=255, reserved=2, max_unit=max_unit)
inv2.obj_set_defaults()
rp2.set_inventory(rp_obj.InventoryList(objects=[inv2]))
# Now we can finally allocate.
@@ -1866,7 +1862,6 @@ class UsageListTestCase(tb.PlacementDbBaseTestCase):
inv = rp_obj.Inventory(resource_provider=db_rp,
resource_class=orc.DISK_GB,
total=1024)
inv.obj_set_defaults()
inv_list = rp_obj.InventoryList(objects=[inv])
db_rp.set_inventory(inv_list)
@@ -2102,7 +2097,6 @@ class ResourceClassTestCase(tb.PlacementDbBaseTestCase):
resource_class='CUSTOM_IRON_NFV',
total=1,
)
inv.obj_set_defaults()
inv_list = rp_obj.InventoryList(objects=[inv])
rp.set_inventory(inv_list)

18
placement/tests/unit/objects/test_resource_provider.py
View File

@@ -207,22 +207,6 @@ class TestInventoryNoDB(_TestCase):
self.assertEqual(_INVENTORY_DB['id'] + 1, objs[1].id)
self.assertEqual(_RESOURCE_PROVIDER_ID, objs[0].resource_provider.id)
def test_non_negative_handling(self):
# NOTE(cdent): Just checking, useless to be actually
# comprehensive here.
rp = resource_provider.ResourceProvider(id=_RESOURCE_PROVIDER_ID,
uuid=_RESOURCE_PROVIDER_UUID)
kwargs = dict(resource_provider=rp,
resource_class=_RESOURCE_CLASS_NAME,
total=16,
reserved=2,
min_unit=1,
max_unit=-8,
step_size=1,
allocation_ratio=1.0)
self.assertRaises(ValueError, resource_provider.Inventory,
**kwargs)
def test_set_defaults(self):
rp = resource_provider.ResourceProvider(id=_RESOURCE_PROVIDER_ID,
uuid=_RESOURCE_PROVIDER_UUID)
@@ -231,7 +215,6 @@ class TestInventoryNoDB(_TestCase):
total=16)
inv = resource_provider.Inventory(self.context, **kwargs)
inv.obj_set_defaults()
self.assertEqual(0, inv.reserved)
self.assertEqual(1, inv.min_unit)
self.assertEqual(1, inv.max_unit)
@@ -246,7 +229,6 @@ class TestInventoryNoDB(_TestCase):
total=16,
reserved=16)
inv = resource_provider.Inventory(self.context, **kwargs)
inv.obj_set_defaults()
self.assertEqual(0, inv.capacity)
inv.reserved = 15

6
placement/util.py
View File

@@ -140,11 +140,17 @@ def pick_last_modified(last_modified, obj):
"""
try:
current_modified = (obj.updated_at or obj.created_at)
# TODO(cdent): NotImplementedError catching can go away when all of
# OVO is gone.
except NotImplementedError:
# If updated_at is not implemented, we are looking at objects that
# have not come from the database, so "now" is the right modified
# time.
current_modified = timeutils.utcnow(with_timezone=True)
if current_modified is None:
# The object was not loaded from the DB, it was created in
# the current context.
current_modified = timeutils.utcnow(with_timezone=True)
if last_modified:
last_modified = max(last_modified, current_modified)
else: