diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py b/manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py index fe37089d4d..f83b227ed0 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py @@ -2315,7 +2315,9 @@ class NetAppRestClient(object): fields = ['state', 'source.svm.name', 'source.path', 'destination.svm.name', 'destination.path', 'transfer.end_time', 'uuid', 'policy.type', - 'transfer_schedule.name', 'transfer.state'] + 'transfer_schedule.name', 'transfer.state', + 'last_transfer_type', 'transfer.bytes_transferred', + 'healthy'] query = {} query['fields'] = ','.join(fields) @@ -2365,7 +2367,14 @@ class NetAppRestClient(object): (self._parse_timestamp(record['transfer']['end_time']) if record.get('transfer', {}).get('end_time') else 0), 'uuid': record['uuid'], - 'policy-type': record.get('policy', {}).get('type') + 'policy-type': record.get('policy', {}).get('type'), + 'is-healthy': ( + 'true' + if record.get('healthy', {}) is True else 'false'), + 'last-transfer-type': record.get('last_transfer_type', None), + 'last-transfer-size': record.get('transfer', + {}).get('bytes_transferred'), + }) return snapmirrors diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py b/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py index 058da55a47..01cabe36ee 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py @@ -73,20 +73,20 @@ def get_backend_configuration(backend_name): return config -def get_backup_configuration(backup_name): +def get_backup_configuration(backup_type): config_stanzas = CONF.list_all_sections() - if backup_name not in config_stanzas: - msg = _("Could not find backend stanza %(backup_name)s in " + if backup_type not in config_stanzas: + msg = _("Could not find backup_type stanza %(backup_type)s in " "configuration which is required for backup workflows " "with the source share. Available stanzas are " "%(stanzas)s") params = { "stanzas": config_stanzas, - "backend_name": backup_name, + "backup_type": backup_type, } raise exception.BadConfigurationException(reason=msg % params) config = configuration.Configuration(driver.share_opts, - config_group=backup_name) + config_group=backup_type) config.append_config_values(na_opts.netapp_backup_opts) return config diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index e2f66befca..01b18eb286 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -64,9 +64,9 @@ class Backup(Enum): BACKUP_TYPE = "backup_type" BACKEND_NAME = "netapp_backup_backend_section_name" DES_VSERVER = "netapp_backup_vserver" - DES_VOLUME = "netapp_backup_share" + DES_VOLUME = "netapp_backup_volume" SM_LABEL = "backup" - DES_VSERVER_PREFIX = "backup_vserver" + DES_VSERVER_PREFIX = "backup" DES_VOLUME_PREFIX = "backup_volume" VOLUME_TYPE = "dp" SM_POLICY = "os_backup_policy" @@ -4350,13 +4350,24 @@ class NetAppCmodeFileStorageLibrary(object): " from command line or API.") # check the backend is related to NetApp - backup_config = data_motion.get_backup_configuration(backup_type) + try: + backup_config = data_motion.get_backup_configuration(backup_type) + except exception.BadConfigurationException: + msg = _("Could not find backup_type '%(backup_type)s' stanza" + " in config file") % {'backup_type': backup_type} + raise exception.BadConfigurationException(reason=msg) backend_name = backup_config.safe_get(Backup.BACKEND_NAME.value) - backend_config = data_motion.get_backend_configuration( - backend_name) + try: + backend_config = data_motion.get_backend_configuration( + backend_name + ) + except exception.BadConfigurationException: + msg = _("Could not find backend '%(backend_name)s' stanza" + " in config file") % {'backend_name': backend_name} + raise exception.BadConfigurationException(reason=msg) if (backend_config.safe_get("netapp_storage_family") != 'ontap_cluster'): - err_msg = _("Wrong vendor backend %s is provided, provide" + err_msg = _("Wrong vendor backend '%s' is provided, provide" " only NetApp backend.") % backend_name raise exception.BackupException(err_msg) @@ -4393,16 +4404,17 @@ class NetAppCmodeFileStorageLibrary(object): # Get the destination vserver and volume for relationship source_path = f"{src_vserver}:{src_vol}" snapmirror_info = src_vserver_client.get_snapmirror_destinations( - source_path=source_path) + source_path=source_path + ) if len(snapmirror_info) > 1: - err_msg = _("Source path %(path)s has more than one relationships." - " To create the share backup, delete the all source" - " volume's SnapMirror relationships using 'snapmirror'" - " ONTAP CLI or System Manger.") + msg = _("Source path '%(path)s' has more than one relationships." + " To create the share backup, delete the all source" + " volume's SnapMirror relationships using 'snapmirror'" + " ONTAP CLI or System Manager.") msg_args = { 'path': source_path } - raise exception.NetAppException(err_msg % msg_args) + raise exception.NetAppException(msg % msg_args) elif len(snapmirror_info) == 1: des_vserver, des_volume = self._get_destination_vserver_and_vol( src_vserver_client, source_path, False) @@ -4431,7 +4443,8 @@ class NetAppCmodeFileStorageLibrary(object): if share_server: self._delete_backup_vserver(backup, des_vserver) - msg = _("Failed to create a volume in vserver %(des_vserver)s") + msg = _("Failed to create a volume in vserver '%(des_vserver)s" + "'") msg_args = {'des_vserver': des_vserver} raise exception.NetAppException(msg % msg_args) @@ -4451,8 +4464,7 @@ class NetAppCmodeFileStorageLibrary(object): Backup.SM_LABEL.value) ] if len(snap_list_with_backup) == 1: - self.is_volume_backup_before = True - + self._set_volume_has_backup_before(True) policy_name = f"{Backup.SM_POLICY.value}_{share_instance['id']}" try: des_vserver_client.create_snapmirror_policy( @@ -4490,9 +4502,9 @@ class NetAppCmodeFileStorageLibrary(object): des_volume, share_server=share_server) msg = _("SnapVault relationship creation or initialization" - " failed between source %(source_vserver)s:" - "%(source_volume)s and destination %(des_vserver)s:" - "%(des_volume)s for share id %(share_id)s.") + " failed between source '%(source_vserver)s:" + "%(source_volume)s' and destination '%(des_vserver)s:" + "%(des_volume)s' for share id %(share_id)s.") msg_args = { 'source_vserver': src_vserver, @@ -4564,7 +4576,7 @@ class NetAppCmodeFileStorageLibrary(object): } msg = _("There is no SnapMirror relationship available for" " source path '%(source_path)s' and destination path" - " '%(des_path)s' yet.") % msg_args + " '%(des_path)s' yet.") LOG.warning(msg, msg_args) return progress_status LOG.debug("SnapMirror details %s:", snapmirror_info) @@ -4582,6 +4594,22 @@ class NetAppCmodeFileStorageLibrary(object): {'progress_status': progress_status}) return progress_status + if snapmirror_info[0].get("is-healthy") == 'false': + self._resource_cleanup_for_backup(backup, + share_instance, + des_vserver, + des_vol, + share_server=share_server) + msg_args = { + 'source_path': source_path, + 'des_path': des_path, + } + msg = _("There is an issue with SnapMirror relationship with" + " source path '%(source_path)s' and destination path" + " '%(des_path)s'. Make sure destination volume is or was " + "not part of any other SnapMirror relationship.") + raise exception.NetAppException(message=msg % msg_args) + # Verify that snapshot is transferred to destination volume snap_name = self._get_backup_snapshot_name(backup, share_instance['id']) @@ -4589,7 +4617,7 @@ class NetAppCmodeFileStorageLibrary(object): des_vol, snap_name) LOG.debug("Snapshot '%(snap_name)s' transferred successfully to" - " destination", {'snap_name': snap_name}) + " destination.", {'snap_name': snap_name}) # previously if volume was part of some relationship and if we delete # all the backup of share then last snapshot will be left on # destination volume, and we can't delete that snapshot due to ONTAP @@ -4611,11 +4639,11 @@ class NetAppCmodeFileStorageLibrary(object): snap_to_delete = snap_list_with_backup[1] else: snap_to_delete = snap_list_with_backup[0] - self.is_volume_backup_before = False + self._set_volume_has_backup_before(False) des_vserver_client.delete_snapshot(des_vol, snap_to_delete, True) LOG.debug("Previous snapshot %{snap_name}s deleted" - " successfully. ", {'snap_name': snap_to_delete}) + " successfully.", {'snap_name': snap_to_delete}) return progress_status @na_utils.trace @@ -4676,7 +4704,7 @@ class NetAppCmodeFileStorageLibrary(object): "total_progress": Backup.TOTAL_PROGRESS_ZERO.value } return progress_status - LOG.debug("SnapMirror relationship of type RST is deleted") + LOG.debug("SnapMirror relationship of type RST is deleted.") snap_name = self._get_backup_snapshot_name(backup, share_instance['id']) snapshot_list = src_vserver_client.list_volume_snapshots(src_vol_name) @@ -4699,7 +4727,7 @@ class NetAppCmodeFileStorageLibrary(object): share_server=share_server, ) except exception.VserverNotFound: - LOG.warning("Vserver associated with share %s was not found.", + LOG.warning("Vserver associated with share '%s' was not found.", share_instance['id']) return src_vol_name = self._get_backend_share_name(share_instance['id']) @@ -4762,7 +4790,7 @@ class NetAppCmodeFileStorageLibrary(object): des_vserver_client.get_snapshot(des_vol, snap_name) is_snapshot_deleted = self._is_snapshot_deleted(False) except (SnapshotResourceNotFound, netapp_api.NaApiError): - LOG.debug("Snapshot %s deleted successfully.", snap_name) + LOG.debug("Snapshot '%s' deleted successfully.", snap_name) if not is_snapshot_deleted: err_msg = _("Snapshot '%(snapshot_name)s' is not deleted" " successfully on ONTAP." @@ -4770,6 +4798,10 @@ class NetAppCmodeFileStorageLibrary(object): LOG.exception(err_msg) raise exception.NetAppException(err_msg) + @na_utils.trace + def _set_volume_has_backup_before(self, value): + self.is_volume_backup_before = value + @na_utils.trace def _is_snapshot_deleted(self, is_deleted): return is_deleted @@ -4839,14 +4871,15 @@ class NetAppCmodeFileStorageLibrary(object): des_vserver_client) if not des_aggr: msg = _("Not able to find any aggregate from ONTAP" - " to create the volume") + " to create the volume. Make sure aggregates are" + " added to destination vserver") raise exception.NetAppException(msg) src_vol = self._get_backend_share_name(share_instance['id']) vol_attr = src_vserver_client.get_volume(src_vol) source_vol_size = vol_attr.get('size') vol_size_in_gb = int(source_vol_size) / units.Gi share_id = share_instance['id'].replace('-', '_') - des_volume = f"backup_volume_{share_id}" + des_volume = f"{Backup.DES_VOLUME_PREFIX.value}_{share_id}" des_vserver_client.create_volume(des_aggr, des_volume, vol_size_in_gb, volume_type='dp') return des_volume @@ -4860,7 +4893,7 @@ class NetAppCmodeFileStorageLibrary(object): snapmirror_info = src_vserver_client.get_snapmirror_destinations( source_path=source_path) if validate_relation and len(snapmirror_info) != 1: - msg = _("There are more then one relationship with the source." + msg = _("There are more then one relationship with the source" " '%(source_path)s'." % {'source_path': source_path}) raise exception.NetAppException(msg) if len(snapmirror_info) == 1: diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index e4ddeee4e4..4ee9c8b795 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -21,6 +21,7 @@ as needed to provision shares. """ import re +from manila.share.drivers.netapp.dataontap.cluster_mode.lib_base import Backup from oslo_log import log from oslo_serialization import jsonutils from oslo_utils import excutils @@ -2393,7 +2394,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary( ] aggr_list = aggr_matching_list share_server_id = share_server['id'] - des_vserver = f"backup_{share_server_id}" + des_vserver = f"{Backup.DES_VSERVER_PREFIX.value}_{share_server_id}" LOG.debug("Creating vserver %s:", des_vserver) try: des_cluster_api_client.create_vserver( diff --git a/manila/share/drivers/netapp/options.py b/manila/share/drivers/netapp/options.py index 18710e9557..deadcb4995 100644 --- a/manila/share/drivers/netapp/options.py +++ b/manila/share/drivers/netapp/options.py @@ -309,7 +309,7 @@ netapp_backup_opts = [ 'are enabled, create separate config sections for each ' 'backup type specifying the "netapp_backup_vserver", ' '"netapp_backup_backend_section_name", ' - '"netapp_backup_share", and ' + '"netapp_backup_volume", and ' '"netapp_snapmirror_job_timeout" as appropriate.' ' Example- netapp_enabled_backup_types = eng_backup,' ' finance_backup'), @@ -322,7 +322,7 @@ netapp_backup_opts = [ help='vserver name of backend that is use for backup the share.' ' When user provide vserver value then backup volume will ' ' be created under this vserver '), - cfg.StrOpt('netapp_backup_share', + cfg.StrOpt('netapp_backup_volume', default='', help='Specify backup share name in case user wanted to backup ' 'the share. Some case user has dedicated volume for backup' diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index 05bc2b5332..1a3613f5c2 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py @@ -3994,8 +3994,10 @@ SNAPMIRROR_GET_ITER_RESPONSE_REST = { "name": "hourly", }, "transfer": { - "state": "success" - } + "state": "success", + "bytes_transferred": "3352", + }, + "last_transfer_type": "update", } ], "num_records": 1, @@ -4012,7 +4014,10 @@ REST_GET_SNAPMIRRORS_RESPONSE = [{ 'uuid': FAKE_UUID, 'policy-type': 'async', 'schedule': 'hourly', - 'transferring-state': 'success' + 'transferring-state': 'success', + 'is-healthy': 'true', + 'last-transfer-size': '3352', + 'last-transfer-type': 'update', }] FAKE_CIFS_RECORDS = { diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py index e69b27bb64..b37a6cb538 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py @@ -2220,7 +2220,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): ':' + fake.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' 'destination.path,transfer.end_time,uuid,policy.type,' - 'transfer_schedule.name,transfer.state' + 'transfer_schedule.name,transfer.state,' + 'last_transfer_type,transfer.bytes_transferred,healthy' } mock_send_request.assert_called_once_with('/snapmirror/relationships', @@ -2252,7 +2253,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): ':' + fake.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' 'destination.path,transfer.end_time,uuid,policy.type,' - 'transfer_schedule.name,transfer.state' + 'transfer_schedule.name,transfer.state,' + 'last_transfer_type,transfer.bytes_transferred,healthy' } mock_send_request.assert_called_once_with('/snapmirror/relationships', @@ -2280,7 +2282,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): ':' + fake.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' 'destination.path,transfer.end_time,uuid,policy.type,' - 'transfer_schedule.name,transfer.state' + 'transfer_schedule.name,transfer.state,' + 'last_transfer_type,transfer.bytes_transferred,healthy' } mock_send_request.assert_called_once_with('/snapmirror/relationships', diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 3793e84695..67d11cbce1 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -81,7 +81,7 @@ def _get_config(): CONF.set_override("netapp_backup_vserver", "fake_backup_share", group=backup_config) - CONF.set_override("netapp_backup_share", + CONF.set_override("netapp_backup_volume", "fake_share_server", group=backup_config) return config @@ -8023,7 +8023,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): backup_config = 'backup_config' fake_config = configuration.Configuration(driver.share_opts, config_group=backup_config) - CONF.set_override("netapp_backup_share", "", + CONF.set_override("netapp_backup_volume", "", group=backup_config) CONF.set_override("netapp_backup_vserver", "", group=backup_config) @@ -8045,7 +8045,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): backup_config = 'backup_config' fake_config = configuration.Configuration(driver.share_opts, config_group=backup_config) - CONF.set_override("netapp_backup_share", "", + CONF.set_override("netapp_backup_volume", "", group=backup_config) CONF.set_override("netapp_backup_vserver", "", group=backup_config) @@ -8240,7 +8240,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self._backup_mock_common_method(mock_des_vserver_client) backup_config = 'backup_config' - CONF.set_override("netapp_backup_share", "", + CONF.set_override("netapp_backup_volume", "", group=backup_config) CONF.set_override("netapp_backup_vserver", "", group=backup_config) @@ -8469,6 +8469,48 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.SHARE_BACKUP, ) + def test_create_backup_when_invalid_backup_type_negative(self): + mock_src_client = mock.Mock() + self.mock_object(self.library, + '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, + mock_src_client))) + self.mock_object(self.mock_dm_session, + 'get_backup_configuration', + mock.Mock( + side_effect=exception.BadConfigurationException, + )) + self.assertRaises( + exception.BadConfigurationException, + self.library.create_backup, + self.context, + fake.SHARE_INSTANCE, + fake.SHARE_BACKUP, + ) + + def test_create_backup_when_invalid_backend_negative(self): + mock_src_client = mock.Mock() + self.mock_object(self.library, + '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, + mock_src_client))) + self.mock_object(data_motion, + 'get_backup_configuration', + mock.Mock(return_value=_get_config())) + + self.mock_object(self.mock_dm_session, + 'get_backend_configuration', + mock.Mock( + side_effect=exception.BadConfigurationException, + )) + self.assertRaises( + exception.BadConfigurationException, + self.library.create_backup, + self.context, + fake.SHARE_INSTANCE, + fake.SHARE_BACKUP, + ) + def test_create_backup_continue_with_status_inprogress(self): vserver_client = mock.Mock() mock_dest_client = mock.Mock() @@ -8578,6 +8620,60 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.SHARE_BACKUP, ) + def test_create_backup_continue_snapshot_left_from_old_relationship(self): + vserver_client = mock.Mock() + mock_dest_client = mock.Mock() + self._backup_mock_common_method(mock_dest_client) + self.mock_object(self.library, + '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, + vserver_client))) + snapmirror_info = [fake.SNAP_MIRROR_INFO] + self.mock_object(mock_dest_client, + 'get_snapmirrors', + mock.Mock(return_value=snapmirror_info)) + snap_list = ["snap1", "snap2"] + self.mock_object(self.library, + '_get_des_volume_backup_snapshots', + mock.Mock(return_value=snap_list)) + + self.library._set_volume_has_backup_before(True) + share_instance = fake.SHARE_INSTANCE + backup = fake.SHARE_BACKUP + self.library.create_backup_continue(self.context, share_instance, + backup) + self.library._set_volume_has_backup_before(False) + + def test_create_backup_continue_relationship_not_healthy_negative(self): + vserver_client = mock.Mock() + mock_dest_client = mock.Mock() + self._backup_mock_common_method(mock_dest_client) + self.mock_object(self.library, + '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, + vserver_client))) + snapmirror_info = [ + { + 'source-vserver': fake.VSERVER1, + 'source-volume': fake.FLEXVOL_NAME, + 'destination-vserver': fake.VSERVER2, + 'destination-volume': fake.FLEXVOL_NAME_1, + 'relationship-status': "idle", + 'last-transfer-type': "update", + 'is-healthy': "false", + } + ] + self.mock_object(mock_dest_client, + 'get_snapmirrors', + mock.Mock(return_value=snapmirror_info)) + self.assertRaises( + exception.NetAppException, + self.library.create_backup_continue, + self.context, + fake.SHARE_INSTANCE, + fake.SHARE_BACKUP, + ) + def test_restore_backup_with_vserver_volume_none(self): vserver_client = mock.Mock() self.mock_object(self.library, @@ -8727,6 +8823,27 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.SHARE_BACKUP, ) + def test_delete_backup_snapshot_not_exist(self): + vserver_client = mock.Mock() + mock_des_client = mock.Mock() + self._backup_mock_common_method(mock_des_client) + self._backup_mock_common_method_for_negative(vserver_client, + mock_des_client) + self.mock_object(self.library, + '_get_des_volume_backup_snapshots', + mock.Mock(return_value=['fake_snapshot1', + 'fake_snapshot2'])) + self.mock_object(self.library, + '_is_snapshot_deleted', + mock.Mock(return_value=True)) + msg = "entry doesn't exist" + self.mock_object(mock_des_client, + 'delete_snapshot', + mock.Mock(side_effect=netapp_api.NaApiError( + message=msg))) + self.library.delete_backup(self.context, fake.SHARE_BACKUP, + fake.SHARE_INSTANCE) + def test__get_backup_progress_status(self): mock_dest_client = mock.Mock() vol_attr = {'name': 'fake_vol', 'size-used': '123454'} diff --git a/releasenotes/notes/bug-2058027-fix-backup-status-in-creating-state-forever-for-wrong-config-a9e10419f33ecb97.yaml b/releasenotes/notes/bug-2058027-fix-backup-status-in-creating-state-forever-for-wrong-config-a9e10419f33ecb97.yaml new file mode 100644 index 0000000000..783f496ce3 --- /dev/null +++ b/releasenotes/notes/bug-2058027-fix-backup-status-in-creating-state-forever-for-wrong-config-a9e10419f33ecb97.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + NetApp driver `bug #2058027 + `_: + Fix the issue for NetApp driver for backup feature. Backup status + update used to fail when administrator misconfigured backup settings + configuration or SnapMirror relationship created during backup + creation was not in healthy state.