Fix extend_volume error handling.

If the async call to the manager/driver failed, the API still updated
the quota and volume size in the DB. Solution is to move these tasks
down to the manager, where we know if the extend succeeded.

Change-Id: I668fd659830bd6d410be64a1f5116377b08a9e96
Fixes: bug 1201814

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 32274011b2d500e9c2c163341833257feab37c21..7204749b10d497f10cc1c018ea314793ce0fb0e0 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -1228,7 +1228,7 @@ class VolumeTestCase(test.TestCase):
         self.assertEqual(snapshots[2].id, u'4')
 
     def test_extend_volume(self):
-        """Test volume can be extended."""
+        """Test volume can be extended at API level."""
         # create a volume and assign to host
         volume = self._create_volume(2)
         self.volume.create_volume(self.context, volume['id'])
@@ -1255,7 +1255,55 @@ class VolumeTestCase(test.TestCase):
         volume_api.extend(self.context, volume, 3)
 
         volume = db.volume_get(context.get_admin_context(), volume['id'])
-        self.assertEquals(volume['size'], 3)
+        self.assertEquals(volume['status'], 'extending')
+
+        # clean up
+        self.volume.delete_volume(self.context, volume['id'])
+
+    def test_extend_volume_manager(self):
+        """Test volume can be extended at the manager level."""
+        def fake_reserve(context, expire=None, project_id=None, **deltas):
+            return ['RESERVATION']
+
+        def fake_reserve_exc(context, expire=None, project_id=None, **deltas):
+            raise exception.OverQuota(overs=['gigabytes'],
+                                      quotas={'gigabytes': 20},
+                                      usages={'gigabytes': {'reserved': 5,
+                                                            'in_use': 15}})
+
+        def fake_extend_exc(volume, new_size):
+            raise exception.CinderException('fake exception')
+
+        volume = self._create_volume(2)
+        self.volume.create_volume(self.context, volume['id'])
+
+        # Test quota exceeded
+        self.stubs.Set(QUOTAS, 'reserve', fake_reserve_exc)
+        self.stubs.Set(QUOTAS, 'commit', lambda x, y, project_id=None: True)
+        self.stubs.Set(QUOTAS, 'rollback', lambda x, y: True)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 2)
+        self.assertEquals(volume['status'], 'error_extending')
+
+        # Test driver exception
+        self.stubs.Set(QUOTAS, 'reserve', fake_reserve)
+        self.stubs.Set(self.volume.driver, 'extend_volume', fake_extend_exc)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 2)
+        self.assertEquals(volume['status'], 'error_extending')
+
+        # Test driver success
+        self.stubs.Set(self.volume.driver, 'extend_volume',
+                       lambda x, y: True)
+        volume['status'] = 'extending'
+        self.volume.extend_volume(self.context, volume['id'], '4')
+        volume = db.volume_get(context.get_admin_context(), volume['id'])
+        self.assertEquals(volume['size'], 4)
+        self.assertEquals(volume['status'], 'available')
 
         # clean up
         self.volume.delete_volume(self.context, volume['id'])

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index fdb4169f70937b524c29aac05f9d58f314787d52..c00573113959e2a9bf0098a321bd3ce1b4c37f24 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -811,41 +811,9 @@ class API(base.Base):
                      "extended: %(new_size)s)") % {'new_size': new_size,
                                                    'size': volume['size']})
             raise exception.InvalidInput(reason=msg)
-        try:
-            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
-        except exception.OverQuota as exc:
-            overs = exc.kwargs['overs']
-            usages = exc.kwargs['usages']
-            quotas = exc.kwargs['quotas']
-
-            def _consumed(name):
-                return (usages[name]['reserved'] + usages[name]['in_use'])
-
-            if 'gigabytes' in overs:
-                msg = _("Quota exceeded for %(s_pid)s, "
-                        "tried to extend volume by "
-                        "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG "
-                        "already consumed)")
-                LOG.warn(msg % {'s_pid': context.project_id,
-                                's_size': size_increase,
-                                'd_consumed': _consumed('gigabytes'),
-                                'd_quota': quotas['gigabytes']})
-                raise exception.VolumeSizeExceedsAvailableQuota()
 
         self.update(context, volume, {'status': 'extending'})
-
-        try:
-            self.volume_rpcapi.extend_volume(context, volume, new_size)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                try:
-                    self.update(context, volume, {'status': 'error_extending'})
-                finally:
-                    QUOTAS.rollback(context, reservations)
-
-        self.update(context, volume, {'size': new_size})
-        QUOTAS.commit(context, reservations)
-        self.update(context, volume, {'status': 'available'})
+        self.volume_rpcapi.extend_volume(context, volume, new_size)
 
 
 class HostAPI(base.Base):

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 7c8500797a9df73d769bc281d47b74fa0bc1d5e9..d658b8d12628148efd2d2295ff68a38f5f583dcd 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -802,14 +802,46 @@ class VolumeManager(manager.SchedulerDependentManager):
             extra_usage_info=extra_usage_info, host=self.host)
 
     def extend_volume(self, context, volume_id, new_size):
-        volume_ref = self.db.volume_get(context, volume_id)
+        volume = self.db.volume_get(context, volume_id)
+        size_increase = (int(new_size)) - volume['size']
+
+        try:
+            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
+        except exception.OverQuota as exc:
+            self.db.volume_update(context, volume['id'],
+                                  {'status': 'error_extending'})
+            overs = exc.kwargs['overs']
+            usages = exc.kwargs['usages']
+            quotas = exc.kwargs['quotas']
+
+            def _consumed(name):
+                return (usages[name]['reserved'] + usages[name]['in_use'])
+
+            if 'gigabytes' in overs:
+                msg = _("Quota exceeded for %(s_pid)s, "
+                        "tried to extend volume by "
+                        "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG "
+                        "already consumed)")
+                LOG.error(msg % {'s_pid': context.project_id,
+                                 's_size': size_increase,
+                                 'd_consumed': _consumed('gigabytes'),
+                                 'd_quota': quotas['gigabytes']})
+            return
 
         try:
-            LOG.info(_("volume %s: extending"), volume_ref['name'])
-            self.driver.extend_volume(volume_ref, new_size)
-            LOG.info(_("volume %s: extended successfully"), volume_ref['name'])
+            LOG.info(_("volume %s: extending"), volume['name'])
+            self.driver.extend_volume(volume, new_size)
+            LOG.info(_("volume %s: extended successfully"), volume['name'])
         except Exception:
             LOG.exception(_("volume %s: Error trying to extend volume"),
                           volume_id)
-            self.db.volume_update(context, volume_ref['id'],
-                                  {'status': 'error_extending'})
+            try:
+                self.db.volume_update(context, volume['id'],
+                                      {'status': 'error_extending'})
+            finally:
+                QUOTAS.rollback(context, reservations)
+                return
+
+        QUOTAS.commit(context, reservations)
+        self.db.volume_update(context, volume['id'], {'size': int(new_size),
+                                                      'status': 'available'})