Remove vestigial system_metadata param from info_from_instance()

The system_metadata argument to info_from_instance() was not used
so it's removed in this change, along with all callers of that
method, which goes quite a ways.

Also, the docstring for the system_metadata argument to
notify_usage_exists() is updated since it is passed in one
specific place: rebuild with a new image. In that case, nova-api
saves off the original instance system_metadata before resetting
the system_metadata based on the new image to rebuild, which is
then passed down through nova-conductor and nova-compute where
it eventually gets used to override "image_meta" in the payload
created in info_from_instance(). It's weird, yes, and essentially
means that for legacy versioned notifications, the instance payload
will always contain the image_meta for the instance before it's
rebuilt with the new image, which is something we don't do for
versioned notifications.

Test test_local_delete_without_info_cache is removed since it's
(1) weird in that it is doing mox-like stuff in a mock-based
test and (2) the code it was meant to test from change
Ie0bba032615d3da06cdd95b221beb37a9b8a377d no longer exists.

Change-Id: Ia1820334dcaceca9d7fa874dd7c553fa1c5befec
Closes-Bug: #1764390

8 files changed, 34 insertions, 97 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 9c120563c3..48bbc330f6 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -730,24 +730,20 @@ class ComputeManager(manager.Manager):
         """Complete deletion for instances in DELETED status but not marked as
         deleted in the DB
         """
-        system_meta = instance.system_metadata
         instance.destroy()
         bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
                 context, instance.uuid)
         self._complete_deletion(context,
                                 instance,
-                                bdms,
-                                system_meta)
+                                bdms)
 
-    def _complete_deletion(self, context, instance, bdms,
-                           system_meta):
+    def _complete_deletion(self, context, instance, bdms):
         self._update_resource_tracker(context, instance)
 
         rt = self._get_resource_tracker()
         rt.reportclient.delete_allocation_for_instance(context, instance.uuid)
 
-        self._notify_about_instance_usage(context, instance, "delete.end",
-                system_metadata=system_meta)
+        self._notify_about_instance_usage(context, instance, "delete.end")
         compute_utils.notify_about_instance_action(context, instance,
                 self.host, action=fields.NotificationAction.DELETE,
                 phase=fields.NotificationPhase.END, bdms=bdms)
@@ -1636,12 +1632,11 @@ class ComputeManager(manager.Manager):
         self.scheduler_client.sync_instance_info(context, self.host, uuids)
 
     def _notify_about_instance_usage(self, context, instance, event_suffix,
-                                     network_info=None, system_metadata=None,
-                                     extra_usage_info=None, fault=None):
+                                     network_info=None, extra_usage_info=None,
+                                     fault=None):
         compute_utils.notify_about_instance_usage(
             self.notifier, context, instance, event_suffix,
             network_info=network_info,
-            system_metadata=system_metadata,
             extra_usage_info=extra_usage_info, fault=fault)
 
     def _deallocate_network(self, context, instance,
@@ -2517,13 +2512,11 @@ class ComputeManager(manager.Manager):
         instance.power_state = power_state.NOSTATE
         instance.terminated_at = timeutils.utcnow()
         instance.save()
-        system_meta = instance.system_metadata
         instance.destroy()
 
         self._complete_deletion(context,
                                 instance,
-                                bdms,
-                                system_meta)
+                                bdms)
 
     @wrap_exception()
     @reverts_task_state
