diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/fakes.py index 0e3e9c73314..5b535de104e 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/fakes.py @@ -2865,6 +2865,7 @@ SNAPMIRROR_GET_ITER_RESPONSE_REST = { "type": "async" }, "state": "snapmirrored", + "transfer": {"state": "success"}, "healthy": True } ], @@ -2957,10 +2958,11 @@ REST_GET_SNAPMIRRORS_RESPONSE = [{ 'lag-time': None, 'last-transfer-end-timestamp': None, 'mirror-state': 'snapmirrored', - 'relationship-status': 'snapmirrored', + 'relationship-status': 'idle', 'source-volume': SM_SOURCE_VOLUME, 'source-vserver': SM_SOURCE_VSERVER, 'uuid': FAKE_UUID, + 'transferring-state': 'success', }] TRANSFERS_GET_ITER_REST = { diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode_rest.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode_rest.py index d0a3c3fbef0..b72cda4ea0e 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode_rest.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode_rest.py @@ -2737,8 +2737,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): 'destination.path': (fake_client.SM_DEST_VSERVER + ':' + fake_client.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' - 'destination.path,transfer.end_time,lag_time,healthy,' - 'uuid' + 'destination.path,transfer.state,transfer.end_time,' + 'lag_time,healthy,uuid' } mock_send_request.assert_called_once_with('/snapmirror/relationships', @@ -2762,8 +2762,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): 'destination.path': (fake_client.SM_DEST_VSERVER + ':' + fake_client.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' - 'destination.path,transfer.end_time,lag_time,healthy,' - 'uuid' + 'destination.path,transfer.state,transfer.end_time,' + 'lag_time,healthy,uuid' } mock_send_request.assert_called_once_with('/snapmirror/relationships', @@ -2789,8 +2789,8 @@ class NetAppRestCmodeClientTestCase(test.TestCase): 'destination.path': (fake_client.SM_DEST_VSERVER + ':' + fake_client.SM_DEST_VOLUME), 'fields': 'state,source.svm.name,source.path,destination.svm.name,' - 'destination.path,transfer.end_time,lag_time,healthy,' - 'uuid' + 'destination.path,transfer.state,transfer.end_time,' + 'lag_time,healthy,uuid' } mock_send_request.assert_called_once_with('/snapmirror/relationships', @@ -3233,37 +3233,23 @@ class NetAppRestCmodeClientTestCase(test.TestCase): self.assertEqual(expected_job, result) def test_break_snapmirror(self): - snapmirror_response = copy.deepcopy( - fake_client.SNAPMIRROR_GET_ITER_RESPONSE_REST) + fake_snapmirror = fake_client.REST_GET_SNAPMIRRORS_RESPONSE + fake_uuid = fake_snapmirror[0]['uuid'] + fake_body = {'state': 'broken_off'} - snapmirror_response['state'] = 'broken_off' - response_list = [snapmirror_response] + self.mock_object(self.client, 'send_request') - self.mock_object(self.client, 'send_request', - side_effect=copy.deepcopy(response_list)) - - expected_job = { - 'operation-id': None, - 'status': None, - 'jobid': fake_client.FAKE_UUID, - 'error-code': None, - 'error-message': None, - 'relationship-uuid': fake_client.FAKE_UUID, - } - - mock_set_snapmirror_state = self.mock_object( - self.client, - '_set_snapmirror_state', - return_value=expected_job) + mock_get_snap = self.mock_object( + self.client, '_get_snapmirrors', + mock.Mock(return_value=fake_snapmirror)) self.client.break_snapmirror( fake_client.SM_SOURCE_VSERVER, fake_client.SM_SOURCE_VOLUME, fake_client.SM_DEST_VSERVER, fake_client.SM_DEST_VOLUME) - mock_set_snapmirror_state.assert_called_once_with( - 'broken-off', - fake_client.SM_SOURCE_VSERVER, fake_client.SM_SOURCE_VOLUME, - fake_client.SM_DEST_VSERVER, fake_client.SM_DEST_VOLUME) + mock_get_snap.assert_called_once() + self.client.send_request.assert_called_once_with( + f'/snapmirror/relationships/{fake_uuid}', 'patch', body=fake_body) def test_break_snapmirror_not_found(self): self.mock_object( @@ -3278,13 +3264,29 @@ class NetAppRestCmodeClientTestCase(test.TestCase): fake_client.SM_DEST_VSERVER, fake_client.SM_DEST_VOLUME) - def test_break_snapmirror_timeout(self): - # when a timeout happens, an exception is thrown by send_request - api_error = netapp_api.NaRetryableError() + def test__break_snapmirror_error(self): + fake_snapmirror = fake_client.REST_GET_SNAPMIRRORS_RESPONSE + self.mock_object(self.client, '_get_snapmirrors', + return_value=fake_snapmirror) self.mock_object(self.client, 'send_request', - side_effect=api_error) + side_effect=self._mock_api_error()) + self.assertRaises(netapp_api.NaApiError, + self.client.break_snapmirror, + fake_client.SM_SOURCE_VSERVER, + fake_client.SM_SOURCE_VOLUME, + fake_client.SM_DEST_VSERVER, + fake_client.SM_DEST_VOLUME) - self.assertRaises(netapp_api.NaRetryableError, + def test__break_snapmirror_exception(self): + fake_snapmirror = copy.deepcopy( + fake_client.REST_GET_SNAPMIRRORS_RESPONSE) + fake_snapmirror[0]['transferring-state'] = 'error' + + self.mock_object( + self.client, '_get_snapmirrors', + mock.Mock(return_value=fake_snapmirror)) + + self.assertRaises(netapp_utils.NetAppDriverException, self.client.break_snapmirror, fake_client.SM_SOURCE_VSERVER, fake_client.SM_SOURCE_VOLUME, diff --git a/cinder/volume/drivers/netapp/dataontap/client/client_cmode_rest.py b/cinder/volume/drivers/netapp/dataontap/client/client_cmode_rest.py index 5d92e45fc7c..db5a2371ea9 100644 --- a/cinder/volume/drivers/netapp/dataontap/client/client_cmode_rest.py +++ b/cinder/volume/drivers/netapp/dataontap/client/client_cmode_rest.py @@ -1956,7 +1956,7 @@ class RestClient(object): destination_vserver=None, destination_volume=None): fields = ['state', 'source.svm.name', 'source.path', - 'destination.svm.name', 'destination.path', + 'destination.svm.name', 'destination.path', 'transfer.state', 'transfer.end_time', 'lag_time', 'healthy', 'uuid'] query = {} @@ -1976,7 +1976,11 @@ class RestClient(object): snapmirrors = [] for record in response.get('records', []): snapmirrors.append({ - 'relationship-status': record.get('state'), + 'relationship-status': ( + 'idle' + if record.get('state') == 'snapmirrored' + else record.get('state')), + 'transferring-state': record.get('transfer', {}).get('state'), 'mirror-state': record['state'], 'source-vserver': record['source']['svm']['name'], 'source-volume': (record['source']['path'].split(':')[1] if @@ -2070,11 +2074,11 @@ class RestClient(object): result = self.send_request('/snapmirror/relationships/' + uuid, 'patch', body=body, wait_on_accepted=wait_result) - job = result['job'] + job_info = { 'operation-id': None, 'status': None, - 'jobid': job.get('uuid'), + 'jobid': result.get('job', {}).get('uuid'), 'error-code': None, 'error-message': None, 'relationship-uuid': uuid, @@ -2247,9 +2251,39 @@ class RestClient(object): destination_vserver, destination_volume): """Breaks a data protection SnapMirror relationship.""" - self._set_snapmirror_state( - 'broken-off', source_vserver, source_volume, - destination_vserver, destination_volume) + interval = 2 + retries = (10 / interval) + + @utils.retry(netapp_api.NaRetryableError, interval=interval, + retries=retries, backoff_rate=1) + def _waiter(): + snapmirror = self.get_snapmirrors( + source_vserver=source_vserver, + source_volume=source_volume, + destination_vserver=destination_vserver, + destination_volume=destination_volume) + + snapmirror_state = None + if snapmirror: + snapmirror_state = snapmirror[0].get('transferring-state') + + if snapmirror_state == 'success': + uuid = snapmirror[0]['uuid'] + body = {'state': 'broken_off'} + self.send_request(f'/snapmirror/relationships/{uuid}', 'patch', + body=body) + return + else: + message = 'Waiting for transfer state to be SUCCESS.' + code = '' + raise netapp_api.NaRetryableError(message=message, code=code) + + try: + return _waiter() + except netapp_api.NaRetryableError: + msg = _("Transfer state did not reach the expected state. Retries " + "exhausted. Aborting.") + raise na_utils.NetAppDriverException(msg) def update_snapmirror(self, source_vserver, source_volume, destination_vserver, destination_volume): diff --git a/releasenotes/notes/bug-2028857-fix-netapp-replica-failover-error-a9cad94ae56af8d0.yaml b/releasenotes/notes/bug-2028857-fix-netapp-replica-failover-error-a9cad94ae56af8d0.yaml new file mode 100644 index 00000000000..bf9abe68630 --- /dev/null +++ b/releasenotes/notes/bug-2028857-fix-netapp-replica-failover-error-a9cad94ae56af8d0.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NetApp ONTAP driver `bug #2028857 + `_: Fixed + errors that were occuring in the replica failover operation when using + ONTAP REST API.