Browse Source

Merge "Fix VolumeAttachment is not bound to a Session" into stable/rocky

changes/00/680000/2
Zuul 1 week ago
parent
commit
7bd4c150a9

+ 6
- 2
cinder/objects/volume_attachment.py View File

@@ -95,8 +95,12 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
95 95
                         jsonutils.loads(value) if value else None)
96 96
             else:
97 97
                 attachment[name] = value
98
-        if 'volume' in expected_attrs:
99
-            db_volume = db_attachment.get('volume')
98
+        # NOTE: hasattr check is necessary to avoid doing a lazy loading when
99
+        # loading VolumeList.get_all: Getting a Volume loads its
100
+        # VolumeAttachmentList, which think they have the volume loaded, but
101
+        # they don't.  More detail on https://review.openstack.org/632549
102
+        if 'volume' in expected_attrs and hasattr(db_attachment, 'volume'):
103
+            db_volume = db_attachment.volume
100 104
             if db_volume:
101 105
                 attachment.volume = objects.Volume._from_db_object(
102 106
                     context, objects.Volume(), db_volume)

+ 22
- 0
cinder/tests/unit/fake_volume.py View File

@@ -15,6 +15,7 @@
15 15
 from oslo_utils.uuidutils import is_uuid_like
16 16
 from oslo_versionedobjects import fields
17 17
 
18
+from cinder.db.sqlalchemy import models
18 19
 from cinder import objects
19 20
 from cinder.objects import fields as c_fields
20 21
 from cinder.tests.unit import fake_constants as fake
@@ -123,3 +124,24 @@ def fake_volume_attachment_obj(context, **updates):
123 124
     return objects.VolumeAttachment._from_db_object(
124 125
         context, objects.VolumeAttachment(),
125 126
         fake_db_volume_attachment(**updates))
127
+
128
+
129
+def volume_db_obj(**updates):
130
+    """Return a volume ORM object."""
131
+    updates.setdefault('id', fake.VOLUME_ID)
132
+    updates.setdefault('size', 1)
133
+    return models.Volume(**updates)
134
+
135
+
136
+def volume_attachment_db_obj(**updates):
137
+    updates.setdefault('id', fake.ATTACHMENT_ID)
138
+    updates.setdefault('volume_id', fake.VOLUME_ID)
139
+    updates.setdefault('volume', volume_db_obj())
140
+    return models.VolumeAttachment(**updates)
141
+
142
+
143
+def volume_attachment_ovo(context, **updates):
144
+    orm = volume_attachment_db_obj(**updates)
145
+    return objects.VolumeAttachment._from_db_object(context,
146
+                                                    objects.VolumeAttachment(),
147
+                                                    orm)

+ 17
- 16
cinder/tests/unit/objects/test_volume_attachment.py View File

@@ -29,8 +29,8 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
29 29
 
30 30
     @mock.patch('cinder.db.sqlalchemy.api.volume_attachment_get')
31 31
     def test_get_by_id(self, volume_attachment_get):
32
-        db_attachment = fake_volume.fake_db_volume_attachment()
33
-        attachment_obj = fake_volume.fake_volume_attachment_obj(self.context)
32
+        db_attachment = fake_volume.volume_attachment_db_obj()
33
+        attachment_obj = fake_volume.volume_attachment_ovo(self.context)
34 34
         volume_attachment_get.return_value = db_attachment
35 35
         attachment = objects.VolumeAttachment.get_by_id(self.context,
36 36
                                                         fake.ATTACHMENT_ID)
@@ -58,11 +58,11 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
58 58
 
59 59
     @mock.patch('cinder.db.sqlalchemy.api.volume_attachment_get')
60 60
     def test_refresh(self, attachment_get):
