Conditionally restore display_name

When restoring a volume from backup, if the target volume looks like
it was auto-created as part of the restore and the backed up metadata
contains a name, then apply the name to the target volume. However,
if we are restoring to an existing volume and it has a non-default
name or we do not have a name to restore, then do not modify the
target volume metadata display_name or display_description. This is
intended to be a less drastic solution than commit 7ee80f7 introduced
back in Juno.

Change-Id: I87a62577665b5bd2a82f3305fb715ba6b24d9a78
Closes-Bug: 1539667

cinder/backup/driver.py

@@ -145,17 +145,25 @@ class BackupMetadataAPI(base.Base):
             LOG.debug("No metadata type '%s' available", type_tag)
 
     @staticmethod
-    def _filter(metadata, fields):
+    def _filter(metadata, fields, excludes=None):
         """Returns set of metadata restricted to required fields.
 
         If fields is empty list, the full set is returned.
+
+        :param metadata: master set of metadata
+        :param fields: list of fields we want to extract
+        :param excludes: fields to be exluded
+        :returns: filtered metadata
         """
         if not fields:
             return metadata
 
+        if not excludes:
+            excludes = []
+
         subset = {}
         for field in fields:
-            if field in metadata:
+            if field in metadata and field not in excludes:
                 subset[field] = metadata[field]
             else:
                 LOG.debug("Excluding field '%s'", field)
@@ -165,6 +173,7 @@ class BackupMetadataAPI(base.Base):
     def _restore_vol_base_meta(self, metadata, volume_id, fields):
         """Restore values to Volume object for provided fields."""
         LOG.debug("Restoring volume base metadata")
+        excludes = []
 
         # Ignore unencrypted backups.
         key = 'encryption_key_id'
@@ -172,7 +181,22 @@ class BackupMetadataAPI(base.Base):
             self._restore_vol_encryption_meta(volume_id,
                                               metadata['volume_type_id'])
 
-        metadata = self._filter(metadata, fields)
+        # NOTE(dosaboy): if the target volume looks like it was auto-created
+        # as part of this restore operation and we have a name to restore
+        # then apply the name to the target volume. However, if that target
+        # volume already existed and it has a name or we do not have a name to
+        # restore, then ignore this key. This is intended to be a less drastic
+        # solution than commit 7ee80f7.
+        key = 'display_name'
+        if key in fields and key in metadata:
+            target_vol = self.db.volume_get(self.context, volume_id)
+            name = target_vol.get(key, '')
+            if (not metadata.get(key) or name and
+                    not name.startswith('restore_backup_')):
+                excludes.append(key)
+                excludes.append('display_description')
+
+        metadata = self._filter(metadata, fields, excludes=excludes)
         self.db.volume_update(self.context, volume_id, metadata)
 
     def _restore_vol_encryption_meta(self, volume_id, src_volume_type_id):
@@ -252,7 +276,10 @@ class BackupMetadataAPI(base.Base):
         Empty field list indicates that all backed up fields should be
         restored.
         """
-        return {self.TYPE_TAG_VOL_META:
+        return {self.TYPE_TAG_VOL_BASE_META:
+                (self._restore_vol_base_meta,
+                 ['display_name', 'display_description']),
+                self.TYPE_TAG_VOL_META:
                 (self._restore_vol_meta, []),
                 self.TYPE_TAG_VOL_GLANCE_META:
                 (self._restore_vol_glance_meta, [])}
@@ -269,7 +296,7 @@ class BackupMetadataAPI(base.Base):
         """
         return {self.TYPE_TAG_VOL_BASE_META:
                 (self._restore_vol_base_meta,
-                 ['encryption_key_id']),
+                 ['display_name', 'display_description', 'encryption_key_id']),
                 self.TYPE_TAG_VOL_META:
                 (self._restore_vol_meta, []),
                 self.TYPE_TAG_VOL_GLANCE_META:

cinder/tests/unit/test_backup_driver_base.py

@@ -26,6 +26,7 @@ from cinder import exception
 from cinder import objects
 from cinder import test
 from cinder.tests.unit.backup import fake_service
