From 332204fec7e96339947aa662b25adc9925609c4f Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Tue, 20 Jan 2015 15:18:36 +0100 Subject: [PATCH] dhcp: move dnsmasq version check to sanity_check We should avoid checking version numbers in runtime. In that way, we may break some existing setups by minimal version bumps that are often not critical for operation. One example is a recent version bump to support IPv6 DHCP stateful address assignment mode. Even though old dnsmasq version made this particular mode to fail to assign addresses to instances, other IPv6 modes, and, even more importantly, all IPv4 networks continued to operate with no issues. So let's move the fatal check from DHCP agent into sanity_check tool to avoid potential breakages on neutron update. In ideal world, we would make the check smarter. Since current version cap is due to missing hwaddr matching for IPv6 clients for old dnsmasq versions, we could preconfigure and start up dnsmasq server in a namespace, and request a IPv6 lease from it. That would require a DHCP IPv6 client installed though, and I'm not sure we can always expect it to be present, so leaving it as-is for now. Since DHCP drivers are pluggable, we cannot drop check_version method from DhcpBase to support other drivers that may live in the wild. Note: we could mark the method as deprecated if we really want to get rid of it. Closes-Bug: #1412818 Change-Id: I7a75cccf3880d3b18005ae0def5a17dfd8a00182 --- neutron/agent/linux/dhcp.py | 19 +----------- neutron/cmd/sanity/checks.py | 24 +++++++++++++++ neutron/cmd/sanity_check.py | 19 ++++++++++++ .../tests/functional/sanity/test_sanity.py | 3 ++ neutron/tests/unit/test_dhcp_agent.py | 5 +--- neutron/tests/unit/test_linux_dhcp.py | 30 ------------------- 6 files changed, 48 insertions(+), 52 deletions(-) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index e41a1de5a..6d186f222 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -287,27 +287,10 @@ class Dnsmasq(DhcpLocalProcess): NEUTRON_NETWORK_ID_KEY = 'NEUTRON_NETWORK_ID' NEUTRON_RELAY_SOCKET_PATH_KEY = 'NEUTRON_RELAY_SOCKET_PATH' - MINIMUM_VERSION = 2.67 @classmethod def check_version(cls): - ver = 0 - try: - cmd = ['dnsmasq', '--version'] - out = utils.execute(cmd) - m = re.search(r"version (\d+\.\d+)", out) - ver = float(m.group(1)) if m else 0 - if ver < cls.MINIMUM_VERSION: - LOG.error(_LE('FAILED VERSION REQUIREMENT FOR DNSMASQ. ' - 'Please ensure that its version is %s ' - 'or above!'), cls.MINIMUM_VERSION) - raise SystemExit(1) - except (OSError, RuntimeError, IndexError, ValueError): - LOG.error(_LE('Unable to determine dnsmasq version. ' - 'Please ensure that its version is %s ' - 'or above!'), cls.MINIMUM_VERSION) - raise SystemExit(1) - return ver + pass @classmethod def existing_dhcp_networks(cls, conf, root_helper): diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index c0c971b25..9c77aa0b0 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import re + import netaddr from neutron.agent.linux import ip_lib @@ -29,6 +31,9 @@ from neutron.plugins.openvswitch.common import constants as ovs_const LOG = logging.getLogger(__name__) +MINIMUM_DNSMASQ_VERSION = 2.67 + + def ovs_vxlan_supported(root_helper, from_ip='192.0.2.1', to_ip='192.0.2.2'): name = "vxlantest-" + utils.get_random_string(6) with ovs_lib.OVSBridge(name, root_helper) as br: @@ -130,3 +135,22 @@ def netns_read_requires_helper(root_helper): finally: ipw.netns.delete(nsname) return not exists + + +def get_minimal_dnsmasq_version_supported(): + return MINIMUM_DNSMASQ_VERSION + + +def dnsmasq_version_supported(): + try: + cmd = ['dnsmasq', '--version'] + out = agent_utils.execute(cmd) + m = re.search(r"version (\d+\.\d+)", out) + ver = float(m.group(1)) if m else 0 + if ver < MINIMUM_DNSMASQ_VERSION: + return False + except (OSError, RuntimeError, IndexError, ValueError) as e: + LOG.debug("Exception while checking minimal dnsmasq version. " + "Exception: %s", e) + return False + return True diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 2686f1a89..fed590376 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -15,6 +15,7 @@ import sys +from neutron.agent import dhcp_agent from neutron.cmd.sanity import checks from neutron.common import config from neutron.i18n import _LE, _LW @@ -29,6 +30,7 @@ cfg.CONF.import_group('VXLAN', 'neutron.plugins.linuxbridge.common.config') cfg.CONF.import_group('ml2', 'neutron.plugins.ml2.config') cfg.CONF.import_group('ml2_sriov', 'neutron.plugins.ml2.drivers.mech_sriov.mech_driver') +dhcp_agent.register_options() class BoolOptCallback(cfg.BoolOpt): @@ -88,6 +90,19 @@ def check_read_netns(): return result +# NOTE(ihrachyshka): since the minimal version is currently capped due to +# missing hwaddr matching in dnsmasq < 2.67, a better version of the check +# would actually start dnsmasq server and issue a DHCP request using a IPv6 +# DHCP client. +def check_dnsmasq_version(): + result = checks.dnsmasq_version_supported() + if not result: + LOG.error(_LE('The installed version of dnsmasq is too old. ' + 'Please update to at least version %s.'), + checks.get_minimal_dnsmasq_version_supported()) + return result + + def check_nova_notify(): result = checks.nova_notify_supported() if not result: @@ -133,6 +148,8 @@ OPTS = [ help=_('Check for VF management support')), BoolOptCallback('read_netns', check_read_netns, help=_('Check netns permission settings')), + BoolOptCallback('dnsmasq_version', check_dnsmasq_version, + help=_('Check minimal dnsmasq version')), ] @@ -160,6 +177,8 @@ def enable_tests_from_config(): cfg.CONF.set_override('vf_management', True) if not cfg.CONF.AGENT.use_helper_for_ns_read: cfg.CONF.set_override('read_netns', True) + if cfg.CONF.dhcp_driver == 'neutron.agent.linux.dhcp.Dnsmasq': + cfg.CONF.set_override('dnsmasq_version', True) def all_tests_passed(): diff --git a/neutron/tests/functional/sanity/test_sanity.py b/neutron/tests/functional/sanity/test_sanity.py index 87703114e..c34162e10 100644 --- a/neutron/tests/functional/sanity/test_sanity.py +++ b/neutron/tests/functional/sanity/test_sanity.py @@ -32,6 +32,9 @@ class SanityTestCase(base.BaseTestCase): def test_nova_notify_runs(self): checks.nova_notify_supported() + def test_dnsmasq_version(self): + checks.dnsmasq_version_supported() + class SanityTestCaseRoot(functional_base.BaseSudoTestCase): """Sanity checks that require root access. diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py index 367bf929b..64475fc55 100644 --- a/neutron/tests/unit/test_dhcp_agent.py +++ b/neutron/tests/unit/test_dhcp_agent.py @@ -539,10 +539,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.mock_init_p = mock.patch('neutron.agent.dhcp.agent.' 'DhcpAgent._populate_networks_cache') self.mock_init = self.mock_init_p.start() - with mock.patch.object(dhcp.Dnsmasq, - 'check_version') as check_v: - check_v.return_value = dhcp.Dnsmasq.MINIMUM_VERSION - self.dhcp = dhcp_agent.DhcpAgent(HOSTNAME) + self.dhcp = dhcp_agent.DhcpAgent(HOSTNAME) self.call_driver_p = mock.patch.object(self.dhcp, 'call_driver') self.call_driver = self.call_driver_p.start() self.schedule_resync_p = mock.patch.object(self.dhcp, diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 0b3797d3e..4b55d9798 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -18,7 +18,6 @@ import os import mock import netaddr from oslo.config import cfg -import testtools from neutron.agent.common import config from neutron.agent.dhcp import config as dhcp_config @@ -697,7 +696,6 @@ class TestDnsmasq(TestBase): def _get_dnsmasq(self, network, process_monitor=None): process_monitor = process_monitor or mock.Mock() return dhcp.Dnsmasq(self.conf, network, - version=dhcp.Dnsmasq.MINIMUM_VERSION, process_monitor=process_monitor) def _test_spawn(self, extra_options, network=FakeDualNetwork(), @@ -1251,34 +1249,6 @@ class TestDnsmasq(TestBase): 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'], sorted(result)) - def _check_version(self, cmd_out, expected_value): - with mock.patch('neutron.agent.linux.utils.execute') as cmd: - cmd.return_value = cmd_out - result = dhcp.Dnsmasq.check_version() - self.assertEqual(result, expected_value) - - def test_check_find_version(self): - # Dnsmasq output currently gives the version number before the - # copyright year, but just in case ... - self._check_version('Copyright 2000-2014. Dnsmasq version 3.33 ...', - float(3.33)) - - def test_check_minimum_version(self): - self._check_version('Dnsmasq version 2.67 Copyright (c)...', - dhcp.Dnsmasq.MINIMUM_VERSION) - - def test_check_future_version(self): - self._check_version('Dnsmasq version 2.99 Copyright (c)...', - float(2.99)) - - def test_check_fail_version(self): - with testtools.ExpectedException(SystemExit): - self._check_version('Dnsmasq version 2.62 Copyright (c)...', 0) - - def test_check_version_failed_cmd_execution(self): - with testtools.ExpectedException(SystemExit): - self._check_version('Error while executing command', 0) - 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,' -- 2.45.2