Do not fail deleting namespace if it does not exist

Note: this is a squash of two changes since they are
dependent on each other, and are currently blocking
the gate queue.

Sometimes cleanup methods are failing in the check and
gate queues trying to delete non-existing namespaces.
Since they could have been deleted asynchronously, don't
raise if the failure is "No such file or directory" since
the system is in the intended state.

Cleaned-up the DHCP agent to longer check for existence
first, and the tests to longer mock-out the namespace
exists check.

Fix test_legacy_router_lifecycle failures

Multi-path routes returned via the pyroute2 library have
their outgoing interfaces in the 'multipath' dictionary
element, not in the route dictionary.  In that case return
all the multipath routes correctly.

Change-Id: I5415cb3a88ff2640a19598a1fcb2278388815343
Closes-bug: #1795482
Closes-bug: #1795548
This commit is contained in:
Brian Haley 2018-10-01 13:49:32 -04:00 committed by Slawek Kaplonski
parent 2bcbd947b3
commit 7e90c5fb73
4 changed files with 114 additions and 19 deletions

View File

@ -262,9 +262,6 @@ class DhcpLocalProcess(DhcpBase):
LOG.warning('Failed trying to delete interface: %s', LOG.warning('Failed trying to delete interface: %s',
self.interface_name) self.interface_name)
if not ip_lib.network_namespace_exists(self.network.namespace):
LOG.debug("Namespace already deleted: %s", self.network.namespace)
return
try: try:
ip_lib.delete_network_namespace(self.network.namespace) ip_lib.delete_network_namespace(self.network.namespace)
except RuntimeError: except RuntimeError:

View File

@ -90,6 +90,13 @@ class IpAddressAlreadyExists(RuntimeError):
super(IpAddressAlreadyExists, self).__init__(message) super(IpAddressAlreadyExists, self).__init__(message)
def _make_route_dict(destination, nexthop, device, scope):
return {'destination': destination,
'nexthop': nexthop,
'device': device,
'scope': scope}
@privileged.default.entrypoint @privileged.default.entrypoint
def get_routing_table(ip_version, namespace=None): def get_routing_table(ip_version, namespace=None):
"""Return a list of dictionaries, each representing a route. """Return a list of dictionaries, each representing a route.
@ -109,14 +116,32 @@ def get_routing_table(ip_version, namespace=None):
if e.errno == errno.ENOENT: if e.errno == errno.ENOENT:
raise NetworkNamespaceNotFound(netns_name=namespace) raise NetworkNamespaceNotFound(netns_name=namespace)
raise raise
routes = []
with pyroute2.IPDB(nl=netns) as ipdb: with pyroute2.IPDB(nl=netns) as ipdb:
ipdb_routes = ipdb.routes ipdb_routes = ipdb.routes
ipdb_interfaces = ipdb.interfaces ipdb_interfaces = ipdb.interfaces
routes = [{'destination': route['dst'], for route in ipdb_routes:
'nexthop': route.get('gateway'), if route['family'] != family:
'device': ipdb_interfaces[route['oif']]['ifname'], continue
'scope': _get_scope_name(route['scope'])} dst = route['dst']
for route in ipdb_routes if route['family'] == family] nexthop = route.get('gateway')
oif = route.get('oif')
scope = _get_scope_name(route['scope'])
# If there is not a valid outgoing interface id, check if
# this is a multipath route (i.e. same destination with
# multiple outgoing interfaces)
if oif:
device = ipdb_interfaces[oif]['ifname']
rt = _make_route_dict(dst, nexthop, device, scope)
routes.append(rt)
elif route.get('multipath'):
for mpr in route['multipath']:
oif = mpr['oif']
device = ipdb_interfaces[oif]['ifname']
rt = _make_route_dict(dst, nexthop, device, scope)
routes.append(rt)
return routes return routes
@ -404,7 +429,11 @@ def remove_netns(name, **kwargs):
:param name: The name of the namespace to remove :param name: The name of the namespace to remove
""" """
netns.remove(name, **kwargs) try:
netns.remove(name, **kwargs)
except OSError as e:
if e.errno != errno.ENOENT:
raise
@privileged.default.entrypoint @privileged.default.entrypoint

View File

