From 38211ae67cb76ade85b08c028b6e88bfc867afc9 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Mon, 20 Apr 2015 17:06:38 +0200 Subject: [PATCH] tests: confirm that _output_hosts_file does not log too often I3ad7864eeb2f959549ed356a1e34fa18804395cc didn't include any regression unit tests to validate that the method won't ever log too often again, reintroducing performance drop in later patches. It didn't play well with stable backports of the fix, where context was lost when doing the backport, that left the bug unfixed in stable/juno even though the patch was merged there [1]. The patch adds an explicit note in the code that suggests not to add new log messages inside the loop to avoid regression, and a unit test was added to capture it. Once the test is merged in master, it will be proposed for stable/juno inclusion, with additional changes that would fix the regression again. Related-Bug: #1414218 Change-Id: I5d43021932d6a994638c348eda277dd8337cf041 (cherry picked from commit 3b74095a935f6d2027e6bf04cc4aa21f8a1b46f2) --- neutron/agent/linux/dhcp.py | 2 ++ neutron/tests/unit/agent/linux/test_dhcp.py | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 55509cb84b2..f594b775c0f 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -483,6 +483,8 @@ class Dnsmasq(DhcpLocalProcess): LOG.debug('Building host file: %s', filename) dhcp_enabled_subnet_ids = [s.id for s in self.network.subnets if s.enable_dhcp] + # NOTE(ihrachyshka): the loop should not log anything inside it, to + # avoid potential performance drop when lots of hosts are dumped for (port, alloc, hostname, name) in self._iter_hosts(): if not alloc: if getattr(port, 'extra_dhcp_opts', False): diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 09a91a7caa5..fca35c1bb8b 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -1403,6 +1403,16 @@ class TestDnsmasq(TestBase): 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'], sorted(result)) + def test__output_hosts_file_log_only_twice(self): + dm = self._get_dnsmasq(FakeDualStackNetworkSingleDHCP()) + with mock.patch.object(dhcp.LOG, 'process') as process: + process.return_value = ('fake_message', {}) + dm._output_hosts_file() + # The method logs twice, at the start of and the end. There should be + # no other logs, no matter how many hosts there are to dump in the + # file. + self.assertEqual(2, process.call_count) + def test_only_populates_dhcp_enabled_subnets(self): exp_host_name = '/dhcp/eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee/host' exp_host_data = ('00:00:80:aa:bb:cc,host-192-168-0-2.openstacklocal,'