Fix DetachedInstanceError for VolumeAttachment
Patch I253123d5451b32f0e3143916e41aaa1af75561c2 fixed the DetachedInstanceError for VolumeAttachment OVOs but only partially, as apparently it was dependent on the SQLAlchemy version due to the use os "hasattr". This patch replaces "hasattr" with a check on the object's dictionary, which will never trigger a Lazy Load. Closes-Bug: #1834845 Change-Id: Iac785eef9be4b9cdb5c739ee0a87949805282867 (cherry picked from commit2e73bede80
) (cherry picked from commit44a15be6a7
) Conflicts: cinder/tests/unit/volume/drivers/test_lvm_driver.py (cherry picked from commiteabf648b7a
)
This commit is contained in:
parent
80e29d3ad5
commit
2abd8a68bb
|
@ -95,11 +95,14 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
|
|||
jsonutils.loads(value) if value else None)
|
||||
else:
|
||||
attachment[name] = value
|
||||
# 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'):
|
||||
# NOTE: Check against the ORM instance's dictionary instead of using
|
||||
# hasattr or get to avoid the lazy loading of the Volume on
|
||||
# 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.opendev.org/632549
|
||||
# and its related bug report.
|
||||
if 'volume' in expected_attrs and 'volume' in vars(db_attachment):
|
||||
db_volume = db_attachment.volume
|
||||
if db_volume:
|
||||
attachment.volume = objects.Volume._from_db_object(
|
||||
|
|
|
@ -307,7 +307,7 @@ class TestVolume(test_objects.BaseObjectsTestCase):
|
|||
db_admin_metadata = [{'key': 'admin_foo', 'value': 'admin_bar'}]
|
||||
db_glance_metadata = [{'key': 'glance_foo', 'value': 'glance_bar'}]
|
||||
db_volume_type = fake_volume.fake_db_volume_type()
|
||||
db_volume_attachments = fake_volume.fake_db_volume_attachment()
|
||||
db_volume_attachments = fake_volume.volume_attachment_db_obj()
|
||||
db_consistencygroup = fake_consistencygroup.fake_db_consistencygroup()
|
||||
db_snapshots = fake_snapshot.fake_db_snapshot()
|
||||
|
||||
|
@ -450,7 +450,7 @@ class TestVolume(test_objects.BaseObjectsTestCase):
|
|||
@mock.patch('cinder.db.sqlalchemy.api.volume_attach')
|
||||
def test_begin_attach(self, volume_attach, metadata_update):
|
||||
volume = fake_volume.fake_volume_obj(self.context)
|
||||
db_attachment = fake_volume.fake_db_volume_attachment(
|
||||
db_attachment = fake_volume.volume_attachment_db_obj(
|
||||
volume_id=volume.id,
|
||||
attach_status=fields.VolumeAttachStatus.ATTACHING)
|
||||
volume_attach.return_value = db_attachment
|
||||
|
|
|
@ -15,6 +15,7 @@
|
|||
import ddt
|
||||
import mock
|
||||
import six
|
||||
from sqlalchemy.orm import attributes
|
||||
|
||||
from cinder import db
|
||||
from cinder import objects
|
||||
|
@ -47,9 +48,24 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
|
|||
self.assertEqual(volume, r)
|
||||
volume_get_mock.assert_called_once_with(self.context, volume.id)
|
||||
|
||||
def test_from_db_object_no_volume(self):
|
||||
original_get = attributes.InstrumentedAttribute.__get__
|
||||
|
||||
def my_get(get_self, instance, owner):
|
||||
self.assertNotEqual('volume', get_self.key)
|
||||
return original_get(get_self, instance, owner)
|
||||
|
||||
# Volume field is not loaded
|
||||
attach = fake_volume.models.VolumeAttachment(id=fake.ATTACHMENT_ID,
|
||||
volume_id=fake.VOLUME_ID)
|
||||
patch_str = 'sqlalchemy.orm.attributes.InstrumentedAttribute.__get__'
|
||||
with mock.patch(patch_str, side_effect=my_get):
|
||||
objects.VolumeAttachment._from_db_object(
|
||||
self.context, objects.VolumeAttachment(), attach)
|
||||
|
||||
@mock.patch('cinder.db.volume_attachment_update')
|
||||
def test_save(self, volume_attachment_update):
|
||||
attachment = fake_volume.fake_volume_attachment_obj(self.context)
|
||||
attachment = fake_volume.volume_attachment_ovo(self.context)
|
||||
attachment.attach_status = fields.VolumeAttachStatus.ATTACHING
|
||||
attachment.save()
|
||||
volume_attachment_update.assert_called_once_with(
|
||||
|
@ -88,7 +104,7 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
|
|||
|
||||
@mock.patch('cinder.db.sqlalchemy.api.volume_attached')
|
||||
def test_volume_attached(self, volume_attached):
|
||||
attachment = fake_volume.fake_volume_attachment_obj(self.context)
|
||||
attachment = fake_volume.volume_attachment_ovo(self.context)
|
||||
updated_values = {'mountpoint': '/dev/sda',
|
||||
'attach_status': fields.VolumeAttachStatus.ATTACHED,
|
||||
'instance_uuid': fake.INSTANCE_ID}
|
||||
|
|
|
@ -1080,10 +1080,10 @@ class LVMISCSITestCase(test_driver.BaseDriverTestCase):
|
|||
'mountpoint': '/dev/vdc',
|
||||
'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
|
||||
attachment1 = fake_volume.fake_volume_attachment_obj(self.context)
|
||||
attachment1 = fake_volume.volume_attachment_ovo(self.context)
|
||||
attachment1.connector = connector_mountpoint_1
|
||||
|
||||
attachment2 = fake_volume.fake_volume_attachment_obj(self.context)
|
||||
attachment2 = fake_volume.volume_attachment_ovo(self.context)
|
||||
attachment2.connector = connector_mountpoint_2
|
||||
|
||||
# Create a multiattach volume object with two active attachments on
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fix DetachedInstanceError is not bound to a Session for VolumeAttachments.
|
||||
This affected VolumeList.get_all, and could make a service fail on startup
|
||||
and make it stay in down state.
|
Loading…
Reference in New Issue