From 50cb08a522cc57dabecd8a9925ff860ab5184c85 Mon Sep 17 00:00:00 2001
From: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com>
Date: Thu, 21 Mar 2024 14:58:21 +0530
Subject: [PATCH] Adding more assertions to ITs and removing unintended changes

Signed-off-by: Shourya Dutta Biswas <114977491+shourya035@users.noreply.github.com>
---
 .../DocrepToRemoteDualReplicationIT.java      | 15 +++++++++++++-
 .../index/remote/RemoteStoreUtils.java        | 20 -------------------
 .../RetentionLeaseBackgroundSyncAction.java   |  3 ---
 .../index/seqno/RetentionLeaseSyncAction.java |  3 ---
 .../checkpoint/PublishCheckpointAction.java   |  2 --
 5 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/DocrepToRemoteDualReplicationIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/DocrepToRemoteDualReplicationIT.java
index df400809a190f..251025faa4e13 100644
--- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/DocrepToRemoteDualReplicationIT.java
+++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/DocrepToRemoteDualReplicationIT.java
@@ -197,6 +197,17 @@ public void testRemotePrimaryDocRepAndRemoteReplica() throws Exception {
         assertReplicaAndPrimaryConsistencyMultiCopy(shardStatsMap, firstBatch, secondBatch, nodes);
     }
 
+    /*
+    Scenario:
+    - Starts 1 docrep backed data node
+    - Creates an index with 0 replica
+    - Starts 1 remote backed data node
+    - Move primary copy from docrep to remote through _cluster/reroute
+    - Expands index to 1 replica
+    - Stops remote enabled node
+    - Ensure doc count is same after failover
+    - Index some more docs to ensure working of failed-over primary
+     */
     public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
         internalCluster().startClusterManagerOnlyNode();
@@ -268,6 +279,7 @@ public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
         long initialPrimaryDocCount = 0;
         for (ShardRouting shardRouting : shardStatsMap.keySet()) {
             if (shardRouting.primary()) {
+                assertTrue(nodes.get(shardRouting.currentNodeId()).isRemoteStoreNode());
                 initialPrimaryDocCount = shardStatsMap.get(shardRouting).getStats().getDocs().getCount();
             }
         }
@@ -279,10 +291,11 @@ public void testFailoverRemotePrimaryToDocrepReplica() throws Exception {
         ensureYellow(FAILOVER_REMOTE_TO_DOCREP);
 
         shardStatsMap = internalCluster().client().admin().indices().prepareStats(FAILOVER_REMOTE_TO_DOCREP).setDocs(true).get().asMap();
-        DiscoveryNodes discoveryNodes = internalCluster().client().admin().cluster().prepareState().get().getState().getNodes();
+        nodes = internalCluster().client().admin().cluster().prepareState().get().getState().getNodes();
         long primaryDocCountAfterFailover = 0;
         for (ShardRouting shardRouting : shardStatsMap.keySet()) {
             if (shardRouting.primary()) {
+                assertFalse(nodes.get(shardRouting.currentNodeId()).isRemoteStoreNode());
                 primaryDocCountAfterFailover = shardStatsMap.get(shardRouting).getStats().getDocs().getCount();
             }
         }
diff --git a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
index 38ececd33608d..6738b90d87222 100644
--- a/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
+++ b/server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
@@ -8,9 +8,7 @@
 
 package org.opensearch.index.remote;
 
-import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.collect.Tuple;
-import org.opensearch.node.remotestore.RemoteStoreNodeService;
 
 import java.util.Arrays;
 import java.util.HashMap;
@@ -18,9 +16,6 @@
 import java.util.Map;
 import java.util.function.Function;
 
-import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
-import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
-
 /**
  * Utils for remote store
  *
@@ -105,19 +100,4 @@ public static void verifyNoMultipleWriters(List<String> mdFiles, Function<String
             }
         });
     }
-
-    /**
-     * Helper method to check the values for the following cluster settings:
-     * - `remote_store.compatibility_mode` (should be `mixed`)
-     * - `migration.direction` (should NOT be `none`)
-     * Used as a source of truth to confirm if a remote store migration is in progress
-     * @param clusterService Current clusterService ref to fetch cluster settings
-     */
-    public static boolean isMigrationDirectionSet(ClusterService clusterService) {
-        RemoteStoreNodeService.Direction migrationDirection = clusterService.getClusterSettings().get(MIGRATION_DIRECTION_SETTING);
-        RemoteStoreNodeService.CompatibilityMode currentCompatiblityMode = clusterService.getClusterSettings()
-            .get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING);
-        return currentCompatiblityMode.equals(RemoteStoreNodeService.CompatibilityMode.MIXED) == true
-            && migrationDirection.equals(RemoteStoreNodeService.Direction.NONE) == false;
-    }
 }
diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java
index ead38f524715e..5fa0a1a6459e7 100644
--- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java
+++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncAction.java
@@ -88,8 +88,6 @@ protected Logger getLogger() {
         return LOGGER;
     }
 
-    private final ClusterService clusterService;
-
     @Inject
     public RetentionLeaseBackgroundSyncAction(
         final Settings settings,
@@ -113,7 +111,6 @@ public RetentionLeaseBackgroundSyncAction(
             Request::new,
             ThreadPool.Names.MANAGEMENT
         );
-        this.clusterService = clusterService;
     }
 
     @Override
diff --git a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java
index 0b1a468b1effd..ca3c7e1d49700 100644
--- a/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java
+++ b/server/src/main/java/org/opensearch/index/seqno/RetentionLeaseSyncAction.java
@@ -90,8 +90,6 @@ protected Logger getLogger() {
         return LOGGER;
     }
 
-    private final ClusterService clusterService;
-
     @Inject
     public RetentionLeaseSyncAction(
         final Settings settings,
@@ -122,7 +120,6 @@ public RetentionLeaseSyncAction(
             systemIndices,
             tracer
         );
-        this.clusterService = clusterService;
     }
 
     @Override
diff --git a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java
index cda8e307eec43..a6bb768d1cf1e 100644
--- a/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java
+++ b/server/src/main/java/org/opensearch/indices/replication/checkpoint/PublishCheckpointAction.java
@@ -58,7 +58,6 @@ public class PublishCheckpointAction extends TransportReplicationAction<
     protected static Logger logger = LogManager.getLogger(PublishCheckpointAction.class);
 
     private final SegmentReplicationTargetService replicationService;
-    private final ClusterService clusterService;
 
     @Inject
     public PublishCheckpointAction(
@@ -85,7 +84,6 @@ public PublishCheckpointAction(
             ThreadPool.Names.REFRESH
         );
         this.replicationService = targetService;
-        this.clusterService = clusterService;
     }
 
     @Override