From 8b05f9d8a078971ee74d7fa1035f5674e9356b76 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Thu, 4 Apr 2024 17:53:27 +0530 Subject: [PATCH] Changing compatibility mode with multiple node versions Signed-off-by: Lakshya Taragi --- .../RemoteStoreMigrationSettingsUpdateIT.java | 2 +- .../TransportClusterUpdateSettingsAction.java | 53 +++++-- ...ransportClusterManagerNodeActionTests.java | 139 ++++++++++++++++-- 3 files changed, 166 insertions(+), 28 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java index 57fca429e44cc..a773abab35728 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java +++ b/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); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 041b91a052b35..a2d3a7de6a5c6 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/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 discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values()); + validateAllNodesOfSameVersion(discoveryNodeList); if (value.equals(RemoteStoreNodeService.CompatibilityMode.STRICT.mode)) { - List discoveryNodeList = new ArrayList<>(clusterState.nodes().getNodes().values()); - Optional remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); - Optional 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 discoveryNodeList) { + Set 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 discoveryNodeList) { + Optional remoteNode = discoveryNodeList.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst(); + Optional 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" + ); + } + } + } diff --git a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java index 8f44314a8bedd..2e669b911f6c6 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java +++ b/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); } + }