Share backups enhancement

Added new column named 'backup_type' in 'share_backups' table
and changes the share common api libs to support the dhss_true
configuration for share backup creation

Partially-implements: bp/share-backup
Change-Id: Ifb88ec096674ea8bc010c1c3f6dea1b51be3beaa

manila/api/openstack/api_version_request.py

@@ -202,13 +202,14 @@ REST_API_VERSION_HISTORY = """
     * 2.82 - Added lock and restriction to share access rules.
     * 2.83 - Added 'disabled_reason' field to services.
     * 2.84 - Added mount_point_name to shares.
+    * 2.85 - Added backup_type field to share backups.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.84"
+_MAX_API_VERSION = "2.85"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -454,3 +454,7 @@ user documentation.
 2.84
 ----
    Added optional ``mount_point_name`` field to share.
+
+2.85
+----
+   Added ``backup_type`` field to share backup object.

manila/api/views/share_backups.py

@@ -23,6 +23,10 @@ class BackupViewBuilder(common.ViewBuilder):
     _collection_name = 'share_backups'
     _collection_links = 'share_backup_links'
 
+    _detail_version_modifiers = [
+        "add_backup_type_field",
+    ]
+
     def summary_list(self, request, backups):
         """Summary view of a list of backups."""
         return self._list_view(self.summary, request, backups)
@@ -68,6 +72,7 @@ class BackupViewBuilder(common.ViewBuilder):
             'restore_progress': backup.get('restore_progress'),
         }
 
+        self.update_versioned_resource_dict(request, backup_dict, backup)
         if policy.check_is_host_admin(context):
             backup_dict['host'] = backup.get('host')
             backup_dict['topic'] = backup.get('topic')
@@ -88,3 +93,7 @@ class BackupViewBuilder(common.ViewBuilder):
             backups_dict[self._collection_links] = backup_links
 
         return backups_dict
+
+    @common.ViewBuilder.versioned_method("2.85")
+    def add_backup_type_field(self, context, backup_dict, backup):
+        backup_dict['backup_type'] = backup.get('backup_type')

manila/common/constants.py

@@ -113,6 +113,7 @@ TASK_STATE_DATA_COPYING_COMPLETING = 'data_copying_completing'
 TASK_STATE_DATA_COPYING_COMPLETED = 'data_copying_completed'
 TASK_STATE_DATA_COPYING_CANCELLED = 'data_copying_cancelled'
 TASK_STATE_DATA_COPYING_ERROR = 'data_copying_error'
+BACKUP_TYPE = "backup_type"
 
 BUSY_TASK_STATES = (
     TASK_STATE_MIGRATION_STARTING,

manila/db/migrations/alembic/versions/2f27d904214c_add_backup_type_column.py

@@ -0,0 +1,49 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""add_backup_type_column
+
+Revision ID: 2f27d904214c
+Revises: 6e32091979e0
+Create Date: 2024-03-10 23:16:18.130654
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '2f27d904214c'
+down_revision = '6e32091979e0'
+
+from alembic import op
+from oslo_log import log
+import sqlalchemy as sa
+
+
+LOG = log.getLogger(__name__)
+share_backups_table_name = 'share_backups'
+column_name = "backup_type"
+
+
+def upgrade():
+    try:
+        op.add_column(share_backups_table_name,
+                      sa.Column(column_name, sa.String(32), nullable=True))
+    except Exception:
+        LOG.error("Column 'backup_type' not created!")
+        raise
+
+
+def downgrade():
+    try:
+        op.drop_column(share_backups_table_name, column_name)
+    except Exception:
+        LOG.error("Column backup_type not dropped!")
+        raise

manila/db/sqlalchemy/models.py

@@ -1533,6 +1533,7 @@ class ShareBackup(BASE, ManilaBase):
     restore_progress = Column(String(32))
     status = Column(String(255))
     fail_reason = Column(String(1023))
+    backup_type = Column(String(32))
     availability_zone_id = Column(String(36),
                                   ForeignKey('availability_zones.id'),
                                   nullable=True)

manila/share/api.py

@@ -29,6 +29,7 @@ from oslo_utils import excutils
 from oslo_utils import strutils
 from oslo_utils import timeutils
 from oslo_utils import uuidutils
+from webob import exc
 
 from manila.api import common as api_common
 from manila.common import constants
@@ -3931,6 +3932,31 @@ class API(base.Base):
                     raise exception.BackupLimitExceeded(
                         allowed=quotas[over])
 
+        # Validate right backup type is provided
+        backup_type = backup.get('backup_options') and backup.get(
+            'backup_options').get(constants.BACKUP_TYPE)
+        filters = {
+            'status': constants.STATUS_AVAILABLE,
+            'share_id': share_id,
+            'topic': CONF.share_topic,
+        }
+        backups = self.db.share_backups_get_all(context, filters)
+        if backup_type and len(backups) > 0:
+            previous_backup_type = backups[0][constants.BACKUP_TYPE]
+            backup_options = backup.get('backup_options')
+            current_backup_type = backup_options.get(constants.BACKUP_TYPE)
+            if previous_backup_type != current_backup_type:
+                err_msg = _("Share '%(share)s' has existing backups with"
+                            " backup_type: '%(correct_backup_type)s'. You must"
+                            " delete these backups to schedule a backup with"
+                            " a different backup_type, or re-use the same"
+                            " backup_type.")
+                msg_args = {
+                    'share': share.get('display_name'),
+                    'correct_backup_type': previous_backup_type,
+                }
+                raise exc.HTTPBadRequest(explanation=err_msg % msg_args)
+
         backup_ref = {}
         try:
             backup_ref = self.db.share_backup_create(
@@ -3944,7 +3970,9 @@ class API(base.Base):
                     'display_description': backup.get('description'),
                     'display_name': backup.get('name'),
                     'size': share['size'],
-                    'availability_zone': share['instance']['availability_zone']
+                    'availability_zone': share['instance']
+                    ['availability_zone'],
+                    'backup_type': backup_type,
                 }
             )
             QUOTAS.commit(context, reservations)

manila/share/driver.py

@@ -3666,7 +3666,8 @@ class ShareDriver(object):
         """
         raise NotImplementedError()
 