diff --git a/nova/compute/utils.py b/nova/compute/utils.py
index 0a86f1ec03..7bb889d136 100644
--- a/nova/compute/utils.py
+++ b/nova/compute/utils.py
@@ -268,16 +268,16 @@ def notify_usage_exists(notifier, context, instance_ref, current_period=False,
     usage auditing purposes.
 
     :param notifier: a messaging.Notifier
-
+    :param context: request context for the current operation
+    :param instance_ref: nova.objects.Instance object from which to report
+        usage
     :param current_period: if True, this will generate a usage for the
         current usage period; if False, this will generate a usage for the
         previous audit period.
-
     :param ignore_missing_network_data: if True, log any exceptions generated
         while getting network info; if False, raise the exception.
-    :param system_metadata: system_metadata DB entries for the instance,
-        if not None.  *NOTE*: Currently unused here in trunk, but needed for
-        potential custom modifications.
+    :param system_metadata: system_metadata override for the instance. If
+        None, the instance_ref.system_metadata will be used.
     :param extra_usage_info: Dictionary containing extra values to add or
         override in the notification if not None.
     """
@@ -301,12 +301,12 @@ def notify_usage_exists(notifier, context, instance_ref, current_period=False,
         extra_info.update(extra_usage_info)
 
     notify_about_instance_usage(notifier, context, instance_ref, 'exists',
-            system_metadata=system_metadata, extra_usage_info=extra_info)
+                                extra_usage_info=extra_info)
 
 
 def notify_about_instance_usage(notifier, context, instance, event_suffix,
-                                network_info=None, system_metadata=None,
-                                extra_usage_info=None, fault=None):
+                                network_info=None, extra_usage_info=None,
+                                fault=None):
     """Send an unversioned legacy notification about an instance.
 
     All new notifications should use notify_about_instance_action which sends
@@ -315,8 +315,6 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix,
     :param notifier: a messaging.Notifier
     :param event_suffix: Event type like "delete.start" or "exists"
     :param network_info: Networking information, if provided.
-    :param system_metadata: system_metadata DB entries for the instance,
-        if provided.
     :param extra_usage_info: Dictionary containing extra values to add or
         override in the notification.
     """
@@ -324,7 +322,7 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix,
         extra_usage_info = {}
 
     usage_info = notifications.info_from_instance(context, instance,
-            network_info, system_metadata, **extra_usage_info)
+            network_info, **extra_usage_info)
 
     if fault:
         # NOTE(johngarbutt) mirrors the format in wrap_exception
@@ -1057,15 +1055,10 @@ class UnlimitedSemaphore(object):
 @contextlib.contextmanager
 def notify_about_instance_delete(notifier, context, instance,
                                  delete_type='delete'):
-    # Pre-load system_metadata because if this context is around an
-    # instance.destroy(), lazy-loading it later would result in an
-    # InstanceNotFound error.
-    system_metadata = instance.system_metadata
     try:
         notify_about_instance_usage(notifier, context, instance,
                                     "%s.start" % delete_type)
         yield
     finally:
         notify_about_instance_usage(notifier, context, instance,
-                                    "%s.end" % delete_type,
-                                    system_metadata=system_metadata)
+                                    "%s.end" % delete_type)
diff --git a/nova/notifications/base.py b/nova/notifications/base.py
index 14606cf581..2c906f766a 100644
--- a/nova/notifications/base.py
+++ b/nova/notifications/base.py
@@ -210,7 +210,7 @@ def send_instance_update_notification(context, instance, old_vm_state=None,
     about instance state changes.
     """
 
-    payload = info_from_instance(context, instance, None, None)
+    payload = info_from_instance(context, instance, None)
 
     # determine how we'll report states
     payload.update(
@@ -379,21 +379,12 @@ def null_safe_isotime(s):
         return str(s) if s else ''
 
 
-def info_from_instance(context, instance, network_info,
-                system_metadata, **kw):
+def info_from_instance(context, instance, network_info, **kw):
     """Get detailed instance information for an instance which is common to all
     notifications.
 
     :param:instance: nova.objects.Instance
     :param:network_info: network_info provided if not None
-    :param:system_metadata: system_metadata DB entries for the instance,
-    if not None
-
-    .. note::
-
-        Currently unused here in trunk, but needed for potential custom
-        modifications.
-
     """
     try:
         # TODO(mriedem): We can eventually drop this when we no longer support
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index f318299270..543af7824c 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -1035,13 +1035,11 @@ class _ComputeAPIUnitTestMixIn(object):
         if 'soft' in delete_type:
             compute_utils.notify_about_instance_usage(
                 self.compute_api.notifier,
-                self.context, inst, 'delete.end',
-                system_metadata=inst.system_metadata)
+                self.context, inst, 'delete.end')
         else:
             compute_utils.notify_about_instance_usage(
                 self.compute_api.notifier,
-                self.context, inst, '%s.end' % delete_type,
-                system_metadata=inst.system_metadata)
+                self.context, inst, '%s.end' % delete_type)
         cell = objects.CellMapping(uuid=uuids.cell,
                                    transport_url='fake://',
                                    database_connection='fake://')
