Browse Source

Merge "Support Incremental Backup Completion In RBD"

changes/76/628076/4
Zuul 1 week ago
parent
commit
6cf1656a94

+ 4
- 0
cinder/backup/api.py View File

@@ -279,7 +279,10 @@ class API(base.Base):
279 279
                 raise exception.InvalidBackup(reason=msg)
280 280
 
281 281
         parent_id = None
282
+        parent = None
283
+
282 284
         if latest_backup:
285
+            parent = latest_backup
283 286
             parent_id = latest_backup.id
284 287
             if latest_backup['status'] != fields.BackupStatus.AVAILABLE:
285 288
                 QUOTAS.rollback(context, reservations)
@@ -315,6 +318,7 @@ class API(base.Base):
315 318
                 'availability_zone': availability_zone,
316 319
                 'snapshot_id': snapshot_id,
317 320
                 'data_timestamp': data_timestamp,
321
+                'parent': parent,
318 322
                 'metadata': metadata or {}
319 323
             }
320 324
             backup = objects.Backup(context=context, **kwargs)

+ 122
- 98
cinder/backup/drivers/ceph.py View File

@@ -43,6 +43,7 @@ restore to a new volume (default).
43 43
 """
44 44
 
45 45
 import fcntl
46
+import json
46 47
 import os
47 48
 import re
48 49
 import subprocess
@@ -314,22 +315,39 @@ class CephBackupDriver(driver.BackupDriver):
314 315
         ioctx.close()
315 316
         client.shutdown()
316 317
 
317
-    def _get_backup_base_name(self, volume_id, backup_id=None,
318
-                              diff_format=False):
318
+    def _format_base_name(self, service_metadata):
319
+        base_name = json.loads(service_metadata)["base"]
320
+        return utils.convert_str(base_name)
321
+
322
+    def _get_backup_base_name(self, volume_id, backup=None):
319 323
         """Return name of base image used for backup.
320 324
 
321 325
         Incremental backups use a new base name so we support old and new style
322 326
         format.
323 327
         """
324 328
         # Ensure no unicode
325
-        if diff_format:
329
+        if not backup:
326 330
             return utils.convert_str("volume-%s.backup.base" % volume_id)
327
-        else:
328
-            if backup_id is None:
329
-                msg = _("Backup id required")
330
-                raise exception.InvalidParameterValue(msg)
331
-            return utils.convert_str("volume-%s.backup.%s"
332
-                                     % (volume_id, backup_id))
331
+
332
+        if backup.service_metadata:
333
+            return self._format_base_name(backup.service_metadata)
334
+
335
+        # 'parent' field will only be present in incremental backups. This is
336
+        # filled by cinder-api
337
+        if backup.parent:
338
+            # Old backups don't have the base name in the service_metadata,
339
+            # so we use the default RBD backup base
340
+            if backup.parent.service_metadata:
341
+                service_metadata = backup.parent.service_metadata
342
+                base_name = self._format_base_name(service_metadata)
343
+            else:
344
+                base_name = utils.convert_str("volume-%s.backup.base"
345
+                                              % volume_id)
346
+
347
+            return base_name
348
+
349
+        return utils.convert_str("volume-%s.backup.%s"
350
+                                 % (volume_id, backup.id))
333 351
 
334 352
     def _discard_bytes(self, volume, offset, length):
335 353
         """Trim length bytes from offset.
@@ -479,7 +497,7 @@ class CephBackupDriver(driver.BackupDriver):
479 497
         if base_name is None:
480 498
             try_diff_format = True
481 499
 
482
-            base_name = self._get_backup_base_name(volume_id, backup.id)
500
+            base_name = self._get_backup_base_name(volume_id, backup=backup)
483 501
             LOG.debug("Trying diff format basename='%(basename)s' for "
484 502
                       "backup base image of volume %(volume)s.",
485 503
                       {'basename': base_name, 'volume': volume_id})
@@ -630,7 +648,7 @@ class CephBackupDriver(driver.BackupDriver):
630 648
         if name not in rbds:
631 649
             LOG.debug("Image '%s' not found - trying diff format name", name)
632 650
             if try_diff_format:
633
-                name = self._get_backup_base_name(volume_id, diff_format=True)
651
+                name = self._get_backup_base_name(volume_id)
634 652
                 if name not in rbds:
635 653
                     LOG.debug("Diff format image '%s' not found", name)
636 654
                     return False, name
@@ -657,50 +675,79 @@ class CephBackupDriver(driver.BackupDriver):
657 675
 
658 676
         return False
659 677
 
678
+    def _full_rbd_backup(self, container, base_name, length):
679
+        """Create the base_image for a full RBD backup."""
680
+        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,
681
+                                  container)) as client:
682
+            self._create_base_image(base_name, length, client)
683
+        # Now we just need to return from_snap=None and image_created=True, if
684
+        # there is some exception in making backup snapshot, will clean up the
685
+        # base image.
686
+        return None, True
687
+
688
+    def _incremental_rbd_backup(self, backup, base_name, length,
689
+                                source_rbd_image, volume_id):
690
+        """Select the last snapshot for a RBD incremental backup."""
691
+
692
+        container = backup.container
693
+        last_incr = backup.parent_id
694
+        LOG.debug("Trying to perform an incremental backup with container: "
695
+                  "%(container)s, base_name: %(base)s, source RBD image: "
696
+                  "%(source)s, volume ID %(volume)s and last incremental "
697
+                  "backup ID: %(incr)s.",
698
+                  {'container': container,
699
+                   'base': base_name,
700
+                   'source': source_rbd_image,
701
+                   'volume': volume_id,
702
+                   'incr': last_incr,
703
+                   })
704
+
705
+        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,
706
+                                  container)) as client:
707
+            base_rbd = eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,
708
+                                                           base_name,
709
+                                                           read_only=True))
710
+            try:
711
+                from_snap = self._get_backup_snap_name(base_rbd,
712
+                                                       base_name,
713
+                                                       last_incr)
714
+                if from_snap is None:
715
+                    msg = (_(
716
+                        "Can't find snapshot from parent %(incr)s and "
717
+                        "base name image %(base)s.") %
718
+                        {'incr': last_incr, 'base': base_name})
719
+                    LOG.error(msg)
720
+                    raise exception.BackupRBDOperationFailed(msg)
721
+            finally:
722
+                base_rbd.close()
723
+
724
+        return from_snap, False
725
+
660 726
     def _backup_rbd(self, backup, volume_file, volume_name, length):
