diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index fbe7b741bf20..e50d00cb68f6 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -660,7 +660,9 @@ class ComputeTaskManager(base.Base): def safe_image_show(ctx, image_id): if image_id: - return self.image_api.get(ctx, image_id) + return self.image_api.get(ctx, image_id, show_deleted=False) + else: + raise exception.ImageNotFound(image_id='') if instance.vm_state == vm_states.SHELVED: instance.task_state = task_states.POWERING_ON @@ -678,8 +680,14 @@ class ComputeTaskManager(base.Base): except exception.ImageNotFound: instance.vm_state = vm_states.ERROR instance.save() - reason = _('Unshelve attempted but the image %s ' - 'cannot be found.') % image_id + + if image_id: + reason = _('Unshelve attempted but the image %s ' + 'cannot be found.') % image_id + else: + reason = _('Unshelve attempted but the image_id is ' + 'not provided') + LOG.error(reason, instance=instance) raise exception.UnshelveException( instance_id=instance.uuid, reason=reason) diff --git a/nova/image/api.py b/nova/image/api.py index a43fdb156a2a..4d83e5e7b8e6 100644 --- a/nova/image/api.py +++ b/nova/image/api.py @@ -67,7 +67,8 @@ class API(object): session = self._get_session(context) return session.detail(context, **kwargs) - def get(self, context, id_or_uri, include_locations=False): + def get(self, context, id_or_uri, include_locations=False, + show_deleted=True): """Retrieves the information record for a single disk image. If the supplied identifier parameter is a UUID, the default driver will be used to return information about the image. If the supplied @@ -83,10 +84,13 @@ class API(object): not support the locations attribute, it will still be included in the returned dict, as an empty list. + :param show_deleted: (Optional) show the image even the status of + image is deleted. """ session, image_id = self._get_session_and_image_id(context, id_or_uri) return session.show(context, image_id, - include_locations=include_locations) + include_locations=include_locations, + show_deleted=show_deleted) def create(self, context, image_info, data=None): """Creates a new image record, optionally passing the image bits to diff --git a/nova/image/glance.py b/nova/image/glance.py index 9797bbd6700d..9934d6c743f6 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -290,7 +290,8 @@ class GlanceImageService(object): return _images - def show(self, context, image_id, include_locations=False): + def show(self, context, image_id, include_locations=False, + show_deleted=True): """Returns a dict with image data for the given opaque image id. :param context: The context object to pass to image client @@ -301,6 +302,8 @@ class GlanceImageService(object): not support the locations attribute, it will still be included in the returned dict, as an empty list. + :param show_deleted: (Optional) show the image even the status of + image is deleted. """ version = 1 if include_locations: @@ -310,6 +313,9 @@ class GlanceImageService(object): except Exception: _reraise_translated_image_exception(image_id) + if not show_deleted and getattr(image, 'deleted', False): + raise exception.ImageNotFound(image_id=image_id) + if not _is_image_available(context, image): raise exception.ImageNotFound(image_id=image_id) diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py index 19951f250ac7..c0e06305ca5f 100644 --- a/nova/tests/conductor/test_conductor.py +++ b/nova/tests/conductor/test_conductor.py @@ -1415,7 +1415,7 @@ class _BaseTaskTestCase(object): e = exc.ImageNotFound(image_id=shelved_image_id) self.conductor_manager.image_api.get( - self.context, shelved_image_id).AndRaise(e) + self.context, shelved_image_id, show_deleted=False).AndRaise(e) self.mox.ReplayAll() system_metadata['shelved_at'] = timeutils.utcnow() @@ -1428,6 +1428,24 @@ class _BaseTaskTestCase(object): self.context, instance) self.assertEqual(instance.vm_state, vm_states.ERROR) + def test_unshelve_offloaded_instance_image_id_is_none(self): + db_instance = jsonutils.to_primitive(self._create_fake_instance()) + instance = objects.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 + system_metadata = instance.system_metadata + system_metadata['shelved_image_id'] = None + instance.save() + + 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 = self._create_fake_instance() instance = objects.Instance.get_by_uuid(self.context, @@ -1443,7 +1461,7 @@ class _BaseTaskTestCase(object): 'unshelve_instance') self.conductor_manager.image_api.get(self.context, - 'fake_image_id').AndReturn('fake_image') + 'fake_image_id', show_deleted=False).AndReturn('fake_image') self.conductor_manager._schedule_instances(self.context, 'fake_image', filter_properties, instance).AndReturn( [{'host': 'fake_host', @@ -1482,7 +1500,8 @@ class _BaseTaskTestCase(object): system_metadata['shelved_host'] = 'fake-mini' self.conductor_manager.unshelve_instance(self.context, instance) _get_image.assert_has_calls([mock.call(self.context, - system_metadata['shelved_image_id'])]) + system_metadata['shelved_image_id'], + show_deleted=False)]) self.assertEqual(vm_states.SHELVED_OFFLOADED, instance.vm_state) def test_unshelve_instance_schedule_and_rebuild_volume_backed(self): @@ -1500,7 +1519,7 @@ class _BaseTaskTestCase(object): 'unshelve_instance') self.conductor_manager.image_api.get(self.context, - 'fake_image_id').AndReturn(None) + 'fake_image_id', show_deleted=False).AndReturn(None) self.conductor_manager._schedule_instances(self.context, None, filter_properties, instance).AndReturn( [{'host': 'fake_host', diff --git a/nova/tests/image/fake.py b/nova/tests/image/fake.py index 739653b21530..0292afba600c 100644 --- a/nova/tests/image/fake.py +++ b/nova/tests/image/fake.py @@ -168,7 +168,8 @@ class _FakeImageService(object): with open(dst_path, 'wb') as data: data.write(self._imagedata.get(image_id, '')) - def show(self, context, image_id, include_locations=False): + def show(self, context, image_id, include_locations=False, + show_deleted=True): """Get data about specified image. Returns a dict containing image data for the given opaque image id. diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py index 675774153a14..63b4d22e1eea 100644 --- a/nova/tests/image/test_glance.py +++ b/nova/tests/image/test_glance.py @@ -904,6 +904,27 @@ class TestShow(test.NoDBTestCase): self.assertIn('locations', info) self.assertEqual(expected, info['locations']) + @mock.patch('nova.image.glance._translate_from_glance') + @mock.patch('nova.image.glance._is_image_available') + def test_do_not_show_deleted_images(self, is_avail_mock, trans_from_mock): + class fake_image_cls(dict): + id = 'b31aa5dd-f07a-4748-8f15-398346887584' + deleted = True + + glance_image = fake_image_cls() + client = mock.MagicMock() + client.call.return_value = glance_image + ctx = mock.sentinel.ctx + service = glance.GlanceImageService(client) + + with testtools.ExpectedException(exception.ImageNotFound): + service.show(ctx, glance_image.id, show_deleted=False) + + client.call.assert_called_once_with(ctx, 1, 'get', + glance_image.id) + self.assertFalse(is_avail_mock.called) + self.assertFalse(trans_from_mock.called) + class TestDetail(test.NoDBTestCase):