Refactor qos_plugin._extend_port_resource_request

In order to speed up the port collection, some DB calls
have been deleted in the extended function
"QoSPlugin._extend_port_resource_request".

With the parent commit, the port DB/OVO object now has the
network QoS policy ID, making the network retrieval unneeded.

In this refactor the QoS policy is collected from the DB only
if a valid QoS policy ID exists, bound to the port or to the
network. This reduces the number of QoS objects collection to
one or zero.

Change-Id: Iadf704c00378da99c502dca3e3b79796c368cac7
Closes-Bug: #1834484
(cherry picked from commit 8b00349a63)
This commit is contained in:
Rodolfo Alonso Hernandez 2019-06-27 17:31:14 +00:00
parent b452c508b6
commit 1229be91d9
3 changed files with 43 additions and 57 deletions

View File

@ -59,7 +59,7 @@ class QosNetworkPolicyBinding(model_base.BASEV2):
primaryjoin='QosNetworkPolicyBinding.network_id == Port.network_id',
foreign_keys=network_id,
backref=sa.orm.backref('qos_network_policy_binding', uselist=False,
viewonly=True))
viewonly=True, lazy='joined'))
class QosFIPPolicyBinding(model_base.BASEV2):

View File

@ -85,31 +85,23 @@ class QoSPlugin(qos.QoSPluginBase):
@resource_extend.extends([port_def.COLLECTION_NAME])
def _extend_port_resource_request(port_res, port_db):
"""Add resource request to a port."""
if isinstance(port_db, ports_object.Port):
qos_id = port_db.qos_policy_id or port_db.qos_network_policy_id
else:
qos_id = None
if port_db.get('qos_policy_binding'):
qos_id = port_db.qos_policy_binding.policy_id
elif port_db.get('qos_network_policy_binding'):
qos_id = port_db.qos_network_policy_binding.policy_id
port_res['resource_request'] = None
qos_policy = policy_object.QosPolicy.get_port_policy(
context.get_admin_context(), port_res['id'])
# Note(lajoskatona): QosPolicyPortBinding is not ready for some
# reasons, so let's try and fetch the QoS policy directly if there is a
# qos_policy_id in port_res.
if (not qos_policy and 'qos_policy_id' in port_res and
port_res['qos_policy_id']):
qos_policy = policy_object.QosPolicy.get_policy_obj(
context.get_admin_context(), port_res['qos_policy_id']
)
# Note(lajoskatona): handle the case when the port inherits qos-policy
# from the network.
if not qos_policy:
net = network_object.Network.get_object(
context.get_admin_context(), id=port_res['network_id'])
if net and net.qos_policy_id:
qos_policy = policy_object.QosPolicy.get_network_policy(
context.get_admin_context(), net.id)
if not qos_policy:
if not qos_id:
return port_res
qos_policy = policy_object.QosPolicy.get_object(
context.get_admin_context(), id=qos_id)
resources = {}
# NOTE(ralonsoh): we should move this translation dict to n-lib.
rule_direction_class = {
nl_constants.INGRESS_DIRECTION:
pl_constants.CLASS_NET_BW_INGRESS_KBPS,
@ -122,6 +114,10 @@ class QoSPlugin(qos.QoSPluginBase):
if not resources:
return port_res
# NOTE(ralonsoh): we should not rely on the current execution order of
# the port extending functions. Although here we have
# port_res[VNIC_TYPE], we should retrieve this value from the port DB
# object instead.
vnic_trait = pl_utils.vnic_type_trait(
port_res[portbindings.VNIC_TYPE])
@ -129,19 +125,16 @@ class QoSPlugin(qos.QoSPluginBase):
# support will be available. See Placement spec:
# https://review.openstack.org/565730
first_segment = network_object.NetworkSegment.get_objects(
context.get_admin_context(),
network_id=port_res['network_id'])[0]
context.get_admin_context(), network_id=port_db.network_id)[0]
if not first_segment or not first_segment.physical_network:
return port_res
physnet_trait = pl_utils.physnet_trait(
first_segment.physical_network)
resource_request = {
port_res['resource_request'] = {
'required': [physnet_trait, vnic_trait],
'resources': resources
}
port_res['resource_request'] = resource_request
'resources': resources}
return port_res
def _get_ports_with_policy(self, context, policy):

View File

@ -117,41 +117,34 @@ class TestQosPlugin(base.BaseQosTestCase):
self.assertIsInstance(call_args[2], policy_object.QosPolicy)
def _create_and_extend_port(self, bw_rules, physical_network='public',
has_qos_policy=True, network_qos=None):
has_qos_policy=True, has_net_qos_policy=False):
network_id = uuidutils.generate_uuid()
if has_qos_policy or network_qos:
policy = self.policy
policy_id = self.policy.id
self.policy.rules = bw_rules
for rule in bw_rules:
rule.qos_policy_id = self.policy.id
else:
policy = None
policy_id = None
port_res = {
"id": uuidutils.generate_uuid(),
"qos_policy_id": policy_id,
"network_id": network_id,
"binding:vnic_type": "normal",
self.port_data = {
'port': {'id': uuidutils.generate_uuid(),
'network_id': network_id}
}
network_mock = mock.MagicMock(id=network_id, qos_policy_id=policy_id)
if has_qos_policy:
self.port_data['port']['qos_policy_id'] = self.policy.id
self.policy.rules = bw_rules
elif has_net_qos_policy:
self.port_data['port']['qos_network_policy_id'] = self.policy.id
self.policy.rules = bw_rules
self.port = ports_object.Port(
self.ctxt, **self.port_data['port'])
port_res = {"binding:vnic_type": "normal"}
segment_mock = mock.MagicMock(network_id=network_id,
physical_network=physical_network)
with mock.patch(
'neutron.objects.network.Network.get_object',
return_value=network_mock
), mock.patch(
'neutron.objects.network.NetworkSegment.get_objects',
return_value=[segment_mock]
), mock.patch(
'neutron.objects.qos.policy.QosPolicy.get_port_policy',
return_value=policy
):
with mock.patch('neutron.objects.network.NetworkSegment.get_objects',
return_value=[segment_mock]), \
mock.patch('neutron.objects.qos.policy.QosPolicy.get_object',
return_value=self.policy):
return qos_plugin.QoSPlugin._extend_port_resource_request(
port_res, {})
port_res, self.port)
def test__extend_port_resource_request_min_bw_rule(self):
self.min_rule.direction = lib_constants.EGRESS_DIRECTION
@ -212,7 +205,7 @@ class TestQosPlugin(base.BaseQosTestCase):
self.min_rule.qos_policy_id = self.policy.id
port = self._create_and_extend_port([self.min_rule],
network_qos=self.policy)
has_net_qos_policy=True)
self.assertEqual(
['CUSTOM_PHYSNET_PUBLIC', 'CUSTOM_VNIC_TYPE_NORMAL'],
port['resource_request']['required']