summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--api-ref/source/samples/driver-property-response.json2
-rw-r--r--ironic/conductor/manager.py5
-rw-r--r--ironic/conductor/task_manager.py17
-rw-r--r--ironic/conf/pxe.py4
-rw-r--r--ironic/drivers/modules/ipmitool.py21
-rw-r--r--ironic/drivers/modules/iscsi_deploy.py3
-rw-r--r--ironic/drivers/modules/pxe.py3
-rw-r--r--ironic/tests/unit/conductor/test_manager.py12
-rw-r--r--ironic/tests/unit/conductor/test_task_manager.py18
-rw-r--r--ironic/tests/unit/drivers/modules/test_ipmitool.py2
-rw-r--r--ironic/tests/unit/drivers/modules/test_iscsi_deploy.py26
-rw-r--r--ironic/tests/unit/drivers/modules/test_pxe.py26
-rw-r--r--releasenotes/notes/broken-driver-update-fc5303340080ef04.yaml5
-rw-r--r--releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml6
-rw-r--r--releasenotes/notes/fix-tftp-master-path-config-77face94f5db9af7.yaml7
-rw-r--r--releasenotes/notes/story-2004266-4725d327900850bf.yaml4
16 files changed, 139 insertions, 22 deletions
diff --git a/api-ref/source/samples/driver-property-response.json b/api-ref/source/samples/driver-property-response.json
index 0cab58c10..ca946852b 100644
--- a/api-ref/source/samples/driver-property-response.json
+++ b/api-ref/source/samples/driver-property-response.json
@@ -7,7 +7,7 @@
"image_no_proxy": "A comma-separated list of host names, IP addresses and domain names (with optional :port) that will be excluded from proxying. To denote a domain name, use a dot to prefix the domain name. This value will be ignored if ``image_http_proxy`` and ``image_https_proxy`` are not specified. Optional.",
"ipmi_address": "IP address or hostname of the node. Required.",
"ipmi_bridging": "bridging_type; default is \"no\". One of \"single\", \"dual\", \"no\". Optional.",
- "ipmi_disable_timeout": "By default ironic will send a raw IPMI command to disable the 60 second timeout for booting. Setting this option to False will NOT send that command; default value is True. Optional.",
+ "ipmi_disable_boot_timeout": "By default ironic will send a raw IPMI command to disable the 60 second timeout for booting. Setting this option to False will NOT send that command; default value is True. Optional.",
"ipmi_force_boot_device": "Whether Ironic should specify the boot device to the BMC each time the server is turned on, eg. because the BMC is not capable of remembering the selected boot device across power cycles; default value is False. Optional.",
"ipmi_local_address": "local IPMB address for bridged requests. Used only if ipmi_bridging is set to \"single\" or \"dual\". Optional.",
"ipmi_password": "password. Optional.",
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index a730b362c..59127f6eb 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -199,7 +199,12 @@ class ConductorManager(base_manager.BaseConductorManager):
driver_factory.check_and_update_node_interfaces(node_obj)
+ # NOTE(dtantsur): if we're updating the driver from an invalid value,
+ # loading the old driver may be impossible. Since we only need to
+ # update the node record in the database, skip loading the driver
+ # completely.
with task_manager.acquire(context, node_id, shared=False,
+ load_driver=False,
purpose='node update') as task:
# Prevent instance_uuid overwriting
if ('instance_uuid' in delta and node_obj.instance_uuid
diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py
index b32d33a71..7852909b4 100644
--- a/ironic/conductor/task_manager.py
+++ b/ironic/conductor/task_manager.py
@@ -149,7 +149,8 @@ def require_exclusive_lock(f):
return wrapper
-def acquire(context, node_id, shared=False, purpose='unspecified action'):
+def acquire(context, node_id, shared=False, purpose='unspecified action',
+ **kwargs):
"""Shortcut for acquiring a lock on a Node.
:param context: Request context.
@@ -162,7 +163,8 @@ def acquire(context, node_id, shared=False, purpose='unspecified action'):
"""
# NOTE(lintan): This is a workaround to set the context of periodic tasks.
context.ensure_thread_contain_context()
- return TaskManager(context, node_id, shared=shared, purpose=purpose)
+ return TaskManager(context, node_id, shared=shared, purpose=purpose,
+ **kwargs)
class TaskManager(object):
@@ -174,7 +176,8 @@ class TaskManager(object):
"""
def __init__(self, context, node_id, shared=False,
- purpose='unspecified action'):
+ purpose='unspecified action',
+ load_driver=True):
"""Create a new TaskManager.
Acquire a lock on a node. The lock can be either shared or
@@ -187,6 +190,9 @@ class TaskManager(object):
:param shared: Boolean indicating whether to take a shared or exclusive
lock. Default: False.
:param purpose: human-readable purpose to put to debug logs.
+ :param load_driver: whether to load the ``driver`` object. Set this to
+ False if loading the driver is undesired or
+ impossible.
:raises: DriverNotFound
:raises: InterfaceNotFoundInEntrypoint
:raises: NodeNotFound
@@ -231,7 +237,10 @@ class TaskManager(object):
context, self.node.id)
self.volume_targets = objects.VolumeTarget.list_by_node_id(
context, self.node.id)
- self.driver = driver_factory.build_driver_for_task(self)
+ if load_driver:
+ self.driver = driver_factory.build_driver_for_task(self)
+ else:
+ self.driver = None
except Exception:
with excutils.save_and_reraise_exception():
diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py
index 1a2454327..b0bffd738 100644
--- a/ironic/conf/pxe.py
+++ b/ironic/conf/pxe.py
@@ -36,7 +36,7 @@ opts = [
default='/var/lib/ironic/master_images',
help=_('On the ironic-conductor node, directory where master '
'instance images are stored on disk. '
- 'Setting to <None> disables image caching.')),
+ 'Setting to the empty string disables image caching.')),
cfg.IntOpt('image_cache_size',
default=20480,
help=_('Maximum size (in MiB) of cache for master images, '
@@ -76,7 +76,7 @@ opts = [
default='/tftpboot/master_images',
help=_('On ironic-conductor node, directory where master TFTP '
'images are stored on disk. '
- 'Setting to <None> disables image caching.')),
+ 'Setting to the empty string disables image caching.')),
cfg.IntOpt('dir_permission',
help=_("The permission that will be applied to the TFTP "
"folders upon creation. This should be set to the "
diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index 9325fb386..12f5370fe 100644
--- a/ironic/drivers/modules/ipmitool.py
+++ b/ironic/drivers/modules/ipmitool.py
@@ -98,11 +98,14 @@ OPTIONAL_PROPERTIES = {
"capable of remembering the selected boot "
"device across power cycles; default value "
"is False. Optional."),
- 'ipmi_disable_timeout': _('By default ironic will send a raw IPMI '
- 'command to disable the 60 second timeout '
- 'for booting. Setting this option to '
- 'False will NOT send that command; default '
- 'value is True. Optional.'),
+ 'ipmi_disable_boot_timeout': _('By default ironic will send a raw IPMI '
+ 'command to disable the 60 second timeout '
+ 'for booting. Setting this option to '
+ 'False will NOT send that command on '
+ 'this node. The '
+ '[ipmi]disable_boot_timeout will be '
+ 'used if this option is not set. '
+ 'Optional.'),
}
COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy()
COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES)
@@ -920,8 +923,8 @@ class IPMIManagement(base.ManagementInterface):
# NOTE(tonyb): Some BMCs do not implement Option 0x03, such as OpenBMC
# and will error when we try to set this. Resulting in an abort. If
# the BMC doesn't support this timeout there isn't a need to disable
- # it. Let's use a driver option to signify that
- idt = task.node.driver_info.get('ipmi_disable_timeout', True)
+ # it. Let's use a driver option to signify that.
+ idt = task.node.driver_info.get('ipmi_disable_boot_timeout', True)
if strutils.bool_from_string(idt):
# note(JayF): IPMI spec indicates unless you send these raw bytes
# the boot device setting times out after 60s. Since it's possible
@@ -932,8 +935,8 @@ class IPMIManagement(base.ManagementInterface):
send_raw(task, timeout_disable)
else:
LOG.info('For node %(node_uuid)s, '
- 'driver_info[\'ipmi_disable_timeout\'] is set to False, '
- 'so not sending ipmi boot-timeout-disable',
+ 'driver_info[\'ipmi_disable_boot_timeout\'] is set '
+ 'to False, so not sending ipmi boot-timeout-disable',
{'node_uuid', task.node.uuid})
if task.node.driver_info.get('ipmi_force_boot_device', False):
diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py
index e0121f2af..bddba54e8 100644
--- a/ironic/drivers/modules/iscsi_deploy.py
+++ b/ironic/drivers/modules/iscsi_deploy.py
@@ -48,8 +48,9 @@ DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb')
class InstanceImageCache(image_cache.ImageCache):
def __init__(self):
+ master_path = CONF.pxe.instance_master_path or None
super(self.__class__, self).__init__(
- CONF.pxe.instance_master_path,
+ master_path,
# MiB -> B
cache_size=CONF.pxe.image_cache_size * 1024 * 1024,
# min -> sec
diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py
index e10c492fa..6e06fb1ee 100644
--- a/ironic/drivers/modules/pxe.py
+++ b/ironic/drivers/modules/pxe.py
@@ -387,8 +387,9 @@ def validate_boot_parameters_for_trusted_boot(node):
@image_cache.cleanup(priority=25)
class TFTPImageCache(image_cache.ImageCache):
def __init__(self):
+ master_path = CONF.pxe.tftp_master_path or None
super(TFTPImageCache, self).__init__(
- CONF.pxe.tftp_master_path,
+ master_path,
# MiB -> B
cache_size=CONF.pxe.image_cache_size * 1024 * 1024,
# min -> sec
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 7a37ad22f..af3dfcbb1 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -566,6 +566,16 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
node.refresh()
self.assertEqual(existing_driver, node.driver)
+ def test_update_node_from_invalid_driver(self):
+ existing_driver = 'fake-hardware'
+ wrong_driver = 'wrong-driver'
+ node = obj_utils.create_test_node(self.context, driver=wrong_driver)
+ node.driver = existing_driver
+ result = self.service.update_node(self.context, node)
+ self.assertEqual(existing_driver, result.driver)
+ node.refresh()
+ self.assertEqual(existing_driver, node.driver)
+
UpdateInterfaces = namedtuple('UpdateInterfaces', ('old', 'new'))
# NOTE(dtantsur): "old" interfaces here do not match the defaults, so that
# we can test resetting them.
@@ -6718,7 +6728,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
'deploy_kernel', 'deploy_ramdisk',
'force_persistent_boot_device', 'ipmi_protocol_version',
'ipmi_force_boot_device', 'deploy_forces_oob_reboot',
- 'ipmi_disable_timeout']
+ 'ipmi_disable_boot_timeout']
self._check_driver_properties("ipmi", expected)
def test_driver_properties_snmp(self):
diff --git a/ironic/tests/unit/conductor/test_task_manager.py b/ironic/tests/unit/conductor/test_task_manager.py
index 37255af50..0e67763ba 100644
--- a/ironic/tests/unit/conductor/test_task_manager.py
+++ b/ironic/tests/unit/conductor/test_task_manager.py
@@ -79,6 +79,24 @@ class TaskManagerTestCase(db_base.DbTestCase):
release_mock.assert_called_once_with(self.context, self.host,
self.node.id)
+ def test_no_driver(self, get_voltgt_mock, get_volconn_mock,
+ get_portgroups_mock, get_ports_mock,
+ build_driver_mock, reserve_mock, release_mock,
+ node_get_mock):
+ reserve_mock.return_value = self.node
+ with task_manager.TaskManager(self.context, 'fake-node-id',
+ load_driver=False) as task:
+ self.assertEqual(self.context, task.context)
+ self.assertEqual(self.node, task.node)
+ self.assertEqual(get_ports_mock.return_value, task.ports)
+ self.assertEqual(get_portgroups_mock.return_value, task.portgroups)
+ self.assertEqual(get_volconn_mock.return_value,
+ task.volume_connectors)
+ self.assertEqual(get_voltgt_mock.return_value, task.volume_targets)
+ self.assertIsNone(task.driver)
+ self.assertFalse(task.shared)
+ self.assertFalse(build_driver_mock.called)
+
def test_excl_nested_acquire(
self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock,
get_ports_mock, build_driver_mock,
diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py
index 5d07e6bfd..252d22626 100644
--- a/ironic/tests/unit/drivers/modules/test_ipmitool.py
+++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py
@@ -1960,7 +1960,7 @@ class IPMIToolDriverTestCase(Base):
with task_manager.acquire(self.context, self.node.uuid) as task:
driver_info = task.node.driver_info
- driver_info['ipmi_disable_timeout'] = 'False'
+ driver_info['ipmi_disable_boot_timeout'] = 'False'
task.node.driver_info = driver_info
self.management.set_boot_device(task, boot_devices.PXE)
diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
index 4ecc1c390..efa64d8a8 100644
--- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
+++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
@@ -1035,3 +1035,29 @@ class CleanUpFullFlowTestCase(db_base.DbTestCase):
+ self.files):
self.assertFalse(os.path.exists(path),
'%s is not expected to exist' % path)
+
+
+class InstanceImageCacheTestCase(db_base.DbTestCase):
+ @mock.patch.object(fileutils, 'ensure_tree')
+ def test_with_master_path(self, mock_ensure_tree):
+ self.config(instance_master_path='/fake/path', group='pxe')
+ self.config(image_cache_size=500, group='pxe')
+ self.config(image_cache_ttl=30, group='pxe')
+
+ cache = iscsi_deploy.InstanceImageCache()
+
+ mock_ensure_tree.assert_called_once_with('/fake/path')
+ self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+ self.assertEqual(30 * 60, cache._cache_ttl)
+
+ @mock.patch.object(fileutils, 'ensure_tree')
+ def test_without_master_path(self, mock_ensure_tree):
+ self.config(instance_master_path='', group='pxe')
+ self.config(image_cache_size=500, group='pxe')
+ self.config(image_cache_ttl=30, group='pxe')
+
+ cache = iscsi_deploy.InstanceImageCache()
+
+ mock_ensure_tree.assert_not_called()
+ self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+ self.assertEqual(30 * 60, cache._cache_ttl)
diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py
index fc3de7edd..b139e268d 100644
--- a/ironic/tests/unit/drivers/modules/test_pxe.py
+++ b/ironic/tests/unit/drivers/modules/test_pxe.py
@@ -1746,3 +1746,29 @@ class PXEValidateRescueTestCase(db_base.DbTestCase):
self.assertRaisesRegex(exception.MissingParameterValue,
'Missing.*rescue_kernel',
task.driver.boot.validate_rescue, task)
+
+
+class TFTPImageCacheTestCase(db_base.DbTestCase):
+ @mock.patch.object(fileutils, 'ensure_tree')
+ def test_with_master_path(self, mock_ensure_tree):
+ self.config(tftp_master_path='/fake/path', group='pxe')
+ self.config(image_cache_size=500, group='pxe')
+ self.config(image_cache_ttl=30, group='pxe')
+
+ cache = pxe.TFTPImageCache()
+
+ mock_ensure_tree.assert_called_once_with('/fake/path')
+ self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+ self.assertEqual(30 * 60, cache._cache_ttl)
+
+ @mock.patch.object(fileutils, 'ensure_tree')
+ def test_without_master_path(self, mock_ensure_tree):
+ self.config(tftp_master_path='', group='pxe')
+ self.config(image_cache_size=500, group='pxe')
+ self.config(image_cache_ttl=30, group='pxe')
+
+ cache = pxe.TFTPImageCache()
+
+ mock_ensure_tree.assert_not_called()
+ self.assertEqual(500 * 1024 * 1024, cache._cache_size)
+ self.assertEqual(30 * 60, cache._cache_ttl)
diff --git a/releasenotes/notes/broken-driver-update-fc5303340080ef04.yaml b/releasenotes/notes/broken-driver-update-fc5303340080ef04.yaml
new file mode 100644
index 000000000..057de4dd9
--- /dev/null
+++ b/releasenotes/notes/broken-driver-update-fc5303340080ef04.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+ - |
+ A bug has been fixed in the node update code that could cause the nodes
+ to become not updatable if their driver is no longer available.
diff --git a/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml b/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml
new file mode 100644
index 000000000..4184a4cd7
--- /dev/null
+++ b/releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+ - |
+ Fixes an issue where the master instance image cache could not be disabled.
+ The configuration option ``[pxe]/instance_master_path`` may now be set to
+ the empty string to disable the cache.
diff --git a/releasenotes/notes/fix-tftp-master-path-config-77face94f5db9af7.yaml b/releasenotes/notes/fix-tftp-master-path-config-77face94f5db9af7.yaml
new file mode 100644
index 000000000..0769ceddc
--- /dev/null
+++ b/releasenotes/notes/fix-tftp-master-path-config-77face94f5db9af7.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+ - |
+ Fixes an issue where the master TFTP image cache could not be disbled.
+ The configuration option ``[pxe]/tftp_master_path`` may now be set to
+ the empty string to disable the cache. For more information, see
+ story `2004608 <https://storyboard.openstack.org/#!/story/2004608>`_.
diff --git a/releasenotes/notes/story-2004266-4725d327900850bf.yaml b/releasenotes/notes/story-2004266-4725d327900850bf.yaml
index 41f34decc..c8e27e8fb 100644
--- a/releasenotes/notes/story-2004266-4725d327900850bf.yaml
+++ b/releasenotes/notes/story-2004266-4725d327900850bf.yaml
@@ -6,8 +6,8 @@ fixes:
received within 60-second timeout (countdown restarts when a Chassis
Control command is received). Some BMCs do not support setting this; if
sent it causes the boot to be aborted instead. For IPMI hardware type a
- new driver option ``node['driver_info']['ipmi_disable_timeout']`` can be
- specified. It is ``True`` by default; set it to ``False`` to bypass
+ new driver option ``node['driver_info']['ipmi_disable_boot_timeout']`` can
+ be specified. It is ``True`` by default; set it to ``False`` to bypass
sending this command. See `story 2004266
<https://storyboard.openstack.org/#!/story/2004266>`_ for additional
information.
View Lorry Controller Status