openstack/manila
Code Issues Proposed changes

Create ports as disabled with external networks

Browse Source 
NeutronNetworkPlugin allows the use of external networks
as share networks. Deployers use this as a way to utilize
Neutron's IPAM capabilities. To provide IP addresses out
of a network, Neutron would need to create ports on the
network, albeit these ports are unused.

A recent change in OVN (a popular Neutron ML2 plugin) [1]
necessitates Manila to explicitly create ports as being
"DOWN" to prevent incorrect ARP responses from OVN; the
eventual, external ports must respond to the ARP requests
from clients.

Change-Id: Icd5b1a2087a4e38e9d0b65f9c0d9a85abad69f07
Closes-Bug: #2074504
(cherry picked from commit 53c8b69c016010e2bbbf3220bcf771a2e9dd8b91)
(cherry picked from commit 55865c5a8be4c7b0100dab291e4e2735c9c0e59d)
(cherry picked from commit f509be3364eeba9be1c8f69a2dc03beb0a0006af)
(cherry picked from commit 58dbf997ea93e5c38b6e7db01a85ae8d9836cd9e)
This commit is contained in:
Goutham Pacha Ravi 2024-07-30 23:05:45 -07:00
parent 0d1e5bee4c
commit fab08ff7b2
3 changed files with 141 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
manila/network/neutron/neutron_network_plugin.py
View File

@ -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:

101
manila/tests/network/neutron/test_neutron_plugin.py
View File

@ -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)

9
releasenotes/notes/bug-2074504-disable-ports-on-neutron-ext-nets-af3ff56da9a928df.yaml Normal file
View File

@ -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
<https://bugs.launchpad.net/manila/+bug/2074504>`_.