Add validation while updating CompatibilityMode setting (#13080) (#13224

)

(cherry picked from commit 695fbde)

Signed-off-by: Lakshya Taragi <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

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

@@ -12,11 +12,13 @@
 import org.opensearch.client.Client;
 import org.opensearch.cluster.metadata.IndexMetadata;
 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.snapshots.SnapshotInfo;
 import org.opensearch.snapshots.SnapshotState;
+import org.opensearch.test.InternalTestCluster;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 import java.nio.file.Path;
@@ -28,6 +30,7 @@
 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.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED;
+import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT;
 import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
@@ -140,6 +143,37 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix
         assertRemoteStoreBackedIndex(restoredIndexName2);
     }
 
+    // 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 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);
+    }
+
     // restore indices from a snapshot
     private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) {
         RestoreSnapshotResponse restoreSnapshotResponse = client.admin()

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

@@ -45,6 +45,7 @@
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.node.DiscoveryNode;
+import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.routing.allocation.AllocationService;
 import org.opensearch.cluster.service.ClusterManagerTaskKeys;
 import org.opensearch.cluster.service.ClusterManagerTaskThrottler;
@@ -53,12 +54,18 @@
 import org.opensearch.common.Priority;
 import org.opensearch.common.inject.Inject;
 import org.opensearch.common.settings.ClusterSettings;
+import org.opensearch.common.settings.Settings;
+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.Locale;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Transport action for updating cluster settings
@@ -251,6 +258,7 @@ public void onFailure(String source, Exception e) {
 
                 @Override
                 public ClusterState execute(final ClusterState currentState) {
+                    validateCompatibilityModeSettingRequest(request, state);
                     final ClusterState clusterState = updater.updateSettings(
                         currentState,
                         clusterSettings.upgradeSettings(request.transientSettings()),
@@ -264,4 +272,49 @@ public ClusterState execute(final ClusterState currentState) {
         );
     }
 
+    /**
+     * Runs various checks associated with changing cluster compatibility mode
+     * @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 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);
+            validateAllNodesOfSameVersion(clusterState.nodes());
+            if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) {
+                validateAllNodesOfSameType(clusterState.nodes());
+            }
+        }
+    }
+
+    /**
+     * Verifies that while trying to change the compatibility mode, all nodes must have the same version.
+     * If not, it throws SettingsException error
+     * @param discoveryNodes current discovery nodes in the cluster
+     */
+    private void validateAllNodesOfSameVersion(DiscoveryNodes discoveryNodes) {
+        if (discoveryNodes.getMaxNodeVersion().equals(discoveryNodes.getMinNodeVersion()) == false) {
+            throw new SettingsException("can not change the compatibility mode when all the nodes in cluster are not of the same version");
+        }
+    }
+
+    /**
+     * 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 discoveryNodes current discovery nodes in the cluster
+     */
+    private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) {
+        Set<Boolean> nodeTypes = discoveryNodes.getNodes()
+            .values()
+            .stream()
+            .map(DiscoveryNode::isRemoteStoreNode)
+            .collect(Collectors.toSet());
+        if (nodeTypes.size() != 1) {
+            throw new SettingsException(
+                "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes"
+            );
+        }
+    }
+
 }

server/src/main/java/org/opensearch/snapshots/RestoreService.java

@@ -697,9 +697,9 @@ private Settings getOverrideSettingsInternal() {
                         // We will use whatever replication strategy provided by user or from snapshot metadata unless
                         // cluster is remote store enabled or user have restricted a specific replication type in the
                         // cluster. If cluster is undergoing remote store migration, replication strategy is strictly SEGMENT type
-                        if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings()) == true
-                            || clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING) == true
-                            || RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings) == true) {
+                        if (RemoteStoreNodeAttribute.isRemoteStoreAttributePresent(clusterService.getSettings())
+                            || clusterSettings.get(IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING)
+                            || RemoteStoreNodeService.isMigratingToRemoteStore(clusterSettings)) {
                             MetadataCreateIndexService.updateReplicationStrategy(
                                 settingsBuilder,
                                 request.indexSettings(),