openstack/neutron
Code Issues Proposed changes

[ovn] Add neutron network to metadata namespace names

Browse Source 
Until this patch the metadata namespace had the format
'ovnmeta-<OVN datapath uuid>' which makes it hard to
relate to the Neutron network for which metadata is
provisioned to. This applies to the name of the tap
interface that connects the network namespace to the
integration bridge.

This patch is changing the name convention and providing
an upgrade path to include the Neutron network ID to make
debugging and troubleshooting easier for developers and
operators.

The new name pattern is:
'ovnmeta-<Neutron network uuid>'

Please, note that with this patch, the old namespaces will
be deleted and new ones will be recreated.

Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
Change-Id: Ic8ffa9c4437aab6fb1878b3a1ebf2c3ab86e3d0c
This commit is contained in:
Daniel Alvarez 2020-12-21 14:07:03 +00:00 committed by Daniel Alvarez Sanchez
parent 74e4cd31f1
commit e4fb06b242
5 changed files with 74 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
neutron/agent/ovn/metadata/agent.py
View File

@ -31,6 +31,7 @@ from neutron.agent.ovn.metadata import driver as metadata_driver
from neutron.agent.ovn.metadata import ovsdb from neutron.agent.ovn.metadata import ovsdb
from neutron.agent.ovn.metadata import server as metadata_server from neutron.agent.ovn.metadata import server as metadata_server
from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils as ovn_utils
from neutron.common import utils from neutron.common import utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config
@ -80,9 +81,10 @@ class PortBindingChassisEvent(row_event.RowEvent):
return return
with _SYNC_STATE_LOCK.read_lock(): with _SYNC_STATE_LOCK.read_lock():
try: try:
LOG.info(self.LOG_MSG, row.logical_port, net_name = ovn_utils.get_network_name_from_datapath(
str(row.datapath.uuid)) row.datapath)
self.agent.update_datapath(str(row.datapath.uuid)) LOG.info(self.LOG_MSG, row.logical_port, net_name)
self.agent.update_datapath(str(row.datapath.uuid), net_name)
except ConfigException: except ConfigException:
# We're now in the reader lock mode, we need to exit the # We're now in the reader lock mode, we need to exit the
# context and then use writer lock # context and then use writer lock
@ -323,14 +325,20 @@ class MetadataAgent(object):
def _vif_ports(self, ports): def _vif_ports(self, ports):
return (p for p in ports if p.type in OVN_VIF_PORT_TYPES) return (p for p in ports if p.type in OVN_VIF_PORT_TYPES)
def teardown_datapath(self, datapath): def teardown_datapath(self, datapath, net_name=None):
"""Unprovision this datapath to stop serving metadata. """Unprovision this datapath to stop serving metadata.
This function will shutdown metadata proxy if it's running and delete This function will shutdown metadata proxy if it's running and delete
the VETH pair, the OVS port and the namespace. the VETH pair, the OVS port and the namespace.
""" """
self.update_chassis_metadata_networks(datapath, remove=True) self.update_chassis_metadata_networks(datapath, remove=True)
namespace = self._get_namespace_name(datapath)
# TODO(dalvarez): Remove this in Y cycle when we are sure that all
# namespaces will be created with the Neutron network UUID and not
# anymore with the OVN datapath UUID.
dp = net_name or datapath
namespace = self._get_namespace_name(dp)
ip = ip_lib.IPWrapper(namespace) ip = ip_lib.IPWrapper(namespace)
# If the namespace doesn't exist, return # If the namespace doesn't exist, return
if not ip.netns.exists(namespace): if not ip.netns.exists(namespace):
@ -340,16 +348,16 @@ class MetadataAgent(object):
namespace) namespace)
metadata_driver.MetadataDriver.destroy_monitored_metadata_proxy( metadata_driver.MetadataDriver.destroy_monitored_metadata_proxy(
self._process_monitor, datapath, self.conf, namespace) self._process_monitor, dp, self.conf, namespace)
veth_name = self._get_veth_name(datapath) veth_name = self._get_veth_name(dp)
self.ovs_idl.del_port(veth_name[0]).execute() self.ovs_idl.del_port(veth_name[0]).execute()
if ip_lib.device_exists(veth_name[0]): if ip_lib.device_exists(veth_name[0]):
ip_lib.IPWrapper().del_veth(veth_name[0]) ip_lib.IPWrapper().del_veth(veth_name[0])
ip.garbage_collect_namespace() ip.garbage_collect_namespace()
def update_datapath(self, datapath): def update_datapath(self, datapath, net_name):
"""Update the metadata service for this datapath. """Update the metadata service for this datapath.
This function will: This function will:
@ -364,9 +372,9 @@ class MetadataAgent(object):
datapath_ports = [p for p in self._vif_ports(ports) if datapath_ports = [p for p in self._vif_ports(ports) if
str(p.datapath.uuid) == datapath] str(p.datapath.uuid) == datapath]
if datapath_ports: if datapath_ports:
self.provision_datapath(datapath) self.provision_datapath(datapath, net_name)
else: else:
self.teardown_datapath(datapath) self.teardown_datapath(datapath, net_name)
def _ensure_datapath_checksum(self, namespace): def _ensure_datapath_checksum(self, namespace):
"""Ensure the correct checksum in the metadata packets in DPDK bridges """Ensure the correct checksum in the metadata packets in DPDK bridges
@ -385,7 +393,7 @@ class MetadataAgent(object):
iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', rule, wrap=False) iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', rule, wrap=False)
iptables_mgr.apply() iptables_mgr.apply()
def provision_datapath(self, datapath): def provision_datapath(self, datapath, net_name):
"""Provision the datapath so that it can serve metadata. """Provision the datapath so that it can serve metadata.
This function will create the namespace and VETH pair if needed This function will create the namespace and VETH pair if needed
@ -395,7 +403,7 @@ class MetadataAgent(object):
:return: The metadata namespace name of this datapath :return: The metadata namespace name of this datapath
""" """
LOG.debug("Provisioning datapath %s", datapath) LOG.debug("Provisioning metadata for network %s", net_name)
port = self.sb_idl.get_metadata_port_network(datapath) port = self.sb_idl.get_metadata_port_network(datapath)
# If there's no metadata port or it doesn't have a MAC or IP # If there's no metadata port or it doesn't have a MAC or IP
# addresses, then tear the namespace down if needed. This might happen # addresses, then tear the namespace down if needed. This might happen
@ -403,10 +411,10 @@ class MetadataAgent(object):
# an IP address. # an IP address.
if not (port and port.mac and if not (port and port.mac and
port.external_ids.get(ovn_const.OVN_CIDRS_EXT_ID_KEY, None)): port.external_ids.get(ovn_const.OVN_CIDRS_EXT_ID_KEY, None)):
LOG.debug("There is no metadata port for datapath %s or it has no " LOG.debug("There is no metadata port for network %s or it has no "
"MAC or IP addresses configured, tearing the namespace " "MAC or IP addresses configured, tearing the namespace "
"down if needed", datapath) "down if needed", net_name)
self.teardown_datapath(datapath) self.teardown_datapath(datapath, net_name)
return return
# First entry of the mac field must be the MAC address. # First entry of the mac field must be the MAC address.
@ -414,9 +422,9 @@ class MetadataAgent(object):
# If it is not, we can't provision the namespace. Tear it down if # If it is not, we can't provision the namespace. Tear it down if
# needed and log the error. # needed and log the error.
if not match: if not match:
LOG.error("Metadata port for datapath %s doesn't have a MAC " LOG.error("Metadata port for network %s doesn't have a MAC "
"address, tearing the namespace down if needed", "address, tearing the namespace down if needed",
datapath) net_name)
self.teardown_datapath(datapath) self.teardown_datapath(datapath)
return return
@ -428,8 +436,8 @@ class MetadataAgent(object):
# Create the VETH pair if it's not created. Also the add_veth function # Create the VETH pair if it's not created. Also the add_veth function
# will create the namespace for us. # will create the namespace for us.
namespace = self._get_namespace_name(datapath) namespace = self._get_namespace_name(net_name)
veth_name = self._get_veth_name(datapath) veth_name = self._get_veth_name(net_name)
ip1 = ip_lib.IPDevice(veth_name[0]) ip1 = ip_lib.IPDevice(veth_name[0])
if ip_lib.device_exists(veth_name[1], namespace): if ip_lib.device_exists(veth_name[1], namespace):
@ -499,9 +507,9 @@ class MetadataAgent(object):
metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy( metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy(
self._process_monitor, namespace, n_const.METADATA_PORT, self._process_monitor, namespace, n_const.METADATA_PORT,
self.conf, bind_address=n_const.METADATA_V4_IP, self.conf, bind_address=n_const.METADATA_V4_IP,
network_id=datapath) network_id=net_name)
self.update_chassis_metadata_networks(datapath) self.update_chassis_metadata_networks(net_name)
return namespace return namespace
def ensure_all_networks_provisioned(self): def ensure_all_networks_provisioned(self):
@ -516,11 +524,13 @@ class MetadataAgent(object):
""" """
# Retrieve all VIF ports in our Chassis # Retrieve all VIF ports in our Chassis
ports = self.sb_idl.get_ports_on_chassis(self.chassis) ports = self.sb_idl.get_ports_on_chassis(self.chassis)
datapaths = {str(p.datapath.uuid) for p in self._vif_ports(ports)} nets = {(str(p.datapath.uuid),
ovn_utils.get_network_name_from_datapath(p.datapath))
for p in self._vif_ports(ports)}
namespaces = [] namespaces = []
# Make sure that all those datapaths are serving metadata # Make sure that all those datapaths are serving metadata
for datapath in datapaths: for datapath, net_name in nets:
netns = self.provision_datapath(datapath) netns = self.provision_datapath(datapath, net_name)
if netns: if netns:
namespaces.append(netns) namespaces.append(netns)

