Improve conductor error cases when unshelving

This commit handles different cases when the key 'shelved_image_id'
is not defined or None. Also it tries to improve the error messages.

Closes-Bug: #1307416
Change-Id: Ia2d0a183eefccdfd6f612d86f60cd2930c1a4b23

nova/conductor/manager.py

@@ -797,11 +797,6 @@ class ComputeTaskManager(base.Base):
                 filter_properties=filter_properties,
                 legacy_bdm_in_spec=legacy_bdm)
 
-    def _get_image(self, context, image_id):
-        if not image_id:
-            return None
-        return self.image_service.show(context, image_id)
-
     def _delete_image(self, context, image_id):
         (image_service, image_id) = glance.get_remote_image_service(context,
                 image_id)
@@ -819,6 +814,10 @@ class ComputeTaskManager(base.Base):
     def unshelve_instance(self, context, instance):
         sys_meta = instance.system_metadata
 
+        def safe_image_show(ctx, image_id):
+            if image_id:
+                return self.image_service.show(ctx, image_id)
+
         if instance.vm_state == vm_states.SHELVED:
             instance.task_state = task_states.POWERING_ON
             instance.save(expected_task_state=task_states.UNSHELVING)
@@ -827,17 +826,19 @@ class ComputeTaskManager(base.Base):
             if snapshot_id:
                 self._delete_image(context, snapshot_id)
         elif instance.vm_state == vm_states.SHELVED_OFFLOADED:
-            try:
-                with compute_utils.EventReporter(context, self.db,
-                        'get_image_info', instance.uuid):
-                    image = self._get_image(context,
-                            sys_meta['shelved_image_id'])
-            except exception.ImageNotFound:
-                with excutils.save_and_reraise_exception():
-                    LOG.error(_('Unshelve cannot find the image with id %s'),
-                              sys_meta['shelved_image_id'], instance=instance)
+            image_id = sys_meta.get('shelved_image_id')
+            with compute_utils.EventReporter(
+                context, self.db, 'get_image_info', instance.uuid):
+                try:
+                    image = safe_image_show(context, image_id)
+                except exception.ImageNotFound:
                     instance.vm_state = vm_states.ERROR
                     instance.save()
+                    reason = _('Unshelve attempted but the image %s '
+                               'cannot be found.') % image_id
+                    LOG.error(reason, instance=instance)
+                    raise exception.UnshelveException(
+                        instance_id=instance.uuid, reason=reason)
 
             try:
                 with compute_utils.EventReporter(context, self.db,

nova/exception.py

@@ -1551,3 +1551,7 @@ class InvalidWatchdogAction(Invalid):
 class NoBlockMigrationForConfigDriveInLibVirt(NovaException):
     msg_fmt = _("Block migration of instances with config drives is not "
                 "supported in libvirt.")
+
+
+class UnshelveException(NovaException):
+    msg_fmt = _("Error during unshelve instance %(instance_id)s: %(reason)s")

nova/tests/conductor/test_conductor.py

@@ -1410,6 +1410,36 @@ class _BaseTaskTestCase(object):
         system_metadata['shelved_host'] = 'fake-mini'
         self.conductor_manager.unshelve_instance(self.context, instance)
 
+    def test_unshelve_offloaded_instance_glance_image_not_found(self):
+        shelved_image_id = "image_not_found"
+
+        db_instance = jsonutils.to_primitive(self._create_fake_instance())
+        instance = instance_obj.Instance.get_by_uuid(
+            self.context,
+            db_instance['uuid'],
+            expected_attrs=['system_metadata'])
+        instance.vm_state = vm_states.SHELVED_OFFLOADED
+        instance.task_state = task_states.UNSHELVING
+        instance.save()
+        system_metadata = instance.system_metadata
+
+        self.mox.StubOutWithMock(self.conductor_manager.image_service, 'show')
+
+        e = exc.ImageNotFound(image_id=shelved_image_id)
+        self.conductor_manager.image_service.show(
+            self.context, shelved_image_id).AndRaise(e)
+        self.mox.ReplayAll()
+
+        system_metadata['shelved_at'] = timeutils.utcnow()
+        system_metadata['shelved_host'] = 'fake-mini'
+        system_metadata['shelved_image_id'] = shelved_image_id
+
+        self.assertRaises(
+            exc.UnshelveException,
+            self.conductor_manager.unshelve_instance,
+            self.context, instance)
+        self.assertEqual(instance.vm_state, vm_states.ERROR)
+
     def test_unshelve_instance_schedule_and_rebuild(self):
         db_instance = jsonutils.to_primitive(self._create_fake_instance())
         instance = instance_obj.Instance.get_by_uuid(self.context,
@@ -1419,12 +1449,12 @@ class _BaseTaskTestCase(object):
         filter_properties = {}
         system_metadata = instance.system_metadata
 
-        self.mox.StubOutWithMock(self.conductor_manager, '_get_image')
+        self.mox.StubOutWithMock(self.conductor_manager.image_service, 'show')
         self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances')
         self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi,
                 'unshelve_instance')
 
-        self.conductor_manager._get_image(self.context,
+        self.conductor_manager.image_service.show(self.context,
                 'fake_image_id').AndReturn('fake_image')
         self.conductor_manager._schedule_instances(self.context,
                 'fake_image', filter_properties, instance).AndReturn(
@@ -1454,7 +1484,7 @@ class _BaseTaskTestCase(object):
             raise exc.NoValidHost(reason='')
 
         with contextlib.nested(
-            mock.patch.object(self.conductor_manager, '_get_image',
+            mock.patch.object(self.conductor_manager.image_service, 'show',
                               return_value='fake_image'),
             mock.patch.object(self.conductor_manager, '_schedule_instances',
                               fake_schedule_instances)
@@ -1476,12 +1506,12 @@ class _BaseTaskTestCase(object):
         filter_properties = {}
         system_metadata = instance.system_metadata
 
-        self.mox.StubOutWithMock(self.conductor_manager, '_get_image')
+        self.mox.StubOutWithMock(self.conductor_manager.image_service, 'show')
         self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances')
         self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi,
                 'unshelve_instance')
 
-        self.conductor_manager._get_image(self.context,
+        self.conductor_manager.image_service.show(self.context,
                 'fake_image_id').AndReturn(None)
         self.conductor_manager._schedule_instances(self.context,
                 None, filter_properties, instance).AndReturn(