Fix VolumeAttachment is not bound to a Session
In some cases, when loading a list of volumes using cinder.objects.VolumeList.get_all we may end up with a SQL alchemy error like this one: sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <VolumeAttachment at 0x7f048ff22290> is not bound to a Session; lazy load operation of attribute 'volume' cannot proceed (Background on this error at: http://sqlalche.me/e/bhk3) This happens because when loading a Cinder Volume we also load the volume attachments, so the Volume OVO tries to create the VolumeAttachmentList OVO, which in turn creates the individual VolumeAttachment OVOs, but these have the volume field set in expected_attrs, but in this case the volume data is not loaded. In most cases we don't see this issue because the session is still available and SQLAlchemy will return this information. Instead of using the dictionary "get" method that will do the lazy loading we use the hasattr method that will return False if the lazy loading exception happens when trying to get the volume. Depends-On: I9f0fec25444ed865d56d0d250fb6d840ab5b4095 Closes-Bug: #1812913 Change-Id: I253123d5451b32f0e3143916e41aaa1af75561c2
This commit is contained in:
parent
0e814633d0
commit
42ce739a08
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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])
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user