Merge "NetApp ONTAP: Fix SnapMirror snapshots not being cleaned up" into stable/xena

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

@@ -838,3 +838,33 @@ class DataMotionSession(object):
             msg = _("Unable to release the snapmirror from source volume %s. "
                     "Retries exhausted. Aborting") % src_volume_name
             raise exception.NetAppException(message=msg)
+
+    def cleanup_previous_snapmirror_relationships(self, replica, replica_list):
+        """Cleanup previous snapmirrors relationships for replica."""
+        LOG.debug("Cleaning up old snapmirror relationships for replica %s.",
+                  replica['id'])
+        src_vol_name, src_vserver, src_backend = (
+            self.get_backend_info_for_share(replica))
+        src_client = get_client_for_backend(src_backend,
+                                            vserver_name=src_vserver)
+
+        # replica_list may contain the replica we are trying to clean up
+        destinations = (r for r in replica_list if r['id'] != replica['id'])
+
+        for destination in destinations:
+            dest_vol_name, dest_vserver, _ = (
+                self.get_backend_info_for_share(destination))
+            try:
+                src_client.release_snapmirror_vol(
+                    src_vserver, src_vol_name, dest_vserver, dest_vol_name)
+            except netapp_api.NaApiError as e:
+                if (e.code == netapp_api.EOBJECTNOTFOUND or
+                        e.code == netapp_api.ESOURCE_IS_DIFFERENT or
+                        "(entry doesn't exist)" in e.message):
+                    LOG.debug(
+                        'Snapmirror destination %s no longer exists for '
+                        'replica %s.', destination['id'], replica['id'])
+                else:
+                    LOG.exception(
+                        'Error releasing snapmirror destination %s for '
+                        'replica %s.', destination['id'], replica['id'])

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

@@ -2656,6 +2656,15 @@ class NetAppCmodeFileStorageLibrary(object):
                                                        share_name)):
                 return constants.REPLICA_STATE_OUT_OF_SYNC
 
+        # NOTE(sfernand): When promoting replicas, the previous source volume
+        # and its destinations are put in an 'out of sync' state and must be
+        # cleaned up once to avoid retaining unused snapshots from the previous
+        # relationship. Replicas already 'in-sync' won't try another cleanup
+        # attempt.
+        if replica['replica_state'] == constants.REPLICA_STATE_OUT_OF_SYNC:
+            dm_session.cleanup_previous_snapmirror_relationships(
+                replica, replica_list)
+
         return constants.REPLICA_STATE_IN_SYNC
 
     def promote_replica(self, context, replica_list, replica, access_rules,

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

@@ -1179,3 +1179,57 @@ class NetAppCDOTDataMotionSessionTestCase(test.TestCase):
         src_mock_client.release_snapmirror_vol.assert_called_once_with(
             fake.VSERVER1, fake.SHARE_NAME, fake.VSERVER2, fake.SHARE_NAME2,
             relationship_info_only=False)
+
+    @ddt.data([{'id': 'src_share'}, {'id': 'dst_share'}],
+              [{'id': 'dst_share'}])
+    def test_cleanup_previous_snapmirror_relationships(self, replica_list):
+        mock_src_client = mock.Mock()
+        src_backend_info = ('src_share', 'src_vserver', 'src_backend')
+        dst_backend_info = ('dst_share', 'dst_vserver', 'dst_backend')
+        self.mock_object(self.dm_session, 'get_backend_info_for_share',
+                         mock.Mock(side_effect=[src_backend_info,
+                                                dst_backend_info]))
+        self.mock_object(data_motion, 'get_client_for_backend',
+                         mock.Mock(return_value=mock_src_client))
+        self.mock_object(mock_src_client, 'release_snapmirror_vol')
+
+        result = self.dm_session.cleanup_previous_snapmirror_relationships(
+            {'id': 'src_share'}, replica_list)
+
+        data_motion.get_client_for_backend.assert_called_once_with(
+            'src_backend', vserver_name='src_vserver')
+        self.dm_session.get_backend_info_for_share.assert_has_calls([
+            mock.call({'id': 'src_share'}),
+            mock.call({'id': 'dst_share'})
+        ])
+        mock_src_client.release_snapmirror_vol.assert_called_once_with(
+            'src_vserver', 'src_share', 'dst_vserver', 'dst_share')
+
+        self.assertIsNone(result)
+
+    @ddt.data(netapp_api.NaApiError(),
+              netapp_api.NaApiError(code=netapp_api.EOBJECTNOTFOUND),
+              netapp_api.NaApiError(code=netapp_api.ESOURCE_IS_DIFFERENT),
+              netapp_api.NaApiError(code='some_random_code',
+                                    message="(entry doesn't exist)"),
+              netapp_api.NaApiError(code='some_random_code',
+                                    message='(actually, entry does exist!)'))
+    def test_cleanup_previous_snapmirror_relationships_does_not_exist(
+            self, release_exception):
+        mock_src_client = mock.Mock()
+        self.mock_object(self.dm_session, 'get_backend_info_for_share',
+                         mock.Mock(return_value=(
+                             mock.Mock(), mock.Mock(), mock.Mock())))
+        self.mock_object(data_motion, 'get_client_for_backend',
+                         mock.Mock(return_value=mock_src_client))
+        self.mock_object(mock_src_client, 'release_snapmirror_vol',
+                         mock.Mock(side_effect=release_exception))
+
+        replica = {'id': 'src_share'}
+        replica_list = [replica, {'id': 'dst_share'}]
+
+        result = self.dm_session.cleanup_previous_snapmirror_relationships(
+            replica, replica_list)
+
+        mock_src_client.release_snapmirror_vol.assert_called()
+        self.assertIsNone(result)

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

@@ -4307,6 +4307,46 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
                                                    fake.SHARE, None, [],
                                                    share_server=None)
 
