Allow concurrect updating of dnsmasq configuration
This allows multiple instances of inspector to try updating dnsmasq configuration simultaneously. The goal is to be able to (test) run an HA inspector on a single node. A new config option `dnsmasq_pxe_filter.purge_dhcp_hostsdir` is introduced to be able to disable purging of the dhcp hosts directory in case multiple inspector instances are expected to run on the same node. Change-Id: I2f7b8d3172f375cf65e759c9b881fcf41649c2f0 Closes-Bug: #1722267
This commit is contained in:
dparalen
parent
f7a3c26aec
commit
4ff0213e87
@ -350,6 +350,13 @@
|
||||
# is expected to be in exclusive control of the driver. (string value)
|
||||
#dhcp_hostsdir = /var/lib/ironic-inspector/dhcp-hostsdir
|
||||
|
||||
# Purge the hostsdir upon driver initialization. Setting to false
|
||||
# makes sense only for deployment of multiple (uncontainerized)
|
||||
# inspector instances on a single node. In this case, the Operator is
|
||||
# responsible for setting up a custom cleaning facility. (boolean
|
||||
# value)
|
||||
#purge_dhcp_hostsdir = true
|
||||
|
||||
# A (shell) command line to start the dnsmasq service upon filter
|
||||
# initialization. Default: don't start. (string value)
|
||||
#dnsmasq_start_command =
|
||||
|
||||
@ -215,6 +215,13 @@ DNSMASQ_PXE_FILTER_OPTS = [
|
||||
help=_('The MAC address cache directory, exposed to dnsmasq.'
|
||||
'This directory is expected to be in exclusive control '
|
||||
'of the driver.')),
|
||||
cfg.BoolOpt('purge_dhcp_hostsdir', default=True,
|
||||
help=_('Purge the hostsdir upon driver initialization. '
|
||||
'Setting to false makes sense only for deployment of '
|
||||
'multiple (uncontainerized) inspector instances on a '
|
||||
'single node. In this case, the Operator is '
|
||||
'responsible for setting up a custom cleaning '
|
||||
'facility.')),
|
||||
cfg.StrOpt('dnsmasq_start_command', default='',
|
||||
help=_('A (shell) command line to start the dnsmasq service '
|
||||
'upon filter initialization. Default: don\'t start.')),
|
||||
|
||||
@ -20,7 +20,9 @@
|
||||
# http://www.thekelleys.org.uk/dnsmasq/docs/dnsmasq-man.html
|
||||
|
||||
|
||||
import fcntl
|
||||
import os
|
||||
import time
|
||||
|
||||
from oslo_concurrency import processutils
|
||||
from oslo_config import cfg
|
||||
@ -34,6 +36,9 @@ from ironic_inspector.pxe_filter import base as pxe_filter
|
||||
CONF = cfg.CONF
|
||||
LOG = log.getLogger(__name__)
|
||||
|
||||
_EXCLUSIVE_WRITE_ATTEMPTS = 10
|
||||
_EXCLUSIVE_WRITE_ATTEMPTS_DELAY = 0.01
|
||||
|
||||
_ROOTWRAP_COMMAND = 'sudo ironic-inspector-rootwrap {rootwrap_config!s}'
|
||||
_MACBL_LEN = len('ff:ff:ff:ff:ff:ff,ignore\n')
|
||||
|
||||
@ -116,6 +121,10 @@ def _purge_dhcp_hostsdir():
|
||||
:returns: None.
|
||||
"""
|
||||
dhcp_hostsdir = CONF.dnsmasq_pxe_filter.dhcp_hostsdir
|
||||
if not CONF.dnsmasq_pxe_filter.purge_dhcp_hostsdir:
|
||||
LOG.debug('Not purging %s; disabled in configuration.', dhcp_hostsdir)
|
||||
return
|
||||
|
||||
LOG.debug('Purging %s', dhcp_hostsdir)
|
||||
for mac in os.listdir(dhcp_hostsdir):
|
||||
path = os.path.join(dhcp_hostsdir, mac)
|
||||
@ -137,6 +146,40 @@ def _get_blacklist():
|
||||
_MACBL_LEN)
|
||||
|
||||
|
||||
def _exclusive_write_or_pass(path, buf):
|
||||
"""Write exclusively or pass if path locked.
|
||||
|
||||
The intention is to be able to run multiple instances of the filter on the
|
||||
same node in multiple inspector processes.
|
||||
|
||||
:param path: where to write to
|
||||
:param buf: the content to write
|
||||
:raises: FileNotFoundError, IOError
|
||||
:returns: True if the write was successful.
|
||||
"""
|
||||
# NOTE(milan) line-buffering enforced to ensure dnsmasq record update
|
||||
# through inotify, which reacts on f.close()
|
||||
attempts = _EXCLUSIVE_WRITE_ATTEMPTS
|
||||
with open(path, 'w', 1) as f:
|
||||
while attempts:
|
||||
try:
|
||||
fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
return bool(f.write(buf))
|
||||
except IOError as e:
|
||||
if e.errno == os.errno.EWOULDBLOCK:
|
||||
LOG.debug('%s locked; will try again (later)', path)
|
||||
attempts -= 1
|
||||
time.sleep(_EXCLUSIVE_WRITE_ATTEMPTS_DELAY)
|
||||
continue
|
||||
raise
|
||||
finally:
|
||||
fcntl.flock(f, fcntl.LOCK_UN)
|
||||
LOG.debug('Failed to write the exclusively-locked path: %(path)s for '
|
||||
'%(attempts)s times', {'attempts': _EXCLUSIVE_WRITE_ATTEMPTS,
|
||||
'path': path})
|
||||
return 0
|
||||
|
||||
|
||||
def _blacklist_mac(mac):
|
||||
"""Creates a dhcp_hostsdir ignore record for the MAC.
|
||||
|
||||
@ -145,11 +188,11 @@ def _blacklist_mac(mac):
|
||||
:returns: None.
|
||||
"""
|
||||
path = os.path.join(CONF.dnsmasq_pxe_filter.dhcp_hostsdir, mac)
|
||||
# NOTE(milan) line-buffering enforced to ensure dnsmasq record update
|
||||
# through inotify, which reacts on f.close()
|
||||
with open(path, 'w', 1) as f:
|
||||
f.write('%s,ignore\n' % mac)
|
||||
LOG.debug('Blacklisted %s', mac)
|
||||
if _exclusive_write_or_pass(path, '%s,ignore\n' % mac):
|
||||
LOG.debug('Blacklisted %s', mac)
|
||||
else:
|
||||
LOG.warning('Failed to blacklist %s; retrying next periodic sync '
|
||||
'time', mac)
|
||||
|
||||
|
||||
def _whitelist_mac(mac):
|
||||
@ -160,10 +203,12 @@ def _whitelist_mac(mac):
|
||||
:returns: None.
|
||||
"""
|
||||
path = os.path.join(CONF.dnsmasq_pxe_filter.dhcp_hostsdir, mac)
|
||||
with open(path, 'w', 1) as f:
|
||||
# remove the ,ignore directive
|
||||
f.write('%s\n' % mac)
|
||||
LOG.debug('Whitelisted %s', mac)
|
||||
# remove the ,ignore directive
|
||||
if _exclusive_write_or_pass(path, '%s\n' % mac):
|
||||
LOG.debug('Whitelisted %s', mac)
|
||||
else:
|
||||
LOG.warning('Failed to whitelist %s; retrying next periodic sync '
|
||||
'time', mac)
|
||||
|
||||
|
||||
def _execute(cmd=None, ignore_errors=False):
|
||||
|
||||
@ -86,6 +86,98 @@ class TestDnsmasqDriverAPI(DnsmasqTestBase):
|
||||
self.stop_command, ignore_errors=True)
|
||||
|
||||
|
||||
class TestExclusiveWriteOrPass(test_base.BaseTest):
|
||||
def setUp(self):
|
||||
super(TestExclusiveWriteOrPass, self).setUp()
|
||||
self.mock_open = self.useFixture(fixtures.MockPatchObject(
|
||||
six.moves.builtins, 'open', new=mock.mock_open())).mock
|
||||
self.mock_fd = self.mock_open.return_value
|
||||
self.mock_fcntl = self.useFixture(fixtures.MockPatchObject(
|
||||
dnsmasq.fcntl, 'flock', autospec=True)).mock
|
||||
self.path = '/foo/bar/baz'
|
||||
self.buf = 'spam'
|
||||
self.fcntl_lock_call = mock.call(
|
||||
self.mock_fd, dnsmasq.fcntl.LOCK_EX | dnsmasq.fcntl.LOCK_NB)
|
||||
self.fcntl_unlock_call = mock.call(self.mock_fd, dnsmasq.fcntl.LOCK_UN)
|
||||
self.mock_log = self.useFixture(fixtures.MockPatchObject(
|
||||
dnsmasq.LOG, 'debug')).mock
|
||||
self.mock_sleep = self.useFixture(fixtures.MockPatchObject(
|
||||
dnsmasq.time, 'sleep')).mock
|
||||
|
||||
def test_write(self):
|
||||
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
|
||||
self.assertIs(bool(self.mock_fd.write.return_value), wrote)
|
||||
self.mock_open.assert_called_once_with(self.path, 'w', 1)
|
||||
self.mock_fcntl.assert_has_calls(
|
||||
[self.fcntl_lock_call, self.fcntl_unlock_call])
|
||||
self.mock_fd.write.assert_called_once_with(self.buf)
|
||||
self.mock_log.assert_not_called()
|
||||
|
||||
def test_write_would_block(self):
|
||||
err = IOError('Oops!')
|
||||
err.errno = os.errno.EWOULDBLOCK
|
||||
# lock/unlock paired calls
|
||||
self.mock_fcntl.side_effect = [
|
||||
# first try
|
||||
err, None,
|
||||
# second try
|
||||
None, None]
|
||||
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
|
||||
|
||||
self.assertIs(bool(self.mock_fd.write.return_value), wrote)
|
||||
self.mock_open.assert_called_once_with(self.path, 'w', 1)
|
||||
self.mock_fcntl.assert_has_calls(
|
||||
[self.fcntl_lock_call, self.fcntl_unlock_call],
|
||||
[self.fcntl_lock_call, self.fcntl_unlock_call])
|
||||
self.mock_fd.write.assert_called_once_with(self.buf)
|
||||
self.mock_log.assert_called_once_with(
|
||||
'%s locked; will try again (later)', self.path)
|
||||
self.mock_sleep.assert_called_once_with(
|
||||
dnsmasq._EXCLUSIVE_WRITE_ATTEMPTS_DELAY)
|
||||
|
||||
def test_write_would_block_too_many_times(self):
|
||||
self.useFixture(fixtures.MonkeyPatch(
|
||||
'ironic_inspector.pxe_filter.dnsmasq._EXCLUSIVE_WRITE_ATTEMPTS',
|
||||
1))
|
||||
err = IOError('Oops!')
|
||||
err.errno = os.errno.EWOULDBLOCK
|
||||
self.mock_fcntl.side_effect = [err, None]
|
||||
|
||||
wrote = dnsmasq._exclusive_write_or_pass(self.path, self.buf)
|
||||
self.assertEqual(0, wrote)
|
||||
self.mock_open.assert_called_once_with(self.path, 'w', 1)
|
||||
self.mock_fcntl.assert_has_calls(
|
||||
[self.fcntl_lock_call, self.fcntl_unlock_call])
|
||||
self.mock_fd.write.assert_not_called()
|
||||
retry_log_call = mock.call('%s locked; will try again (later)',
|
||||
self.path)
|
||||
failed_log_call = mock.call(
|
||||
'Failed to write the exclusively-locked path: %(path)s for '
|
||||
'%(attempts)s times', {
|
||||
'attempts': dnsmasq._EXCLUSIVE_WRITE_ATTEMPTS,
|
||||
'path': self.path
|
||||
})
|
||||
self.mock_log.assert_has_calls([retry_log_call, failed_log_call])
|
||||
self.mock_sleep.assert_called_once_with(
|
||||
dnsmasq._EXCLUSIVE_WRITE_ATTEMPTS_DELAY)
|
||||
|
||||
def test_write_custom_ioerror(self):
|
||||
|
||||
err = IOError('Oops!')
|
||||
err.errno = os.errno.EBADF
|
||||
self.mock_fcntl.side_effect = [err, None]
|
||||
|
||||
self.assertRaisesRegex(
|
||||
IOError, 'Oops!', dnsmasq._exclusive_write_or_pass, self.path,
|
||||
self.buf)
|
||||
|
||||
self.mock_open.assert_called_once_with(self.path, 'w', 1)
|
||||
self.mock_fcntl.assert_has_calls(
|
||||
[self.fcntl_lock_call, self.fcntl_unlock_call])
|
||||
self.mock_fd.write.assert_not_called()
|
||||
self.mock_log.assert_not_called()
|
||||
|
||||
|
||||
class TestMACHandlers(test_base.BaseTest):
|
||||
def setUp(self):
|
||||
super(TestMACHandlers, self).setUp()
|
||||
@ -102,26 +194,22 @@ class TestMACHandlers(test_base.BaseTest):
|
||||
self.mock_join = self.useFixture(
|
||||
fixtures.MockPatchObject(os.path, 'join')).mock
|
||||
self.mock_join.return_value = "%s/%s" % (self.dhcp_hostsdir, self.mac)
|
||||
self.mock__exclusive_write_or_pass = self.useFixture(
|
||||
fixtures.MockPatchObject(dnsmasq, '_exclusive_write_or_pass')).mock
|
||||
|
||||
def test__whitelist_mac(self):
|
||||
with mock.patch.object(six.moves.builtins, 'open',
|
||||
new=mock.mock_open()) as mock_open:
|
||||
dnsmasq._whitelist_mac(self.mac)
|
||||
dnsmasq._whitelist_mac(self.mac)
|
||||
|
||||
mock_fd = mock_open.return_value
|
||||
self.mock_join.assert_called_once_with(self.dhcp_hostsdir, self.mac)
|
||||
mock_open.assert_called_once_with(self.mock_join.return_value, 'w', 1)
|
||||
mock_fd.write.assert_called_once_with('%s\n' % self.mac)
|
||||
self.mock__exclusive_write_or_pass.assert_called_once_with(
|
||||
self.mock_join.return_value, '%s\n' % self.mac)
|
||||
|
||||
def test__blacklist_mac(self):
|
||||
with mock.patch.object(six.moves.builtins, 'open',
|
||||
new=mock.mock_open()) as mock_open:
|
||||
dnsmasq._blacklist_mac(self.mac)
|
||||
dnsmasq._blacklist_mac(self.mac)
|
||||
|
||||
mock_fd = mock_open.return_value
|
||||
self.mock_join.assert_called_once_with(self.dhcp_hostsdir, self.mac)
|
||||
mock_open.assert_called_once_with(self.mock_join.return_value, 'w', 1)
|
||||
mock_fd.write.assert_called_once_with('%s,ignore\n' % self.mac)
|
||||
self.mock__exclusive_write_or_pass.assert_called_once_with(
|
||||
self.mock_join.return_value, '%s,ignore\n' % self.mac)
|
||||
|
||||
def test__get_blacklist(self):
|
||||
self.mock_listdir.return_value = [self.mac]
|
||||
@ -152,6 +240,14 @@ class TestMACHandlers(test_base.BaseTest):
|
||||
self.mock_remove.assert_called_once_with('%s/%s' % (self.dhcp_hostsdir,
|
||||
self.mac))
|
||||
|
||||
def test_disabled__purge_dhcp_hostsdir(self):
|
||||
CONF.set_override('purge_dhcp_hostsdir', False, 'dnsmasq_pxe_filter')
|
||||
|
||||
dnsmasq._purge_dhcp_hostsdir()
|
||||
self.mock_listdir.assert_not_called()
|
||||
self.mock_join.assert_not_called()
|
||||
self.mock_remove.assert_not_called()
|
||||
|
||||
|
||||
class TestSync(DnsmasqTestBase):
|
||||
def setUp(self):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user