Allow clearing remote_store.compatibility_mode setting (#13646) (#13761)

Signed-off-by: Bhumika Saini <sabhumik@amazon.com>
opensearch-project · May 21, 2024 · d1f00b5 · d1f00b5
1 parent fe89133
commit d1f00b5
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Fixed
 - Fix negative RequestStats metric issue ([#13553](https://github.com/opensearch-project/OpenSearch/pull/13553))
 - Fix get field mapping API returns 404 error in mixed cluster with multiple versions ([#13624](https://github.com/opensearch-project/OpenSearch/pull/13624))
+- Allow clearing `remote_store.compatibility_mode` setting ([#13646](https://github.com/opensearch-project/OpenSearch/pull/13646))
 
 ### Security
 

diff --git a/...ClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/...ClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java
@@ -14,6 +14,7 @@
 import org.opensearch.test.InternalTestCluster;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
+import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Optional;
 
@@ -26,6 +27,8 @@
 public class RemoteStoreMigrationSettingsUpdateIT extends RemoteStoreMigrationShardAllocationBaseTestCase {
 
     private Client client;
+    private String nonRemoteNodeName;
+    private String remoteNodeName;
 
     // remote store backed index setting tests
 
@@ -120,18 +123,7 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
     // compatibility mode setting test
 
     public void testSwitchToStrictMode() throws Exception {
-        logger.info("Initialize cluster");
-        initializeCluster(false);
-
-        logger.info("Create a mixed mode cluster");
-        setClusterMode(MIXED.mode);
-        addRemote = true;
-        String remoteNodeName = internalCluster().startNode();
-        addRemote = false;
-        String nonRemoteNodeName = internalCluster().startNode();
-        internalCluster().validateClusterFormed();
-        assertNodeInCluster(remoteNodeName);
-        assertNodeInCluster(nonRemoteNodeName);
+        createMixedModeCluster();
 
         logger.info("Attempt switching to strict mode");
         SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode));
@@ -140,12 +132,39 @@ public void testSwitchToStrictMode() throws Exception {
             exception.getMessage()
         );
 
+        stopRemoteNode();
+
+        logger.info("Attempt switching to strict mode");
+        setClusterMode(STRICT.mode);
+    }
+
+    public void testClearCompatibilityModeSetting() throws Exception {
+        createMixedModeCluster();
+        stopRemoteNode();
+
+        logger.info("Attempt clearing compatibility mode");
+        clearClusterMode();
+    }
+
+    private void stopRemoteNode() throws IOException {
         logger.info("Stop remote node so that cluster had only non-remote nodes");
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
         ensureStableCluster(2);
+    }
 
-        logger.info("Attempt switching to strict mode");
-        setClusterMode(STRICT.mode);
+    private void createMixedModeCluster() {
+        logger.info("Initialize cluster");
+        initializeCluster(false);
+
+        logger.info("Create a mixed mode cluster");
+        setClusterMode(MIXED.mode);
+        addRemote = true;
+        remoteNodeName = internalCluster().startNode();
+        addRemote = false;
+        nonRemoteNodeName = internalCluster().startNode();
+        internalCluster().validateClusterFormed();
+        assertNodeInCluster(remoteNodeName);
+        assertNodeInCluster(nonRemoteNodeName);
     }
 
     // bootstrap a cluster

diff --git a/.../java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java b/.../java/org/opensearch/remotemigration/RemoteStoreMigrationShardAllocationBaseTestCase.java
@@ -55,6 +55,11 @@ protected void setClusterMode(String mode) {
         assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
     }
 
+    protected void clearClusterMode() {
+        updateSettingsRequest.persistentSettings(Settings.builder().putNull(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()));
+        assertAcked(internalCluster().client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
+    }
+
     // set the migration direction for cluster [remote_store, docrep, none]
     protected void setDirection(String direction) {
         updateSettingsRequest.persistentSettings(Settings.builder().put(MIGRATION_DIRECTION_SETTING.getKey(), direction));

diff --git a/...va/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/...va/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java
@@ -66,7 +66,6 @@
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.Locale;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -285,9 +284,9 @@ public ClusterState execute(final ClusterState currentState) {
     public void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) {
         Settings settings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build();
         if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(settings)) {
-            String value = settings.get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey()).toLowerCase(Locale.ROOT);
+            String value = RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(settings).mode;
             validateAllNodesOfSameVersion(clusterState.nodes());
-            if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
+            if (RemoteStoreNodeService.CompatibilityMode.STRICT.mode.equals(value)) {
                 validateAllNodesOfSameType(clusterState.nodes());
                 validateIndexSettings(clusterState);
             }