661
-        """Create an incremental backup from an RBD image."""
727
+        """Create an incremental or full backup from an RBD image."""
662 728
         rbd_user = volume_file.rbd_user
663 729
         rbd_pool = volume_file.rbd_pool
664 730
         rbd_conf = volume_file.rbd_conf
665 731
         source_rbd_image = eventlet.tpool.Proxy(volume_file.rbd_image)
666 732
         volume_id = backup.volume_id
667
-        updates = {}
668
-        base_name = self._get_backup_base_name(volume_id, diff_format=True)
669
-        image_created = False
670
-        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,
671
-                                  backup.container)) as client:
672
-            # If from_snap does not exist at the destination (and the
673
-            # destination exists), this implies a previous backup has failed.
674
-            # In this case we will force a full backup.
675
-            #
676
-            # TODO(dosaboy): find a way to repair the broken backup
677
-            #
678
-            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list(
679
-                    ioctx=client.ioctx):
680
-                src_vol_snapshots = self.get_backup_snaps(source_rbd_image)
681
-                if src_vol_snapshots:
682
-                    # If there are source volume snapshots but base does not
683
-                    # exist then we delete it and set from_snap to None
684
-                    LOG.debug("Volume '%(volume)s' has stale source "
685
-                              "snapshots so deleting them.",
686
-                              {'volume': volume_id})
687
-                for snap in src_vol_snapshots:
688
-                    from_snap = snap['name']
689
-                    source_rbd_image.remove_snap(from_snap)
690
-                from_snap = None
691
-
692
-                # Create new base image
693
-                self._create_base_image(base_name, length, client)
694
-                image_created = True
695
-            else:
696
-                # If a from_snap is defined and is present in the source volume
697
-                # image but does not exist in the backup base then we look down
698
-                # the list of source volume snapshots and find the latest one
699
-                # for which a backup snapshot exist in the backup base. Until
700
-                # that snapshot is reached, we delete all the other snapshots
701
-                # for which backup snapshot does not exist.
702
-                from_snap = self._get_most_recent_snap(source_rbd_image,
703
-                                                       base_name, client)
733
+        base_name = None
734
+
735
+        # If backup.parent_id is None performs full RBD backup
736
+        if backup.parent_id is None:
737
+            base_name = self._get_backup_base_name(volume_id, backup=backup)
738
+            from_snap, image_created = self._full_rbd_backup(backup.container,
739
+                                                             base_name,
740
+                                                             length)
741
+        # Otherwise performs incremental rbd backup
742
+        else:
743
+            # Find the base name from the parent backup's service_metadata
744
+            base_name = self._get_backup_base_name(volume_id, backup=backup)
745
+            rbd_img = source_rbd_image
746
+            from_snap, image_created = self._incremental_rbd_backup(backup,
747
+                                                                    base_name,
748
+                                                                    length,
749
+                                                                    rbd_img,
750
+                                                                    volume_id)
704 751
 
705 752
         LOG.debug("Using --from-snap '%(snap)s' for incremental backup of "
706 753
                   "volume %(volume)s.",
@@ -744,14 +791,8 @@ class CephBackupDriver(driver.BackupDriver):
744 791
                           "source volume='%(volume)s'.",
745 792
                           {'snapshot': new_snap, 'volume': volume_id})
746 793
                 source_rbd_image.remove_snap(new_snap)
747
-        # We update the parent_id here. The from_snap is of the format:
748
-        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the
749
-        # backup_id of the parent only from from_snap and set it as
750
-        # parent_id
751
-        if from_snap:
752
-            parent_id = from_snap.split('.')
753
-            updates = {'parent_id': parent_id[1]}
754
-        return updates
794
+
795
+        return {'service_metadata': '{"base": "%s"}' % base_name}
755 796
 
756 797
     def _file_is_rbd(self, volume_file):
757 798
         """Returns True if the volume_file is actually an RBD image."""
@@ -765,7 +806,7 @@ class CephBackupDriver(driver.BackupDriver):
765 806
         image.
766 807
         """
767 808
         volume_id = backup.volume_id
768
-        backup_name = self._get_backup_base_name(volume_id, backup.id)
809
+        backup_name = self._get_backup_base_name(volume_id, backup=backup)
769 810
 
770 811
         with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,
771 812
                                   backup.container)) as client:
@@ -868,23 +909,6 @@ class CephBackupDriver(driver.BackupDriver):
868 909
         LOG.debug("Found snapshot '%s'", snaps[0])
869 910
         return snaps[0]
870 911
 
871
-    def _get_most_recent_snap(self, rbd_image, base_name, client):
872
-        """Get the most recent backup snapshot of the provided image.
873
-
874
-        Returns name of most recent backup snapshot or None if there are no
875
-        backup snapshots.
876
-        """
877
-        src_vol_backup_snaps = self.get_backup_snaps(rbd_image, sort=True)
878
-        from_snap = None
879
-
880
-        for snap in src_vol_backup_snaps:
881
-            if self._snap_exists(base_name, snap['name'], client):
882
-                from_snap = snap['name']
883
-                break
884
-            rbd_image.remove_snap(snap['name'])
885
-
886
-        return from_snap
887
-
888 912
     def _get_volume_size_gb(self, volume):
889 913
         """Return the size in gigabytes of the given volume.
