Merge "Fix netns_cleanup interrupted on rwd I/O"

neutron/agent/linux/ip_lib.py

@@ -131,14 +131,10 @@ class IPWrapper(SubProcessBase):
                 cmd = ['ip', 'netns', 'exec', self.namespace,
                        'find', SYS_NET_PATH, '-maxdepth', '1',
                        '-type', 'l', '-printf', '%f ']
-                output_str = utils.execute(
+                output = utils.execute(
                     cmd,
                     run_as_root=True,
-                    log_fail_as_error=self.log_fail_as_error)
+                    log_fail_as_error=self.log_fail_as_error).split()
-                # NOTE(dalvarez): Logging the output of this call due to
-                # bug1654287.
-                LOG.debug('get_devices(): %s', output_str)
-                output = output_str.split()
             except RuntimeError:
                 # We could be racing with a cron job deleting namespaces.
                 # Just return a empty list if the namespace is deleted.

neutron/cmd/netns_cleanup.py

@@ -35,7 +35,6 @@ from neutron.agent.linux import interface
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import utils
 from neutron.common import config
-from neutron.common import utils as common_utils
 from neutron.conf.agent import cmd
 from neutron.conf.agent import dhcp as dhcp_config
 
@@ -162,14 +161,19 @@ def wait_until_no_listen_pids_namespace(namespace, timeout=SIGTERM_WAITTIME):
     If after timeout seconds, there are remaining processes in the namespace,
     then a PidsInNamespaceException will be thrown.
     """
-    # Would be better to handle an eventlet.timeout.Timeout exception
+    # NOTE(dalvarez): This function can block forever if
-    # but currently there's a problem importing eventlet since it's
+    # find_listen_pids_in_namespace never returns which is really unlikely. We
-    # doing a local import from cmd/eventlet which doesn't have a
+    # can't use wait_until_true because we might get interrupted by eventlet
-    # timeout module
+    # Timeout during our I/O with rootwrap daemon and that will lead to errors
-    common_utils.wait_until_true(
+    # in subsequent calls to utils.execute grabbing always the output of the
-        lambda: not find_listen_pids_namespace(namespace),
+    # previous command
-        timeout=SIGTERM_WAITTIME,
+    start = end = time.time()
-        exception=PidsInNamespaceException)
+    while end - start < timeout:
+        if not find_listen_pids_namespace(namespace):
+            return
+        time.sleep(1)
+        end = time.time()
+    raise PidsInNamespaceException
 
 
 def _kill_listen_processes(namespace, force=False):

neutron/tests/unit/cmd/test_netns_cleanup.py

@@ -274,11 +274,9 @@ class TestNetnsCleanup(base.BaseTestCase):
     def test_kill_listen_processes(self):
         with mock.patch.object(util, '_kill_listen_processes',
                                return_value=1) as mock_kill_listen:
-            with mock.patch('neutron.common.utils.wait_until_true')\
+            with mock.patch.object(util, 'wait_until_no_listen_pids_namespace',
-                    as wait_until_true_mock:
+                                   side_effect=[util.PidsInNamespaceException,
-                wait_until_true_mock.side_effect = [
+                                                None]):
-                    util.PidsInNamespaceException,
-                    None]
                 namespace = mock.ANY
                 util.kill_listen_processes(namespace)
                 mock_kill_listen.assert_has_calls(
@@ -288,10 +286,8 @@ class TestNetnsCleanup(base.BaseTestCase):
     def test_kill_listen_processes_still_procs(self):
         with mock.patch.object(util, '_kill_listen_processes',
                                return_value=1):
-            with mock.patch('neutron.common.utils.wait_until_true')\
+            with mock.patch.object(util, 'wait_until_no_listen_pids_namespace',
-                    as wait_until_true_mock:
+                            side_effect=util.PidsInNamespaceException):
-                wait_until_true_mock.side_effect = (
-                    util.PidsInNamespaceException)
                 namespace = mock.ANY
                 with testtools.ExpectedException(
                         util.PidsInNamespaceException):
@@ -300,13 +296,14 @@ class TestNetnsCleanup(base.BaseTestCase):
     def test_kill_listen_processes_no_procs(self):
         with mock.patch.object(util, '_kill_listen_processes',
                                return_value=0) as mock_kill_listen:
-            with mock.patch('neutron.common.utils.wait_until_true')\
+            with mock.patch.object(util,
-                    as wait_until_true_mock:
+                                   'wait_until_no_listen_pids_namespace')\
+                    as wait_until_mock:
                 namespace = mock.ANY
                 util.kill_listen_processes(namespace)
                 mock_kill_listen.assert_called_once_with(namespace,
                                                          force=False)
-                self.assertFalse(wait_until_true_mock.called)
+                self.assertFalse(wait_until_mock.called)
 
     def _test_destroy_namespace_helper(self, force, num_devices):
         ns = 'qrouter-6e322ac7-ab50-4f53-9cdc-d1d3c1164b6d'