diff --git a/manila/network/neutron/neutron_network_plugin.py b/manila/network/neutron/neutron_network_plugin.py index 727d58edcf..c44c4dc2b6 100644 --- a/manila/network/neutron/neutron_network_plugin.py +++ b/manila/network/neutron/neutron_network_plugin.py @@ -123,14 +123,17 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): def include_network_info(self, share_network_subnet): """Includes share-network-subnet with plugin specific data.""" - self._store_neutron_net_info(None, share_network_subnet, save_db=False) + self._store_and_get_neutron_net_info(None, + share_network_subnet, + save_db=False) - def _store_neutron_net_info(self, context, share_network_subnet, - save_db=True): - self._save_neutron_network_data(context, share_network_subnet, - save_db=save_db) + def _store_and_get_neutron_net_info(self, context, share_network_subnet, + save_db=True): + is_external_network = self._save_neutron_network_data( + context, share_network_subnet, save_db=save_db) self._save_neutron_subnet_data(context, share_network_subnet, save_db=save_db) + return is_external_network def allocate_network(self, context, share_server, share_network=None, share_network_subnet=None, **kwargs): @@ -156,17 +159,23 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): self._verify_share_network(share_server['id'], share_network) self._verify_share_network_subnet(share_server['id'], share_network_subnet) - self._store_neutron_net_info(context, share_network_subnet) + is_external_network = self._store_and_get_neutron_net_info( + context, share_network_subnet) allocation_count = kwargs.get('count', 1) device_owner = kwargs.get('device_owner', 'share') ports = [] for current_count in range(0, allocation_count): - ports.append(self._create_port( - context, share_server, share_network, - share_network_subnet, device_owner, - current_count)) + ports.append( + self._create_port(context, + share_server, + share_network, + share_network_subnet, + device_owner, + current_count, + is_external_network=is_external_network), + ) return ports @@ -176,7 +185,7 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): self._verify_share_network_subnet(share_server['id'], share_network_subnet) - self._store_neutron_net_info(context, share_network_subnet) + self._store_and_get_neutron_net_info(context, share_network_subnet) # We begin matching the allocations to known neutron ports and # finally return the non-consumed allocations @@ -313,23 +322,37 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): self._delete_port(context, port) def _get_port_create_args(self, share_server, share_network_subnet, - device_owner, count=0): + device_owner, count=0, + is_external_network=False): return { "network_id": share_network_subnet['neutron_net_id'], "subnet_id": share_network_subnet['neutron_subnet_id'], "device_owner": 'manila:' + device_owner, "device_id": share_server.get('id'), - "name": share_server.get('id') + '_' + str(count) + "name": share_server.get('id') + '_' + str(count), + # NOTE (gouthamr): we create disabled ports with external networks + # since the actual ports are not managed by neutron. The ports + # neutron creates merely assist in IPAM. + "admin_state_up": not is_external_network, } def _create_port(self, context, share_server, share_network, - share_network_subnet, device_owner, count=0): + share_network_subnet, device_owner, count=0, + is_external_network=False): create_args = self._get_port_create_args( - share_server, share_network_subnet, device_owner, count) + share_server, share_network_subnet, device_owner, count, + is_external_network=is_external_network) port = self.neutron_api.create_port( share_network['project_id'], **create_args) + if is_external_network: + msg = ( + f"Port '{port['id']}' is disabled to prevent improper " + f"routing on an external network." + ) + LOG.info(msg) + ip_address = self._get_matched_ip_address( port['fixed_ips'], share_network_subnet['ip_version']) port_dict = { @@ -380,6 +403,7 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): share_network_subnet['neutron_net_id']) segmentation_id = None network_type = None + is_external_network = net_info.get('router:external', False) if self._is_neutron_multi_segment(share_network_subnet, net_info): # we have a multi segment network and need to identify the @@ -414,6 +438,8 @@ class NeutronNetworkPlugin(network.NetworkBaseAPI): self.db.share_network_subnet_update( context, share_network_subnet['id'], provider_nw_dict) + return is_external_network + def _save_neutron_subnet_data(self, context, share_network_subnet, save_db=True): subnet_info = self.neutron_api.get_subnet( @@ -579,10 +605,13 @@ class NeutronBindNetworkPlugin(NeutronNetworkPlugin): raise exception.NetworkBindException(msg) def _get_port_create_args(self, share_server, share_network_subnet, - device_owner, count=0): + device_owner, count=0, + is_external_network=False): arguments = super( NeutronBindNetworkPlugin, self)._get_port_create_args( - share_server, share_network_subnet, device_owner, count) + share_server, share_network_subnet, device_owner, count, + is_external_network=is_external_network + ) arguments['host_id'] = self.config.neutron_host_id arguments['binding:vnic_type'] = self.config.neutron_vnic_type if self.binding_profiles: diff --git a/manila/tests/network/neutron/test_neutron_plugin.py b/manila/tests/network/neutron/test_neutron_plugin.py index f9b9c87003..14661ca8ac 100644 --- a/manila/tests/network/neutron/test_neutron_plugin.py +++ b/manila/tests/network/neutron/test_neutron_plugin.py @@ -215,6 +215,56 @@ class NeutronNetworkPluginTest(test.TestCase): with test_utils.create_temp_config_with_opts(config_data): return plugin.NeutronNetworkPlugin() + @mock.patch.object(db_api, 'network_allocation_create', + mock.Mock(return_values=fake_network_allocation)) + @mock.patch.object(db_api, 'share_network_get', + mock.Mock(return_value=fake_share_network)) + @mock.patch.object(db_api, 'share_server_get', + mock.Mock(return_value=fake_share_server)) + def test_allocate_network_external_neutron_network(self): + has_provider_nw_ext = mock.patch.object( + self.plugin, '_has_provider_network_extension').start() + has_provider_nw_ext.return_value = True + save_nw_data = mock.patch.object(self.plugin, + '_save_neutron_network_data', + mock.Mock(return_value=True)).start() + save_subnet_data = mock.patch.object( + self.plugin, + '_save_neutron_subnet_data').start() + + with mock.patch.object(self.plugin.neutron_api, 'create_port', + mock.Mock(return_value=fake_neutron_port)): + self.plugin.allocate_network( + self.fake_context, + fake_share_server, + fake_share_network, + fake_share_network_subnet, + allocation_info={'count': 1}) + + has_provider_nw_ext.assert_any_call() + save_nw_data.assert_called_once_with(self.fake_context, + fake_share_network_subnet, + save_db=True) + save_subnet_data.assert_called_once_with(self.fake_context, + fake_share_network_subnet, + save_db=True) + self.plugin.neutron_api.create_port.assert_called_once_with( + fake_share_network['project_id'], + network_id=fake_share_network_subnet['neutron_net_id'], + subnet_id=fake_share_network_subnet['neutron_subnet_id'], + device_owner='manila:share', + device_id=fake_share_network['id'], + name=fake_share_network['id'] + '_0', + admin_state_up=False, + ) + db_api.network_allocation_create.assert_called_once_with( + self.fake_context, + fake_network_allocation) + + has_provider_nw_ext.stop() + save_nw_data.stop() + save_subnet_data.stop() + @mock.patch.object(db_api, 'network_allocation_create', mock.Mock(return_values=fake_network_allocation)) @mock.patch.object(db_api, 'share_network_get', @@ -253,7 +303,9 @@ class NeutronNetworkPluginTest(test.TestCase): subnet_id=fake_share_network_subnet['neutron_subnet_id'], device_owner='manila:share', device_id=fake_share_network['id'], - name=fake_share_network['id'] + '_0') + name=fake_share_network['id'] + '_0', + admin_state_up=False, + ) db_api.network_allocation_create.assert_called_once_with( self.fake_context, fake_network_allocation) @@ -291,14 +343,18 @@ class NeutronNetworkPluginTest(test.TestCase): subnet_id=fake_share_network_subnet['neutron_subnet_id'], device_owner='manila:share', device_id=fake_share_network['id'], - name=fake_share_network['id'] + '_0'), + name=fake_share_network['id'] + '_0', + admin_state_up=False, + ), mock.call( fake_share_network['project_id'], network_id=fake_share_network_subnet['neutron_net_id'], subnet_id=fake_share_network_subnet['neutron_subnet_id'], device_owner='manila:share', device_id=fake_share_network['id'], - name=fake_share_network['id'] + '_1'), + name=fake_share_network['id'] + '_1', + admin_state_up=False, + ) ] db_api_calls = [ mock.call(self.fake_context, fake_network_allocation), @@ -356,7 +412,7 @@ class NeutronNetworkPluginTest(test.TestCase): neutron_ports[3]['id'] = 'fake_port_id_3' self.mock_object(self.plugin, '_verify_share_network_subnet') - self.mock_object(self.plugin, '_store_neutron_net_info') + self.mock_object(self.plugin, '_store_and_get_neutron_net_info') self.mock_object(self.plugin.neutron_api, 'list_ports', mock.Mock(return_value=neutron_ports)) @@ -433,7 +489,7 @@ class NeutronNetworkPluginTest(test.TestCase): self.plugin._verify_share_network_subnet.assert_called_once_with( fake_share_server['id'], fake_share_network_subnet) - self.plugin._store_neutron_net_info( + self.plugin._store_and_get_neutron_net_info( self.fake_context, fake_share_network_subnet) def test__get_ports_respective_to_ips_multiple_fixed_ips(self): @@ -555,6 +611,7 @@ class NeutronNetworkPluginTest(test.TestCase): 'provider:network_type': 'vlan', 'provider:segmentation_id': 1000, 'mtu': 1509, + 'router:external': True, } share_nw_update_dict = { 'network_type': 'vlan', @@ -565,8 +622,8 @@ class NeutronNetworkPluginTest(test.TestCase): with mock.patch.object(self.plugin.neutron_api, 'get_network', mock.Mock(return_value=neutron_nw_info)): - self.plugin._save_neutron_network_data(self.fake_context, - fake_share_network_subnet) + is_external_network = self.plugin._save_neutron_network_data( + self.fake_context, fake_share_network_subnet) self.plugin.neutron_api.get_network.assert_called_once_with( fake_share_network_subnet['neutron_net_id']) @@ -574,6 +631,7 @@ class NeutronNetworkPluginTest(test.TestCase): self.fake_context, fake_share_network_subnet['id'], share_nw_update_dict) + self.assertTrue(is_external_network) @mock.patch.object(db_api, 'share_network_subnet_update', mock.Mock()) def test_save_neutron_network_data_multi_segment(self): @@ -945,7 +1003,8 @@ class NeutronBindNetworkPluginTest(test.TestCase): self.mock_object(self.bind_plugin, '_has_provider_network_extension') self.bind_plugin._has_provider_network_extension.return_value = True save_nw_data = self.mock_object(self.bind_plugin, - '_save_neutron_network_data') + '_save_neutron_network_data', + mock.Mock(return_value=False)) save_subnet_data = self.mock_object(self.bind_plugin, '_save_neutron_subnet_data') self.mock_object(self.bind_plugin, '_wait_for_ports_bind') @@ -982,6 +1041,7 @@ class NeutronBindNetworkPluginTest(test.TestCase): 'device_owner': 'manila:share', 'device_id': fake_share_network['id'], 'name': fake_share_network['id'] + '_0', + 'admin_state_up': True, } self.bind_plugin.neutron_api.create_port.assert_called_once_with( fake_share_network['project_id'], **expected_kwargs) @@ -1065,6 +1125,7 @@ class NeutronBindNetworkPluginTest(test.TestCase): 'device_owner': 'manila:share', 'device_id': fake_share_network_multi['id'], 'name': fake_share_network['id'] + '_0', + 'admin_state_up': True, } self.bind_plugin.neutron_api.create_port.assert_called_once_with( fake_share_network_multi['project_id'], **expected_kwargs) @@ -1181,6 +1242,7 @@ class NeutronBindNetworkPluginTest(test.TestCase): 'device_owner': 'manila:' + fake_device_owner, 'device_id': fake_share_server['id'], 'name': fake_share_server['id'] + '_0', + 'admin_state_up': True, } if neutron_binding_profiles: expected_create_args['binding:profile'] = { @@ -1235,7 +1297,8 @@ class NeutronBindNetworkPluginTest(test.TestCase): 'subnet_id': fake_share_network_subnet['neutron_subnet_id'], 'device_owner': 'manila:' + fake_device_owner, 'device_id': fake_share_server['id'], - 'name': fake_share_server['id'] + '_0' + 'name': fake_share_server['id'] + '_0', + 'admin_state_up': True, } self.assertEqual(expected_create_args, create_args) @@ -1480,7 +1543,8 @@ class NeutronBindSingleNetworkPluginTest(test.TestCase): self.mock_object(self.bind_plugin, '_has_provider_network_extension') self.bind_plugin._has_provider_network_extension.return_value = True save_nw_data = self.mock_object(self.bind_plugin, - '_save_neutron_network_data') + '_save_neutron_network_data', + mock.Mock(return_value=False)) save_subnet_data = self.mock_object(self.bind_plugin, '_save_neutron_subnet_data') self.mock_object(self.bind_plugin, '_wait_for_ports_bind') @@ -1513,6 +1577,7 @@ class NeutronBindSingleNetworkPluginTest(test.TestCase): 'device_owner': 'manila:share', 'device_id': fake_share_network['id'], 'name': fake_share_network['id'] + '_0', + 'admin_state_up': True, } self.bind_plugin.neutron_api.create_port.assert_called_once_with( fake_share_network['project_id'], **expected_kwargs) @@ -1621,6 +1686,7 @@ class NeutronBindSingleNetworkPluginTest(test.TestCase): 'device_owner': 'manila:' + fake_device_owner, 'device_id': fake_share_server['id'], 'name': fake_share_server['id'] + '_0', + 'admin_state_up': True, } if neutron_binding_profiles: expected_create_args['binding:profile'] = { @@ -1675,7 +1741,8 @@ class NeutronBindSingleNetworkPluginTest(test.TestCase): 'subnet_id': fake_share_network_subnet['neutron_subnet_id'], 'device_owner': 'manila:' + fake_device_owner, 'device_id': fake_share_server['id'], - 'name': fake_share_server['id'] + '_0' + 'name': fake_share_server['id'] + '_0', + 'admin_state_up': True, } self.assertEqual(expected_create_args, create_args) @@ -1709,7 +1776,8 @@ class NeutronBindNetworkPluginWithNormalTypeTest(test.TestCase): self.mock_object(self.bind_plugin, '_has_provider_network_extension') self.bind_plugin._has_provider_network_extension.return_value = True save_nw_data = self.mock_object(self.bind_plugin, - '_save_neutron_network_data') + '_save_neutron_network_data', + mock.Mock(return_value=False)) save_subnet_data = self.mock_object(self.bind_plugin, '_save_neutron_subnet_data') self.mock_object(self.bind_plugin, '_wait_for_ports_bind') @@ -1745,6 +1813,7 @@ class NeutronBindNetworkPluginWithNormalTypeTest(test.TestCase): 'device_owner': 'manila:share', 'device_id': fake_share_server['id'], 'name': fake_share_server['id'] + '_0', + 'admin_state_up': True, } self.bind_plugin.neutron_api.create_port.assert_called_once_with( fake_share_network['project_id'], **expected_kwargs) @@ -1802,7 +1871,8 @@ class NeutronBindSingleNetworkPluginWithNormalTypeTest(test.TestCase): self.mock_object(self.bind_plugin, '_has_provider_network_extension') self.bind_plugin._has_provider_network_extension.return_value = True save_nw_data = self.mock_object(self.bind_plugin, - '_save_neutron_network_data') + '_save_neutron_network_data', + mock.Mock(return_value=False)) save_subnet_data = self.mock_object(self.bind_plugin, '_save_neutron_subnet_data') self.mock_object(self.bind_plugin, '_wait_for_ports_bind') @@ -1835,6 +1905,7 @@ class NeutronBindSingleNetworkPluginWithNormalTypeTest(test.TestCase): 'device_owner': 'manila:share', 'device_id': fake_share_network['id'], 'name': fake_share_network['id'] + '_0', + 'admin_state_up': True, } self.bind_plugin.neutron_api.create_port.assert_called_once_with( fake_share_network['project_id'], **expected_kwargs) @@ -1907,7 +1978,7 @@ class NeutronBindSingleNetworkPluginWithNormalTypeTest(test.TestCase): def test_include_network_info(self): instance = self._setup_include_network_info() - self.mock_object(instance, '_store_neutron_net_info') + self.mock_object(instance, '_store_and_get_neutron_net_info') instance.include_network_info(fake_share_network) - instance._store_neutron_net_info.assert_called_once_with( + instance._store_and_get_neutron_net_info.assert_called_once_with( None, fake_share_network, save_db=False) diff --git a/releasenotes/notes/bug-2074504-disable-ports-on-neutron-ext-nets-af3ff56da9a928df.yaml b/releasenotes/notes/bug-2074504-disable-ports-on-neutron-ext-nets-af3ff56da9a928df.yaml new file mode 100644 index 0000000000..1130592934 --- /dev/null +++ b/releasenotes/notes/bug-2074504-disable-ports-on-neutron-ext-nets-af3ff56da9a928df.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + When using Neutron networks tagged as `external` (unmanaged + provider networks) as share networks, Manila now creates ports + with `admin_state_up=False` (disabled). This change addresses + ARP failures that can occur when using OVN as the Neutron ML2 + plugin. For more information, refer to `bug 2074504 + `_.