Switching to strict mode with mixed cluster

Signed-off-by: Lakshya Taragi <[email protected]>

server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationAllocationIT.java

@@ -33,7 +33,6 @@
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT;
-import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.NONE;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
@@ -386,8 +385,6 @@ public void initializeCluster(boolean remoteClusterManager) {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
         internalCluster().startNodes(1);
         client = internalCluster().client();
-        setClusterMode(STRICT.mode);
-        setDirection(NONE.direction);
     }
 
     // set the compatibility mode of cluster [strict, mixed]

server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java

@@ -11,9 +11,11 @@
 import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
 import org.opensearch.client.Client;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.common.settings.SettingsException;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.indices.replication.common.ReplicationType;
+import org.opensearch.test.InternalTestCluster;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 import java.util.Optional;
@@ -25,7 +27,6 @@
 import static org.opensearch.index.IndexSettings.INDEX_REMOTE_TRANSLOG_BUFFER_INTERVAL_SETTING;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT;
-import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.NONE;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE;
 import static org.opensearch.remotemigration.RemoteStoreMigrationAllocationIT.TEST_INDEX;
 import static org.opensearch.remotemigration.RemoteStoreMigrationAllocationIT.assertNodeInCluster;
@@ -76,6 +77,37 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode()
         assertRemoteStoreBackedIndex(indexName2);
     }
 
+    // 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);
+
+        logger.info(" --> attempt switching to strict mode");
+        SettingsException exception = assertThrows(SettingsException.class, () -> setClusterMode(STRICT.mode));
+        assertEquals(
+            "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes",
+            exception.getMessage()
+        );
+
+        logger.info(" --> stop remote node");
+        internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
+        ensureStableCluster(2);
+
+        logger.info(" --> attempt switching to strict mode");
+        setClusterMode(STRICT.mode);
+    }
+
     // verify that the created index is not remote store backed
     private void assertNonRemoteStoreBackedIndex(String indexName) {
         Settings indexSettings = client.admin().indices().prepareGetIndex().execute().actionGet().getSettings().get(indexName);
@@ -118,8 +150,6 @@ private void initializeCluster(boolean remoteClusterManager) {
         internalCluster().setBootstrapClusterManagerNodeIndex(0);
         internalCluster().startNodes(1);
         client = internalCluster().client();
-        setClusterMode(STRICT.mode);
-        setDirection(NONE.direction);
     }
 
 }

server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java

@@ -36,6 +36,7 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.opensearch.OpenSearchException;
+import org.opensearch.Version;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction;
 import org.opensearch.cluster.AckedClusterStateUpdateTask;
@@ -53,12 +54,17 @@
 import org.opensearch.common.Priority;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.common.settings.ClusterSettings;
+import org.opensearch.common.settings.SettingsException;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.common.io.stream.StreamInput;
+import org.opensearch.node.remotestore.RemoteStoreNodeService;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
 
 /**
  * Transport action for updating cluster settings
@@ -137,6 +143,7 @@ protected void clusterManagerOperation(
         final ClusterState state,
         final ActionListener<ClusterUpdateSettingsResponse> listener
     ) {
+        validateCompatibilityModeSettingRequest(request, state);
         final SettingsUpdater updater = new SettingsUpdater(clusterSettings);
         clusterService.submitStateUpdateTask(
             "cluster_update_settings",
@@ -264,4 +271,27 @@ public ClusterState execute(final ClusterState currentState) {
         );
     }
 
+    /**
+     * Verifies that while trying to switch to STRICT compatibility mode, all nodes must be of the
+     * same type (all remote or all non-remote). If not, it throws SettingsException error
+     * @param request cluster settings update request, for settings to be updated and new values
+     * @param clusterState current state of cluster, for information on nodes
+     */
+    public static void validateCompatibilityModeSettingRequest(ClusterUpdateSettingsRequest request, ClusterState clusterState) {
+        if (RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.exists(request.persistentSettings())) {
+            String value = request.persistentSettings().get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey());
+            if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
+                List<DiscoveryNode> discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values());
+                Optional<DiscoveryNode> remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
+                Optional<DiscoveryNode> nonRemoteNode = discoveryNodeList.stream()
+                    .filter(dn -> dn.isRemoteStoreNode() == false)
+                    .findFirst();
+                if (remoteNode.isPresent() && nonRemoteNode.isPresent()) {
+                    throw new SettingsException(
+                        "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes"
+                    );
+                }
+            }
+        }
+    }
 }

server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java

@@ -16,10 +16,13 @@
 import org.opensearch.OpenSearchException;
 import org.opensearch.Version;
 import org.opensearch.action.ActionRequestValidationException;
+import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
+import org.opensearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.action.support.ThreadedActionListener;
 import org.opensearch.action.support.replication.ClusterStateCreationUtils;
+import org.opensearch.cluster.ClusterName;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.NotClusterManagerException;
 import org.opensearch.cluster.block.ClusterBlock;
