Merge "Do not put the whole JSON error from the agent in last_error"

3 files changed, 26 insertions, 8 deletions

diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index e667d8baa..c905d6a9b 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -31,6 +31,7 @@ from ironic.conductor import utils as manager_utils
 from ironic.conf import CONF
 from ironic.drivers import base
 from ironic.drivers.modules import agent_base
+from ironic.drivers.modules import agent_client
 from ironic.drivers.modules import boot_mode_utils
 from ironic.drivers.modules import deploy_utils
 
@@ -283,7 +284,7 @@ class AgentDeployMixin(agent_base.AgentDeployMixin):
         # the prepare_image command is complete
         command = self._client.get_commands_status(node)[-1]
         if command['command_status'] == 'FAILED':
-            return command['command_error']
+            return agent_client.get_command_error(command)
 
     @METRICS.timer('AgentDeployMixin.reboot_to_instance')
     def reboot_to_instance(self, task):
diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py
index ffcab213a..a85832de5 100644
--- a/ironic/drivers/modules/agent_base.py
+++ b/ironic/drivers/modules/agent_base.py
@@ -361,8 +361,7 @@ def execute_step(task, step, step_type):
     if not result.get('command_status'):
         _raise(step_type, _(
             'Agent on node %(node)s returned bad command result: '
-            '%(result)s') % {'node': task.node.uuid,
-                             'result': result.get('command_error')})
+            '%(result)s') % {'node': task.node.uuid, 'result': result})
     return states.CLEANWAIT if step_type == 'clean' else states.DEPLOYWAIT
 
 
@@ -648,7 +647,7 @@ class HeartbeatMixin(object):
             # handler.
             fail_reason = (_('Agent returned bad result for command '
                              'finalize_rescue: %(result)s') %
-                           {'result': result.get('command_error')})
+                           {'result': agent_client.get_command_error(result)})
             raise exception.InstanceRescueFailure(node=node.uuid,
                                                   instance=node.instance_uuid,
                                                   reason=fail_reason)
@@ -1016,7 +1015,7 @@ class AgentDeployMixin(HeartbeatMixin):
             msg = (_('Agent returned error for %(type)s step %(step)s on node '
                      '%(node)s : %(err)s.') %
                    {'node': node.uuid,
-                    'err': command.get('command_error'),
+                    'err': agent_client.get_command_error(command),
                     'step': current_step,
                     'type': step_type})
             LOG.error(msg)
@@ -1283,7 +1282,7 @@ class AgentDeployMixin(HeartbeatMixin):
                     msg = (_("Failed to install a bootloader when "
                              "deploying node %(node)s. Error: %(error)s") %
                            {'node': node.uuid,
-                            'error': result['command_error']})
+                            'error': agent_client.get_command_error(result)})
                     log_and_raise_deployment_error(task, msg)
                 else:
                     # Its possible the install will fail if the IPA image
@@ -1291,7 +1290,7 @@ class AgentDeployMixin(HeartbeatMixin):
                     LOG.info('Could not install bootloader for whole disk '
                              'image for node %(node)s, Error: %(error)s"',
                              {'node': node.uuid,
-                              'error': result['command_error']})
+                              'error': agent_client.get_command_error(result)})
                     return
 
         try:
diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py
index 59d1aa203..f66008ebe 100644
--- a/ironic/drivers/modules/agent_client.py
+++ b/ironic/drivers/modules/agent_client.py
@@ -31,6 +31,24 @@ METRICS = metrics_utils.get_metrics_logger(__name__)
 DEFAULT_IPA_PORTAL_PORT = 3260
 
 
+def get_command_error(command):
+    """Extract an error string from the command result.
+
+    :param command: Command information from the agent.
+    :return: Error string.
+    """
+    error = command.get('command_error')
+    if error is None:
+        LOG.error('Agent returned invalid response: missing command_error in '
+                  '%s', command)
+        return _('Invalid agent response')
+
+    if isinstance(error, dict):
+        return error.get('details') or error.get('message') or str(error)
+    else:
+        return error
+
+
 class AgentClient(object):
     """Client for interacting with nodes via a REST API."""
     @METRICS.timer('AgentClient.__init__')
@@ -64,7 +82,7 @@ class AgentClient(object):
                       {'method': method, 'node': node.uuid, 'error': error})
             raise exception.AgentAPIError(node=node.uuid,
                                           status=error.get('code'),
-                                          error=result.get('faultstring'))
+                                          error=get_command_error(result))
 
     @METRICS.timer('AgentClient._wait_for_command')
     @retrying.retry(