61
-        db_attachment1 = fake_volume.fake_db_volume_attachment()
62
-        attachment_obj1 = fake_volume.fake_volume_attachment_obj(self.context)
63
-        db_attachment2 = db_attachment1.copy()
64
-        db_attachment2['mountpoint'] = '/dev/sdc'
65
-        attachment_obj2 = fake_volume.fake_volume_attachment_obj(
61
+        db_attachment1 = fake_volume.volume_attachment_db_obj()
62
+        attachment_obj1 = fake_volume.volume_attachment_ovo(self.context)
63
+        db_attachment2 = fake_volume.volume_attachment_db_obj()
64
+        db_attachment2.mountpoint = '/dev/sdc'
65
+        attachment_obj2 = fake_volume.volume_attachment_ovo(
66 66
             self.context, mountpoint='/dev/sdc')
67 67
 
68 68
         # On the second volume_attachment_get, return the volume attachment
@@ -167,33 +167,34 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
167 167
 class TestVolumeAttachmentList(test_objects.BaseObjectsTestCase):
168 168
     @mock.patch('cinder.db.volume_attachment_get_all_by_volume_id')
169 169
     def test_get_all_by_volume_id(self, get_used_by_volume_id):
170
-        db_attachment = fake_volume.fake_db_volume_attachment()
170
+        db_attachment = fake_volume.volume_attachment_db_obj()
171 171
         get_used_by_volume_id.return_value = [db_attachment]
172
-        attachment_obj = fake_volume.fake_volume_attachment_obj(self.context)
172
+        attachment_obj = fake_volume.volume_attachment_ovo(self.context)
173 173
 
174 174
         attachments = objects.VolumeAttachmentList.get_all_by_volume_id(
175 175
             self.context, mock.sentinel.volume_id)
176
+
176 177
         self.assertEqual(1, len(attachments))
177
-        TestVolumeAttachment._compare(self, attachment_obj, attachments[0])
178
+        self._compare(self, attachment_obj, attachments[0])
178 179
 
179 180
     @mock.patch('cinder.db.volume_attachment_get_all_by_host')
180 181
     def test_get_all_by_host(self, get_by_host):
181
-        db_attachment = fake_volume.fake_db_volume_attachment()
182
-        attachment_obj = fake_volume.fake_volume_attachment_obj(self.context)
182
+        db_attachment = fake_volume.volume_attachment_db_obj()
183
+        attachment_obj = fake_volume.volume_attachment_ovo(self.context)
183 184
         get_by_host.return_value = [db_attachment]
184 185
 
185 186
         attachments = objects.VolumeAttachmentList.get_all_by_host(
186 187
             self.context, mock.sentinel.host)
187 188
         self.assertEqual(1, len(attachments))
188
-        TestVolumeAttachment._compare(self, attachment_obj, attachments[0])
189
+        self._compare(self, attachment_obj, attachments[0])
189 190
 
190 191
     @mock.patch('cinder.db.volume_attachment_get_all_by_instance_uuid')
191 192
     def test_get_all_by_instance_uuid(self, get_by_instance_uuid):
192
-        db_attachment = fake_volume.fake_db_volume_attachment()
193
+        db_attachment = fake_volume.volume_attachment_db_obj()
193 194
         get_by_instance_uuid.return_value = [db_attachment]
194
-        attachment_obj = fake_volume.fake_volume_attachment_obj(self.context)
195
+        attachment_obj = fake_volume.volume_attachment_ovo(self.context)
195 196
 
196 197
         attachments = objects.VolumeAttachmentList.get_all_by_instance_uuid(
197 198
             self.context, mock.sentinel.uuid)
198 199
         self.assertEqual(1, len(attachments))
199
-        TestVolumeAttachment._compare(self, attachment_obj, attachments[0])
200
+        self._compare(self, attachment_obj, attachments[0])

+ 3
- 3
cinder/tests/unit/volume/drivers/test_lvm_driver.py View File

@@ -1000,15 +1000,15 @@ class LVMISCSITestCase(test_driver.BaseDriverTestCase):
1000 1000
                            'host': 'fakehost2',
1001 1001
                            'initiator': 'iqn.2012-07.org.fake:02'}
1002 1002
 
1003
-        host1_attachment1 = fake_volume.fake_volume_attachment_obj(
1003
+        host1_attachment1 = fake_volume.volume_attachment_ovo(
1004 1004
             self.context)
1005 1005
         host1_attachment1.connector = host1_connector
1006 1006
 
1007
-        host1_attachment2 = fake_volume.fake_volume_attachment_obj(
1007
+        host1_attachment2 = fake_volume.volume_attachment_ovo(
1008 1008
             self.context)
1009 1009
         host1_attachment2.connector = host1_connector
1010 1010
 
1011
-        host2_attachment = fake_volume.fake_volume_attachment_obj(self.context)
1011
+        host2_attachment = fake_volume.volume_attachment_ovo(self.context)
1012 1012
         host2_attachment.connector = host2_connector
1013 1013
 
1014 1014
         # Create a multiattach volume object with two active attachments on

Loading…
Cancel
Save