Skip to content

Commit

Permalink
Do not require sudo/rootwrap to check for dnsmasq version
Browse files Browse the repository at this point in the history
The dnsmasq version check does not need root privileges
to run as with low privs it works just fine. As a side
effect, the use of the rootwrapper was causing unit tests
to hung because the execute call in check_version was not
being stubbed out. Weirdly enough this wasn't caught in
a previous Gerrit run; there must be a passwordless sudo
lurking around somewhere in the Gerrit infrastructure.

Fixes bug #1178800

Change-Id: I4d0bd218768eec2439d3907587682ff35884a262
  • Loading branch information
armando-migliaccio committed May 10, 2013
1 parent 21575de commit 91b56e4
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 15 deletions.
2 changes: 0 additions & 2 deletions etc/quantum/rootwrap.d/dhcp.filters
Expand Up @@ -11,9 +11,7 @@
# dhcp-agent
ip_exec_dnsmasq: DnsmasqNetnsFilter, /sbin/ip, root
dnsmasq: DnsmasqFilter, /sbin/dnsmasq, root
dnsmasq_ver: DnsmasqVersionFilter, /sbin/dnsmasq, root
dnsmasq_usr: DnsmasqFilter, /usr/sbin/dnsmasq, root
dnsmasq_usr_ver: DnsmasqVersionFilter, /usr/sbin/dnsmasq, root
# dhcp-agent uses kill as well, that's handled by the generic KillFilter
# it looks like these are the only signals needed, per
# quantum/agent/linux/dhcp.py
Expand Down
2 changes: 1 addition & 1 deletion quantum/agent/dhcp_agent.py
Expand Up @@ -80,7 +80,7 @@ def __init__(self, host=None):
self.device_manager = DeviceManager(self.conf, self.plugin_rpc)
self.lease_relay = DhcpLeaseRelay(self.update_lease)

self.dhcp_driver_cls.check_version(self.root_helper)
self.dhcp_driver_cls.check_version()
self._populate_networks_cache()

def _populate_networks_cache(self):
Expand Down
8 changes: 4 additions & 4 deletions quantum/agent/linux/dhcp.py
Expand Up @@ -101,7 +101,7 @@ def existing_dhcp_networks(cls, conf, root_helper):
raise NotImplementedError

@classmethod
def check_version(cls, root_helper):
def check_version(cls):
"""Execute version checks on DHCP server."""

raise NotImplementedError
Expand Down Expand Up @@ -224,19 +224,19 @@ class Dnsmasq(DhcpLocalProcess):
MINIMUM_VERSION = 2.59

@classmethod
def check_version(cls, root_helper):
def check_version(cls):
is_valid_version = None
try:
cmd = ['dnsmasq', '--version']
out = utils.execute(cmd, root_helper)
out = utils.execute(cmd)
ver = re.findall("\d+.\d+", out)[0]
is_valid_version = float(ver) >= cls.MINIMUM_VERSION
if not is_valid_version:
LOG.warning(_('FAILED VERSION REQUIREMENT FOR DNSMASQ. '
'DHCP AGENT MAY NOT RUN CORRECTLY! '
'Please ensure that its version is %s '
'or above!'), cls.MINIMUM_VERSION)
except (RuntimeError, IndexError, ValueError):
except (OSError, RuntimeError, IndexError, ValueError):
LOG.warning(_('Unable to determine dnsmasq version. '
'Please ensure that its version is %s '
'or above!'), cls.MINIMUM_VERSION)
Expand Down
6 changes: 0 additions & 6 deletions quantum/rootwrap/filters.py
Expand Up @@ -171,12 +171,6 @@ def get_environment(self, userargs):
return env


class DnsmasqVersionFilter(CommandFilter):
"""Specific filter to check dnsmasq version."""
def match(self, userargs):
return userargs[0] == "dnsmasq" and userargs[1] == "--version"


class DnsmasqNetnsFilter(DnsmasqFilter):
"""Specific filter for the dnsmasq call (which includes env)."""

Expand Down
4 changes: 2 additions & 2 deletions quantum/tests/unit/test_linux_dhcp.py
Expand Up @@ -142,7 +142,7 @@ def test_existing_dhcp_networks_abstract_error(self):

def test_check_version_abstract_error(self):
self.assertRaises(NotImplementedError,
dhcp.DhcpBase.check_version, None)
dhcp.DhcpBase.check_version)

def test_base_abc_error(self):
self.assertRaises(TypeError, dhcp.DhcpBase, None)
Expand Down Expand Up @@ -720,7 +720,7 @@ def active_fake(self, instance, cls):
def _check_version(self, cmd_out, expected_value):
with mock.patch('quantum.agent.linux.utils.execute') as cmd:
cmd.return_value = cmd_out
result = dhcp.Dnsmasq.check_version('sudo')
result = dhcp.Dnsmasq.check_version()
self.assertEqual(result, expected_value)

def test_check_minimum_version(self):
Expand Down

0 comments on commit 91b56e4

Please sign in to comment.