890 914
 
@@ -938,17 +962,23 @@ class CephBackupDriver(driver.BackupDriver):
938 962
         volume_file.seek(0)
939 963
         length = self._get_volume_size_gb(volume)
940 964
 
941
-        do_full_backup = False
942
-        if self._file_is_rbd(volume_file):
943
-            # If volume an RBD, attempt incremental backup.
944
-            LOG.debug("Volume file is RBD: attempting incremental backup.")
965
+        if backup.snapshot_id:
966
+            do_full_backup = True
967
+        elif self._file_is_rbd(volume_file):
968
+            # If volume an RBD, attempt incremental or full backup.
969
+            do_full_backup = False
970
+            LOG.debug("Volume file is RBD: attempting optimized backup")
945 971
             try:
946
-                updates = self._backup_rbd(backup, volume_file,
947
-                                           volume.name, length)
972
+                updates = self._backup_rbd(backup, volume_file, volume.name,
973
+                                           length)
948 974
             except exception.BackupRBDOperationFailed:
949
-                LOG.debug("Forcing full backup of volume %s.", volume.id)
950
-                do_full_backup = True
975
+                with excutils.save_and_reraise_exception():
976
+                    self.delete_backup(backup)
951 977
         else:
978
+            if backup.parent_id:
979
+                LOG.debug("Volume file is NOT RBD: can't perform"
980
+                          "incremental backup.")
981
+                raise exception.BackupRBDOperationFailed
952 982
             LOG.debug("Volume file is NOT RBD: will do full backup.")
953 983
             do_full_backup = True
954 984
 
@@ -970,11 +1000,6 @@ class CephBackupDriver(driver.BackupDriver):
970 1000
         LOG.debug("Backup '%(backup_id)s' of volume %(volume_id)s finished.",
971 1001
                   {'backup_id': backup.id, 'volume_id': volume.id})
972 1002
 
973
-        # If updates is empty then set parent_id to None. This will
974
-        # take care if --incremental flag is used in CLI but a full
975
-        # backup is performed instead
976
-        if not updates and backup.parent_id:
977
-            updates = {'parent_id': None}
978 1003
         return updates
979 1004
 