-    def create_backup(self, context, share_instance, backup):
+    def create_backup(self, context, share_instance, backup,
+                      share_server=None):
         """Starts backup of a given share_instance into backup.
 
         Driver should implement this method if willing to perform backup of
@@ -3677,10 +3678,12 @@ class ShareDriver(object):
         :param context: The 'context.RequestContext' object for the request.
         :param share_instance: Reference to the original share instance.
         :param backup: Share backup model.
+        :param share_server: share server in case of dhss_true
         """
         raise NotImplementedError()
 
-    def create_backup_continue(self, context, share_instance, backup):
+    def create_backup_continue(self, context, share_instance, backup,
+                               share_server=None):
         """Continue backup of a given share_instance into backup.
 
         Driver must implement this method if it supports 'create_backup'
@@ -3690,14 +3693,17 @@ class ShareDriver(object):
         :param context: The 'context.RequestContext' object for the request.
         :param share_instance: Reference to the original share instance.
         :param backup: Share backup model.
+        :param share_server: share server in case of dhss_true
         """
         raise NotImplementedError()
 
-    def delete_backup(self, context, backup):
+    def delete_backup(self, context, backup, share_instance,
+                      share_server=None):
         """Is called to remove backup."""
         raise NotImplementedError()
 
-    def restore_backup(self, context, backup, share_instance):
+    def restore_backup(self, context, backup, share_instance,
+                       share_server=None):
         """Starts restoring backup into a given share_instance.
 
         Driver should implement this method if willing to perform restore of
@@ -3708,10 +3714,12 @@ class ShareDriver(object):
         :param context: The 'context.RequestContext' object for the request.
         :param share_instance: Reference to the original share instance.
         :param backup: Share backup model.
+        :param share_server: share server in case of dhss_true
         """
         raise NotImplementedError()
 
-    def restore_backup_continue(self, context, backup, share_instance):
+    def restore_backup_continue(self, context, backup, share_instance,
+                                share_server=None):
         """Continue restore of a given backup into share_instance.
 
         Driver must implement this method if it supports 'restore_backup'
@@ -3721,5 +3729,6 @@ class ShareDriver(object):
         :param context: The 'context.RequestContext' object for the request.
         :param share_instance: Reference to the original share instance.
         :param backup: Share backup model.
+        :param share_server: share server in case of dhss_true
         """
         raise NotImplementedError()

manila/share/manager.py

@@ -5121,9 +5121,10 @@ class ShareManager(manager.SchedulerDependentManager):
 
         LOG.info('Create backup started, backup: %(backup)s share: '
                  '%(share)s.', {'backup': backup_id, 'share': share_id})
-
         try:
-            self.driver.create_backup(context, share_instance, backup)
+            share_server = self._get_share_server(context, share)
+            self.driver.create_backup(context, share_instance, backup,
+                                      share_server=share_server)
         except Exception as err:
             with excutils.save_and_reraise_exception():
                 LOG.error("Failed to create share backup %s by driver.",
@@ -5151,8 +5152,9 @@ class ShareManager(manager.SchedulerDependentManager):
             share_instance = self._get_share_instance(context, share)
             result = {}
             try:
+                share_server = self._get_share_server(context, share_instance)
                 result = self.driver.create_backup_continue(
-                    context, share_instance, backup)
+                    context, share_instance, backup, share_server=share_server)
                 progress = result.get('total_progress', '0')
                 self.db.share_backup_update(context, backup_id,
                                             {'progress': progress})
@@ -5181,8 +5183,13 @@ class ShareManager(manager.SchedulerDependentManager):
 
         backup_id = backup['id']
         project_id = backup['project_id']
+        share_id = backup['share_id']
+        share = self.db.share_get(context, share_id)
+        share_instance = self._get_share_instance(context, share)
         try:
-            self.driver.delete_backup(context, backup)
+            share_server = self._get_share_server(context, share_instance)
+            self.driver.delete_backup(context, backup, share_instance,
+                                      share_server=share_server)
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.error("Failed to delete share backup %s.", backup_id)
@@ -5218,7 +5225,9 @@ class ShareManager(manager.SchedulerDependentManager):
         share_instance = self._get_share_instance(context, share)
 
         try:
-            self.driver.restore_backup(context, backup, share_instance)
+            share_server = self._get_share_server(context, share_instance)
+            self.driver.restore_backup(context, backup, share_instance,
+                                       share_server=share_server)
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.error("Failed to restore backup %(backup)s to share "
@@ -5256,10 +5265,12 @@ class ShareManager(manager.SchedulerDependentManager):
 
                 share_id = share['id']
                 share_instance = self._get_share_instance(context, share)
-                result = {}
                 try:
+                    share_server = self._get_share_server(
+                        context, share_instance)
                     result = self.driver.restore_backup_continue(
-                        context, backup, share_instance)
+                        context, backup, share_instance,
+                        share_server=share_server)
                     progress = result.get('total_progress', '0')
                     self.db.share_backup_update(
                         context, backup_id, {'restore_progress': progress})

manila/tests/api/v2/test_share_backups.py

@@ -15,6 +15,7 @@ from oslo_config import cfg
 from unittest import mock
 from webob import exc
 
+from manila.api.openstack import api_version_request as api_version
 from manila.api.v2 import share_backups
 from manila.common import constants
 from manila import context
@@ -65,7 +66,9 @@ class ShareBackupsApiTest(test.TestCase):
 
         return backup, req
 
-    def _get_fake_backup(self, admin=False, summary=False, **values):
+    def _get_fake_backup(self, admin=False, summary=False,
+                         apiversion=share_backups.MIN_SUPPORTED_API_VERSION,
+                         **values):
         backup = fake_share.fake_backup(**values)
         backup['updated_at'] = '2016-06-12T19:57:56.506805'
         expected_keys = {'id', 'share_id', 'status'}
@@ -86,6 +89,11 @@ class ShareBackupsApiTest(test.TestCase):
                 'progress': backup.get('progress'),
                 'restore_progress': backup.get('restore_progress'),
             })
+            if self.is_microversion_ge(apiversion, '2.85'):
+                expected_backup.update({
+                    'backup_type': backup.get('backup_type'),
+                })
+
             if admin:
                 expected_backup.update({
                     'host': backup.get('host'),
@@ -189,16 +197,19 @@ class ShareBackupsApiTest(test.TestCase):
         self.mock_policy_check.assert_called_once_with(
             self.member_context, self.resource_name, 'get_all')
 
-    def test_list_share_backups_detail(self):
+    @ddt.data(share_backups.MIN_SUPPORTED_API_VERSION,
-        fake_backup, expected_backup = self._get_fake_backup()
+              api_version._MAX_API_VERSION)
-
+    def test_list_share_backups_detail(self, apiversion):
+        fake_backup, expected_backup = self._get_fake_backup(
+            apiversion=apiversion,
+        )
         self.mock_object(share.API, 'get',
                          mock.Mock(return_value={'id': 'FAKE_SHAREID'}))
         self.mock_object(share_backups.db, 'share_backups_get_all',
                          mock.Mock(return_value=[fake_backup]))
         req = fakes.HTTPRequest.blank(
             '/share-backups?share_id=FAKE_SHARE_ID',
-            version=self.api_version, experimental=True)
+            version=apiversion, experimental=True)
         req.environ['manila.context'] = (
             self.member_context)
         req_context = req.environ['manila.context']

manila/tests/share/drivers/dummy.py

@@ -1020,30 +1020,35 @@ class DummyDriver(driver.ShareDriver):
         }
 
     @slow_me_down
