openstack/aodh
Code Issues Proposed changes

Fixes failed notification when deleting instance

Browse Source 
In the notifier plugin for nova, the class Instance that translates instance_ref
returned by conduct.API to nova_client like Instance is not very robust, which
results some attributes are none or missing, and then causes pollsters raise
AttributeError or NoneTypeError

In this change, add the missing or none attributes to the Instance class.

Change-Id: I3e5aa6382021633a6bf7240bf58d3c3bed074785
Fixs: bug #1206731
This commit is contained in:
Guangyu Suo 2013-08-08 06:39:40 +08:00
parent b59a03109c
commit f73f3e17af
2 changed files with 96 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
ceilometer/compute/nova_notifier.py
View File

@ -31,6 +31,7 @@ for name in ['openstack', 'openstack.common', 'openstack.common.log']:
sys.modules['ceilometer.' + name] = sys.modules['nova.' + name] sys.modules['ceilometer.' + name] = sys.modules['nova.' + name]
from nova import conductor from nova import conductor
from nova import utils
from stevedore import extension from stevedore import extension
@ -46,7 +47,7 @@ from ceilometer.openstack.common.gettextutils import _
LOG = logging.getLogger('nova.ceilometer.notifier') LOG = logging.getLogger('nova.ceilometer.notifier')
_gatherer = None _gatherer = None
instance_info_source = conductor.API() conductor_api = conductor.API()
class DeletedInstanceStatsGatherer(object): class DeletedInstanceStatsGatherer(object):
@ -100,9 +101,16 @@ class Instance(object):
dictionary. This class makes an object from the dictonary so we dictionary. This class makes an object from the dictonary so we
can pass it to the pollsters. can pass it to the pollsters.
""" """
def __init__(self, info): def __init__(self, context, info):
for k, v in info.iteritems(): for k, v in info.iteritems():
setattr(self, k, v) if k == 'name':
setattr(self, 'OS-EXT-SRV-ATTR:instance_name', v)
elif k == 'metadata':
setattr(self, k, utils.metadata_to_dict(v))
else:
setattr(self, k, v)
self.flavor_name = conductor_api.instance_type_get(
context, self.instance_type_id).get('name', 'UNKNOWN')
LOG.debug(_('INFO %r'), info) LOG.debug(_('INFO %r'), info)
@property @property
@ -113,7 +121,7 @@ class Instance(object):
def flavor(self): def flavor(self):
return { return {
'id': self.instance_type_id, 'id': self.instance_type_id,
'name': self.instance_type.get('name', 'UNKNOWN'), 'name': self.flavor_name,
} }
@property @property
@ -124,6 +132,10 @@ class Instance(object):
def image(self): def image(self):
return {'id': self.image_ref} return {'id': self.image_ref}
@property
def name(self):
return self.display_name
def notify(context, message): def notify(context, message):
if message['event_type'] != 'compute.instance.delete.start': if message['event_type'] != 'compute.instance.delete.start':
@ -136,7 +148,7 @@ def notify(context, message):
LOG.debug(_('polling final stats for %r'), instance_id) LOG.debug(_('polling final stats for %r'), instance_id)
# Ask for the instance details # Ask for the instance details
instance_ref = instance_info_source.instance_get_by_uuid( instance_ref = conductor_api.instance_get_by_uuid(
context, context,
instance_id, instance_id,
) )
@ -148,7 +160,7 @@ def notify(context, message):
# Extend the payload with samples from our plugins. We only need # Extend the payload with samples from our plugins. We only need
# to send some of the data from the counter objects, since a lot # to send some of the data from the counter objects, since a lot
# of the fields are the same. # of the fields are the same.
instance = Instance(instance_ref) instance = Instance(context, instance_ref)
counters = gatherer(instance) counters = gatherer(instance)
payload['samples'] = [{'name': c.name, payload['samples'] = [{'name': c.name,
'type': c.type, 'type': c.type,

126
nova_tests/test_notifier.py
View File

@ -59,6 +59,7 @@ import ceilometer.openstack.common.fixture.moxstubout
from ceilometer.compute import nova_notifier from ceilometer.compute import nova_notifier
from ceilometer import sample from ceilometer import sample
from ceilometer.compute.pollsters import util
from ceilometer.tests import base from ceilometer.tests import base
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
@ -69,8 +70,8 @@ class TestNovaNotifier(base.TestCase):
class Pollster(object): class Pollster(object):
instances = [] instances = []
test_data = sample.Sample( test_data_1 = sample.Sample(
name='test', name='test1',
type=sample.TYPE_CUMULATIVE, type=sample.TYPE_CUMULATIVE,
unit='units-go-here', unit='units-go-here',
volume=1, volume=1,
@ -84,10 +85,17 @@ class TestNovaNotifier(base.TestCase):
def get_samples(self, manager, cache, instance): def get_samples(self, manager, cache, instance):
self.instances.append((manager, instance)) self.instances.append((manager, instance))
return [self.test_data] test_data_2 = util.make_counter_from_instance(
instance,
name='test2',
type=sample.TYPE_CUMULATIVE,
unit='units-go-here',
volume=1,
)
return [self.test_data_1, test_data_2]
def get_counter_names(self): def get_counter_names(self):
return ['test'] return ['test1', 'test2']
@mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock()) @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock())
def setUp(self): def setUp(self):
@ -104,41 +112,40 @@ class TestNovaNotifier(base.TestCase):
self.context = context.get_admin_context() self.context = context.get_admin_context()
fake_network.set_stub_network_methods(self.stubs) fake_network.set_stub_network_methods(self.stubs)
instance_data = {"display_name": "instance-1", self.instance_data = {"display_name": "instance-1",
'OS-EXT-SRV-ATTR:instance_name': 'instance-1', "id": 1,
"id": 1, "image_ref": "FAKE",
"image_ref": "FAKE", "user_id": "FAKE",
"user_id": "FAKE", "project_id": "FAKE",
"project_id": "FAKE", "display_name": "FAKE NAME",
"display_name": "FAKE NAME", "hostname": "abcdef",
"hostname": "abcdef", "reservation_id": "FAKE RID",
"reservation_id": "FAKE RID", "instance_type_id": 1,
"instance_type_id": 1, "architecture": "x86",
"architecture": "x86", "memory_mb": "1024",
"memory_mb": "1024", "root_gb": "20",
"root_gb": "20", "ephemeral_gb": "0",
"ephemeral_gb": "0", "vcpus": 1,
"vcpus": 1, 'node': "fakenode",
'node': "fakenode", "host": "fakehost",
"host": "fakehost", "availability_zone":
"availability_zone": "1e3ce043029547f1a61c1996d1a531a4",
"1e3ce043029547f1a61c1996d1a531a4", "created_at": '2012-05-08 20:23:41',
"created_at": '2012-05-08 20:23:41', "launched_at": '2012-05-08 20:25:45',
"launched_at": '2012-05-08 20:25:45', "terminated_at": '2012-05-09 20:23:41',
"terminated_at": '2012-05-09 20:23:41', "os_type": "linux",
"os_type": "linux", "kernel_id": "kernelid",
"kernel_id": "kernelid", "ramdisk_id": "ramdiskid",
"ramdisk_id": "ramdiskid", "vm_state": vm_states.ACTIVE,
"vm_state": vm_states.ACTIVE, "task_state": None,
"task_state": None, "access_ip_v4": "192.168.5.4",
"access_ip_v4": "192.168.5.4", "access_ip_v6": "2001:DB8::0",
"access_ip_v6": "2001:DB8::0", "metadata": {},
"metadata": {}, "uuid": "144e08f4-00cb-11e2-888e-5453ed1bbb5f",
"uuid": "144e08f4-00cb-11e2-888e-5453ed1bbb5f", "system_metadata": {}}
"system_metadata": {}}
self.instance = nova_instance.Instance() self.instance = nova_instance.Instance()
for key, value in instance_data.iteritems(): for key, value in self.instance_data.iteritems():
setattr(self.instance, key, value) setattr(self.instance, key, value)
self.stubs.Set(db, 'instance_info_cache_delete', self.do_nothing) self.stubs.Set(db, 'instance_info_cache_delete', self.do_nothing)
@ -166,6 +173,8 @@ class TestNovaNotifier(base.TestCase):
]) ])
self.ext_mgr = ext_mgr self.ext_mgr = ext_mgr
self.gatherer = nova_notifier.DeletedInstanceStatsGatherer(ext_mgr) self.gatherer = nova_notifier.DeletedInstanceStatsGatherer(ext_mgr)
# Initialize the global _gatherer in nova_notifier to use the
# gatherer in this test instead of the gatherer in nova_notifier.
nova_notifier.initialize_gatherer(self.gatherer) nova_notifier.initialize_gatherer(self.gatherer)
# Terminate the instance to trigger the notification. # Terminate the instance to trigger the notification.
@ -180,9 +189,12 @@ class TestNovaNotifier(base.TestCase):
# The code that looks up the instance uses a global # The code that looks up the instance uses a global
# reference to the API, so we also have to patch that to # reference to the API, so we also have to patch that to
# return our fake data. # return our fake data.
mock.patch.object(nova_notifier.instance_info_source, mock.patch.object(nova_notifier.conductor_api,
'instance_get_by_uuid', 'instance_get_by_uuid',
self.fake_instance_ref_get), self.fake_instance_ref_get),
mock.patch.object(nova_notifier.conductor_api,
'instance_type_get',
self.fake_instance_type_get),
mock.patch('nova.openstack.common.notifier.rpc_notifier.notify', mock.patch('nova.openstack.common.notifier.rpc_notifier.notify',
self.notify) self.notify)
): ):
@ -194,10 +206,14 @@ class TestNovaNotifier(base.TestCase):
super(TestNovaNotifier, self).tearDown() super(TestNovaNotifier, self).tearDown()
nova_notifier._gatherer = None nova_notifier._gatherer = None
# The instance returned by conductor API is a dictionary actually,
# and it will be transformed to an nova_notifier.Instance object
# that looks like what the novaclient gives them.
def fake_instance_ref_get(self, context, id_): def fake_instance_ref_get(self, context, id_):
if self.instance.uuid == id_: return self.instance_data
return self.instance
return {} def fake_instance_type_get(self, context, id_):
return {'id': '1', 'name': 'm1.tiny'}
@staticmethod @staticmethod
def do_nothing(*args, **kwargs): def do_nothing(*args, **kwargs):
@ -229,13 +245,27 @@ class TestNovaNotifier(base.TestCase):
continue continue
payload = message['payload'] payload = message['payload']
samples = payload['samples'] samples = payload['samples']
self.assertEqual(len(samples), 1)
s = payload['samples'][0] # Because the playload's samples doesn't include instance
self.assertEqual(s, {'name': 'test', # metadata, we can't check the metadata field directly.
'type': sample.TYPE_CUMULATIVE, # But if we make a mistake in the instance attributes, such
'unit': 'units-go-here', # as missing instance.name or instance.flavor['name'], it
'volume': 1, # will raise AttributeError, which results the number of
}) # the samples doesn't equal to 2.
self.assertEqual(len(samples), 2)
s1 = payload['samples'][0]
self.assertEqual(s1, {'name': 'test1',
'type': sample.TYPE_CUMULATIVE,
'unit': 'units-go-here',
'volume': 1,
})
s2 = payload['samples'][1]
self.assertEqual(s2, {'name': 'test2',
'type': sample.TYPE_CUMULATIVE,
'unit': 'units-go-here',
'volume': 1,
})
break break
else: else:
assert False, 'Did not find expected event' assert False, 'Did not find expected event'