From 91657c1612b4cc037c74b77f4b3548f843c10fcd Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Tue, 12 Nov 2013 19:31:45 +0000 Subject: [PATCH] Optionally delete namespaces when they are no longer needed Adds a configuration option to tell the network agents to delete namespaces when they are no longer in use. The option defaults to False so that the agent will not attempt to delete namespaces in environments where this is not safe. This has been working well in deployments where iproute2 has been patched with commit 58a3e8270fe72f8ed92687d3a3132c2a708582dd or it is new enough to include it without being patched. Change-Id: Ice5242c6f0446d16aaaa7ee353d674310297ef72 Closes-Bug: #1250596 Related-Bug: #1052535 (cherry picked from commit 7336f3bd27d138b3d11d601f977a1e3df2a44b3e) Related-Bug: #1175695 --- etc/dhcp_agent.ini | 8 ++++++++ etc/l3_agent.ini | 8 ++++++++ neutron/agent/l3_agent.py | 10 +++++++++- neutron/agent/linux/dhcp.py | 12 ++++++++++++ neutron/tests/unit/test_l3_agent.py | 21 +++++++++++++++++++++ neutron/tests/unit/test_linux_dhcp.py | 18 +++++++++++++++++- 6 files changed, 75 insertions(+), 2 deletions(-) diff --git a/etc/dhcp_agent.ini b/etc/dhcp_agent.ini index 33498ab3702..70ee75bc31b 100644 --- a/etc/dhcp_agent.ini +++ b/etc/dhcp_agent.ini @@ -70,3 +70,11 @@ # Location of Metadata Proxy UNIX domain socket # metadata_proxy_socket = $state_path/metadata_proxy + +# dhcp_delete_namespaces, which is false by default, can be set to True if +# namespaces can be deleted cleanly on the host running the dhcp agent. +# Do not enable this until you understand the problem with the Linux iproute +# utility mentioned in https://bugs.launchpad.net/neutron/+bug/1052535 and +# you are sure that your version of iproute does not suffer from the problem. +# If True, namespaces will be deleted when a dhcp server is disabled. +# dhcp_delete_namespaces = False diff --git a/etc/l3_agent.ini b/etc/l3_agent.ini index 3815071cf28..6e7f44fed03 100644 --- a/etc/l3_agent.ini +++ b/etc/l3_agent.ini @@ -63,3 +63,11 @@ # Location of Metadata Proxy UNIX domain socket # metadata_proxy_socket = $state_path/metadata_proxy + +# router_delete_namespaces, which is false by default, can be set to True if +# namespaces can be deleted cleanly on the host running the L3 agent. +# Do not enable this until you understand the problem with the Linux iproute +# utility mentioned in https://bugs.launchpad.net/neutron/+bug/1052535 and +# you are sure that your version of iproute does not suffer from the problem. +# If True, namespaces will be deleted when a router is destroyed. +# router_delete_namespaces = False diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index b31c7465bef..c2d57e5597c 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -180,6 +180,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): "by the agents.")), cfg.BoolOpt('enable_metadata_proxy', default=True, help=_("Allow running metadata proxy.")), + cfg.BoolOpt('router_delete_namespaces', default=False, + help=_("Delete namespace after removing a router.")), cfg.StrOpt('metadata_proxy_socket', default='$state_path/metadata_proxy', help=_('Location of Metadata Proxy UNIX domain ' @@ -272,7 +274,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): bridge=self.conf.external_network_bridge, namespace=namespace, prefix=EXTERNAL_DEV_PREFIX) - #TODO(garyk) Address the failure for the deletion of the namespace + + if self.conf.router_delete_namespaces: + try: + ns_ip.netns.delete(namespace) + except RuntimeError: + msg = _('Failed trying to delete namespace: %s') + LOG.exception(msg % namespace) def _create_router_namespace(self, ri): ip_wrapper_root = ip_lib.IPWrapper(self.root_helper) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 7d25bcf806b..afd6b662113 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -50,6 +50,8 @@ OPTS = [ cfg.StrOpt('dnsmasq_dns_server', help=_('Use another DNS server before any in ' '/etc/resolv.conf.')), + cfg.BoolOpt('dhcp_delete_namespaces', default=False, + help=_("Delete namespace after removing a dhcp server.")), cfg.IntOpt( 'dnsmasq_lease_max', default=(2 ** 24), @@ -189,6 +191,16 @@ class DhcpLocalProcess(DhcpBase): self._remove_config_files() + if not retain_port: + if self.conf.dhcp_delete_namespaces and self.network.namespace: + ns_ip = ip_lib.IPWrapper(self.root_helper, + self.network.namespace) + try: + ns_ip.netns.delete(self.network.namespace) + except RuntimeError: + msg = _('Failed trying to delete namespace: %s') + LOG.exception(msg, self.network.namespace) + def _remove_config_files(self): confs_dir = os.path.abspath(os.path.normpath(self.conf.dhcp_confs)) conf_dir = os.path.join(confs_dir, self.network.id) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index becb04a7b4e..070927b1f76 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -628,9 +628,14 @@ class TestBasicRouterOperations(base.BaseTestCase): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + pm = self.external_process.return_value + pm.reset_mock() + agent._destroy_router_namespace = mock.MagicMock() agent._destroy_router_namespaces() + self.assertEqual(pm.disable.call_count, 2) + self.assertEqual(agent._destroy_router_namespace.call_count, 2) def test_destroy_namespace_with_router_id(self): @@ -649,11 +654,27 @@ class TestBasicRouterOperations(base.BaseTestCase): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + pm = self.external_process.return_value + pm.reset_mock() + agent._destroy_router_namespace = mock.MagicMock() agent._destroy_router_namespaces(self.conf.router_id) + self.assertEqual(pm.disable.call_count, 1) + self.assertEqual(agent._destroy_router_namespace.call_count, 1) + def test_destroy_router_namespace_skips_ns_removal(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + agent._destroy_router_namespace("fakens") + self.assertEqual(self.mock_ip.netns.delete.call_count, 0) + + def test_destroy_router_namespace_removes_ns(self): + self.conf.set_override('router_delete_namespaces', True) + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + agent._destroy_router_namespace("fakens") + self.mock_ip.netns.delete.assert_called_once_with("fakens") + def _configure_metadata_proxy(self, enableflag=True): if not enableflag: self.conf.set_override('enable_metadata_proxy', False) diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 7842acb6f0f..6f6f0d2d694 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -495,13 +495,29 @@ class TestDhcpLocalProcess(TestBase): mocks['pid'].__get__ = mock.Mock(return_value=5) mocks['interface_name'].__get__ = mock.Mock(return_value='tap0') lp = LocalChild(self.conf, network) - lp.disable() + with mock.patch('neutron.agent.linux.ip_lib.IPWrapper') as ip: + lp.disable() self.mock_mgr.assert_has_calls([mock.call(self.conf, 'sudo', None), mock.call().destroy(network, 'tap0')]) exp_args = ['kill', '-9', 5] self.execute.assert_called_once_with(exp_args, 'sudo') + self.assertEqual(ip.return_value.netns.delete.call_count, 0) + + def test_disable_delete_ns(self): + self.conf.set_override('dhcp_delete_namespaces', True) + attrs_to_mock = dict([(a, mock.DEFAULT) for a in ['active', 'pid']]) + + with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks: + mocks['active'].__get__ = mock.Mock(return_value=False) + mocks['pid'].__get__ = mock.Mock(return_value=False) + lp = LocalChild(self.conf, FakeDualNetwork()) + with mock.patch('neutron.agent.linux.ip_lib.IPWrapper') as ip: + lp.disable() + + ip.return_value.netns.delete.assert_called_with('qdhcp-ns') + def test_pid(self): with mock.patch('__builtin__.open') as mock_open: mock_open.return_value.__enter__ = lambda s: s