Merge "Fix backup creation errors with NetApp driver"

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

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:
+    if backup_type not in config_stanzas:
-        msg = _("Could not find backend stanza %(backup_name)s in "
+        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
 

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(
+        try:
-            backend_name)
+            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."
+            msg = _("Source path '%(path)s' has more than one relationships."
-                        " To create the share backup, delete the all source"
+                    " To create the share backup, delete the all source"
-                        " volume's SnapMirror relationships using 'snapmirror'"
+                    " volume's SnapMirror relationships using 'snapmirror'"
-                        " ONTAP CLI or System Manger.")
+                    " 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:"
+                        " failed between source '%(source_vserver)s:"
-                        "%(source_volume)s and destination %(des_vserver)s:"
+                        "%(source_volume)s' and destination '%(des_vserver)s:"
-                        "%(des_volume)s  for share id %(share_id)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:

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(

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'

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 = {

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',

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'}

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
+    <https://bugs.launchpad.net/manila/+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.