Changing compatibility mode with multiple node versions

Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com>

Author: ltaragi

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

@@ -100,7 +100,7 @@ public void testSwitchToStrictMode() throws Exception {
             exception.getMessage()
         );
 
-        logger.info(" --> stop remote node");
+        logger.info(" --> stop remote node so that cluster had only non-remote nodes");
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(remoteNodeName));
         ensureStableCluster(2);
 

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

@@ -65,6 +65,8 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Transport action for updating cluster settings
@@ -272,26 +274,51 @@ 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
+     * 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 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());
+            String value = request.persistentSettings()
+                .get(RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey())
+                .toLowerCase();
+            List<DiscoveryNode> discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values());
+            validateAllNodesOfSameVersion(discoveryNodeList);
             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"
-                    );
-                }
+                validateAllNodesOfSameType(discoveryNodeList);
             }
         }
     }
+
+    /**
+     * Verifies that while trying to change the compatibility mode, all nodes must have the same version.
+     * If not, it throws SettingsException error
+     * @param discoveryNodeList list of the current discovery nodes in the cluster
+     */
+    private static void validateAllNodesOfSameVersion(List<DiscoveryNode> discoveryNodeList) {
+        Set<Version> versions = discoveryNodeList.stream().map(DiscoveryNode::getVersion).collect(Collectors.toSet());
+        if (versions.size() != 1) {
+            throw new SettingsException(
+                "can not change the compatibility mode when all the nodes in cluster are not of the same version. Present versions: "
+                    + versions
+            );
+        }
+    }
+
+    /**
+     * 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 discoveryNodeList list of the current discovery nodes in the cluster
+     */
+    private static void validateAllNodesOfSameType(List<DiscoveryNode> discoveryNodeList) {
+        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

@@ -37,6 +37,7 @@
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.service.ClusterManagerThrottlingException;
 import org.opensearch.cluster.service.ClusterService;
+import org.opensearch.common.UUIDs;
 import org.opensearch.common.action.ActionFuture;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.SettingsException;
@@ -84,6 +85,9 @@
 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.opensearch.test.VersionUtils.randomCompatibleVersion;
+import static org.opensearch.test.VersionUtils.randomOpenSearchVersion;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
@@ -711,26 +715,28 @@ public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() {
         Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build();
         FeatureFlags.initializeFeatureFlags(nodeSettings);
 
+        // request to change cluster compatibility mode to STRICT
+        Settings currentCompatibilityModeSettings = Settings.builder()
+            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.MIXED)
+            .build();
+        Settings intendedCompatibilityModeSettings = Settings.builder()
+            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.STRICT)
+            .build();
         ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
-        request.persistentSettings(
-            Settings.builder().put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.STRICT).build()
-        );
+        request.persistentSettings(intendedCompatibilityModeSettings);
 
-        DiscoveryNode nonRemoteNode = getNonRemoteNode();
-        DiscoveryNode remoteNode = getRemoteNode();
+        // mixed cluster (containing both remote and non-remote nodes)
+        DiscoveryNode nonRemoteNode1 = getNonRemoteNode();
+        DiscoveryNode remoteNode1 = getRemoteNode();
 
         DiscoveryNodes discoveryNodes = DiscoveryNodes.builder()
-            .add(nonRemoteNode)
-            .localNodeId(nonRemoteNode.getId())
-            .add(remoteNode)
-            .localNodeId(remoteNode.getId())
+            .add(nonRemoteNode1)
+            .localNodeId(nonRemoteNode1.getId())
+            .add(remoteNode1)
+            .localNodeId(remoteNode1.getId())
             .build();
 
-        Settings mixedModeCompatibilitySettings = Settings.builder()
-            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.MIXED)
-            .build();
-
-        Metadata metadata = Metadata.builder().persistentSettings(mixedModeCompatibilitySettings).build();
+        Metadata metadata = Metadata.builder().persistentSettings(currentCompatibilityModeSettings).build();
 
         ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).nodes(discoveryNodes).build();
 
@@ -742,5 +748,110 @@ public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() {
             "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes",
             exception.getMessage()
         );
