diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 8e6470233e..cc896b0f8a 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -826,11 +826,16 @@ def set_failed_state(task, msg): :param task: a TaskManager instance containing the node to act on. :param msg: the message to set in last_error of the node. - :raises: InvalidState if the event is not allowed by the associated - state machine. """ - task.process_event('fail') node = task.node + try: + task.process_event('fail') + except exception.InvalidState: + msg2 = (_LE('Internal error. Node %(node)s in provision state ' + '"%(state)s" could not transition to a failed state.') + % {'node': node.uuid, 'state': node.provision_state}) + LOG.exception(msg2) + try: manager_utils.node_power_action(task, states.POWER_OFF) except Exception: @@ -839,11 +844,11 @@ def set_failed_state(task, msg): 'should be removed from Ironic or put in maintenance ' 'mode until the problem is resolved.') % node.uuid) LOG.exception(msg2) - finally: - # NOTE(deva): node_power_action() erases node.last_error - # so we need to set it again here. - node.last_error = msg - node.save() + + # NOTE(deva): node_power_action() erases node.last_error + # so we need to set it here. + node.last_error = msg + node.save() def get_single_nic_with_vif_port_id(task): diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py index 885fd4e1fd..784236d0a8 100644 --- a/ironic/tests/drivers/test_deploy_utils.py +++ b/ironic/tests/drivers/test_deploy_utils.py @@ -1033,6 +1033,40 @@ class OtherFunctionTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, utils.parse_root_device_hints, self.node) + @mock.patch.object(utils, 'LOG', autospec=True) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(task_manager.TaskManager, 'process_event', + autospec=True) + def _test_set_failed_state(self, mock_event, mock_power, mock_log, + event_value=None, power_value=None, + log_calls=None): + err_msg = 'some failure' + mock_event.side_effect = event_value + mock_power.side_effect = power_value + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + utils.set_failed_state(task, err_msg) + mock_event.assert_called_once_with(task, 'fail') + mock_power.assert_called_once_with(task, states.POWER_OFF) + self.assertEqual(err_msg, task.node.last_error) + if log_calls: + mock_log.exception.assert_has_calls(log_calls) + else: + self.assertFalse(mock_log.called) + + def test_set_failed_state(self): + exc_state = exception.InvalidState('invalid state') + exc_param = exception.InvalidParameterValue('invalid parameter') + mock_call = mock.call(mock.ANY) + self._test_set_failed_state() + self._test_set_failed_state(event_value=exc_state, + log_calls=[mock_call]) + self._test_set_failed_state(power_value=exc_param, + log_calls=[mock_call]) + self._test_set_failed_state(event_value=exc_state, + power_value=exc_param, + log_calls=[mock_call, mock_call]) + @mock.patch.object(disk_partitioner.DiskPartitioner, 'commit', lambda _: None) class WorkOnDiskTestCase(tests_base.TestCase):