Store image properties with instance system_metadata

In implementing adding 'image_meta' to instance notifications,
this was committed:

https://review.openstack.org/#/c/7309
commit 53adfd289bf7226f1c6f59c17608e42f3083f130

However, it's buggy because an instance could have been deleted after we
built from it... and bin/instance-usage-audit uses an admin context with
which we cannot query glance.

This stores image properties with instances in nova as system_metadata
and notifications will use that data.

Fixes bug 997833

Change-Id: I50575969b5cb28adaae9a713e749dc486772c417

3 files changed, 69 insertions, 24 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 32b7d79ce..d503ee34f 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -576,6 +576,13 @@ class API(BaseAPI):
                     security_group_name)
             security_groups.append(group['id'])
 
+        # Store image properties so we can use them later
+        # (for notifications, etc).  Only store what we can.
+        base_options.setdefault('system_metadata', {})
+        for key, value in image['properties'].iteritems():
+            new_value = str(value)[:255]
+            base_options['system_metadata']['image_%s' % key] = new_value
+
         base_options.setdefault('launch_index', 0)
         instance = self.db.instance_create(context, base_options)
         instance_id = instance['id']
@@ -617,8 +624,7 @@ class API(BaseAPI):
             block_device_mapping):
             updates['shutdown_terminate'] = False
 
-        instance = self.update(context, instance, **updates)
-        return instance
+        return self.update(context, instance, **updates)
 
     def _default_display_name(self, instance_id):
         return "Server %s" % instance_id
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 382464e7d..0e1afb861 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -602,7 +602,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                                      launched_at=utils.utcnow())
 
     def _notify_about_instance_usage(self, context, instance, event_suffix,
-                                     network_info=None,
+                                     network_info=None, system_metadata=None,
                                      extra_usage_info=None):
         # NOTE(sirp): The only thing this wrapper function does extra is handle
         # the passing in of `self.host`. Ordinarily this will just be
@@ -610,6 +610,7 @@ class ComputeManager(manager.SchedulerDependentManager):
         # `__init__`.
         compute_utils.notify_about_instance_usage(
                 context, instance, event_suffix, network_info=network_info,
+                system_metadata=system_metadata,
                 extra_usage_info=extra_usage_info, host=self.host)
 
     def _deallocate_network(self, context, instance):
@@ -714,17 +715,23 @@ class ComputeManager(manager.SchedulerDependentManager):
 
     def _delete_instance(self, context, instance):
         """Delete an instance on this host."""
-        instance_id = instance['id']
+        instance_uuid = instance['uuid']
         self._notify_about_instance_usage(context, instance, "delete.start")
         self._shutdown_instance(context, instance, 'Terminating')
-        self._cleanup_volumes(context, instance['uuid'])
+        self._cleanup_volumes(context, instance_uuid)
         instance = self._instance_update(context,
-                              instance_id,
+                              instance_uuid,
                               vm_state=vm_states.DELETED,
                               task_state=None,
                               terminated_at=utils.utcnow())
-        self.db.instance_destroy(context, instance_id)
-        self._notify_about_instance_usage(context, instance, "delete.end")
+        # Pull the system_metadata before we delete the instance, so we
+        # can pass it to delete.end notification, as it will not be able
+        # to look it up anymore, if it needs it.
+        system_meta = self.db.instance_system_metadata_get(context,
+                instance_uuid)
+        self.db.instance_destroy(context, instance_uuid)
+        self._notify_about_instance_usage(context, instance, "delete.end",
+                system_metadata=system_meta)
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @checks_instance_lock
diff --git a/nova/compute/utils.py b/nova/compute/utils.py
index a975ce77f..c00626129 100644
--- a/nova/compute/utils.py
+++ b/nova/compute/utils.py
@@ -22,7 +22,6 @@ import nova.context
 from nova import db
 from nova import exception
 from nova import flags
-from nova import image
 from nova import log
 from nova import network
 from nova.network import model as network_model
@@ -36,7 +35,7 @@ LOG = log.getLogger(__name__)
 
 def notify_usage_exists(context, instance_ref, current_period=False,
                         ignore_missing_network_data=True,
-                        extra_usage_info=None):
+                        system_metadata=None, extra_usage_info=None):
     """Generates 'exists' notification for an instance for usage auditing
     purposes.
 
@@ -46,6 +45,11 @@ def notify_usage_exists(context, instance_ref, current_period=False,
 
     :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 extra_usage_info: Dictionary containing extra values to add or
+        override in the notification if not None.
     """
 
     admin_context = nova.context.get_admin_context(read_deleted='yes')
@@ -88,8 +92,18 @@ def notify_usage_exists(context, instance_ref, current_period=False,
 
         bw[label] = dict(bw_in=b.bw_in, bw_out=b.bw_out)
 
+    if system_metadata is None:
+        try:
+            system_metadata = db.instance_system_metadata_get(
+                    context, instance_ref.uuid)
+        except exception.NotFound:
+            system_metadata = {}
+
     # add image metadata to the notification:
-    image_meta = _get_image_meta(context, instance_ref)
+    image_meta = {}
+    for md_key, md_value in system_metadata.iteritems():
+        if md_key.startswith('image_'):
+            image_meta[md_key[6:]] = md_value
 
     extra_info = dict(audit_period_beginning=str(audit_start),
                       audit_period_ending=str(audit_end),
@@ -98,14 +112,8 @@ def notify_usage_exists(context, instance_ref, current_period=False,
     if extra_usage_info:
         extra_info.update(extra_usage_info)
 
-    notify_about_instance_usage(
-            context, instance_ref, 'exists', extra_usage_info=extra_info)
-
-
-def _get_image_meta(context, instance_ref):
-    image_service, image_id = image.get_image_service(context,
-            instance_ref["image_ref"])
-    return image_service.show(context, image_id)
+    notify_about_instance_usage(context, instance_ref, 'exists',
+            system_metadata=system_metadata, extra_usage_info=extra_info)
 
 
 def legacy_network_info(network_model):
@@ -223,7 +231,18 @@ def legacy_network_info(network_model):
     return network_info
 
 
-def _usage_from_instance(context, instance_ref, network_info=None, **kw):
+def _usage_from_instance(context, instance_ref, network_info,
+        system_metadata, **kw):
+    """
+    Get usage information for an instance which is common to all
+    notifications.
+
+    :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.
+    """
+
     def null_safe_str(s):
         return str(s) if s else ''
 
@@ -256,16 +275,29 @@ def _usage_from_instance(context, instance_ref, network_info=None, **kw):
 
 
 def notify_about_instance_usage(context, instance, event_suffix,
-                                network_info=None, extra_usage_info=None,
-                                host=None):
+                                network_info=None, system_metadata=None,
+                                extra_usage_info=None, host=None):
+    """
+    Send a notification about an instance.
+
+    :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.
+    :param host: Compute host for the instance, if specified.  Default is
+        FLAGS.host
+    """
+
     if not host:
         host = FLAGS.host
 
     if not extra_usage_info:
         extra_usage_info = {}
 
-    usage_info = _usage_from_instance(
-            context, instance, network_info=network_info, **extra_usage_info)
+    usage_info = _usage_from_instance(context, instance, network_info,
+            system_metadata, **extra_usage_info)
 
     notifier_api.notify(context, 'compute.%s' % host,
                         'compute.instance.%s' % event_suffix,