Merge "Fix quota updating during soft delete and restore"

3 files changed, 104 insertions, 67 deletions

diff --git a/nova/api/openstack/compute/contrib/deferred_delete.py b/nova/api/openstack/compute/contrib/deferred_delete.py
index 8eaea04bb..ea7ac00f9 100644
--- a/nova/api/openstack/compute/contrib/deferred_delete.py
+++ b/nova/api/openstack/compute/contrib/deferred_delete.py
@@ -42,6 +42,9 @@ class DeferredDeleteController(wsgi.Controller):
         instance = self.compute_api.get(context, id)
         try:
             self.compute_api.restore(context, instance)
+        except exception.QuotaError as error:
+            raise exc.HTTPRequestEntityTooLarge(explanation=unicode(error),
+                                                headers={'Retry-After': 0})
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'restore')
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 858e55070..a947dab1f 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -825,50 +825,29 @@ class API(base.Base):
 
         return dict(old_ref.iteritems()), dict(instance_ref.iteritems())
 
-    @wrap_check_policy
-    @check_instance_lock
-    @check_instance_state(vm_state=None, task_state=None)
-    def soft_delete(self, context, instance):
-        """Terminate an instance."""
-        LOG.debug(_('Going to try to soft delete instance'),
-                  instance=instance)
-
+    def _delete(self, context, instance, cb, **instance_attrs):
         if instance['disable_terminate']:
+            LOG.info(_('instance termination disabled'),
+                     instance=instance)
             return
 
-        # NOTE(jerdfelt): The compute daemon handles reclaiming instances
-        # that are in soft delete. If there is no host assigned, there is
-        # no daemon to reclaim, so delete it immediately.
-        if instance['host']:
-            instance = self.update(context, instance,
-                                   task_state=task_states.SOFT_DELETING,
-                                   expected_task_state=None,
-                                   deleted_at=timeutils.utcnow())
-
-            self.compute_rpcapi.soft_delete_instance(context, instance)
-        else:
-            LOG.warning(_('No host for instance, deleting immediately'),
-                        instance=instance)
-            try:
-                self.db.instance_destroy(context, instance['uuid'])
-            except exception.InstanceNotFound:
-                # NOTE(comstud): Race condition.  Instance already gone.
-                pass
-
-    def _delete(self, context, instance):
         host = instance['host']
+        bdms = self.db.block_device_mapping_get_all_by_instance(
+                    context, instance['uuid'])
         reservations = None
         try:
-
-            #Note(maoy): no expected_task_state needs to be set
+            # NOTE(maoy): no expected_task_state needs to be set
+            attrs = {'progress': 0}
+            attrs.update(instance_attrs)
             old, updated = self._update(context,
                                         instance,
-                                        task_state=task_states.DELETING,
-                                        progress=0)
+                                        **attrs)
 
             # Avoid double-counting the quota usage reduction
             # where delete is already in progress
-            if old['task_state'] != task_states.DELETING:
+            if (old['vm_state'] != vm_states.SOFT_DELETED and
+                old['task_state'] not in (task_states.DELETING,
+                                          task_states.SOFT_DELETING)):
                 reservations = QUOTAS.reserve(context,
                                               instances=-1,
                                               cores=-instance['vcpus'],
@@ -878,12 +857,11 @@ class API(base.Base):
                 # Just update database, nothing else we can do
                 constraint = self.db.constraint(host=self.db.equal_any(host))
                 try:
-                    result = self.db.instance_destroy(context,
-                                                      instance['uuid'],
-                                                      constraint)
+                    self.db.instance_destroy(context, instance['uuid'],
+                                             constraint)
                     if reservations:
                         QUOTAS.commit(context, reservations)
-                    return result
+                    return
                 except exception.ConstraintNotMet:
                     # Refresh to get new host information
                     instance = self.get(context, instance['uuid'])
@@ -918,9 +896,7 @@ class API(base.Base):
                             reservations=downsize_reservations)
 
             is_up = False
-            bdms = self.db.block_device_mapping_get_all_by_instance(
-                context, instance["uuid"])
-            #Note(jogo): db allows for multiple compute services per host
+            # NOTE(jogo): db allows for multiple compute services per host
             try:
                 services = self.db.service_get_all_compute_by_host(
                         context.elevated(), instance['host'])
@@ -929,8 +905,7 @@ class API(base.Base):
             for service in services:
                 if utils.service_is_up(service):
                     is_up = True
-                    self.compute_rpcapi.terminate_instance(context, instance,
-                                                           bdms)
+                    cb(context, instance, bdms)
                     break
             if not is_up:
                 # If compute node isn't up, just delete from DB
@@ -989,40 +964,69 @@ class API(base.Base):
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=None, task_state=None)
-    def delete(self, context, instance):
+    def soft_delete(self, context, instance):
         """Terminate an instance."""
-        LOG.debug(_("Going to try to terminate instance"), instance=instance)
+        LOG.debug(_('Going to try to soft delete instance'),
+                  instance=instance)
 
-        if instance['disable_terminate']:
-            return
+        def soft_delete(context, instance, bdms):
+            self.compute_rpcapi.soft_delete_instance(context, instance)
+
+        self._delete(context, instance, soft_delete,
+                     task_state=task_states.SOFT_DELETING,
+                     deleted_at=timeutils.utcnow())
 
