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

diff --git a/cinder/tests/test_vmware_io_util.py b/cinder/tests/test_vmware_io_util.py
index 6c13291f21a9c0aff6df1f8073e2872036e41363..f77fec0d6bb96561c2093560c26a4d54cddfc049 100644 (file)
--- 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 cce8cfd7686c6dab1dfbd533460ff844240ef247..8df802a3c70f82cc25c2c43035631ab4c0dec849 100644 (file)
--- 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 514f45fe2015fa35a8cb325e9f39da1eca9d6bc9..a0471954b84512e265b2da00a760e3413189f0f8 100644 (file)
--- 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 3b47e28938b20187b2ec987afb4673df2778b45c..211cf0921441d80355f243aa6db148fc3e6a426f 100644 (file)
--- 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