openstack
/
neutron
Code Issues Proposed changes

ovn: first tear down old metadata namespaces, then deploy new 

While the reverse order may work, it's considered invalid by OVN and not
guaranteed to work properly since OVN may not necessarily know which of
two ports is the one to configure.

This configuration also triggered a bug in OVN where tearing down a port
after deploying a new one resulted in removing flows that serve the
port.

There is a patch up for review for OVN [1] to better handle multiple
assignment of the same port, but it doesn't make the setup any more
valid.

[1] http://patchwork.ozlabs.org/project/ovn/patch/20221114092437.2807815-1-xsimonar@redhat.com/

Conflicts:
      neutron/agent/ovn/metadata/agent.py

Closes-Bug: #1997092
Change-Id: Ic7dbc4e8b00423e58f69646a9e3cedc6f72d6c63
(cherry picked from commit 3093aaab13)
This commit is contained in:
Ihar Hrachyshka 2022-11-16 18:47:04 +00:00
parent 90865c06af
commit e6878c12c3
2 changed files with 51 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -320,6 +320,12 @@ class MetadataAgent(object):
"br-int instead.")
return 'br-int'
def get_networks(self):
ports = self.sb_idl.get_ports_on_chassis(self.chassis)
return {(str(p.datapath.uuid),
ovn_utils.get_network_name_from_datapath(p.datapath))
for p in self._vif_ports(ports)}
@_sync_lock
def sync(self):
"""Agent sync.
@ -328,16 +334,26 @@ class MetadataAgent(object):
chassis are serving metadata. Also, it will tear down those namespaces
which were serving metadata but are no longer needed.
"""
metadata_namespaces = self.ensure_all_networks_provisioned()
# first, clean up namespaces that should no longer deploy
system_namespaces = tuple(
ns.decode('utf-8') if isinstance(ns, bytes) else ns
for ns in ip_lib.list_network_namespaces())
nets = self.get_networks()
metadata_namespaces = [
self._get_namespace_name(net[1])
for net in nets
]
unused_namespaces = [ns for ns in system_namespaces if
ns.startswith(NS_PREFIX) and
ns not in metadata_namespaces]
for ns in unused_namespaces:
self.teardown_datapath(self._get_datapath_name(ns))
# now that all obsolete namespaces are cleaned up, deploy required
# networks
self.ensure_all_networks_provisioned(nets)
@staticmethod
def _get_veth_name(datapath):
return ['{}{}{}'.format(n_const.TAP_DEVICE_PREFIX,
@ -431,8 +447,6 @@ class MetadataAgent(object):
and assign the IP addresses to the interface corresponding to the
metadata port of the network. It will also remove existing IP
addresses that are no longer needed.
:return: The metadata namespace name of this datapath
"""
LOG.debug("Provisioning metadata for network %s", net_name)
port = self.sb_idl.get_metadata_port_network(datapath)
@ -541,31 +555,17 @@ class MetadataAgent(object):
network_id=net_name)
self.update_chassis_metadata_networks(net_name)
return namespace
def ensure_all_networks_provisioned(self):
"""Ensure that all datapaths are provisioned.
def ensure_all_networks_provisioned(self, nets):
"""Ensure that all requested datapaths are provisioned.
This function will make sure that all datapaths with ports bound to
our chassis have its namespace, VETH pair and OVS port created and
metadata proxy is up and running.
:return: A list with the namespaces that are currently serving
metadata
This function will make sure that requested datapaths have their
namespaces, VETH pair and OVS ports created and metadata proxies are up
and running.
"""
# Retrieve all VIF ports in our Chassis
ports = self.sb_idl.get_ports_on_chassis(self.chassis)
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, net_name in nets:
netns = self.provision_datapath(datapath, net_name)
if netns:
namespaces.append(netns)
return namespaces
self.provision_datapath(datapath, net_name)
# NOTE(lucasagomes): Even tho the metadata agent is a multi-process
# application, there's only one Southbound database IDL instance in

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

@ -71,19 +71,29 @@ class TestMetadataAgent(base.BaseTestCase):
self.agent.chassis = 'chassis'
self.agent.ovn_bridge = 'br-int'
self.ports = []
for i in range(0, 3):
self.ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
external_ids={'name': 'neutron-%d' % i})))
self.agent.sb_idl.get_ports_on_chassis.return_value = self.ports
def test_sync(self):
with mock.patch.object(
self.agent, 'ensure_all_networks_provisioned') as enp,\
mock.patch.object(
ip_lib, 'list_network_namespaces') as lnn,\
mock.patch.object(
self.agent, 'teardown_datapath') as tdp:
enp.return_value = ['ovnmeta-1', 'ovnmeta-2']
lnn.return_value = ['ovnmeta-1', 'ovnmeta-2']
self.agent.sync()
enp.assert_called_once_with()
enp.assert_called_once_with({
(p.datapath.uuid, p.datapath.uuid)
for p in self.ports
})
lnn.assert_called_once_with()
tdp.assert_not_called()
@ -95,18 +105,20 @@ class TestMetadataAgent(base.BaseTestCase):
ip_lib, 'list_network_namespaces') as lnn,\
mock.patch.object(
self.agent, 'teardown_datapath') as tdp:
enp.return_value = ['ovnmeta-1', 'ovnmeta-2']
lnn.return_value = ['ovnmeta-1', 'ovnmeta-2', 'ovnmeta-3',
'ns1', 'ns2']
self.agent.sync()
enp.assert_called_once_with()
enp.assert_called_once_with({
(p.datapath.uuid, p.datapath.uuid)
for p in self.ports
})
lnn.assert_called_once_with()
tdp.assert_called_once_with('3')
def test_ensure_all_networks_provisioned(self):
"""Test networks are provisioned.
def test_get_networks(self):
"""Test which networks are provisioned.
This test simulates that this chassis has the following ports:
* datapath '0': 1 port
@ -115,44 +127,27 @@ class TestMetadataAgent(base.BaseTestCase):
* datapath '3': 1 port with type 'external'
* datapath '5': 1 port with type 'unknown'
It is expected that only datapaths '0', '1' and '2' are provisioned
once.
It is expected that only datapaths '0', '1' and '2' are scheduled for
provisioning.
"""
ports = []
for i in range(0, 3):
ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
external_ids={'name': 'neutron-%d' % i})))
ports.append(makePort(datapath=DatapathInfo(uuid='1',
self.ports.append(makePort(datapath=DatapathInfo(uuid='1',
external_ids={'name': 'neutron-1'})))
ports.append(makePort(datapath=DatapathInfo(uuid='3',
self.ports.append(makePort(datapath=DatapathInfo(uuid='3',
external_ids={'name': 'neutron-3'}), type='external'))
ports.append(makePort(datapath=DatapathInfo(uuid='5',
self.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,\
mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis',
return_value=ports):
self.agent.ensure_all_networks_provisioned()
expected_calls = [mock.call(str(i), str(i)) for i in range(0, 4)]
self.assertEqual(sorted(expected_calls),
sorted(pdp.call_args_list))
expected_networks = {(str(i), str(i)) for i in range(0, 4)}
self.assertEqual(expected_networks, self.agent.get_networks())
def test_update_datapath_provision(self):
ports = []
for i in range(0, 3):
ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
external_ids={'name': 'neutron-%d' % i})))
ports.append(makePort(datapath=DatapathInfo(uuid='3',
self.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):
mock.patch.object(self.agent, 'teardown_datapath') as tdp:
self.agent.update_datapath('1', 'a')
self.agent.update_datapath('3', 'b')
expected_calls = [mock.call('1', 'a'), mock.call('3', 'b')]
@ -160,16 +155,9 @@ class TestMetadataAgent(base.BaseTestCase):
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),
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):
mock.patch.object(self.agent, 'teardown_datapath') as tdp:
self.agent.update_datapath('5', 'a')
tdp.assert_called_once_with('5', 'a')
pdp.assert_not_called()