Update timedelta and old schedules as per netapp_snapmirror_schedule

Asynchronous SnapMirror schedules are set using netapp config option
'netapp_snapmirror_schedule'. The delta for determining replica is
in-sync or out-of-sync updated to twice the schedule time seconds.
Also, not only new snapmirrors, but also old ones should
have a schedule according to the current
'netapp_snapmirror_schedule' config.

Closes-bug: #1996859
Depends-On: I0390f82dfdc130d49e3af6928996dd730e3cf69f
Change-Id: Ifbe0575f6c359929344763666e4d93d8c6084e83

manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py

@@ -2192,7 +2192,8 @@ class NetAppRestClient(object):
 
         fields = ['state', 'source.svm.name', 'source.path',
                   'destination.svm.name', 'destination.path',
-                  'transfer.end_time', 'uuid', 'policy.type']
+                  'transfer.end_time', 'uuid', 'policy.type',
+                  'transfer_schedule.name']
 
         query = {}
         query['fields'] = ','.join(fields)
@@ -2223,6 +2224,7 @@ class NetAppRestClient(object):
             snapmirrors.append({
                 'relationship-status': record.get('state'),
                 'mirror-state': record.get('state'),
+                'schedule': record['transfer_schedule']['name'],
                 'source-vserver': record['source']['svm']['name'],
                 'source-volume': (record['source']['path'].split(':')[1] if
                                   record.get('source') else None),
@@ -2959,7 +2961,7 @@ class NetAppRestClient(object):
     def _set_snapmirror_state(self, state, source_path, destination_path,
                               source_vserver, source_volume,
                               destination_vserver, destination_volume,
-                              wait_result=True):
+                              wait_result=True, schedule=None):
         """Change the snapmirror state between two volumes."""
 
         snapmirror = self.get_snapmirrors(source_path, destination_path,
@@ -2978,7 +2980,12 @@ class NetAppRestClient(object):
             raise na_utils.NetAppDriverException(msg)
 
         uuid = snapmirror[0]['uuid']
-        body = {'state': state}
+        body = {}
+        if state:
+            body.update({'state': state})
+        if schedule:
+            body.update({"transfer_schedule": {'name': schedule}})
+
         result = self.send_request(f'/snapmirror/relationships/{uuid}',
                                    'patch', body=body,
                                    wait_on_accepted=wait_result)
@@ -3023,6 +3030,29 @@ class NetAppRestClient(object):
             source_vserver, source_volume,
             dest_vserver, dest_volume, wait_result=False)
 
+    @na_utils.trace
+    def modify_snapmirror_vol(self, source_vserver, source_volume,
+                              dest_vserver, dest_volume,
+                              schedule=None, policy=None, tries=None,
+                              max_transfer_rate=None):
+        """Modifies a SnapMirror relationship between volumes."""
+        return self._modify_snapmirror(
+            source_vserver=source_vserver, dest_vserver=dest_vserver,
+            source_volume=source_volume, dest_volume=dest_volume,
+            schedule=schedule)
+
+    @na_utils.trace
+    def _modify_snapmirror(self, source_path=None, dest_path=None,
+                           source_vserver=None, dest_vserver=None,
+                           source_volume=None, dest_volume=None,
+                           schedule=None):
+        """Modifies a SnapMirror relationship."""
+        return self._set_snapmirror_state(
+            None, source_path, dest_path,
+            source_vserver, source_volume,
+            dest_vserver, dest_volume, wait_result=False,
+            schedule=schedule)
+
     @na_utils.trace
     def create_volume_clone(self, volume_name, parent_volume_name,
                             parent_snapshot_name=None, split=False,

manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py

@@ -175,6 +175,7 @@ class DataMotionSession(object):
             source_volume=src_volume_name, dest_volume=dest_volume_name,
             desired_attributes=['relationship-status',
                                 'mirror-state',
+                                'schedule',
                                 'source-vserver',
                                 'source-volume',
                                 'last-transfer-end-timestamp',
@@ -422,6 +423,27 @@ class DataMotionSession(object):
                                           dest_vserver,
                                           dest_volume_name)
 
+    def modify_snapmirror(self, source_share_obj, dest_share_obj,
+                          schedule=None):
+        """Modify SnapMirror relationship: set schedule"""
+        dest_volume_name, dest_vserver, dest_backend = (
+            self.get_backend_info_for_share(dest_share_obj))
+        dest_client = get_client_for_backend(dest_backend,
+                                             vserver_name=dest_vserver)
+
+        src_volume_name, src_vserver, __ = self.get_backend_info_for_share(
+            source_share_obj)
+
+        if schedule is None:
+            config = get_backend_configuration(dest_backend)
+            schedule = config.netapp_snapmirror_schedule
+
+        dest_client.modify_snapmirror_vol(src_vserver,
+                                          src_volume_name,
+                                          dest_vserver,
+                                          dest_volume_name,
+                                          schedule=schedule)
+
     def resume_snapmirror(self, source_share_obj, dest_share_obj):
         """Resume SnapMirror relationship from a quiesced state."""
         dest_volume_name, dest_vserver, dest_backend = (

manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py

@@ -23,6 +23,7 @@ import copy
 import datetime
 import json
 import math
+import re
 import socket
 
 from oslo_config import cfg
@@ -172,6 +173,8 @@ class NetAppCmodeFileStorageLibrary(object):
         self._backend_name = self.configuration.safe_get(
             'share_backend_name') or driver_name
         self.message_api = message_api.API()
+        self._snapmirror_schedule = self._convert_schedule_to_seconds(
+            schedule=self.configuration.netapp_snapmirror_schedule)
 
     @na_utils.trace
     def do_setup(self, context):
@@ -2552,6 +2555,35 @@ class NetAppCmodeFileStorageLibrary(object):
         self._delete_share(replica, vserver, vserver_client,
                            remove_export=is_readable, remove_qos=is_readable)
 
+    @na_utils.trace
+    def _convert_schedule_to_seconds(self, schedule='hourly'):
+        """Convert snapmirror schedule to seconds."""
+        results = re.findall(r'[\d]+|[^d]+', schedule)
+        if not results or len(results) > 2:
+            return 3600  # default (1 hour)
+
+        if len(results) == 2:
+            try:
+                num = int(results[0])
+            except ValueError:
+                return 3600
+            schedule = results[1]
+        else:
+            num = 1
+            schedule = results[0]
+        schedule = schedule.lower()
+        if schedule in ['min', 'minute']:
+            return (num * 60)
+        if schedule in ['hour', 'hourly']:
+            return (num * 3600)
+        if schedule in ['day', 'daily']:
+            return (num * 24 * 3600)
+        if schedule in ['week', 'weekly']:
+            return (num * 7 * 24 * 3600)
+        if schedule in ['month', 'monthly']:
+            return (num * 30 * 24 * 2600)
+        return 3600
+
     def update_replica_state(self, context, replica_list, replica,
                              access_rules, share_snapshots, share_server=None,
                              replication=True):
@@ -2624,14 +2656,26 @@ class NetAppCmodeFileStorageLibrary(object):
                 LOG.exception("Could not resync snapmirror.")
                 return constants.STATUS_ERROR
 
+        current_schedule = snapmirror.get('schedule')
+        new_schedule = self.configuration.netapp_snapmirror_schedule
+        if current_schedule != new_schedule:
+            dm_session.modify_snapmirror(active_replica, replica,
+                                         schedule=new_schedule)
+            LOG.debug('Modify snapmirror schedule for replica:'
+                      '%(replica)s from %(from)s to %(to)s',
+                      {'replica': replica['id'],
+                       'from': current_schedule,
+                       'to': new_schedule})
+
         last_update_timestamp = float(
             snapmirror.get('last-transfer-end-timestamp', 0))
-        # TODO(ameade): Have a configurable RPO for replicas, for now it is
+        # Recovery Point Objective (RPO) indicates the point in time to
-        # one hour.
+        # which data can be recovered. The RPO target is typically less
+        # than twice the replication schedule.
         if (last_update_timestamp and
             (timeutils.is_older_than(
                 datetime.datetime.utcfromtimestamp(last_update_timestamp)
-                .isoformat(), 3600))):
+                .isoformat(), (2 * self._snapmirror_schedule)))):
             return constants.REPLICA_STATE_OUT_OF_SYNC
 
         replica_backend = share_utils.extract_host(replica['host'],

manila/tests/share/drivers/netapp/dataontap/client/fakes.py

@@ -3936,7 +3936,10 @@ SNAPMIRROR_GET_ITER_RESPONSE_REST = {
                 "type": "async"
             },
             "state": "snapmirrored",
-            "healthy": True
+            "healthy": True,
+            "transfer_schedule": {
+                "name": "hourly",
+            },
         }
     ],
     "num_records": 1,
