diff --git a/neutron/db/qos/models.py b/neutron/db/qos/models.py index b8b8411a6c4..54e2bd5e1ec 100644 --- a/neutron/db/qos/models.py +++ b/neutron/db/qos/models.py @@ -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): diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py index 2b478324d6d..25ff80a5e34 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -88,31 +88,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, @@ -125,6 +117,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]) @@ -132,19 +128,16 @@ class QoSPlugin(qos.QoSPluginBase): # support will be available. See Placement spec: # https://review.opendev.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): diff --git a/neutron/tests/unit/services/qos/test_qos_plugin.py b/neutron/tests/unit/services/qos/test_qos_plugin.py index 99871d31b60..b09ecff3cc7 100644 --- a/neutron/tests/unit/services/qos/test_qos_plugin.py +++ b/neutron/tests/unit/services/qos/test_qos_plugin.py @@ -116,41 +116,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 @@ -211,7 +204,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']