diff --git a/ironic_python_agent/extensions/clean.py b/ironic_python_agent/extensions/clean.py index 87d2a4fe7..1b6275341 100644 --- a/ironic_python_agent/extensions/clean.py +++ b/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' % - {'step': step['step'], 'err': e}) + msg = ('Unexpected exception performing clean step %(step)s. ' + '%(cls)s: %(err)s' % {'step': step['step'], + 'cls': e.__class__.__name__, + 'err': e}) LOG.exception(msg) raise errors.CleaningError(msg) diff --git a/ironic_python_agent/extensions/deploy.py b/ironic_python_agent/extensions/deploy.py index 333668914..d9dc8490e 100644 --- a/ironic_python_agent/extensions/deploy.py +++ b/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' % - {'step': step['step'], 'err': e}) + msg = ('Unexpected exception performing deploy step %(step)s. ' + '%(cls)s: %(err)s' % {'step': step['step'], + 'cls': e.__class__.__name__, + 'err': e}) LOG.exception(msg) raise errors.DeploymentError(msg) diff --git a/ironic_python_agent/tests/unit/extensions/test_clean.py b/ironic_python_agent/tests/unit/extensions/test_clean.py index 43cd8ec90..50299ce39 100644 --- a/ironic_python_agent/tests/unit/extensions/test_clean.py +++ b/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( diff --git a/ironic_python_agent/tests/unit/extensions/test_deploy.py b/ironic_python_agent/tests/unit/extensions/test_deploy.py index 32cd0f997..5cd52b329 100644 --- a/ironic_python_agent/tests/unit/extensions/test_deploy.py +++ b/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(