From e1ec05dc0da3b53337b1ca6d80d79741dc62c640 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Sat, 11 Jul 2015 13:57:41 +0200 Subject: [PATCH] Move import and export backup metadata to object Encoding and decoding of backup metadata was being done in the backup driver, but since this is not driver specific data we are talking about it makes more sense to have this logic in the backup object. This patch moves the logic to the object while still allowing to have importing/exporting driver specific backup data, even when this is not being used at the moment. Related-Bug: #1455043 Related-Bug: #1476416 Change-Id: I690748e4542880d5e926f3cbf8cedfc86de5cb1f --- cinder/backup/driver.py | 42 ++++++++++-------- cinder/backup/manager.py | 10 ++++- cinder/objects/backup.py | 29 +++++++++++++ cinder/tests/unit/objects/test_backup.py | 45 ++++++++++++++++++++ cinder/tests/unit/test_backup_driver_base.py | 18 +++----- 5 files changed, 111 insertions(+), 33 deletions(-) diff --git a/cinder/backup/driver.py b/cinder/backup/driver.py index c893b79b2..37597ca74 100644 --- a/cinder/backup/driver.py +++ b/cinder/backup/driver.py @@ -16,7 +16,6 @@ """Base class for all backup drivers.""" import abc -import base64 from oslo_config import cfg from oslo_log import log as logging @@ -351,30 +350,37 @@ class BackupDriver(base.Base): return def export_record(self, backup): - """Export backup record. + """Export driver specific backup record information. - Default backup driver implementation. - Serialize the backup record describing the backup into a string. + If backup backend needs additional driver specific information to + import backup record back into the system it must overwrite this method + and return it here as a dictionary so it can be serialized into a + string. - :param backup: backup entry to export - :returns backup_url - a string describing the backup record + Default backup driver implementation has no extra information. + + :param backup: backup object to export + :returns driver_info - dictionary with extra information """ - retval = jsonutils.dumps(backup) - if six.PY3: - retval = retval.encode('utf-8') - return base64.encodestring(retval) + return {} + + def import_record(self, backup, driver_info): + """Import driver specific backup record information. - def import_record(self, backup_url): - """Import and verify backup record. + If backup backend needs additional driver specific information to + import backup record back into the system it must overwrite this method + since it will be called with the extra information that was provided by + export_record when exporting the backup. - Default backup driver implementation. - De-serialize the backup record into a dictionary, so we can - update the database. + Default backup driver implementation does nothing since it didn't + export any specific data in export_record. - :param backup_url: driver specific backup record string - :returns dictionary object with database updates + :param backup: backup object to export + :param driver_info: dictionary with driver specific backup record + information + :returns nothing """ - return jsonutils.loads(base64.decodestring(backup_url)) + return @six.add_metaclass(abc.ABCMeta) diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index e9449ba34..561a175db 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -567,7 +567,8 @@ class BackupManager(manager.SchedulerDependentManager): try: utils.require_driver_initialized(self.driver) backup_service = self.service.get_backup_driver(context) - backup_url = backup_service.export_record(backup) + driver_info = backup_service.export_record(backup) + backup_url = backup.encode_record(driver_info=driver_info) backup_record['backup_url'] = backup_url except Exception as err: msg = six.text_type(err) @@ -616,9 +617,14 @@ class BackupManager(manager.SchedulerDependentManager): else: # Yes... try: + # Deserialize backup record information + backup_options = backup.decode_record(backup_url) + + # Extract driver specific info and pass it to the driver + driver_options = backup_options.pop('driver_info', {}) utils.require_driver_initialized(self.driver) backup_service = self.service.get_backup_driver(context) - backup_options = backup_service.import_record(backup_url) + backup_service.import_record(backup, driver_options) except Exception as err: msg = six.text_type(err) self._update_backup_error(backup, context, msg) diff --git a/cinder/objects/backup.py b/cinder/objects/backup.py index b6f17674d..99be8dfb7 100644 --- a/cinder/objects/backup.py +++ b/cinder/objects/backup.py @@ -12,12 +12,18 @@ # License for the specific language governing permissions and limitations # under the License. +import base64 +import binascii + from oslo_config import cfg from oslo_log import log as logging +from oslo_serialization import jsonutils from oslo_versionedobjects import fields +import six from cinder import db from cinder import exception +from cinder.i18n import _ from cinder import objects from cinder.objects import base from cinder import utils @@ -111,6 +117,29 @@ class Backup(base.CinderPersistentObject, base.CinderObject, with self.obj_as_admin(): db.backup_destroy(self._context, self.id) + @staticmethod + def decode_record(backup_url): + """Deserialize backup metadata from string into a dictionary. + + :raises: InvalidInput + """ + try: + return jsonutils.loads(base64.decodestring(backup_url)) + except binascii.Error: + msg = _("Can't decode backup record.") + except ValueError: + msg = _("Can't parse backup record.") + raise exception.InvalidInput(reason=msg) + + @base.remotable + def encode_record(self, **kwargs): + """Serialize backup object, with optional extra info, into a string.""" + kwargs.update(self) + retval = jsonutils.dumps(kwargs) + if six.PY3: + retval = retval.encode('utf-8') + return base64.encodestring(retval) + @base.CinderObjectRegistry.register class BackupList(base.ObjectListBase, base.CinderObject): diff --git a/cinder/tests/unit/objects/test_backup.py b/cinder/tests/unit/objects/test_backup.py index cbc8fd626..dfb701576 100644 --- a/cinder/tests/unit/objects/test_backup.py +++ b/cinder/tests/unit/objects/test_backup.py @@ -15,6 +15,7 @@ import mock from cinder import context +from cinder import exception from cinder import objects from cinder.tests.unit import fake_volume from cinder.tests.unit import objects as test_objects @@ -84,6 +85,50 @@ class TestBackup(test_objects.BaseObjectsTestCase): self.assertEqual('2', backup.temp_volume_id) self.assertEqual('3', backup.temp_snapshot_id) + def test_import_record(self): + backup = objects.Backup(context=self.context, id=1) + export_string = backup.encode_record() + imported_backup = objects.Backup.decode_record(export_string) + + # Make sure we don't lose data when converting from string + self.assertDictEqual(dict(backup), imported_backup) + + def test_import_record_additional_info(self): + backup = objects.Backup(context=self.context, id=1) + extra_info = {'driver': {'key1': 'value1', 'key2': 'value2'}} + extra_info_copy = extra_info.copy() + export_string = backup.encode_record(extra_info=extra_info) + imported_backup = objects.Backup.decode_record(export_string) + + # Dictionary passed should not be modified + self.assertDictEqual(extra_info_copy, extra_info) + + # Make sure we don't lose data when converting from string and that + # extra info is still there + expected = dict(backup) + expected['extra_info'] = extra_info + self.assertDictEqual(expected, imported_backup) + + def test_import_record_additional_info_cant_overwrite(self): + backup = objects.Backup(context=self.context, id=1) + export_string = backup.encode_record(id='fake_id') + imported_backup = objects.Backup.decode_record(export_string) + + # Make sure the extra_info can't overwrite basic data + self.assertDictEqual(dict(backup), imported_backup) + + def test_import_record_decoding_error(self): + export_string = '123456' + self.assertRaises(exception.InvalidInput, + objects.Backup.decode_record, + export_string) + + def test_import_record_parsing_error(self): + export_string = '' + self.assertRaises(exception.InvalidInput, + objects.Backup.decode_record, + export_string) + class TestBackupList(test_objects.BaseObjectsTestCase): @mock.patch('cinder.db.backup_get_all', return_value=[fake_backup]) diff --git a/cinder/tests/unit/test_backup_driver_base.py b/cinder/tests/unit/test_backup_driver_base.py index b234c47ce..95ca2b446 100644 --- a/cinder/tests/unit/test_backup_driver_base.py +++ b/cinder/tests/unit/test_backup_driver_base.py @@ -14,7 +14,6 @@ # under the License. """ Tests for the backup service base driver. """ -import base64 import uuid import mock @@ -74,20 +73,13 @@ class BackupBaseDriverTestCase(test.TestCase): self.driver.put_metadata(self.volume_id, json_metadata) def test_export_record(self): - export_string = self.driver.export_record(self.backup) - export_dict = jsonutils.loads(base64.decodestring(export_string)) - # Make sure we don't lose data when converting to string - for key in _backup_db_fields: - self.assertTrue(key in export_dict) - self.assertEqual(export_dict[key], self.backup[key]) + export_record = self.driver.export_record(self.backup) + self.assertDictEqual({}, export_record) def test_import_record(self): - export_string = self.driver.export_record(self.backup) - imported_backup = self.driver.import_record(export_string) - # Make sure we don't lose data when converting from string - for key in _backup_db_fields: - self.assertTrue(key in imported_backup) - self.assertEqual(self.backup[key], imported_backup[key]) + export_record = {'key1': 'value1'} + self.assertIsNone(self.driver.import_record(self.backup, + export_record)) class BackupMetadataAPITestCase(test.TestCase): -- 2.45.2