From e70ec7aabd079c2c6795018424631755de8231f8 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Thu, 8 Oct 2015 01:57:38 -0700 Subject: [PATCH] Fixes logging of failure in deletion of swift temporary object Change log level for failure to delete swift temporary object. Change-Id: I5111a589aea15512fd2cc145a02e19e2d61154ab Closes-Bug: #1504012 --- ironic/common/exception.py | 5 ++ ironic/common/swift.py | 6 +++ ironic/drivers/modules/ilo/common.py | 9 +++- ironic/tests/unit/common/test_swift.py | 21 ++++++++ .../unit/drivers/modules/ilo/test_common.py | 49 +++++++++++++++++++ .../unit/drivers/modules/ilo/test_deploy.py | 22 +++++++++ 6 files changed, 110 insertions(+), 2 deletions(-) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 3d6a9f9f3c..0db5014923 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -546,6 +546,11 @@ class SwiftOperationError(IronicException): _msg_fmt = _("Swift operation '%(operation)s' failed: %(error)s") +class SwiftObjectNotFoundError(SwiftOperationError): + _msg_fmt = _("Swift object %(object)s from container %(container)s " + "not found. Operation '%(operation)s' failed.") + + class SNMPFailure(IronicException): _msg_fmt = _("SNMP operation '%(operation)s' failed: %(error)s") diff --git a/ironic/common/swift.py b/ironic/common/swift.py index 8fa2d6585a..ea82fdc700 100644 --- a/ironic/common/swift.py +++ b/ironic/common/swift.py @@ -151,12 +151,18 @@ class SwiftAPI(object): :param container: The name of the container in which Swift object is placed. :param object: The name of the object in Swift to be deleted. + :raises: SwiftObjectNotFoundError, if object is not found in Swift. :raises: SwiftOperationError, if operation with Swift fails. """ try: self.connection.delete_object(container, object) except swift_exceptions.ClientException as e: operation = _("delete object") + if e.http_status == 404: + raise exception.SwiftObjectNotFoundError(object=object, + container=container, + operation=operation) + raise exception.SwiftOperationError(operation=operation, error=e) def head_object(self, container, object): diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index bf3d8a9958..b814315b81 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -31,6 +31,7 @@ from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common.i18n import _LE from ironic.common.i18n import _LI +from ironic.common.i18n import _LW from ironic.common import images from ironic.common import swift from ironic.common import utils @@ -514,9 +515,13 @@ def cleanup_vmedia_boot(task): try: swift_api = swift.SwiftAPI() swift_api.delete_object(container, object_name) + except exception.SwiftObjectNotFoundError as e: + LOG.warning(_LW("Temporary object associated with virtual floppy " + "was already deleted from Swift. Error: %s"), e) except exception.SwiftOperationError as e: - LOG.exception(_LE("Error while deleting %(object_name)s from " - "%(container)s. Error: %(error)s"), + LOG.exception(_LE("Error while deleting temporary swift object " + "%(object_name)s from %(container)s associated " + "with virtual floppy. Error: %(error)s"), {'object_name': object_name, 'container': container, 'error': e}) else: diff --git a/ironic/tests/unit/common/test_swift.py b/ironic/tests/unit/common/test_swift.py index 55ef688488..ef2776a6ca 100644 --- a/ironic/tests/unit/common/test_swift.py +++ b/ironic/tests/unit/common/test_swift.py @@ -136,6 +136,27 @@ class SwiftTestCase(base.TestCase): connection_obj_mock.delete_object.assert_called_once_with('container', 'object') + def test_delete_object_exc_resource_not_found(self, connection_mock): + swiftapi = swift.SwiftAPI() + exc = swift_exception.ClientException("Resource not found", + http_status=404) + connection_obj_mock = connection_mock.return_value + connection_obj_mock.delete_object.side_effect = exc + self.assertRaises(exception.SwiftObjectNotFoundError, + swiftapi.delete_object, 'container', 'object') + connection_obj_mock.delete_object.assert_called_once_with('container', + 'object') + + def test_delete_object_exc(self, connection_mock): + swiftapi = swift.SwiftAPI() + exc = swift_exception.ClientException("Operation error") + connection_obj_mock = connection_mock.return_value + connection_obj_mock.delete_object.side_effect = exc + self.assertRaises(exception.SwiftOperationError, + swiftapi.delete_object, 'container', 'object') + connection_obj_mock.delete_object.assert_called_once_with('container', + 'object') + def test_head_object(self, connection_mock): swiftapi = swift.SwiftAPI() connection_obj_mock = connection_mock.return_value diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index a5ddf7e8fb..05c97c860a 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -481,6 +481,55 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): 'ilo_cont', 'image-node-uuid') eject_mock.assert_called_once_with(task) + @mock.patch.object(ilo_common.LOG, 'exception', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', + spec_set=True, autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, '_get_floppy_image_name', spec_set=True, + autospec=True) + def test_cleanup_vmedia_boot_exc(self, get_name_mock, swift_api_mock, + eject_mock, log_mock): + exc = exception.SwiftOperationError('error') + swift_obj_mock = swift_api_mock.return_value + swift_obj_mock.delete_object.side_effect = exc + CONF.ilo.swift_ilo_container = 'ilo_cont' + + get_name_mock.return_value = 'image-node-uuid' + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + ilo_common.cleanup_vmedia_boot(task) + swift_obj_mock.delete_object.assert_called_once_with( + 'ilo_cont', 'image-node-uuid') + self.assertTrue(log_mock.called) + eject_mock.assert_called_once_with(task) + + @mock.patch.object(ilo_common.LOG, 'warning', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', + spec_set=True, autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, '_get_floppy_image_name', spec_set=True, + autospec=True) + def test_cleanup_vmedia_boot_exc_resource_not_found(self, get_name_mock, + swift_api_mock, + eject_mock, log_mock): + exc = exception.SwiftObjectNotFoundError('error') + swift_obj_mock = swift_api_mock.return_value + swift_obj_mock.delete_object.side_effect = exc + CONF.ilo.swift_ilo_container = 'ilo_cont' + + get_name_mock.return_value = 'image-node-uuid' + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + ilo_common.cleanup_vmedia_boot(task) + swift_obj_mock.delete_object.assert_called_once_with( + 'ilo_cont', 'image-node-uuid') + self.assertTrue(log_mock.called) + eject_mock.assert_called_once_with(task) + @mock.patch.object(ilo_common, 'eject_vmedia_devices', spec_set=True, autospec=True) @mock.patch.object(ilo_common, 'destroy_floppy_image_from_web_server', diff --git a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py index f20000fe82..25d6e8447c 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py @@ -297,6 +297,28 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', 'boot-object') + @mock.patch.object(ilo_deploy.LOG, 'exception', spec_set=True, + autospec=True) + @mock.patch.object(ilo_deploy, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + def test__clean_up_boot_iso_for_instance_exc(self, swift_mock, + boot_object_name_mock, + log_mock): + swift_obj_mock = swift_mock.return_value + exc = exception.SwiftObjectNotFoundError('error') + swift_obj_mock.delete_object.side_effect = exc + CONF.ilo.swift_ilo_container = 'ilo-cont' + boot_object_name_mock.return_value = 'boot-object' + i_info = self.node.instance_info + i_info['ilo_boot_iso'] = 'swift:bootiso' + self.node.instance_info = i_info + self.node.save() + ilo_deploy._clean_up_boot_iso_for_instance(self.node) + swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', + 'boot-object') + self.assertTrue(log_mock.called) + @mock.patch.object(utils, 'unlink_without_raise', spec_set=True, autospec=True) def test__clean_up_boot_iso_for_instance_on_webserver(self, unlink_mock):