Try to enable dnsmasq process several times

Sometimes, during restart of dnsmasq process it may happend that
after process is killed, start attempt is made too fast, before
old process really unbind from IP address on which it was listening.
That causes an issue with starting dnsmasq process again.

In patch [1] disable() method was changed that it can wait
until process is really not active (no pid for it) but that didn't
solve the problem with starting a new dnsmasq process completely and
sometimes it still happens, at least in functional tests.

So now, enable() method is changed so that it will try to enable
dnsmasq process for 1 minute, until it will really be spawned properly.

[1] https://review.openstack.org/#/c/634390/

Change-Id: I18d73b787fa3ab8803e12d5e5eb2bb7109205aba
Closes-Bug: #1811126
(cherry picked from commit 157e09e6af)

neutron/agent/linux/dhcp.py

@@ -39,6 +39,7 @@ from neutron.agent.linux import ip_lib
 from neutron.agent.linux import iptables_manager
 from neutron.cmd import runtime_checks as checks
 from neutron.common import constants as n_const
+from neutron.common import exceptions as n_exc
 from neutron.common import ipv6_utils
 from neutron.common import utils as common_utils
 from neutron.ipam import utils as ipam_utils
@@ -217,13 +218,26 @@ class DhcpLocalProcess(DhcpBase):
 
     def enable(self):
         """Enables DHCP for this network by spawning a local process."""
-        if self.active:
-            self.restart()
-        elif self._enable_dhcp():
-            fileutils.ensure_tree(self.network_conf_dir, mode=0o755)
-            interface_name = self.device_manager.setup(self.network)
-            self.interface_name = interface_name
-            self.spawn_process()
+        try:
+            common_utils.wait_until_true(self._enable)
+        except common_utils.WaitTimeout:
+            LOG.error("Failed to start DHCP process for network %s",
+                      self.network.id)
+
+    def _enable(self):
+        try:
+            if self.active:
+                self.restart()
+            elif self._enable_dhcp():
+                fileutils.ensure_tree(self.network_conf_dir, mode=0o755)
+                interface_name = self.device_manager.setup(self.network)
+                self.interface_name = interface_name
+                self.spawn_process()
+            return True
+        except n_exc.ProcessExecutionError as error:
+            LOG.debug("Spawning DHCP process for network %s failed; "
+                      "Error: %s", self.network.id, error)
+            return False
 
     def _get_process_manager(self, cmd_callback=None):
         return external_process.ProcessManager(
@@ -237,10 +251,9 @@ class DhcpLocalProcess(DhcpBase):
     def disable(self, retain_port=False, block=False):
         """Disable DHCP for this network by killing the local process."""
         self.process_monitor.unregister(self.network.id, DNSMASQ_SERVICE_NAME)
-        pm = self._get_process_manager()
-        pm.disable()
+        self._get_process_manager().disable()
         if block:
-            common_utils.wait_until_true(lambda: not pm.active)
+            common_utils.wait_until_true(lambda: not self.active)
         if not retain_port:
             self._destroy_namespace_and_port()
         self._remove_config_files()

neutron/tests/unit/agent/linux/test_dhcp.py

@@ -25,6 +25,7 @@ from oslo_utils import fileutils
 import testtools
 
 from neutron.agent.linux import dhcp
+from neutron.common import exceptions as n_exc
 from neutron.conf.agent import common as config
 from neutron.conf.agent import dhcp as dhcp_config
 from neutron.conf import common as base_config
@@ -1111,23 +1112,30 @@ class TestDhcpLocalProcess(TestBase):
     def test_enable(self, ensure_dir):
         attrs_to_mock = dict(
             [(a, mock.DEFAULT) for a in
-                ['active', 'interface_name']]
+                ['active', 'interface_name', 'spawn_process']]
         )
 
         with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
             mocks['active'].__get__ = mock.Mock(return_value=False)
             mocks['interface_name'].__set__ = mock.Mock()
+            mocks['spawn_process'].side_effect = [
+                n_exc.ProcessExecutionError(
+                    returncode=2, message="Test dnsmasq start failed"),
+                None]
             lp = LocalChild(self.conf,
                             FakeDualNetwork())
+
             lp.enable()
 
             self.mock_mgr.assert_has_calls(
                 [mock.call(self.conf, None),
                  mock.call().setup(mock.ANY)])
-            self.assertEqual(lp.called, ['spawn'])
-            self.assertTrue(mocks['interface_name'].__set__.called)
-            ensure_dir.assert_called_with(
-                '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc', mode=0o755)
+            self.assertEqual(2, mocks['interface_name'].__set__.call_count)
+            ensure_dir.assert_has_calls([
+                mock.call(
+                    '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc', mode=0o755),
+                mock.call(
+                    '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc', mode=0o755)])
 
     def _assert_disabled(self, lp):
         self.assertTrue(lp.process_monitor.unregister.called)