From: Zhiteng Huang Date: Tue, 13 Aug 2013 04:57:17 +0000 (+0800) Subject: Raise exception when Glance metadata not found. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3e68c6be7405bb239ad7d2c82117344bdddcfa99;p=openstack-build%2Fcinder-build.git Raise exception when Glance metadata not found. It'd be better to raise exception when trying to copy Glance metadata from source (volume/snapshot) to destination (volume/snapshot) rather than silent failure, which is exactly the reason there's unspotted error inside glance_meta unittest but it was able to pass. With this fix, one should _not_ directly call glance_metadata_copy() without looking at source's bootable flags. This patch also refactors _create_volume_from_snapshot() and _create_from_source_volume() to only do Glance metadata copy when needed (bootable is True). Fix bug: 1211632 Change-Id: I28f065e21cf24c81f98b00e171fcbe4f64ec76b6 --- diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index a81498071..e588b7562 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1802,11 +1802,16 @@ def volume_type_extra_specs_update_or_create(context, volume_type_id, @require_context @require_volume_exists def _volume_glance_metadata_get(context, volume_id, session=None): - return model_query(context, models.VolumeGlanceMetadata, session=session).\ + rows = model_query(context, models.VolumeGlanceMetadata, session=session).\ filter_by(volume_id=volume_id).\ filter_by(deleted=False).\ all() + if not rows: + raise exception.GlanceMetadataNotFound(id=volume_id) + + return rows + @require_context @require_volume_exists @@ -1819,11 +1824,16 @@ def volume_glance_metadata_get(context, volume_id): @require_context @require_snapshot_exists def _volume_snapshot_glance_metadata_get(context, snapshot_id, session=None): - return model_query(context, models.VolumeGlanceMetadata, session=session).\ + rows = model_query(context, models.VolumeGlanceMetadata, session=session).\ filter_by(snapshot_id=snapshot_id).\ filter_by(deleted=False).\ all() + if not rows: + raise exception.GlanceMetadataNotFound(id=snapshot_id) + + return rows + @require_context @require_snapshot_exists diff --git a/cinder/exception.py b/cinder/exception.py index d49bc0daa..ef5261fdb 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -552,6 +552,10 @@ class GlanceMetadataExists(Invalid): " exists for volume id %(volume_id)s") +class GlanceMetadataNotFound(NotFound): + message = _("Glance metadata for volume/snapshot %(id)s cannot be found.") + + class ExportFailure(Invalid): message = _("Failed to export for volume: %(reason)s") diff --git a/cinder/tests/test_volume_glance_metadata.py b/cinder/tests/test_volume_glance_metadata.py index 5aa42d8b1..bc8afb96d 100644 --- a/cinder/tests/test_volume_glance_metadata.py +++ b/cinder/tests/test_volume_glance_metadata.py @@ -92,11 +92,8 @@ class VolumeGlanceMetadataTestCase(test.TestCase): vol_metadata = db.volume_glance_metadata_create(ctxt, 1, 'key1', 'value1') db.volume_glance_metadata_delete_by_volume(ctxt, 1) - metadata = db.volume_glance_metadata_get(ctxt, 1) - self.assertEqual(len(metadata), 0) - db.volume_glance_metadata_delete_by_volume(ctxt, 1) - metadata = db.volume_glance_metadata_get(ctxt, 1) - self.assertEqual(len(metadata), 0) + self.assertRaises(exception.GlanceMetadataNotFound, + db.volume_glance_metadata_get, ctxt, 1) def test_vol_glance_metadata_copy_to_snapshot(self): ctxt = context.get_admin_context() @@ -120,10 +117,9 @@ class VolumeGlanceMetadataTestCase(test.TestCase): db.volume_create(ctxt, {'id': 100, 'source_volid': 1}) vol_meta = db.volume_glance_metadata_create(ctxt, 1, 'key1', 'value1') - db.volume_glance_metadata_copy_from_volume_to_volume(ctxt, 100, 1) + db.volume_glance_metadata_copy_from_volume_to_volume(ctxt, 1, 100) - expected_meta = {'id': '100', - 'key': 'key1', + expected_meta = {'key': 'key1', 'value': 'value1'} for meta in db.volume_glance_metadata_get(ctxt, 100): diff --git a/cinder/volume/flows/create_volume.py b/cinder/volume/flows/create_volume.py index fb11e178c..ec87060a8 100644 --- a/cinder/volume/flows/create_volume.py +++ b/cinder/volume/flows/create_volume.py @@ -1176,18 +1176,20 @@ class CreateVolumeFromSpecTask(CinderTask): raise exception.MetadataUpdateFailure(reason=ex) if make_bootable: self._enable_bootable_flag(context, volume_id) - try: - LOG.debug(_("Copying metadata from snapshot %(snap_volume_id)s" - " to %(volume_id)s") % {'snap_volume_id': snapshot_id, - 'volume_id': volume_id}) - self.db.volume_glance_metadata_copy_to_volume(context, volume_id, - snapshot_id) - except exception.CinderException as ex: - LOG.exception(_("Failed updating volume %(volume_id)s metadata" - " using the provided glance snapshot " - "%(snapshot_id)s metadata") % - {'volume_id': volume_id, 'snapshot_id': snapshot_id}) - raise exception.MetadataCopyFailure(reason=ex) + try: + LOG.debug(_("Copying metadata from snapshot " + "%(snap_volume_id)s to %(volume_id)s") % + {'snap_volume_id': snapshot_id, + 'volume_id': volume_id}) + self.db.volume_glance_metadata_copy_to_volume( + context, volume_id, snapshot_id) + except exception.CinderException as ex: + LOG.exception(_("Failed updating volume %(volume_id)s " + "metadata using the provided glance " + "snapshot %(snapshot_id)s metadata") % + {'volume_id': volume_id, + 'snapshot_id': snapshot_id}) + raise exception.MetadataCopyFailure(reason=ex) return model_update def _enable_bootable_flag(self, context, volume_id): @@ -1214,22 +1216,24 @@ class CreateVolumeFromSpecTask(CinderTask): # will not destroy the volume (although they could in the future). if srcvol_ref.bootable: self._enable_bootable_flag(context, volume_ref['id']) - try: - msg = _('Copying metadata from source volume %(source_volid)s' - ' to cloned volume %(clone_vol_id)s') - LOG.debug(msg % {'source_volid': source_volid, - 'clone_vol_id': volume_ref['id'], }) - self.db.volume_glance_metadata_copy_from_volume_to_volume( - context, - source_volid, - volume_ref['id']) - except exception.CinderException as ex: - LOG.exception(_("Failed updating cloned volume %(volume_id)s" - " metadata using the provided source volumes" - " %(source_volid)s metadata") % - {'volume_id': volume_ref['id'], - 'source_volid': source_volid}) - raise exception.MetadataCopyFailure(reason=ex) + try: + LOG.debug(_('Copying metadata from source volume ' + '%(source_volid)s to cloned volume ' + '%(clone_vol_id)s') % { + 'source_volid': source_volid, + 'clone_vol_id': volume_ref['id'], + }) + self.db.volume_glance_metadata_copy_from_volume_to_volume( + context, + source_volid, + volume_ref['id']) + except exception.CinderException as ex: + LOG.exception(_("Failed updating cloned volume %(volume_id)s" + " metadata using the provided source volumes" + " %(source_volid)s metadata") % + {'volume_id': volume_ref['id'], + 'source_volid': source_volid}) + raise exception.MetadataCopyFailure(reason=ex) return model_update def _copy_image_to_volume(self, context, volume_ref, diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 40fe19750..89b517c39 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -293,9 +293,19 @@ class VolumeManager(manager.SchedulerDependentManager): self.db.snapshot_update(context, snapshot_ref['id'], {'status': 'available', 'progress': '100%'}) - self.db.volume_glance_metadata_copy_to_snapshot(context, - snapshot_ref['id'], - volume_id) + + vol_ref = self.db.volume_get(context, volume_id) + if vol_ref.bootable: + try: + self.db.volume_glance_metadata_copy_to_snapshot( + context, snapshot_ref['id'], volume_id) + except exception.CinderException as ex: + LOG.exception(_("Failed updating %(snapshot_id)s" + " metadata using the provided volumes" + " %(volume_id)s metadata") % + {'volume_id': volume_id, + 'snapshot_id': snapshot_id}) + raise exception.MetadataCopyFailure(reason=ex) LOG.info(_("snapshot %s: created successfully"), snapshot_ref['name']) self._notify_about_snapshot_usage(context, snapshot_ref, "create.end") return snapshot_id