@@ -1343,7 +1341,7 @@ class _ComputeAPIUnitTestMixIn(object):
                                 constraint='constraint').AndReturn(fake_inst)
             compute_utils.notify_about_instance_usage(
                     self.compute_api.notifier, self.context,
-                    inst, 'delete.end', system_metadata={})
+                    inst, 'delete.end')
 
         self.mox.ReplayAll()
 
@@ -1392,8 +1390,7 @@ class _ComputeAPIUnitTestMixIn(object):
         inst.destroy()
         compute_utils.notify_about_instance_usage(
                     self.compute_api.notifier, self.context,
-                    inst, 'delete.end',
-                    system_metadata=inst.system_metadata)
+                    inst, 'delete.end')
 
         self.mox.ReplayAll()
         self.compute_api._local_delete(self.context, inst, bdms,
@@ -1522,40 +1519,6 @@ class _ComputeAPIUnitTestMixIn(object):
 
         do_test(self)
 
-    def test_local_delete_without_info_cache(self):
-        inst = self._create_instance_obj()
-
-        with test.nested(
-            mock.patch.object(inst, 'destroy'),
-            mock.patch.object(self.context, 'elevated'),
-            mock.patch.object(self.compute_api.network_api,
-                              'deallocate_for_instance'),
-            mock.patch.object(db, 'instance_system_metadata_get'),
-            mock.patch.object(compute_utils,
-                              'notify_about_instance_usage')
-        ) as (
-            inst_destroy, context_elevated, net_api_deallocate_for_instance,
-            db_instance_system_metadata_get, notify_about_instance_usage
-        ):
-
-            compute_utils.notify_about_instance_usage(
-                        self.compute_api.notifier, self.context,
-                        inst, 'delete.start')
-            self.context.elevated().MultipleTimes().AndReturn(self.context)
-            if self.cell_type != 'api':
-                self.compute_api.network_api.deallocate_for_instance(
-                            self.context, inst)
-
-            inst.destroy()
-            compute_utils.notify_about_instance_usage(
-                        self.compute_api.notifier, self.context,
-                        inst, 'delete.end',
-                        system_metadata=inst.system_metadata)
-            inst.info_cache = None
-            self.compute_api._local_delete(self.context, inst, [],
-                                           'delete',
-                                           self._fake_do_delete)
-
     def test_delete_disabled(self):
         inst = self._create_instance_obj()
         inst.disable_terminate = True
diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py
index 2ae3d03336..859aa058e6 100644
--- a/nova/tests/unit/compute/test_compute_utils.py
+++ b/nova/tests/unit/compute/test_compute_utils.py
@@ -1067,7 +1067,7 @@ class ComputeUtilsTestCase(test.NoDBTestCase):
             mock.call(mock.sentinel.notifier, self.context, instance,
                       'delete.start'),
             mock.call(mock.sentinel.notifier, self.context, instance,
-                      'delete.end', system_metadata=instance.system_metadata)
+                      'delete.end')
         ]
         mock_notify_usage.assert_has_calls(expected_notify_calls)
 
diff --git a/nova/tests/unit/notifications/test_base.py b/nova/tests/unit/notifications/test_base.py
index 65e3401de6..0e21acb294 100644
--- a/nova/tests/unit/notifications/test_base.py
+++ b/nova/tests/unit/notifications/test_base.py
@@ -97,8 +97,7 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
         instance = fake_instance.fake_instance_obj(ctxt, image_ref=uuids.image)
         instance.system_metadata = {}
         instance.metadata = {}
