diff --git a/cinder/objects/volume_attachment.py b/cinder/objects/volume_attachment.py index 28dd66c667e..75effc06832 100644 --- a/cinder/objects/volume_attachment.py +++ b/cinder/objects/volume_attachment.py @@ -95,8 +95,12 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject, jsonutils.loads(value) if value else None) else: attachment[name] = value - if 'volume' in expected_attrs: - db_volume = db_attachment.get('volume') + # NOTE: hasattr check is necessary to avoid doing a lazy loading when + # loading VolumeList.get_all: Getting a Volume loads its + # VolumeAttachmentList, which think they have the volume loaded, but + # they don't. More detail on https://review.openstack.org/632549 + if 'volume' in expected_attrs and hasattr(db_attachment, 'volume'): + db_volume = db_attachment.volume if db_volume: attachment.volume = objects.Volume._from_db_object( context, objects.Volume(), db_volume) diff --git a/cinder/tests/unit/fake_volume.py b/cinder/tests/unit/fake_volume.py index 95a25dfbc61..9bf46419cf3 100644 --- a/cinder/tests/unit/fake_volume.py +++ b/cinder/tests/unit/fake_volume.py @@ -15,6 +15,7 @@ from oslo_utils.uuidutils import is_uuid_like from oslo_versionedobjects import fields +from cinder.db.sqlalchemy import models from cinder import objects from cinder.objects import fields as c_fields from cinder.tests.unit import fake_constants as fake @@ -123,3 +124,24 @@ def fake_volume_attachment_obj(context, **updates): return objects.VolumeAttachment._from_db_object( context, objects.VolumeAttachment(), fake_db_volume_attachment(**updates)) + + +def volume_db_obj(**updates): + """Return a volume ORM object.""" + updates.setdefault('id', fake.VOLUME_ID) + updates.setdefault('size', 1) + return models.Volume(**updates) + + +def volume_attachment_db_obj(**updates): + updates.setdefault('id', fake.ATTACHMENT_ID) + updates.setdefault('volume_id', fake.VOLUME_ID) + updates.setdefault('volume', volume_db_obj()) + return models.VolumeAttachment(**updates) + + +def volume_attachment_ovo(context, **updates): + orm = volume_attachment_db_obj(**updates) + return objects.VolumeAttachment._from_db_object(context, + objects.VolumeAttachment(), + orm) diff --git a/cinder/tests/unit/objects/test_volume_attachment.py b/cinder/tests/unit/objects/test_volume_attachment.py index fd854ef90c3..101c394296f 100644 --- a/cinder/tests/unit/objects/test_volume_attachment.py +++ b/cinder/tests/unit/objects/test_volume_attachment.py @@ -29,8 +29,8 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase): @mock.patch('cinder.db.sqlalchemy.api.volume_attachment_get') def test_get_by_id(self, volume_attachment_get): - db_attachment = fake_volume.fake_db_volume_attachment() - attachment_obj = fake_volume.fake_volume_attachment_obj(self.context) + db_attachment = fake_volume.volume_attachment_db_obj() + attachment_obj = fake_volume.volume_attachment_ovo(self.context) volume_attachment_get.return_value = db_attachment attachment = objects.VolumeAttachment.get_by_id(self.context, fake.ATTACHMENT_ID) @@ -58,11 +58,11 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase): @mock.patch('cinder.db.sqlalchemy.api.volume_attachment_get') def test_refresh(self, attachment_get): - db_attachment1 = fake_volume.fake_db_volume_attachment() - attachment_obj1 = fake_volume.fake_volume_attachment_obj(self.context) - db_attachment2 = db_attachment1.copy() - db_attachment2['mountpoint'] = '/dev/sdc' - attachment_obj2 = fake_volume.fake_volume_attachment_obj( + db_attachment1 = fake_volume.volume_attachment_db_obj() + attachment_obj1 = fake_volume.volume_attachment_ovo(self.context) + db_attachment2 = fake_volume.volume_attachment_db_obj() + db_attachment2.mountpoint = '/dev/sdc' + attachment_obj2 = fake_volume.volume_attachment_ovo( self.context, mountpoint='/dev/sdc') # On the second volume_attachment_get, return the volume attachment @@ -167,33 +167,34 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase): class TestVolumeAttachmentList(test_objects.BaseObjectsTestCase): @mock.patch('cinder.db.volume_attachment_get_all_by_volume_id') def test_get_all_by_volume_id(self, get_used_by_volume_id): - db_attachment = fake_volume.fake_db_volume_attachment() + db_attachment = fake_volume.volume_attachment_db_obj() get_used_by_volume_id.return_value = [db_attachment] - attachment_obj = fake_volume.fake_volume_attachment_obj(self.context) + attachment_obj = fake_volume.volume_attachment_ovo(self.context) attachments = objects.VolumeAttachmentList.get_all_by_volume_id( self.context, mock.sentinel.volume_id) + self.assertEqual(1, len(attachments)) - TestVolumeAttachment._compare(self, attachment_obj, attachments[0]) + self._compare(self, attachment_obj, attachments[0]) @mock.patch('cinder.db.volume_attachment_get_all_by_host') def test_get_all_by_host(self, get_by_host): - db_attachment = fake_volume.fake_db_volume_attachment() - attachment_obj = fake_volume.fake_volume_attachment_obj(self.context) + db_attachment = fake_volume.volume_attachment_db_obj() + attachment_obj = fake_volume.volume_attachment_ovo(self.context) get_by_host.return_value = [db_attachment] attachments = objects.VolumeAttachmentList.get_all_by_host( self.context, mock.sentinel.host) self.assertEqual(1, len(attachments)) - TestVolumeAttachment._compare(self, attachment_obj, attachments[0]) + self._compare(self, attachment_obj, attachments[0]) @mock.patch('cinder.db.volume_attachment_get_all_by_instance_uuid') def test_get_all_by_instance_uuid(self, get_by_instance_uuid): - db_attachment = fake_volume.fake_db_volume_attachment() + db_attachment = fake_volume.volume_attachment_db_obj() get_by_instance_uuid.return_value = [db_attachment] - attachment_obj = fake_volume.fake_volume_attachment_obj(self.context) + attachment_obj = fake_volume.volume_attachment_ovo(self.context) attachments = objects.VolumeAttachmentList.get_all_by_instance_uuid( self.context, mock.sentinel.uuid) self.assertEqual(1, len(attachments)) - TestVolumeAttachment._compare(self, attachment_obj, attachments[0]) + self._compare(self, attachment_obj, attachments[0]) diff --git a/cinder/tests/unit/volume/drivers/test_lvm_driver.py b/cinder/tests/unit/volume/drivers/test_lvm_driver.py index 7da792d2686..e13973c212a 100644 --- a/cinder/tests/unit/volume/drivers/test_lvm_driver.py +++ b/cinder/tests/unit/volume/drivers/test_lvm_driver.py @@ -1000,15 +1000,15 @@ class LVMISCSITestCase(test_driver.BaseDriverTestCase): 'host': 'fakehost2', 'initiator': 'iqn.2012-07.org.fake:02'} - host1_attachment1 = fake_volume.fake_volume_attachment_obj( + host1_attachment1 = fake_volume.volume_attachment_ovo( self.context) host1_attachment1.connector = host1_connector - host1_attachment2 = fake_volume.fake_volume_attachment_obj( + host1_attachment2 = fake_volume.volume_attachment_ovo( self.context) host1_attachment2.connector = host1_connector - host2_attachment = fake_volume.fake_volume_attachment_obj(self.context) + host2_attachment = fake_volume.volume_attachment_ovo(self.context) host2_attachment.connector = host2_connector # Create a multiattach volume object with two active attachments on