@@ -3951,7 +3954,8 @@ REST_GET_SNAPMIRRORS_RESPONSE = [{
     'source-volume': SM_SOURCE_VOLUME,
     'source-vserver': SM_SOURCE_VSERVER,
     'uuid': FAKE_UUID,
-    'policy-type': 'async'
+    'policy-type': 'async',
+    'schedule': 'hourly'
 }]
 
 FAKE_CIFS_RECORDS = {

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py

@@ -2185,7 +2185,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase):
             'destination.path': (fake.SM_DEST_VSERVER +
                                  ':' + fake.SM_DEST_VOLUME),
             'fields': 'state,source.svm.name,source.path,destination.svm.name,'
-                      'destination.path,transfer.end_time,uuid,policy.type'
+                      'destination.path,transfer.end_time,uuid,policy.type,'
+                      'transfer_schedule.name'
         }
 
         mock_send_request.assert_called_once_with('/snapmirror/relationships',
@@ -2216,7 +2217,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase):
             'destination.path': (fake.SM_DEST_VSERVER +
                                  ':' + fake.SM_DEST_VOLUME),
             'fields': 'state,source.svm.name,source.path,destination.svm.name,'
-                      'destination.path,transfer.end_time,uuid,policy.type'
+                      'destination.path,transfer.end_time,uuid,policy.type,'
+                      'transfer_schedule.name'
         }
 
         mock_send_request.assert_called_once_with('/snapmirror/relationships',
@@ -2243,7 +2245,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase):
             'destination.path': (fake.SM_DEST_VSERVER +
                                  ':' + fake.SM_DEST_VOLUME),
             'fields': 'state,source.svm.name,source.path,destination.svm.name,'
