Add image_name to create and rebuild notifications

Fixes bug 1027197

Without image_name in notifications, one has to use the image_ref
and hit glance again.
The create and rebuild functions already contain image_metadata, hence
adding the image_name in their notification can prevent additional
call to glance.

Change-Id: I3813eafa24b125575bcdceac8d30ab6bd909cfe4

5 files changed, 72 insertions, 11 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 034bce09e..bc92048e9 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -463,9 +463,11 @@ class ComputeManager(manager.SchedulerDependentManager):
             instance = self.db.instance_get_by_uuid(context, instance_uuid)
             self._check_instance_not_already_created(context, instance)
             image_meta = self._check_image_size(context, instance)
+            extra_usage_info = {"image_name": image_meta['name']}
             self._start_building(context, instance)
             self._notify_about_instance_usage(
-                    context, instance, "create.start")
+                    context, instance, "create.start",
+                    extra_usage_info=extra_usage_info)
             network_info = self._allocate_network(context, instance,
                                                   requested_networks)
             try:
@@ -487,8 +489,8 @@ class ComputeManager(manager.SchedulerDependentManager):
                     self._update_access_ip(context, instance, network_info)
 
                 self._notify_about_instance_usage(context, instance,
-                        "create.end", network_info=network_info)
-
+                        "create.end", network_info=network_info,
+                        extra_usage_info=extra_usage_info)
         except exception.InstanceNotFound:
             LOG.warn(_("Instance not found."), instance_uuid=instance_uuid)
         except Exception as e:
@@ -959,6 +961,8 @@ class ComputeManager(manager.SchedulerDependentManager):
 
         instance = self.db.instance_get_by_uuid(context, instance_uuid)
 
+        image_meta = _get_image_meta(context, image_ref)
+
         # This instance.exists message should contain the original
         # image_ref, not the new one.  Since the DB has been updated
         # to point to the new one... we have to override it.
@@ -968,8 +972,9 @@ class ComputeManager(manager.SchedulerDependentManager):
                 current_period=True, extra_usage_info=extra_usage_info)
 
         # This message should contain the new image_ref