-    def create_backup(self, context, share_instance, backup):
+    def create_backup(self, context, share_instance, backup,
+                      share_server=None):
         LOG.debug("Created backup %(backup)s of share %(share)s "
                   "using dummy driver.",
                   {'backup': backup['id'],
                    'share': share_instance['share_id']})
 
-    def create_backup_continue(self, context, share_instance, backup):
+    def create_backup_continue(self, context, share_instance, backup,
+                               share_server=None):
         LOG.debug("Continue backup %(backup)s of share %(share)s "
                   "using dummy driver.",
                   {'backup': backup['id'],
                    'share': share_instance['share_id']})
         return {'total_progress': '100'}
 
-    def delete_backup(self, context, backup):
+    def delete_backup(self, context, backup, share_instance,
+                      share_server=None):
         LOG.debug("Deleted backup '%s' using dummy driver.", backup['id'])
 
     @slow_me_down
-    def restore_backup(self, context, backup, share_instance):
+    def restore_backup(self, context, backup, share_instance,
+                       share_server=None):
         LOG.debug("Restored backup %(backup)s into share %(share)s "
                   "using dummy driver.",
                   {'backup': backup['id'],
                    'share': share_instance['share_id']})
 
-    def restore_backup_continue(self, context, share_instance, backup):
+    def restore_backup_continue(self, context, backup, share_instance,
+                                share_server=None):
         LOG.debug("Continue restore of backup %(backup)s into share "
                   "%(share)s using dummy driver.",
                   {'backup': backup['id'],

manila/tests/share/test_api.py

@@ -7247,7 +7247,11 @@ class ShareAPITestCase(test.TestCase):
         self.mock_object(data_rpc.DataAPI, 'create_backup', mock.Mock())
         self.mock_object(self.share_rpcapi, 'create_backup', mock.Mock())
 
-        backup = {'display_name': 'tmp_backup', 'backup_options': backup_opts}
+        backup = {
+            'display_name': 'tmp_backup',
+            'backup_options': backup_opts,
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
         self.api.create_share_backup(self.context, share, backup)
 
         quota.QUOTAS.reserve.assert_called_once()
@@ -7263,8 +7267,10 @@ class ShareAPITestCase(test.TestCase):
 
     def test_create_share_backup_share_error_state(self):
         share = db_utils.create_share(is_public=True, status='error')
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
-
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
         self.assertRaises(exception.InvalidShare,
                           self.api.create_share_backup,
                           self.context, share, backup)
@@ -7272,7 +7278,10 @@ class ShareAPITestCase(test.TestCase):
     def test_create_share_backup_share_busy_task_state(self):
         share = db_utils.create_share(
             is_public=True, task_state='data_copying_in_progress')
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
 
         self.assertRaises(exception.ShareBusyException,
                           self.api.create_share_backup,
@@ -7284,7 +7293,10 @@ class ShareAPITestCase(test.TestCase):
         snapshot = db_utils.create_snapshot(
             share_id=share['id'], status='available', size=1)
 
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
 
         self.mock_object(db_api, 'share_snapshot_get_all_for_share',
                          mock.Mock(return_value=[snapshot]))
@@ -7298,7 +7310,10 @@ class ShareAPITestCase(test.TestCase):
                                  has_replicas=True,
                                  status=constants.STATUS_AVAILABLE,
                                  is_soft_deleted=False)
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
 
         self.assertRaises(exception.InvalidShare,
                           self.api.create_share_backup,
@@ -7314,7 +7329,10 @@ class ShareAPITestCase(test.TestCase):
         share = fakes.fake_share(id='fake_id',
                                  status=constants.STATUS_AVAILABLE,
                                  is_soft_deleted=False, size=5)
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
 
         usages = {'backup_gigabytes': {'reserved': 5, 'in_use': 5},
                   'backups': {'reserved': 5, 'in_use': 5}}
@@ -7342,7 +7360,10 @@ class ShareAPITestCase(test.TestCase):
         self.mock_object(data_rpc.DataAPI, 'create_backup', mock.Mock())
         self.mock_object(self.share_rpcapi, 'create_backup', mock.Mock())
 
-        backup = {'display_name': 'tmp_backup'}
+        backup = {
+            'display_name': 'tmp_backup',
+            'id': 'dbe28dff-ce7d-4c3d-a795-c31f6fee31ab',
+        }
         self.assertRaises(exception.ManilaException,
                           self.api.create_share_backup,
                           self.context, share, backup)

releasenotes/notes/share-backup-netapp-driver-core-6a2328756b14f541.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Driver-advantaged share backups can now have a "backup_type".
+    It is also possible to create share backups in environments
+    that support hard multi-tenancy (driver_handles_share_servers=True).