980 1005
     def _full_restore(self, backup, dest_file, dest_name, length,
@@ -989,13 +1014,10 @@ class CephBackupDriver(driver.BackupDriver):
989 1014
             # If a source snapshot is provided we assume the base is diff
990 1015
             # format.
991 1016
             if src_snap:
992
-                diff_format = True
1017
+                backup_name = self._get_backup_base_name(backup.volume_id,
1018
+                                                         backup=backup)
993 1019
             else:
994
-                diff_format = False
995
-
996
-            backup_name = self._get_backup_base_name(backup.volume_id,
997
-                                                     backup_id=backup.id,
998
-                                                     diff_format=diff_format)
1020
+                backup_name = self._get_backup_base_name(backup.volume_id)
999 1021
 
1000 1022
             # Retrieve backup volume
1001 1023
             src_rbd = eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,
@@ -1022,7 +1044,7 @@ class CephBackupDriver(driver.BackupDriver):
1022 1044
         post-process and resize it back to its expected size.
1023 1045
         """
1024 1046
         backup_base = self._get_backup_base_name(backup.volume_id,
1025
-                                                 diff_format=True)
1047
+                                                 backup=backup)
1026 1048
 
1027 1049
         with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,
1028 1050
                                   backup.container)) as client:
@@ -1047,7 +1069,7 @@ class CephBackupDriver(driver.BackupDriver):
1047 1069
         rbd_pool = restore_file.rbd_pool
1048 1070
         rbd_conf = restore_file.rbd_conf
1049 1071
         base_name = self._get_backup_base_name(backup.volume_id,
1050
-                                               diff_format=True)
1072
+                                               backup=backup)
1051 1073
 
1052 1074
         LOG.debug("Attempting incremental restore from base='%(base)s' "
1053 1075
                   "snap='%(snap)s'",
@@ -1179,8 +1201,10 @@ class CephBackupDriver(driver.BackupDriver):
1179 1201
         """
1180 1202
         length = int(volume.size) * units.Gi
1181 1203
 
1182
-        base_name = self._get_backup_base_name(backup.volume_id,
1183
-                                               diff_format=True)
1204
+        if backup.service_metadata:
1205
+            base_name = self._get_backup_base_name(backup.volume_id, backup)
1206
+        else:
1207
+            base_name = self._get_backup_base_name(backup.volume_id)
1184 1208
 
1185 1209
         with eventlet.tpool.Proxy(rbd_driver.RADOSClient(
1186 1210
                                   self, backup.container)) as client:

+ 10
- 1
cinder/objects/backup.py View File

@@ -40,7 +40,8 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
40 40
     # Version 1.4: Add restore_volume_id
41 41
     # Version 1.5: Add metadata
42 42
     # Version 1.6: Add encryption_key_id
43
-    VERSION = '1.6'
43
+    # Version 1.7: Add parent
44
+    VERSION = '1.7'
44 45
 
45 46
     OPTIONAL_FIELDS = ('metadata',)
46 47
 
@@ -55,6 +56,7 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
55 56
         'availability_zone': fields.StringField(nullable=True),
56 57
         'container': fields.StringField(nullable=True),
57 58
         'parent_id': fields.StringField(nullable=True),
59
+        'parent': fields.ObjectField('Backup', nullable=True),
58 60
         'status': c_fields.BackupStatusField(nullable=True),
59 61
         'fail_reason': fields.StringField(nullable=True),
60 62
         'size': fields.IntegerField(nullable=True),
@@ -110,8 +112,14 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
110 112
 
111 113
     def obj_make_compatible(self, primitive, target_version):
112 114
         """Make an object representation compatible with a target version."""
115
+        added_fields = (((1, 7), ('parent',)),)
116
+
113 117
         super(Backup, self).obj_make_compatible(primitive, target_version)
114 118
         target_version = versionutils.convert_version_to_tuple(target_version)
119
+        for version, remove_fields in added_fields:
120
+            if target_version < version:
121
+                for obj_field in remove_fields:
122
+                    primitive.pop(obj_field, None)
115 123
 
116 124
     @classmethod
117 125
     def _from_db_object(cls, context, backup, db_backup, expected_attrs=None):
@@ -174,6 +182,7 @@ class Backup(base.CinderPersistentObject, base.CinderObject,
174 182
                 self.metadata = db.backup_metadata_update(self._context,
175 183
                                                           self.id, metadata,
176 184
                                                           True)
185
+            updates.pop('parent', None)
177 186
             db.backup_update(self._context, self.id, updates)
178 187
 
179 188
         self.obj_reset_changes()

+ 1
- 0
cinder/objects/base.py View File

@@ -150,6 +150,7 @@ OBJ_VERSIONS.add('1.34', {'VolumeAttachment': '1.3'})
150 150
 OBJ_VERSIONS.add('1.35', {'Backup': '1.6', 'BackupImport': '1.6'})
151 151
 OBJ_VERSIONS.add('1.36', {'RequestSpec': '1.4'})
152 152
 OBJ_VERSIONS.add('1.37', {'RequestSpec': '1.5'})
153
+OBJ_VERSIONS.add('1.38', {'Backup': '1.7', 'BackupImport': '1.7'})
153 154
 
154 155
 
155 156
 class CinderObjectRegistry(base.VersionedObjectRegistry):

+ 142
- 105
cinder/tests/unit/backup/drivers/test_backup_ceph.py View File

@@ -15,6 +15,7 @@
15 15
 """ Tests for Ceph backup service."""
16 16
 
17 17
 import hashlib
18
+import json
18 19
 import os
19 20
 import tempfile
20 21
 import threading
@@ -39,6 +40,7 @@ from cinder.i18n import _
39 40
 from cinder import objects
40 41
 from cinder import test
41 42
 from cinder.tests.unit import fake_constants as fake
43
+import cinder.volume.drivers.rbd as rbd_driver
42 44
 
43 45
 # This is used to collect raised exceptions so that tests may check what was
44 46
 # raised.
@@ -119,6 +121,14 @@ class BackupCephTestCase(test.TestCase):
119 121
                   'user_id': userid, 'project_id': projectid}
120 122
         return db.backup_create(self.ctxt, backup)['id']
121 123
 
124
+    def _create_parent_backup_object(self):
125
+        tmp_backup_id = fake.BACKUP3_ID
126
+        self._create_backup_db_entry(tmp_backup_id, self.volume_id,
127
+                                     self.volume_size)
128
+        tmp_backup = objects.Backup.get_by_id(self.ctxt, tmp_backup_id)
129
+        tmp_backup.service_metadata = 'mock_base_name'
130
+        return tmp_backup
131
+
122 132
     def time_inc(self):
123 133
         self.counter += 1
124 134
         return self.counter
@@ -170,6 +180,22 @@ class BackupCephTestCase(test.TestCase):
170 180
         self.backup = objects.Backup.get_by_id(self.ctxt, self.backup_id)
171 181
         self.backup.container = "backups"
172 182
 
183
+        # Create parent backup of volume
184
+        self.parent_backup = self._create_parent_backup_object()
185
+
186
+        # Create alternate backup with parent
187
+        self.alt_backup_id = fake.BACKUP2_ID
188
+        self._create_backup_db_entry(self.alt_backup_id, self.volume_id,
189
+                                     self.volume_size)
190
+
191
+        self.alt_backup = objects.Backup.get_by_id(self.ctxt,
192
+                                                   self.alt_backup_id)
193
+
194
+        base_name = "volume-%s.backup.%s" % (self.volume_id, self.backup_id)
195
+        self.alt_backup.container = "backups"
196
+        self.alt_backup.parent = self.backup
197
+        self.alt_backup.parent.service_metadata = '{"base": "%s"}' % base_name
198
+
173 199
         # Create alternate volume.
174 200
         self.alt_volume_id = str(uuid.uuid4())
175 201
         self._create_volume_db_entry(self.alt_volume_id, self.volume_size)
@@ -255,24 +281,6 @@ class BackupCephTestCase(test.TestCase):
255 281
         self.assertFalse(oldformat)
256 282
         self.assertEqual(1 | 2 | 4 | 64, features)
257 283
 
258
-    @common_mocks
259
-    def test_get_most_recent_snap(self):
260
-        last = 'backup.%s.snap.9824923.1212' % (uuid.uuid4())
261
-
262
-        image = self.mock_rbd.Image.return_value
263
-        with mock.patch.object(self.service, '_snap_exists') as \
264
-                mock_snap_exists:
265
-            mock_snap_exists.return_value = True
266
-            image.list_snaps.return_value = \
267
-                [{'name': 'backup.%s.snap.6423868.2342' % (uuid.uuid4())},
268
-                 {'name': 'backup.%s.snap.1321319.3235' % (uuid.uuid4())},
269
-                 {'name': last},
270
-                 {'name': 'backup.%s.snap.3824923.1412' % (uuid.uuid4())}]
271
-            base_name = "mock_base"
272
-            client = mock.Mock()
273
-            snap = self.service._get_most_recent_snap(image, base_name, client)
274
-        self.assertEqual(last, snap)
275
-
276 284
     @common_mocks
277 285
     def test_get_backup_snap_name(self):
278 286
         snap_name = 'backup.%s.snap.3824923.1412' % (uuid.uuid4())
@@ -415,7 +423,7 @@ class BackupCephTestCase(test.TestCase):
415 423
         with mock.patch.object(self.service, '_backup_metadata'):
416 424
             with mock.patch.object(self.service, '_discard_bytes'):
417 425
                 with tempfile.NamedTemporaryFile() as test_file:
418
-                    self.service.backup(self.backup, self.volume_file)
426
+                    self.service.backup(self.alt_backup, self.volume_file)
419 427
 
420 428
                     # Ensure the files are equal
421 429
                     self.assertEqual(checksum.digest(), self.checksum.digest())
@@ -424,25 +432,34 @@ class BackupCephTestCase(test.TestCase):
424 432
         self.assertNotEqual(threading.current_thread(), thread_dict['thread'])
425 433
 
426 434
     @common_mocks
427
-    def test_get_backup_base_name(self):
428
-        name = self.service._get_backup_base_name(self.volume_id,
429
-                                                  diff_format=True)
435
+    def test_get_backup_base_name_without_backup_param(self):
436
+        """Test _get_backup_base_name without backup."""
437
+        name = self.service._get_backup_base_name(self.volume_id)
430 438
         self.assertEqual("volume-%s.backup.base" % (self.volume_id), name)
431 439
 
432
-        self.assertRaises(exception.InvalidParameterValue,
433
-                          self.service._get_backup_base_name,
434
-                          self.volume_id)
440
+    @common_mocks
441
+    def test_get_backup_base_name_w_backup_and_no_parent(self):
442
+        """Test _get_backup_base_name with backup and no parent."""
443
+        name = self.service._get_backup_base_name(self.volume_id,
444
+                                                  self.backup)
445
+        self.assertEqual("volume-%s.backup.%s" %
446
+                         (self.volume_id, self.backup.id), name)
435 447
 
436
-        name = self.service._get_backup_base_name(self.volume_id, '1234')
437
-        self.assertEqual("volume-%s.backup.%s" % (self.volume_id, '1234'),
438
-                         name)
448
+    @common_mocks
449
+    def test_get_backup_base_name_w_backup_and_parent(self):
450
+        """Test _get_backup_base_name with backup and parent."""
451
+        name = self.service._get_backup_base_name(self.volume_id,
452
+                                                  self.alt_backup)
453
+        base_name = json.loads(self.alt_backup.parent.service_metadata)
454
+        self.assertEqual(base_name["base"], name)
439 455
 
440 456
     @common_mocks
441 457
     @mock.patch('fcntl.fcntl', spec=True)
442 458
     @mock.patch('subprocess.Popen', spec=True)
443 459
     def test_backup_volume_from_rbd(self, mock_popen, mock_fnctl):
460
+        """Test full RBD backup generated successfully."""
444 461
         backup_name = self.service._get_backup_base_name(self.volume_id,
445
-                                                         diff_format=True)
462
+                                                         self.alt_backup)
446 463
 
