From 94de8d75ff08bfc63d0d4a68083783b2a878508d Mon Sep 17 00:00:00 2001 From: Kevin_Zheng Date: Thu, 15 Mar 2018 17:21:50 +0800 Subject: [PATCH] Add request_id to instance action notifications As it was agreed on the Rocky PTG [1] it is useful to have the request_id of in the payload of every instance action versioned notification. For example it could help the deployer connect the state change described in the notification with the user action, the request, on the REST API. So this patch proposes to extend the InstanceActionPayload versioned object with a new request_id field and populate the request_id from the context object used for emitting the instance action notifications. [1] https://etherpad.openstack.org/p/nova-ptg-rocky L391 Implements: bp add-request-id-to-instance-action-notifications Change-Id: I7243b60938d6e9c7c2bc2aacdba5c667cca8ec9b --- .../InstanceActionPayload.json | 2 +- .../InstanceActionRescuePayload.json | 2 +- .../InstanceActionResizePrepPayload.json | 2 +- .../InstanceActionSnapshotPayload.json | 2 +- .../InstanceActionVolumePayload.json | 2 +- .../InstanceActionVolumeSwapPayload.json | 2 +- .../InstanceCreatePayload.json | 2 +- .../common_payloads/InstancePayload.json | 5 +- .../InstanceUpdatePayload.json | 2 +- nova/compute/utils.py | 7 ++ nova/notifications/base.py | 1 + nova/notifications/objects/instance.py | 70 +++++++++++++------ .../notification_sample_base.py | 7 ++ .../notifications/objects/test_instance.py | 35 ++++++++++ .../objects/test_notification.py | 24 ++++--- ...sioned-notifications-fd0b525bd37b7e41.yaml | 9 +++ 16 files changed, 132 insertions(+), 42 deletions(-) create mode 100644 nova/tests/unit/notifications/objects/test_instance.py create mode 100644 releasenotes/notes/add-req-id-to-versioned-notifications-fd0b525bd37b7e41.yaml diff --git a/doc/notification_samples/common_payloads/InstanceActionPayload.json b/doc/notification_samples/common_payloads/InstanceActionPayload.json index d76ac8d501..b6b4eb37e6 100644 --- a/doc/notification_samples/common_payloads/InstanceActionPayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionPayload.json @@ -5,5 +5,5 @@ }, "nova_object.name":"InstanceActionPayload", "nova_object.namespace":"nova", - "nova_object.version":"1.5" + "nova_object.version":"1.6" } diff --git a/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json b/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json index 8938ec1858..74fe77fff4 100644 --- a/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json @@ -4,5 +4,5 @@ "rescue_image_ref": "a2459075-d96c-40d5-893e-577ff92e721c" }, "nova_object.name": "InstanceActionRescuePayload", - "nova_object.version": "1.0" + "nova_object.version": "1.1" } diff --git a/doc/notification_samples/common_payloads/InstanceActionResizePrepPayload.json b/doc/notification_samples/common_payloads/InstanceActionResizePrepPayload.json index 1fff907888..c5bc759020 100644 --- a/doc/notification_samples/common_payloads/InstanceActionResizePrepPayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionResizePrepPayload.json @@ -27,5 +27,5 @@ "task_state": "resize_prep" }, "nova_object.name": "InstanceActionResizePrepPayload", - "nova_object.version": "1.0" + "nova_object.version": "1.1" } diff --git a/doc/notification_samples/common_payloads/InstanceActionSnapshotPayload.json b/doc/notification_samples/common_payloads/InstanceActionSnapshotPayload.json index fa3655c48d..32370569f5 100644 --- a/doc/notification_samples/common_payloads/InstanceActionSnapshotPayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionSnapshotPayload.json @@ -5,5 +5,5 @@ }, "nova_object.name":"InstanceActionSnapshotPayload", "nova_object.namespace":"nova", - "nova_object.version":"1.6" + "nova_object.version":"1.7" } diff --git a/doc/notification_samples/common_payloads/InstanceActionVolumePayload.json b/doc/notification_samples/common_payloads/InstanceActionVolumePayload.json index 4e49c42a06..5610cdf022 100644 --- a/doc/notification_samples/common_payloads/InstanceActionVolumePayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionVolumePayload.json @@ -5,5 +5,5 @@ }, "nova_object.name": "InstanceActionVolumePayload", "nova_object.namespace": "nova", - "nova_object.version": "1.3" + "nova_object.version": "1.4" } \ No newline at end of file diff --git a/doc/notification_samples/common_payloads/InstanceActionVolumeSwapPayload.json b/doc/notification_samples/common_payloads/InstanceActionVolumeSwapPayload.json index 943cc7b841..4737095e41 100644 --- a/doc/notification_samples/common_payloads/InstanceActionVolumeSwapPayload.json +++ b/doc/notification_samples/common_payloads/InstanceActionVolumeSwapPayload.json @@ -6,5 +6,5 @@ }, "nova_object.name": "InstanceActionVolumeSwapPayload", "nova_object.namespace": "nova", - "nova_object.version": "1.5" + "nova_object.version": "1.6" } diff --git a/doc/notification_samples/common_payloads/InstanceCreatePayload.json b/doc/notification_samples/common_payloads/InstanceCreatePayload.json index 85cda56315..f549f6b2c4 100644 --- a/doc/notification_samples/common_payloads/InstanceCreatePayload.json +++ b/doc/notification_samples/common_payloads/InstanceCreatePayload.json @@ -19,5 +19,5 @@ "tags": ["tag"] }, "nova_object.name":"InstanceCreatePayload", - "nova_object.version": "1.7" + "nova_object.version": "1.8" } diff --git a/doc/notification_samples/common_payloads/InstancePayload.json b/doc/notification_samples/common_payloads/InstancePayload.json index 9ee6d579bd..aabc0eaaca 100644 --- a/doc/notification_samples/common_payloads/InstancePayload.json +++ b/doc/notification_samples/common_payloads/InstancePayload.json @@ -34,9 +34,10 @@ "flavor": {"$ref": "FlavorPayload.json#"}, "updated_at": "2012-10-29T13:42:11Z", "user_id":"fake", - "uuid":"178b0921-8f85-4257-88b6-2e743b5a975c" + "uuid":"178b0921-8f85-4257-88b6-2e743b5a975c", + "request_id": "req-5b6c791d-5709-4f36-8fbe-c3e02869e35d" }, "nova_object.name":"InstancePayload", "nova_object.namespace":"nova", - "nova_object.version":"1.5" + "nova_object.version":"1.6" } diff --git a/doc/notification_samples/common_payloads/InstanceUpdatePayload.json b/doc/notification_samples/common_payloads/InstanceUpdatePayload.json index 8b84a074ad..088fb1331e 100644 --- a/doc/notification_samples/common_payloads/InstanceUpdatePayload.json +++ b/doc/notification_samples/common_payloads/InstanceUpdatePayload.json @@ -29,5 +29,5 @@ }, "nova_object.name": "InstanceUpdatePayload", "nova_object.namespace": "nova", - "nova_object.version": "1.6" + "nova_object.version": "1.7" } \ No newline at end of file diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 0a86f1ec03..a04b6380fe 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -368,6 +368,7 @@ def notify_about_instance_action(context, instance, host, action, phase=None, """ fault, priority = _get_fault_and_priority_from_exc(exception) payload = instance_notification.InstanceActionPayload( + context=context, instance=instance, fault=fault, bdms=bdms) @@ -399,6 +400,7 @@ def notify_about_instance_create(context, instance, host, phase=None, """ fault, priority = _get_fault_and_priority_from_exc(exception) payload = instance_notification.InstanceCreatePayload( + context=context, instance=instance, fault=fault, bdms=bdms) @@ -428,6 +430,7 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, """ fault, priority = _get_fault_and_priority_from_exc(exception) payload = instance_notification.InstanceActionVolumePayload( + context=context, instance=instance, fault=fault, volume_id=volume_id) @@ -457,6 +460,7 @@ def notify_about_instance_rescue_action( """ fault, priority = _get_fault_and_priority_from_exc(exception) payload = instance_notification.InstanceActionRescuePayload( + context=context, instance=instance, fault=fault, rescue_image_ref=rescue_image_ref) @@ -512,6 +516,7 @@ def notify_about_volume_swap(context, instance, host, phase, """ fault, priority = _get_fault_and_priority_from_exc(exception) payload = instance_notification.InstanceActionVolumeSwapPayload( + context=context, instance=instance, fault=fault, old_volume_id=old_volume_id, @@ -542,6 +547,7 @@ def notify_about_instance_snapshot(context, instance, host, phase, :param snapshot_image_id: the ID of the snapshot """ payload = instance_notification.InstanceActionSnapshotPayload( + context=context, instance=instance, fault=None, snapshot_image_id=snapshot_image_id) @@ -572,6 +578,7 @@ def notify_about_resize_prep_instance(context, instance, host, phase, """ payload = instance_notification.InstanceActionResizePrepPayload( + context=context, instance=instance, fault=None, new_flavor=flavor_notification.FlavorPayload(flavor=new_flavor)) diff --git a/nova/notifications/base.py b/nova/notifications/base.py index 14606cf581..d18deed450 100644 --- a/nova/notifications/base.py +++ b/nova/notifications/base.py @@ -263,6 +263,7 @@ def _send_versioned_instance_update(context, instance, payload, host, service): for label, bw in payload['bandwidth'].items()] versioned_payload = instance_notification.InstanceUpdatePayload( + context=context, instance=instance, state_update=state_update, audit_period=audit_period, diff --git a/nova/notifications/objects/instance.py b/nova/notifications/objects/instance.py index 1ef22d7819..64c4f1ad96 100644 --- a/nova/notifications/objects/instance.py +++ b/nova/notifications/objects/instance.py @@ -65,7 +65,8 @@ class InstancePayload(base.NotificationPayloadBase): # Version 1.3: Add key_name field # Version 1.4: Add BDM related data # Version 1.5: Add updated_at field - VERSION = '1.5' + # Version 1.6: Add request_id field + VERSION = '1.6' fields = { 'uuid': fields.UUIDField(), 'user_id': fields.StringField(nullable=True), @@ -105,10 +106,12 @@ class InstancePayload(base.NotificationPayloadBase): 'metadata': fields.DictOfStringsField(), 'locked': fields.BooleanField(), - 'auto_disk_config': fields.DiskConfigField() + 'auto_disk_config': fields.DiskConfigField(), + + 'request_id': fields.StringField(nullable=True), } - def __init__(self, instance, bdms=None): + def __init__(self, context, instance, bdms=None): super(InstancePayload, self).__init__() network_info = instance.get_network_info() self.ip_addresses = IpPayload.from_network_info(network_info) @@ -117,6 +120,12 @@ class InstancePayload(base.NotificationPayloadBase): self.block_devices = BlockDevicePayload.from_bdms(bdms) else: self.block_devices = BlockDevicePayload.from_instance(instance) + # NOTE(Kevin_Zheng): Don't include request_id for periodic tasks, + # RequestContext for periodic tasks does not include project_id + # and user_id. Consider modify this once periodic tasks got a + # consistent request_id. + self.request_id = context.request_id if (context.project_id and + context.user_id) else None self.populate_schema(instance=instance) @@ -130,13 +139,16 @@ class InstanceActionPayload(InstancePayload): # Version 1.3: Added key_name field to InstancePayload # Version 1.4: Add BDM related data # Version 1.5: Added updated_at field to InstancePayload - VERSION = '1.5' + # Version 1.6: Added request_id field to InstancePayload + VERSION = '1.6' fields = { 'fault': fields.ObjectField('ExceptionPayload', nullable=True), + 'request_id': fields.StringField(nullable=True), } - def __init__(self, instance, fault, bdms=None): - super(InstanceActionPayload, self).__init__(instance=instance, + def __init__(self, context, instance, fault, bdms=None): + super(InstanceActionPayload, self).__init__(context=context, + instance=instance, bdms=bdms) self.fault = fault @@ -147,14 +159,16 @@ class InstanceActionVolumePayload(InstanceActionPayload): # Version 1.1: Added key_name field to InstancePayload # Version 1.2: Add BDM related data # Version 1.3: Added updated_at field to InstancePayload + # Version 1.4: Added request_id field to InstancePayload - VERSION = '1.3' + VERSION = '1.4' fields = { 'volume_id': fields.UUIDField() } - def __init__(self, instance, fault, volume_id): + def __init__(self, context, instance, fault, volume_id): super(InstanceActionVolumePayload, self).__init__( + context=context, instance=instance, fault=fault) self.volume_id = volume_id @@ -169,14 +183,16 @@ class InstanceActionVolumeSwapPayload(InstanceActionPayload): # Version 1.3: Added key_name field to InstancePayload # Version 1.4: Add BDM related data # Version 1.5: Added updated_at field to InstancePayload - VERSION = '1.5' + # Version 1.6: Added request_id field to InstancePayload + VERSION = '1.6' fields = { 'old_volume_id': fields.UUIDField(), 'new_volume_id': fields.UUIDField(), } - def __init__(self, instance, fault, old_volume_id, new_volume_id): + def __init__(self, context, instance, fault, old_volume_id, new_volume_id): super(InstanceActionVolumeSwapPayload, self).__init__( + context=context, instance=instance, fault=fault) self.old_volume_id = old_volume_id @@ -197,15 +213,17 @@ class InstanceCreatePayload(InstanceActionPayload): # 1.5: Add BDM related data to InstancePayload # 1.6: Add tags field to InstanceCreatePayload # 1.7: Added updated_at field to InstancePayload - VERSION = '1.7' + # 1.8: Added request_id field to InstancePayload + VERSION = '1.8' fields = { 'keypairs': fields.ListOfObjectsField('KeypairPayload'), 'tags': fields.ListOfStringsField(), } - def __init__(self, instance, fault, bdms): + def __init__(self, context, instance, fault, bdms): super(InstanceCreatePayload, self).__init__( + context=context, instance=instance, fault=fault, bdms=bdms) @@ -220,13 +238,15 @@ class InstanceActionResizePrepPayload(InstanceActionPayload): # No SCHEMA as all the additional fields are calculated # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Added request_id field to InstancePayload + VERSION = '1.1' fields = { 'new_flavor': fields.ObjectField('FlavorPayload', nullable=True) } - def __init__(self, instance, fault, new_flavor): + def __init__(self, context, instance, fault, new_flavor): super(InstanceActionResizePrepPayload, self).__init__( + context=context, instance=instance, fault=fault) self.new_flavor = new_flavor @@ -241,7 +261,8 @@ class InstanceUpdatePayload(InstancePayload): # Version 1.4: Added key_name field to InstancePayload # Version 1.5: Add BDM related data # Version 1.6: Added updated_at field to InstancePayload - VERSION = '1.6' + # Version 1.7: Added request_id field to InstancePayload + VERSION = '1.7' fields = { 'state_update': fields.ObjectField('InstanceStateUpdatePayload'), 'audit_period': fields.ObjectField('AuditPeriodPayload'), @@ -250,9 +271,10 @@ class InstanceUpdatePayload(InstancePayload): 'tags': fields.ListOfStringsField(), } - def __init__(self, instance, state_update, audit_period, bandwidth, - old_display_name): - super(InstanceUpdatePayload, self).__init__(instance=instance) + def __init__(self, context, instance, state_update, audit_period, + bandwidth, old_display_name): + super(InstanceUpdatePayload, self).__init__( + context=context, instance=instance) self.state_update = state_update self.audit_period = audit_period self.bandwidth = bandwidth @@ -264,13 +286,15 @@ class InstanceUpdatePayload(InstancePayload): @nova_base.NovaObjectRegistry.register_notification class InstanceActionRescuePayload(InstanceActionPayload): # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Added request_id field to InstancePayload + VERSION = '1.1' fields = { 'rescue_image_ref': fields.UUIDField(nullable=True) } - def __init__(self, instance, fault, rescue_image_ref): + def __init__(self, context, instance, fault, rescue_image_ref): super(InstanceActionRescuePayload, self).__init__( + context=context, instance=instance, fault=fault) self.rescue_image_ref = rescue_image_ref @@ -591,13 +615,15 @@ class InstanceActionSnapshotPayload(InstanceActionPayload): # from using InstanceActionPayload 1.5 to this new payload and # also it added a new field so we wanted to keep the version # number increasing to signal the change. - VERSION = '1.6' + # Version 1.7: Added request_id field to InstancePayload + VERSION = '1.7' fields = { 'snapshot_image_id': fields.UUIDField(), } - def __init__(self, instance, fault, snapshot_image_id): + def __init__(self, context, instance, fault, snapshot_image_id): super(InstanceActionSnapshotPayload, self).__init__( + context=context, instance=instance, fault=fault) self.snapshot_image_id = snapshot_image_id diff --git a/nova/tests/functional/notification_sample_tests/notification_sample_base.py b/nova/tests/functional/notification_sample_tests/notification_sample_base.py index 6c27ded349..56245c5c90 100644 --- a/nova/tests/functional/notification_sample_tests/notification_sample_base.py +++ b/nova/tests/functional/notification_sample_tests/notification_sample_base.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import os import time @@ -84,6 +85,12 @@ class NotificationSampleTestBase(test.TestCase, self.addCleanup(nova.tests.unit.image.fake.FakeImageService_reset) self.useFixture(nova_fixtures.PlacementFixture()) + context_patcher = self.mock_gen_request_id = mock.patch( + 'oslo_context.context.generate_request_id', + return_value='req-5b6c791d-5709-4f36-8fbe-c3e02869e35d') + self.mock_gen_request_id = context_patcher.start() + self.addCleanup(context_patcher.stop) + self.start_service('conductor') self.start_service('scheduler') self.start_service('network', manager=CONF.network_manager) diff --git a/nova/tests/unit/notifications/objects/test_instance.py b/nova/tests/unit/notifications/objects/test_instance.py new file mode 100644 index 0000000000..8ddded045a --- /dev/null +++ b/nova/tests/unit/notifications/objects/test_instance.py @@ -0,0 +1,35 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova import context as nova_context +from nova.network import model as network_model +from nova.notifications.objects import instance as instance_notification +from nova import objects +from nova import test +from nova.tests.unit import fake_instance + + +class TestInstanceNotification(test.NoDBTestCase): + + def test_instance_payload_request_id_periodic_task(self): + """Tests that creating an InstancePayload from the type of request + context used during a periodic task will not populate the + payload request_id field since it is not an end user request. + """ + ctxt = nova_context.get_admin_context() + instance = fake_instance.fake_instance_obj(ctxt) + # Set some other fields otherwise populate_schema tries to hit the DB. + instance.metadata = {} + instance.info_cache = objects.InstanceInfoCache( + network_info=network_model.NetworkInfo([])) + payload = instance_notification.InstancePayload(ctxt, instance) + self.assertIsNone(payload.request_id) diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 237bc29ded..c07f77d266 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -379,26 +379,26 @@ notification_object_data = { 'FlavorNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'FlavorPayload': '1.4-2e7011b8b4e59167fe8b7a0a81f0d452', 'InstanceActionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionPayload': '1.5-fb2804ce9b681bfb217e729153c22611', + 'InstanceActionPayload': '1.6-e9e4cbb94e07d3bcaa22743f41e094c8', 'InstanceActionRescueNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionRescuePayload': '1.0-a29f3339d0b8c3bcc997ab5d19d898d5', + 'InstanceActionRescuePayload': '1.1-99b9b25574b77abf6d3e5a0cea341b06', 'InstanceActionResizePrepNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionResizePrepPayload': '1.0-3a23d3dd6516964a51c256b2f8b4646c', + 'InstanceActionResizePrepPayload': '1.1-9dd5cd4124c660a86e3f00a2df222b8e', 'InstanceActionVolumeNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionVolumePayload': '1.3-f175b22ac6d6d0aea2bac21e12156e77', + 'InstanceActionVolumePayload': '1.4-83fcb4c12327da998116844ef4a16235', 'InstanceActionVolumeSwapNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionVolumeSwapPayload': '1.5-bccb88cda36276d20a9b3e427b999929', + 'InstanceActionVolumeSwapPayload': '1.6-bb322fd649d3626c7a83d5f2d9a866d4', 'InstanceCreateNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceCreatePayload': '1.7-a35b2f3aa64dcc262ebb830e78939bdb', - 'InstancePayload': '1.5-201d852973dbcb5caab89082a3140487', + 'InstanceCreatePayload': '1.8-aab72bba998af21dc2e34b31e3c376ea', + 'InstancePayload': '1.6-b1e7818c7adf158e8a6e87e0944b0b21', 'InstanceActionSnapshotNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceActionSnapshotPayload': '1.6-6f96ad137957d802aac94c90337fd950', + 'InstanceActionSnapshotPayload': '1.7-73f96d93ca47750bb6a45e4ab1d268fd', 'InstanceStateUpdatePayload': '1.0-07e111c0fa0f6db0f79b0726d593e3da', 'InstanceUpdateNotification': '1.0-a73147b93b520ff0061865849d3dfa56', - 'InstanceUpdatePayload': '1.6-9145c7cac4208eb841ceaaa9c10b2d9b', + 'InstanceUpdatePayload': '1.7-d48dd2cf8310c8f250dfeb65fd9df97a', 'IpPayload': '1.0-8ecf567a99e516d4af094439a7632d34', 'KeypairNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'KeypairPayload': '1.0-6daebbbde0e1bf35c1556b1ecd9385c1', @@ -500,8 +500,12 @@ class TestInstanceNotification(test.NoDBTestCase): self.instance.tags = objects.TagList() # Make sure that the notification payload chooses the values in # instance.flavor.$value instead of instance.$value + mock_context = mock.MagicMock() + mock_context.project_id = 'fake_project_id' + mock_context.user_id = 'fake_user_id' + mock_context.request_id = 'fake_req_id' notification_base._send_versioned_instance_update( - mock.MagicMock(), + mock_context, self.instance, self.payload, 'host', diff --git a/releasenotes/notes/add-req-id-to-versioned-notifications-fd0b525bd37b7e41.yaml b/releasenotes/notes/add-req-id-to-versioned-notifications-fd0b525bd37b7e41.yaml new file mode 100644 index 0000000000..d9f3093836 --- /dev/null +++ b/releasenotes/notes/add-req-id-to-versioned-notifications-fd0b525bd37b7e41.yaml @@ -0,0 +1,9 @@ +--- + +features: + - | + The ``request_id`` field has been added to all + instance action and instance update versioned + notification payloads. Note that notifications + triggered by periodic tasks will have the + ``request_id`` field set to be ``None``.