4
neutron/common/ovn/utils.py
View File

@ -561,3 +561,7 @@ def parse_ovn_lb_port_forwarding(ovn_rtr_lb_pfs):
fip_dict[protocol] = fip_dict_proto fip_dict[protocol] = fip_dict_proto
result[fip_id] = fip_dict result[fip_id] = fip_dict
return result return result
def get_network_name_from_datapath(datapath):
return datapath.external_ids['name'].replace('neutron-', '')

8
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
View File

@ -816,8 +816,14 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):
rows = self.db_list_rows('Port_Binding').execute(check_error=True) rows = self.db_list_rows('Port_Binding').execute(check_error=True)
# TODO(twilson) It would be useful to have a db_find that takes a # TODO(twilson) It would be useful to have a db_find that takes a
# comparison function # comparison function
# TODO(dalvarez): Remove the comparison to r.datapath.uuid in Y cycle
# when we are sure that all namespaces will be created with the
# Neutron network UUID and not anymore with the OVN datapath UUID.
return [r for r in rows return [r for r in rows
if (r.mac and str(r.datapath.uuid) == network) and if (r.mac and (
str(r.datapath.uuid) == network or
utils.get_network_name_from_datapath(
r.datapath) == network)) and
ip_address in r.mac[0].split(' ')] ip_address in r.mac[0].split(' ')]
def set_port_cidrs(self, name, cidrs): def set_port_cidrs(self, name, cidrs):

