From 96f4529dd6a884694840f1678d475b4004d3429d Mon Sep 17 00:00:00 2001 From: Ryan McNair Date: Mon, 14 Mar 2016 17:37:35 +0000 Subject: [PATCH] Fix volume migration VolumeType exception Since Iabf9c3fab56ffef50695ce45745f193273822b39 modified the default loaded attributes, the obj_attr_is_set check in finish_volume_migration is now True for VolumeType. This should not get copied to the new volume (since it's lazy-loaded based on the volume_type_id), so this patch adds volume_type to the list of keys to ignore. To get test coverage, test_finish_volume_migration is updated to use the default expected_attributes and dict comprehension is updated so it succeeds without glance_metadata being present. Change-Id: Id3670dba79afb0d9d1b686ac2772c955372e97af Closes-Bug: #1555862 Depends-On: I2caf4d3f4aa088d099548e6e88d1776b4cc5810c --- cinder/objects/volume.py | 2 +- cinder/tests/unit/objects/test_volume.py | 27 +++++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index abf1cf9b4..56c64d867 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -395,7 +395,7 @@ class Volume(base.CinderPersistentObject, base.CinderObject, # end of migration because we want to keep the original volume id # in the DB but now pointing to the migrated volume. skip = ({'id', 'provider_location', 'glance_metadata', - 'volume_type_id'} | set(self.obj_extra_fields)) + 'volume_type_id', 'volume_type'} | set(self.obj_extra_fields)) for key in set(dest_volume.fields.keys()) - skip: # Only swap attributes that are already set. We do not want to # unexpectedly trigger a lazy-load. diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index 2759faabe..f6004135d 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -346,14 +346,21 @@ class TestVolume(test_objects.BaseObjectsTestCase): src_vol_type_id, dest_vol_type_id): src_volume_db = fake_volume.fake_db_volume( **{'id': fake.volume_id, 'volume_type_id': src_vol_type_id}) + if src_vol_type_id: + src_volume_db['volume_type'] = fake_volume.fake_db_volume_type( + id=src_vol_type_id) dest_volume_db = fake_volume.fake_db_volume( **{'id': fake.volume2_id, 'volume_type_id': dest_vol_type_id}) + if dest_vol_type_id: + dest_volume_db['volume_type'] = fake_volume.fake_db_volume_type( + id=dest_vol_type_id) + expected_attrs = objects.Volume._get_expected_attrs(self.context) src_volume = objects.Volume._from_db_object( self.context, objects.Volume(), src_volume_db, - expected_attrs=['metadata', 'glance_metadata']) + expected_attrs=expected_attrs) dest_volume = objects.Volume._from_db_object( self.context, objects.Volume(), dest_volume_db, - expected_attrs=['metadata', 'glance_metadata']) + expected_attrs=expected_attrs) updated_dest_volume = src_volume.finish_volume_migration( dest_volume) self.assertEqual('deleting', updated_dest_volume.migration_status) @@ -361,6 +368,8 @@ class TestVolume(test_objects.BaseObjectsTestCase): updated_dest_volume.display_description) self.assertEqual(src_volume.id, updated_dest_volume._name_id) self.assertTrue(volume_update.called) + ctxt, vol_id, updates = volume_update.call_args[0] + self.assertNotIn('volume_type', updates) # Ensure that the destination volume type has not been overwritten self.assertEqual(dest_vol_type_id, @@ -369,12 +378,14 @@ class TestVolume(test_objects.BaseObjectsTestCase): # finish_volume_migration ignore_keys = ('id', 'provider_location', '_name_id', 'migration_status', 'display_description', 'status', - 'volume_type_id') - dest_vol_filtered = {k: v for k, v in updated_dest_volume.iteritems() - if k not in ignore_keys} - src_vol_filtered = {k: v for k, v in src_volume.iteritems() - if k not in ignore_keys} - self.assertEqual(src_vol_filtered, dest_vol_filtered) + 'volume_type_id', 'volume_glance_metadata', + 'volume_type') + + dest_vol_dict = {k: updated_dest_volume[k] for k in + updated_dest_volume.keys() if k not in ignore_keys} + src_vol_dict = {k: src_volume[k] for k in src_volume.keys() + if k not in ignore_keys} + self.assertEqual(src_vol_dict, dest_vol_dict) def test_volume_with_metadata_serialize_deserialize_no_changes(self): updates = {'volume_glance_metadata': [{'key': 'foo', 'value': 'bar'}], -- 2.45.2