From 70538a5fbc8a09dbbd627bd6f9b5a3db7794a70d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 27 Jun 2018 14:38:26 +0100 Subject: [PATCH] network: Retrieve tunneled status in '_get_physnet_info' This determines whether the network is an L3 tunnel provider network and allows us to populate the appropriate attribute on the 'NetworkMetadata' object returned by the function. This requires renaming the function to represent its new purpose. Part of blueprint numa-aware-vswitches Change-Id: I535a64ab16abc9a7d34220fcc3dba36a4d663bfe --- nova/network/neutronv2/api.py | 33 ++++++--- nova/tests/unit/network/test_neutronv2.py | 82 +++++++++++++++++------ 2 files changed, 86 insertions(+), 29 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index ad21b6cfa2..ee439f2350 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -56,6 +56,7 @@ DEFAULT_SECGROUP = 'default' BINDING_PROFILE = 'binding:profile' BINDING_HOST_ID = 'binding:host_id' MIGRATING_ATTR = 'migrating_to' +L3_NETWORK_TYPES = ['vxlan', 'gre', 'geneve'] def reset_state(): @@ -1538,17 +1539,21 @@ class API(base_api.NetworkAPI): raise exception.FixedIpNotFoundForSpecificInstance( instance_uuid=instance.uuid, ip=address) - def _get_physnet_info(self, context, neutron, net_id): + def _get_physnet_tunneled_info(self, context, neutron, net_id): """Retrieve detailed network info. :param context: The request context. :param neutron: The neutron client object. :param net_id: The ID of the network to retrieve information for. - :return: Name of physical network as indicated by the - ``provider:physical_network`` attribute of the network, else None - if that attribute is not defined. + :return: A tuple containing the physnet name, if defined, and the + tunneled status of the network. If the network uses multiple + segments, the first segment that defines a physnet value will be + used for the physnet name. """ + def is_tunneled(net): + return net.get('provider:network_type') in L3_NETWORK_TYPES + if self._has_multi_provider_extension(context, neutron=neutron): network = neutron.show_network(net_id, fields='segments').get('network') @@ -1565,7 +1570,7 @@ class API(base_api.NetworkAPI): # physical networks. physnet_name = net.get('provider:physical_network') if physnet_name: - return physnet_name + return physnet_name, is_tunneled(net) # Raising here as at least one segment should # have a physical network provided. @@ -1575,8 +1580,9 @@ class API(base_api.NetworkAPI): raise exception.NovaException(message=msg) net = neutron.show_network( - net_id, fields=['provider:physical_network']).get('network') - return net.get('provider:physical_network') + net_id, fields=['provider:physical_network', + 'provider:network_type']).get('network') + return net.get('provider:physical_network'), is_tunneled(net) @staticmethod def _get_trusted_mode_from_port(port): @@ -1639,19 +1645,28 @@ class API(base_api.NetworkAPI): for request_net in requested_networks: physnet = None trusted = None + tunneled_ = False 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( + physnet, tunneled_ = self._get_physnet_tunneled_info( context, neutron, network_id) elif request_net.network_id and not request_net.auto_allocate: network_id = request_net.network_id - physnet = self._get_physnet_info( + physnet, tunneled_ = self._get_physnet_tunneled_info( context, neutron, network_id) + # All tunneled traffic must use the same logical NIC so we just + # need to know if there is one or more tunneled networks present. + tunneled = tunneled or tunneled_ + + # ...conversely, there can be multiple physnets, which will + # generally be mapped to different NICs, and some requested + # networks may use the same physnet. As a result, we need to know + # the *set* of physnets from every network requested if physnet: physnets.add(physnet) diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 0676056dad..fd2cd30998 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -3045,7 +3045,7 @@ class TestNeutronv2(TestNeutronv2Base): self.assertEqual(0, len(networks)) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) - def test_get_physnet_info_multi_segment(self, mock_get_client): + def test_get_physnet_tunneled_info_multi_segment(self, mock_get_client): api = neutronapi.API() self.mox.ResetAll() test_net = {'network': {'segments': @@ -3062,15 +3062,16 @@ class TestNeutronv2(TestNeutronv2Base): mock_client = mock_get_client() mock_client.list_extensions.return_value = test_ext_list mock_client.show_network.return_value = test_net - physnet_name = api._get_physnet_info( + physnet_name, tunneled = api._get_physnet_tunneled_info( self.context, mock_client, 'test-net') mock_client.show_network.assert_called_once_with( 'test-net', fields='segments') self.assertEqual('physnet10', physnet_name) + self.assertFalse(tunneled) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) - def test_get_physnet_info_vlan_with_multi_segment_ext( + def test_get_physnet_tunneled_info_vlan_with_multi_segment_ext( self, mock_get_client): api = neutronapi.API() self.mox.ResetAll() @@ -3084,15 +3085,17 @@ class TestNeutronv2(TestNeutronv2Base): mock_client = mock_get_client() mock_client.list_extensions.return_value = test_ext_list mock_client.show_network.return_value = test_net - physnet_name = api._get_physnet_info( + physnet_name, tunneled = api._get_physnet_tunneled_info( self.context, mock_client, 'test-net') mock_client.show_network.assert_called_with( - 'test-net', fields=['provider:physical_network']) + 'test-net', fields=['provider:physical_network', + 'provider:network_type']) self.assertEqual('physnet10', physnet_name) + self.assertFalse(tunneled) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) - def test_get_physnet_info_multi_segment_no_physnet( + def test_get_physnet_tunneled_info_multi_segment_no_physnet( self, mock_get_client): api = neutronapi.API() self.mox.ResetAll() @@ -3111,9 +3114,49 @@ class TestNeutronv2(TestNeutronv2Base): mock_client.list_extensions.return_value = test_ext_list mock_client.show_network.return_value = test_net self.assertRaises(exception.NovaException, - api._get_physnet_info, + api._get_physnet_tunneled_info, self.context, mock_client, 'test-net') + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_get_physnet_tunneled_info_tunneled( + self, mock_get_client): + api = neutronapi.API() + self.mox.ResetAll() + test_net = {'network': {'provider:network_type': 'vxlan'}} + test_ext_list = {'extensions': []} + + mock_client = mock_get_client() + mock_client.list_extensions.return_value = test_ext_list + mock_client.show_network.return_value = test_net + physnet_name, tunneled = api._get_physnet_tunneled_info( + self.context, mock_client, 'test-net') + + mock_client.show_network.assert_called_once_with( + 'test-net', fields=['provider:physical_network', + 'provider:network_type']) + self.assertTrue(tunneled) + self.assertIsNone(physnet_name) + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_get_phynet_tunneled_info_non_tunneled( + self, mock_get_client): + api = neutronapi.API() + self.mox.ResetAll() + test_net = {'network': {'provider:network_type': 'vlan'}} + test_ext_list = {'extensions': []} + + mock_client = mock_get_client() + mock_client.list_extensions.return_value = test_ext_list + mock_client.show_network.return_value = test_net + physnet_name, tunneled = api._get_physnet_tunneled_info( + self.context, mock_client, 'test-net') + + mock_client.show_network.assert_called_once_with( + 'test-net', fields=['provider:physical_network', + 'provider:network_type']) + self.assertFalse(tunneled) + self.assertIsNone(physnet_name) + def _test_get_port_vnic_info(self, mock_get_client, binding_vnic_type, expected_vnic_type): @@ -4941,10 +4984,10 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): self.assertFalse(mock_get_client.called) self.assertIsNone(network_metadata) - @mock.patch.object(neutronapi.API, '_get_physnet_info') + @mock.patch.object(neutronapi.API, '_get_physnet_tunneled_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): + mock_get_physnet_tunneled_info): """Ensure physnet info is not retrieved for auto-allocated networks. This isn't possible so we shouldn't attempt to do it. @@ -4958,15 +5001,15 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): network_metadata = api.create_resource_requests( self.context, requested_networks, pci_requests) - mock_get_physnet_info.assert_not_called() + mock_get_physnet_tunneled_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_physnet_tunneled_info') @mock.patch.object(neutronapi.API, "_get_port_vnic_info") @mock.patch.object(neutronapi, 'get_client') def test_create_resource_requests(self, getclient, - mock_get_port_vnic_info, mock_get_physnet_info): + mock_get_port_vnic_info, mock_get_physnet_tunneled_info): requested_networks = objects.NetworkRequestList( objects = [ objects.NetworkRequest(port_id=uuids.portid_1), @@ -4987,11 +5030,12 @@ 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', '', 'physnet1', 'physnet2', 'physnet3', - 'physnet4', + # _get_physnet_tunneled_info should be called for every NetworkRequest + # (so seven times) + mock_get_physnet_tunneled_info.side_effect = [ + ('physnet1', False), ('physnet1', False), ('', True), + ('physnet1', False), ('physnet2', False), ('physnet3', False), + ('physnet4', False), ] api = neutronapi.API() @@ -5018,9 +5062,7 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): 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) + self.assertTrue(network_metadata.tunneled) @mock.patch.object(neutronapi, 'get_client') def test_associate_floating_ip_conflict(self, mock_get_client):