From 7ac4fc76f2d777fd4e0ec4245eaf989f99917be2 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Thu, 14 Aug 2014 20:36:20 +0530 Subject: [PATCH] VMware: Handle exceptions raised by image update Exceptions raised by glance image update are not handled while writing to image. This results in volume being stuck in uploading state for a long time even after image transfer failure. This patch fixes the exception handling. Change-Id: I7651c96968caa49c5421d68ee4dbe9e55f7f53b1 Closes-Bug: #1357239 Partial-Bug: #1269345 --- cinder/tests/test_vmware_io_util.py | 84 +++++++++++++++++++ cinder/volume/drivers/vmware/error_util.py | 5 ++ cinder/volume/drivers/vmware/io_util.py | 32 ++++--- cinder/volume/drivers/vmware/vmware_images.py | 8 +- 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/cinder/tests/test_vmware_io_util.py b/cinder/tests/test_vmware_io_util.py index 6c13291f2..f77fec0d6 100644 --- a/cinder/tests/test_vmware_io_util.py +++ b/cinder/tests/test_vmware_io_util.py @@ -19,9 +19,11 @@ Unit tests for image transfer utility classes. import math +from eventlet import greenthread import mock from cinder import test +from cinder.volume.drivers.vmware import error_util from cinder.volume.drivers.vmware import io_util @@ -65,3 +67,85 @@ class ThreadSafePipeTest(test.TestCase): max_transfer_size = 30 queue = io_util.ThreadSafePipe(10, 30) self.assertEqual(max_transfer_size, queue.tell()) + + +class GlanceWriteThreadTest(test.TestCase): + """Tests for GlanceWriteThread class.""" + + def _create_image_writer(self): + self._image_service = mock.Mock() + self._context = mock.Mock() + self._input_file = mock.Mock() + self._image_id = mock.Mock() + return io_util.GlanceWriteThread(self._context, self._input_file, + self._image_service, self._image_id) + + @mock.patch.object(greenthread, 'sleep') + def test_start(self, mock_sleep): + writer = self._create_image_writer() + status_list = ['queued', 'saving', 'active'] + + def image_service_show_side_effect(context, image_id): + status = status_list.pop(0) + return {'status': status} + + self._image_service.show.side_effect = image_service_show_side_effect + exp_calls = [mock.call(self._context, + self._image_id)] * len(status_list) + + writer.start() + self.assertTrue(writer.wait()) + self._image_service.update.assert_called_once_with( + self._context, self._image_id, {}, data=self._input_file) + self.assertEqual(exp_calls, self._image_service.show.call_args_list) + + def test_start_with_killed_status(self): + writer = self._create_image_writer() + + def image_service_show_side_effect(_context, _image_id): + return {'status': 'killed'} + + self._image_service.show.side_effect = image_service_show_side_effect + + writer.start() + self.assertRaises(error_util.ImageTransferException, writer.wait) + self._image_service.update.assert_called_once_with( + self._context, self._image_id, {}, data=self._input_file) + self._image_service.show.assert_called_once_with(self._context, + self._image_id) + + def test_start_with_unknown_status(self): + writer = self._create_image_writer() + + def image_service_show_side_effect(_context, _image_id): + return {'status': 'unknown'} + + self._image_service.show.side_effect = image_service_show_side_effect + + writer.start() + self.assertRaises(error_util.ImageTransferException, writer.wait) + self._image_service.update.assert_called_once_with( + self._context, self._image_id, {}, data=self._input_file) + self._image_service.show.assert_called_once_with(self._context, + self._image_id) + + def test_start_with_image_service_update_exception(self): + writer = self._create_image_writer() + self._image_service.update.side_effect = Exception + + writer.start() + self.assertRaises(error_util.ImageTransferException, writer.wait) + self._image_service.update.assert_called_once_with( + self._context, self._image_id, {}, data=self._input_file) + self.assertFalse(self._image_service.show.called) + + def test_start_with_image_service_show_exception(self): + writer = self._create_image_writer() + self._image_service.show.side_effect = Exception + + writer.start() + self.assertRaises(error_util.ImageTransferException, writer.wait) + self._image_service.update.assert_called_once_with( + self._context, self._image_id, {}, data=self._input_file) + self._image_service.show.assert_called_once_with(self._context, + self._image_id) diff --git a/cinder/volume/drivers/vmware/error_util.py b/cinder/volume/drivers/vmware/error_util.py index cce8cfd76..8df802a3c 100644 --- a/cinder/volume/drivers/vmware/error_util.py +++ b/cinder/volume/drivers/vmware/error_util.py @@ -78,3 +78,8 @@ class InvalidAdapterTypeException(VMwareDriverException): class InvalidDiskTypeException(VMwareDriverException): """Thrown when the disk type is invalid.""" message = _("Invalid disk type: %(disk_type)s.") + + +class ImageTransferException(VMwareDriverException): + """Thrown when there is an error during image transfer.""" + message = _("Error occurred during image transfer.") diff --git a/cinder/volume/drivers/vmware/io_util.py b/cinder/volume/drivers/vmware/io_util.py index 514f45fe2..a0471954b 100644 --- a/cinder/volume/drivers/vmware/io_util.py +++ b/cinder/volume/drivers/vmware/io_util.py @@ -24,9 +24,9 @@ from eventlet import event from eventlet import greenthread from eventlet import queue -from cinder import exception from cinder.i18n import _ from cinder.openstack.common import log as logging +from cinder.volume.drivers.vmware import error_util LOG = logging.getLogger(__name__) IO_THREAD_SLEEP_TIME = .01 @@ -106,13 +106,15 @@ class GlanceWriteThread(object): LOG.debug("Initiating image service update on image: %(image)s " "with meta: %(meta)s" % {'image': self.image_id, 'meta': self.image_meta}) - self.image_service.update(self.context, - self.image_id, - self.image_meta, - data=self.input_file) - self._running = True - while self._running: - try: + + try: + self.image_service.update(self.context, + self.image_id, + self.image_meta, + data=self.input_file) + + self._running = True + while self._running: image_meta = self.image_service.show(self.context, self.image_id) image_status = image_meta.get('status') @@ -127,7 +129,7 @@ class GlanceWriteThread(object): msg = (_("Glance image: %s is in killed state.") % self.image_id) LOG.error(msg) - excep = exception.CinderException(msg) + excep = error_util.ImageTransferException(msg) self.done.send_exception(excep) elif image_status in ['saving', 'queued']: greenthread.sleep(GLANCE_POLL_INTERVAL) @@ -137,11 +139,15 @@ class GlanceWriteThread(object): "- %(state)s") % {'id': self.image_id, 'state': image_status} LOG.error(msg) - excep = exception.CinderException(msg) + excep = error_util.ImageTransferException(msg) self.done.send_exception(excep) - except Exception as exc: - self.stop() - self.done.send_exception(exc) + except Exception as ex: + self.stop() + msg = (_("Error occurred while writing to image: %s") % + self.image_id) + LOG.exception(msg) + excep = error_util.ImageTransferException(ex) + self.done.send_exception(excep) greenthread.spawn(_inner) return self.done diff --git a/cinder/volume/drivers/vmware/vmware_images.py b/cinder/volume/drivers/vmware/vmware_images.py index 3b47e2893..211cf0921 100644 --- a/cinder/volume/drivers/vmware/vmware_images.py +++ b/cinder/volume/drivers/vmware/vmware_images.py @@ -18,9 +18,9 @@ Utility functions for Image transfer. from eventlet import timeout -from cinder import exception from cinder.i18n import _ from cinder.openstack.common import log as logging +from cinder.volume.drivers.vmware import error_util from cinder.volume.drivers.vmware import io_util from cinder.volume.drivers.vmware import read_write_util as rw_util @@ -79,8 +79,10 @@ def start_transfer(context, timeout_secs, read_file_handle, max_data_size, write_thread.stop() # Log and raise the exception. - LOG.exception(exc) - raise exception.CinderException(exc) + LOG.exception(_("Error occurred during image transfer.")) + if isinstance(exc, error_util.ImageTransferException): + raise + raise error_util.ImageTransferException(exc) finally: timer.cancel() # No matter what, try closing the read and write handles, if it so -- 2.45.2