openstack/neutron
Code Issues Proposed changes

dhcp: move dnsmasq version check to sanity_check

Browse Source 
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
This commit is contained in:
Ihar Hrachyshka 2015-01-20 15:18:36 +01:00
parent 51fcac0389
commit 332204fec7
6 changed files with 48 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
neutron/agent/linux/dhcp.py
View File

@ -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):

24
neutron/cmd/sanity/checks.py
View File

@ -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

19
neutron/cmd/sanity_check.py
View File

@ -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():

3
neutron/tests/functional/sanity/test_sanity.py
View File

@ -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.

5
neutron/tests/unit/test_dhcp_agent.py
View File

@ -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,

30
neutron/tests/unit/test_linux_dhcp.py
View File

@ -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,'