-        payload = base.info_from_instance(
-            ctxt, instance, network_info=None, system_metadata=None)
+        payload = base.info_from_instance(ctxt, instance, network_info=None)
         self.assertEqual(instance.image_ref, payload['image_ref_url'])
         mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)
 
@@ -114,8 +113,7 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
             'fake-user', 'fake-project', auth_token='fake-token')
         instance = fake_instance.fake_instance_obj(ctxt, image_ref=uuids.image)
         self.assertRaises(ks_exc.EndpointNotFound, base.info_from_instance,
-                          ctxt, instance, network_info=None,
-                          system_metadata=None)
+                          ctxt, instance, network_info=None)
         mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)
 
 
diff --git a/nova/tests/unit/test_notifications.py b/nova/tests/unit/test_notifications.py
index f994f692de..c41d0daaa0 100644
--- a/nova/tests/unit/test_notifications.py
+++ b/nova/tests/unit/test_notifications.py
@@ -356,20 +356,20 @@ class NotificationsTestCase(test.TestCase):
 
     def test_payload_has_fixed_ip_labels(self):
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("fixed_ips", info)
         self.assertEqual(info["fixed_ips"][0]["label"], "test1")
 
     def test_payload_has_vif_mac_address(self):
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("fixed_ips", info)
         self.assertEqual(self.net_info[0]['address'],
                          info["fixed_ips"][0]["vif_mac"])
 
     def test_payload_has_cell_name_empty(self):
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("cell_name", info)
         self.assertIsNone(self.instance.cell_name)
         self.assertEqual("", info["cell_name"])
@@ -377,13 +377,13 @@ class NotificationsTestCase(test.TestCase):
     def test_payload_has_cell_name(self):
         self.instance.cell_name = "cell1"
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("cell_name", info)
         self.assertEqual("cell1", info["cell_name"])
 
     def test_payload_has_progress_empty(self):
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("progress", info)
         self.assertIsNone(self.instance.progress)
         self.assertEqual("", info["progress"])
@@ -391,7 +391,7 @@ class NotificationsTestCase(test.TestCase):
     def test_payload_has_progress(self):
         self.instance.progress = 50
         info = notifications.info_from_instance(self.context, self.instance,
-                                                  self.net_info, None)
+                                                  self.net_info)
         self.assertIn("progress", info)
         self.assertEqual(50, info["progress"])
 
@@ -406,7 +406,7 @@ class NotificationsTestCase(test.TestCase):
         self.instance.flavor.memory_mb = 30
         self.instance.flavor.ephemeral_gb = 40
         info = notifications.info_from_instance(self.context, self.instance,
-                                                self.net_info, None)
+                                                self.net_info)
         self.assertEqual(10, info['vcpus'])
         self.assertEqual(20, info['root_gb'])
         self.assertEqual(30, info['memory_mb'])
@@ -421,7 +421,7 @@ class NotificationsTestCase(test.TestCase):
         self.instance.launched_at = time
 
         info = notifications.info_from_instance(self.context, self.instance,
-                                                self.net_info, None)
+                                                self.net_info)
 
         self.assertEqual('2017-02-02T16:45:00.000000', info['terminated_at'])
         self.assertEqual('2017-02-02T16:45:00.000000', info['launched_at'])
diff --git a/nova/tests/unit/utils.py b/nova/tests/unit/utils.py
index bd2e0dc14f..28d36159f5 100644
--- a/nova/tests/unit/utils.py
+++ b/nova/tests/unit/utils.py
@@ -314,8 +314,7 @@ def assert_instance_delete_notification_by_uuid(
         mock.call(expected_notifier,
                   expected_context,
                   match_by_instance_uuid,
-                  'delete.end',
-                  system_metadata={})])
+                  'delete.end')])
 
     for call in mock_notify.call_args_list:
         if expect_targeted_context: