Removing remote store specific cluster settings instead rely on node …

…attributes, fixed UTs and ITs

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

distribution/src/config/opensearch.yml

@@ -92,10 +92,10 @@ ${path.logs}
 # cluster.remote_store.enabled: true
 #
 # Repository to use for segment upload while enforcing remote store for an index
-# cluster.remote_store.segment.repository: my-repo-1
+# node.attr.remote_store.segment.repository : my-repo-1
 #
 # Repository to use for translog upload while enforcing remote store for an index
-# cluster.remote_store.translog.repository: my-repo-1
+# node.attr.remote_store.translog.repository : my-repo-1
 #
 # ---------------------------------- Experimental Features -----------------------------------
 #

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

@@ -26,10 +26,15 @@
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Locale;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
+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;
+import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY;
+import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY;
+import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 public abstract class AbstractRemoteStoreMockRepositoryIntegTestCase extends AbstractSnapshotIntegTestCase {
@@ -47,7 +52,6 @@ protected Settings featureFlagSettings() {
     public void setup() {
         FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
         FeatureFlagSetter.set(FeatureFlags.SEGMENT_REPLICATION_EXPERIMENTAL);
-        internalCluster().startClusterManagerOnlyNode(remoteStoreClusterSettings(REPOSITORY_NAME, TRANSLOG_REPOSITORY_NAME));
     }
 
     @Override
@@ -66,34 +70,64 @@ protected Settings remoteStoreIndexSettings(int numberOfReplicas) {
             .build();
     }
 
+    public Settings buildRemoteStoreNodeAttributes(Path repoLocation, double ioFailureRate, String skipExceptionBlobList, long maxFailure) {
+        String segmentRepoTypeAttributeKey = String.format(
+            Locale.getDefault(),
+            "node.attr." + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
+            REPOSITORY_NAME
+        );
+        String translogRepoTypeAttributeKey = String.format(
+            Locale.getDefault(),
+            "node.attr." + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
+            TRANSLOG_REPOSITORY_NAME
+        );
+        String segmentRepoSettingsAttributeKeyPrefix = String.format(
+            Locale.getDefault(),
+            "node.attr." + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX,
+            REPOSITORY_NAME
+        );
+        String translogRepoSettingsAttributeKeyPrefix = String.format(
+            Locale.getDefault(),
+            "node.attr." + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX,
+            TRANSLOG_REPOSITORY_NAME
+        );
+
+        return Settings.builder()
+            .put("node.attr." + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, REPOSITORY_NAME)
+            .put(segmentRepoTypeAttributeKey, "mock")
+            .put(segmentRepoSettingsAttributeKeyPrefix + "location", repoLocation)
+            .put(segmentRepoSettingsAttributeKeyPrefix + "random_control_io_exception_rate", ioFailureRate)
+            .put(segmentRepoSettingsAttributeKeyPrefix + "skip_exception_on_verification_file", true)
+            .put(segmentRepoSettingsAttributeKeyPrefix + "skip_exception_on_list_blobs", true)
+            .put(segmentRepoSettingsAttributeKeyPrefix + "skip_exception_on_blobs", skipExceptionBlobList)
+            .put(segmentRepoSettingsAttributeKeyPrefix + "max_failure_number", maxFailure)
+            .put("node.attr." + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, TRANSLOG_REPOSITORY_NAME)
+            .put(translogRepoTypeAttributeKey, "mock")
+            .put(translogRepoSettingsAttributeKeyPrefix + "location", repoLocation)
+            .build();
+    }
+
     protected void deleteRepo() {
         logger.info("--> Deleting the repository={}", REPOSITORY_NAME);
         assertAcked(clusterAdmin().prepareDeleteRepository(REPOSITORY_NAME));
         logger.info("--> Deleting the repository={}", TRANSLOG_REPOSITORY_NAME);
         assertAcked(clusterAdmin().prepareDeleteRepository(TRANSLOG_REPOSITORY_NAME));
     }
 
-    protected String setup(Path repoLocation, double ioFailureRate, String skipExceptionBlobList, long maxFailure) {
+    protected String setup(Path repoLocation, double ioFailureRate, String skipExceptionBlobList, long maxFailure) throws Exception {
         logger.info("--> Creating repository={} at the path={}", REPOSITORY_NAME, repoLocation);
+        logger.info("--> Creating repository={} at the path={}", TRANSLOG_REPOSITORY_NAME, repoLocation);
         // The random_control_io_exception_rate setting ensures that 10-25% of all operations to remote store results in
         /// IOException. skip_exception_on_verification_file & skip_exception_on_list_blobs settings ensures that the
         // repository creation can happen without failure.
-        createRepository(
-            REPOSITORY_NAME,
-            "mock",
-            Settings.builder()
-                .put("location", repoLocation)
-                .put("random_control_io_exception_rate", ioFailureRate)
-                .put("skip_exception_on_verification_file", true)
-                .put("skip_exception_on_list_blobs", true)
-                // Skipping is required for metadata as it is part of recovery
-                .put("skip_exception_on_blobs", skipExceptionBlobList)
-                .put("max_failure_number", maxFailure)
-        );
-        logger.info("--> Creating repository={} at the path={}", TRANSLOG_REPOSITORY_NAME, repoLocation);
-        createRepository(TRANSLOG_REPOSITORY_NAME, "mock", Settings.builder().put("location", repoLocation));
+        Settings settings = Settings.builder()
+            .put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), randomBoolean() ? ReplicationType.SEGMENT : ReplicationType.DOCUMENT)
+            .put(buildRemoteStoreNodeAttributes(repoLocation, ioFailureRate, skipExceptionBlobList, maxFailure))
+            .build();
 
-        String dataNodeName = internalCluster().startDataOnlyNodes(1).get(0);
+        internalCluster().startClusterManagerOnlyNode(settings);
+        String dataNodeName = internalCluster().startDataOnlyNode(settings);
+        // assertRepositoryMetadataPresentInClusterState();
         createIndex(INDEX_NAME);
         logger.info("--> Created index={}", INDEX_NAME);
         ensureYellowAndNoInitializingShards(INDEX_NAME);

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

@@ -75,8 +75,8 @@ public void testDefaultRemoteStoreNoUserOverrideExceptReplicationTypeSegment() t
         verifyRemoteStoreIndexSettings(
             indexSettings,
             "true",
-            "my-segment-repo-1",
-            "my-translog-repo-1",
+            REPOSITORY_NAME,
+            REPOSITORY_2_NAME,
             ReplicationType.SEGMENT.toString(),
             IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
         );

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

@@ -13,12 +13,9 @@
 import org.opensearch.cluster.metadata.IndexMetadata;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.indices.replication.common.ReplicationType;
-import org.opensearch.test.FeatureFlagSetter;
 import org.opensearch.test.OpenSearchIntegTestCase;
-import org.junit.After;
 import org.junit.Before;
 
 import java.util.Locale;
@@ -28,53 +25,15 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
 import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
-import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.containsString;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
-public class CreateRemoteIndexIT extends OpenSearchIntegTestCase {
-
-    @After
-    public void teardown() {
-        assertAcked(clusterAdmin().prepareDeleteRepository("my-segment-repo-1"));
-        assertAcked(clusterAdmin().prepareDeleteRepository("my-translog-repo-1"));
-        assertAcked(clusterAdmin().prepareDeleteRepository("my-custom-repo"));
-    }
-
-    @Override
-    protected Settings nodeSettings(int nodeOriginal) {
-        Settings settings = super.nodeSettings(nodeOriginal);
-        Settings.Builder builder = Settings.builder()
-            .put(remoteStoreClusterSettings("my-segment-repo-1", "my-translog-repo-1"))
-            .put(settings);
-        return builder.build();
-    }
-
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build();
-    }
+public class CreateRemoteIndexIT extends RemoteStoreBaseIntegTestCase {
 
     @Before
-    public void setup() {
-        FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
-        internalCluster().startClusterManagerOnlyNode();
-        assertAcked(
-            clusterAdmin().preparePutRepository("my-segment-repo-1")
-                .setType("fs")
-                .setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
-        );
-        assertAcked(
-            clusterAdmin().preparePutRepository("my-translog-repo-1")
-                .setType("fs")
-                .setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
-        );
-        assertAcked(
-            clusterAdmin().preparePutRepository("my-custom-repo")
-                .setType("fs")
-                .setSettings(Settings.builder().put("location", randomRepoPath().toAbsolutePath()))
-        );
+    public void setup() throws Exception {
+        internalCluster().startNodes(2);
     }
 
     public void testDefaultRemoteStoreNoUserOverride() throws Exception {
@@ -91,8 +50,8 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
         verifyRemoteStoreIndexSettings(
             indexSettings,
             "true",
-            "my-segment-repo-1",
-            "my-translog-repo-1",
+            REPOSITORY_NAME,
+            REPOSITORY_2_NAME,
             ReplicationType.SEGMENT.toString(),
             IndexSettings.DEFAULT_REMOTE_TRANSLOG_BUFFER_INTERVAL
         );

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

@@ -27,7 +27,9 @@
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.opensearch.test.disruption.NetworkDisruption;
 import org.opensearch.test.transport.MockTransportService;
+import org.junit.Before;
 
+import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
@@ -43,12 +45,20 @@
 public class PrimaryTermValidationIT extends RemoteStoreBaseIntegTestCase {
 
     private static final String INDEX_NAME = "remote-store-test-idx-1";
+    protected Path absolutePath;
+    protected Path absolutePath2;
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(MockTransportService.TestPlugin.class);
     }
 
+    @Before
+    public void setup() {
+        absolutePath = randomRepoPath().toAbsolutePath();
+        absolutePath2 = randomRepoPath().toAbsolutePath();
+    }
+
     public void testPrimaryTermValidation() throws Exception {
         // Follower checker interval is lower compared to leader checker so that the cluster manager can remove the node
         // with network partition faster. The follower check retry count is also kept 1.
@@ -59,12 +69,10 @@ public void testPrimaryTermValidation() throws Exception {
             .put(FollowersChecker.FOLLOWER_CHECK_TIMEOUT_SETTING.getKey(), "1s")
             .put(FollowersChecker.FOLLOWER_CHECK_INTERVAL_SETTING.getKey(), "1s")
             .put(FollowersChecker.FOLLOWER_CHECK_RETRY_COUNT_SETTING.getKey(), 1)
-            .put(remoteStoreClusterSettings(REPOSITORY_NAME, REPOSITORY_2_NAME, true))
+            .put(remoteStoreClusterSettings(REPOSITORY_NAME, absolutePath, REPOSITORY_2_NAME, absolutePath2))
             .build();
         internalCluster().startClusterManagerOnlyNode(clusterSettings);
         internalCluster().startDataOnlyNodes(2, clusterSettings);
-        ensureStableCluster(3);
-        assertRepositoryMetadataPresentInClusterState();
 
         // Create index
         createIndex(INDEX_NAME, remoteStoreIndexSettings(1));

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

@@ -18,7 +18,6 @@
 import java.nio.file.Path;
 
 import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
-import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
 public class RemoteIndexPrimaryRelocationIT extends IndexPrimaryRelocationIT {
@@ -29,15 +28,12 @@ public class RemoteIndexPrimaryRelocationIT extends IndexPrimaryRelocationIT {
 
     public void setup() {
         absolutePath = randomRepoPath().toAbsolutePath();
-        assertAcked(
-            clusterAdmin().preparePutRepository(REPOSITORY_NAME).setType("fs").setSettings(Settings.builder().put("location", absolutePath))
-        );
     }
 
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(remoteStoreClusterSettings(REPOSITORY_NAME, REPOSITORY_NAME, false))
+            .put(remoteStoreClusterSettings(REPOSITORY_NAME, absolutePath))
             .build();
     }
 

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

@@ -15,18 +15,14 @@
 import org.opensearch.index.IndexSettings;
 import org.opensearch.indices.recovery.IndexRecoveryIT;
 import org.opensearch.indices.replication.common.ReplicationType;
-import org.opensearch.repositories.fs.FsRepository;
 import org.opensearch.test.OpenSearchIntegTestCase;
 import org.hamcrest.Matcher;
 import org.hamcrest.Matchers;
 import org.junit.After;
+import org.junit.Before;
 
-import java.util.Locale;
+import java.nio.file.Path;
 
-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;
-import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY;
-import static org.opensearch.action.admin.cluster.remotestore.RemoteStoreNode.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY;
 import static org.opensearch.remotestore.RemoteStoreBaseIntegTestCase.remoteStoreClusterSettings;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
@@ -35,33 +31,18 @@ public class RemoteIndexRecoveryIT extends IndexRecoveryIT {
 
     protected static final String REPOSITORY_NAME = "test-remote-store-repo";
 
+    protected Path repositoryPath;
+
+    @Before
+    public void setup() {
+        repositoryPath = randomRepoPath().toAbsolutePath();
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(remoteStoreClusterSettings(REPOSITORY_NAME))
-            .put(repositoryNodeAttributes(REPOSITORY_NAME, FsRepository.TYPE, randomRepoPath().toAbsolutePath().toString()))
-            .build();
-    }
-
-    private Settings repositoryNodeAttributes(String name, String type, String location) {
-        String segmentRepoNameAttributeKey = "node.attr." + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY;
-        String translogRepoNameAttributeKey = "node.attr." + REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY;
-        String typeAttributeKey = String.format(
-            Locale.getDefault(),
-            "node.attr." + REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
-            name
-        );
-        String settingsAttributeKey = String.format(
-            Locale.getDefault(),
-            "node.attr." + REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX,
-            name
-        );
-        return Settings.builder()
-            .put(segmentRepoNameAttributeKey, name)
-            .put(translogRepoNameAttributeKey, name)
-            .put(typeAttributeKey, type)
-            .put(settingsAttributeKey + "location", location)
+            .put(remoteStoreClusterSettings(REPOSITORY_NAME, repositoryPath, REPOSITORY_NAME, repositoryPath))
             .build();
     }
 

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

@@ -29,7 +29,6 @@
 import org.opensearch.snapshots.SnapshotState;
 import org.opensearch.test.InternalTestCluster;
 import org.junit.After;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.nio.file.Path;
@@ -47,23 +46,21 @@ public class RemoteRestoreSnapshotIT extends AbstractSnapshotIntegTestCase {
     private static final String BASE_REMOTE_REPO = "test-rs-repo" + TEST_REMOTE_STORE_REPO_SUFFIX;
     private Path remoteRepoPath;
 
-    @Before
-    public void setup() {
-        remoteRepoPath = randomRepoPath().toAbsolutePath();
-        createRepository(BASE_REMOTE_REPO, "fs", remoteRepoPath);
-    }
-
     @After
     public void teardown() {
+        remoteRepoPath = null;
         assertAcked(clusterAdmin().prepareDeleteRepository(BASE_REMOTE_REPO));
     }
 
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
+        if (remoteRepoPath == null) {
+            remoteRepoPath = randomRepoPath().toAbsolutePath();
+        }
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
             .put(FeatureFlags.REMOTE_STORE, "true")
-            .put(remoteStoreClusterSettings(BASE_REMOTE_REPO))
+            .put(remoteStoreClusterSettings(BASE_REMOTE_REPO, remoteRepoPath))
             .build();
     }
 

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

@@ -25,15 +25,14 @@ public class RemoteSegmentStatsFromNodesStatsIT extends RemoteStoreBaseIntegTest
     private static final int CLUSTER_MANAGER_NODE_COUNT = 3;
 
     @Before
-    public void setup() throws Exception {
+    public void setup() {
         setupCustomCluster();
     }
 
-    private void setupCustomCluster() throws Exception {
+    private void setupCustomCluster() {
         internalCluster().startClusterManagerOnlyNodes(CLUSTER_MANAGER_NODE_COUNT);
         internalCluster().startDataOnlyNodes(DATA_NODE_COUNT);
         ensureStableCluster(DATA_NODE_COUNT + CLUSTER_MANAGER_NODE_COUNT);
-        assertRepositoryMetadataPresentInClusterState();
     }
 
     /**