447 464
         def mock_write_data():
448 465
             self.volume_file.seek(0)
@@ -483,8 +500,11 @@ class BackupCephTestCase(test.TestCase):
483 500
                                  {'name': 'backup.mock.snap.15341241.90'},
484 501
                                  {'name': 'backup.mock.snap.199994362.10'}])
485 502
 
486
-                            output = self.service.backup(self.backup, rbdio)
487
-                            self.assertDictEqual({}, output)
503
+                            output = self.service.backup(self.alt_backup,
504
+                                                         rbdio)
505
+                            base_name = '{"base": "%s"}' % backup_name
506
+                            service_meta = {'service_metadata': base_name}
507
+                            self.assertDictEqual(service_meta, output)
488 508
 
489 509
                             self.assertEqual(['popen_init',
490 510
                                               'read',
@@ -494,7 +514,7 @@ class BackupCephTestCase(test.TestCase):
494 514
                                               'communicate'], self.callstack)
495 515
 
496 516
                             self.assertFalse(mock_full_backup.called)
497
-                            self.assertTrue(mock_get_backup_snaps.called)
517
+                            self.assertFalse(mock_get_backup_snaps.called)
498 518
 
499 519
                             # Ensure the files are equal
500 520
                             self.assertEqual(checksum.digest(),
@@ -505,7 +525,7 @@ class BackupCephTestCase(test.TestCase):
505 525
         with mock.patch.object(self.service, '_backup_rbd') as \
506 526
                 mock_backup_rbd, mock.patch.object(self.service,
507 527
                                                    '_backup_metadata'):
508
-            mock_backup_rbd.return_value = {'parent_id': 'mock'}
528
+            mock_backup_rbd.return_value = {'service_metadata': 'base_name'}
509 529
             image = self.service.rbd.Image()
510 530
             meta = linuxrbd.RBDImageMetadata(image,
511 531
                                              'pool_foo',
@@ -513,15 +533,14 @@ class BackupCephTestCase(test.TestCase):
513 533
                                              'conf_foo')
514 534
             rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
515 535
             output = self.service.backup(self.backup, rbdio)
516
-            self.assertDictEqual({'parent_id': 'mock'}, output)
536
+            self.assertDictEqual({'service_metadata': 'base_name'}, output)
517 537
 
518 538
     @common_mocks
519
-    def test_backup_volume_from_rbd_set_parent_id_none(self):
520
-        backup_name = self.service._get_backup_base_name(
521
-            self.volume_id, diff_format=True)
539
+    def test_backup_volume_from_rbd_got_exception(self):
540
+        base_name = self.service._get_backup_base_name(self.volume_id,
541
+                                                       self.alt_backup)
522 542
 
523
-        self.mock_rbd.RBD().list.return_value = [backup_name]
524
-        self.backup.parent_id = 'mock_parent_id'
543
+        self.mock_rbd.RBD().list.return_value = [base_name]
525 544
 
526 545
         with mock.patch.object(self.service, 'get_backup_snaps'), \
527 546
                 mock.patch.object(self.service, '_rbd_diff_transfer') as \
@@ -551,29 +570,54 @@ class BackupCephTestCase(test.TestCase):
551 570
                                                          'conf_foo')
