Merge "Simplify error messages when running clean/deploy step"

ironic_python_agent/extensions/clean.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from ironic_lib import exception as il_exc
 from oslo_log import log
 
 from ironic_python_agent import errors
@@ -73,9 +74,14 @@ class CleanExtension(base.BaseAgentExtension):
             raise ValueError(msg)
         try:
             result = hardware.dispatch_to_managers(step['step'], node, ports)
+        except (errors.RESTError, il_exc.IronicException):
+            LOG.exception('Error performing clean step %s', step['step'])
+            raise
         except Exception as e:
-            msg = ('Error performing clean step %(step)s: %(err)s' %
+            msg = ('Unexpected exception performing clean step %(step)s. '
-                   {'step': step['step'], 'err': e})
+                   '%(cls)s: %(err)s' % {'step': step['step'],
+                                         'cls': e.__class__.__name__,
+                                         'err': e})
             LOG.exception(msg)
             raise errors.CleaningError(msg)
 

ironic_python_agent/extensions/deploy.py

@@ -10,6 +10,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from ironic_lib import exception as il_exc
 from oslo_log import log
 
 from ironic_python_agent import errors
@@ -75,9 +76,14 @@ class DeployExtension(base.BaseAgentExtension):
         try:
             result = hardware.dispatch_to_managers(step['step'], node, ports,
                                                    **kwargs)
+        except (errors.RESTError, il_exc.IronicException):
+            LOG.exception('Error performing deploy step %s', step['step'])
+            raise
         except Exception as e:
-            msg = ('Error performing deploy step %(step)s: %(err)s' %
+            msg = ('Unexpected exception performing deploy step %(step)s. '
-                   {'step': step['step'], 'err': e})
+                   '%(cls)s: %(err)s' % {'step': step['step'],
+                                         'cls': e.__class__.__name__,
+                                         'err': e})
             LOG.exception(msg)
             raise errors.DeploymentError(msg)
 

ironic_python_agent/tests/unit/extensions/test_clean.py

@@ -209,7 +209,8 @@ class TestCleanExtension(base.IronicAgentTest):
                 autospec=True)
     def test_execute_clean_step_fail(self, mock_version, mock_dispatch,
                                      mock_cache_node):
-        mock_dispatch.side_effect = RuntimeError
+        err = errors.BlockDeviceError("I'm a teapot")
+        mock_dispatch.side_effect = err
 
         async_result = self.agent_extension.execute_clean_step(
             step=self.step['GenericHardwareManager'][0], node=self.node,
@@ -217,6 +218,29 @@ class TestCleanExtension(base.IronicAgentTest):
         async_result.join()
 
         self.assertEqual('FAILED', async_result.command_status)
+        self.assertEqual(err, async_result.command_error)
+
+        mock_version.assert_called_once_with(self.version)
+        mock_dispatch.assert_called_once_with(
+            self.step['GenericHardwareManager'][0]['step'],
+            self.node, self.ports)
+        mock_cache_node.assert_called_once_with(self.node)
+
+    @mock.patch('ironic_python_agent.hardware.dispatch_to_managers',
+                autospec=True)
+    @mock.patch('ironic_python_agent.hardware.check_versions',
+                autospec=True)
+    def test_execute_clean_step_exception(self, mock_version, mock_dispatch,
+                                          mock_cache_node):
+        mock_dispatch.side_effect = RuntimeError('boom')
+
+        async_result = self.agent_extension.execute_clean_step(
+            step=self.step['GenericHardwareManager'][0], node=self.node,
+            ports=self.ports, clean_version=self.version)
+        async_result.join()
+
+        self.assertEqual('FAILED', async_result.command_status)
+        self.assertIn('RuntimeError: boom', str(async_result.command_error))
 
         mock_version.assert_called_once_with(self.version)
         mock_dispatch.assert_called_once_with(

ironic_python_agent/tests/unit/extensions/test_deploy.py

@@ -235,7 +235,8 @@ class TestDeployExtension(base.IronicAgentTest):
                 autospec=True)
     def test_execute_deploy_step_fail(self, mock_version, mock_dispatch,
                                       mock_cache_node):
-        mock_dispatch.side_effect = RuntimeError
+        err = errors.BlockDeviceError("I'm a teapot")
+        mock_dispatch.side_effect = err
 
         async_result = self.agent_extension.execute_deploy_step(
             step=self.step['GenericHardwareManager'][0], node=self.node,
@@ -243,6 +244,29 @@ class TestDeployExtension(base.IronicAgentTest):
         async_result.join()
 
         self.assertEqual('FAILED', async_result.command_status)
+        self.assertEqual(err, async_result.command_error)
+
+        mock_version.assert_called_once_with(self.version)
+        mock_dispatch.assert_called_once_with(
+            self.step['GenericHardwareManager'][0]['step'],
+            self.node, self.ports)
+        mock_cache_node.assert_called_once_with(self.node)
+
+    @mock.patch('ironic_python_agent.hardware.dispatch_to_managers',
+                autospec=True)
+    @mock.patch('ironic_python_agent.hardware.check_versions',
+                autospec=True)
+    def test_execute_deploy_step_exception(self, mock_version, mock_dispatch,
+                                           mock_cache_node):
+        mock_dispatch.side_effect = RuntimeError('boom')
+
+        async_result = self.agent_extension.execute_deploy_step(
+            step=self.step['GenericHardwareManager'][0], node=self.node,
+            ports=self.ports, deploy_version=self.version)
+        async_result.join()
+
+        self.assertEqual('FAILED', async_result.command_status)
+        self.assertIn('RuntimeError: boom', str(async_result.command_error))
 
         mock_version.assert_called_once_with(self.version)
         mock_dispatch.assert_called_once_with(