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

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'])

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'})
 
 
 class HostAPI(base.Base):

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:
-            LOG.info(_("volume %s: extending"), volume_ref['name'])
+            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
-            self.driver.extend_volume(volume_ref, new_size)
+        except exception.OverQuota as exc:
-            LOG.info(_("volume %s: extended successfully"), volume_ref['name'])
+            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['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'],
+            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'})