openstack/nova
Code Issues Proposed changes

Use enum value instead of string service name

Browse Source 
In the nova notification code path the nova service / binary names
are hardcoded. There is a possible confusion as the REST API uses
'nova-osapi_compute' while the notifications use 'nova-api'
as the name of the nova compute service binary.
To avoid possible mixing of the names a new enum is introduced
in the notification code path with the valid values form
notification perspective.

Change-Id: Ia85a8b6c34a3efaf3ef509aab316294b9c0c2fd1
Closes-Bug: #1696152
This commit is contained in:
Balazs Gibizer 2017-07-06 12:59:04 +02:00
parent f468dae28a
commit 7c0132c5f4
11 changed files with 58 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
nova/compute/utils.py
View File

@ -339,7 +339,8 @@ def _get_fault_and_priority_from_exc(exception):
def notify_about_instance_action(context, instance, host, action, phase=None,
source='nova-compute', exception=None):
source=fields.NotificationSource.COMPUTE,
exception=None):
"""Send versioned notification about the action made on the instance
:param instance: the instance which the action performed on
:param host: the host emitting the notification
@ -366,7 +367,8 @@ def notify_about_instance_action(context, instance, host, action, phase=None,
def notify_about_instance_create(context, instance, host, phase=None,
source='nova-compute', exception=None):
source=fields.NotificationSource.COMPUTE,
exception=None):
"""Send versioned notification about instance creation
:param context: the request context
@ -394,8 +396,8 @@ def notify_about_instance_create(context, instance, host, phase=None,
def notify_about_volume_attach_detach(context, instance, host, action, phase,
source='nova-compute', volume_id=None,
exception=None):
source=fields.NotificationSource.COMPUTE,
volume_id=None, exception=None):
"""Send versioned notification about the action made on the instance
:param instance: the instance which the action performed on
:param host: the host emitting the notification
@ -435,7 +437,7 @@ def notify_about_keypair_action(context, keypair, action, phase):
notification = keypair_notification.KeypairNotification(
priority=fields.NotificationPriority.INFO,
publisher=notification_base.NotificationPublisher(
host=CONF.host, source='nova-api'),
host=CONF.host, source=fields.NotificationSource.API),
event_type=notification_base.EventType(
object='keypair',
action=action,
@ -469,7 +471,7 @@ def notify_about_volume_swap(context, instance, host, action, phase,
context=context,
priority=priority,
publisher=notification_base.NotificationPublisher(
host=host, source='nova-compute'),
host=host, source=fields.NotificationSource.COMPUTE),
event_type=notification_base.EventType(
object='instance', action=action, phase=phase),
payload=payload).emit(context)
@ -511,7 +513,7 @@ def notify_about_aggregate_action(context, aggregate, action, phase):
notification = aggregate_notification.AggregateNotification(
priority=fields.NotificationPriority.INFO,
publisher=notification_base.NotificationPublisher(
host=CONF.host, source='nova-api'),
host=CONF.host, source=fields.NotificationSource.API),
event_type=notification_base.EventType(
object='aggregate',
action=action,

7
nova/notifications/objects/base.py
View File

@ -139,11 +139,14 @@ class NotificationPayloadBase(NotificationObject):
class NotificationPublisher(NotificationObject):
# Version 1.0: Initial version
# 2.0: The binary field has been renamed to source
VERSION = '2.0'
# 2.1: The type of the source field changed from string to enum.
# This only needs a minor bump as the enum uses the possible
# values of the previous string field
VERSION = '2.1'
fields = {
'host': fields.StringField(nullable=False),
'source': fields.StringField(nullable=False),
'source': fields.NotificationSourceField(nullable=False),
}
def __init__(self, host, source):

23
nova/objects/fields.py
View File

@ -779,6 +779,25 @@ class NotificationPhase(BaseNovaEnum):
ALL = (START, END, ERROR)
class NotificationSource(BaseNovaEnum):
"""Represents possible nova binary service names in notification envelope.
The publisher_id field of the nova notifications consists of the name of
the host and the name of the service binary that emits the notification.
The below values are the ones that is used in every notification. Please
note that on the REST API the nova-api service binary is called
nova-osapi_compute. This is not reflected here as notifications always used
the name nova-api instead.
"""
COMPUTE = 'nova-compute'
API = 'nova-api'
CONDUCTOR = 'nova-conductor'
SCHEDULER = 'nova-scheduler'
ALL = (API, COMPUTE, CONDUCTOR, SCHEDULER)
class NotificationAction(BaseNovaEnum):
UPDATE = 'update'
EXCEPTION = 'exception'
@ -1188,6 +1207,10 @@ class NotificationActionField(BaseEnumField):
AUTO_TYPE = NotificationAction()
class NotificationSourceField(BaseEnumField):
AUTO_TYPE = NotificationSource()
class InstanceStateField(BaseEnumField):
AUTO_TYPE = InstanceState()

2
nova/objects/flavor.py
View File

@ -626,7 +626,7 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
payload = payload_type(self)
notification_type(
publisher=notification.NotificationPublisher(
host=CONF.host, source="nova-api"),
host=CONF.host, source=fields.NotificationSource.API),
event_type=notification.EventType(object="flavor",
action=action),
priority=fields.NotificationPriority.INFO,

2
nova/tests/unit/db/test_db_api.py
View File

@ -8253,7 +8253,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin):
def test_compute_node_statistics_with_other_service(self):
other_service = self.service_dict.copy()
other_service['topic'] = 'fake-topic'
other_service['binary'] = 'nova-fake'
other_service['binary'] = 'nova-api'
db.service_create(self.ctxt, other_service)
stats = db.compute_node_statistics(self.ctxt)

11
nova/tests/unit/notifications/objects/test_notification.py
View File

@ -117,7 +117,7 @@ class TestNotificationBase(test.NoDBTestCase):
'id': 123,
'uuid': uuids.service,
'host': 'fake-host',
'binary': 'nova-fake',
'binary': 'nova-compute',
'topic': 'fake-service-topic',
'report_count': 1,
'forced_down': False,
@ -166,7 +166,7 @@ class TestNotificationBase(test.NoDBTestCase):
expected_event_type,
expected_payload):
mock_notifier.prepare.assert_called_once_with(
publisher_id='nova-fake:fake-host')
publisher_id='nova-compute:fake-host')
mock_notify = mock_notifier.prepare.return_value.info
self.assertTrue(mock_notify.called)
self.assertEqual(mock_notify.call_args[0][0], mock_context)
@ -196,8 +196,9 @@ class TestNotificationBase(test.NoDBTestCase):
event_type=notification.EventType(
object='test_object',
action=fields.NotificationAction.UPDATE),
publisher=notification.NotificationPublisher(host='fake-host',
source='nova-fake'),
publisher=notification.NotificationPublisher(
host='fake-host',
source='nova-compute'),
priority=fields.NotificationPriority.INFO,
payload=self.payload)
@ -390,7 +391,7 @@ notification_object_data = {
'IpPayload': '1.0-8ecf567a99e516d4af094439a7632d34',
'KeypairNotification': '1.0-a73147b93b520ff0061865849d3dfa56',
'KeypairPayload': '1.0-6daebbbde0e1bf35c1556b1ecd9385c1',
'NotificationPublisher': '2.0-578ee5fea2922ff32e8917621f140857',
'NotificationPublisher': '2.1-9f89fe4abb80f9a7b726e59800c905de',
'ServiceStatusNotification': '1.0-a73147b93b520ff0061865849d3dfa56',
'ServiceStatusPayload': '1.1-7b6856bd879db7f3ecbcd0ca9f35f92f',
}

2
nova/tests/unit/objects/test_instance.py
View File

@ -171,7 +171,7 @@ class _TestInstanceObject(object):
fake_keypairlist.obj_to_primitive())
fake_service = {'created_at': None, 'updated_at': None,
'deleted_at': None, 'deleted': False, 'id': 123,
'host': 'fake-host', 'binary': 'nova-fake',
'host': 'fake-host', 'binary': 'nova-compute',
'topic': 'fake-service-topic', 'report_count': 1,
'forced_down': False, 'disabled': False,
'disabled_reason': None, 'last_seen_up': None,

2
nova/tests/unit/objects/test_service.py
View File

@ -42,7 +42,7 @@ def _fake_service(**kwargs):
'id': 123,
'uuid': uuidsentinel.service,
'host': 'fake-host',
'binary': 'nova-fake',
'binary': 'nova-compute',
'topic': 'fake-service-topic',
'report_count': 1,
'forced_down': False,

11
nova/tests/unit/test_exception.py
View File

@ -71,7 +71,7 @@ class WrapExceptionTestCase(test.NoDBTestCase):
def test_wrap_exception_unknown_module(self):
ctxt = context.get_admin_context()
wrapped = exception_wrapper.wrap_exception(
rpc.get_notifier('fake'), binary='fake-binary')
rpc.get_notifier('fake'), binary='nova-compute')
self.assertRaises(
TypeError, wrapped(bad_function_unknown_module), None, ctxt)
self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS))
@ -81,7 +81,7 @@ class WrapExceptionTestCase(test.NoDBTestCase):
def test_wrap_exception_with_notifier(self):
wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'),
binary='fake-binary')
binary='nova-compute')
ctxt = context.get_admin_context()
self.assertRaises(test.TestingException,
wrapped(bad_function_exception), 1, ctxt, 3, zoo=3)
@ -98,7 +98,8 @@ class WrapExceptionTestCase(test.NoDBTestCase):
self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS))
notification = fake_notifier.VERSIONED_NOTIFICATIONS[0]
self.assertEqual('compute.exception', notification['event_type'])
self.assertEqual('fake-binary:fake-mini', notification['publisher_id'])
self.assertEqual('nova-compute:fake-mini',
notification['publisher_id'])
self.assertEqual('ERROR', notification['priority'])
payload = notification['payload']
@ -120,7 +121,7 @@ class WrapExceptionTestCase(test.NoDBTestCase):
mock_notifier.is_enabled.return_value = False
wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'),
binary='fake-binary')
binary='nova-compute')
ctxt = context.get_admin_context()
self.assertRaises(test.TestingException,
wrapped(bad_function_exception), 1, ctxt, 3, zoo=3)
@ -133,7 +134,7 @@ class WrapExceptionTestCase(test.NoDBTestCase):
self.flags(notification_format='unversioned', group='notifications')
wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'),
binary='fake-binary')
binary='nova-compute')
ctxt = context.get_admin_context()
self.assertRaises(test.TestingException,
wrapped(bad_function_exception), 1, ctxt, 3, zoo=3)

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

@ -417,16 +417,16 @@ class NotificationsTestCase(test.TestCase):
def test_update_with_service_name(self):
notifications.send_update_with_states(self.context, self.instance,
vm_states.BUILDING, vm_states.BUILDING, task_states.SPAWNING,
None, service="testservice")
None, service="nova-compute")
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS))
# service name should default to 'compute'
notif = fake_notifier.NOTIFICATIONS[0]
self.assertEqual('testservice.testhost', notif.publisher_id)
self.assertEqual('nova-compute.testhost', notif.publisher_id)
notif = fake_notifier.VERSIONED_NOTIFICATIONS[0]
self.assertEqual('nova-testservice:testhost', notif['publisher_id'])
self.assertEqual('nova-compute:testhost', notif['publisher_id'])
def test_update_with_host_name(self):
notifications.send_update_with_states(self.context, self.instance,

4
nova/tests/unit/test_service.py
View File

@ -89,7 +89,7 @@ class ServiceTestCase(test.NoDBTestCase):
def setUp(self):
super(ServiceTestCase, self).setUp()
self.host = 'foo'
self.binary = 'nova-fake'
self.binary = 'nova-compute'
self.topic = 'fake'
def test_create(self):
@ -109,7 +109,7 @@ class ServiceTestCase(test.NoDBTestCase):
self.binary,
self.topic,
'nova.tests.unit.test_service.FakeManager')
exp = "<Service: host=foo, binary=nova-fake, " \
exp = "<Service: host=foo, binary=nova-compute, " \
"manager_class_name=nova.tests.unit.test_service.FakeManager>"
self.assertEqual(exp, repr(serv))