From de72526791968f6ac375740c26ef07aff37f6a18 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 26 Jun 2018 10:55:55 +0100 Subject: [PATCH] network: Always retrieve network information if available We're going to need to consume information about the physnet and tunnel status of networks. While we don't currently retrieve the latter, we do retrieve the former but only for SR-IOV ports. Modify this so physnet information is retrieved for any non-auto and non-null network requests. This is stored in a new object, 'NetworkMetadata', which defines fields for both physnets and tunneled networks. The latter attribute of this object is unset for now, but this is not an issue as the value returned from this function is not yet used. Part of blueprint numa-aware-vswitches Change-Id: I3dde6074d69e299f2844675ef968c8f949722395 --- nova/network/base_api.py | 3 ++ nova/network/neutronv2/api.py | 31 +++++++++++--- nova/objects/__init__.py | 1 + nova/objects/fields.py | 5 +++ nova/objects/network_metadata.py | 46 +++++++++++++++++++++ nova/tests/unit/network/test_neutronv2.py | 50 +++++++++++++++++++---- nova/tests/unit/objects/test_objects.py | 1 + 7 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 nova/objects/network_metadata.py diff --git a/nova/network/base_api.py b/nova/network/base_api.py index c66f973ec5b2..0a6dc956472f 100644 --- a/nova/network/base_api.py +++ b/nova/network/base_api.py @@ -280,6 +280,9 @@ class NetworkAPI(base.Base): :param pci_requests: The list of PCI requests to which additional PCI requests created here will be added. :type pci_requests: nova.objects.InstancePCIRequests + + :returns: An instance of ``objects.NetworkMetadata`` for use by the + scheduler or None. """ raise NotImplementedError() diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index b9de2321b0f7..ad21b6cfa233 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -1618,26 +1618,43 @@ class API(base_api.NetworkAPI): pci_requests=None): """Retrieve all information for the networks passed at the time of creating the server. + + :param context: The request context. + :param requested_networks: The networks requested for the server. + :type requested_networks: nova.objects.RequestedNetworkList + :param pci_requests: The list of PCI requests to which additional PCI + requests created here will be added. + :type pci_requests: nova.objects.InstancePCIRequests + + :returns: An instance of ``objects.NetworkMetadata`` for use by the + scheduler or None. """ if not requested_networks or requested_networks.no_allocate: - return + return None + + physnets = set() + tunneled = False neutron = get_client(context, admin=True) for request_net in requested_networks: physnet = None trusted = None vnic_type = network_model.VNIC_TYPE_NORMAL + pci_request_id = None if request_net.port_id: vnic_type, trusted, network_id = self._get_port_vnic_info( context, neutron, request_net.port_id) physnet = self._get_physnet_info( context, neutron, network_id) - LOG.debug("Creating PCI device request for port_id=%s, " - "vnic_type=%s, phynet_name=%s, trusted=%s", - request_net.port_id, vnic_type, physnet, - trusted) - pci_request_id = None + elif request_net.network_id and not request_net.auto_allocate: + network_id = request_net.network_id + physnet = self._get_physnet_info( + context, neutron, network_id) + + if physnet: + physnets.add(physnet) + if vnic_type in network_model.VNIC_TYPES_SRIOV: # TODO(moshele): To differentiate between the SR-IOV legacy # and SR-IOV ovs hardware offload we will leverage the nic @@ -1665,6 +1682,8 @@ class API(base_api.NetworkAPI): # Add pci_request_id into the requested network request_net.pci_request_id = pci_request_id + return objects.NetworkMetadata(physnets=physnets, tunneled=tunneled) + def _can_auto_allocate_network(self, context, neutron): """Helper method to determine if we can auto-allocate networks diff --git a/nova/objects/__init__.py b/nova/objects/__init__.py index 08c8cd03dc54..34a87c06565e 100644 --- a/nova/objects/__init__.py +++ b/nova/objects/__init__.py @@ -57,6 +57,7 @@ def register_all(): __import__('nova.objects.migration_context') __import__('nova.objects.monitor_metric') __import__('nova.objects.network') + __import__('nova.objects.network_metadata') __import__('nova.objects.network_request') __import__('nova.objects.numa') __import__('nova.objects.pci_device') diff --git a/nova/objects/fields.py b/nova/objects/fields.py index 23b4f6520a71..7d716c6affa7 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -77,6 +77,7 @@ PCIAddressField = fields.PCIAddressField Enum = fields.Enum Field = fields.Field FieldType = fields.FieldType +String = fields.String Set = fields.Set Dict = fields.Dict List = fields.List @@ -89,6 +90,10 @@ IPV4Network = fields.IPV4Network IPV6Network = fields.IPV6Network +class SetOfStringsField(AutoTypedField): + AUTO_TYPE = Set(String()) + + class BaseNovaEnum(Enum): def __init__(self, **kwargs): super(BaseNovaEnum, self).__init__(valid_values=self.__class__.ALL) diff --git a/nova/objects/network_metadata.py b/nova/objects/network_metadata.py new file mode 100644 index 000000000000..e01afb384e2c --- /dev/null +++ b/nova/objects/network_metadata.py @@ -0,0 +1,46 @@ +# Copyright 2018 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova.objects import base +from nova.objects import fields + + +@base.NovaObjectRegistry.register +class NetworkMetadata(base.NovaObject): + """Hold aggregate metadata for a collection on networks. + + This object holds aggregate information for a collection of neutron + networks. There are two types of network collections we care about and use + this for: the collection of networks configured or requested for a guest + and the collection of networks available to a host. We want this + information to allow us to map a given neutron network to the logical NICs + it does or will use (or, rather, to identify the NUMA affinity of those + NICs and therefore the networks). Given that there are potentially tens of + thousands of neutron networks accessible from a given host and tens or + hundreds of networks configured for an instance, we need a way to group + networks by some common attribute that would identify the logical NIC it + would use. For L2 networks, this is the physnet attribute (e.g. + ``provider:physical_network=provider1``), which is an arbitrary string used + to distinguish between multiple physical (in the sense of physical wiring) + networks. For L3 (tunneled) networks, this is merely the fact that they are + L3 networks (e.g. ``provider:network_type=vxlan``) because, in neutron, + *all* L3 networks must use the same logical NIC. + """ + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'physnets': fields.SetOfStringsField(), + 'tunneled': fields.BooleanField(), + } diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 656ac2509d14..0676056dada4 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -4925,18 +4925,42 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): self.assertTrue(mock_log.called) @mock.patch.object(neutronapi, 'get_client') - def test_create_resource_requests_no_allocate(self, getclient): - """Tests that create_resource_requests is a noop if - networks are specifically requested to not be allocated. + def test_create_resource_requests_no_allocate(self, mock_get_client): + """Ensure physnet info is not retrieved when networks are not to be + allocated. """ requested_networks = objects.NetworkRequestList(objects=[ objects.NetworkRequest(network_id=net_req_obj.NETWORK_ID_NONE) ]) pci_requests = objects.InstancePCIRequests() api = neutronapi.API() - api.create_resource_requests( + + network_metadata = api.create_resource_requests( self.context, requested_networks, pci_requests) - self.assertFalse(getclient.called) + + self.assertFalse(mock_get_client.called) + self.assertIsNone(network_metadata) + + @mock.patch.object(neutronapi.API, '_get_physnet_info') + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_create_resource_requests_auto_allocated(self, mock_get_client, + mock_get_physnet_info): + """Ensure physnet info is not retrieved for auto-allocated networks. + + This isn't possible so we shouldn't attempt to do it. + """ + requested_networks = objects.NetworkRequestList(objects=[ + objects.NetworkRequest(network_id=net_req_obj.NETWORK_ID_AUTO) + ]) + pci_requests = objects.InstancePCIRequests() + api = neutronapi.API() + + network_metadata = api.create_resource_requests( + self.context, requested_networks, pci_requests) + + mock_get_physnet_info.assert_not_called() + self.assertEqual(set(), network_metadata.physnets) + self.assertFalse(network_metadata.tunneled) @mock.patch.object(neutronapi.API, '_get_physnet_info') @mock.patch.object(neutronapi.API, "_get_port_vnic_info") @@ -4953,6 +4977,8 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): objects.NetworkRequest(port_id=uuids.portid_5), objects.NetworkRequest(port_id=uuids.trusted_port)]) pci_requests = objects.InstancePCIRequests(requests=[]) + # _get_port_vnic_info should be called for every NetworkRequest with a + # port_id attribute (so six times) mock_get_port_vnic_info.side_effect = [ (model.VNIC_TYPE_DIRECT, None, 'netN'), (model.VNIC_TYPE_NORMAL, None, 'netN'), @@ -4961,12 +4987,15 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): (model.VNIC_TYPE_DIRECT_PHYSICAL, None, 'netN'), (model.VNIC_TYPE_DIRECT, True, 'netN'), ] + # _get_physnet_info should be called for every NetworkRequest (so seven + # times) mock_get_physnet_info.side_effect = [ - 'physnet1', '', 'physnet1', 'physnet2', 'physnet3', 'physnet4', + 'physnet1', 'physnet1', '', 'physnet1', 'physnet2', 'physnet3', + 'physnet4', ] api = neutronapi.API() - api.create_resource_requests( + network_metadata = api.create_resource_requests( self.context, requested_networks, pci_requests) self.assertEqual(5, len(pci_requests.requests)) @@ -4986,6 +5015,13 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): else: self.assertNotIn(pci_request.PCI_TRUSTED_TAG, spec) + self.assertItemsEqual( + ['physnet1', 'physnet2', 'physnet3', 'physnet4'], + network_metadata.physnets) + # TODO(stephenfin): Expand this to be a positive check when we start + # retrieving this information + self.assertFalse(network_metadata.tunneled) + @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_objects.py b/nova/tests/unit/objects/test_objects.py index 457446908bb2..d0843ad70967 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1131,6 +1131,7 @@ object_data = { 'Network': '1.2-a977ab383aa462a479b2fae8211a5dde', 'NetworkInterfaceMetadata': '1.2-6f3d480b40fe339067b1c0dd4d656716', 'NetworkList': '1.2-69eca910d8fa035dfecd8ba10877ee59', + 'NetworkMetadata': '1.0-2cb8d21b34f87b0261d3e1d1ae5cf218', 'NetworkRequest': '1.2-af1ff2d986999fbb79377712794d82aa', 'NetworkRequestList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'PciDevice': '1.6-2a2612baaa1786679e52084e82ca7e66',