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 <geguileo@redhat.com>
Change-Id: I017602353e96cf9f0922074f94276002b17d1359
Closes-Bug: #1862635
(cherry picked from commit a98969380a)
modified:
cinder/tests/unit/objects/test_backup.py - pyflake issue #202
(cherry picked from commit 6dfbec7f74)
(cherry picked from commit 196b6786db)
This commit is contained in:
Sofia Enriquez 2020-02-20 16:25:50 +00:00 committed by Brian Rosmaita
parent 7068da1c0d
commit bcea359d54
2 changed files with 40 additions and 3 deletions

View File

@ -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 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
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 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
# 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)

View File

@ -143,6 +143,20 @@ class TestBackup(test_objects.BaseObjectsTestCase):
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 @@ class TestBackup(test_objects.BaseObjectsTestCase):
# 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 @@ class TestBackup(test_objects.BaseObjectsTestCase):
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):