552 571
                         rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
553 572
                         mock_get_backup_snaps.return_value = (
554
-                            [{'name': 'backup.mock.snap.153464362.12'},
555
-                             {'name': 'backup.mock.snap.199994362.10'}])
556
-                        output = self.service.backup(self.backup, rbdio)
557
-                        self.assertIsNone(output['parent_id'])
573
+                            [{'name': 'backup.mock.snap.153464362.12',
574
+                              'backup_id': 'mock_parent_id'},
575
+                             {'name': 'backup.mock.snap.199994362.10',
576
+                              'backup_id': 'mock'}])
577
+                        self.assertRaises(exception.BackupRBDOperationFailed,
578
+                                          self.service.backup,
579
+                                          self.alt_backup, rbdio)
558 580
 
559 581
     @common_mocks
560 582
     def test_backup_rbd_set_parent_id(self):
561
-        backup_name = self.service._get_backup_base_name(
562
-            self.volume_id, diff_format=True)
583
+        base_name = self.service._get_backup_base_name(self.volume_id,
584
+                                                       self.alt_backup)
563 585
         vol_name = self.volume.name
564 586
         vol_length = self.volume.size
565 587
 
566
-        self.mock_rbd.RBD().list.return_value = [backup_name]
588
+        self.mock_rbd.RBD().list.return_value = [base_name]
567 589
 
568 590
         with mock.patch.object(self.service, '_snap_exists'), \
569
-                mock.patch.object(self.service, '_get_backup_base_name') as \
570
-                mock_get_backup_base_name, \
571
-                mock.patch.object(self.service, '_get_most_recent_snap') as \
572
-                mock_get_most_recent_snap, \
591
+                mock.patch.object(self.service, '_get_backup_snap_name') as \
592
+                mock_get_backup_snap_name, \
573 593
                 mock.patch.object(self.service, '_rbd_diff_transfer'):
574
-            mock_get_backup_base_name.return_value = backup_name
575
-            mock_get_most_recent_snap.return_value = (
576
-                'backup.mock.snap.153464362.12')
594
+            image = self.service.rbd.Image()
595
+            mock_get_backup_snap_name.return_value = 'mock_snap_name'
596
+            meta = linuxrbd.RBDImageMetadata(image,
597
+                                             'pool_foo',
598
+                                             'user_foo',
599
+                                             'conf_foo')
600
+            rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
601
+            rbdio.seek(0)
602
+            output = self.service._backup_rbd(self.alt_backup, rbdio,
603
+                                              vol_name, vol_length)
604
+            base_name = '{"base": "%s"}' % base_name
605
+            self.assertEqual({'service_metadata': base_name}, output)
606
+            self.backup.parent_id = None
607
+
608
+    @common_mocks
609
+    def test_backup_rbd_without_parent_id(self):
610
+        full_backup_name = self.service._get_backup_base_name(self.volume_id,
611
+                                                              self.alt_backup)
612
+        vol_name = self.volume.name
613
+        vol_length = self.volume.size
614
+
615
+        with mock.patch.object(self.service, '_rbd_diff_transfer'), \
616
+                mock.patch.object(self.service, '_create_base_image') as \
617
+                mock_create_base_image, mock.patch.object(
618
+                rbd_driver, 'RADOSClient') as mock_rados_client:
619
+            client = mock.Mock()
620
+            mock_rados_client.return_value.__enter__.return_value = client
577 621
             image = self.service.rbd.Image()
578 622
             meta = linuxrbd.RBDImageMetadata(image,
579 623
                                              'pool_foo',
@@ -581,9 +625,12 @@ class BackupCephTestCase(test.TestCase):
581 625
                                              'conf_foo')
582 626
             rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
583 627
             rbdio.seek(0)
584
-            output = self.service._backup_rbd(self.backup, rbdio,
628
+            output = self.service._backup_rbd(self.alt_backup, rbdio,
585 629
                                               vol_name, vol_length)
586
-            self.assertDictEqual({'parent_id': 'mock'}, output)
630
+            mock_create_base_image.assert_called_with(full_backup_name,
631
+                                                      vol_length, client)
632
+            base_name = '{"base": "%s"}' % full_backup_name
633
+            self.assertEqual({'service_metadata': base_name}, output)
587 634
 
588 635
     @common_mocks
589 636
     @mock.patch('fcntl.fcntl', spec=True)
@@ -597,7 +644,7 @@ class BackupCephTestCase(test.TestCase):
597 644
         self._try_delete_base_image().
598 645
         """
599 646
         backup_name = self.service._get_backup_base_name(self.volume_id,
600
-                                                         diff_format=True)
647
+                                                         self.alt_backup)
601 648
 
602 649
         def mock_write_data():
603 650
             self.volume_file.seek(0)
@@ -662,7 +709,7 @@ class BackupCephTestCase(test.TestCase):
662 709
                             self.assertRaises(
663 710
                                 self.service.rbd.ImageNotFound,
664 711
                                 self.service.backup,
665
-                                self.backup, rbdio)
712
+                                self.alt_backup, rbdio)
666 713
 
667 714
     @common_mocks
668 715
     @mock.patch('fcntl.fcntl', spec=True)
@@ -675,7 +722,7 @@ class BackupCephTestCase(test.TestCase):
675 722
         second exception occurs in self.delete_backup().
676 723
         """
677 724
         backup_name = self.service._get_backup_base_name(self.volume_id,
678
-                                                         diff_format=True)
725
+                                                         self.alt_backup)
679 726
 
