diff options
author | Dmitry Tantsur <dtantsur@protonmail.com> | 2022-02-17 17:28:47 +0100 |
---|---|---|
committer | Dmitry Tantsur <dtantsur@protonmail.com> | 2022-02-17 19:16:52 +0100 |
commit | daa7dba3312630e2672dad33a4df87f75fa7396d (patch) | |
tree | 3022ce9c660a6a816f2f73845f695e11467761cf /ironic | |
parent | 8452de687efe14a335fd93784b528c8eeea4db10 (diff) | |
download | ironic-daa7dba3312630e2672dad33a4df87f75fa7396d.tar.gz |
Shorten error messages in commonly used modules
* Do not mention "deploy driver", it's not a thing.
* Be careful with the pattern "Error: %s" or "Reason: %s". It is good
for long introductory sentences, but looks poor for shorter ones and
becomes really problematic when several instances are concatenated.
This change updates deploy_utils, agent code and conductor modules.
Change-Id: Ie1efea02b5f1a174e9ef8c5253ce9754a60b4c56
Diffstat (limited to 'ironic')
-rw-r--r-- | ironic/conductor/cleaning.py | 5 | ||||
-rw-r--r-- | ironic/conductor/deployments.py | 9 | ||||
-rw-r--r-- | ironic/conductor/manager.py | 18 | ||||
-rw-r--r-- | ironic/conductor/utils.py | 16 | ||||
-rw-r--r-- | ironic/drivers/modules/agent_base.py | 12 | ||||
-rw-r--r-- | ironic/drivers/modules/deploy_utils.py | 10 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_manager.py | 2 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_agent_base.py | 13 |
8 files changed, 39 insertions, 46 deletions
diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py index 091ca64c6..cb801fc8c 100644 --- a/ironic/conductor/cleaning.py +++ b/ironic/conductor/cleaning.py @@ -68,8 +68,7 @@ def do_node_clean(task, clean_steps=None, disable_ramdisk=False): if not disable_ramdisk: task.driver.network.validate(task) except exception.InvalidParameterValue as e: - msg = (_('Validation failed. Cannot clean node %(node)s. ' - 'Error: %(msg)s') % + msg = (_('Validation of node %(node)s for cleaning failed: %(msg)s') % {'node': node.uuid, 'msg': e}) return utils.cleaning_error_handler(task, msg) @@ -115,7 +114,7 @@ def do_node_clean(task, clean_steps=None, disable_ramdisk=False): task, disable_ramdisk=disable_ramdisk) except (exception.InvalidParameterValue, exception.NodeCleaningFailure) as e: - msg = (_('Cannot clean node %(node)s. Error: %(msg)s') + msg = (_('Cannot clean node %(node)s: %(msg)s') % {'node': node.uuid, 'msg': e}) return utils.cleaning_error_handler(task, msg) diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index fee436c32..3b72dcb03 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -99,7 +99,7 @@ def start_deploy(task, manager, configdrive=None, event='deploy', except exception.InvalidParameterValue as e: raise exception.InstanceDeployFailure( _("Failed to validate deploy or power info for node " - "%(node_uuid)s. Error: %(msg)s") % + "%(node_uuid)s: %(msg)s") % {'node_uuid': node.uuid, 'msg': e}, code=e.code) try: @@ -131,8 +131,7 @@ def do_node_deploy(task, conductor_id=None, configdrive=None, task, ('Error while uploading the configdrive for %(node)s ' 'to Swift') % {'node': node.uuid}, - _('Failed to upload the configdrive to Swift. ' - 'Error: %s') % e, + _('Failed to upload the configdrive to Swift: %s') % e, clean_up=False) except db_exception.DBDataError as e: with excutils.save_and_reraise_exception(): @@ -193,7 +192,7 @@ def do_node_deploy(task, conductor_id=None, configdrive=None, utils.deploying_error_handler( task, 'Error while getting deploy steps; cannot deploy to node ' - '%(node)s. Error: %(err)s' % {'node': node.uuid, 'err': e}, + '%(node)s: %(err)s' % {'node': node.uuid, 'err': e}, _("Cannot get deploy steps; failed to deploy: %s") % e) if not node.driver_internal_info.get('deploy_steps'): @@ -280,7 +279,7 @@ def do_next_deploy_step(task, step_index): # Avoid double handling of failures. For example, set_failed_state # from deploy_utils already calls deploying_error_handler. if task.node.provision_state != states.DEPLOYFAIL: - log_msg = ('Node %(node)s failed deploy step %(step)s. Error: ' + log_msg = ('Node %(node)s failed deploy step %(step)s: ' '%(err)s' % {'node': node.uuid, 'step': node.deploy_step, 'err': e}) utils.deploying_error_handler( diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 9b1c20d9b..e66357fbe 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -698,7 +698,7 @@ class ConductorManager(base_manager.BaseConductorManager): raise exception.InstanceRescueFailure( instance=node.instance_uuid, node=node.uuid, - reason=_("Validation failed. Error: %s") % e) + reason=_("Validation failed: %s") % e) try: task.process_event( 'rescue', @@ -792,7 +792,7 @@ class ConductorManager(base_manager.BaseConductorManager): raise exception.InstanceUnrescueFailure( instance=node.instance_uuid, node=node.uuid, - reason=_("Validation failed. Error: %s") % e) + reason=_("Validation failed: %s") % e) try: task.process_event( @@ -853,7 +853,7 @@ class ConductorManager(base_manager.BaseConductorManager): task.driver.rescue.clean_up(task) except Exception as e: LOG.exception('Failed to clean up rescue for node %(node)s ' - 'after aborting the operation. Error: %(err)s', + 'after aborting the operation: %(err)s', {'node': node.uuid, 'err': e}) error_msg = _('Failed to clean up rescue after aborting ' 'the operation') @@ -1073,7 +1073,7 @@ class ConductorManager(base_manager.BaseConductorManager): with excutils.save_and_reraise_exception(): LOG.exception('Error in tear_down of node %(node)s: %(err)s', {'node': node.uuid, 'err': e}) - error = _("Failed to tear down. Error: %s") % e + error = _("Failed to tear down: %s") % e utils.node_history_record(task.node, event=error, event_type=states.UNPROVISION, error=True, @@ -1162,8 +1162,8 @@ class ConductorManager(base_manager.BaseConductorManager): task.driver.power.validate(task) task.driver.network.validate(task) except exception.InvalidParameterValue as e: - msg = (_('Validation failed. Cannot clean node %(node)s. ' - 'Error: %(msg)s') % + msg = (_('Validation of node %(node)s for cleaning ' + 'failed: %(msg)s') % {'node': node.uuid, 'msg': e}) raise exception.InvalidParameterValue(msg) @@ -1357,8 +1357,7 @@ class ConductorManager(base_manager.BaseConductorManager): with excutils.save_and_reraise_exception(): LOG.exception('Error in aborting the inspection of ' 'node %(node)s', {'node': node.uuid}) - error = _('Failed to abort inspection. ' - 'Error: %s') % e + error = _('Failed to abort inspection: %s') % e utils.node_history_record(task.node, event=error, event_type=states.INTROSPECTION, error=True, @@ -1553,8 +1552,7 @@ class ConductorManager(base_manager.BaseConductorManager): "while trying to get power state.")) except Exception as e: LOG.debug("During power_failure_recovery, could " - "not get power state for node %(node)s, " - "Error: %(err)s.", + "not get power state for node %(node)s: %(err)s", {'node': task.node.uuid, 'err': e}) else: handle_recovery(task, power_state) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index ff5656052..4a0d68a5d 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -249,8 +249,8 @@ def _can_skip_state_change(task, new_state): except Exception as e: with excutils.save_and_reraise_exception(): error = _( - "Failed to change power state to '%(target)s'. " - "Error: %(error)s") % {'target': new_state, 'error': e} + "Failed to change power state to '%(target)s': %(error)s") % { + 'target': new_state, 'error': e} node_history_record(node, event=error, error=True) node['target_power_state'] = states.NOSTATE node.save() @@ -332,7 +332,7 @@ def node_power_action(task, new_state, timeout=None): node['target_power_state'] = states.NOSTATE error = _( "Failed to change power state to '%(target_state)s' " - "by '%(new_state)s'. Error: %(error)s") % { + "by '%(new_state)s': %(error)s") % { 'target_state': target_state, 'new_state': new_state, 'error': e} @@ -365,7 +365,7 @@ def node_power_action(task, new_state, timeout=None): task.driver.storage.detach_volumes(task) except exception.StorageError as e: LOG.warning("Volume detachment for node %(node)s " - "failed. Error: %(error)s", + "failed: %(error)s", {'node': node.uuid, 'error': e}) @@ -1539,8 +1539,8 @@ def node_change_boot_mode(task, target_boot_mode): 'class': type(exc).__name__, 'exc': exc}, exc_info=not isinstance(exc, exception.IronicException)) task.node.last_error = ( - "Failed to change boot mode to '%(target)s'. " - "Error: %(err)s" % {'target': target_boot_mode, 'err': exc}) + "Failed to change boot mode to '%(target)s: %(err)s" % { + 'target': target_boot_mode, 'err': exc}) task.node.save() else: LOG.info("Changed boot_mode to %(mode)s for node %(node)s", @@ -1587,8 +1587,8 @@ def node_change_secure_boot(task, secure_boot_target): 'class': type(exc).__name__, 'exc': exc}, exc_info=not isinstance(exc, exception.IronicException)) task.node.last_error = ( - "Failed to change secure_boot state to '%(target)s'. " - "Error: %(err)s" % {'target': secure_boot_target, 'err': exc}) + "Failed to change secure_boot state to '%(target)s': %(err)s" % { + 'target': secure_boot_target, 'err': exc}) task.node.save() else: LOG.info("Changed secure_boot state to %(state)s for node %(node)s", diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 8f480aca7..582c36d90 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -502,7 +502,7 @@ class HeartbeatMixin(object): msg = _('Failed to process the next deploy step') self.process_next_step(task, 'deploy') except Exception as e: - last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + last_error = _('%(msg)s: %(exc)s') % {'msg': msg, 'exc': e} LOG.exception('Asynchronous exception for node %(node)s: %(err)s', {'node': task.node.uuid, 'err': last_error}) # Do not call the error handler is the node is already DEPLOYFAIL @@ -537,7 +537,7 @@ class HeartbeatMixin(object): if not polling: self.continue_cleaning(task) except Exception as e: - last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + last_error = _('%(msg)s: %(exc)s') % {'msg': msg, 'exc': e} log_msg = ('Asynchronous exception for node %(node)s: %(err)s' % {'node': task.node.uuid, 'err': last_error}) if node.provision_state in (states.CLEANING, states.CLEANWAIT): @@ -549,7 +549,7 @@ class HeartbeatMixin(object): try: self._finalize_rescue(task) except Exception as e: - last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + last_error = _('%(msg)s: %(exc)s') % {'msg': msg, 'exc': e} LOG.exception('Asynchronous exception for node %(node)s: %(err)s', {'node': task.node.uuid, 'err': last_error}) if task.node.provision_state in (states.RESCUING, @@ -1194,7 +1194,7 @@ class AgentDeployMixin(HeartbeatMixin, AgentOobStepsMixin): 'command "sync"') LOG.warning( 'Failed to flush the file system prior to hard ' - 'rebooting the node %(node)s. Error: %(error)s', + 'rebooting the node %(node)s: %(error)s', {'node': node.uuid, 'error': error}) manager_utils.node_power_action(task, states.POWER_OFF) @@ -1327,7 +1327,7 @@ class AgentDeployMixin(HeartbeatMixin, AgentOobStepsMixin): ) if result['command_status'] == 'FAILED': msg = (_("Failed to install a bootloader when " - "deploying node %(node)s. Error: %(error)s") % + "deploying node %(node)s: %(error)s") % {'node': node.uuid, 'error': agent_client.get_command_error(result)}) log_and_raise_deployment_error(task, msg) @@ -1341,7 +1341,7 @@ class AgentDeployMixin(HeartbeatMixin, AgentOobStepsMixin): persistent=persistent) except Exception as e: msg = (_("Failed to change the boot device to %(boot_dev)s " - "when deploying node %(node)s. Error: %(error)s") % + "when deploying node %(node)s: %(error)s") % {'boot_dev': boot_devices.DISK, 'node': node.uuid, 'error': e}) log_and_raise_deployment_error(task, msg, exc=e) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index e1aabb500..e9b3d504f 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -832,8 +832,7 @@ def get_image_instance_info(node): return info -_ERR_MSG_INVALID_DEPLOY = _("Cannot validate parameter for driver deploy. " - "Invalid parameter %(param)s. Reason: %(reason)s") +_ERR_MSG_INVALID_DEPLOY = _("Invalid parameter %(param)s: %(reason)s") def parse_instance_info(node): @@ -865,8 +864,7 @@ def parse_instance_info(node): i_info['ramdisk'] = info.get('ramdisk') i_info['root_gb'] = info.get('root_gb') - error_msg = _("Cannot validate driver deploy. Some parameters were missing" - " in node's instance_info") + error_msg = _("Some parameters were missing in node's instance_info") check_for_missing_params(i_info, error_msg) # This is used in many places, so keep it even for whole-disk images. @@ -1078,7 +1076,7 @@ def _validate_image_url(node, url, secret=False): except exception.ImageRefValidationFailed as e: with excutils.save_and_reraise_exception(): LOG.error("The specified URL is not a valid HTTP(S) URL or is " - "not reachable for node %(node)s. Error: %(msg)s", + "not reachable for node %(node)s: %(msg)s", {'node': node.uuid, 'msg': e}) @@ -1458,5 +1456,5 @@ def get_root_device_for_deploy(node): except ValueError as e: raise exception.InvalidParameterValue( _('Failed to validate the root device hints %(hints)s (from the ' - 'node\'s %(source)s) for node %(node)s. Error: %(error)s') % + 'node\'s %(source)s) for node %(node)s: %(error)s') % {'node': node.uuid, 'hints': hints, 'source': source, 'error': e}) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 7016576b7..5c184ba94 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -8188,7 +8188,7 @@ class DoNodeInspectAbortTestCase(mgr_utils.CommonMixIn, "abort") node.refresh() self.assertTrue(mock_log.exception.called) - self.assertIn('Failed to abort inspection.', node.last_error) + self.assertIn('Failed to abort inspection', node.last_error) @mock.patch('ironic.drivers.modules.fake.FakeInspect.abort', autospec=True) @mock.patch('ironic.conductor.task_manager.acquire', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 7639c83f9..97daca79f 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -278,8 +278,8 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): task, mock.ANY, collect_logs=True) log_mock.assert_called_once_with( 'Asynchronous exception for node %(node)s: %(err)s', - {'err': 'Failed to process the next deploy step. ' - 'Error: LlamaException', + {'err': 'Failed to process the next deploy step: ' + 'LlamaException', 'node': task.node.uuid}) @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @@ -307,8 +307,8 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): self.assertFalse(failed_mock.called) log_mock.assert_called_once_with( 'Asynchronous exception for node %(node)s: %(err)s', - {'err': 'Failed to process the next deploy step. ' - 'Error: LlamaException', + {'err': 'Failed to process the next deploy step: ' + 'LlamaException', 'node': task.node.uuid}) @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) @@ -460,8 +460,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): mock_finalize.assert_called_once_with(mock.ANY, task) mock_rescue_err_handler.assert_called_once_with( - task, 'Node failed to perform ' - 'rescue operation. Error: some failure') + task, 'Node failed to perform rescue operation: some failure') @mock.patch.object(agent_base.LOG, 'error', autospec=True) def test_heartbeat_records_cleaning_deploying(self, log_mock): @@ -827,7 +826,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): 'deployment do not support the command "sync"') log_mock.assert_called_once_with( 'Failed to flush the file system prior to hard rebooting the ' - 'node %(node)s. Error: %(error)s', + 'node %(node)s: %(error)s', {'node': task.node.uuid, 'error': log_error}) self.assertFalse(mock_collect.called) |