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 e72bd07f12..56fdb3f33b 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/data_motion.py @@ -1169,6 +1169,58 @@ def release_snapmirror(): "Retries exhausted. Aborting") % src_volume_name raise exception.NetAppException(message=msg) + def has_leftover_source_snapmirrors(self, replica, replica_list): + """Check for stale relationships where replica is still the source. + + After a replica is promoted, the previously active replica is demoted + and becomes a SnapMirror destination of the new source. Its old + outgoing relationships (demoted_replica -> other replicas) should have + been released during promotion, but that release is best-effort and is + skipped when the demoted replica's host is unreachable at promote time + (see delete_snapmirror). Once the host is reachable again, such a + relationship lingers as a broken-off orphan on this source endpoint. + + Returns True if this replica still has a source-side relationship + pointing at another replica in replica_list, i.e. a leftover from when + it was the active source that must be cleaned up. + """ + src_vol_name, src_vserver, src_backend = ( + self.get_backend_info_for_share(replica)) + try: + src_client = get_client_for_backend(src_backend, + vserver_name=src_vserver) + except Exception: + # Source host still unreachable; nothing we can do this cycle. + LOG.debug('Could not reach source backend %(backend)s to check ' + 'for leftover snapmirror relationships for replica ' + '%(replica)s.', + {'backend': src_backend, 'replica': replica['id']}) + return False + + # Only relationships whose destination is another known replica are + # considered ours to clean up (known pairings only). + expected_dest_volumes = { + self.get_backend_info_for_share(r)[0] + for r in replica_list if r['id'] != replica['id']} + + try: + destinations = src_client.get_snapmirror_destinations( + source_vserver=src_vserver, source_volume=src_vol_name) + except netapp_api.NaApiError: + LOG.exception('Error listing snapmirror destinations for replica ' + '%s.', replica['id']) + return False + + for destination in destinations: + if destination.get('destination-volume') in expected_dest_volumes: + LOG.debug('Found leftover source snapmirror relationship for ' + 'replica %(replica)s to destination volume ' + '%(vol)s.', + {'replica': replica['id'], + 'vol': destination.get('destination-volume')}) + return True + return False + 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.", 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 85c84369ba..bf586a16e3 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -3941,6 +3941,15 @@ def update_replica_state(self, context, replica_list, replica, if replica['replica_state'] == constants.REPLICA_STATE_OUT_OF_SYNC: dm_session.cleanup_previous_snapmirror_relationships( replica, replica_list) + elif dm_session.has_leftover_source_snapmirrors(replica, replica_list): + # NOTE(mescher): A replica that reached 'in-sync' can still carry a + # broken-off relationship left from when it was the active source, + # if the release was skipped during promotion because its host was + # unreachable at that time (see delete_snapmirror). Now that the + # host is reachable, release the leftover so it does not linger as + # an orphan on the backend. This does not affect the replica_state. + dm_session.cleanup_previous_snapmirror_relationships( + replica, replica_list) return constants.REPLICA_STATE_IN_SYNC 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 d6f3f85d83..eaad8b713e 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 @@ -1511,6 +1511,86 @@ def test_cleanup_previous_snapmirror_relationships_does_not_exist( mock_src_client.release_snapmirror_vol.assert_called() self.assertIsNone(result) + def test_has_leftover_source_snapmirrors_found(self): + 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, 'get_snapmirror_destinations', + mock.Mock(return_value=[{'destination-volume': 'dst_share'}])) + + replica = {'id': 'src_share'} + replica_list = [replica, {'id': 'dst_share'}] + + result = self.dm_session.has_leftover_source_snapmirrors( + replica, replica_list) + + mock_src_client.get_snapmirror_destinations.assert_called_once_with( + source_vserver='src_vserver', source_volume='src_share') + self.assertTrue(result) + + def test_has_leftover_source_snapmirrors_no_known_destination(self): + 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)) + # Destination volume does not belong to any known replica. + self.mock_object( + mock_src_client, 'get_snapmirror_destinations', + mock.Mock(return_value=[{'destination-volume': 'unknown_vol'}])) + + replica = {'id': 'src_share'} + replica_list = [replica, {'id': 'dst_share'}] + + result = self.dm_session.has_leftover_source_snapmirrors( + replica, replica_list) + + self.assertFalse(result) + + def test_has_leftover_source_snapmirrors_source_unreachable(self): + src_backend_info = ('src_share', 'src_vserver', 'src_backend') + self.mock_object(self.dm_session, 'get_backend_info_for_share', + mock.Mock(return_value=src_backend_info)) + self.mock_object(data_motion, 'get_client_for_backend', + mock.Mock(side_effect=Exception('unreachable'))) + + replica = {'id': 'src_share'} + replica_list = [replica, {'id': 'dst_share'}] + + result = self.dm_session.has_leftover_source_snapmirrors( + replica, replica_list) + + self.assertFalse(result) + + def test_has_leftover_source_snapmirrors_list_error(self): + 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, 'get_snapmirror_destinations', + mock.Mock(side_effect=netapp_api.NaApiError())) + + replica = {'id': 'src_share'} + replica_list = [replica, {'id': 'dst_share'}] + + result = self.dm_session.has_leftover_source_snapmirrors( + replica, replica_list) + + self.assertFalse(result) + def test_get_most_available_aggr_of_vserver(self): vserver_client = mock.Mock() aggr_space_attr = {fake.AGGREGATE: {'available': 5678}, 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 8fd5109353..311ed6b3f7 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 @@ -5104,6 +5104,8 @@ def test_update_replica_state_in_sync(self): self.mock_object(self.mock_dm_session, 'get_policy_from_share_replica_metadata', mock.Mock(return_value=('MirrorAllSnapshots', False))) + self.mock_dm_session.has_leftover_source_snapmirrors = mock.Mock( + return_value=False) result = self.library.update_replica_state(None, [fake.SHARE], fake.SHARE, None, [], @@ -5113,6 +5115,53 @@ def test_update_replica_state_in_sync(self): .assert_not_called()) self.assertEqual(constants.REPLICA_STATE_IN_SYNC, result) + def test_update_replica_state_in_sync_with_leftover_snapmirror(self): + # A replica already 'in-sync' that still has a leftover source-side + # relationship from when it was the active source must trigger a + # one-time cleanup, without changing the reported replica_state. + fake_snapmirror = { + 'mirror-state': 'snapmirrored', + 'schedule': self.library.configuration.netapp_snapmirror_schedule, + 'source-vserver': 'fake_source_vserver', + 'source-volume': 'fake_source_volume', + 'policy-type': 'async', + 'relationship-status': 'idle', + 'last-transfer-end-timestamp': '%s' % float(time.time()) + } + active_replica = fake.SHARE + in_sync_replica = copy.deepcopy(fake.SHARE) + in_sync_replica['replica_state'] = constants.REPLICA_STATE_IN_SYNC + replica_list = [in_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]) + 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)) + self.mock_object(self.mock_dm_session, + 'get_policy_from_share_replica_metadata', + mock.Mock(return_value=('MirrorAllSnapshots', False))) + self.mock_dm_session.has_leftover_source_snapmirrors = mock.Mock( + return_value=True) + + result = self.library.update_replica_state( + None, replica_list, in_sync_replica, None, [], share_server=None) + + (self.mock_dm_session.has_leftover_source_snapmirrors + .assert_called_once_with(in_sync_replica, replica_list)) + (self.mock_dm_session.cleanup_previous_snapmirror_relationships + .assert_called_once_with(in_sync_replica, replica_list)) + self.assertEqual(constants.REPLICA_STATE_IN_SYNC, result) + def test_update_replica_state_replica_change_to_in_sync(self): fake_snapmirror = { 'mirror-state': 'snapmirrored', @@ -5240,6 +5289,8 @@ def test_update_replica_state_in_sync_with_snapshots(self): mock_backend_config = fake.get_config_cmode() self.mock_object(data_motion, 'get_backend_configuration', mock.Mock(return_value=mock_backend_config)) + self.mock_dm_session.has_leftover_source_snapmirrors = mock.Mock( + return_value=False) result = self.library.update_replica_state(None, [fake.SHARE], fake.SHARE, None, snapshots,