From 7c0132c5f4e12818cdc246760fc97e6667fc3aee Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 6 Jul 2017 12:59:04 +0200 Subject: [PATCH] Use enum value instead of string service name 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 --- nova/compute/utils.py | 16 +++++++------ nova/notifications/objects/base.py | 7 ++++-- nova/objects/fields.py | 23 +++++++++++++++++++ nova/objects/flavor.py | 2 +- nova/tests/unit/db/test_db_api.py | 2 +- .../objects/test_notification.py | 11 +++++---- nova/tests/unit/objects/test_instance.py | 2 +- nova/tests/unit/objects/test_service.py | 2 +- nova/tests/unit/test_exception.py | 11 +++++---- nova/tests/unit/test_notifications.py | 6 ++--- nova/tests/unit/test_service.py | 4 ++-- 11 files changed, 58 insertions(+), 28 deletions(-) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 68945662ccf4..c3d55f1a5201 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -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, diff --git a/nova/notifications/objects/base.py b/nova/notifications/objects/base.py index 02de61d4ac43..ec873eef7cea 100644 --- a/nova/notifications/objects/base.py +++ b/nova/notifications/objects/base.py @@ -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): diff --git a/nova/objects/fields.py b/nova/objects/fields.py index ab5f8f900d92..c852ed0d592f 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -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() diff --git a/nova/objects/flavor.py b/nova/objects/flavor.py index 3da9be2e0f33..cfbfb21301d3 100644 --- a/nova/objects/flavor.py +++ b/nova/objects/flavor.py @@ -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, diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index a373aae43bce..c78a5bdcb0c1 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -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) diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index fbb17f02edf1..2ea8224dd6c6 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -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', } diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 4992f309c4f0..778b614257b4 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -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, diff --git a/nova/tests/unit/objects/test_service.py b/nova/tests/unit/objects/test_service.py index 7a4a5e925379..55198ae19d53 100644 --- a/nova/tests/unit/objects/test_service.py +++ b/nova/tests/unit/objects/test_service.py @@ -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, diff --git a/nova/tests/unit/test_exception.py b/nova/tests/unit/test_exception.py index 0c698e26c265..3089ea21656f 100644 --- a/nova/tests/unit/test_exception.py +++ b/nova/tests/unit/test_exception.py @@ -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) diff --git a/nova/tests/unit/test_notifications.py b/nova/tests/unit/test_notifications.py index 6de2aae78568..e49edd622648 100644 --- a/nova/tests/unit/test_notifications.py +++ b/nova/tests/unit/test_notifications.py @@ -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, diff --git a/nova/tests/unit/test_service.py b/nova/tests/unit/test_service.py index e5eeb87ecd3b..7d08bfb42275 100644 --- a/nova/tests/unit/test_service.py +++ b/nova/tests/unit/test_service.py @@ -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 = "" self.assertEqual(exp, repr(serv))