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 commit 2e73bede80
)
This commit is contained in:
parent
742eb13901
commit
44a15be6a7
|
@ -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}
|
||||
|
|
|
@ -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