From e4fb06b24299a7ecf10b05ef6ddc2d883c40e5a1 Mon Sep 17 00:00:00 2001 From: Daniel Alvarez Date: Mon, 21 Dec 2020 14:07:03 +0000 Subject: [PATCH] [ovn] Add neutron network to metadata namespace names Until this patch the metadata namespace had the format 'ovnmeta-' 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-' Please, note that with this patch, the old namespaces will be deleted and new ones will be recreated. Signed-off-by: Daniel Alvarez Change-Id: Ic8ffa9c4437aab6fb1878b3a1ebf2c3ab86e3d0c --- neutron/agent/ovn/metadata/agent.py | 58 +++++++++++-------- neutron/common/ovn/utils.py | 4 ++ .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 8 ++- .../unit/agent/ovn/metadata/test_agent.py | 39 +++++++------ ...include-network-name-e6e4e5f6ff69e7ed.yaml | 7 +++ 5 files changed, 74 insertions(+), 42 deletions(-) create mode 100644 releasenotes/notes/ovnmeta-namespaces-include-network-name-e6e4e5f6ff69e7ed.yaml diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 204860adc9c..e3aea2bd6d7 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -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 server as metadata_server 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.conf.plugins.ml2.drivers.ovn import ovn_conf as config @@ -80,9 +81,10 @@ class PortBindingChassisEvent(row_event.RowEvent): return with _SYNC_STATE_LOCK.read_lock(): try: - LOG.info(self.LOG_MSG, row.logical_port, - str(row.datapath.uuid)) - self.agent.update_datapath(str(row.datapath.uuid)) + net_name = ovn_utils.get_network_name_from_datapath( + row.datapath) + LOG.info(self.LOG_MSG, row.logical_port, net_name) + self.agent.update_datapath(str(row.datapath.uuid), net_name) except ConfigException: # We're now in the reader lock mode, we need to exit the # context and then use writer lock @@ -323,14 +325,20 @@ class MetadataAgent(object): def _vif_ports(self, ports): 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. This function will shutdown metadata proxy if it's running and delete the VETH pair, the OVS port and the namespace. """ 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) # If the namespace doesn't exist, return if not ip.netns.exists(namespace): @@ -340,16 +348,16 @@ class MetadataAgent(object): namespace) 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() if ip_lib.device_exists(veth_name[0]): ip_lib.IPWrapper().del_veth(veth_name[0]) ip.garbage_collect_namespace() - def update_datapath(self, datapath): + def update_datapath(self, datapath, net_name): """Update the metadata service for this datapath. This function will: @@ -364,9 +372,9 @@ class MetadataAgent(object): datapath_ports = [p for p in self._vif_ports(ports) if str(p.datapath.uuid) == datapath] if datapath_ports: - self.provision_datapath(datapath) + self.provision_datapath(datapath, net_name) else: - self.teardown_datapath(datapath) + self.teardown_datapath(datapath, net_name) def _ensure_datapath_checksum(self, namespace): """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.apply() - def provision_datapath(self, datapath): + def provision_datapath(self, datapath, net_name): """Provision the datapath so that it can serve metadata. 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 """ - LOG.debug("Provisioning datapath %s", datapath) + LOG.debug("Provisioning metadata for network %s", net_name) port = self.sb_idl.get_metadata_port_network(datapath) # 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 @@ -403,10 +411,10 @@ class MetadataAgent(object): # an IP address. if not (port and port.mac and 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 " - "down if needed", datapath) - self.teardown_datapath(datapath) + "down if needed", net_name) + self.teardown_datapath(datapath, net_name) return # 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 # needed and log the error. 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", - datapath) + net_name) self.teardown_datapath(datapath) return @@ -428,8 +436,8 @@ class MetadataAgent(object): # Create the VETH pair if it's not created. Also the add_veth function # will create the namespace for us. - namespace = self._get_namespace_name(datapath) - veth_name = self._get_veth_name(datapath) + namespace = self._get_namespace_name(net_name) + veth_name = self._get_veth_name(net_name) ip1 = ip_lib.IPDevice(veth_name[0]) if ip_lib.device_exists(veth_name[1], namespace): @@ -499,9 +507,9 @@ class MetadataAgent(object): metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy( self._process_monitor, namespace, n_const.METADATA_PORT, 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 def ensure_all_networks_provisioned(self): @@ -516,11 +524,13 @@ class MetadataAgent(object): """ # Retrieve all VIF ports in our 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 = [] # Make sure that all those datapaths are serving metadata - for datapath in datapaths: - netns = self.provision_datapath(datapath) + for datapath, net_name in nets: + netns = self.provision_datapath(datapath, net_name) if netns: namespaces.append(netns) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 9dbbba5691d..c878b5f53bb 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -561,3 +561,7 @@ def parse_ovn_lb_port_forwarding(ovn_rtr_lb_pfs): fip_dict[protocol] = fip_dict_proto result[fip_id] = fip_dict return result + + +def get_network_name_from_datapath(datapath): + return datapath.external_ids['name'].replace('neutron-', '') diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index 4bea7d8412b..d71895c0a74 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -816,8 +816,14 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): 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 # 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 - 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(' ')] def set_port_cidrs(self, name, cidrs): diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 3f690d5fd14..cfd73a1db84 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -33,7 +33,7 @@ from neutron.tests import base OvnPortInfo = collections.namedtuple( '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, @@ -119,11 +119,14 @@ class TestMetadataAgent(base.BaseTestCase): ports = [] for i in range(0, 3): - ports.append(makePort(datapath=DatapathInfo(uuid=str(i)))) - ports.append(makePort(datapath=DatapathInfo(uuid='1'))) - ports.append(makePort(datapath=DatapathInfo(uuid='3'), - type='external')) - ports.append(makePort(datapath=DatapathInfo(uuid='5'), type='unknown')) + ports.append(makePort(datapath=DatapathInfo(uuid=str(i), + external_ids={'name': 'neutron-%d' % i}))) + ports.append(makePort(datapath=DatapathInfo(uuid='1', + external_ids={'name': 'neutron-1'}))) + 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', return_value=None) as pdp,\ @@ -131,40 +134,42 @@ class TestMetadataAgent(base.BaseTestCase): return_value=ports): 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), sorted(pdp.call_args_list)) def test_update_datapath_provision(self): ports = [] for i in range(0, 3): - ports.append(makePort(datapath=DatapathInfo(uuid=str(i)))) - ports.append(makePort(datapath=DatapathInfo(uuid='3'), - type='external')) + ports.append(makePort(datapath=DatapathInfo(uuid=str(i), + external_ids={'name': 'neutron-%d' % i}))) + ports.append(makePort(datapath=DatapathInfo(uuid='3', + external_ids={'name': 'neutron-3'}), type='external')) with mock.patch.object(self.agent, 'provision_datapath', return_value=None) as pdp,\ mock.patch.object(self.agent, 'teardown_datapath') as tdp,\ mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', return_value=ports): - self.agent.update_datapath('1') - self.agent.update_datapath('3') - expected_calls = [mock.call('1'), mock.call('3')] + self.agent.update_datapath('1', 'a') + self.agent.update_datapath('3', 'b') + expected_calls = [mock.call('1', 'a'), mock.call('3', 'b')] pdp.assert_has_calls(expected_calls) tdp.assert_not_called() def test_update_datapath_teardown(self): ports = [] 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', return_value=None) as pdp,\ mock.patch.object(self.agent, 'teardown_datapath') as tdp,\ mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', return_value=ports): - self.agent.update_datapath('5') - tdp.assert_called_once_with('5') + self.agent.update_datapath('5', 'a') + tdp.assert_called_once_with('5', 'a') pdp.assert_not_called() def test_teardown_datapath(self): @@ -243,7 +248,7 @@ class TestMetadataAgent(base.BaseTestCase): # We need to assert that it was deleted first. self.agent.ovs_idl.list_br.return_value.execute.return_value = ( ['br-int', 'br-fake']) - self.agent.provision_datapath('1') + self.agent.provision_datapath('1', '1') # Check that the port was deleted from br-fake self.agent.ovs_idl.del_port.assert_called_once_with( diff --git a/releasenotes/notes/ovnmeta-namespaces-include-network-name-e6e4e5f6ff69e7ed.yaml b/releasenotes/notes/ovnmeta-namespaces-include-network-name-e6e4e5f6ff69e7ed.yaml new file mode 100644 index 00000000000..90b91afa094 --- /dev/null +++ b/releasenotes/notes/ovnmeta-namespaces-include-network-name-e6e4e5f6ff69e7ed.yaml @@ -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.