+from cinder.volume import volume_types
 
 _backup_db_fields = ['id', 'user_id', 'project_id',
                      'volume_id', 'host', 'availability_zone',
@@ -96,6 +97,7 @@ class BackupMetadataAPITestCase(test.TestCase):
         super(BackupMetadataAPITestCase, self).setUp()
         self.ctxt = context.get_admin_context()
         self.volume_id = str(uuid.uuid4())
+        self.backup_id = str(uuid.uuid4())
         self.volume_display_name = 'vol-1'
         self.volume_display_description = 'test vol'
         self._create_volume_db_entry(self.volume_id, 1,
@@ -162,15 +164,46 @@ class BackupMetadataAPITestCase(test.TestCase):
     def test_v1_restore_factory(self):
         fact = self.bak_meta_api._v1_restore_factory()
 
-        keys = [self.bak_meta_api.TYPE_TAG_VOL_META,
+        keys = [self.bak_meta_api.TYPE_TAG_VOL_BASE_META,
+                self.bak_meta_api.TYPE_TAG_VOL_META,
                 self.bak_meta_api.TYPE_TAG_VOL_GLANCE_META]
 
         self.assertEqual(set([]),
                          set(keys).symmetric_difference(set(fact.keys())))
 
         meta_container = {self.bak_meta_api.TYPE_TAG_VOL_BASE_META:
-                          {'display_name': 'vol-2',
+                          {'display_name': 'my-backed-up-volume',
-                           'display_description': 'description'},
+                           'display_description': 'backed up description'},
+                          self.bak_meta_api.TYPE_TAG_VOL_META: {},
+                          self.bak_meta_api.TYPE_TAG_VOL_GLANCE_META: {}}
+
+        # Emulate restore to new volume
+        volume_id = str(uuid.uuid4())
+        vol_name = 'restore_backup_%s' % (self.backup_id)
+        self._create_volume_db_entry(volume_id, 1, vol_name, 'fake volume')
+
+        for f in fact:
+            func = fact[f][0]
+            fields = fact[f][1]
+            func(meta_container[f], volume_id, fields)
+
+        vol = db.volume_get(self.ctxt, volume_id)
+        self.assertEqual('my-backed-up-volume', vol['display_name'])
+        self.assertEqual('backed up description', vol['display_description'])
+
+    def test_v1_restore_factory_no_restore_name(self):
+        fact = self.bak_meta_api._v1_restore_factory()
+
+        keys = [self.bak_meta_api.TYPE_TAG_VOL_BASE_META,
+                self.bak_meta_api.TYPE_TAG_VOL_META,
+                self.bak_meta_api.TYPE_TAG_VOL_GLANCE_META]
+
+        self.assertEqual(set([]),
+                         set(keys).symmetric_difference(set(fact.keys())))
+
+        meta_container = {self.bak_meta_api.TYPE_TAG_VOL_BASE_META:
+                          {'display_name': 'my-backed-up-volume',
+                           'display_description': 'backed up description'},
                           self.bak_meta_api.TYPE_TAG_VOL_META: {},
                           self.bak_meta_api.TYPE_TAG_VOL_GLANCE_META: {}}
         for f in fact:
@@ -193,10 +226,27 @@ class BackupMetadataAPITestCase(test.TestCase):
         self.assertEqual(set([]),
                          set(keys).symmetric_difference(set(fact.keys())))
 
+        volume_types.create(self.ctxt, 'faketype')
+        vol_type = volume_types.get_volume_type_by_name(self.ctxt, 'faketype')
+
+        meta_container = {self.bak_meta_api.TYPE_TAG_VOL_BASE_META:
+                          {'encryption_key_id': '123',
+                           'volume_type_id': vol_type.get('id'),
+                           'display_name': 'vol-2',
+                           'display_description': 'description'},
+                          self.bak_meta_api.TYPE_TAG_VOL_META: {},
+                          self.bak_meta_api.TYPE_TAG_VOL_GLANCE_META: {}}
+
         for f in fact:
             func = fact[f][0]
             fields = fact[f][1]
-            func({}, self.volume_id, fields)
+            func(meta_container[f], self.volume_id, fields)
+
+        vol = db.volume_get(self.ctxt, self.volume_id)
+        self.assertEqual(self.volume_display_name, vol['display_name'])
+        self.assertEqual(self.volume_display_description,
+                         vol['display_description'])
+        self.assertEqual('123', vol['encryption_key_id'])
 
     def test_restore_vol_glance_meta(self):
         # Fields is an empty list for _restore_vol_glance_meta method.