openstack/deb-nova
Code Issues Proposed changes

Make notifications use Instance.get_flavor()

Browse Source 
This makes the notifications.info_from_instance() method use the
Instance.get_flavor() method to retrieve flavor information, instead
of assuming that it is in system_metadata. This requires that it is
passed an Instance object, so this patch also converts the code to
make that assumption (which needs doing anyway) and fixes remaining
dict-using callers to pass the object. This will be required by a
subsequent patch to refactor flavor storage.

Related to blueprint flavor-from-sysmeta-to-blob

Change-Id: I6e4e07807cb6cd173d54137d4fe8d9b1a0d49709
This commit is contained in:
Dan Smith
2014-11-12 14:39:33 -08:00
parent c00fd06bb4
commit 9f384a41ef
3 changed files with 89 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
nova/notifications.py
View File

@@ -25,7 +25,6 @@ from oslo.utils import excutils
from oslo.utils import timeutils
import six
from nova.compute import flavors
import nova.context
from nova import db
from nova.i18n import _
@@ -336,11 +335,12 @@ def image_meta(system_metadata):
return image_meta
def info_from_instance(context, instance_ref, network_info,
def info_from_instance(context, instance, network_info,
system_metadata, **kw):
"""Get detailed instance information for an instance which is common to all
notifications.
:param:instance: nova.objects.Instance
:param:network_info: network_info provided if not None
:param:system_metadata: system_metadata DB entries for the instance,
if not None
@@ -364,71 +364,68 @@ def info_from_instance(context, instance_ref, network_info,
else:
return str(s) if s else ''
image_ref_url = glance.generate_image_url(instance_ref['image_ref'])
image_ref_url = glance.generate_image_url(instance.image_ref)
instance_type = flavors.extract_flavor(instance_ref)
instance_type = instance.get_flavor()
instance_type_name = instance_type.get('name', '')
instance_flavorid = instance_type.get('flavorid', '')
if system_metadata is None:
system_metadata = utils.instance_sys_meta(instance_ref)
instance_info = dict(
# Owner properties
tenant_id=instance_ref['project_id'],
user_id=instance_ref['user_id'],
tenant_id=instance.project_id,
user_id=instance.user_id,
# Identity properties
instance_id=instance_ref['uuid'],
display_name=instance_ref['display_name'],
reservation_id=instance_ref['reservation_id'],
hostname=instance_ref['hostname'],
instance_id=instance.uuid,
display_name=instance.display_name,
reservation_id=instance.reservation_id,
hostname=instance.hostname,
# Type properties
instance_type=instance_type_name,
instance_type_id=instance_ref['instance_type_id'],
instance_type_id=instance.instance_type_id,
instance_flavor_id=instance_flavorid,
architecture=instance_ref['architecture'],
architecture=instance.architecture,
# Capacity properties
memory_mb=instance_ref['memory_mb'],
disk_gb=instance_ref['root_gb'] + instance_ref['ephemeral_gb'],
vcpus=instance_ref['vcpus'],
memory_mb=instance.memory_mb,
disk_gb=instance.root_gb + instance.ephemeral_gb,
vcpus=instance.vcpus,
# Note(dhellmann): This makes the disk_gb value redundant, but
# we are keeping it for backwards-compatibility with existing
# users of notifications.
root_gb=instance_ref['root_gb'],
ephemeral_gb=instance_ref['ephemeral_gb'],
root_gb=instance.root_gb,
ephemeral_gb=instance.ephemeral_gb,
# Location properties
host=instance_ref['host'],
node=instance_ref['node'],
availability_zone=instance_ref['availability_zone'],
cell_name=null_safe_str(instance_ref['cell_name']),
host=instance.host,
node=instance.node,
availability_zone=instance.availability_zone,
cell_name=null_safe_str(instance.cell_name),
# Date properties
created_at=str(instance_ref['created_at']),
created_at=str(instance.created_at),
# Terminated and Deleted are slightly different (although being
# terminated and not deleted is a transient state), so include
# both and let the recipient decide which they want to use.
terminated_at=null_safe_isotime(instance_ref.get('terminated_at')),
deleted_at=null_safe_isotime(instance_ref.get('deleted_at')),
launched_at=null_safe_isotime(instance_ref.get('launched_at')),
terminated_at=null_safe_isotime(instance.get('terminated_at', None)),
deleted_at=null_safe_isotime(instance.get('deleted_at', None)),
launched_at=null_safe_isotime(instance.get('launched_at', None)),
# Image properties
image_ref_url=image_ref_url,
os_type=instance_ref['os_type'],
kernel_id=instance_ref['kernel_id'],
ramdisk_id=instance_ref['ramdisk_id'],
os_type=instance.os_type,
kernel_id=instance.kernel_id,
ramdisk_id=instance.ramdisk_id,
# Status properties
state=instance_ref['vm_state'],
state_description=null_safe_str(instance_ref.get('task_state')),
progress=null_safe_int(instance_ref['progress']),
state=instance.vm_state,
state_description=null_safe_str(instance.task_state),
progress=null_safe_int(instance.progress),
# accessIPs
access_ip_v4=instance_ref['access_ip_v4'],
access_ip_v6=instance_ref['access_ip_v6'],
access_ip_v4=instance.access_ip_v4,
access_ip_v6=instance.access_ip_v6,
)
if network_info is not None:
@@ -441,11 +438,11 @@ def info_from_instance(context, instance_ref, network_info,
instance_info['fixed_ips'] = fixed_ips
# add image metadata
image_meta_props = image_meta(system_metadata)
image_meta_props = image_meta(instance.system_metadata)
instance_info["image_meta"] = image_meta_props
# add instance metadata
instance_info['metadata'] = utils.instance_meta(instance_ref)
instance_info['metadata'] = instance.metadata
instance_info.update(kw)
return instance_info

5
nova/objects/instance.py
View File

@@ -540,7 +540,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
self._from_db_object(context, self, inst_ref,
expected_attrs=expected_attrs)
notifications.send_update(context, old_ref, inst_ref)
notifications.send_update(context, old_ref, self)
self.obj_reset_changes()
@base.remotable
@@ -667,8 +667,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject):
md_was_changed = 'metadata' in self.obj_what_changed()
del self.metadata[key]
self._orig_metadata.pop(key, None)
instance_dict = base.obj_to_primitive(self)
notifications.send_update(context, instance_dict, instance_dict)
notifications.send_update(context, self, self)
if not md_was_changed:
self.obj_reset_changes(['metadata'])

103
nova/tests/unit/test_notifications.py
View File

@@ -24,9 +24,10 @@ from nova.compute import flavors
from nova.compute import task_states
from nova.compute import vm_states
from nova import context
from nova import db
from nova.network import api as network_api
from nova import notifications
from nova import objects
from nova.objects import base as obj_base
from nova import test
from nova.tests.unit import fake_network
from nova.tests.unit import fake_notifier
@@ -68,22 +69,23 @@ class NotificationsTestCase(test.TestCase):
def _wrapped_create(self, params=None):
instance_type = flavors.get_flavor_by_name('m1.tiny')
sys_meta = flavors.save_flavor_info({}, instance_type)
inst = {}
inst['image_ref'] = 1
inst['user_id'] = self.user_id
inst['project_id'] = self.project_id
inst['instance_type_id'] = instance_type['id']
inst['root_gb'] = 0
inst['ephemeral_gb'] = 0
inst['access_ip_v4'] = '1.2.3.4'
inst['access_ip_v6'] = 'feed:5eed'
inst['display_name'] = 'test_instance'
inst['hostname'] = 'test_instance_hostname'
inst['node'] = 'test_instance_node'
inst['system_metadata'] = sys_meta
inst = objects.Instance(image_ref=1,
user_id=self.user_id,
project_id=self.project_id,
instance_type_id=instance_type['id'],
root_gb=0,
ephemeral_gb=0,
access_ip_v4='1.2.3.4',
access_ip_v6='feed::5eed',
display_name='test_instance',
hostname='test_instance_hostname',
node='test_instance_node',
system_metadata=sys_meta)
inst._context = self.context
if params:
inst.update(params)
return db.instance_create(self.context, inst)
inst.create()
return inst
def test_send_api_fault_disabled(self):
self.flags(notify_api_faults=False)
@@ -115,12 +117,12 @@ class NotificationsTestCase(test.TestCase):
self.flags(notify_on_state_change=None)
old = copy.copy(self.instance)
self.instance["vm_state"] = vm_states.ACTIVE
self.instance.vm_state = vm_states.ACTIVE
old_vm_state = old['vm_state']
new_vm_state = self.instance["vm_state"]
new_vm_state = self.instance.vm_state
old_task_state = old['task_state']
new_task_state = self.instance["task_state"]
new_task_state = self.instance.task_state
notifications.send_update_with_states(self.context, self.instance,
old_vm_state, new_vm_state, old_task_state, new_task_state,
@@ -136,12 +138,12 @@ class NotificationsTestCase(test.TestCase):
# we should not get a notification on task stgate chagne now
old = copy.copy(self.instance)
self.instance["task_state"] = task_states.SPAWNING
self.instance.task_state = task_states.SPAWNING
old_vm_state = old['vm_state']
new_vm_state = self.instance["vm_state"]
new_vm_state = self.instance.vm_state
old_task_state = old['task_state']
new_task_state = self.instance["task_state"]
new_task_state = self.instance.task_state
notifications.send_update_with_states(self.context, self.instance,
old_vm_state, new_vm_state, old_task_state, new_task_state,
@@ -158,10 +160,10 @@ class NotificationsTestCase(test.TestCase):
def test_send_no_notif(self):
# test notification on send no initial vm state:
old_vm_state = self.instance['vm_state']
new_vm_state = self.instance['vm_state']
old_task_state = self.instance['task_state']
new_task_state = self.instance['task_state']
old_vm_state = self.instance.vm_state
new_vm_state = self.instance.vm_state
old_task_state = self.instance.task_state
new_task_state = self.instance.task_state
notifications.send_update_with_states(self.context, self.instance,
old_vm_state, new_vm_state, old_task_state, new_task_state,
@@ -170,22 +172,21 @@ class NotificationsTestCase(test.TestCase):
self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
def test_send_on_vm_change(self):
old = obj_base.obj_to_primitive(self.instance)
old['task_state'] = None
# pretend we just transitioned to ACTIVE:
params = {"vm_state": vm_states.ACTIVE}
(old_ref, new_ref) = db.instance_update_and_get_original(self.context,
self.instance['uuid'], params)
notifications.send_update(self.context, old_ref, new_ref)
self.instance.task_state = task_states.SPAWNING
notifications.send_update(self.context, old, self.instance)
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
def test_send_on_task_change(self):
old = obj_base.obj_to_primitive(self.instance)
old['task_state'] = None
# pretend we just transitioned to task SPAWNING:
params = {"task_state": task_states.SPAWNING}
(old_ref, new_ref) = db.instance_update_and_get_original(self.context,
self.instance['uuid'], params)
notifications.send_update(self.context, old_ref, new_ref)
self.instance.task_state = task_states.SPAWNING
notifications.send_update(self.context, old, self.instance)
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
@@ -204,11 +205,11 @@ class NotificationsTestCase(test.TestCase):
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
notif = fake_notifier.NOTIFICATIONS[0]
payload = notif.payload
access_ip_v4 = self.instance["access_ip_v4"]
access_ip_v6 = self.instance["access_ip_v6"]
display_name = self.instance["display_name"]
hostname = self.instance["hostname"]
node = self.instance["node"]
access_ip_v4 = str(self.instance.access_ip_v4)
access_ip_v6 = str(self.instance.access_ip_v6)
display_name = self.instance.display_name
hostname = self.instance.hostname
node = self.instance.node
self.assertEqual(vm_states.BUILDING, payload["old_state"])
self.assertEqual(vm_states.ACTIVE, payload["state"])
@@ -229,10 +230,10 @@ class NotificationsTestCase(test.TestCase):
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
notif = fake_notifier.NOTIFICATIONS[0]
payload = notif.payload
access_ip_v4 = self.instance["access_ip_v4"]
access_ip_v6 = self.instance["access_ip_v6"]
display_name = self.instance["display_name"]
hostname = self.instance["hostname"]
access_ip_v4 = str(self.instance.access_ip_v4)
access_ip_v6 = str(self.instance.access_ip_v6)
display_name = self.instance.display_name
hostname = self.instance.hostname
self.assertEqual(vm_states.BUILDING, payload["old_state"])
self.assertEqual(vm_states.BUILDING, payload["state"])
@@ -290,11 +291,11 @@ class NotificationsTestCase(test.TestCase):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.assertIn("cell_name", info)
self.assertIsNone(self.instance['cell_name'])
self.assertIsNone(self.instance.cell_name)
self.assertEqual("", info["cell_name"])
def test_payload_has_cell_name(self):
self.instance['cell_name'] = "cell1"
self.instance.cell_name = "cell1"
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.assertIn("cell_name", info)
@@ -304,11 +305,11 @@ class NotificationsTestCase(test.TestCase):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.assertIn("progress", info)
self.assertIsNone(self.instance['progress'])
self.assertIsNone(self.instance.progress)
self.assertEqual("", info["progress"])
def test_payload_has_progress(self):
self.instance['progress'] = 50
self.instance.progress = 50
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.assertIn("progress", info)
@@ -319,8 +320,8 @@ class NotificationsTestCase(test.TestCase):
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
notif = fake_notifier.NOTIFICATIONS[0]
payload = notif.payload
access_ip_v4 = self.instance["access_ip_v4"]
access_ip_v6 = self.instance["access_ip_v6"]
access_ip_v4 = str(self.instance.access_ip_v4)
access_ip_v6 = str(self.instance.access_ip_v6)
self.assertEqual(payload["access_ip_v4"], access_ip_v4)
self.assertEqual(payload["access_ip_v6"], access_ip_v6)
@@ -332,8 +333,8 @@ class NotificationsTestCase(test.TestCase):
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
notif = fake_notifier.NOTIFICATIONS[0]
payload = notif.payload
old_display_name = self.instance["display_name"]
new_display_name = new_name_inst["display_name"]
old_display_name = self.instance.display_name
new_display_name = new_name_inst.display_name
self.assertEqual(payload["old_display_name"], old_display_name)
self.assertEqual(payload["display_name"], new_display_name)