From 06584dc10f59fc7d4fc953fa93f7b4bb6ffc5123 Mon Sep 17 00:00:00 2001 From: jichenjc Date: Fri, 18 Nov 2016 18:02:11 +0800 Subject: [PATCH] Catch InstanceNotFound exception during simple_tenant_usage collection, there is possibility that instance got delete, so we need to catch the exception because instance.flavor might cause a instance.get_by_uuid call. commit b52876f912156be3f55d1c78898f9050ec498b5a mentioned so this patch didn't use the flavor object directly Backport note: since the original fix did not include tests, this backport squashes 14eeaf1c5203ea2b843a44a7130b76938a396e8e into itself. Change-Id: Icbfd1a4b97873d44bafd16754d09fff7a9d2b7af Closes-Bug: 1643444 Closes-Bug: 1692893 (cherry picked from commit 83e6dbf5815e2b0920e3afa404efbc46c09077e9) --- .../openstack/compute/simple_tenant_usage.py | 16 +++-- .../compute/test_simple_tenant_usage.py | 66 ++++++++++++++++++- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/nova/api/openstack/compute/simple_tenant_usage.py b/nova/api/openstack/compute/simple_tenant_usage.py index 27f7084c3c45..97a9dba7e211 100644 --- a/nova/api/openstack/compute/simple_tenant_usage.py +++ b/nova/api/openstack/compute/simple_tenant_usage.py @@ -119,14 +119,18 @@ class SimpleTenantUsageController(wsgi.Controller): info['instance_id'] = instance.uuid info['name'] = instance.display_name - - info['memory_mb'] = instance.flavor.memory_mb - info['local_gb'] = (instance.flavor.root_gb + - instance.flavor.ephemeral_gb) - info['vcpus'] = instance.flavor.vcpus - info['tenant_id'] = instance.project_id + try: + info['memory_mb'] = instance.flavor.memory_mb + info['local_gb'] = (instance.flavor.root_gb + + instance.flavor.ephemeral_gb) + info['vcpus'] = instance.flavor.vcpus + except exception.InstanceNotFound: + # This is rare case, instance disappear during analysis + # As it's just info collection, we can try next one + continue + # NOTE(mriedem): We need to normalize the start/end times back # to timezone-naive so the response doesn't change after the # conversion to objects. diff --git a/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py b/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py index 4b4373087960..cd3083147922 100644 --- a/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py +++ b/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py @@ -85,6 +85,48 @@ def _fake_instance(start, end, instance_id, tenant_id, flavor=flavor) +def _fake_instance_deleted_flavorless(context, start, end, instance_id, + tenant_id, vm_state=vm_states.ACTIVE): + return objects.Instance( + context=context, + deleted=instance_id, + id=instance_id, + uuid=getattr(uuids, 'instance_%d' % instance_id), + image_ref='1', + project_id=tenant_id, + user_id='fakeuser', + display_name='name', + instance_type_id=FAKE_INST_TYPE['id'], + launched_at=start, + terminated_at=end, + deleted_at=start, + vm_state=vm_state, + memory_mb=MEMORY_MB, + vcpus=VCPUS, + root_gb=ROOT_GB, + ephemeral_gb=EPHEMERAL_GB) + + +@classmethod +def fake_get_active_deleted_flavorless(cls, context, begin, end=None, + project_id=None, host=None, + expected_attrs=None, use_slave=False, + limit=None, marker=None): + # First get some normal instances to have actual usage + instances = [ + _fake_instance(START, STOP, x, + project_id or 'faketenant_%s' % (x // SERVERS)) + for x in range(TENANTS * SERVERS)] + # Then get some deleted instances with no flavor to test bugs 1643444 and + # 1692893 (duplicates) + instances.extend([ + _fake_instance_deleted_flavorless( + context, START, STOP, x, + project_id or 'faketenant_%s' % (x // SERVERS)) + for x in range(TENANTS * SERVERS)]) + return objects.InstanceList(objects=instances) + + @classmethod def fake_get_active_by_window_joined(cls, context, begin, end=None, project_id=None, host=None, @@ -95,8 +137,6 @@ def fake_get_active_by_window_joined(cls, context, begin, end=None, for x in range(TENANTS * SERVERS)]) -@mock.patch('nova.objects.InstanceList.get_active_by_window_joined', - fake_get_active_by_window_joined) class SimpleTenantUsageTestV21(test.TestCase): policy_rule_prefix = "os_compute_api:os-simple-tenant-usage" controller = simple_tenant_usage_v21.SimpleTenantUsageController() @@ -129,9 +169,27 @@ class SimpleTenantUsageTestV21(test.TestCase): int(usages[i]['total_vcpus_usage'])) self.assertFalse(usages[i].get('server_usages')) + # NOTE(artom) Test for bugs 1643444 and 1692893 (duplicates). We simulate a + # situation where an instance has been deleted (moved to shadow table) and + # its corresponding instance_extra row has been archived (deleted from + # shadow table). + @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', + fake_get_active_deleted_flavorless) + @mock.patch.object( + objects.Instance, '_load_flavor', + side_effect=exception.InstanceNotFound(instance_id='fake-id')) + def test_verify_index_deleted_flavorless(self, mock_load): + with mock.patch.object(self.controller, '_get_flavor', + return_value=None): + self._test_verify_index(START, STOP) + + @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', + fake_get_active_by_window_joined) def test_verify_index(self): self._test_verify_index(START, STOP) + @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', + fake_get_active_by_window_joined) def test_verify_index_future_end_time(self): future = NOW + datetime.timedelta(hours=HOURS) self._test_verify_index(START, future) @@ -143,6 +201,8 @@ class SimpleTenantUsageTestV21(test.TestCase): future = NOW + datetime.timedelta(hours=HOURS) self._test_verify_show(START, future) + @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', + fake_get_active_by_window_joined) def _get_tenant_usages(self, detailed=''): req = fakes.HTTPRequest.blank('?detailed=%s&start=%s&end=%s' % (detailed, START.isoformat(), STOP.isoformat())) @@ -186,6 +246,8 @@ class SimpleTenantUsageTestV21(test.TestCase): for i in range(TENANTS): self.assertIsNone(usages[i].get('server_usages')) + @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', + fake_get_active_by_window_joined) def _test_verify_show(self, start, stop): tenant_id = 1 req = fakes.HTTPRequest.blank('?start=%s&end=%s' %