openstack
/
neutron
Code Issues Proposed changes

Fixed PortBindingLevelDbObjectTestCase 

The test class was not inheriting from testlib_api.SqlTestCase which
means that the test cases were never executed.

Once it was executed, it became obvious that the object has an issue
with SQL query scalability. Specifically, it was failing the
test_get_objects_queries_constant test case that validates that the
number of queries to fetch a single object is the same as for multiple
objects. This happens because PortBindingLevel's 'segment' synthetic
field is not constructed from db model relationship but through issuing
a get_object OVO call that triggers a query per fetched PortBindingLevel
object.

To fix the issue, the patch added a new 'segment' relationship to
PortBindingLevel db model, and made OVO use attributes from the
relationship to construct 'segment' synthetic field.

Another issue revealed by enabling the test class is that foreign_key
definition for the 'segment' synthetic field located in NetworkSegment
object refers to 'segment_id' field but the field with this name is not
defined on PortBindingLevel object. The easiest thing we can do to
resolve the discrepancy is adding the 'segment_id' UUID typed field to
PortBindingLevel, which is what the patch does. (An alternative could be
hacking our way around the lack of the field, building another layer of
complexity in the base OVO class for this special case; I figured it's
probably counterproductive, and we can live with 'segment_id' field even
though it's arguably somewhat redundant - 'segment' object field already
carries the ID.)

Adding a new field to PortBindingLevel object means that we need to bump
its version, as well as versions of all objects that use the newly
modified field type for their own fields. The only object that refers to
the type is Port.  That's why we also bump the version of Port object.
To make older agents that use RPC callbacks work with the change, we
also provide corresponding obj_make_compatible methods for both Port and
PortBindingLevel objects.

Change-Id: I1ec56c57f87218520c2080c0b3450f1eabc13224
This commit is contained in:
Ihar Hrachyshka 2018-05-07 21:31:37 +00:00 committed by Lujin
parent 37bf4889ba
commit b74af5ff65
5 changed files with 91 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
neutron/objects/ports.py
View File

@ -107,7 +107,8 @@ class DistributedPortBinding(PortBindingBase):
@base.NeutronObjectRegistry.register
class PortBindingLevel(base.NeutronDbObject):
# Version 1.0: Initial version
VERSION = '1.0'
# Version 1.1: Added segment_id
VERSION = '1.1'
db_model = ml2_models.PortBindingLevel
@ -121,6 +122,9 @@ class PortBindingLevel(base.NeutronDbObject):
'segment': obj_fields.ObjectField(
'NetworkSegment', nullable=True
),
# arguably redundant but allows us to define foreign key for 'segment'
# synthetic field inside NetworkSegment definition
'segment_id': common_types.UUIDField(nullable=True),
}
synthetic_fields = ['segment']
@ -140,6 +144,11 @@ class PortBindingLevel(base.NeutronDbObject):
return super(PortBindingLevel, cls).get_objects(
context, _pager, validate_filters, **kwargs)
def obj_make_compatible(self, primitive, target_version):
_target_version = versionutils.convert_version_to_tuple(target_version)
if _target_version < (1, 1):
primitive.pop('segment_id', None)
@base.NeutronObjectRegistry.register
class IPAllocation(base.NeutronDbObject):
@ -253,7 +262,8 @@ class SecurityGroupPortBinding(base.NeutronDbObject):
class Port(base.NeutronDbObject):
# Version 1.0: Initial version
# Version 1.1: Add data_plane_status field
VERSION = '1.1'
# Version 1.2: Added segment_id to binding_levels
VERSION = '1.2'
db_model = models_v2.Port
@ -450,9 +460,13 @@ class Port(base.NeutronDbObject):
def obj_make_compatible(self, primitive, target_version):
_target_version = versionutils.convert_version_to_tuple(target_version)
if _target_version < (1, 1):
primitive.pop('data_plane_status', None)
if _target_version < (1, 2):
binding_levels = primitive.get('binding_levels', [])
for lvl in binding_levels:
lvl['versioned_object.version'] = '1.0'
lvl['versioned_object.data'].pop('segment_id', None)
@classmethod
def get_ports_by_router(cls, context, router_id, owner, subnet):

4
neutron/plugins/ml2/models.py
View File

@ -19,6 +19,7 @@ from neutron_lib.db import model_base
import sqlalchemy as sa
from sqlalchemy import orm
from neutron.db.models import segment as segment_models
from neutron.db import models_v2
BINDING_PROFILE_LEN = 4095
@ -90,6 +91,9 @@ class PortBindingLevel(model_base.BASEV2):
load_on_pending=True,
backref=orm.backref("binding_levels", lazy='subquery',
cascade='delete'))
segment = orm.relationship(
segment_models.NetworkSegment,
load_on_pending=True)
revises_on_change = ('port', )

31
neutron/tests/unit/objects/test_base.py
View File

