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:
parent
b452c508b6
commit
1229be91d9
|
@ -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):
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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']
|
||||
|
|
Loading…
Reference in New Issue