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

diff --git a/cinder/backup/driver.py b/cinder/backup/driver.py
index c893b79b27fbe8a3b73e3818c5169740f5ecbd7c..37597ca74a412aa0292312fad9e2a8db0e450cc5 100644 (file)
--- 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 e9449ba34be1beeebb89252a6954127bfddf81de..561a175db6cc78625caa8c4da08bd8843351c0c3 100644 (file)
--- 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 b6f17674dd293f8b8a465b8873eff96e5d19df3c..99be8dfb7a74348b74b310ad51d8eb53328b91fa 100644 (file)
--- 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 cbc8fd6267b2f4d7eea00f51c15917a6f1bb276a..dfb7015769e25ede7257b6e3bbd6d5175bcf5aa8 100644 (file)
--- 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 b234c47ce4b53802391b15f4fd8024a84b96ea04..95ca2b446ea1ff9b515a10464b1ffd591b703a86 100644 (file)
--- 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):