openstack/neutron
Code Issues Proposed changes

OVN: Always try and create a metadata port on subnets

Browse Source 
When a subnet is updated, for example, to disable then
re-enable DHCP on it, if there is no metadata port it
will just return without trying to allocate an IP,
leaving DHCP unusable on the subnet.  This could happen
if an admin, even accidentally, deletes the DHCP port
on a subnet while DHCP is disabled.

This also makes OVN behave like ML2/OVS, which will
re-create the DHCP port when the enable_dhcp flag is
changed to false and back to true.

Change-Id: I943f2fb4db9dc33dc372f844d6133faff415befe
Closes-bug: #2015377
(cherry picked from commit 267efd2984)
(cherry picked from commit 0681f8b3ad)
This commit is contained in:
Brian Haley 2023-04-07 17:06:40 -04:00 committed by Edward Hope-Morley
parent ce16399ef3
commit 47b9511ec5
5 changed files with 121 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -2318,7 +2318,7 @@ class OVNClient(object):
mport_updated = False
if subnet['ip_version'] == const.IP_VERSION_4:
mport_updated = self.update_metadata_port(
context, network['id'], subnet=subnet)
context, network, subnet=subnet)
if subnet['ip_version'] == const.IP_VERSION_6 or not mport_updated:
# NOTE(ralonsoh): if IPv4 but the metadata port has not been
# updated, the DHPC options register has not been created.
@ -2338,7 +2338,7 @@ class OVNClient(object):
subnet['id'])['subnet']
if subnet['enable_dhcp'] or ovn_subnet:
self.update_metadata_port(context, network['id'], subnet=subnet)
self.update_metadata_port(context, network, subnet=subnet)
check_rev_cmd = self._nb_idl.check_revision_number(
subnet['id'], subnet, ovn_const.TYPE_SUBNETS)
@ -2434,8 +2434,9 @@ class OVNClient(object):
if not ovn_conf.is_ovn_metadata_enabled():
return
if self._find_metadata_port(context, network['id']):
return
metadata_port = self._find_metadata_port(context, network['id'])
if metadata_port:
return metadata_port
# Create a neutron port for DHCP/metadata services
filters = {'network_id': [network['id']]}
@ -2450,16 +2451,19 @@ class OVNClient(object):
}
}
# TODO(boden): rehome create_port into neutron-lib
p_utils.create_port(self._plugin, context, port)
return p_utils.create_port(self._plugin, context, port)
def update_metadata_port(self, context, network_id, subnet=None):
def update_metadata_port(self, context, network, subnet=None):
"""Update metadata port.
This function will allocate an IP address for the metadata port of
the given network in all its IPv4 subnets or the given subnet. Returns
"True" if the metadata port has been updated and "False" if OVN
metadata is disabled or the metadata port does not exist.
metadata is disabled or the metadata port does not exist or
cannot be created.
"""
network_id = network['id']
def update_metadata_port_fixed_ips(metadata_port, add_subnet_ids,
del_subnet_ids):
wanted_fixed_ips = [
@ -2478,11 +2482,11 @@ class OVNClient(object):
if not ovn_conf.is_ovn_metadata_enabled():
return False
# Retrieve the metadata port of this network
metadata_port = self._find_metadata_port(context, network_id)
# Retrieve or create the metadata port of this network
metadata_port = self.create_metadata_port(context, network)
if not metadata_port:
LOG.error("Metadata port couldn't be found for network %s",
network_id)
LOG.error("Metadata port could not be found or created "
"for network %s", network_id)
return False
port_subnet_ids = set(ip['subnet_id'] for ip in

2
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py
View File

@ -951,7 +951,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
try:
# Make sure that this port has an IP address in all the
# subnets
self._ovn_client.update_metadata_port(ctx, net['id'])
self._ovn_client.update_metadata_port(ctx, net)
except n_exc.IpAddressGenerationFailure:
LOG.error('Could not allocate IP addresses for '
'metadata port in network %s', net['id'])

44
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
View File

@ -995,6 +995,7 @@ class TestMetadataPorts(base.TestOVNFunctionalBase):
def setUp(self, *args, **kwargs):
super().setUp(*args, **kwargs)
self.plugin = self.mech_driver._plugin
self._ovn_client = self.mech_driver._ovn_client
self.meta_regex = re.compile(r'%s,(\d+\.\d+\.\d+\.\d+)' %
constants.METADATA_V4_CIDR)
@ -1017,7 +1018,7 @@ class TestMetadataPorts(base.TestOVNFunctionalBase):
res = self._list_ports(self.fmt, net_id=net_id)
return self.deserialize(self.fmt, res)['ports']
def _check_metadata_port(self, net_id, fixed_ip):
def _check_metadata_port(self, net_id, fixed_ip, fail=True):
for port in self._list_ports_ovn(net_id=net_id):
if ovn_client.OVNClient.is_metadata_port(port):
self.assertEqual(net_id, port['network_id'])
@ -1027,13 +1028,17 @@ class TestMetadataPorts(base.TestOVNFunctionalBase):
self.assertEqual([], port['fixed_ips'])
return port['id']
self.fail('Metadata port is not present in network %s or data is not '
'correct' % self.n1_id)
if fail:
self.fail('Metadata port is not present in network %s or data is '
'not correct' % self.n1_id)
def _check_subnet_dhcp_options(self, subnet_id, cidr):
# This method checks the DHCP options CIDR and returns, if exits, the
# metadata port IP address, included in the classless static routes.
# This method checks DHCP options for a subnet ID, and if they exist,
# verifies the CIDR matches. Returns the metadata port IP address
# if it is included in the classless static routes, else returns None.
dhcp_opts = self._ovn_client._nb_idl.get_subnet_dhcp_options(subnet_id)
if not dhcp_opts['subnet']:
return
self.assertEqual(cidr, dhcp_opts['subnet']['cidr'])
routes = dhcp_opts['subnet']['options'].get('classless_static_route')
if not routes:
@ -1062,6 +1067,35 @@ class TestMetadataPorts(base.TestOVNFunctionalBase):
fixed_ip = {'subnet_id': subnet['id'], 'ip_address': metatada_ip}
self._check_metadata_port(self.n1_id, fixed_ip)
def test_update_subnet_ipv4(self):
self._create_network_ovn(metadata_enabled=True)
subnet = self._create_subnet_ovn('10.0.0.0/24')
metatada_ip = self._check_subnet_dhcp_options(subnet['id'],
'10.0.0.0/24')
fixed_ip = {'subnet_id': subnet['id'], 'ip_address': metatada_ip}
port_id = self._check_metadata_port(self.n1_id, fixed_ip)
# Disable DHCP, port should still be present
subnet['enable_dhcp'] = False
self._ovn_client.update_subnet(self.context, subnet,
self.n1['network'])
port_id = self._check_metadata_port(self.n1_id, None)
self.assertIsNone(self._check_subnet_dhcp_options(subnet['id'], []))
# Delete metadata port
self.plugin.delete_port(self.context, port_id)
port_id = self._check_metadata_port(self.n1_id, None, fail=False)
self.assertIsNone(port_id)
# Enable DHCP, metadata port should have been re-created
subnet['enable_dhcp'] = True
self._ovn_client.update_subnet(self.context, subnet,
self.n1['network'])
metatada_ip = self._check_subnet_dhcp_options(subnet['id'],
'10.0.0.0/24')
fixed_ip = {'subnet_id': subnet['id'], 'ip_address': metatada_ip}
port_id = self._check_metadata_port(self.n1_id, fixed_ip)
def test_subnet_ipv4_no_metadata(self):
self._create_network_ovn(metadata_enabled=False)
subnet = self._create_subnet_ovn('10.0.0.0/24')

65
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
View File

@ -32,6 +32,7 @@ from neutron_lib import constants as const
from neutron_lib import context
from neutron_lib import exceptions as n_exc
from neutron_lib.plugins import directory
from neutron_lib.plugins import utils as p_utils
from neutron_lib.tests import tools
from neutron_lib.utils import net as n_net
from oslo_concurrency import processutils
@ -1736,7 +1737,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
self.mech_driver.update_subnet_postcommit(context)
esd.assert_called_once_with(
context.current, context.network.current, mock.ANY)
umd.assert_called_once_with(mock.ANY, 'id', subnet=subnet)
umd.assert_called_once_with(mock.ANY, context.network.current,
subnet=subnet)
def test_update_subnet_postcommit_disable_dhcp(self):
self.mech_driver.nb_ovn.get_subnet_dhcp_options.return_value = {
@ -1752,7 +1754,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
'update_metadata_port') as umd:
self.mech_driver.update_subnet_postcommit(context)
dsd.assert_called_once_with(context.current['id'], mock.ANY)
umd.assert_called_once_with(mock.ANY, 'id', subnet=subnet)
umd.assert_called_once_with(mock.ANY, context.network.current,
subnet=subnet)
def test_update_subnet_postcommit_update_dhcp(self):
self.mech_driver.nb_ovn.get_subnet_dhcp_options.return_value = {
@ -1769,7 +1772,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
self.mech_driver.update_subnet_postcommit(context)
usd.assert_called_once_with(
context.current, context.network.current, mock.ANY)
umd.assert_called_once_with(mock.ANY, 'id', subnet=subnet)
umd.assert_called_once_with(mock.ANY, context.network.current,
subnet=subnet)
def test__get_port_options(self):
with mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \
@ -1955,9 +1959,10 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_metaport.return_value = {'fixed_ips': fixed_ips,
'id': 'metadata_id'}
mock_get_subnets.return_value = [{'id': 'subnet1'}]
network = {'id': 'net_id'}
subnet = {'id': 'subnet1', 'enable_dhcp': True}
self.mech_driver._ovn_client.update_metadata_port(
self.context, 'net_id', subnet=subnet)
self.context, network, subnet=subnet)
mock_update_port.assert_not_called()
# Subnet without DHCP, present in port.
@ -1967,7 +1972,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_get_subnets.return_value = [{'id': 'subnet1'}]
subnet = {'id': 'subnet1', 'enable_dhcp': False}
self.mech_driver._ovn_client.update_metadata_port(
self.context, 'net_id', subnet=subnet)
self.context, network, subnet=subnet)
port = {'id': 'metadata_id',
'port': {'network_id': 'net_id', 'fixed_ips': []}}
mock_update_port.assert_called_once_with(mock.ANY, 'metadata_id',
@ -1980,7 +1985,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_get_subnets.return_value = []
subnet = {'id': 'subnet1', 'enable_dhcp': True}
self.mech_driver._ovn_client.update_metadata_port(
self.context, 'net_id', subnet=subnet)
self.context, network, subnet=subnet)
fixed_ips = [{'subnet_id': 'subnet1'}]
port = {'id': 'metadata_id',
'port': {'network_id': 'net_id', 'fixed_ips': fixed_ips}}
@ -1994,7 +1999,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_get_subnets.return_value = []
subnet = {'id': 'subnet1', 'enable_dhcp': False}
self.mech_driver._ovn_client.update_metadata_port(
self.context, 'net_id', subnet=subnet)
self.context, network, subnet=subnet)
mock_update_port.assert_not_called()
def test_update_metadata_port_no_subnet(self):
@ -2011,10 +2016,11 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_get_subnets.return_value = [{'id': 'subnet1'},
{'id': 'subnet2'}]
fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
network = {'id': 'net_id'}
mock_metaport.return_value = {'fixed_ips': fixed_ips,
'id': 'metadata_id'}
self.mech_driver._ovn_client.update_metadata_port(self.context,
'net_id')
network)
port = {'id': 'metadata_id',
'port': {'network_id': 'net_id', 'fixed_ips': fixed_ips}}
fixed_ips.append({'subnet_id': 'subnet2'})
@ -2025,10 +2031,11 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
# Port with IP in subnet1; subnet1 with DHCP, subnet2 without DHCP.
mock_get_subnets.return_value = [{'id': 'subnet1'}]
fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
network = {'id': 'net_id'}
mock_metaport.return_value = {'fixed_ips': fixed_ips,
'id': 'metadata_id'}
self.mech_driver._ovn_client.update_metadata_port(self.context,
'net_id')
network)
mock_update_port.assert_not_called()
# Port with IP in subnet1; subnet1 without DHCP.
@ -2037,13 +2044,51 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
mock_metaport.return_value = {'fixed_ips': fixed_ips,
'id': 'metadata_id'}
self.mech_driver._ovn_client.update_metadata_port(self.context,
'net_id')
network)
port = {'id': 'metadata_id',
'port': {'network_id': 'net_id', 'fixed_ips': []}}
mock_update_port.assert_called_once_with(
mock.ANY, 'metadata_id', port)
mock_update_port.reset_mock()
def test_update_metadata_port_no_port(self):
ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', True,
group='ovn')
with mock.patch.object(
self.mech_driver._ovn_client, '_find_metadata_port') as \
mock_find_metaport, \
mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \
mock_get_subnets, \
mock.patch.object(p_utils, 'create_port') as \
mock_create_port:
# Subnet with DHCP, no port, port created.
network = {'id': 'net_id', 'project_id': 'project_id-foo'}
subnet = {'id': 'subnet1', 'enable_dhcp': True}
fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
port = {'id': 'metadata_id',
'network_id': 'net_id',
'device_owner': const.DEVICE_OWNER_DISTRIBUTED,
'device_id': 'ovnmeta-%s' % 'net_id',
'fixed_ips': fixed_ips}
mock_get_subnets.return_value = [subnet]
mock_find_metaport.return_value = None
# Subnet with DHCP, no port, port create failure.
mock_create_port.return_value = None
ret_status = self.mech_driver._ovn_client.update_metadata_port(
self.context, network, subnet=subnet)
self.assertFalse(ret_status)
mock_create_port.assert_called_once()
# Subnet with DHCP, no port, port created successfully.
mock_create_port.reset_mock()
mock_create_port.return_value = port
ret_status = self.mech_driver._ovn_client.update_metadata_port(
self.context, network, subnet=subnet)
self.assertTrue(ret_status)
mock_create_port.assert_called_once()
@mock.patch.object(provisioning_blocks, 'is_object_blocked')
@mock.patch.object(provisioning_blocks, 'provisioning_complete')
def test_notify_dhcp_updated(self, mock_prov_complete, mock_is_obj_block):

11
releasenotes/notes/ovn-recreate-metadata-port-76e2c0e651267aa0.yaml Normal file
View File

@ -0,0 +1,11 @@
---
fixes:
- |
Fix an issue in the OVN driver where network metadata could
become unavailable if the metadata port was ever deleted, even
if accidental. To re-create the port, a user can now disable,
then enable, DHCP for one of the subnets associated with the
network using the Neutron API. This will try and create the
port, similar to what happens in the DHCP agent for ML2/OVS.
For more information, see bug `2015377
<https://bugs.launchpad.net/ubuntu/+source/neutron/+bug/2015377>`_.