From bb43a075f79fde8e57019671aab62e14d0bb2741 Mon Sep 17 00:00:00 2001 From: Roman Podolyaka Date: Tue, 5 Mar 2013 18:53:51 +0200 Subject: [PATCH] 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 --- quantum/agent/dhcp_agent.py | 22 ++++++++++++++ quantum/agent/linux/dhcp.py | 31 ++++++++++++++++++++ quantum/tests/unit/test_dhcp_agent.py | 29 ++++++++++++++++++ quantum/tests/unit/test_linux_dhcp.py | 42 +++++++++++++++++++++++++++ 4 files changed, 124 insertions(+) diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 0397ae198..1de865337 100644 --- 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 38e7fd273..8b6f62d5a 100644 --- 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 9ac8dea75..d2b0dbe86 100644 --- 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 a3ab7fbce..dabd1dcf8 100644 --- 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) -- 2.45.2