Updating/Fixing tests

Signed-off-by: Dharmesh 💤 <sdharms@amazon.com>

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreBaseIntegTestCase.java

@@ -16,6 +16,7 @@
 import org.opensearch.action.index.IndexResponse;
 import org.opensearch.action.support.WriteRequest;
 import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.cluster.metadata.RepositoriesMetadata;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.util.FeatureFlags;
@@ -25,6 +26,7 @@
 import org.opensearch.indices.replication.common.ReplicationType;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.junit.After;
+import org.junit.Before;
 
 import java.io.IOException;
 import java.nio.file.FileVisitResult;
@@ -36,6 +38,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
@@ -114,13 +117,10 @@ protected boolean addMockInternalEngine() {
 
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
-        if (nodeAttributesSettings == null) {
-            nodeAttributesSettings = remoteStoreNodeAttributes(REPOSITORY_NAME, REPOSITORY_2_NAME);
-        }
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
             .put(remoteStoreClusterSettings(REPOSITORY_NAME, REPOSITORY_2_NAME, true))
-            .put(nodeAttributesSettings)
+            .put(remoteStoreNodeAttributes(REPOSITORY_NAME, REPOSITORY_2_NAME))
             .build();
     }
 
@@ -192,12 +192,15 @@ public static Settings remoteStoreClusterSettings(String segmentRepoName, String
     }
 
     public Settings remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) {
+        if (nodeAttributesSettings != null) {
+            return nodeAttributesSettings;
+        }
         absolutePath = randomRepoPath().toAbsolutePath();
         absolutePath2 = randomRepoPath().toAbsolutePath();
         if (segmentRepoName.equals(translogRepoName)) {
             absolutePath2 = absolutePath;
         }
-        return Settings.builder()
+        nodeAttributesSettings = Settings.builder()
             .put("node.attr." + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, segmentRepoName)
             .put(
                 String.format(Locale.getDefault(), "node.attr." + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT, segmentRepoName),
@@ -219,6 +222,7 @@ public Settings remoteStoreNodeAttributes(String segmentRepoName, String translo
                 absolutePath2.toString()
             )
             .build();
+        return nodeAttributesSettings;
     }
 
     private Settings defaultIndexSettings() {
@@ -260,18 +264,9 @@ protected void putRepository(Path path, String repoName) {
         assertAcked(clusterAdmin().preparePutRepository(repoName).setType("fs").setSettings(Settings.builder().put("location", path)));
     }
 
-    protected void setupRepo() {
-        setupRepo(true);
-    }
-
-    protected void setupRepo(boolean startDedicatedClusterManager) {
-        if (startDedicatedClusterManager) {
-            internalCluster().startClusterManagerOnlyNode();
-        }
-        absolutePath = randomRepoPath().toAbsolutePath();
-        putRepository(absolutePath);
-        absolutePath2 = randomRepoPath().toAbsolutePath();
-        putRepository(absolutePath2, REPOSITORY_2_NAME);
+    @Before
+    public void setup() throws Exception {
+        assertRepositoryMetadataPresentInClusterState();
     }
 
     @After
@@ -299,4 +294,16 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
         return filesExisting.get();
     }
 
+    void assertRepositoryMetadataPresentInClusterState() throws Exception {
+        assertBusy(() -> {
+            RepositoriesMetadata repositoriesMetadata = client().admin()
+                .cluster()
+                .prepareState()
+                .get()
+                .getState()
+                .metadata()
+                .custom(RepositoriesMetadata.TYPE);
+            assertTrue(repositoriesMetadata != null && !repositoriesMetadata.repositories().isEmpty());
+        }, 30, TimeUnit.SECONDS);
+    }
 }

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java

@@ -12,14 +12,16 @@
 import org.opensearch.action.admin.indices.recovery.RecoveryResponse;
 import org.opensearch.action.index.IndexResponse;
 import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.cluster.metadata.RepositoriesMetadata;
+import org.opensearch.cluster.metadata.RepositoryMetadata;
 import org.opensearch.cluster.routing.RecoverySource;
+import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.indices.recovery.RecoveryState;
 import org.opensearch.plugins.Plugin;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.opensearch.test.transport.MockTransportService;
 import org.hamcrest.MatcherAssert;
-import org.junit.Before;
 
 import java.nio.file.Path;
 import java.util.Arrays;
@@ -35,7 +37,7 @@
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.oneOf;
 
-@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 0)
+@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class RemoteStoreIT extends RemoteStoreBaseIntegTestCase {
 
     protected final String INDEX_NAME = "remote-store-test-idx-1";
@@ -45,18 +47,13 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(MockTransportService.TestPlugin.class);
     }
 
