openstack
/
nova
Code Issues Proposed changes

Decrement quota usage when deleting an instance in cell0 

When we fail to schedule an instance, e.g. there are no hosts
available, conductor creates the instance in the cell0 database
and deletes the build request. At this point quota usage
has been incremented in the main 'nova' database.

When the instance is deleted, the build request is already gone
so _delete_while_booting returns False and we lookup the instance
in cell0 and delete it from there, but that flow wasn't decrementing
quota usage like _delete_while_booting was.

This change adds the same quota usage decrement handling that
_delete_while_booting performs.

Change-Id: I4cb0169ce0de537804ab9129bc671d75ce5f7953
Partial-Bug: #1670627
This commit is contained in:
Matt Riedemann 2017-03-08 21:51:07 -05:00
parent c3d6739ba5
commit 018068c4ca
2 changed files with 105 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
nova/compute/api.py
View File

@ -1843,11 +1843,24 @@ class API(base.Base):
# acceptable to skip the rest of the delete processing.
cell, instance = self._lookup_instance(context, instance.uuid)
if cell and instance:
with nova_context.target_cell(context, cell):
with compute_utils.notify_about_instance_delete(
self.notifier, context, instance):
instance.destroy()
return
# Conductor may have buried the instance in cell0 but
# quotas must still be decremented in the main cell DB.
project_id, user_id = quotas_obj.ids_from_instance(
context, instance)
# This is confusing but actually decrements quota usage.
quotas = self._create_reservations(context,
instance,
instance.task_state,
project_id, user_id)
quotas.commit()
try:
with nova_context.target_cell(context, cell):
with compute_utils.notify_about_instance_delete(
self.notifier, context, instance):
instance.destroy()
return
except exception.InstanceNotFound:
quotas.rollback()
if not instance:
# Instance is already deleted.
return

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

@ -1487,6 +1487,93 @@ class _ComputeAPIUnitTestMixIn(object):
test()
@mock.patch('nova.context.target_cell')
@mock.patch('nova.compute.utils.notify_about_instance_delete')
@mock.patch('nova.objects.Instance.destroy')
def test_delete_instance_from_cell0(self, destroy_mock, notify_mock,
target_cell_mock):
"""Tests the case that the instance does not have a host and was not
deleted while building, so conductor put it into cell0 so the API has
to delete the instance from cell0 and decrement the quota from the
main cell.
"""
instance = self._create_instance_obj({'host': None})
cell0 = objects.CellMapping(uuid=objects.CellMapping.CELL0_UUID)
quota_mock = mock.MagicMock()
with test.nested(
mock.patch.object(self.compute_api, '_delete_while_booting',
return_value=False),
mock.patch.object(self.compute_api, '_lookup_instance',
return_value=(cell0, instance)),
mock.patch.object(self.compute_api, '_create_reservations',
return_value=quota_mock)
) as (
_delete_while_booting, _lookup_instance, _create_reservations
):
self.compute_api._delete(
self.context, instance, 'delete', mock.NonCallableMock())
_delete_while_booting.assert_called_once_with(
self.context, instance)
_lookup_instance.assert_called_once_with(
self.context, instance.uuid)
_create_reservations.assert_called_once_with(
self.context, instance, instance.task_state,
self.context.project_id, instance.user_id)
quota_mock.commit.assert_called_once_with()
target_cell_mock.assert_called_once_with(self.context, cell0)
notify_mock.assert_called_once_with(
self.compute_api.notifier, self.context, instance)
destroy_mock.assert_called_once_with()
@mock.patch('nova.context.target_cell')
@mock.patch('nova.compute.utils.notify_about_instance_delete')
@mock.patch('nova.objects.Instance.destroy')
@mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid',
# This just lets us exit the test early.
side_effect=test.TestingException)
def test_delete_instance_from_cell0_rollback_quota(
self, bdms_get_by_instance_uuid, destroy_mock, notify_mock,
target_cell_mock):
"""Tests the case that the instance does not have a host and was not
deleted while building, so conductor put it into cell0 so the API has
to delete the instance from cell0 and decrement the quota from the
main cell. When we go to delete the instance, it's already gone so we
rollback the quota change.
"""
instance = self._create_instance_obj({'host': None})
cell0 = objects.CellMapping(uuid=objects.CellMapping.CELL0_UUID)
quota_mock = mock.MagicMock()
destroy_mock.side_effect = exception.InstanceNotFound(
instance_id=instance.uuid)
with test.nested(
mock.patch.object(self.compute_api, '_delete_while_booting',
return_value=False),
mock.patch.object(self.compute_api, '_lookup_instance',
return_value=(cell0, instance)),
mock.patch.object(self.compute_api, '_create_reservations',
return_value=quota_mock)
) as (
_delete_while_booting, _lookup_instance, _create_reservations
):
self.assertRaises(
test.TestingException, self.compute_api._delete,
self.context, instance, 'delete', mock.NonCallableMock())
_delete_while_booting.assert_called_once_with(
self.context, instance)
_lookup_instance.assert_called_once_with(
self.context, instance.uuid)
_create_reservations.assert_called_once_with(
self.context, instance, instance.task_state,
self.context.project_id, instance.user_id)
quota_mock.commit.assert_called_once_with()
target_cell_mock.assert_called_once_with(self.context, cell0)
notify_mock.assert_called_once_with(
self.compute_api.notifier, self.context, instance)
destroy_mock.assert_called_once_with()
quota_mock.rollback.assert_called_once_with()
@mock.patch.object(context, 'target_cell')
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid',
side_effect=exception.InstanceMappingNotFound(