openstack/nova
Code Issues Proposed changes

Merge "Avoid deleting allocations for instances being built"

Browse Source
This commit is contained in:
Zuul
2017-11-06 05:32:51 +00:00
committed by Gerrit Code Review
parent cab83d9d64 d176175db4
commit 570f616d92
2 changed files with 75 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
nova/compute/resource_tracker.py
View File

@@ -1152,6 +1152,7 @@ class ResourceTracker(object):
known_instances = set(self.tracked_instances.keys())
allocations = self.reportclient.get_allocations_for_resource_provider(
cn.uuid) or {}
read_deleted_context = context.elevated(read_deleted='yes')
for consumer_uuid, alloc in allocations.items():
if consumer_uuid in known_instances:
LOG.debug("Instance %s actively managed on this compute host "
@@ -1167,9 +1168,23 @@ class ResourceTracker(object):
# We know these are instances now, so proceed
instance_uuid = consumer_uuid
try:
instance = objects.Instance.get_by_uuid(context, instance_uuid,
instance = objects.Instance.get_by_uuid(read_deleted_context,
instance_uuid,
expected_attrs=[])
except exception.InstanceNotFound:
# The instance isn't even in the database. Either the scheduler
# _just_ created an allocation for it and we're racing with the
# creation in the cell database, or the instance was deleted
# and fully archived before we got a chance to run this. The
# former is far more likely than the latter. Avoid deleting
# allocations for a building instance here.
LOG.info("Instance %(uuid)s has allocations against this "
"compute host but is not found in the database.",
{'uuid': instance_uuid},
exc_info=False)
continue
if instance.deleted:
# The instance is gone, so we definitely want to remove
# allocations associated with it.
# NOTE(jaypipes): This will not be true if/when we support

134
nova/tests/unit/compute/test_resource_tracker.py
View File

@@ -107,6 +107,7 @@ _INSTANCE_TYPE_FIXTURES = {
'rxtx_factor': 0,
'vcpu_weight': 1,
'extra_specs': {},
'deleted': 0,
},
2: {
'id': 2,
@@ -120,6 +121,7 @@ _INSTANCE_TYPE_FIXTURES = {
'rxtx_factor': 0,
'vcpu_weight': 1,
'extra_specs': {},
'deleted': 0,
},
}
@@ -128,11 +130,11 @@ _INSTANCE_TYPE_OBJ_FIXTURES = {
1: objects.Flavor(id=1, flavorid='fakeid-1', name='fake1.small',
memory_mb=128, vcpus=1, root_gb=1,
ephemeral_gb=0, swap=0, rxtx_factor=0,
vcpu_weight=1, extra_specs={}),
vcpu_weight=1, extra_specs={}, deleted=False),
2: objects.Flavor(id=2, flavorid='fakeid-2', name='fake1.medium',
memory_mb=256, vcpus=2, root_gb=5,
ephemeral_gb=0, swap=0, rxtx_factor=0,
vcpu_weight=1, extra_specs={}),
vcpu_weight=1, extra_specs={}, deleted=False),
}
@@ -192,6 +194,7 @@ _INSTANCE_FIXTURES = [
flavor = _INSTANCE_TYPE_OBJ_FIXTURES[1],
old_flavor = _INSTANCE_TYPE_OBJ_FIXTURES[1],
new_flavor = _INSTANCE_TYPE_OBJ_FIXTURES[1],
deleted = False,
),
objects.Instance(
id=2,
@@ -215,6 +218,7 @@ _INSTANCE_FIXTURES = [
flavor = _INSTANCE_TYPE_OBJ_FIXTURES[2],
old_flavor = _INSTANCE_TYPE_OBJ_FIXTURES[2],
new_flavor = _INSTANCE_TYPE_OBJ_FIXTURES[2],
deleted = False,
),
]
@@ -477,7 +481,7 @@ class TestUpdateAvailableResources(BaseTestCase):
# parameter that update_available_resource() eventually passes
# to _update().
with mock.patch.object(self.rt, '_update') as update_mock:
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
self.rt.update_available_resource(mock.MagicMock(), _NODENAME)
return update_mock
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
@@ -533,16 +537,16 @@ class TestUpdateAvailableResources(BaseTestCase):
vd = self.driver_mock
vd.get_available_resource.assert_called_once_with(_NODENAME)
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
get_mock.assert_called_once_with(mock.ANY, _HOSTNAME,
_NODENAME,
expected_attrs=[
'system_metadata',
'numa_topology',
'flavor',
'migration_context'])
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME,
_NODENAME)
migr_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
migr_mock.assert_called_once_with(mock.ANY, _HOSTNAME,
_NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
@@ -584,8 +588,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 5, # 6GB avail - 1 GB reserved
@@ -632,8 +635,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 5, # 6 - 1 used
@@ -694,8 +696,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 6,
@@ -759,8 +760,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 5,
@@ -819,8 +819,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 1,
@@ -877,8 +876,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
'free_disk_gb': 1,
@@ -943,8 +941,7 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources()
get_cn_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME)
expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
vals = {
# 6 total - 1G existing - 5G new flav - 1G old flav
@@ -1741,7 +1738,7 @@ class TestResize(BaseTestCase):
mig_context_obj = _MIGRATION_CONTEXT_FIXTURES[instance.uuid]
instance.migration_context = mig_context_obj
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
self.rt.update_available_resource(mock.MagicMock(), _NODENAME)
migration = objects.Migration(
id=3,
@@ -1843,7 +1840,7 @@ class TestResize(BaseTestCase):
ctx = mock.MagicMock()
# Init compute node
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
self.rt.update_available_resource(mock.MagicMock(), _NODENAME)
expected = self.rt.compute_nodes[_NODENAME].obj_clone()
instance = _INSTANCE_FIXTURES[0].obj_clone()
@@ -1959,7 +1956,7 @@ class TestResize(BaseTestCase):
migr_mock.return_value = []
get_cn_mock.return_value = _COMPUTE_NODE_FIXTURES[0]
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
self.rt.update_available_resource(mock.MagicMock(), _NODENAME)
instance = _INSTANCE_FIXTURES[0].obj_clone()
instance.task_state = task_states.RESIZE_MIGRATING
@@ -2086,7 +2083,7 @@ class TestResize(BaseTestCase):
migr_mock.return_value = []
get_cn_mock.return_value = _COMPUTE_NODE_FIXTURES[0].obj_clone()
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
self.rt.update_available_resource(mock.MagicMock(), _NODENAME)
# Instance #1 is resizing to instance type 2 which has 2 vCPUs, 256MB
# RAM and 5GB root disk.
@@ -2222,7 +2219,8 @@ class TestRebuild(BaseTestCase):
migr_mock.return_value = []
get_cn_mock.return_value = _COMPUTE_NODE_FIXTURES[0].obj_clone()
self.rt.update_available_resource(mock.sentinel.ctx, _NODENAME)
ctx = mock.MagicMock()
self.rt.update_available_resource(ctx, _NODENAME)
# Now emulate the evacuate command by calling rebuild_claim() on the
# resource tracker as the compute manager does, supplying a Migration
@@ -2476,16 +2474,40 @@ class TestUpdateUsageFromInstance(BaseTestCase):
rc.get_allocations_for_resource_provider = mock.MagicMock(
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
mock_inst_get.side_effect = exc.InstanceNotFound(
instance_id=uuids.deleted)
mock_inst_get.return_value = objects.Instance(
uuid=uuids.deleted, deleted=True)
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.sentinel.ctx
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# Only one call should be made to delete allocations, and that should
# be for the first instance created above
rc.delete_allocation_for_instance.assert_called_once_with(
uuids.deleted)
mock_inst_get.assert_called_once_with(
ctx.elevated.return_value,
uuids.deleted,
expected_attrs=[])
ctx.elevated.assert_called_once_with(read_deleted='yes')
@mock.patch('nova.objects.Instance.get_by_uuid')
def test_remove_deleted_instances_allocations_building_instance(self,
mock_inst_get):
rc = self.rt.reportclient
self.rt.tracked_instances = {}
allocs = {uuids.deleted: "fake_deleted_instance"}
rc.get_allocations_for_resource_provider = mock.MagicMock(
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
mock_inst_get.side_effect = exc.InstanceNotFound(
instance_id=uuids.deleted)
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# Instance wasn't found in the database at all, so the allocation
# should not have been deleted
self.assertFalse(rc.delete_allocation_for_instance.called)
@mock.patch('nova.objects.Instance.get_by_uuid')
def test_remove_deleted_instances_allocations_ignores_migrations(self,
@@ -2498,10 +2520,10 @@ class TestUpdateUsageFromInstance(BaseTestCase):
rc.get_allocations_for_resource_provider = mock.MagicMock(
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
mock_inst_get.side_effect = exc.InstanceNotFound(
instance_id=uuids.deleted)
mock_inst_get.return_value = objects.Instance(
uuid=uuids.deleted, deleted=True)
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.sentinel.ctx
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(
ctx, cn, [mig])
@@ -2528,7 +2550,7 @@ class TestUpdateUsageFromInstance(BaseTestCase):
mock_inst_get.side_effect = get_by_uuid
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.sentinel.ctx
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# Scheduled instances should not have their allocations removed
@@ -2557,48 +2579,10 @@ class TestUpdateUsageFromInstance(BaseTestCase):
mock_get.return_value = instance
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.sentinel.ctx
ctx = mock.MagicMock()
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
mock_delete_allocs.assert_not_called()
@mock.patch('nova.objects.Instance.get_by_uuid')
def test_remove_deleted_instances_allocations_no_instance(self,
mock_inst_get):
# If for some reason an instance is no longer available, but
# there are allocations for it, we want to be sure those
# allocations are removed, not that an InstanceNotFound
# exception is not caught. Here we set up some allocations,
# one of which is for an instance that can no longer be
# found.
rc = self.rt.reportclient
self.rt.tracked_instances = {}
# Create 1 instance
instance = _INSTANCE_FIXTURES[0].obj_clone()
instance.uuid = uuids.inst0
# Mock out the allocation call
allocs = {uuids.scheduled: "fake_scheduled_instance",
uuids.inst0: "fake_instance_gone"}
rc.get_allocations_for_resource_provider = mock.MagicMock(
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
def get_by_uuid(ctx, inst_uuid, expected_attrs=None):
ret = _INSTANCE_FIXTURES[0].obj_clone()
ret.uuid = inst_uuid
if inst_uuid == uuids.scheduled:
ret.host = None
return ret
raise exc.InstanceNotFound(instance_id=inst_uuid)
mock_inst_get.side_effect = get_by_uuid
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.sentinel.ctx
# Call the method.
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# One call should be made to delete allocations, for our
# instance that no longer exists.
rc.delete_allocation_for_instance.assert_called_once_with(uuids.inst0)
def test_remove_deleted_instances_allocations_known_instance(self):
"""Tests the case that actively tracked instances for the
given node do not have their allocations removed.
@@ -2619,9 +2603,9 @@ class TestUpdateUsageFromInstance(BaseTestCase):
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(mock.sentinel.ctx, cn,
[])
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# We don't delete the allocation because the node is tracking the
# instance and has allocations for it.
rc.delete_allocation_for_instance.assert_not_called()
@@ -2653,9 +2637,9 @@ class TestUpdateUsageFromInstance(BaseTestCase):
return_value=allocs)
rc.delete_allocation_for_instance = mock.MagicMock()
cn = self.rt.compute_nodes[_NODENAME]
ctx = mock.MagicMock()
# Call the method.
self.rt._remove_deleted_instances_allocations(mock.sentinel.ctx, cn,
[])
self.rt._remove_deleted_instances_allocations(ctx, cn, [])
# We don't delete the allocation because we're not sure what to do.
# NOTE(mriedem): This is not actually the behavior we want. This is
# testing the current behavior but in the future when we get smart