From d6cd4420bb33501e267b10ccc3a1f26ab2c8c85b Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 8 Jul 2021 18:47:46 +0200 Subject: [PATCH] Parse extended resource request from the port data The format of the value of the resource_request field of the port has been changed by the port-resource-request-groups Neutron API extension. This patch adds a new factory method for RequestGroup to parse the data and create the RequestGroup objects corresponding to the request. Also the neutron interface of nova changed to use the new factory if the new neutron API extension is present. Note that we have to keep the old logic in place to support the scenario when Nova is upgraded first and therefore Neutron is still using the old resource_request format. Also note that some of the so far skipped functional tests started to pass as they are negative tests resulting in an unsuccessful operation due to not related reasons and the code already works good enough to hit those negative code paths. blueprint: qos-minimum-guaranteed-packet-rate Change-Id: If7b80c8725d9a8183d2df05c824461e8ee5f45d0 --- nova/network/neutron.py | 29 +++++-- nova/objects/request_spec.py | 48 +++++++++++ .../test_servers_resource_request.py | 20 ----- nova/tests/unit/network/test_neutron.py | 81 ++++++++++++++++++- nova/tests/unit/objects/test_request_spec.py | 58 +++++++++++++ 5 files changed, 208 insertions(+), 28 deletions(-) diff --git a/nova/network/neutron.py b/nova/network/neutron.py index cf5d366ab9..eab727881e 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -2065,6 +2065,8 @@ class API: tunneled = False neutron = get_client(context, admin=True) + has_extended_resource_request_extension = ( + self._has_extended_resource_request_extension(context, neutron)) resource_requests = [] for request_net in requested_networks: @@ -2111,13 +2113,26 @@ class API: resource_requests.extend(dp_request_groups) if resource_request: - # NOTE(gibi): explicitly orphan the RequestGroup by setting - # context=None as we never intended to save it to the DB. - resource_requests.append( - objects.RequestGroup.from_port_request( - context=None, - port_uuid=request_net.port_id, - port_resource_request=resource_request)) + if has_extended_resource_request_extension: + # need to handle the new resource request format + # NOTE(gibi): explicitly orphan the RequestGroup by + # setting context=None as we never intended to save it + # to the DB. + resource_requests.extend( + objects.RequestGroup.from_extended_port_request( + context=None, + port_resource_request=resource_request)) + else: + # keep supporting the old format of the + # resource_request + # NOTE(gibi): explicitly orphan the RequestGroup by + # setting context=None as we never intended to save it + # to the DB. + resource_requests.append( + objects.RequestGroup.from_port_request( + context=None, + port_uuid=request_net.port_id, + port_resource_request=resource_request)) elif request_net.network_id and not request_net.auto_allocate: network_id = request_net.network_id diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index ce4529b2a2..652f5fd074 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -1186,6 +1186,54 @@ class RequestGroup(base.NovaEphemeralObject): obj.obj_set_defaults() return obj + @classmethod + def from_extended_port_request(cls, context, port_resource_request): + """Create the group objects from the resource request of a neutron port + + :param context: the request context + :param port_resource_request: the resource_request attribute of the + neutron port + For example: + + port_resource_request = { + "request_groups": + [ + { + "id": + "required": [CUSTOM_PHYSNET_2, + CUSTOM_VNIC_TYPE_NORMAL], + "resources":{ + NET_PACKET_RATE_KILOPACKET_PER_SEC: 1000 + } + }, + { + "id": + "required": [CUSTOM_PHYSNET_2, + CUSTOM_VNIC_TYPE_NORMAL], + "resources": { + "NET_BW_IGR_KILOBIT_PER_SEC": 1000, + "NET_BW_EGR_KILOBIT_PER_SEC": 1000, + }, + } + ] + } + """ + group_objs = [] + for group in port_resource_request.get("request_groups", []): + # NOTE(gibi): Placement rejects allocation candidates where a + # request group has traits but no resources specified. This is why + # resources are handled as mandatory below but not traits. + obj = cls( + context=context, + use_same_provider=True, + resources=group['resources'], + required_traits=set(group.get('required', [])), + requester_id=group['id']) + obj.obj_set_defaults() + group_objs.append(obj) + + return group_objs + def obj_make_compatible(self, primitive, target_version): super(RequestGroup, self).obj_make_compatible( primitive, target_version) diff --git a/nova/tests/functional/test_servers_resource_request.py b/nova/tests/functional/test_servers_resource_request.py index d844452ac4..e77adcd275 100644 --- a/nova/tests/functional/test_servers_resource_request.py +++ b/nova/tests/functional/test_servers_resource_request.py @@ -1674,21 +1674,6 @@ class MultiGroupResourceRequestBasedSchedulingTest( def test_interface_attach_with_resource_request_no_candidates(self): super().test_interface_attach_with_resource_request_no_candidates() - @unittest.expectedFailure - def test_interface_attach_with_resource_request_pci_claim_fails(self): - super().test_interface_attach_with_resource_request_pci_claim_fails() - - @unittest.expectedFailure - def test_interface_attach_sriov_with_qos_pci_update_fails(self): - super().test_interface_attach_sriov_with_qos_pci_update_fails() - - @unittest.expectedFailure - def test_interface_attach_sriov_with_qos_pci_update_fails_cleanup_fails( - self - ): - super( - ).test_interface_attach_sriov_with_qos_pci_update_fails_cleanup_fails() - @unittest.expectedFailure def test_interface_detach_with_port_with_bandwidth_request(self): super().test_interface_detach_with_port_with_bandwidth_request() @@ -1701,11 +1686,6 @@ class MultiGroupResourceRequestBasedSchedulingTest( def test_two_sriov_ports_one_with_request_two_available_pfs(self): super().test_two_sriov_ports_one_with_request_two_available_pfs() - @unittest.expectedFailure - def test_one_sriov_port_no_vf_and_bandwidth_available_on_the_same_pf(self): - super( - ).test_one_sriov_port_no_vf_and_bandwidth_available_on_the_same_pf() - @unittest.expectedFailure def test_sriov_macvtap_port_with_resource_request(self): super().test_sriov_macvtap_port_with_resource_request() diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 0a60ae125e..cec0a053ed 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -5920,7 +5920,12 @@ class TestAPI(TestAPIBase): port_uuid=uuids.trusted_port, port_resource_request=mock.sentinel.resource_request2), ]) - mock_has_extended_res_req.assert_called_once_with(self.context) + mock_has_extended_res_req.assert_has_calls( + [ + mock.call(self.context), + mock.call(self.context, getclient.return_value), + ] + ) @mock.patch.object( neutronapi.API, '_has_extended_resource_request_extension', @@ -6035,6 +6040,80 @@ class TestAPI(TestAPIBase): self.api.create_resource_requests, self.context, requested_networks, pci_requests=None) + @mock.patch( + 'nova.network.neutron.API.support_create_with_resource_request', + new=mock.Mock(return_value=True)) + @mock.patch.object( + neutronapi.API, '_has_extended_resource_request_extension', + return_value=True) + @mock.patch( + 'nova.objects.request_spec.RequestGroup.from_extended_port_request') + @mock.patch.object(neutronapi.API, '_get_physnet_tunneled_info') + @mock.patch.object(neutronapi.API, "_get_port_vnic_info") + @mock.patch.object(neutronapi, 'get_client') + def test_create_resource_request_extended( + self, getclient, mock_get_port_vnic_info, + mock_get_physnet_tunneled_info, mock_from_port_request, + mock_has_extended_res_req + ): + requested_networks = objects.NetworkRequestList( + objects=[ + objects.NetworkRequest(port_id=uuids.portid_1), + objects.NetworkRequest(port_id=uuids.portid_2), + objects.NetworkRequest(port_id=uuids.portid_3), + ]) + pci_requests = objects.InstancePCIRequests(requests=[]) + mock_get_port_vnic_info.side_effect = [ + (model.VNIC_TYPE_NORMAL, None, 'netN', + mock.sentinel.resource_request1, None, None), + (model.VNIC_TYPE_NORMAL, None, 'netN', + mock.sentinel.resource_request2, None, None), + (model.VNIC_TYPE_NORMAL, None, 'netN', None, None, None), + ] + # _get_physnet_tunneled_info should be called for every NetworkRequest + mock_get_physnet_tunneled_info.side_effect = [ + ('physnet1', False), ('physnet2', False), ('physnet3', False)] + api = neutronapi.API() + + # Simulate that both port1 and port2 have such an extended resource + # request that is resolved to more than one request groups, but port3 + # has no request + mock_from_port_request.side_effect = [ + [ + mock.sentinel.port1_request_group1, + mock.sentinel.port1_request_group2, + ], + [ + mock.sentinel.port2_request_group1, + mock.sentinel.port2_request_group2, + ], + ] + + result = api.create_resource_requests( + self.context, requested_networks, pci_requests) + network_metadata, port_resource_requests = result + + # assert that all the request groups are collected from both ports + self.assertEqual( + [ + mock.sentinel.port1_request_group1, + mock.sentinel.port1_request_group2, + mock.sentinel.port2_request_group1, + mock.sentinel.port2_request_group2, + ], + port_resource_requests) + + mock_from_port_request.assert_has_calls([ + mock.call( + context=None, + port_resource_request=mock.sentinel.resource_request1), + mock.call( + context=None, + port_resource_request=mock.sentinel.resource_request2), + ]) + mock_has_extended_res_req.assert_called_once_with( + self.context, getclient.return_value) + @mock.patch.object(neutronapi, 'get_client') def test_associate_floating_ip_conflict(self, mock_get_client): """Tests that if Neutron raises a Conflict we handle it and re-raise diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index 2963909c32..93a1672cc8 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -1044,6 +1044,64 @@ class TestRequestGroupObject(test.NoDBTestCase): self.assertEqual([], rg.aggregates) self.assertEqual([], rg.provider_uuids) + def test_from_extended_port_request(self): + port_resource_request = { + "request_groups": [ + { + "id": uuids.group_id1, + "resources": { + "NET_BW_IGR_KILOBIT_PER_SEC": 1000, + "NET_BW_EGR_KILOBIT_PER_SEC": 1000}, + "required": [ + "CUSTOM_PHYSNET2", "CUSTOM_VNIC_TYPE_NORMAL"] + }, + { + "id": uuids.group_id2, + "resources": { + "NET_PACKET_RATE_KILOPACKET_PER_SEC": 1000 + }, + "required": ["CUSTOM_VNIC_TYPE_NORMAL"] + } + ], + "same_subtree": [ + uuids.group_id1, + uuids.group_id2, + ], + } + + rgs = request_spec.RequestGroup.from_extended_port_request( + self.context, port_resource_request) + + # two separate groups are returned + self.assertEqual(2, len(rgs)) + + self.assertTrue(rgs[0].use_same_provider) + self.assertEqual( + {"NET_BW_IGR_KILOBIT_PER_SEC": 1000, + "NET_BW_EGR_KILOBIT_PER_SEC": 1000}, + rgs[0].resources) + self.assertEqual( + {"CUSTOM_PHYSNET2", "CUSTOM_VNIC_TYPE_NORMAL"}, + rgs[0].required_traits) + self.assertEqual(uuids.group_id1, rgs[0].requester_id) + # and the rest is defaulted + self.assertEqual(set(), rgs[0].forbidden_traits) + self.assertEqual([], rgs[0].aggregates) + self.assertEqual([], rgs[0].provider_uuids) + + self.assertTrue(rgs[1].use_same_provider) + self.assertEqual( + {"NET_PACKET_RATE_KILOPACKET_PER_SEC": 1000}, + rgs[1].resources) + self.assertEqual( + {"CUSTOM_VNIC_TYPE_NORMAL"}, + rgs[1].required_traits) + self.assertEqual(uuids.group_id2, rgs[1].requester_id) + # and the rest is defaulted + self.assertEqual(set(), rgs[1].forbidden_traits) + self.assertEqual([], rgs[1].aggregates) + self.assertEqual([], rgs[1].provider_uuids) + def test_compat_requester_and_provider(self): req_obj = objects.RequestGroup( requester_id=uuids.requester, provider_uuids=[uuids.rp1],