Merge "Make tenant_usage fall back to instance_type_id"

2 files changed, 92 insertions, 1 deletions

diff --git a/nova/api/openstack/compute/contrib/simple_tenant_usage.py b/nova/api/openstack/compute/contrib/simple_tenant_usage.py
index 0fa9b9e40..0428c8a77 100644
--- a/nova/api/openstack/compute/contrib/simple_tenant_usage.py
+++ b/nova/api/openstack/compute/contrib/simple_tenant_usage.py
@@ -25,6 +25,7 @@ from nova.api.openstack import wsgi
 from nova.api.openstack import xmlutil
 from nova.compute import api
 from nova.compute import instance_types
+from nova import exception
 from nova.openstack.common import timeutils
 
 authorize_show = extensions.extension_authorizer('compute',
@@ -103,6 +104,30 @@ class SimpleTenantUsageController(object):
             # instance hasn't launched, so no charge
             return 0
 
+    def _get_flavor(self, context, compute_api, instance, flavors_cache):
+        """Get flavor information from the instance's system_metadata,
+        allowing a fallback to lookup by-id for deleted instances only"""
+        try:
+            return instance_types.extract_instance_type(instance)
+        except KeyError:
+            if not instance['deleted']:
+                # Only support the fallback mechanism for deleted instances
+                # that would have been skipped by migration #153
+                raise
+
+        flavor_type = instance['instance_type_id']
+        if flavor_type in flavors_cache:
+            return flavors_cache[flavor_type]
+
+        try:
+            it_ref = compute_api.get_instance_type(context, flavor_type)
+            flavors_cache[flavor_type] = it_ref
+        except exception.InstanceTypeNotFound:
+            # can't bill if there is no instance type
+            it_ref = None
+
+        return it_ref
+
     def _tenant_usages_for_period(self, context, period_start,
                                   period_stop, tenant_id=None, detailed=True):
 
@@ -119,7 +144,9 @@ class SimpleTenantUsageController(object):
             info['hours'] = self._hours_for(instance,
                                             period_start,
                                             period_stop)
-            flavor = instance_types.extract_instance_type(instance)
+            flavor = self._get_flavor(context, compute_api, instance, flavors)
+            if not flavor:
+                continue
 
             info['instance_id'] = instance['uuid']
             info['name'] = instance['display_name']
diff --git a/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py b/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py
index ab9906135..ea692c8e6 100644
--- a/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py
+++ b/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py
@@ -24,6 +24,7 @@ from nova.api.openstack.compute.contrib import simple_tenant_usage
 from nova.compute import api
 from nova.compute import instance_types
 from nova import context
+from nova import exception
 from nova.openstack.common import jsonutils
 from nova.openstack.common import policy as common_policy
 from nova.openstack.common import timeutils
@@ -404,3 +405,66 @@ class SimpleTenantUsageSerializerTest(test.TestCase):
         self.assertEqual(len(raw_usages), len(tree))
         for idx, child in enumerate(tree):
             self._verify_tenant_usage(raw_usages[idx], child)
+
+
+class SimpleTenantUsageControllerTest(test.TestCase):
+    def setUp(self):
+        super(SimpleTenantUsageControllerTest, self).setUp()
+        self.controller = simple_tenant_usage.SimpleTenantUsageController()
+
+        class FakeComputeAPI:
+            def get_instance_type(self, context, flavor_type):
+                if flavor_type == 1:
+                    return instance_types.get_default_instance_type()
+                else:
+                    raise exception.InstanceTypeNotFound(flavor_type)
+
+        self.compute_api = FakeComputeAPI()
+        self.context = None
+
+        now = datetime.datetime.now()
+        self.baseinst = dict(display_name='foo',
+                             launched_at=now - datetime.timedelta(1),
+                             terminated_at=now,
+                             instance_type_id=1,
+                             vm_state='deleted',
+                             deleted=0)
+        basetype = instance_types.get_default_instance_type()
+        sys_meta = utils.dict_to_metadata(
+            instance_types.save_instance_type_info({}, basetype))
+        self.baseinst['system_metadata'] = sys_meta
+        self.basetype = instance_types.extract_instance_type(self.baseinst)
+
+    def test_get_flavor_from_sys_meta(self):
+        # Non-deleted instances get their type information from their
+        # system_metadata
+        flavor = self.controller._get_flavor(self.context, self.compute_api,
+                                             self.baseinst, {})
+        self.assertEqual(flavor, self.basetype)
+
+    def test_get_flavor_from_non_deleted_with_id_fails(self):
+        # If an instance is not deleted and missing type information from
+        # system_metadata, then that's a bug
+        inst_without_sys_meta = dict(self.baseinst, system_metadata=[])
+        self.assertRaises(KeyError,
+                          self.controller._get_flavor, self.context,
+                          self.compute_api, inst_without_sys_meta, {})
+
+    def test_get_flavor_from_deleted_with_id(self):
+        # Deleted instances may not have type info in system_metadata,
+        # so verify that they get their type from a lookup of their
+        # instance_type_id
+        inst_without_sys_meta = dict(self.baseinst, system_metadata=[],
+                                     deleted=1)
+        flavor = self.controller._get_flavor(self.context, self.compute_api,
+                                             inst_without_sys_meta, {})
+        self.assertEqual(flavor, instance_types.get_default_instance_type())
+
+    def test_get_flavor_from_deleted_with_id_of_deleted(self):
+        # Verify the legacy behavior of instance_type_id pointing to a
+        # missing type being non-fatal
+        inst_without_sys_meta = dict(self.baseinst, system_metadata=[],
+                                     deleted=1, instance_type_id=2)
+        flavor = self.controller._get_flavor(self.context, self.compute_api,
+                                             inst_without_sys_meta, {})
+        self.assertEqual(flavor, None)