From 7336f3bd27d138b3d11d601f977a1e3df2a44b3e 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 --- 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 1fea47858..529e391d6 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 624e3e3e6..777e10ad7 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 f867da9ca..c9bbe1352 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 fdb6c9b75..865cd4bbb 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -51,6 +51,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), @@ -190,6 +192,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 166733a5c..1cf9ca4ab 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -656,9 +656,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): @@ -677,11 +682,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 eec3e8690..528dbb5cf 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -527,13 +527,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 -- 2.45.2