+        (self.mock_dm_session.cleanup_previous_snapmirror_relationships
+         .assert_not_called())
+        self.assertEqual(constants.REPLICA_STATE_IN_SYNC, result)
+
+    def test_update_replica_state_replica_change_to_in_sycn(self):
+        fake_snapmirror = {
+            'mirror-state': 'snapmirrored',
+            'relationship-status': 'idle',
+            'last-transfer-end-timestamp': '%s' % float(time.time())
+        }
+        # fake SHARE has replica_state set to active already
+        active_replica = fake.SHARE
+        out_of_sync_replica = copy.deepcopy(fake.SHARE)
+        out_of_sync_replica['replica_state'] = (
+            constants.REPLICA_STATE_OUT_OF_SYNC)
+        replica_list = [out_of_sync_replica, active_replica]
+        vserver_client = mock.Mock()
+        self.mock_object(vserver_client, 'volume_exists',
+                         mock.Mock(return_value=True))
+        self.mock_object(self.library,
+                         '_get_vserver',
+                         mock.Mock(return_value=(fake.VSERVER1,
+                                                 vserver_client)))
+        self.mock_dm_session.get_snapmirrors = mock.Mock(
+            return_value=[fake_snapmirror])
+        mock_config = mock.Mock()
+        mock_config.safe_get = mock.Mock(return_value=0)
+        self.mock_object(data_motion, 'get_backend_configuration',
+                         mock.Mock(return_value=mock_config))
+        self.mock_object(self.library,
+                         '_is_readable_replica',
+                         mock.Mock(return_value=False))
+
+        result = self.library.update_replica_state(
+            None, replica_list, out_of_sync_replica,
+            None, [], share_server=None)
+
+        # Expect a snapmirror cleanup as replica was in out of sync state
+        (self.mock_dm_session.cleanup_previous_snapmirror_relationships
+         .assert_called_once_with(out_of_sync_replica, replica_list))
         self.assertEqual(constants.REPLICA_STATE_IN_SYNC, result)
 
     def test_update_replica_state_backend_volume_absent(self):
@@ -4350,6 +4390,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
                                                    fake.SHARE, None, snapshots,
                                                    share_server=None)
 
+        (self.mock_dm_session.cleanup_previous_snapmirror_relationships
+         .assert_not_called())
         self.assertEqual(constants.REPLICA_STATE_IN_SYNC, result)
 
     def test_update_replica_state_missing_snapshot(self):

releasenotes/notes/bug-1982808-netapp-fix-snapmirror-snapshots-not-cleaned-up-63cc98cd468adbd1.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    NetApp driver `bug #1982808
+    <https://bugs.launchpad.net/manila/+bug/1982808>`_: Fixed issue
+    preventing the storage system from proper clean up unused SnapMirror
+    snapshots after a replica promote, significantly increasing the amount
+    of space consumed in ONTAP volumes by snapshots.
+