Merge "fix netns deletion of broken namespaces" into unmaintained/yoga

This commit is contained in:
Zuul 2024-06-24 15:39:24 +00:00 committed by Gerrit Code Review
commit e86a4609f0
4 changed files with 60 additions and 2 deletions

View File

@ -260,7 +260,22 @@ class IPWrapper(SubProcessBase):
return ip return ip
def namespace_is_empty(self): def namespace_is_empty(self):
return not self.get_devices() try:
return not self.get_devices()
except OSError as e:
# This can happen if we previously got terminated in the middle of
# removing this namespace. In this case the bind mount of the
# namespace under /var/run/netns will be removed, but the namespace
# file is still there. As the bind mount is gone we can no longer
# access the namespace to validate that it is empty. But since it
# should have already been removed we are sure that the check has
# passed the last time and since the namespace is unuseable that
# can not have changed.
# Future calls to pyroute2 to remove that namespace will clean up
# the leftover file.
if e.errno == errno.EINVAL:
return True
raise e
def garbage_collect_namespace(self): def garbage_collect_namespace(self):
"""Conditionally destroy the namespace if it is empty.""" """Conditionally destroy the namespace if it is empty."""

View File

@ -371,7 +371,10 @@ class MetadataAgent(object):
ns.startswith(NS_PREFIX) and ns.startswith(NS_PREFIX) and
ns not in metadata_namespaces] ns not in metadata_namespaces]
for ns in unused_namespaces: for ns in unused_namespaces:
self.teardown_datapath(self._get_datapath_name(ns)) try:
self.teardown_datapath(self._get_datapath_name(ns))
except Exception:
LOG.exception('Error unable to destroy namespace: %s', ns)
# resync all network namespaces based on the associated datapaths, # resync all network namespaces based on the associated datapaths,
# even those that are already running. This is to make sure # even those that are already running. This is to make sure

View File

@ -356,6 +356,21 @@ class TestIpWrapper(base.BaseTestCase):
self.assertNotIn(mock.call().delete('ns'), self.assertNotIn(mock.call().delete('ns'),
ip_ns_cmd_cls.mock_calls) ip_ns_cmd_cls.mock_calls)
def test_garbage_collect_namespace_existing_broken(self):
with mock.patch.object(ip_lib, 'IpNetnsCommand') as ip_ns_cmd_cls:
ip_ns_cmd_cls.return_value.exists.return_value = True
ip = ip_lib.IPWrapper(namespace='ns')
with mock.patch.object(ip, 'get_devices',
side_effect=OSError(errno.EINVAL, None)
) as mock_get_devices:
self.assertTrue(ip.garbage_collect_namespace())
mock_get_devices.assert_called_once_with()
expected = [mock.call().delete('ns')]
ip_ns_cmd_cls.assert_has_calls(expected)
@mock.patch.object(priv_lib, 'create_interface') @mock.patch.object(priv_lib, 'create_interface')
def test_add_vlan(self, create): def test_add_vlan(self, create):
retval = ip_lib.IPWrapper().add_vlan('eth0.1', 'eth0', '1') retval = ip_lib.IPWrapper().add_vlan('eth0.1', 'eth0', '1')

View File

@ -134,6 +134,31 @@ class TestMetadataAgent(base.BaseTestCase):
lnn.assert_called_once_with() lnn.assert_called_once_with()
tdp.assert_called_once_with('3') tdp.assert_called_once_with('3')
def test_sync_teardown_namespace_does_not_crash_on_error(self):
"""Test that sync tears down unneeded metadata namespaces.
Even if that fails it continues to provision other datapaths
"""
with mock.patch.object(
self.agent, 'provision_datapath') as pdp,\
mock.patch.object(
ip_lib, 'list_network_namespaces',
return_value=['ovnmeta-1', 'ovnmeta-2', 'ovnmeta-3',
'ns1', 'ns2']) as lnn,\
mock.patch.object(
self.agent, 'teardown_datapath',
side_effect=Exception()) as tdp:
self.agent.sync()
pdp.assert_has_calls(
[
mock.call(p.datapath)
for p in self.ports
],
any_order=True
)
lnn.assert_called_once_with()
tdp.assert_called_once_with('3')
def test_get_networks_datapaths(self): def test_get_networks_datapaths(self):
"""Test get_networks_datapaths returns only datapath objects for the """Test get_networks_datapaths returns only datapath objects for the
networks containing vif ports of type ''(blank) and 'external'. networks containing vif ports of type ''(blank) and 'external'.