From da23b652fb408c641535c0b3123bbc8362290686 Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Wed, 7 Dec 2022 17:13:50 +0000 Subject: [PATCH] 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 --- .../dataontap/client/client_cmode_rest.py | 36 +++++++++++-- .../dataontap/cluster_mode/data_motion.py | 22 ++++++++ .../netapp/dataontap/cluster_mode/lib_base.py | 50 +++++++++++++++++-- .../drivers/netapp/dataontap/client/fakes.py | 8 ++- .../client/test_client_cmode_rest.py | 37 ++++++++++++-- .../cluster_mode/test_data_motion.py | 1 + .../dataontap/cluster_mode/test_lib_base.py | 19 +++++++ ...snapmirror-schedules-b565d4163663ffa0.yaml | 10 ++++ 8 files changed, 172 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/bug-1996859-update-timedelta-and-old-snapmirror-schedules-b565d4163663ffa0.yaml 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 c393ef12e6..b09f10e4fa 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py +++ b/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, 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 95cc69932c..519cf63a06 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py +++ b/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 = ( 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 c64b6a5bfe..7e67303ac1 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/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 - # one hour. + # Recovery Point Objective (RPO) indicates the point in time to + # 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'], diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index 78ec71e698..c22bfe4ab8 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/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 = { 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 b37ce73487..2d4ccf8953 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 @@ -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', diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_data_motion.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_data_motion.py index 2164e06c00..c83721262a 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_data_motion.py +++ b/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', 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 e8495a1414..8f184cdfbc 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 @@ -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()) } diff --git a/releasenotes/notes/bug-1996859-update-timedelta-and-old-snapmirror-schedules-b565d4163663ffa0.yaml b/releasenotes/notes/bug-1996859-update-timedelta-and-old-snapmirror-schedules-b565d4163663ffa0.yaml new file mode 100644 index 0000000000..783b43d68d --- /dev/null +++ b/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 `_