680 727
         def mock_write_data():
681 728
             self.volume_file.seek(0)
@@ -733,12 +780,11 @@ class BackupCephTestCase(test.TestCase):
733 780
                         self.assertRaises(
734 781
                             self.service.rbd.ImageBusy,
735 782
                             self.service.backup,
736
-                            self.backup, rbdio)
783
+                            self.alt_backup, rbdio)
737 784
 
738 785
     @common_mocks
739 786
     def test_backup_rbd_from_snap(self):
740
-        backup_name = self.service._get_backup_base_name(self.volume_id,
741
-                                                         diff_format=True)
787
+        backup_name = self.service._get_backup_base_name(self.volume_id)
742 788
         vol_name = self.volume['name']
743 789
         vol_length = self.service._get_volume_size_gb(self.volume)
744 790
 
@@ -780,44 +826,36 @@ class BackupCephTestCase(test.TestCase):
780 826
 
781 827
     @common_mocks
782 828
     def test_backup_rbd_from_snap2(self):
783
-        backup_name = self.service._get_backup_base_name(self.volume_id,
784
-                                                         diff_format=True)
829
+        base_name = self.service._get_backup_base_name(self.volume_id,
830
+                                                       self.alt_backup)
785 831
         vol_name = self.volume['name']
786 832
         vol_length = self.service._get_volume_size_gb(self.volume)
787 833
 
788 834
         self.mock_rbd.RBD().list = mock.Mock()
789
-        self.mock_rbd.RBD().list.return_value = [backup_name]
835
+        self.mock_rbd.RBD().list.return_value = [base_name]
790 836
 
791
-        with mock.patch.object(self.service, '_get_most_recent_snap') as \
792
-                mock_get_most_recent_snap:
793
-            with mock.patch.object(self.service, '_get_backup_base_name') as \
794
-                    mock_get_backup_base_name:
795
-                with mock.patch.object(self.service, '_rbd_diff_transfer') as \
796
-                        mock_rbd_diff_transfer:
797
-                    with mock.patch.object(self.service,
798
-                                           '_get_new_snap_name') as \
799
-                            mock_get_new_snap_name:
800
-                        mock_get_backup_base_name.return_value = (
801
-                            backup_name)
802
-                        mock_get_most_recent_snap.return_value = (
803
-                            'backup.mock.snap.153464362.12')
804
-                        mock_get_new_snap_name.return_value = 'new_snap'
805
-                        image = self.service.rbd.Image()
806
-                        meta = linuxrbd.RBDImageMetadata(image,
807
-                                                         'pool_foo',
808
-                                                         'user_foo',
809
-                                                         'conf_foo')
810
-                        rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
811
-                        rbdio.seek(0)
812
-                        self.service._backup_rbd(self.backup, rbdio,
813
-                                                 vol_name, vol_length)
814
-                        mock_rbd_diff_transfer.assert_called_with(
815
-                            vol_name, 'pool_foo', backup_name,
816
-                            self.backup.container, src_user='user_foo',
817
-                            src_conf='conf_foo',
818
-                            dest_conf='/etc/ceph/ceph.conf',
819
-                            dest_user='cinder', src_snap='new_snap',
820
-                            from_snap='backup.mock.snap.153464362.12')
837
+        with mock.patch.object(self.service, '_get_backup_base_name') as \
838
+                mock_get_backup_base_name:
839
+            with mock.patch.object(self.service, '_rbd_diff_transfer') as \
840
+                    mock_rbd_diff_transfer:
841
+                with mock.patch.object(self.service, '_get_new_snap_name') as \
842
+                        mock_get_new_snap_name:
843
+                    mock_get_backup_base_name.return_value = base_name
844
+                    mock_get_new_snap_name.return_value = 'new_snap'
845
+                    image = self.service.rbd.Image()
846
+                    meta = linuxrbd.RBDImageMetadata(image, 'pool_foo',
847
+                                                     'user_foo', 'conf_foo')
848
+                    rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
849
+                    rbdio.seek(0)
850
+                    self.service._backup_rbd(self.alt_backup, rbdio, vol_name,
851
+                                             vol_length)
852
+                    mock_rbd_diff_transfer.assert_called_with(
853
+                        vol_name, 'pool_foo', base_name,
854
+                        self.backup.container, src_user='user_foo',
855
+                        src_conf='conf_foo',
856
+                        dest_conf='/etc/ceph/ceph.conf',
857
+                        dest_user='cinder', src_snap='new_snap',
858
+                        from_snap=None)
821 859
 
822 860
     @common_mocks
823 861
     def test_backup_vol_length_0(self):
@@ -848,7 +886,7 @@ class BackupCephTestCase(test.TestCase):
848 886
     @common_mocks
849 887
     def test_restore(self):
850 888
         backup_name = self.service._get_backup_base_name(self.volume_id,
851
-                                                         diff_format=True)
889
+                                                         self.alt_backup)
852 890
 
853 891
         self.mock_rbd.RBD.return_value.list.return_value = [backup_name]
854 892
 
@@ -870,7 +908,7 @@ class BackupCephTestCase(test.TestCase):
870 908
                 with tempfile.NamedTemporaryFile() as test_file:
871 909
                     self.volume_file.seek(0)
872 910
 
