Fix detection of deleted networks in DHCP agent.

The DHCP-agent uses an in-memory networks cache to find out which networks must
be deleted and which ones must be updated. In a case of agent restart the networks
cache is empty and it's not possible to cleanup DHCP-processes serving networks
which were deleted while the DHCP-agent was down. The proposed fix fills the networks
cache when the agent starts using a list of networks which have existing config files.

Fixes: bug #1135948
Change-Id: I27758389755cd19bca9fdbeda9cc1a123370f527

diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py
index 0397ae1985be4bc73427a9884130f254902ead0b..1de865337da87925b7276ae7cc3e42cbc23c3943 100644 (file)
--- a/quantum/agent/dhcp_agent.py
+++ b/quantum/agent/dhcp_agent.py
@@ -79,6 +79,28 @@ class DhcpAgent(manager.Manager):
         self.device_manager = DeviceManager(self.conf, self.plugin_rpc)
         self.lease_relay = DhcpLeaseRelay(self.update_lease)
 
+        self._populate_networks_cache()
+
+    def _populate_networks_cache(self):
+        """Populate the networks cache when the DHCP-agent starts"""
+
+        try:
+            existing_networks = self.dhcp_driver_cls.existing_dhcp_networks(
+                self.conf,
+                self.root_helper
+            )
+
+            for net_id in existing_networks:
+                net = DictModel({"id": net_id, "subnets": [], "ports": []})
+                self.cache.put(net)
+        except NotImplementedError:
+            # just go ahead with an empty networks cache
+            LOG.debug(
+                _("The '%s' DHCP-driver does not support retrieving of a "
+                  "list of existing networks"),
+                self.conf.dhcp_driver
+            )
+
     def after_start(self):
         self.run()
         LOG.info(_("DHCP agent started"))

diff --git a/quantum/agent/linux/dhcp.py b/quantum/agent/linux/dhcp.py
index 38e7fd273f89f4cc5372efaa80e428892c6495c0..8b6f62d5a3dc6ef835f390588fc5ae9d39b5d758 100644 (file)
--- a/quantum/agent/linux/dhcp.py
+++ b/quantum/agent/linux/dhcp.py
@@ -18,6 +18,7 @@
 import abc
 import os
 import re
+import shutil
 import socket
 import StringIO
 import sys
@@ -29,6 +30,7 @@ from quantum.agent.linux import ip_lib
 from quantum.agent.linux import utils
 from quantum.openstack.common import jsonutils
 from quantum.openstack.common import log as logging
+from quantum.openstack.common import uuidutils
 
 LOG = logging.getLogger(__name__)
 
@@ -92,6 +94,12 @@ class DhcpBase(object):
     def reload_allocations(self):
         """Force the DHCP server to reload the assignment database."""
 
+    @classmethod
+    def existing_dhcp_networks(cls, conf, root_helper):
+        """Return a list of existing networks ids (ones we have configs for)"""
+
+        raise NotImplementedError
+
 
 class DhcpLocalProcess(DhcpBase):
     PORTS = []
@@ -134,6 +142,13 @@ class DhcpLocalProcess(DhcpBase):
         else:
             LOG.debug(_('No DHCP started for %s'), self.network.id)
 
+        self._remove_config_files()
+
+    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)
+        shutil.rmtree(conf_dir, ignore_errors=True)
+
     def get_conf_file_name(self, kind, ensure_conf_dir=False):
         """Returns the file name for a given kind of config file."""
         confs_dir = os.path.abspath(os.path.normpath(self.conf.dhcp_confs))
@@ -206,6 +221,22 @@ class Dnsmasq(DhcpLocalProcess):
     QUANTUM_NETWORK_ID_KEY = 'QUANTUM_NETWORK_ID'
     QUANTUM_RELAY_SOCKET_PATH_KEY = 'QUANTUM_RELAY_SOCKET_PATH'
 
