From 6909a7c213eb3f453bb8de0410da71b0c32d84d2 Mon Sep 17 00:00:00 2001 From: agireesh Date: Thu, 29 Feb 2024 17:05:43 +0530 Subject: [PATCH] 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 | 3 +- .../openstack/rest_api_version_history.rst | 4 ++ manila/api/views/share_backups.py | 9 ++++ manila/common/constants.py | 1 + .../2f27d904214c_add_backup_type_column.py | 49 +++++++++++++++++++ manila/db/sqlalchemy/models.py | 1 + manila/share/api.py | 30 +++++++++++- manila/share/driver.py | 19 +++++-- manila/share/manager.py | 25 +++++++--- manila/tests/api/v2/test_share_backups.py | 21 ++++++-- manila/tests/share/drivers/dummy.py | 15 ++++-- manila/tests/share/test_api.py | 37 +++++++++++--- ...p-netapp-driver-core-6a2328756b14f541.yaml | 6 +++ 13 files changed, 188 insertions(+), 32 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/2f27d904214c_add_backup_type_column.py create mode 100644 releasenotes/notes/share-backup-netapp-driver-core-6a2328756b14f541.yaml diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 7507624b25..0fbcd5e9f4 100644 --- a/manila/api/openstack/api_version_request.py +++ b/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 diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 777ac37b2b..d1023ae879 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/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. diff --git a/manila/api/views/share_backups.py b/manila/api/views/share_backups.py index 12e5d5f629..c4ebcdd555 100644 --- a/manila/api/views/share_backups.py +++ b/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') diff --git a/manila/common/constants.py b/manila/common/constants.py index 45a750a091..d92601d3a7 100644 --- a/manila/common/constants.py +++ b/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, diff --git a/manila/db/migrations/alembic/versions/2f27d904214c_add_backup_type_column.py b/manila/db/migrations/alembic/versions/2f27d904214c_add_backup_type_column.py new file mode 100644 index 0000000000..12f009c037 --- /dev/null +++ b/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 diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 4e3bf66d2c..d418368138 100644 --- a/manila/db/sqlalchemy/models.py +++ b/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) diff --git a/manila/share/api.py b/manila/share/api.py index bce1fbeb56..6540f7079f 100644 --- a/manila/share/api.py +++ b/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) diff --git a/manila/share/driver.py b/manila/share/driver.py index 91574a8828..bf27a2934c 100644 --- a/manila/share/driver.py +++ b/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() diff --git a/manila/share/manager.py b/manila/share/manager.py index d98b3929bd..4fdabe9f94 100644 --- a/manila/share/manager.py +++ b/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}) diff --git a/manila/tests/api/v2/test_share_backups.py b/manila/tests/api/v2/test_share_backups.py index 6e2bfefeb9..d493d0eadc 100644 --- a/manila/tests/api/v2/test_share_backups.py +++ b/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): - fake_backup, expected_backup = self._get_fake_backup() - + @ddt.data(share_backups.MIN_SUPPORTED_API_VERSION, + 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'] diff --git a/manila/tests/share/drivers/dummy.py b/manila/tests/share/drivers/dummy.py index bec6094c00..afd451bf26 100644 --- a/manila/tests/share/drivers/dummy.py +++ b/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'], diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 25c6800176..59dfea25d5 100644 --- a/manila/tests/share/test_api.py +++ b/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) diff --git a/releasenotes/notes/share-backup-netapp-driver-core-6a2328756b14f541.yaml b/releasenotes/notes/share-backup-netapp-driver-core-6a2328756b14f541.yaml new file mode 100644 index 0000000000..a9cac4a5ae --- /dev/null +++ b/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).