873
-                    self.service.restore(self.backup, self.volume_id,
911
+                    self.service.restore(self.alt_backup, self.volume_id,
874 912
                                          test_file)
875 913
 
876 914
                     checksum = hashlib.sha256()
@@ -965,8 +1003,7 @@ class BackupCephTestCase(test.TestCase):
965 1003
     @common_mocks
966 1004
     def test_delete_backup_snapshot(self):
967 1005
         snap_name = 'backup.%s.snap.3824923.1412' % (uuid.uuid4())
968
-        base_name = self.service._get_backup_base_name(self.volume_id,
969
-                                                       diff_format=True)
1006
+        base_name = self.service._get_backup_base_name(self.volume_id)
970 1007
         self.mock_rbd.RBD.remove_snap = mock.Mock()
971 1008
         thread_dict = {}
972 1009
 
@@ -996,16 +1033,16 @@ class BackupCephTestCase(test.TestCase):
996 1033
     @mock.patch('cinder.backup.drivers.ceph.VolumeMetadataBackup', spec=True)
997 1034
     def test_try_delete_base_image_diff_format(self, mock_meta_backup):
998 1035
         backup_name = self.service._get_backup_base_name(self.volume_id,
999
-                                                         diff_format=True)
1036
+                                                         self.alt_backup)
1000 1037
 
1001 1038
         self.mock_rbd.RBD.return_value.list.return_value = [backup_name]
1002 1039
 
1003 1040
         with mock.patch.object(self.service, '_delete_backup_snapshot') as \
1004 1041
                 mock_del_backup_snap:
1005
-            snap_name = self.service._get_new_snap_name(self.backup_id)
1042
+            snap_name = self.service._get_new_snap_name(self.alt_backup_id)
1006 1043
             mock_del_backup_snap.return_value = (snap_name, 0)
1007 1044
 
1008
-            self.service.delete_backup(self.backup)
1045
+            self.service.delete_backup(self.alt_backup)
1009 1046
             self.assertTrue(mock_del_backup_snap.called)
1010 1047
 
1011 1048
         self.assertTrue(self.mock_rbd.RBD.return_value.list.called)
@@ -1015,7 +1052,7 @@ class BackupCephTestCase(test.TestCase):
1015 1052
     @mock.patch('cinder.backup.drivers.ceph.VolumeMetadataBackup', spec=True)
1016 1053
     def test_try_delete_base_image(self, mock_meta_backup):
1017 1054
         backup_name = self.service._get_backup_base_name(self.volume_id,
1018
-                                                         self.backup_id)
1055
+                                                         self.alt_backup)
1019 1056
         thread_dict = {}
1020 1057
 
1021 1058
         def mock_side_effect(ioctx, base_name):
@@ -1024,7 +1061,7 @@ class BackupCephTestCase(test.TestCase):
1024 1061
         self.mock_rbd.RBD.return_value.list.return_value = [backup_name]
1025 1062
         self.mock_rbd.RBD.return_value.remove.side_effect = mock_side_effect
1026 1063
         with mock.patch.object(self.service, 'get_backup_snaps'):
1027
-            self.service.delete_backup(self.backup)
1064
+            self.service.delete_backup(self.alt_backup)
1028 1065
             self.assertTrue(self.mock_rbd.RBD.return_value.remove.called)
1029 1066
             self.assertNotEqual(threading.current_thread(),
1030 1067
                                 thread_dict['thread'])
@@ -1033,7 +1070,7 @@ class BackupCephTestCase(test.TestCase):
1033 1070
     def test_try_delete_base_image_busy(self):
1034 1071
         """This should induce retries then raise rbd.ImageBusy."""
1035 1072
         backup_name = self.service._get_backup_base_name(self.volume_id,
1036
-                                                         self.backup_id)
1073
+                                                         self.alt_backup)
1037 1074
 
1038 1075
         rbd = self.mock_rbd.RBD.return_value
1039 1076
         rbd.list.return_value = [backup_name]
@@ -1043,7 +1080,7 @@ class BackupCephTestCase(test.TestCase):
1043 1080
                 mock_get_backup_snaps:
1044 1081
             self.assertRaises(self.mock_rbd.ImageBusy,
1045 1082
                               self.service._try_delete_base_image,
1046
-                              self.backup)
1083
+                              self.alt_backup)
1047 1084
             self.assertTrue(mock_get_backup_snaps.called)
1048 1085
 
1049 1086
         self.assertTrue(rbd.list.called)

+ 2
- 2
cinder/tests/unit/objects/test_objects.py View File

@@ -23,9 +23,9 @@ from cinder import test
23 23
 # NOTE: The hashes in this list should only be changed if they come with a
24 24
 # corresponding version bump in the affected objects.
25 25
 object_data = {
26
-    'Backup': '1.6-c7ede487ba6fbcdd2a4711343cd972be',
26
+    'Backup': '1.7-fffdbcd5da3c30750916fa2cc0e8ffb5',
27 27
     'BackupDeviceInfo': '1.0-74b3950676c690538f4bc6796bd0042e',
28
-    'BackupImport': '1.6-c7ede487ba6fbcdd2a4711343cd972be',
28
+    'BackupImport': '1.7-fffdbcd5da3c30750916fa2cc0e8ffb5',
29 29
     'BackupList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e',
30 30
     'CleanupRequest': '1.0-e7c688b893e1d5537ccf65cc3eb10a28',
31 31
     'Cluster': '1.1-e2c533eb8cdd8d229b6c45c6cf3a9e2c',

+ 5
- 0
releasenotes/notes/support-incremental-backup-completion-in-rbd-1f2165fefcc470d1.yaml View File

@@ -0,0 +1,5 @@
1
+---
2
+fixes:
3
+  - Fixed issue where all Ceph RBD backups would be incremental after the
4
+    first one. The driver now honors whether ``--incremental`` is specified or
5
+    not.

Loading…
Cancel
Save