OVN: Always try and create a metadata port on subnets
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
This commit is contained in:
Brian Haley
parent
208421910d
commit
267efd2984
|
|
@ -2336,7 +2336,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.
|
||||
|
|
@ -2356,7 +2356,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)
|
||||
|
|
@ -2452,8 +2452,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']]}
|
||||
|
|
@ -2468,16 +2469,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 = [
|
||||
|
|
@ -2496,11 +2500,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
|
||||
|
|
|
|||
|
|
@ -960,7 +960,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'])
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ from neutron_lib import context
|
|||
from neutron_lib import exceptions as n_exc
|
||||
from neutron_lib.placement import utils as place_utils
|
||||
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
|
||||
|
|
@ -1754,7 +1755,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 = {
|
||||
|
|
@ -1770,7 +1772,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 = {
|
||||
|
|
@ -1787,7 +1790,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 \
|
||||
|
|
@ -1979,9 +1983,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.
|
||||
|
|
@ -1991,7 +1996,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',
|
||||
|
|
@ -2004,7 +2009,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}}
|
||||
|
|
@ -2018,7 +2023,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):
|
||||
|
|
@ -2035,10 +2040,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'})
|
||||
|
|
@ -2049,10 +2055,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.
|
||||
|
|
@ -2061,13 +2068,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):
|
||||
|
|
|
|||
|
|
@ -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>`_.
|
||||
Loading…
Reference in New Issue