openstack/nova
Code Issues Proposed changes

Add separate instance.create payload type

Browse Source 
There are two parallel efforts to add new fields to the instance.create
notification only. Currently instance.create notification uses the generic
InstanceActionPayload ovo as the payload type. This patch creates a separate
InstanceCreatePayload ovo so that fields like keypairs and tags can be added
only to this specific payload class later in separate patches.

The keypair and tags field are not added directly to the base InstancePayload
class because both field need extra db query to fetch. So sending them at
every instance action (e.g. reboot) could cause db load.

The version of the instance.create payload is not bumped as the content
of the payload is the same just the name of the type is changed.

Change-Id: I90fb7beb450100de8f4e542921b99f043c529a0f
This commit is contained in:
Balazs Gibizer
2017-05-05 17:56:55 +02:00
parent 8238e8aaa0
commit 3b72d60589
9 changed files with 115 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
doc/notification_samples/instance-create-end.json
View File

@@ -66,7 +66,7 @@
"user_id":"fake",
"uuid":"178b0921-8f85-4257-88b6-2e743b5a975c"
},
"nova_object.name":"InstanceActionPayload",
"nova_object.name":"InstanceCreatePayload",
"nova_object.namespace":"nova",
"nova_object.version":"1.2"
},

2
doc/notification_samples/instance-create-error.json
View File

@@ -63,7 +63,7 @@
"user_id":"fake",
"uuid":"178b0921-8f85-4257-88b6-2e743b5a975c"
},
"nova_object.name":"InstanceActionPayload",
"nova_object.name":"InstanceCreatePayload",
"nova_object.namespace":"nova",
"nova_object.version":"1.2"
},

2
doc/notification_samples/instance-create-start.json
View File

@@ -53,7 +53,7 @@
"user_id":"fake",
"uuid":"178b0921-8f85-4257-88b6-2e743b5a975c"
},
"nova_object.name":"InstanceActionPayload",
"nova_object.name":"InstanceCreatePayload",
"nova_object.namespace":"nova",
"nova_object.version":"1.2"
},

32
nova/compute/manager.py
View File

