From 7b1cc3e164307c83243f3a3a8867afa8da0c023f Mon Sep 17 00:00:00 2001 From: Lakshya Taragi Date: Fri, 12 Apr 2024 17:07:37 +0530 Subject: [PATCH] Remove static Signed-off-by: Lakshya Taragi --- .../RemoteStoreMigrationSettingsUpdateIT.java | 32 +++++------ .../TransportClusterUpdateSettingsAction.java | 6 +-- .../opensearch/snapshots/RestoreService.java | 6 +-- ...ransportClusterManagerNodeActionTests.java | 53 ++++++++++++++++--- 4 files changed, 68 insertions(+), 29 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java index fbc75f58de405..c3720e6fbbd09 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotemigration/RemoteStoreMigrationSettingsUpdateIT.java @@ -16,9 +16,9 @@ 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.snapshots.SnapshotInfo; import org.opensearch.snapshots.SnapshotState; +import org.opensearch.test.InternalTestCluster; import org.opensearch.test.OpenSearchIntegTestCase; import java.nio.file.Path; @@ -143,21 +143,6 @@ public void testNewRestoredIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMix assertRemoteStoreBackedIndex(restoredIndexName2); } - // restore indices from a snapshot - private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) { - RestoreSnapshotResponse restoreSnapshotResponse = client.admin() - .cluster() - .prepareRestoreSnapshot(snapshotRepoName, snapshotName) - .setWaitForCompletion(false) - .setIndices(TEST_INDEX) - .setRenamePattern(TEST_INDEX) - .setRenameReplacement(restoredIndexName) - .get(); - - assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); - ensureGreen(restoredIndexName); - } - // compatibility mode setting test public void testSwitchToStrictMode() throws Exception { @@ -189,6 +174,21 @@ public void testSwitchToStrictMode() throws Exception { setClusterMode(STRICT.mode); } + // restore indices from a snapshot + private void restoreSnapshot(String snapshotRepoName, String snapshotName, String restoredIndexName) { + RestoreSnapshotResponse restoreSnapshotResponse = client.admin() + .cluster() + .prepareRestoreSnapshot(snapshotRepoName, snapshotName) + .setWaitForCompletion(false) + .setIndices(TEST_INDEX) + .setRenamePattern(TEST_INDEX) + .setRenameReplacement(restoredIndexName) + .get(); + + assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED); + ensureGreen(restoredIndexName); + } + // 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); 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 2d2e5ca696c00..e6c149216da09 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 @@ -277,7 +277,7 @@ public ClusterState execute(final ClusterState currentState) { * @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) { + 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); @@ -293,7 +293,7 @@ public static void validateCompatibilityModeSettingRequest(ClusterUpdateSettings * If not, it throws SettingsException error * @param discoveryNodes current discovery nodes in the cluster */ - private static void validateAllNodesOfSameVersion(DiscoveryNodes discoveryNodes) { + 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"); } @@ -304,7 +304,7 @@ private static void validateAllNodesOfSameVersion(DiscoveryNodes discoveryNodes) * same type (all remote or all non-remote). If not, it throws SettingsException error * @param discoveryNodes current discovery nodes in the cluster */ - private static void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) { + private void validateAllNodesOfSameType(DiscoveryNodes discoveryNodes) { Set nodeTypes = discoveryNodes.getNodes() .values() .stream() diff --git a/server/src/main/java/org/opensearch/snapshots/RestoreService.java b/server/src/main/java/org/opensearch/snapshots/RestoreService.java index b79a6a88250f8..e6a6b747c2baf 100644 --- a/server/src/main/java/org/opensearch/snapshots/RestoreService.java +++ b/server/src/main/java/org/opensearch/snapshots/RestoreService.java @@ -680,9 +680,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(), 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 29a1856344fcc..b3c58164fccbb 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 @@ -24,6 +24,7 @@ import org.opensearch.action.support.replication.ClusterStateCreationUtils; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.EmptyClusterInfoService; import org.opensearch.cluster.NotClusterManagerException; import org.opensearch.cluster.block.ClusterBlock; import org.opensearch.cluster.block.ClusterBlockException; @@ -35,6 +36,10 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.allocation.AllocationService; +import org.opensearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; +import org.opensearch.cluster.routing.allocation.decider.AllocationDeciders; +import org.opensearch.cluster.routing.allocation.decider.MaxRetryAllocationDecider; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.UUIDs; @@ -52,9 +57,11 @@ import org.opensearch.discovery.ClusterManagerNotDiscoveredException; import org.opensearch.node.NodeClosedException; import org.opensearch.node.remotestore.RemoteStoreNodeService; +import org.opensearch.snapshots.EmptySnapshotsInfoService; import org.opensearch.tasks.Task; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.gateway.TestGatewayAllocator; import org.opensearch.test.transport.CapturingTransport; import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; @@ -67,10 +74,10 @@ import java.io.IOException; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; -import java.util.Objects; import java.util.Map; -import java.util.HashMap; +import java.util.Objects; import java.util.Set; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CyclicBarrier; @@ -744,10 +751,26 @@ public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() { Metadata metadata = Metadata.builder().persistentSettings(currentCompatibilityModeSettings).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(metadata).nodes(discoveryNodes).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)), + clusterService.getClusterSettings() + ); final SettingsException exception = expectThrows( SettingsException.class, - () -> TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, clusterState) + () -> transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, clusterState) ); assertEquals( "can not switch to STRICT compatibility mode when the cluster contains both remote and non-remote nodes", @@ -771,7 +794,7 @@ public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() { .localNodeId(nonRemoteNode2.getId()) .build(); ClusterState sameTypeClusterState = ClusterState.builder(clusterState).nodes(discoveryNodes).build(); - TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState); + transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState); // cluster with only non-remote nodes discoveryNodes = DiscoveryNodes.builder() @@ -781,7 +804,7 @@ public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() { .localNodeId(remoteNode2.getId()) .build(); sameTypeClusterState = ClusterState.builder(sameTypeClusterState).nodes(discoveryNodes).build(); - TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState); + transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameTypeClusterState); } public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersions() { @@ -835,11 +858,27 @@ public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersion .metadata(metadata) .nodes(discoveryNodes) .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)), + clusterService.getClusterSettings() + ); // changing compatibility mode when all nodes are not of the same version final SettingsException exception = expectThrows( SettingsException.class, - () -> TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, differentVersionClusterState) + () -> transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, differentVersionClusterState) ); assertThat( exception.getMessage(), @@ -862,7 +901,7 @@ public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersion .build(); ClusterState sameVersionClusterState = ClusterState.builder(differentVersionClusterState).nodes(discoveryNodes).build(); - TransportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameVersionClusterState); + transportClusterUpdateSettingsAction.validateCompatibilityModeSettingRequest(request, sameVersionClusterState); } private Map getRemoteStoreNodeAttributes() {