@ -1816,15 +1816,42 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
for local_field, foreign_key in foreign_keys.items():
objclass_fields[local_field] = obj.get(foreign_key)
# remember which fields were nullified so that later we know what
# to assert for each child field
nullified_fields = set()
# cut off more depth levels to simplify object field generation
# (for example, nullify segment field for PortBindingLevel objects
# to avoid creating a Segment object (and back-linking it to the
# original network of the port)
for child_field in self._get_object_synthetic_fields(objclass):
if objclass.fields[child_field].nullable:
objclass_fields[child_field] = None
nullified_fields.add(child_field)
# initialize the child object
synth_field_obj = objclass(self.context, **objclass_fields)
# nullify nullable UUID fields since they may otherwise trigger
# foreign key violations
for field_name in get_obj_persistent_fields(synth_field_obj):
child_field = objclass.fields[field_name]
if child_field.nullable:
if isinstance(child_field, common_types.UUIDField):
synth_field_obj[field_name] = None
nullified_fields.add(field_name)
synth_field_obj.create()
# reload the parent object under test
obj = cls_.get_object(self.context, **obj._get_composite_keys())
# check that the stored database model now has filled relationships
# check that the stored database model now has correct attr values
dbattr = obj.fields_need_translation.get(field, field)
self.assertTrue(getattr(obj.db_obj, dbattr, None))
if field in nullified_fields:
self.assertIsNone(getattr(obj.db_obj, dbattr, None))
else:
self.assertIsNotNone(getattr(obj.db_obj, dbattr, None))
# reset the object so that we can compare it to other clean objects
obj.obj_reset_changes([field])

4
neutron/tests/unit/objects/test_objects.py
View File

@ -62,9 +62,9 @@ object_data = {
'NetworkPortSecurity': '1.0-b30802391a87945ee9c07582b4ff95e3',
'NetworkRBAC': '1.0-c8a67f39809c5a3c8c7f26f2f2c620b2',
'NetworkSegment': '1.0-57b7f2960971e3b95ded20cbc59244a8',
'Port': '1.1-5bf48d12a7bf7f5b7a319e8003b437a5',
'Port': '1.2-5bf48d12a7bf7f5b7a319e8003b437a5',
'PortBinding': '1.0-3306deeaa6deb01e33af06777d48d578',
'PortBindingLevel': '1.0-de66a4c61a083b8f34319fa9dde5b060',
'PortBindingLevel': '1.1-50d47f63218f87581b6cd9a62db574e5',
'PortDataPlaneStatus': '1.0-25be74bda46c749653a10357676c0ab2',
'PortDNS': '1.1-c5ca2dc172bdd5fafee3fc986d1d7023',
'PortSecurity': '1.0-b30802391a87945ee9c07582b4ff95e3',

44
neutron/tests/unit/objects/test_ports.py
View File

@ -194,10 +194,6 @@ class PortBindingLevelIfaceObjTestCase(
def setUp(self):
super(PortBindingLevelIfaceObjTestCase, self).setUp()
# for this object, the model contains segment_id but we expose it
# through an ObjectField that is loaded without a relationship
for obj in self.db_objs:
obj['segment_id'] = None
self.pager_map[self._test_class.obj_name()] = (
obj_base.Pager(sorts=[('port_id', True), ('level', True)]))
self.pager_map[network.NetworkSegment.obj_name()] = (
@ -206,10 +202,16 @@ class PortBindingLevelIfaceObjTestCase(
class PortBindingLevelDbObjectTestCase(
obj_test_base.BaseDbObjectTestCase):
obj_test_base.BaseDbObjectTestCase, testlib_api.SqlTestCase):
_test_class = ports.PortBindingLevel
def setUp(self):
super(PortBindingLevelDbObjectTestCase, self).setUp()
self.update_obj_fields(
{'port_id': lambda: self._create_test_port_id(),
'segment_id': lambda: self._create_test_segment_id()})
class PortIfaceObjTestCase(obj_test_base.BaseObjectIfaceTestCase):
@ -359,3 +361,35 @@ class PortDbObjectTestCase(obj_test_base.BaseDbObjectTestCase,
port_v1_0 = port_new.obj_to_primitive(target_version='1.0')
self.assertNotIn('data_plane_status',
port_v1_0['versioned_object.data'])
def test_v1_2_to_v1_1_drops_segment_id_in_binding_levels(self):
port_new = self._create_test_port()
segment = network.NetworkSegment(
self.context,
# TODO(ihrachys) we should be able to create a segment object
# without explicitly specifying id, but it's currently not working
id=uuidutils.generate_uuid(),
network_id=port_new.network_id,
network_type='vxlan')
segment.create()
# TODO(ihrachys) we should be able to create / update level objects via
# Port object, but it's currently not working
binding = ports.PortBindingLevel(
self.context, port_id=port_new.id,
host='host1', level=0, segment_id=segment.id)
binding.create()
port_new = ports.Port.get_object(self.context, id=port_new.id)
port_v1_1 = port_new.obj_to_primitive(target_version='1.1')
lvl = port_v1_1['versioned_object.data']['binding_levels'][0]
self.assertNotIn('segment_id', lvl['versioned_object.data'])
# check that we also downgraded level object version
self.assertEqual('1.0', lvl['versioned_object.version'])
# finally, prove that binding primitive is now identical to direct
# downgrade of the binding object
binding_v1_0 = binding.obj_to_primitive(target_version='1.0')
self.assertEqual(binding_v1_0, lvl)