From 7e90c5fb73612d73b393a7aa0517fb17d81faf77 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Mon, 1 Oct 2018 13:49:32 -0400 Subject: [PATCH] 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 --- neutron/agent/linux/dhcp.py | 3 - neutron/privileged/agent/linux/ip_lib.py | 41 +++++++++-- neutron/tests/unit/agent/linux/test_dhcp.py | 18 +++-- neutron/tests/unit/agent/linux/test_ip_lib.py | 71 +++++++++++++++++++ 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 42261cb91a5..fd20cce591c 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -262,9 +262,6 @@ class DhcpLocalProcess(DhcpBase): LOG.warning('Failed trying to delete interface: %s', self.interface_name) - if not ip_lib.network_namespace_exists(self.network.namespace): - LOG.debug("Namespace already deleted: %s", self.network.namespace) - return try: ip_lib.delete_network_namespace(self.network.namespace) except RuntimeError: diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index b5f0b72a382..a47c7bfee9d 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -90,6 +90,13 @@ class IpAddressAlreadyExists(RuntimeError): 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 def get_routing_table(ip_version, namespace=None): """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: raise NetworkNamespaceNotFound(netns_name=namespace) raise + routes = [] with pyroute2.IPDB(nl=netns) as ipdb: ipdb_routes = ipdb.routes ipdb_interfaces = ipdb.interfaces - routes = [{'destination': route['dst'], - 'nexthop': route.get('gateway'), - 'device': ipdb_interfaces[route['oif']]['ifname'], - 'scope': _get_scope_name(route['scope'])} - for route in ipdb_routes if route['family'] == family] + for route in ipdb_routes: + if route['family'] != family: + continue + dst = route['dst'] + 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 @@ -404,7 +429,11 @@ def remove_netns(name, **kwargs): :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 diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 20b806c3c14..2563257150f 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -1134,9 +1134,7 @@ class TestDhcpLocalProcess(TestBase): self.assertTrue(lp.process_monitor.unregister.called) self.assertTrue(self.external_process().disable.called) - @mock.patch('neutron.agent.linux.ip_lib.network_namespace_exists') - def test_disable_not_active(self, namespace_exists): - namespace_exists.return_value = False + def test_disable_not_active(self): attrs_to_mock = dict([(a, mock.DEFAULT) for a in ['active', 'interface_name']]) with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks: @@ -1145,11 +1143,15 @@ class TestDhcpLocalProcess(TestBase): network = FakeDualNetwork() lp = LocalChild(self.conf, network) 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( network, 'tap0') self._assert_disabled(lp) + delete_ns.assert_called_with('qdhcp-ns') + def test_disable_retain_port(self): attrs_to_mock = dict([(a, mock.DEFAULT) for a in ['active', 'interface_name']]) @@ -1161,9 +1163,7 @@ class TestDhcpLocalProcess(TestBase): lp.disable(retain_port=True) self._assert_disabled(lp) - @mock.patch('neutron.agent.linux.ip_lib.network_namespace_exists') - def test_disable(self, namespace_exists): - namespace_exists.return_value = True + def test_disable(self): attrs_to_mock = {'active': mock.DEFAULT} with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks: @@ -1177,9 +1177,7 @@ class TestDhcpLocalProcess(TestBase): 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, namespace_exists): - namespace_exists.return_value = True + def test_disable_config_dir_removed_after_destroy(self): parent = mock.MagicMock() parent.attach_mock(self.rmtree, 'rmtree') parent.attach_mock(self.mock_mgr, 'DeviceManager') diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 7f6b41621d6..ba8c0a960ab 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -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): super(TestGetRoutingTable, self).setUp() self.addCleanup(privileged.default.set_client_mode, True) @@ -1610,6 +1659,28 @@ class TestGetRoutingTable(base.BaseTestCase): 'scope': 'universe'}] 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): def setUp(self):