From 1072a87e7a2130ca73519bbbe6ac5c23f6b560fc Mon Sep 17 00:00:00 2001 From: Shubh Sahu Date: Wed, 24 Apr 2024 16:06:28 +0530 Subject: [PATCH] Adding validation for remote-cluster-state to be enabled before putting migration direction to remote-store Signed-off-by: Shubh Sahu --- .../MigrationBaseTestCase.java | 4 +- .../TransportClusterUpdateSettingsAction.java | 24 ++++++ ...ransportClusterManagerNodeActionTests.java | 86 +++++++++++++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java index 0c35f91121059..859f180dd67a1 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/MigrationBaseTestCase.java @@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; 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.opensearch.repositories.fs.ReloadableFsRepository.REPOSITORIES_FAILRATE_SETTING; @@ -62,10 +63,11 @@ protected Settings nodeSettings(int nodeOrdinal) { .put(super.nodeSettings(nodeOrdinal)) .put(extraSettings) .put(remoteStoreClusterSettings(REPOSITORY_NAME, segmentRepoPath, REPOSITORY_2_NAME, translogRepoPath)) + .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) .build(); } else { logger.info("Adding docrep node"); - return Settings.builder().put(super.nodeSettings(nodeOrdinal)).build(); + return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true).build(); } } 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 e6c149216da09..14e3e7cc449a9 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 @@ -67,6 +67,9 @@ import java.util.Set; import java.util.stream.Collectors; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; + /** * Transport action for updating cluster settings * @@ -259,6 +262,7 @@ public void onFailure(String source, Exception e) { @Override public ClusterState execute(final ClusterState currentState) { validateCompatibilityModeSettingRequest(request, state); + validateRemoteClusterStateEnabled(request); final ClusterState clusterState = updater.updateSettings( currentState, clusterSettings.upgradeSettings(request.transientSettings()), @@ -317,4 +321,24 @@ private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) { } } + /** + * Verifies that remote cluster state is enabled if the compatibility mode is set to MIXED + * and migration direction is getting updated to remote store + * @param request cluster settings update request, for settings to be updated and new values + * @throws SettingsException if remote cluster state is not enabled + */ + public void validateRemoteClusterStateEnabled(ClusterUpdateSettingsRequest request) { + Settings settings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build(); + boolean isMixedOrGettingMixed = clusterSettings.get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING) + .equals(RemoteStoreNodeService.CompatibilityMode.MIXED) + || RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING.get(settings) + .equals(RemoteStoreNodeService.CompatibilityMode.MIXED); + boolean isGettingRemote = RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING.get(settings) + .equals(RemoteStoreNodeService.Direction.REMOTE_STORE); + if (isMixedOrGettingMixed && isGettingRemote) { + if (clusterSettings.get(REMOTE_CLUSTER_STATE_ENABLED_SETTING) == false) { + throw new SettingsException("can not switch migration direction to remote store when remote cluster state is not enabled"); + } + } + } } 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 b3eb2443fa940..63d3874d58a1d 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 @@ -44,6 +44,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.UUIDs; import org.opensearch.common.action.ActionFuture; +import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; @@ -86,6 +87,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -901,6 +903,90 @@ public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersion transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameVersionClusterState); } + // Test to not allow switching migration direction to remote store when remote cluster state is disabled + public void testFailValidateRemoteClusterStateEnabled() { + Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + + Settings remoteMigrationDirectionUpdateSettings = Settings.builder() + .put(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .build(); + + ClusterUpdateSettingsRequest remoteMigrationDirectionUpdateRequest = new ClusterUpdateSettingsRequest(); + remoteMigrationDirectionUpdateRequest.persistentSettings(remoteMigrationDirectionUpdateSettings); + + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + clusterSettings.applySettings( + (Settings.builder() + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), RemoteStoreNodeService.CompatibilityMode.MIXED) + .build()) + ); + + AllocationService allocationService = new AllocationService( + new AllocationDeciders(Collections.singleton(new MaxRetryAllocationDecider())), + new TestGatewayAllocator(), + new BalancedShardsAllocator(Settings.EMPTY), + EmptyClusterInfoService.INSTANCE, + EmptySnapshotsInfoService.INSTANCE + ); + TransportClusterUpdateSettingsAction transportClusterUpdateSettingsAction = new TransportClusterUpdateSettingsAction( + transportService, + clusterService, + threadPool, + allocationService, + new ActionFilters(Collections.emptySet()), + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + clusterSettings + ); + + // will expect exception when remote cluster state is not enabled + final SettingsException exception = expectThrows( + SettingsException.class, + () -> transportClusterUpdateSettingsAction.validateRemoteClusterStateEnabled(remoteMigrationDirectionUpdateRequest) + ); + assertEquals("can not switch migration direction to remote store when remote cluster state is not enabled", exception.getMessage()); + } + + public void testPassValidateRemoteClusterStateEnabled() { + Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + + Settings remoteMigrationDirectionUpdateSettings = Settings.builder() + .put(RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .build(); + + ClusterUpdateSettingsRequest remoteMigrationDirectionUpdateRequest = new ClusterUpdateSettingsRequest(); + remoteMigrationDirectionUpdateRequest.persistentSettings(remoteMigrationDirectionUpdateSettings); + + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + clusterSettings.applySettings( + (Settings.builder() + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), RemoteStoreNodeService.CompatibilityMode.MIXED) + .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .build()) + ); + + AllocationService allocationService = new AllocationService( + new AllocationDeciders(Collections.singleton(new MaxRetryAllocationDecider())), + new TestGatewayAllocator(), + new BalancedShardsAllocator(Settings.EMPTY), + EmptyClusterInfoService.INSTANCE, + EmptySnapshotsInfoService.INSTANCE + ); + TransportClusterUpdateSettingsAction transportClusterUpdateSettingsAction = new TransportClusterUpdateSettingsAction( + transportService, + clusterService, + threadPool, + allocationService, + new ActionFilters(Collections.emptySet()), + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + clusterSettings + ); + + // will allow switching migration direction to remoteStore, as remote store is enabled + transportClusterUpdateSettingsAction.validateRemoteClusterStateEnabled(remoteMigrationDirectionUpdateRequest); + } + private Map getRemoteStoreNodeAttributes() { Map remoteStoreNodeAttributes = new HashMap<>(); remoteStoreNodeAttributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1");