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

6 files changed, 112 insertions, 32 deletions

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',