39
neutron/tests/unit/agent/ovn/metadata/test_agent.py
View File

@ -33,7 +33,7 @@ from neutron.tests import base
OvnPortInfo = collections.namedtuple( OvnPortInfo = collections.namedtuple(
'OvnPortInfo', ['datapath', 'type', 'mac', 'external_ids', 'logical_port']) 'OvnPortInfo', ['datapath', 'type', 'mac', 'external_ids', 'logical_port'])
DatapathInfo = collections.namedtuple('DatapathInfo', 'uuid') DatapathInfo = collections.namedtuple('DatapathInfo', ['uuid', 'external_ids'])
def makePort(datapath=None, type='', mac=None, external_ids=None, def makePort(datapath=None, type='', mac=None, external_ids=None,
@ -119,11 +119,14 @@ class TestMetadataAgent(base.BaseTestCase):
ports = [] ports = []
for i in range(0, 3): for i in range(0, 3):
ports.append(makePort(datapath=DatapathInfo(uuid=str(i)))) ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
ports.append(makePort(datapath=DatapathInfo(uuid='1'))) external_ids={'name': 'neutron-%d' % i})))
ports.append(makePort(datapath=DatapathInfo(uuid='3'), ports.append(makePort(datapath=DatapathInfo(uuid='1',
type='external')) external_ids={'name': 'neutron-1'})))
ports.append(makePort(datapath=DatapathInfo(uuid='5'), type='unknown')) ports.append(makePort(datapath=DatapathInfo(uuid='3',
external_ids={'name': 'neutron-3'}), type='external'))
ports.append(makePort(datapath=DatapathInfo(uuid='5',
external_ids={'name': 'neutron-5'}), type='unknown'))
with mock.patch.object(self.agent, 'provision_datapath', with mock.patch.object(self.agent, 'provision_datapath',
return_value=None) as pdp,\ return_value=None) as pdp,\
@ -131,40 +134,42 @@ class TestMetadataAgent(base.BaseTestCase):
return_value=ports): return_value=ports):
self.agent.ensure_all_networks_provisioned() self.agent.ensure_all_networks_provisioned()
expected_calls = [mock.call(str(i)) for i in range(0, 4)] expected_calls = [mock.call(str(i), str(i)) for i in range(0, 4)]
self.assertEqual(sorted(expected_calls), self.assertEqual(sorted(expected_calls),
sorted(pdp.call_args_list)) sorted(pdp.call_args_list))
def test_update_datapath_provision(self): def test_update_datapath_provision(self):
ports = [] ports = []
for i in range(0, 3): for i in range(0, 3):
ports.append(makePort(datapath=DatapathInfo(uuid=str(i)))) ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
ports.append(makePort(datapath=DatapathInfo(uuid='3'), external_ids={'name': 'neutron-%d' % i})))
type='external')) ports.append(makePort(datapath=DatapathInfo(uuid='3',
external_ids={'name': 'neutron-3'}), type='external'))
with mock.patch.object(self.agent, 'provision_datapath', with mock.patch.object(self.agent, 'provision_datapath',
return_value=None) as pdp,\ return_value=None) as pdp,\
mock.patch.object(self.agent, 'teardown_datapath') as tdp,\ mock.patch.object(self.agent, 'teardown_datapath') as tdp,\
mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
return_value=ports): return_value=ports):
self.agent.update_datapath('1') self.agent.update_datapath('1', 'a')
self.agent.update_datapath('3') self.agent.update_datapath('3', 'b')
expected_calls = [mock.call('1'), mock.call('3')] expected_calls = [mock.call('1', 'a'), mock.call('3', 'b')]
pdp.assert_has_calls(expected_calls) pdp.assert_has_calls(expected_calls)
tdp.assert_not_called() tdp.assert_not_called()
def test_update_datapath_teardown(self): def test_update_datapath_teardown(self):
ports = [] ports = []
for i in range(0, 3): for i in range(0, 3):
ports.append(makePort(datapath=DatapathInfo(uuid=str(i)))) ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
external_ids={'name': 'neutron-%d' % i})))
with mock.patch.object(self.agent, 'provision_datapath', with mock.patch.object(self.agent, 'provision_datapath',
return_value=None) as pdp,\ return_value=None) as pdp,\
mock.patch.object(self.agent, 'teardown_datapath') as tdp,\ mock.patch.object(self.agent, 'teardown_datapath') as tdp,\
mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
return_value=ports): return_value=ports):
self.agent.update_datapath('5') self.agent.update_datapath('5', 'a')
tdp.assert_called_once_with('5') tdp.assert_called_once_with('5', 'a')
pdp.assert_not_called() pdp.assert_not_called()
def test_teardown_datapath(self): def test_teardown_datapath(self):
@ -243,7 +248,7 @@ class TestMetadataAgent(base.BaseTestCase):
# We need to assert that it was deleted first. # We need to assert that it was deleted first.
self.agent.ovs_idl.list_br.return_value.execute.return_value = ( self.agent.ovs_idl.list_br.return_value.execute.return_value = (
['br-int', 'br-fake']) ['br-int', 'br-fake'])
self.agent.provision_datapath('1') self.agent.provision_datapath('1', '1')
# Check that the port was deleted from br-fake # Check that the port was deleted from br-fake
self.agent.ovs_idl.del_port.assert_called_once_with( self.agent.ovs_idl.del_port.assert_called_once_with(

7
releasenotes/notes/ovnmeta-namespaces-include-network-name-e6e4e5f6ff69e7ed.yaml Normal file
View File

@ -0,0 +1,7 @@
---
other:
- |
The ``OVN Metadata Agent`` now creates the network namespaces including the
Neutron network UUID in its name. Previously, the OVN datapath UUID was used
and it was not obvious for operators and during debugging to figure out which
namespace corresponded to what Neutron network.