-                      'destination.path,transfer.end_time,uuid,policy.type'
+                      'destination.path,transfer.end_time,uuid,policy.type,'
+                      'transfer_schedule.name'
         }
 
         mock_send_request.assert_called_once_with('/snapmirror/relationships',
@@ -2944,6 +2947,34 @@ class NetAppRestCmodeClientTestCase(test.TestCase):
 
         self.assertEqual(expected_job, result)
 
+    def test_modify_snapmirror_vol(self):
+
+        expected_job = {
+            'operation-id': None,
+            'status': None,
+            'jobid': fake.FAKE_UUID,
+            'error-code': None,
+            'error-message': None,
+        }
+
+        mock_set_snapmirror_state = self.mock_object(
+            self.client,
+            '_set_snapmirror_state',
+            mock.Mock(return_value=expected_job))
+
+        result = self.client.modify_snapmirror_vol(
+            fake.SM_SOURCE_VSERVER, fake.SM_SOURCE_VOLUME,
+            fake.SM_DEST_VSERVER, fake.SM_DEST_VOLUME,
+            None)
+
+        mock_set_snapmirror_state.assert_called_once_with(
+            None, None, None,
+            fake.SM_SOURCE_VSERVER, fake.SM_SOURCE_VOLUME,
+            fake.SM_DEST_VSERVER, fake.SM_DEST_VOLUME,
+            wait_result=False, schedule=None)
+
+        self.assertEqual(expected_job, result)
+
     def test__abort_snapmirror(self):
         return_snp = fake.REST_GET_SNAPMIRRORS_RESPONSE
         mock_get_snap = self.mock_object(self.client, '_get_snapmirrors',

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_data_motion.py

@@ -799,6 +799,7 @@ class NetAppCDOTDataMotionSessionTestCase(test.TestCase):
             dest_volume=self.fake_dest_vol_name,
             desired_attributes=['relationship-status',
                                 'mirror-state',
+                                'schedule',
                                 'source-vserver',
                                 'source-volume',
                                 'last-transfer-end-timestamp',

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py

@@ -3991,6 +3991,19 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         data_motion.get_client_for_backend.assert_called_once_with(
             fake.BACKEND_NAME, vserver_name=fake.VSERVER1)
 
+    @ddt.data({'seconds': 3600, 'schedule': 'hourly'},
+              {'seconds': (5 * 3600), 'schedule': '5hourly'},
+              {'seconds': (30 * 60), 'schedule': '30minute'},
+              {'seconds': (2 * 24 * 3600), 'schedule': '2DAY'},
+              {'seconds': 3600, 'schedule': 'fake_shedule'},
+              {'seconds': 3600, 'schedule': 'fake2'},
+              {'seconds': 3600, 'schedule': '10fake'})
+    @ddt.unpack
+    def test__convert_schedule_to_seconds(self, seconds, schedule):
+        expected_return = seconds
+        actual_return = self.library._convert_schedule_to_seconds(schedule)
+        self.assertEqual(expected_return, actual_return)
+
     def test_update_replica_state_no_snapmirror_share_creating(self):
         vserver_client = mock.Mock()
         self.mock_object(vserver_client, 'volume_exists',
@@ -4209,6 +4222,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
     def test_update_replica_state_stale_snapmirror(self):
         fake_snapmirror = {
             'mirror-state': 'snapmirrored',
+            'schedule': self.library.configuration.netapp_snapmirror_schedule,
             'last-transfer-end-timestamp': '%s' % float(
                 timeutils.utcnow_ts() - 10000)
         }
@@ -4224,6 +4238,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_object(self.library,
                          '_is_readable_replica',
                          mock.Mock(return_value=False))
+        mock_backend_config = fake.get_config_cmode()
+        self.mock_object(data_motion, 'get_backend_configuration',
+                         mock.Mock(return_value=mock_backend_config))
 
         result = self.library.update_replica_state(None, [fake.SHARE],
                                                    fake.SHARE, None, [],
@@ -4234,6 +4251,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
     def test_update_replica_state_in_sync(self):
         fake_snapmirror = {
             'mirror-state': 'snapmirrored',
+            'schedule': self.library.configuration.netapp_snapmirror_schedule,
             'relationship-status': 'idle',
             'last-transfer-end-timestamp': '%s' % float(time.time())
         }
@@ -4276,6 +4294,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
     def test_update_replica_state_in_sync_with_snapshots(self):
         fake_snapmirror = {
             'mirror-state': 'snapmirrored',
+            'schedule': self.library.configuration.netapp_snapmirror_schedule,
             'relationship-status': 'idle',
             'last-transfer-end-timestamp': '%s' % float(time.time())
         }

releasenotes/notes/bug-1996859-update-timedelta-and-old-snapmirror-schedules-b565d4163663ffa0.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    NetApp ONTAP driver fixed to consider timestamp delta calculated from
+    `netapp_snapmirror_schedule` config option instead of fixed one hour
+    value. Delta is calculated as twice the time of the option. Also, ensure
+    periodically that existent snapmirrors have the schedule property
+    according to the `netapp_snapmirror_schedule` configuration value. For
+    more details, please refer
+    `Launchpad bug #1996859 <https://bugs.launchpad.net/manila/+bug/1996859>`_