+
+        DiscoveryNode nonRemoteNode2 = getNonRemoteNode();
+        DiscoveryNode remoteNode2 = getRemoteNode();
+
+        // cluster with only non-remote nodes
+        discoveryNodes = DiscoveryNodes.builder()
+            .add(nonRemoteNode1)
+            .localNodeId(nonRemoteNode1.getId())
+            .add(nonRemoteNode2)
+            .localNodeId(nonRemoteNode2.getId())
+            .build();
+        ClusterState sameTypeClusterState = ClusterState.builder(clusterState).nodes(discoveryNodes).build();
+        TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState);
+
+        // cluster with only non-remote nodes
+        discoveryNodes = DiscoveryNodes.builder()
+            .add(remoteNode1)
+            .localNodeId(remoteNode1.getId())
+            .add(remoteNode2)
+            .localNodeId(remoteNode2.getId())
+            .build();
+        sameTypeClusterState = ClusterState.builder(sameTypeClusterState).nodes(discoveryNodes).build();
+        TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState);
+    }
+
+    public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersions() {
+        Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build();
+        FeatureFlags.initializeFeatureFlags(nodeSettings);
+
+        // request to change cluster compatibility mode
+        boolean toStrictMode = randomBoolean();
+        Settings currentCompatibilityModeSettings = Settings.builder()
+            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), CompatibilityMode.MIXED)
+            .build();
+        Settings intendedCompatibilityModeSettings = Settings.builder()
+            .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), toStrictMode ? CompatibilityMode.STRICT : CompatibilityMode.MIXED)
+            .build();
+        ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest();
+        request.persistentSettings(intendedCompatibilityModeSettings);
+
+        // two different but compatible open search versions for the discovery nodes
+        final Version version1 = randomOpenSearchVersion(random());
+        final Version version2 = randomCompatibleVersion(random(), version1);
+
+        assert version1.equals(version2) == false : "current nodes in the cluster must be of different versions";
+
+        DiscoveryNode discoveryNode1 = new DiscoveryNode(
+            UUIDs.base64UUID(),
+            buildNewFakeTransportAddress(),
+            toStrictMode ? remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO) : Collections.emptyMap(),
+            DiscoveryNodeRole.BUILT_IN_ROLES,
+            version1
+        );
+        DiscoveryNode discoveryNode2 = new DiscoveryNode(
+            UUIDs.base64UUID(),
+            buildNewFakeTransportAddress(),
+            toStrictMode ? remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO) : Collections.emptyMap(),
+            DiscoveryNodeRole.BUILT_IN_ROLES,
+            version2 // not same as discoveryNode1
+        );
+
+        DiscoveryNodes discoveryNodes = DiscoveryNodes.builder()
+            .add(discoveryNode1)
+            .localNodeId(discoveryNode1.getId())
+            .add(discoveryNode2)
+            .localNodeId(discoveryNode2.getId())
+            .build();
+
+        Metadata metadata = Metadata.builder().persistentSettings(currentCompatibilityModeSettings).build();
+
+        ClusterState differentVersionClusterState = ClusterState.builder(ClusterName.DEFAULT)
+            .metadata(metadata)
+            .nodes(discoveryNodes)
+            .build();
+
+        // changing compatibility mode when all nodes are not of the same version
+        final SettingsException exception = expectThrows(
+            SettingsException.class,
+            () -> TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, differentVersionClusterState)
+        );
+        assertThat(
+            exception.getMessage(),
+            containsString(
+                "can not change the compatibility mode when all the nodes in cluster are not of the same version. Present versions: ["
+            )
+        );
+
+        // changing compatibility mode when all nodes are of the same version
+        discoveryNode2 = new DiscoveryNode(
+            UUIDs.base64UUID(),
+            buildNewFakeTransportAddress(),
+            toStrictMode ? remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO) : Collections.emptyMap(),
+            DiscoveryNodeRole.BUILT_IN_ROLES,
+            version1 // same as discoveryNode1
+        );
+        discoveryNodes = DiscoveryNodes.builder()
+            .add(discoveryNode1)
+            .localNodeId(discoveryNode1.getId())
+            .add(discoveryNode2)
+            .localNodeId(discoveryNode2.getId())
+            .build();
+
+        ClusterState sameVersionClusterState = ClusterState.builder(differentVersionClusterState).nodes(discoveryNodes).build();
+        TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameVersionClusterState);
     }
+
 }