+        extra_usage_info = {'image_name': image_meta['name']}
         self._notify_about_instance_usage(context, instance,
-                "rebuild.start")
+                "rebuild.start", extra_usage_info=extra_usage_info)
 
         current_power_state = self._get_power_state(context, instance)
         self._instance_update(context,
@@ -998,8 +1003,6 @@ class ComputeManager(manager.SchedulerDependentManager):
         instance.admin_pass = kwargs.get('new_pass',
                 utils.generate_password(FLAGS.password_length))
 
-        image_meta = _get_image_meta(context, image_ref)
-
         self.driver.spawn(context, instance, image_meta,
                           self._legacy_nw_info(network_info), device_info)
 
@@ -1012,7 +1015,8 @@ class ComputeManager(manager.SchedulerDependentManager):
                                          launched_at=timeutils.utcnow())
 
         self._notify_about_instance_usage(context, instance, "rebuild.end",
-                                          network_info=network_info)
+                                          network_info=network_info,
+                                          extra_usage_info=extra_usage_info)
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @checks_instance_lock
diff --git a/nova/tests/api/ec2/test_cinder_cloud.py b/nova/tests/api/ec2/test_cinder_cloud.py
index dd99a45dd..fe684d7cd 100644
--- a/nova/tests/api/ec2/test_cinder_cloud.py
+++ b/nova/tests/api/ec2/test_cinder_cloud.py
@@ -86,6 +86,7 @@ class CinderCloudTestCase(test.TestCase):
 
         def fake_show(meh, context, id):
             return {'id': id,
+                    'name': 'fake_name',
                     'container_format': 'ami',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -518,6 +519,7 @@ class CinderCloudTestCase(test.TestCase):
             {'device_name': '/dev/sdc4', 'no_device': True}]
         image1 = {
             'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+            'name': 'fake_name',
             'properties': {
                 'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                 'type': 'machine',
@@ -532,6 +534,7 @@ class CinderCloudTestCase(test.TestCase):
                                   'snapshot_id': 01234567}]
         image2 = {
             'id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
+            'name': 'fake_name',
             'properties': {
                 'kernel_id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
                 'type': 'machine',
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 1a0f429cb..ec9ab03d7 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -96,6 +96,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show(meh, context, id):
             return {'id': id,
+                    'name': 'fake_name',
                     'container_format': 'ami',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1123,6 +1124,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_detail(meh, context, **kwargs):
             return [{'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                     'name': 'fake_name',
                      'container_format': 'ami',
                      'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1190,6 +1192,7 @@ class CloudTestCase(test.TestCase):
             {'device_name': '/dev/sdc4', 'no_device': True}]
         image1 = {
             'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+            'name': 'fake_name',
             'properties': {
                 'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                 'type': 'machine',
@@ -1204,6 +1207,7 @@ class CloudTestCase(test.TestCase):
                                   'snapshot_id': 01234567}]
         image2 = {
             'id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
+            'name': 'fake_name',
             'properties': {
                 'kernel_id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
                 'type': 'machine',
@@ -1313,6 +1317,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show(meh, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                         'ramdisk_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1362,6 +1367,7 @@ class CloudTestCase(test.TestCase):
 
         fake_metadata = {
             'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+            'name': 'fake_name',
             'container_format': 'ami',
             'properties': {
                 'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1402,6 +1408,7 @@ class CloudTestCase(test.TestCase):
             # NOTE(vish): We are mocking s3 so make sure we have converted
             #             to ids instead of uuids.
             return {'id': 1,
+            'name': 'fake_name',
             'container_format': 'ami',
             'properties': {
                 'kernel_id': 1,
@@ -1634,6 +1641,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show(self, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                         'type': 'machine'},
@@ -1667,6 +1675,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show(self, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                         'type': 'machine'},
@@ -1700,6 +1709,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show_no_state(self, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
                         'ramdisk_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1718,6 +1728,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show_decrypt(self, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'container_format': 'ami',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -1737,6 +1748,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show_stat_active(self, context, id):
             return {'id': 'cedef40a-ed67-4d10-800e-17455edce175',
+                    'name': 'fake_name',
                     'container_format': 'ami',
                     'properties': {
                         'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
@@ -2416,6 +2428,7 @@ class CloudTestCase(test.TestCase):
 
             return {
                 'id': id_,
+                'name': 'fake_name',
                 'properties': prop_base,
                 'container_format': 'ami',
                 'status': 'active'}
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 376de04d3..6dacd7459 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -126,6 +126,7 @@ class BaseTestCase(test.TestCase):
 
         def fake_show(meh, context, id):
             return {'id': id, 'min_disk': None, 'min_ram': None,
+                    'name': 'fake_name',
                     'properties': {'kernel_id': 'fake_kernel_id',
                                    'ramdisk_id': 'fake_ramdisk_id',
                                    'something_else': 'meow'}}
@@ -883,12 +884,14 @@ class ComputeTestCase(BaseTestCase):
         inst_ref = db.instance_get_by_uuid(self.context, instance_uuid)
         msg = test_notifier.NOTIFICATIONS[0]
         self.assertEquals(msg['event_type'], 'compute.instance.create.start')
+        self.assertEquals(msg['payload']['image_name'], 'fake_name')
         # The last event is the one with the sugar in it.
         msg = test_notifier.NOTIFICATIONS[1]
         self.assertEquals(msg['priority'], 'INFO')
         self.assertEquals(msg['event_type'], 'compute.instance.create.end')
         payload = msg['payload']
         self.assertEquals(payload['tenant_id'], self.project_id)
+        self.assertEquals(payload['image_name'], 'fake_name')
         self.assertEquals(payload['user_id'], self.user_id)
         self.assertEquals(payload['instance_id'], inst_ref.uuid)
         self.assertEquals(payload['instance_type'], 'm1.tiny')
@@ -1125,11 +1128,13 @@ class ComputeTestCase(BaseTestCase):
         self.assertEquals(msg['event_type'],
                           'compute.instance.rebuild.start')
         self.assertEquals(msg['payload']['image_ref_url'], new_image_ref_url)
+        self.assertEquals(msg['payload']['image_name'], 'fake_name')
         msg = test_notifier.NOTIFICATIONS[2]
         self.assertEquals(msg['event_type'],
                           'compute.instance.rebuild.end')
         self.assertEquals(msg['priority'], 'INFO')
         payload = msg['payload']
+        self.assertEquals(payload['image_name'], 'fake_name')
         self.assertEquals(payload['tenant_id'], self.project_id)
         self.assertEquals(payload['user_id'], self.user_id)
         self.assertEquals(payload['instance_id'], inst_ref['uuid'])
@@ -2126,6 +2131,7 @@ class ComputeAPITestCase(BaseTestCase):
                                    security_group_api=self.security_group_api)
         self.fake_image = {
             'id': 1,
+            'name': 'fake_name',
             'properties': {'kernel_id': 'fake_kernel_id',
                            'ramdisk_id': 'fake_ramdisk_id'},
         }
@@ -2192,6 +2198,7 @@ class ComputeAPITestCase(BaseTestCase):
             img = copy.copy(self.fake_image)
             img['min_ram'] = 2
             img['min_disk'] = 2
+            img['name'] = 'fake_name'
             return img
         self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
 
@@ -2237,7 +2244,7 @@ class ComputeAPITestCase(BaseTestCase):
             self.assertEqual(sys_metadata,
                     {'image_kernel_id': 'fake_kernel_id',
                      'image_ramdisk_id': 'fake_ramdisk_id',
-                     'image_something_else': 'meow'})
+                     'image_something_else': 'meow', })
         finally:
             db.instance_destroy(self.context, ref[0]['uuid'])
 
@@ -2609,9 +2616,9 @@ class ComputeAPITestCase(BaseTestCase):
                 instance_uuid)
         self.assertEqual(sys_metadata,
                 {'image_kernel_id': 'fake_kernel_id',
-                 'image_ramdisk_id': 'fake_ramdisk_id',
-                 'image_something_else': 'meow',
-                 'preserved': 'preserve this!'})
+                'image_ramdisk_id': 'fake_ramdisk_id',
+                'image_something_else': 'meow',
+                'preserved': 'preserve this!'})
         db.instance_destroy(self.context, instance['uuid'])
 
     def test_reboot_soft(self):
diff --git a/nova/tests/test_compute_utils.py b/nova/tests/test_compute_utils.py
index c3f06d11c..ee415da66 100644
--- a/nova/tests/test_compute_utils.py
+++ b/nova/tests/test_compute_utils.py
@@ -174,3 +174,37 @@ class UsageInfoTestCase(test.TestCase):
         self.assertEquals(payload['image_meta'], {})
         image_ref_url = "%s/images/1" % utils.generate_glance_url()
         self.assertEquals(payload['image_ref_url'], image_ref_url)
+
+    def test_notify_about_instance_usage(self):
+        instance_id = self._create_instance()
+        instance = db.instance_get(self.context, instance_id)
+        # Set some system metadata
+        sys_metadata = {'image_md_key1': 'val1',
+                        'image_md_key2': 'val2',
+                        'other_data': 'meow'}
+        extra_usage_info = {'image_name': 'fake_name'}
+        db.instance_system_metadata_update(self.context, instance['uuid'],
+                sys_metadata, False)
+        compute_utils.notify_about_instance_usage(self.context, instance,
+        'create.start', extra_usage_info=extra_usage_info)
+        self.assertEquals(len(test_notifier.NOTIFICATIONS), 1)
+        msg = test_notifier.NOTIFICATIONS[0]
+        self.assertEquals(msg['priority'], 'INFO')
+        self.assertEquals(msg['event_type'], 'compute.instance.create.start')
+        payload = msg['payload']
+        self.assertEquals(payload['tenant_id'], self.project_id)
+        self.assertEquals(payload['user_id'], self.user_id)
+        self.assertEquals(payload['instance_id'], instance.uuid)
+        self.assertEquals(payload['instance_type'], 'm1.tiny')
+        type_id = instance_types.get_instance_type_by_name('m1.tiny')['id']
+        self.assertEquals(str(payload['instance_type_id']), str(type_id))
+        for attr in ('display_name', 'created_at', 'launched_at',
+                     'state', 'state_description', 'image_meta'):
+            self.assertTrue(attr in payload,
+                            msg="Key %s not in payload" % attr)
+        self.assertEquals(payload['image_meta'],
+                {'md_key1': 'val1', 'md_key2': 'val2'})
+        self.assertEquals(payload['image_name'], 'fake_name')
+        image_ref_url = "%s/images/1" % utils.generate_glance_url()
+        self.assertEquals(payload['image_ref_url'], image_ref_url)
+        self.compute.terminate_instance(self.context, instance['uuid'])