Merge "Optionally delete namespaces when they are no longer needed"

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

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

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)

neutron/agent/linux/dhcp.py

@@ -52,6 +52,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),
@@ -191,6 +193,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)

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)

neutron/tests/unit/test_linux_dhcp.py

@@ -525,13 +525,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