Merge "Decrement quota usage when deleting an instance in cell0" into stable/ocata
This commit is contained in:
Jenkins
Gerrit Code Review
commit
bf12a7146b
|
|
@ -1788,11 +1788,32 @@ 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:
|
||||
# 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)
|
||||
|
||||
# We have to get the flavor from the instance while the
|
||||
# context is still targeted to where the instance lives.
|
||||
with nova_context.target_cell(context, cell):
|
||||
with compute_utils.notify_about_instance_delete(
|
||||
self.notifier, context, instance):
|
||||
instance.destroy()
|
||||
return
|
||||
quota_flavor = self._get_flavor_for_reservation(
|
||||
instance)
|
||||
|
||||
# This is confusing but actually decrements quota usage.
|
||||
quotas = self._create_reservations(context,
|
||||
instance,
|
||||
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
|
||||
except exception.InstanceNotFound:
|
||||
quotas.rollback()
|
||||
if not instance:
|
||||
# Instance is already deleted.
|
||||
return
|
||||
|
|
@ -1977,21 +1998,33 @@ class API(base.Base):
|
|||
src_host, quotas.reservations,
|
||||
cast=False)
|
||||
|
||||
def _get_flavor_for_reservation(self, instance):
|
||||
"""Returns the flavor needed for _create_reservations.
|
||||
|
||||
This is used when the context is targeted to a cell that is
|
||||
different from the one that the instance lives in.
|
||||
"""
|
||||
if instance.task_state in (task_states.RESIZE_MIGRATED,
|
||||
task_states.RESIZE_FINISH):
|
||||
return instance.old_flavor
|
||||
return instance.flavor
|
||||
|
||||
def _create_reservations(self, context, instance, original_task_state,
|
||||
project_id, user_id):
|
||||
project_id, user_id, flavor=None):
|
||||
# NOTE(wangpan): if the instance is resizing, and the resources
|
||||
# are updated to new instance type, we should use
|
||||
# the old instance type to create reservation.
|
||||
# see https://bugs.launchpad.net/nova/+bug/1099729 for more details
|
||||
if original_task_state in (task_states.RESIZE_MIGRATED,
|
||||
task_states.RESIZE_FINISH):
|
||||
old_flavor = instance.old_flavor
|
||||
old_flavor = flavor or instance.old_flavor
|
||||
instance_vcpus = old_flavor.vcpus
|
||||
vram_mb = old_flavor.extra_specs.get('hw_video:ram_max_mb', 0)
|
||||
instance_memory_mb = old_flavor.memory_mb + vram_mb
|
||||
else:
|
||||
instance_vcpus = instance.flavor.vcpus
|
||||
instance_memory_mb = instance.flavor.memory_mb
|
||||
flavor = flavor or instance.flavor
|
||||
instance_vcpus = flavor.vcpus
|
||||
instance_memory_mb = flavor.memory_mb
|
||||
|
||||
quotas = objects.Quotas(context=context)
|
||||
quotas.reserve(project_id=project_id,
|
||||
|
|
|
|||
|
|
@ -135,9 +135,8 @@ class TestDeleteFromCell0CheckQuota(test.TestCase):
|
|||
self._delete_server(server['id'])
|
||||
self._wait_for_instance_delete(server['id'])
|
||||
|
||||
# Now check the quota again. Because we have not fixed the bug, the
|
||||
# quota is still going to be showing a usage for instances. When the
|
||||
# bug is fixed, ending usage should be current usage - 1.
|
||||
# Now check the quota again. Since the bug is fixed, ending usage
|
||||
# should be current usage - 1.
|
||||
ending_usage = self.api.get_limits()
|
||||
self.assertEqual(current_usage['absolute']['totalInstancesUsed'],
|
||||
self.assertEqual(current_usage['absolute']['totalInstancesUsed'] - 1,
|
||||
ending_usage['absolute']['totalInstancesUsed'])
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
import contextlib
|
||||
import datetime
|
||||
|
||||
import ddt
|
||||
import iso8601
|
||||
import mock
|
||||
from mox3 import mox
|
||||
|
|
@ -67,6 +68,7 @@ SHELVED_IMAGE_NOT_AUTHORIZED = 'fake-shelved-image-not-authorized'
|
|||
SHELVED_IMAGE_EXCEPTION = 'fake-shelved-image-exception'
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class _ComputeAPIUnitTestMixIn(object):
|
||||
def setUp(self):
|
||||
super(_ComputeAPIUnitTestMixIn, self).setUp()
|
||||
|
|
@ -1482,6 +1484,138 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
|
||||
test()
|
||||
|
||||
@ddt.data((task_states.RESIZE_MIGRATED, True),
|
||||
(task_states.RESIZE_FINISH, True),
|
||||
(None, False))
|
||||
@ddt.unpack
|
||||
def test_get_flavor_for_reservation(self, task_state, is_old):
|
||||
instance = self._create_instance_obj({'task_state': task_state})
|
||||
flavor = self.compute_api._get_flavor_for_reservation(instance)
|
||||
expected_flavor = instance.old_flavor if is_old else instance.flavor
|
||||
self.assertEqual(expected_flavor, flavor)
|
||||
|
||||
@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,
|
||||
'_get_flavor_for_reservation',
|
||||
return_value=instance.flavor),
|
||||
mock.patch.object(self.compute_api, '_create_reservations',
|
||||
return_value=quota_mock)
|
||||
) as (
|
||||
_delete_while_booting, _lookup_instance,
|
||||
_get_flavor_for_reservation, _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)
|
||||
_get_flavor_for_reservation.assert_called_once_with(instance)
|
||||
_create_reservations.assert_called_once_with(
|
||||
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),
|
||||
mock.call().__enter__(),
|
||||
mock.call().__exit__(None, None, None),
|
||||
# Destroy the instance.
|
||||
mock.call(self.context, cell0),
|
||||
mock.call().__enter__(),
|
||||
mock.call().__exit__(None, None, None),
|
||||
]
|
||||
target_cell_mock.assert_has_calls(expected_target_cell_calls)
|
||||
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,
|
||||
'_get_flavor_for_reservation',
|
||||
return_value=instance.flavor),
|
||||
mock.patch.object(self.compute_api, '_create_reservations',
|
||||
return_value=quota_mock)
|
||||
) as (
|
||||
_delete_while_booting, _lookup_instance,
|
||||
_get_flavor_for_reservation, _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)
|
||||
_get_flavor_for_reservation.assert_called_once_with(instance)
|
||||
_create_reservations.assert_called_once_with(
|
||||
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),
|
||||
mock.call().__enter__(),
|
||||
mock.call().__exit__(None, None, None),
|
||||
# Destroy the instance.
|
||||
mock.call(self.context, cell0),
|
||||
mock.call().__enter__(),
|
||||
mock.call().__exit__(
|
||||
exception.InstanceNotFound,
|
||||
destroy_mock.side_effect,
|
||||
mock.ANY),
|
||||
]
|
||||
target_cell_mock.assert_has_calls(expected_target_cell_calls)
|
||||
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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue