diff options
| -rw-r--r-- | ironic/conductor/utils.py | 8 | ||||
| -rw-r--r-- | ironic/conf/pxe.py | 2 | ||||
| -rw-r--r-- | ironic/drivers/modules/iscsi_deploy.py | 3 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_utils.py | 30 | ||||
| -rw-r--r-- | ironic/tests/unit/drivers/modules/test_iscsi_deploy.py | 26 | ||||
| -rw-r--r-- | releasenotes/notes/cleanwait_timeout_fail-4323ba7d4d4da3e6.yaml | 8 | ||||
| -rw-r--r-- | releasenotes/notes/fix-instance-master-path-config-fa524c907a7888e5.yaml | 6 | ||||
| -rw-r--r-- | zuul.d/project.yaml | 7 |
8 files changed, 75 insertions, 15 deletions
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 2667862e1..e7f2588b1 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -359,8 +359,10 @@ def cleanup_cleanwait_timeout(task): "check if the ramdisk responsible for the cleaning is " "running on the node. Failed on step %(step)s.") % {'step': task.node.clean_step}) - cleaning_error_handler(task, msg=last_error, - set_fail_state=True) + # NOTE(rloo): this is called from the periodic task for cleanwait timeouts, + # via the task manager's process_event(). The node has already been moved + # to CLEANFAIL, so the error handler doesn't need to set the fail state. + cleaning_error_handler(task, msg=last_error, set_fail_state=False) def cleaning_error_handler(task, msg, tear_down_cleaning=True, @@ -393,7 +395,7 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, 'reason: %(err)s' % {'err': e, 'uuid': node.uuid}) LOG.exception(msg) - if set_fail_state: + if set_fail_state and node.provision_state != states.CLEANFAIL: target_state = states.MANAGEABLE if manual_clean else None task.process_event('fail', target_state=target_state) diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py index cb251d9a2..a37e5ae75 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, ' diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 76a262b2c..609661654 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -47,8 +47,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/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 7c3649ef1..0d371fdd8 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -1116,7 +1116,7 @@ class ErrorHandlersTestCase(tests_base.TestCase): msg="Timeout reached while cleaning the node. Please " "check if the ramdisk responsible for the cleaning is " "running on the node. Failed on step {}.", - set_fail_state=True) + set_fail_state=False) def test_cleanup_cleanwait_timeout(self): self.node.provision_state = states.CLEANFAIL @@ -1133,16 +1133,18 @@ class ErrorHandlersTestCase(tests_base.TestCase): conductor_utils.cleanup_cleanwait_timeout(self.task) self.assertEqual({}, self.node.clean_step) self.assertNotIn('clean_step_index', self.node.driver_internal_info) - self.task.process_event.assert_called_once_with('fail', - target_state=None) + self.assertFalse(self.task.process_event.called) self.assertTrue(self.node.maintenance) self.assertEqual(clean_error, self.node.maintenance_reason) - def test_cleaning_error_handler(self): - self.node.provision_state = states.CLEANING + def _test_cleaning_error_handler(self, prov_state=states.CLEANING): + self.node.provision_state = prov_state target = 'baz' self.node.target_provision_state = target - self.node.driver_internal_info = {} + self.node.clean_step = {'key': 'val'} + self.node.driver_internal_info = { + 'cleaning_reboot': True, + 'clean_step_index': 0} msg = 'error bar' conductor_utils.cleaning_error_handler(self.task, msg) self.node.save.assert_called_once_with() @@ -1153,8 +1155,20 @@ class ErrorHandlersTestCase(tests_base.TestCase): self.assertEqual(msg, self.node.maintenance_reason) driver = self.task.driver.deploy driver.tear_down_cleaning.assert_called_once_with(self.task) - self.task.process_event.assert_called_once_with('fail', - target_state=None) + if prov_state == states.CLEANFAIL: + self.assertFalse(self.task.process_event.called) + else: + self.task.process_event.assert_called_once_with('fail', + target_state=None) + + def test_cleaning_error_handler(self): + self._test_cleaning_error_handler() + + def test_cleaning_error_handler_cleanwait(self): + self._test_cleaning_error_handler(prov_state=states.CLEANWAIT) + + def test_cleaning_error_handler_cleanfail(self): + self._test_cleaning_error_handler(prov_state=states.CLEANFAIL) def test_cleaning_error_handler_manual(self): target = states.MANAGEABLE diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 93dbaa790..f1f614c20 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -1009,3 +1009,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/releasenotes/notes/cleanwait_timeout_fail-4323ba7d4d4da3e6.yaml b/releasenotes/notes/cleanwait_timeout_fail-4323ba7d4d4da3e6.yaml new file mode 100644 index 000000000..9587fe80d --- /dev/null +++ b/releasenotes/notes/cleanwait_timeout_fail-4323ba7d4d4da3e6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes an issue with a baremetal node that times out during cleaning. + The ironic-conductor was attempting to change the node's provision state + to 'clean failed' twice, resulting in the node's ``last_error`` being set + incorrectly. This no longer happens. For more information, see + `story 2004299 <https://storyboard.openstack.org/#!/story/2004299>`_. 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/zuul.d/project.yaml b/zuul.d/project.yaml index 971976465..8af55b5f0 100644 --- a/zuul.d/project.yaml +++ b/zuul.d/project.yaml @@ -10,7 +10,9 @@ jobs: - ironic-dsvm-standalone - ironic-grenade-dsvm - - ironic-grenade-dsvm-multinode-multitenant + # Temporarily mark job as non-voting + - ironic-grenade-dsvm-multinode-multitenant: + voting: false - ironic-tempest-dsvm-bfv - ironic-tempest-dsvm-ipa-partition-redfish-tinyipa - ironic-tempest-dsvm-ipa-partition-uefi-pxe_ipmitool-tinyipa @@ -29,7 +31,8 @@ jobs: - ironic-dsvm-standalone - ironic-grenade-dsvm - - ironic-grenade-dsvm-multinode-multitenant + # Temporarily disable job since it's non-voting + # - ironic-grenade-dsvm-multinode-multitenant - ironic-tempest-dsvm-bfv - ironic-tempest-dsvm-ipa-partition-redfish-tinyipa - ironic-tempest-dsvm-ipa-partition-uefi-pxe_ipmitool-tinyipa |