From 9b5871c658e1ec77bc29da507a2471a8b81b3702 Mon Sep 17 00:00:00 2001 From: Gary Kotton Date: Mon, 10 Sep 2012 04:19:09 -0400 Subject: [PATCH] Ensure that l3 agent does not crash on restart Fixes bug 1048108 The changeset also removed the namespace garbage collection from the port unplug. This attempts to delete the namespace. This is problematic as there may be additional attributes to the namespace that need to be dealt with. Change-Id: I418a1ec4e9b65e5bea67ae84414019c3d6b54214 --- quantum/agent/l3_agent.py | 30 +++++++++++++------------ quantum/agent/linux/interface.py | 8 ------- quantum/agent/linux/ip_lib.py | 5 +++-- quantum/tests/unit/test_linux_dhcp.py | 12 ++++++---- quantum/tests/unit/test_linux_ip_lib.py | 5 +++-- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py index bff54f834..6a302b888 100644 --- a/quantum/agent/l3_agent.py +++ b/quantum/agent/l3_agent.py @@ -29,6 +29,7 @@ from quantum.agent.common import config from quantum.agent.linux import interface from quantum.agent.linux import ip_lib from quantum.agent.linux import iptables_manager +from quantum.agent.linux import utils from quantum.db import l3_db from quantum.openstack.common import cfg from quantum.openstack.common import importutils @@ -147,22 +148,19 @@ class L3NATAgent(object): def _destroy_router_namespace(self, namespace): ns_ip = ip_lib.IPWrapper(self.conf.root_helper, namespace=namespace) - for d in ns_ip.get_devices(): + for d in ns_ip.get_devices(exclude_loopback=True): if d.name.startswith(INTERNAL_DEV_PREFIX): # device is on default bridge - self.driver.unplug(d.name) + self.driver.unplug(d.name, namespace=namespace) elif d.name.startswith(EXTERNAL_DEV_PREFIX): self.driver.unplug(d.name, - bridge=self.conf.external_network_bridge) - if self.conf.use_namespaces: - ns_ip.netns.delete(namespace) + bridge=self.conf.external_network_bridge, + namespace=namespace) + #(TODO) Address the failure for the deletion of the namespace def _create_router_namespace(self, ri): ip_wrapper_root = ip_lib.IPWrapper(self.conf.root_helper) - ip_wrapper_root.netns.add(ri.ns_name()) - - ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, - namespace=ri.ns_name()) + ip_wrapper = ip_wrapper_root.ensure_namespace(ri.ns_name()) ip_wrapper.netns.execute(['sysctl', '-w', 'net.ipv4.ip_forward=1']) def daemon_loop(self): @@ -364,10 +362,13 @@ class L3NATAgent(object): gw_ip = ex_gw_port['subnet']['gateway_ip'] if ex_gw_port['subnet']['gateway_ip']: cmd = ['route', 'add', 'default', 'gw', gw_ip] - ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, - namespace=ri.ns_name()) if self.conf.use_namespaces: - ip_wrapper.netns.execute(cmd) + ip_wrapper = ip_lib.IPWrapper(self.conf.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(cmd, check_exit_code=False) + else: + utils.execute(cmd, check_exit_code=False, + root_helper=self.conf.root_helper) for (c, r) in self.external_gateway_filter_rules(): ri.iptables_manager.ipv4['filter'].add_rule(c, r) @@ -384,7 +385,8 @@ class L3NATAgent(object): root_helper=self.conf.root_helper, namespace=ri.ns_name()): self.driver.unplug(interface_name, - bridge=self.conf.external_network_bridge) + bridge=self.conf.external_network_bridge, + namespace=ri.ns_name()) ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address'] for c, r in self.external_gateway_filter_rules(): @@ -442,7 +444,7 @@ class L3NATAgent(object): if ip_lib.device_exists(interface_name, root_helper=self.conf.root_helper, namespace=ri.ns_name()): - self.driver.unplug(interface_name) + self.driver.unplug(interface_name, namespace=ri.ns_name()) if ex_gw_port: ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address'] diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py index 8cf9a4dd9..36017c05e 100644 --- a/quantum/agent/linux/interface.py +++ b/quantum/agent/linux/interface.py @@ -154,10 +154,6 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): bridge = ovs_lib.OVSBridge(bridge, self.conf.root_helper) bridge.delete_port(device_name) - if namespace: - ip = ip_lib.IPWrapper(self.conf.root_helper, namespace) - ip.garbage_collect_namespace() - class BridgeInterfaceDriver(LinuxInterfaceDriver): """Driver for creating bridge interfaces.""" @@ -200,10 +196,6 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): LOG.error(_("Failed unplugging interface '%s'") % device_name) - if namespace: - ip = ip_lib.IPWrapper(self.conf.root_helper, namespace) - ip.garbage_collect_namespace() - class RyuInterfaceDriver(OVSInterfaceDriver): """Driver for creating a Ryu OVS interface.""" diff --git a/quantum/agent/linux/ip_lib.py b/quantum/agent/linux/ip_lib.py index cdde30a9c..510dcec61 100644 --- a/quantum/agent/linux/ip_lib.py +++ b/quantum/agent/linux/ip_lib.py @@ -338,7 +338,7 @@ class IpNetnsCommand(IpCommandBase): def delete(self, name): self._as_root('delete', name, use_root_namespace=True) - def execute(self, cmds, addl_env={}): + def execute(self, cmds, addl_env={}, check_exit_code=True): if not self._parent.root_helper: raise exceptions.SudoRequired() elif not self._parent.namespace: @@ -347,7 +347,8 @@ class IpNetnsCommand(IpCommandBase): return utils.execute( ['%s=%s' % pair for pair in addl_env.items()] + ['ip', 'netns', 'exec', self._parent.namespace] + list(cmds), - root_helper=self._parent.root_helper) + root_helper=self._parent.root_helper, + check_exit_code=check_exit_code) def exists(self, name): output = self._as_root('list', options='o', use_root_namespace=True) diff --git a/quantum/tests/unit/test_linux_dhcp.py b/quantum/tests/unit/test_linux_dhcp.py index 8a32f81c7..9d239506f 100644 --- a/quantum/tests/unit/test_linux_dhcp.py +++ b/quantum/tests/unit/test_linux_dhcp.py @@ -331,7 +331,8 @@ class TestDhcpLocalProcess(TestBase): self.assertFalse(delegate.called) exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5] - self.execute.assert_called_once_with(exp_args, root_helper='sudo') + self.execute.assert_called_once_with(exp_args, root_helper='sudo', + check_exit_code=True) def test_disable(self): attrs_to_mock = dict([(a, mock.DEFAULT) for a in @@ -348,7 +349,8 @@ class TestDhcpLocalProcess(TestBase): delegate.assert_has_calls([mock.call.destroy(network, 'tap0')]) exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5] - self.execute.assert_called_once_with(exp_args, root_helper='sudo') + self.execute.assert_called_once_with(exp_args, root_helper='sudo', + check_exit_code=True) def test_pid(self): with mock.patch('__builtin__.open') as mock_open: @@ -452,7 +454,8 @@ class TestDnsmasq(TestBase): dm.spawn_process() self.assertTrue(mocks['_output_opts_file'].called) self.execute.assert_called_once_with(expected, - root_helper='sudo') + root_helper='sudo', + check_exit_code=True) def test_spawn(self): self._test_spawn([]) @@ -539,7 +542,8 @@ tag:tag1,option:classless-static-route,%s,%s""".lstrip() % (fake_v6, self.safe.assert_has_calls([mock.call(exp_host_name, exp_host_data), mock.call(exp_opt_name, exp_opt_data)]) - self.execute.assert_called_once_with(exp_args, root_helper='sudo') + self.execute.assert_called_once_with(exp_args, root_helper='sudo', + check_exit_code=True) def _test_lease_relay_script_helper(self, action, lease_remaining, path_exists=True): diff --git a/quantum/tests/unit/test_linux_ip_lib.py b/quantum/tests/unit/test_linux_ip_lib.py index 18958b6fa..c7f96918a 100644 --- a/quantum/tests/unit/test_linux_ip_lib.py +++ b/quantum/tests/unit/test_linux_ip_lib.py @@ -580,7 +580,8 @@ class TestIpNetnsCommand(TestIPCmdBase): self.netns_cmd.execute(['ip', 'link', 'list']) execute.assert_called_once_with(['ip', 'netns', 'exec', 'ns', 'ip', 'link', 'list'], - root_helper='sudo') + root_helper='sudo', + check_exit_code=True) def test_execute_env_var_prepend(self): self.parent.namespace = 'ns' @@ -590,7 +591,7 @@ class TestIpNetnsCommand(TestIPCmdBase): execute.assert_called_once_with( ['FOO=1', 'BAR=2', 'ip', 'netns', 'exec', 'ns', 'ip', 'link', 'list'], - root_helper='sudo') + root_helper='sudo', check_exit_code=True) class TestDeviceExists(unittest.TestCase): -- 2.45.2