-        self._delete(context, instance)
+    def _delete_instance(self, context, instance):
+        def terminate(context, instance, bdms):
+            self.compute_rpcapi.terminate_instance(context, instance, bdms)
+
+        self._delete(context, instance, terminate,
+                     task_state=task_states.DELETING)
+
+    @wrap_check_policy
+    @check_instance_lock
+    @check_instance_state(vm_state=None, task_state=None)
+    def delete(self, context, instance):
+        """Terminate an instance."""
+        LOG.debug(_("Going to try to terminate instance"), instance=instance)
+        self._delete_instance(context, instance)
 
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.SOFT_DELETED])
     def restore(self, context, instance):
         """Restore a previously deleted (but not reclaimed) instance."""
-        if instance['host']:
-            instance = self.update(context, instance,
-                        task_state=task_states.RESTORING,
-                        expected_task_state=None,
-                        deleted_at=None)
-            self.compute_rpcapi.restore_instance(context, instance)
-        else:
-            self.update(context,
-                        instance,
-                        vm_state=vm_states.ACTIVE,
-                        task_state=None,
-                        expected_task_state=None,
-                        deleted_at=None)
+        # Reserve quotas
+        instance_type = instance['instance_type']
+        num_instances, quota_reservations = self._check_num_instances_quota(
+                context, instance_type, 1, 1)
+
+        try:
+            if instance['host']:
+                instance = self.update(context, instance,
+                            task_state=task_states.RESTORING,
+                            expected_task_state=None,
+                            deleted_at=None)
+                self.compute_rpcapi.restore_instance(context, instance)
+            else:
+                self.update(context,
+                            instance,
+                            vm_state=vm_states.ACTIVE,
+                            task_state=None,
+                            expected_task_state=None,
+                            deleted_at=None)
+
+            QUOTAS.commit(context, quota_reservations)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                QUOTAS.rollback(context, quota_reservations)
 
     @wrap_check_policy
     @check_instance_lock
     @check_instance_state(vm_state=[vm_states.SOFT_DELETED])
     def force_delete(self, context, instance):
         """Force delete a previously deleted (but not reclaimed) instance."""
-        self._delete(context, instance)
+        self._delete_instance(context, instance)
 
     @wrap_check_policy
     @check_instance_lock
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 0e7148a9a..4370888bf 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -3263,7 +3263,12 @@ class ComputeAPITestCase(BaseTestCase):
         db.instance_destroy(self.context, instance['uuid'])
 
     def test_delete_soft(self):
-        instance, instance_uuid = self._run_instance()
+        instance, instance_uuid = self._run_instance(params={
+                'host': FLAGS.host})
+
+        self.mox.StubOutWithMock(nova.quota.QUOTAS, 'commit')
+        nova.quota.QUOTAS.commit(mox.IgnoreArg(), mox.IgnoreArg())
+        self.mox.ReplayAll()
 
         self.compute_api.soft_delete(self.context, instance)
 
@@ -3273,10 +3278,11 @@ class ComputeAPITestCase(BaseTestCase):
         db.instance_destroy(self.context, instance['uuid'])
 
     def test_delete_soft_fail(self):
-        instance, instance_uuid = self._run_instance()
-
+        instance, instance_uuid = self._run_instance(params={
+                'host': FLAGS.host})
         instance = db.instance_update(self.context, instance_uuid,
                                       {'disable_terminate': True})
+
         self.compute_api.soft_delete(self.context, instance)
 
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
@@ -3284,6 +3290,27 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_delete_soft_rollback(self):
+        instance, instance_uuid = self._run_instance(params={
+                'host': FLAGS.host})
+
+        self.mox.StubOutWithMock(nova.quota.QUOTAS, 'rollback')
+        nova.quota.QUOTAS.rollback(mox.IgnoreArg(), mox.IgnoreArg())
+        self.mox.ReplayAll()
+
+        def fail(*args, **kwargs):
+            raise test.TestingException()
+        self.stubs.Set(self.compute_api.compute_rpcapi, 'soft_delete_instance',
+                       fail)
+
+        self.assertRaises(test.TestingException, self.compute_api.soft_delete,
+                          self.context, instance)
+
+        instance = db.instance_get_by_uuid(self.context, instance_uuid)
+        self.assertEqual(instance['task_state'], task_states.SOFT_DELETING)
+
+        db.instance_destroy(self.context, instance['uuid'])
+
     def test_force_delete(self):
         """Ensure instance can be deleted after a soft delete"""
         instance = jsonutils.to_primitive(self._create_fake_instance(params={
@@ -3378,9 +3405,8 @@ class ComputeAPITestCase(BaseTestCase):
 
     def test_restore(self):
         """Ensure instance can be restored from a soft delete"""
-        instance = jsonutils.to_primitive(self._create_fake_instance())
-        instance_uuid = instance['uuid']
-        self.compute.run_instance(self.context, instance=instance)
+        instance, instance_uuid = self._run_instance(params={
+                'host': FLAGS.host})
 
         instance = db.instance_get_by_uuid(self.context, instance_uuid)
         self.compute_api.soft_delete(self.context, instance)
@@ -3393,6 +3419,10 @@ class ComputeAPITestCase(BaseTestCase):
                                       {'vm_state': vm_states.SOFT_DELETED,
                                        'task_state': None})
 
+        self.mox.StubOutWithMock(nova.quota.QUOTAS, 'commit')
+        nova.quota.QUOTAS.commit(mox.IgnoreArg(), mox.IgnoreArg())
+        self.mox.ReplayAll()
+
         self.compute_api.restore(self.context, instance)
 
         instance = db.instance_get_by_uuid(self.context, instance_uuid)