@@ -1918,9 +1918,8 @@ class ComputeManager(manager.Manager):
image_name = image.get('name')
self._notify_about_instance_usage(context, instance, 'create.start',
extra_usage_info={'image_name': image_name})
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.START)
# NOTE(mikal): cache the keystone roles associated with the instance
@@ -1966,17 +1965,15 @@ class ComputeManager(manager.Manager):
with excutils.save_and_reraise_exception():
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
except exception.ComputeResourcesUnavailable as e:
LOG.debug(e.format_message(), instance=instance)
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
raise exception.RescheduledException(
instance_uuid=instance.uuid, reason=e.format_message())
@@ -1985,9 +1982,8 @@ class ComputeManager(manager.Manager):
LOG.debug(e.format_message(), instance=instance)
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
except (exception.FixedIpLimitExceeded,
exception.NoMoreNetworks, exception.NoMoreFixedIps) as e:
@@ -1995,9 +1991,8 @@ class ComputeManager(manager.Manager):
instance=instance)
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
msg = _('Failed to allocate the network(s) with error %s, '
'not rescheduling.') % e.format_message()
@@ -2011,9 +2006,8 @@ class ComputeManager(manager.Manager):
instance=instance)
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
msg = _('Failed to allocate the network(s), not rescheduling.')
raise exception.BuildAbortException(instance_uuid=instance.uuid,
@@ -2029,18 +2023,16 @@ class ComputeManager(manager.Manager):
exception.InvalidInput) as e:
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
raise exception.BuildAbortException(instance_uuid=instance.uuid,
reason=e.format_message())
except Exception as e:
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
raise exception.RescheduledException(
instance_uuid=instance.uuid, reason=six.text_type(e))
@@ -2073,18 +2065,16 @@ class ComputeManager(manager.Manager):
with excutils.save_and_reraise_exception():
self._notify_about_instance_usage(context, instance,
'create.error', fault=e)
compute_utils.notify_about_instance_action(
compute_utils.notify_about_instance_create(
context, instance, self.host,
action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.ERROR, exception=e)
self._update_scheduler_instance_info(context, instance)
self._notify_about_instance_usage(context, instance, 'create.end',
extra_usage_info={'message': _('Success')},
network_info=network_info)
compute_utils.notify_about_instance_action(context, instance,
self.host, action=fields.NotificationAction.CREATE,
phase=fields.NotificationPhase.END)
compute_utils.notify_about_instance_create(context, instance,
self.host, phase=fields.NotificationPhase.END)
@contextlib.contextmanager
def _build_resources(self, context, instance, requested_networks,

28
nova/compute/utils.py
View File

@@ -365,6 +365,34 @@ def notify_about_instance_action(context, instance, host, action, phase=None,
notification.emit(context)
def notify_about_instance_create(context, instance, host, phase=None,
binary='nova-compute', exception=None):
"""Send versioned notification about instance creation
:param context: the request context
:param instance: the instance being created
:param host: the host emitting the notification
:param phase: the phase of the creation
:param binary: the binary emitting the notification
:param exception: the thrown exception (used in error notifications)
"""
fault, priority = _get_fault_and_priority_from_exc(exception)
payload = instance_notification.InstanceCreatePayload(
instance=instance,
fault=fault)
notification = instance_notification.InstanceCreateNotification(
context=context,
priority=priority,
publisher=notification_base.NotificationPublisher(
host=host, binary=binary),
event_type=notification_base.EventType(
object='instance',
action=fields.NotificationAction.CREATE,
phase=phase),
payload=payload)
notification.emit(context)
def notify_about_volume_attach_detach(context, instance, host, action, phase,
binary='nova-compute', volume_id=None,
exception=None):

33
nova/notifications/objects/instance.py
View File

@@ -153,6 +153,23 @@ class InstanceActionVolumeSwapPayload(InstanceActionPayload):
self.new_volume_id = new_volume_id
@nova_base.NovaObjectRegistry.register_notification
class InstanceCreatePayload(InstanceActionPayload):
# No SCHEMA as all the additional fields are calculated
# Version 1.2: Initial version. It starts at 1.2 to match with the version
# of the InstanceActionPayload at the time when this specific
# payload is created as a child of it so that the
# instance.create notification using this new payload does not
# have decreasing version.
VERSION = '1.2'
def __init__(self, instance, fault):
super(InstanceCreatePayload, self).__init__(
instance=instance,
fault=fault)
@nova_base.NovaObjectRegistry.register_notification
class InstanceUpdatePayload(InstancePayload):
# Version 1.0: Initial version
@@ -339,9 +356,6 @@ class InstanceStateUpdatePayload(base.NotificationPayloadBase):
# @base.notification_sample('instance-unrescue-end.json')
@base.notification_sample('instance-unshelve-start.json')
@base.notification_sample('instance-unshelve-end.json')
@base.notification_sample('instance-create-start.json')
@base.notification_sample('instance-create-end.json')
@base.notification_sample('instance-create-error.json')
@nova_base.NovaObjectRegistry.register_notification
class InstanceActionNotification(base.NotificationBase):
# Version 1.0: Initial version
@@ -389,3 +403,16 @@ class InstanceActionVolumeNotification(base.NotificationBase):
fields = {
'payload': fields.ObjectField('InstanceActionVolumePayload')
}
@base.notification_sample('instance-create-start.json')
@base.notification_sample('instance-create-end.json')
@base.notification_sample('instance-create-error.json')
@nova_base.NovaObjectRegistry.register_notification
class InstanceCreateNotification(base.NotificationBase):
# Version 1.0: Initial version
VERSION = '1.0'
fields = {
'payload': fields.ObjectField('InstanceCreatePayload')
}

16
nova/tests/unit/compute/test_compute_mgr.py
View File

@@ -4360,7 +4360,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
'_shutdown_instance'),
mock.patch.object(self.compute,
'_validate_instance_group_policy'),
mock.patch('nova.compute.utils.notify_about_instance_action')
mock.patch('nova.compute.utils.notify_about_instance_create')
) as (spawn, save,
_build_networks_for_instance, _notify_about_instance_usage,
_shutdown_instance, _validate_instance_group_policy,
@@ -4387,9 +4387,9 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
mock_notify.assert_has_calls([
mock.call(self.context, self.instance, 'fake-mini',
action='create', phase='start'),
phase='start'),
mock.call(self.context, self.instance, 'fake-mini',
action='create', phase='error', exception=exc)])
phase='error', exception=exc)])
save.assert_has_calls([
mock.call(),
@@ -4797,10 +4797,10 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.context, mock.sentinel.instance,
requested_networks=requested_networks)
@mock.patch('nova.compute.utils.notify_about_instance_action')
@mock.patch('nova.compute.utils.notify_about_instance_create')
@mock.patch.object(manager.ComputeManager, '_instance_update')
def test_launched_at_in_create_end_notification(self,
mock_instance_update, mock_notify_instance_action):
mock_instance_update, mock_notify_instance_create):
def fake_notify(*args, **kwargs):
if args[2] == 'create.end':
@@ -4829,11 +4829,11 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
mock_notify.call_count - 1]
self.assertEqual(expected_call, create_end_call)
mock_notify_instance_action.assert_has_calls([
mock_notify_instance_create.assert_has_calls([
mock.call(self.context, self.instance, 'fake-mini',
action='create', phase='start'),
phase='start'),
mock.call(self.context, self.instance, 'fake-mini',
action='create', phase='end')])
phase='end')])
def test_access_ip_set_when_instance_set_to_active(self):

