From 55e6021ce36b2f2d4ef9222252b9ab784f67d9f7 Mon Sep 17 00:00:00 2001
From: Chris Behrens <cbehrens@codestud.com>
Date: Wed, 9 May 2012 19:20:46 +0000
Subject: Defer image_ref update to manager on rebuild

We shouldn't update the DB and use it for passing the new image_ref for
a rebuild.  This causes a usage exists notification for the new image
instead of the old... and technically the new image is not accurate
until we shutdown the old instance and try building the new one..

Fixes bug 997245

Change-Id: Ia4bd4b8af90080a11875d9ee56661286bc3f5593
---
 nova/compute/api.py     |  2 +-
 nova/compute/manager.py | 28 +++++++++++++++++++++-------
 nova/compute/utils.py   |  3 +--
 3 files changed, 23 insertions(+), 10 deletions(-)

(limited to 'nova/compute')

diff --git a/nova/compute/api.py b/nova/compute/api.py
index b54aa1b57..116bfb9af 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1345,7 +1345,6 @@ class API(BaseAPI):
 
         self.update(context,
                     instance,
-                    image_ref=image_href,
                     vm_state=vm_states.REBUILDING,
                     task_state=None,
                     progress=0,
@@ -1354,6 +1353,7 @@ class API(BaseAPI):
         rebuild_params = {
             "new_pass": admin_password,
             "injected_files": files_to_inject,
+            "image_ref": image_href,
         }
 
         self._cast_compute_message('rebuild_instance', context, instance,
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index aa94448d8..faccfef50 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -793,7 +793,7 @@ class ComputeManager(manager.SchedulerDependentManager):
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @checks_instance_lock
     @wrap_instance_fault
-    def rebuild_instance(self, context, instance_uuid, **kwargs):
+    def rebuild_instance(self, context, instance_uuid, image_ref, **kwargs):
         """Destroy and re-make this instance.
 
         A 'rebuild' effectively purges all existing data from the system and
@@ -805,7 +805,8 @@ class ComputeManager(manager.SchedulerDependentManager):
         :param new_pass: password to set on rebuilt instance
         """
         try:
-            self._rebuild_instance(context, instance_uuid, kwargs)
+            self._rebuild_instance(context, instance_uuid, image_ref,
+                    kwargs)
         except exception.ImageNotFound:
             LOG.error(_('Cannot rebuild instance because the given image does '
                         'not exist.'),
@@ -816,7 +817,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                       context=context, instance_uuid=instance_uuid)
             self._set_instance_error_state(context, instance_uuid)
 
-    def _rebuild_instance(self, context, instance_uuid, kwargs):
+    def _rebuild_instance(self, context, instance_uuid, image_ref, kwargs):
         context = context.elevated()
 
         LOG.audit(_("Rebuilding instance"), context=context,
@@ -825,7 +826,17 @@ class ComputeManager(manager.SchedulerDependentManager):
         instance = self.db.instance_get_by_uuid(context, instance_uuid)
         compute_utils.notify_usage_exists(
                 context, instance, current_period=True)
-        self._notify_about_instance_usage(context, instance, "rebuild.start")
+
+        # NOTE(comstud): Since we've not updated the DB yet with the new
+        # image_ref, and rebuild.start should have image_ref_url pointing
+        # to the new image ref, we need to override it.
+        # Arguably, image_ref_url should still contain the old image at
+        # this point and we should pass a 'new_image_ref_url', instead.
+        image_ref_url = utils.generate_image_url(image_ref)
+        extra_usage_info = {'image_ref_url': image_ref_url}
+        self._notify_about_instance_usage(context, instance,
+                "rebuild.start", extra_usage_info=extra_usage_info)
+
         current_power_state = self._get_power_state(context, instance)
         self._instance_update(context,
                               instance_uuid,
@@ -836,17 +847,20 @@ class ComputeManager(manager.SchedulerDependentManager):
         network_info = self._get_instance_nw_info(context, instance)
         self.driver.destroy(instance, self._legacy_nw_info(network_info))
 
-        self._instance_update(context,
+        # NOTE(comstud): Now that we've shutdown the old instance, we
+        # can update the image_ref in the DB.
+        instance = self._instance_update(context,
                               instance_uuid,
                               vm_state=vm_states.REBUILDING,
-                              task_state=task_states.BLOCK_DEVICE_MAPPING)
+                              task_state=task_states.BLOCK_DEVICE_MAPPING,
+                              image_ref=image_ref)
 
         instance.injected_files = kwargs.get('injected_files', [])
         network_info = self.network_api.get_instance_nw_info(context,
                                                              instance)
         device_info = self._setup_block_device_mapping(context, instance)
 
-        self._instance_update(context,
+        instance = self._instance_update(context,
                               instance_uuid,
                               vm_state=vm_states.REBUILDING,
                               task_state=task_states.SPAWNING)
diff --git a/nova/compute/utils.py b/nova/compute/utils.py
index 5da501c8a..6c2b265dc 100644
--- a/nova/compute/utils.py
+++ b/nova/compute/utils.py
@@ -212,8 +212,7 @@ def _usage_from_instance(context, instance_ref, network_info=None, **kw):
     def null_safe_str(s):
         return str(s) if s else ''
 
-    image_ref_url = "%s/images/%s" % (utils.generate_glance_url(),
-                                      instance_ref['image_ref'])
+    image_ref_url = utils.generate_image_url(instance_ref['image_ref'])
 
     usage_info = dict(
           tenant_id=instance_ref['project_id'],
-- 
cgit