-    @Before
-    public void setup() {
-        setupRepo();
-    }
-
     @Override
     public Settings indexSettings() {
         return remoteStoreIndexSettings(0);
     }
 
     private void testPeerRecovery(int numberOfIterations, boolean invokeFlush) throws Exception {
-        internalCluster().startDataOnlyNodes(3);
+        internalCluster().startNodes(3);
         createIndex(INDEX_NAME, remoteStoreIndexSettings(0));
         ensureYellowAndNoInitializingShards(INDEX_NAME);
         ensureGreen(INDEX_NAME);
@@ -116,8 +113,8 @@ public void testPeerRecoveryWithRemoteStoreAndRemoteTranslogRefresh() throws Exc
         testPeerRecovery(randomIntBetween(2, 5), false);
     }
 
-    private void verifyRemoteStoreCleanup() throws Exception {
-        internalCluster().startDataOnlyNodes(3);
+    public void verifyRemoteStoreCleanup() throws Exception {
+        internalCluster().startNodes(3);
         createIndex(INDEX_NAME, remoteStoreIndexSettings(1));
 
         indexData(5, randomBoolean(), INDEX_NAME);
@@ -143,7 +140,7 @@ public void testRemoteTranslogCleanup() throws Exception {
     }
 
     public void testStaleCommitDeletionWithInvokeFlush() throws Exception {
-        internalCluster().startDataOnlyNodes(1);
+        internalCluster().startNode();
         createIndex(INDEX_NAME, remoteStoreIndexSettings(1, 10000l, -1));
         int numberOfIterations = randomIntBetween(5, 15);
         indexData(numberOfIterations, true, INDEX_NAME);
@@ -170,7 +167,7 @@ public void testStaleCommitDeletionWithInvokeFlush() throws Exception {
     }
 
     public void testStaleCommitDeletionWithoutInvokeFlush() throws Exception {
-        internalCluster().startDataOnlyNodes(1);
+        internalCluster().startNode();
         createIndex(INDEX_NAME, remoteStoreIndexSettings(1, 10000l, -1));
         int numberOfIterations = randomIntBetween(5, 15);
         indexData(numberOfIterations, false, INDEX_NAME);

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreRepositoryRegistrationIT.java

@@ -26,7 +26,7 @@
 import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
 import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT;
 
-@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 0)
+@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
 public class RemoteStoreRepositoryRegistrationIT extends RemoteStoreBaseIntegTestCase {
 
     @Override
@@ -51,7 +51,7 @@ private RepositoryMetadata buildRepositoryMetadata(DiscoveryNode node, String na
         return new RepositoryMetadata(name, type, settings.build());
     }
 
-    private void assertRemoteStoreRepositoryOnAllNodes() {
+    private void assertRemoteStoreRepositoryOnAllNodes() throws Exception {
         RepositoriesMetadata repositories = internalCluster().getInstance(ClusterService.class, internalCluster().getNodeNames()[0])
             .state()
             .metadata()
@@ -69,34 +69,19 @@ private void assertRemoteStoreRepositoryOnAllNodes() {
         }
     }
 
-    public void testSingleNodeClusterRepositoryRegistration() {
-        internalCluster().startClusterManagerOnlyNode(remoteStoreNodeAttributes(REPOSITORY_NAME, REPOSITORY_2_NAME));
-        ensureStableCluster(1);
-
+    public void testSingleNodeClusterRepositoryRegistration() throws Exception {
+        internalCluster().startNode();
         assertRemoteStoreRepositoryOnAllNodes();
     }
 
-    public void testMultiNodeClusterRepositoryRegistration() {
-        Settings clusterSettings = remoteStoreNodeAttributes(REPOSITORY_NAME, REPOSITORY_2_NAME);
-        internalCluster().startClusterManagerOnlyNode(clusterSettings);
-        internalCluster().startNodes(3, clusterSettings);
-        ensureStableCluster(4);
-
-        assertRemoteStoreRepositoryOnAllNodes();
-    }
-
-    public void testMultiNodeClusterOnlyDataRepositoryRegistration() {
-        Settings clusterSettings = remoteStoreNodeAttributes(REPOSITORY_NAME, REPOSITORY_2_NAME);
-        internalCluster().startNodes(3, clusterSettings);
-        ensureStableCluster(3);
-
+    public void testMultiNodeClusterRepositoryRegistration() throws Exception {
+        internalCluster().startNodes(3);
         assertRemoteStoreRepositoryOnAllNodes();
     }
 
-    public void testMultiNodeClusterRepositoryRegistrationWithMultipleMasters() {
+    public void testMultiNodeClusterRepositoryRegistrationWithMultipleMasters() throws Exception {
         internalCluster().startClusterManagerOnlyNodes(3);
         internalCluster().startNodes(3);
-        ensureStableCluster(6);
 
         assertRemoteStoreRepositoryOnAllNodes();
     }

server/src/main/java/org/opensearch/action/admin/cluster/remotestore/RemoteStoreService.java

@@ -35,10 +35,10 @@ public class RemoteStoreService {
     private static final Logger logger = LogManager.getLogger(RemoteStoreService.class);
     private final Supplier<RepositoriesService> repositoriesService;
     private final ThreadPool threadPool;
-    public static final Setting<String> REMOTE_STORE_COMPATIBILITY_MODE_SETTING = Setting.simpleString(
+    public static final Setting<CompatibilityMode> REMOTE_STORE_COMPATIBILITY_MODE_SETTING = new Setting<>(
         "remote_store.compatibility_mode",
-        CompatibilityMode.STRICT.value,
-        CompatibilityMode::validate,
+        CompatibilityMode.STRICT.name(),
+        CompatibilityMode::parseString,
         Setting.Property.Dynamic,
         Setting.Property.NodeScope
     );
@@ -52,7 +52,13 @@ public enum CompatibilityMode {
         STRICT("strict"),
         ALLOW_MIX("allow_mix");
 
-        public static CompatibilityMode validate(String compatibilityMode) {
+        public final String mode;
+
+        CompatibilityMode(String mode) {
+            this.mode = mode;
+        }
+
+        public static CompatibilityMode parseString(String compatibilityMode) {
             try {
                 return CompatibilityMode.valueOf(compatibilityMode.toUpperCase(Locale.ROOT));
             } catch (IllegalArgumentException e) {
@@ -66,12 +72,6 @@ public static CompatibilityMode validate(String compatibilityMode) {
                 );
             }
         }
-
-        public final String value;
-
-        CompatibilityMode(String value) {
-            this.value = value;
-        }
     }
 
     public RemoteStoreService(Supplier<RepositoriesService> repositoriesService, ThreadPool threadPool) {

server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java

@@ -61,6 +61,7 @@
 import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
+import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreService.CompatibilityMode;
 import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreService.CompatibilityMode.STRICT;
 import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
 import static org.opensearch.cluster.decommission.DecommissionHelper.nodeCommissioned;
@@ -186,8 +187,11 @@ public ClusterTasksResult<Task> execute(ClusterState currentState, List<Task> jo
             final DiscoveryNode node = joinTask.node();
             if (joinTask.isBecomeClusterManagerTask() || joinTask.isFinishElectionTask()) {
                 // noop
-            } else if (currentNodes.nodeExistsWithSameRoles(node)) {
-                logger.debug("received a join request for an existing node [{}]", node);
+            } else if (currentNodes.nodeExists(node)) {
+                if (currentNodes.nodeExistsWithSameRoles(node)) {
+                    logger.debug("received a join request for an existing node [{}]", node);
+                }
+
                 if (node.isRemoteStoreNode()) {
                     /** cluster state is updated here as elect leader task can have same node present in join task as
                      *  well as current node. We want the repositories to be added in cluster state during first node
@@ -477,8 +481,8 @@ public static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode
 
         // TODO: The below check is valid till we support migration, once we start supporting migration a remote
         // store node will be able to join a non remote store cluster and vice versa. #7986
-        String remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(currentState.metadata().settings());
-        if (STRICT.value.equals(remoteStoreCompatibilityMode)) {
+        CompatibilityMode remoteStoreCompatibilityMode = REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(currentState.metadata().settings());
+        if (STRICT.equals(remoteStoreCompatibilityMode)) {
             DiscoveryNode existingNode = existingNodes.get(0);
             if (joiningNode.isRemoteStoreNode()) {
                 if (existingNode.isRemoteStoreNode()) {

server/src/test/java/org/opensearch/repositories/RepositoriesServiceTests.java

@@ -306,6 +306,9 @@ public void verify(String verificationToken, DiscoveryNode localNode) {
 
         }
 
+        @Override
+        public void verifyLocally(DiscoveryNode localNode) {}
+
         @Override
         public boolean isReadOnly() {
             return false;