diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index afe2a4fcad..f30cc4c788 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1158,7 +1158,20 @@ class ConductorManager(base_manager.BaseConductorManager): # Remove lessee, as it was automatically added node.lessee = None node.del_driver_internal_info('automatic_lessee') - network.remove_vifs_from_node(task) + try: + network.remove_vifs_from_node(task) + except Exception as e: + 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: %s") % e + utils.node_history_record(task.node, event=error, + event_type=states.UNPROVISION, + error=True, + user=task.context.user_id) + task.process_event('fail') + node.save() if node.allocation_id: allocation = objects.Allocation.get_by_id(task.context, diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 0f2b553f2e..ad5514308a 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2657,6 +2657,36 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): # Verify reservation has been cleared. self.assertIsNone(node.reservation) + @mock.patch('ironic.common.network.remove_vifs_from_node', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down', + autospec=True) + def test__do_node_tear_remove_vif_failed(self, mock_tear_down, + mock_remove): + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.DELETING, + target_provision_state=states.AVAILABLE, + instance_uuid=uuidutils.generate_uuid(), + instance_info={'foo': 'bar'}, + driver_internal_info={'is_whole_disk_image': False, + 'deploy_steps': {}, + 'instance': {'ephemeral_gb': 10}}) + port = obj_utils.create_test_port( + self.context, node_id=node.id, + internal_info={'tenant_vif_port_id': 'foo'}) + + mock_remove.side_effect = exception.IronicException('Oops') + task = task_manager.TaskManager(self.context, node.uuid) + self._start_service() + self.assertRaises(exception.IronicException, + self.service._do_node_tear_down, + task, node.provision_state) + node.refresh() + port.refresh() + self.assertEqual(states.ERROR, node.provision_state) + mock_tear_down.assert_called_once_with(task.driver.deploy, task) + @mgr_utils.mock_record_keepalive class DoProvisioningActionTestCase(mgr_utils.ServiceSetUpMixin, diff --git a/releasenotes/notes/unhandled-remove-vif-exception-89fd332d0c1feea7.yaml b/releasenotes/notes/unhandled-remove-vif-exception-89fd332d0c1feea7.yaml new file mode 100644 index 0000000000..692d185cff --- /dev/null +++ b/releasenotes/notes/unhandled-remove-vif-exception-89fd332d0c1feea7.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an error that node does not move to failed state when + removing vif failed due to unexpected errors during tear down. \ No newline at end of file