Skip to content

Commit

Permalink
Add image_name to create and rebuild notifications
Browse files Browse the repository at this point in the history
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
  • Loading branch information
isethi committed Jul 23, 2012
1 parent c94cbe2 commit cc74222
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 11 deletions.
18 changes: 11 additions & 7 deletions nova/compute/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,11 @@ def _run_instance(self, context, instance_uuid,
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:
Expand All @@ -487,8 +489,8 @@ def _run_instance(self, context, instance_uuid,
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:
Expand Down Expand Up @@ -959,6 +961,8 @@ def _rebuild_instance(self, context, instance_uuid, orig_image_ref,

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.
Expand All @@ -968,8 +972,9 @@ def _rebuild_instance(self, context, instance_uuid, orig_image_ref,
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,
Expand Down Expand Up @@ -998,8 +1003,6 @@ def _rebuild_instance(self, context, instance_uuid, orig_image_ref,
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)

Expand All @@ -1012,7 +1015,8 @@ def _rebuild_instance(self, context, instance_uuid, orig_image_ref,
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
Expand Down
3 changes: 3 additions & 0 deletions nova/tests/api/ec2/test_cinder_cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def setUp(self):

def fake_show(meh, context, id):
return {'id': id,
'name': 'fake_name',
'container_format': 'ami',
'properties': {
'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
Expand Down Expand Up @@ -518,6 +519,7 @@ def _setUpImageSet(self, create_volumes_and_snapshots=False):
{'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',
Expand All @@ -532,6 +534,7 @@ def _setUpImageSet(self, create_volumes_and_snapshots=False):
'snapshot_id': 01234567}]
image2 = {
'id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
'name': 'fake_name',
'properties': {
'kernel_id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
'type': 'machine',
Expand Down
13 changes: 13 additions & 0 deletions nova/tests/api/ec2/test_cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ def setUp(self):

def fake_show(meh, context, id):
return {'id': id,
'name': 'fake_name',
'container_format': 'ami',
'properties': {
'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
Expand Down Expand Up @@ -1123,6 +1124,7 @@ def test_describe_images(self):

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',
Expand Down Expand Up @@ -1190,6 +1192,7 @@ def _setUpImageSet(self, create_volumes_and_snapshots=False):
{'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',
Expand All @@ -1204,6 +1207,7 @@ def _setUpImageSet(self, create_volumes_and_snapshots=False):
'snapshot_id': 01234567}]
image2 = {
'id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
'name': 'fake_name',
'properties': {
'kernel_id': '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6',
'type': 'machine',
Expand Down Expand Up @@ -1313,6 +1317,7 @@ def test_describe_image_attribute(self):

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',
Expand Down Expand Up @@ -1362,6 +1367,7 @@ def test_modify_image_attribute(self):

fake_metadata = {
'id': 'cedef40a-ed67-4d10-800e-17455edce175',
'name': 'fake_name',
'container_format': 'ami',
'properties': {
'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
Expand Down Expand Up @@ -1402,6 +1408,7 @@ def fake_create(*args, **kwargs):
# 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,
Expand Down Expand Up @@ -1634,6 +1641,7 @@ def test_run_instances(self):

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'},
Expand Down Expand Up @@ -1667,6 +1675,7 @@ def test_run_instances_availability_zone(self):

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'},
Expand Down Expand Up @@ -1700,6 +1709,7 @@ def test_run_instances_image_state_none(self):

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',
Expand All @@ -1718,6 +1728,7 @@ def test_run_instances_image_state_invalid(self):

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',
Expand All @@ -1737,6 +1748,7 @@ def test_run_instances_image_status_active(self):

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',
Expand Down Expand Up @@ -2416,6 +2428,7 @@ def fake_show(self, context, id_):

return {
'id': id_,
'name': 'fake_name',
'properties': prop_base,
'container_format': 'ami',
'status': 'active'}
Expand Down
15 changes: 11 additions & 4 deletions nova/tests/compute/test_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def setUp(self):

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'}}
Expand Down Expand Up @@ -883,12 +884,14 @@ def test_run_instance_usage_notification(self):
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')
Expand Down Expand Up @@ -1125,11 +1128,13 @@ def test_rebuild_instance_notification(self):
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'])
Expand Down Expand Up @@ -2126,6 +2131,7 @@ def fake_get_nw_info(cls, ctxt, instance):
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'},
}
Expand Down Expand Up @@ -2192,6 +2198,7 @@ def fake_show(*args):
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)

Expand Down Expand Up @@ -2237,7 +2244,7 @@ def test_create_instance_sets_system_metadata(self):
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'])

Expand Down Expand Up @@ -2609,9 +2616,9 @@ def update_wrapper(*args, **kwargs):
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):
Expand Down
34 changes: 34 additions & 0 deletions nova/tests/test_compute_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,37 @@ def test_notify_usage_exists_instance_not_found(self):
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'])

0 comments on commit cc74222

Please sign in to comment.