+    @classmethod
+    def existing_dhcp_networks(cls, conf, root_helper):
+        """Return a list of existing networks ids (ones we have configs for)"""
+
+        confs_dir = os.path.abspath(os.path.normpath(conf.dhcp_confs))
+
+        class FakeNetwork:
+            def __init__(self, net_id):
+                self.id = net_id
+
+        return [
+            c for c in os.listdir(confs_dir)
+            if (uuidutils.is_uuid_like(c) and
+                cls(conf, FakeNetwork(c), root_helper).active)
+        ]
+
     def spawn_process(self):
         """Spawns a Dnsmasq process for the network."""
         env = {

diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py
index 9ac8dea758d655f4759a8b79bd138bc9ff608f23..d2b0dbe864a99361d4c2694a14800793fef78760 100644 (file)
--- a/quantum/tests/unit/test_dhcp_agent.py
+++ b/quantum/tests/unit/test_dhcp_agent.py
@@ -121,6 +121,7 @@ class TestDhcpAgent(base.BaseTestCase):
         self.driver_cls_p = mock.patch(
             'quantum.agent.dhcp_agent.importutils.import_class')
         self.driver = mock.Mock(name='driver')
+        self.driver.existing_dhcp_networks.return_value = []
         self.driver_cls = self.driver_cls_p.start()
         self.driver_cls.return_value = self.driver
 
@@ -324,6 +325,34 @@ class TestDhcpAgent(base.BaseTestCase):
                 sleep.assert_called_once_with(dhcp.conf.resync_interval)
                 self.assertFalse(dhcp.needs_resync)
 
+    def test_populate_cache_on_start_without_active_networks_support(self):
+        # emul dhcp driver that doesn't support retrieving of active networks
+        self.driver.existing_dhcp_networks.side_effect = NotImplementedError
+
+        with mock.patch.object(dhcp_agent.LOG, 'debug') as log:
+            dhcp = dhcp_agent.DhcpAgent(HOSTNAME)
+
+            self.driver.existing_dhcp_networks.assert_called_once_with(
+                dhcp.conf,
+                cfg.CONF.root_helper
+            )
+
+            self.assertFalse(dhcp.cache.get_network_ids())
+            self.assertTrue(log.called)
+
+    def test_populate_cache_on_start(self):
+        networks = ['aaa', 'bbb']
+        self.driver.existing_dhcp_networks.return_value = networks
+
+        dhcp = dhcp_agent.DhcpAgent(HOSTNAME)
+
+        self.driver.existing_dhcp_networks.assert_called_once_with(
+            dhcp.conf,
+            cfg.CONF.root_helper
+        )
+
+        self.assertEquals(set(networks), set(dhcp.cache.get_network_ids()))
+
 
 class TestLogArgs(base.BaseTestCase):
 

diff --git a/quantum/tests/unit/test_linux_dhcp.py b/quantum/tests/unit/test_linux_dhcp.py
index a3ab7fbce7293c4d33b80bf2918454ae27a205e8..dabd1dcf881162a7f7a8f6e002706fc34e19836f 100644 (file)
--- a/quantum/tests/unit/test_linux_dhcp.py
+++ b/quantum/tests/unit/test_linux_dhcp.py
@@ -135,6 +135,11 @@ class FakeV4NoGatewayNetwork:
 
 
 class TestDhcpBase(base.BaseTestCase):
+    def test_existing_dhcp_networks_abstract_error(self):
+        self.assertRaises(NotImplementedError,
+                          dhcp.DhcpBase.existing_dhcp_networks,
+                          None, None)
+
     def test_base_abc_error(self):
         self.assertRaises(TypeError, dhcp.DhcpBase, None)
 
@@ -630,3 +635,40 @@ tag:tag1,option:classless-static-route,%s,%s""".lstrip() % (fake_v6,
 
     def test_lease_relay_script_add_socket_missing(self):
         self._test_lease_relay_script_helper('add', 120, False)
+
+    def test_remove_config_files(self):
+        net = FakeV4Network()
+        path = '/opt/data/quantum/dhcp'
+        self.conf.dhcp_confs = path
+
+        with mock.patch('shutil.rmtree') as rmtree:
+            lp = LocalChild(self.conf, net)
+            lp._remove_config_files()
+
+            rmtree.assert_called_once_with(os.path.join(path, net.id),
+                                           ignore_errors=True)
+
+    def test_existing_dhcp_networks(self):
+        path = '/opt/data/quantum/dhcp'
+        self.conf.dhcp_confs = path
+
+        cases = {
+            # network_uuid --> is_dhcp_alive?
+            'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa': True,
+            'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb': False,
+            'not_uuid_like_name': True
+        }
+
+        def active_fake(self, instance, cls):
+            return cases[instance.network.id]
+
+        with mock.patch('os.listdir') as mock_listdir:
+            with mock.patch.object(dhcp.Dnsmasq, 'active') as mock_active:
+                mock_active.__get__ = active_fake
+                mock_listdir.return_value = cases.keys()
+
+                result = dhcp.Dnsmasq.existing_dhcp_networks(self.conf, 'sudo')
+
+                mock_listdir.assert_called_once_with(path)
+                self.assertEquals(['aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'],
+                                  result)