openstack
/
nova
Code Issues Proposed changes

Commit usage decrement after destroying instance 

This fixes a regression in Ocata where we were always
decrementing quota usage during instance delete even
if we failed to delete the instance. Now the reservation
is properly committed after the instance is destroyed.

The related functional test is updated to show this working
correctly now.

Conflicts:
      nova/compute/api.py
      nova/tests/unit/compute/test_compute_api.py

NOTE(mriedem): The conflict is due to not having change
edf51119fa in stable/ocata.

Change-Id: I5200e72c195e12b5a069cbae3f209492ed569fb4
Closes-Bug: #1678326
(cherry picked from commit 5c1af55cbe)
This commit is contained in:
Matt Riedemann 2017-03-31 21:08:55 -04:00
parent 01fa555b36
commit 361b883831
3 changed files with 10 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
nova/compute/api.py
View File

@ -1723,8 +1723,6 @@ class API(base.Base):
instance.task_state,
project_id, user_id)
try:
quotas.commit()
# NOTE(alaski): Though the conductor halts the build process it
# does not currently delete the instance record. This is
# because in the near future the instance record will not be
@ -1745,6 +1743,7 @@ class API(base.Base):
instance.destroy()
else:
instance.destroy()
quotas.commit()
except exception.InstanceNotFound:
quotas.rollback()
@ -1805,17 +1804,17 @@ class API(base.Base):
instance.task_state,
project_id, user_id,
flavor=quota_flavor)
quotas.commit()
try:
with nova_context.target_cell(context, cell):
with compute_utils.notify_about_instance_delete(
self.notifier, context, instance):
instance.destroy()
return
# Now commit the quota reservation to decrement usage.
quotas.commit()
except exception.InstanceNotFound:
quotas.rollback()
# Instance is already deleted.
return
# The instance was deleted or is already gone.
return
if not instance:
# Instance is already deleted.
return

9
nova/tests/functional/regressions/test_bug_1678326.py
View File

@ -62,9 +62,7 @@ class TestDeleteFromCell0CheckQuotaRollback(
current_usage['absolute']['totalInstancesUsed'])
# Now we stub out instance.destroy to fail with InstanceNotFound
# which triggers the quotas.rollback call, which until the bug is
# fixed actually does nothing because we have already committed the
# reservation before we actually tried to destroy the instance.
# which triggers the quotas.rollback call.
original_instance_destroy = objects.Instance.destroy
def fake_instance_destroy(*args, **kwargs):
@ -79,8 +77,7 @@ class TestDeleteFromCell0CheckQuotaRollback(
self.stub_out('nova.objects.instance.Instance.destroy',
original_instance_destroy)
# Now check the quota again. Until the bug is fixed, quota usage will
# have been decremented even though we failed to delete the instance.
# Now check the quota again. We should be back to the pre-delete usage.
ending_usage = self.api.get_limits()
self.assertEqual(current_usage['absolute']['totalInstancesUsed'] - 1,
self.assertEqual(current_usage['absolute']['totalInstancesUsed'],
ending_usage['absolute']['totalInstancesUsed'])

5
nova/tests/unit/compute/test_compute_api.py
View File

@ -1459,7 +1459,7 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertTrue(
self.compute_api._delete_while_booting(self.context,
inst))
self.assertTrue(quota_mock.commit.called)
self.assertFalse(quota_mock.commit.called)
test()
@ -1479,7 +1479,7 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertTrue(
self.compute_api._delete_while_booting(self.context,
inst))
self.assertTrue(quota_mock.commit.called)
self.assertFalse(quota_mock.commit.called)
self.assertTrue(quota_mock.rollback.called)
test()
@ -1593,7 +1593,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, instance, instance.task_state,
self.context.project_id, instance.user_id,
flavor=instance.flavor)
quota_mock.commit.assert_called_once_with()
expected_target_cell_calls = [
# Get the instance.flavor.
mock.call(self.context, cell0),