NetApp ONTAP: Fixed errors on failover-host operation with REST API

Fixed the functions _set_snapmirror_state and break_snapmirror that
were causing a failure in the failover-host operation.

Change-Id: Ib4ebe3cafb67a16aef4b961ccf30dade99fdec7f
Closes-Bug: #2028857

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

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,

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):

releasenotes/notes/bug-2028857-fix-netapp-replica-failover-error-a9cad94ae56af8d0.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    NetApp ONTAP driver `bug #2028857
+    <https://bugs.launchpad.net/cinder/+bug/2028857>`_: Fixed
+    errors that were occuring in the replica failover operation when using
+    ONTAP REST API.