33
nova/tests/unit/compute/test_compute_utils.py
View File

@@ -518,6 +518,39 @@ class UsageInfoTestCase(test.TestCase):
self.assertEqual(payload['image_uuid'], uuids.fake_image_ref)
def test_notify_about_instance_create(self):
instance = create_instance(self.context)
compute_utils.notify_about_instance_create(
self.context,
instance,
host='fake-compute',
phase='start')
self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS))
notification = fake_notifier.VERSIONED_NOTIFICATIONS[0]
self.assertEqual('INFO', notification['priority'])
self.assertEqual('instance.create.start', notification['event_type'])
self.assertEqual('nova-compute:fake-compute',
notification['publisher_id'])
payload = notification['payload']['nova_object.data']
self.assertEqual('fake', payload['tenant_id'])
self.assertEqual('fake', payload['user_id'])
self.assertEqual(instance['uuid'], payload['uuid'])
flavorid = flavors.get_flavor_by_name('m1.tiny')['flavorid']
flavor = payload['flavor']['nova_object.data']
self.assertEqual(flavorid, str(flavor['flavorid']))
for attr in ('display_name', 'created_at', 'launched_at',
'state', 'task_state', 'display_description', 'locked',
'auto_disk_config'):
self.assertIn(attr, payload, "Key %s not in payload" % attr)
self.assertEqual(uuids.fake_image_ref, payload['image_uuid'])
def test_notify_about_volume_swap(self):
instance = create_instance(self.context)

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

@@ -381,6 +381,8 @@ notification_object_data = {
'InstanceActionVolumeSwapNotification':
'1.0-a73147b93b520ff0061865849d3dfa56',
'InstanceActionVolumeSwapPayload': '1.2-d7925b763e0795f8e5c1aa0e95bd67bd',
'InstanceCreateNotification': '1.0-a73147b93b520ff0061865849d3dfa56',
'InstanceCreatePayload': '1.2-b7b2481bcd0e1edcc1970ef7150df5aa',
'InstancePayload': '1.2-a1988f6fe728bd4b478353a85c48ad55',
'InstanceStateUpdatePayload': '1.0-07e111c0fa0f6db0f79b0726d593e3da',
'InstanceUpdateNotification': '1.0-a73147b93b520ff0061865849d3dfa56',