@@ -28,14 +31,17 @@
 import org.opensearch.cluster.block.ClusterBlocks;
 import org.opensearch.cluster.coordination.FailedToCommitClusterStateException;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
+import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodeRole;
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.service.ClusterManagerThrottlingException;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.action.ActionFuture;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.common.settings.SettingsException;
 import org.opensearch.common.unit.TimeValue;
+import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.util.concurrent.ThreadContext;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.action.ActionResponse;
@@ -68,6 +74,14 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.SEGMENT_REPO;
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.TRANSLOG_REPO;
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.remoteStoreNodeAttributes;
+import static org.opensearch.cluster.routing.allocation.RemoteStoreMigrationAllocationDeciderTests.getNonRemoteNode;
+import static org.opensearch.cluster.routing.allocation.RemoteStoreMigrationAllocationDeciderTests.getRemoteNode;
+import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL;
+import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode;
+import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
 import static org.opensearch.test.ClusterServiceUtils.createClusterService;
 import static org.opensearch.test.ClusterServiceUtils.setState;
 import static org.hamcrest.Matchers.equalTo;
@@ -692,4 +706,41 @@ protected void masterOperation(Task task, Request request, ClusterState state, A
         assertFalse(retried.get());
         assertFalse(exception.get());
     }
+
+    public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() {
+        Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build();
+        FeatureFlags.initializeFeatureFlags(nodeSettings);
+
+        ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
+        request.persistentSettings(
+            Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.STRICT).build()
+        );
+
+        DiscoveryNode nonRemoteNode = getNonRemoteNode();
+        DiscoveryNode remoteNode = getRemoteNode();
+
+        DiscoveryNodes discoveryNodes = DiscoveryNodes.builder()
+            .add(nonRemoteNode)
+            .localNodeId(nonRemoteNode.getId())
+            .add(remoteNode)
+            .localNodeId(remoteNode.getId())
+            .build();
+
+        Settings mixedModeCompatibilitySettings = Settings.builder()
+            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.MIXED)
+            .build();
+
+        Metadata metadata = Metadata.builder().persistentSettings(mixedModeCompatibilitySettings).build();
+
+        ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).nodes(discoveryNodes).build();
+
+        final SettingsException exception = expectThrows(
+            SettingsException.class,
+            () -> TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, clusterState)
+        );
+        assertEquals(
+            "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes",
+            exception.getMessage()
+        );
+    }
 }

server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java

@@ -922,16 +922,16 @@ private DiscoveryNode newDiscoveryNode(Map<String, String> attributes) {
         );
     }
 
-    private static final String SEGMENT_REPO = "segment-repo";
-    private static final String TRANSLOG_REPO = "translog-repo";
+    public static final String SEGMENT_REPO = "segment-repo";
+    public static final String TRANSLOG_REPO = "translog-repo";
     private static final String CLUSTER_STATE_REPO = "cluster-state-repo";
     private static final String COMMON_REPO = "remote-repo";
 
-    private Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) {
+    public static Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName) {
         return remoteStoreNodeAttributes(segmentRepoName, translogRepoName, CLUSTER_STATE_REPO);
     }
 
-    private Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName, String clusterStateRepo) {
+    private static Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, String translogRepoName, String clusterStateRepo) {
         String segmentRepositoryTypeAttributeKey = String.format(
             Locale.getDefault(),
             REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,
@@ -968,7 +968,7 @@ private Map<String, String> remoteStoreNodeAttributes(String segmentRepoName, St
         };
     }
 
-    private Map<String, String> remoteStateNodeAttributes(String clusterStateRepo) {
+    private static Map<String, String> remoteStateNodeAttributes(String clusterStateRepo) {
         String clusterStateRepositoryTypeAttributeKey = String.format(
             Locale.getDefault(),
             REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT,

server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java

@@ -60,16 +60,16 @@
 import org.opensearch.node.remotestore.RemoteStoreNodeService;
 
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Locale;
-import java.util.Map;
 
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.SEGMENT_REPO;
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.TRANSLOG_REPO;
+import static org.opensearch.cluster.coordination.JoinTaskExecutorTests.remoteStoreNodeAttributes;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_SEGMENT_STORE_REPOSITORY;
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
 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.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL;
-import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING;
 import static org.hamcrest.core.Is.is;
@@ -659,21 +659,16 @@ private ClusterState getInitialClusterState(
     }
 
     // get a dummy non-remote node
-    private DiscoveryNode getNonRemoteNode() {
+    public static DiscoveryNode getNonRemoteNode() {
         return new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT);
     }
 
     // get a dummy remote node
-    public DiscoveryNode getRemoteNode() {
-        Map<String, String> attributes = new HashMap<>();
-        attributes.put(
-            REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY,
-            "REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_VALUE"
-        );
+    public static DiscoveryNode getRemoteNode() {
         return new DiscoveryNode(
             UUIDs.base64UUID(),
             buildNewFakeTransportAddress(),
-            attributes,
+            remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO),
             DiscoveryNodeRole.BUILT_IN_ROLES,
             Version.CURRENT
         );