@ -1134,9 +1134,7 @@ class TestDhcpLocalProcess(TestBase):
self.assertTrue(lp.process_monitor.unregister.called) self.assertTrue(lp.process_monitor.unregister.called)
self.assertTrue(self.external_process().disable.called) self.assertTrue(self.external_process().disable.called)
@mock.patch('neutron.agent.linux.ip_lib.network_namespace_exists') def test_disable_not_active(self):
def test_disable_not_active(self, namespace_exists):
namespace_exists.return_value = False
attrs_to_mock = dict([(a, mock.DEFAULT) for a in attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name']]) ['active', 'interface_name']])
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks: with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
@ -1145,11 +1143,15 @@ class TestDhcpLocalProcess(TestBase):
network = FakeDualNetwork() network = FakeDualNetwork()
lp = LocalChild(self.conf, network) lp = LocalChild(self.conf, network)
lp.device_manager = mock.Mock() lp.device_manager = mock.Mock()
lp.disable() with mock.patch('neutron.agent.linux.ip_lib.'
'delete_network_namespace') as delete_ns:
lp.disable()
lp.device_manager.destroy.assert_called_once_with( lp.device_manager.destroy.assert_called_once_with(
network, 'tap0') network, 'tap0')
self._assert_disabled(lp) self._assert_disabled(lp)
delete_ns.assert_called_with('qdhcp-ns')
def test_disable_retain_port(self): def test_disable_retain_port(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name']]) ['active', 'interface_name']])
@ -1161,9 +1163,7 @@ class TestDhcpLocalProcess(TestBase):
lp.disable(retain_port=True) lp.disable(retain_port=True)
self._assert_disabled(lp) self._assert_disabled(lp)
@mock.patch('neutron.agent.linux.ip_lib.network_namespace_exists') def test_disable(self):
def test_disable(self, namespace_exists):
namespace_exists.return_value = True
attrs_to_mock = {'active': mock.DEFAULT} attrs_to_mock = {'active': mock.DEFAULT}
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks: with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
@ -1177,9 +1177,7 @@ class TestDhcpLocalProcess(TestBase):
delete_ns.assert_called_with('qdhcp-ns') delete_ns.assert_called_with('qdhcp-ns')
@mock.patch('neutron.agent.linux.ip_lib.network_namespace_exists') def test_disable_config_dir_removed_after_destroy(self):
def test_disable_config_dir_removed_after_destroy(self, namespace_exists):
namespace_exists.return_value = True
parent = mock.MagicMock() parent = mock.MagicMock()
parent.attach_mock(self.rmtree, 'rmtree') parent.attach_mock(self.rmtree, 'rmtree')
parent.attach_mock(self.mock_mgr, 'DeviceManager') parent.attach_mock(self.mock_mgr, 'DeviceManager')

View File

@ -1553,6 +1553,55 @@ class TestGetRoutingTable(base.BaseTestCase):
} }
] ]
ip_db_multipath_routes = [
{
'dst_len': 24,
'family': socket.AF_INET,
'proto': 3,
'tos': 0,
'dst': '10.0.1.0/24',
'flags': 16,
'ipdb_priority': 0,
'metrics': {},
'scope': 0,
'encap': {},
'src_len': 0,
'table': 254,
'multipath': ({'oif': 1, 'family': socket.AF_INET},
{'oif': 2, 'dst_len': 24, 'family': socket.AF_INET,
'proto': 2, 'tos': 0, 'pref': '00',
'priority': 256, 'flags': 0, 'encap': {},
'src_len': 0, 'table': 254, 'type': 1,
'scope': 0}),
'type': 1,
'gateway': '10.0.0.1',
'ipdb_scope': 'system'
}, {
'metrics': {},
'dst_len': 64,
'family': socket.AF_INET6,
'proto': 2,
'tos': 0,
'dst': '1111:1111:1111:1111::/64',
'pref': '00',
'ipdb_priority': 0,
'priority': 256,
'flags': 0,
'encap': {},
'src_len': 0,
'table': 254,
'multipath': ({'oif': 1, 'family': socket.AF_INET6},
{'oif': 2, 'dst_len': 64, 'family': socket.AF_INET6,
'proto': 2, 'tos': 0, 'pref': '00',
'priority': 256, 'flags': 0, 'encap': {},
'src_len': 0, 'table': 254, 'type': 1,
'scope': 0}),
'type': 1,
'scope': 0,
'ipdb_scope': 'system'
}
]
def setUp(self): def setUp(self):
super(TestGetRoutingTable, self).setUp() super(TestGetRoutingTable, self).setUp()
self.addCleanup(privileged.default.set_client_mode, True) self.addCleanup(privileged.default.set_client_mode, True)
@ -1610,6 +1659,28 @@ class TestGetRoutingTable(base.BaseTestCase):
'scope': 'universe'}] 'scope': 'universe'}]
self._test_get_routing_table(6, self.ip_db_routes, expected) self._test_get_routing_table(6, self.ip_db_routes, expected)
def test_get_routing_table_multipath_4(self):
expected = [{'destination': '10.0.1.0/24',
'nexthop': '10.0.0.1',
'device': 'lo',
'scope': 'universe'},
{'destination': '10.0.1.0/24',
'nexthop': '10.0.0.1',
'device': 'tap-1',
'scope': 'universe'}]
self._test_get_routing_table(4, self.ip_db_multipath_routes, expected)
def test_get_routing_table_multipath_6(self):
expected = [{'destination': '1111:1111:1111:1111::/64',
'nexthop': None,
'device': 'lo',
'scope': 'universe'},
{'destination': '1111:1111:1111:1111::/64',
'nexthop': None,
'device': 'tap-1',
'scope': 'universe'}]
self._test_get_routing_table(6, self.ip_db_multipath_routes, expected)
class TestIpNeighCommand(TestIPCmdBase): class TestIpNeighCommand(TestIPCmdBase):
def setUp(self): def setUp(self):