Cinder backup export broken

Incremental patch I516b7c82b05b26e81195f7f106d43a9e0804082d
introduced a regression, breaking the volume backup export
functionality.

Apparently, the newly introduced "parent" db field isn't
serialized properly.

This is the traceback from the launchpad bug:
oslo_versionedobjects.exception.ObjectActionError:
Object action obj_load_attr failed because: attribute
parent not lazy-loadable

Looking at the traceback, and the Backup OVO code it seems
like we have 2 problems:

- We don't support lazy loading the parent
- We try to serialize the parent

Co-Authored-By: Gorka Eguileor <[email protected]>
Change-Id: I017602353e96cf9f0922074f94276002b17d1359
Closes-Bug: #1862635
(cherry picked from commit a989693)
modified:
cinder/tests/unit/objects/test_backup.py - pyflake issue #202
(cherry picked from commit 6dfbec7)
(cherry picked from commit 196b678)

cinder/objects/backup.py

@@ -43,7 +43,7 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
     # Version 1.7: Add parent
     VERSION = '1.7'
 
-    OPTIONAL_FIELDS = ('metadata',)
+    OPTIONAL_FIELDS = ('metadata', 'parent')
 
     fields = {
         'id': fields.UUIDField(),
@@ -156,6 +156,11 @@ def obj_load_attr(self, attrname):
         if not self._context:
             raise exception.OrphanedObjectError(method='obj_load_attr',
                                                 objtype=self.obj_name())
+        if attrname == 'parent':
+            if self.parent_id:
+                self.parent = self.get_by_id(self._context, self.parent_id)
+            else:
+                self.parent = None
         self.obj_reset_changes(fields=[attrname])
 
     def obj_what_changed(self):
@@ -212,7 +217,7 @@ def encode_record(self, **kwargs):
         # We don't want to export extra fields and we want to force lazy
         # loading, so we can't use dict(self) or self.obj_to_primitive
         record = {name: field.to_primitive(self, name, getattr(self, name))
-                  for name, field in self.fields.items()}
+                  for name, field in self.fields.items() if name != 'parent'}
         # We must update kwargs instead of record to ensure we don't overwrite
         # "real" data from the backup
         kwargs.update(record)

cinder/tests/unit/objects/test_backup.py

@@ -143,6 +143,20 @@ def test_obj_field_metadata(self):
                                 metadata={'test_key': 'test_value'})
         self.assertEqual({'test_key': 'test_value'}, backup.metadata)
 
+    @mock.patch('cinder.objects.backup.Backup.get_by_id',
+                return_value=None)
+    def test_obj_field_parent(self, mock_lzy_ld):
+        backup = objects.Backup(context=self.context,
+                                parent_id=None)
+        self.assertIsNone(backup.parent)
+
+        # Bug #1862635: should trigger a lazy load
+        backup = objects.Backup(context=self.context,
+                                parent_id=fake.UUID5)
+        # need noqa here because of pyflakes issue #202
+        _ = backup.parent  # noqa
+        mock_lzy_ld.assert_called_once()
+
     def test_import_record(self):
         utils.replace_obj_loader(self, objects.Backup)
         backup = objects.Backup(context=self.context, id=fake.BACKUP_ID,
@@ -154,6 +168,24 @@ def test_import_record(self):
         # Make sure we don't lose data when converting from string
         self.assertDictEqual(self._expected_backup(backup), imported_backup)
 
+    @mock.patch('cinder.db.get_by_id', return_value=fake_backup)
+    def test_import_record_w_parent(self, backup_get):
+        full_backup = objects.Backup.get_by_id(self.context, fake.USER_ID)
+        self._compare(self, fake_backup, full_backup)
+
+        utils.replace_obj_loader(self, objects.Backup)
+        incr_backup = objects.Backup(context=self.context,
+                                     id=fake.BACKUP2_ID,
+                                     parent=full_backup,
+                                     parent_id=full_backup['id'],
+                                     num_dependent_backups=0)
+        export_string = incr_backup.encode_record()
+        imported_backup = objects.Backup.decode_record(export_string)
+
+        # Make sure we don't lose data when converting from string
+        self.assertDictEqual(self._expected_backup(incr_backup),
+                             imported_backup)
+
     def test_import_record_additional_info(self):
         utils.replace_obj_loader(self, objects.Backup)
         backup = objects.Backup(context=self.context, id=fake.BACKUP_ID,
@@ -175,7 +207,7 @@ def test_import_record_additional_info(self):
 
     def _expected_backup(self, backup):
         record = {name: field.to_primitive(backup, name, getattr(backup, name))
-                  for name, field in backup.fields.items()}
+                  for name, field in backup.fields.items() if name != 'parent'}
         return record
 
     def test_import_record_additional_info_cant_overwrite(self):