From 334c59d64418ede9323fcc56b3a64cfa4462ae99 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 6 Apr 2022 16:10:29 -0700 Subject: [PATCH 01/13] Add unit tests back to test backwards compatitability of deprecated master role in node.roles setting Signed-off-by: Tianli Feng --- .../admin/cluster/stats/ClusterStatsIT.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java index 5b48268dfa89f..f2023ce48429a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java @@ -51,6 +51,7 @@ import org.hamcrest.Matchers; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -144,6 +145,25 @@ public void testNodeCounts() { } } + // + public void testNodeCountsWithDeprecatedMasterRole(){ + int total = 1; + Settings settings = Settings.builder() + .putList( + NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE.roleName()) + ) + .build(); + internalCluster().startNode(settings); + waitForNodes(total); + + Map expectedCounts = new HashMap<>(); + expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); + expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1); + + ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get(); + assertCounts(response.getNodesStats().getCounts(), total, expectedCounts); + } + private static void incrementCountForRole(String role, Map counts) { Integer count = counts.get(role); if (count == null) { From c57b6950104588fdd3e6acd2fd52c005e43660a9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 6 Apr 2022 16:23:42 -0700 Subject: [PATCH 02/13] Adjust format by spotlessApply task Signed-off-by: Tianli Feng --- DEVELOPER_GUIDE.md | 2 +- .../admin/cluster/stats/ClusterStatsIT.java | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9b1bc933eb1e3..ee3a6df5fd71b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -458,7 +458,7 @@ Work to make sure that OpenSearch can scale in a distributed manner. Includes: -- Nodes (Master, Data, Compute, Ingest, Discovery, etc.) +- Nodes (Cluster Manager, Data, Compute, Ingest, Discovery, etc.) - Replication & Merge Policies (Document, Segment level) - Snapshot/Restore (repositories; S3, Azure, GCP, NFS) - Translog (e.g., OpenSearch, Kafka, Kinesis) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java index f2023ce48429a..461981c87a103 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java @@ -145,25 +145,27 @@ public void testNodeCounts() { } } - // - public void testNodeCountsWithDeprecatedMasterRole(){ + // Validate assigning value "master" to setting "node.roles" can get correct count in Node Stats response after MASTER_ROLE deprecated. + public void testNodeCountsWithDeprecatedMasterRole() { int total = 1; Settings settings = Settings.builder() - .putList( - NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE.roleName()) - ) - .build(); + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE.roleName())) + .build(); internalCluster().startNode(settings); waitForNodes(total); - + Map expectedCounts = new HashMap<>(); + expectedCounts.put(DiscoveryNodeRole.DATA_ROLE.roleName(), 0); expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1); + expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), 0); + expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), 0); + expectedCounts.put(ClusterStatsNodes.Counts.COORDINATING_ONLY, 0); ClusterStatsResponse response = client().admin().cluster().prepareClusterStats().get(); assertCounts(response.getNodesStats().getCounts(), total, expectedCounts); } - + private static void incrementCountForRole(String role, Map counts) { Integer count = counts.get(role); if (count == null) { From 90cc89440d42f0d803449b694477b2faf00e0f26 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 6 Apr 2022 16:24:52 -0700 Subject: [PATCH 03/13] Revert unexpected change Signed-off-by: Tianli Feng --- DEVELOPER_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index ee3a6df5fd71b..9b1bc933eb1e3 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -458,7 +458,7 @@ Work to make sure that OpenSearch can scale in a distributed manner. Includes: -- Nodes (Cluster Manager, Data, Compute, Ingest, Discovery, etc.) +- Nodes (Master, Data, Compute, Ingest, Discovery, etc.) - Replication & Merge Policies (Document, Segment level) - Snapshot/Restore (repositories; S3, Azure, GCP, NFS) - Translog (e.g., OpenSearch, Kafka, Kinesis) From 69b5757c4cbcf5cb4635e449124e1718f42009b4 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 6 Apr 2022 17:09:33 -0700 Subject: [PATCH 04/13] Add unit test in add voting config exclusions request Signed-off-by: Tianli Feng --- ...AddVotingConfigExclusionsRequestTests.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java index a92e4e4a6c536..67b6aae9f9bf7 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java @@ -450,6 +450,70 @@ public void testResolveAndCheckMaximum() { assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } + /* + * As of 2.0, MASTER_ROLE is deprecated to promote inclusive language. + * Validate node with MASTER_ROLE can be resolved by resolveVotingConfigExclusions() like before. + */ + public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { + final DiscoveryNode localNode = new DiscoveryNode( + "local", + "local", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode); + + final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) + .nodes(new Builder().add(localNode).localNodeId(localNode.getId())) + .build(); + + assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState), contains(localNodeExclusion)); + assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); + } + + public void testResolveByNodeIdWithDeprecatedMasterRole() { + final DiscoveryNode node = new DiscoveryNode( + "nodeName", + "nodeId", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + final VotingConfigExclusion nodeExclusion = new VotingConfigExclusion(node); + + final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) + .nodes(new Builder().add(node)) + .build(); + + assertThat( + new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[] { "nodeId" }, Strings.EMPTY_ARRAY, TimeValue.ZERO).resolveVotingConfigExclusions(clusterState), contains(nodeExclusion) + ); + } + + public void testResolveByNodeNameWithDeprecatedMasterRole() { + final DiscoveryNode node = new DiscoveryNode( + "nodeName", + "nodeId", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + final VotingConfigExclusion nodeExclusion = new VotingConfigExclusion(node); + + final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) + .nodes(new Builder().add(node)) + .build(); + + assertThat( + new AddVotingConfigExclusionsRequest("nodeName").resolveVotingConfigExclusions(clusterState), + contains(nodeExclusion) + ); + } + private static AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) { return new AddVotingConfigExclusionsRequest( nodeDescriptions, From b15a558a512f5507d542aa86b153df0821f5d93b Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 7 Apr 2022 14:09:07 -0700 Subject: [PATCH 05/13] Add tests for master role in TransportActions Signed-off-by: Tianli Feng --- ...AddVotingConfigExclusionsRequestTests.java | 59 +- .../shrink/TransportResizeActionTests.java | 35 ++ .../TransportMultiSearchActionTests.java | 26 + ...ransportClusterManagerNodeActionTests.java | 528 ++++++++++++++++++ .../TransportMasterNodeActionTests.java | 4 +- 5 files changed, 618 insertions(+), 34 deletions(-) create mode 100644 server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java index 67b6aae9f9bf7..7c66379f0a16c 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java @@ -456,18 +456,18 @@ public void testResolveAndCheckMaximum() { */ public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { final DiscoveryNode localNode = new DiscoveryNode( - "local", - "local", - buildNewFakeTransportAddress(), - emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT + "local", + "local", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT ); final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode); final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) - .nodes(new Builder().add(localNode).localNodeId(localNode.getId())) - .build(); + .nodes(new Builder().add(localNode).localNodeId(localNode.getId())) + .build(); assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState), contains(localNodeExclusion)); assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); @@ -475,43 +475,38 @@ public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { public void testResolveByNodeIdWithDeprecatedMasterRole() { final DiscoveryNode node = new DiscoveryNode( - "nodeName", - "nodeId", - buildNewFakeTransportAddress(), - emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT + "nodeName", + "nodeId", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT ); final VotingConfigExclusion nodeExclusion = new VotingConfigExclusion(node); - final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) - .nodes(new Builder().add(node)) - .build(); + final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder().add(node)).build(); assertThat( - new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[] { "nodeId" }, Strings.EMPTY_ARRAY, TimeValue.ZERO).resolveVotingConfigExclusions(clusterState), contains(nodeExclusion) + new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[] { "nodeId" }, Strings.EMPTY_ARRAY, TimeValue.ZERO) + .resolveVotingConfigExclusions(clusterState), + contains(nodeExclusion) ); } - + public void testResolveByNodeNameWithDeprecatedMasterRole() { final DiscoveryNode node = new DiscoveryNode( - "nodeName", - "nodeId", - buildNewFakeTransportAddress(), - emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT + "nodeName", + "nodeId", + buildNewFakeTransportAddress(), + emptyMap(), + singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT ); final VotingConfigExclusion nodeExclusion = new VotingConfigExclusion(node); - final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) - .nodes(new Builder().add(node)) - .build(); + final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder().add(node)).build(); - assertThat( - new AddVotingConfigExclusionsRequest("nodeName").resolveVotingConfigExclusions(clusterState), - contains(nodeExclusion) - ); + assertThat(new AddVotingConfigExclusionsRequest("nodeName").resolveVotingConfigExclusions(clusterState), contains(nodeExclusion)); } private static AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) { diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index e4b79ac54f8fd..8f86d61891278 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -276,6 +276,41 @@ public void testShrinkIndexSettings() { assertEquals(request.waitForActiveShards(), activeShardCount); } + // Validate the Action works correctly on a node with deprecated 'master' role + public void testPassNumRoutingShardsOnNodeWithDeprecatedMasterRole() { + final Set roles = Set.of(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE); + + ClusterState clusterState = ClusterState.builder( + createClusterState("source", 1, 0, Settings.builder().put("index.blocks.write", true).build()) + ) + .nodes( + DiscoveryNodes.builder().add(new DiscoveryNode("node1", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT)) + ) + .build(); + AllocationService service = new AllocationService( + new AllocationDeciders(Collections.singleton(new MaxRetryAllocationDecider())), + new TestGatewayAllocator(), + new BalancedShardsAllocator(Settings.EMPTY), + EmptyClusterInfoService.INSTANCE, + EmptySnapshotsInfoService.INSTANCE + ); + + RoutingTable routingTable = service.reroute(clusterState, "reroute").routingTable(); + clusterState = ClusterState.builder(clusterState).routingTable(routingTable).build(); + // now we start the shard + routingTable = OpenSearchAllocationTestCase.startInitializingShardsAndReroute(service, clusterState, "source").routingTable(); + clusterState = ClusterState.builder(clusterState).routingTable(routingTable).build(); + + ResizeRequest resizeRequest = new ResizeRequest("target", "source"); + resizeRequest.setResizeType(ResizeType.SPLIT); + resizeRequest.getTargetIndexRequest().settings(Settings.builder().put("index.number_of_shards", 2).build()); + TransportResizeAction.prepareCreateIndexRequest(resizeRequest, clusterState, null, "source", "target"); + + resizeRequest.getTargetIndexRequest() + .settings(Settings.builder().put("index.number_of_routing_shards", 2).put("index.number_of_shards", 2).build()); + TransportResizeAction.prepareCreateIndexRequest(resizeRequest, clusterState, null, "source", "target"); + } + private DiscoveryNode newNode(String nodeId) { final Set roles = Collections.unmodifiableSet( new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) diff --git a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java index 09ab2438bd106..6cbac74948150 100644 --- a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java @@ -286,4 +286,30 @@ public void testDefaultMaxConcurrentSearches() { assertThat(result, equalTo(1)); } + // Validate the Action works correctly on a node with deprecated 'master' role + public void testDefaultMaxConcurrentSearchesOnNodeWithDeprecatedMasterRole() { + DiscoveryNodes.Builder builder = DiscoveryNodes.builder(); + builder.add( + new DiscoveryNode( + "master", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ) + ); + builder.add( + new DiscoveryNode( + "data", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.DATA_ROLE), + Version.CURRENT + ) + ); + + ClusterState state = ClusterState.builder(new ClusterName("_name")).nodes(builder).build(); + int result = TransportMultiSearchAction.defaultMaxConcurrentSearches(10, state); + assertThat(result, equalTo(10)); + } } diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java new file mode 100644 index 0000000000000..5e393d1335fea --- /dev/null +++ b/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java @@ -0,0 +1,528 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.action.support.master; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.opensearch.OpenSearchException; +import org.opensearch.Version; +import org.opensearch.action.ActionFuture; +import org.opensearch.action.ActionListener; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ActionResponse; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.PlainActionFuture; +import org.opensearch.action.support.ThreadedActionListener; +import org.opensearch.action.support.replication.ClusterStateCreationUtils; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.NotMasterException; +import org.opensearch.cluster.block.ClusterBlock; +import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.block.ClusterBlockLevel; +import org.opensearch.cluster.block.ClusterBlocks; +import org.opensearch.cluster.coordination.FailedToCommitClusterStateException; +import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodeRole; +import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.discovery.MasterNotDiscoveredException; +import org.opensearch.node.NodeClosedException; +import org.opensearch.rest.RestStatus; +import org.opensearch.tasks.Task; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.transport.CapturingTransport; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.ConnectTransportException; +import org.opensearch.transport.TransportService; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashSet; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.opensearch.test.ClusterServiceUtils.createClusterService; +import static org.opensearch.test.ClusterServiceUtils.setState; + +public class TransportClusterManagerNodeActionTests extends OpenSearchTestCase { + private static ThreadPool threadPool; + + private ClusterService clusterService; + private TransportService transportService; + private CapturingTransport transport; + private DiscoveryNode localNode; + private DiscoveryNode remoteNode; + private DiscoveryNode[] allNodes; + + @BeforeClass + public static void beforeClass() { + threadPool = new TestThreadPool("TransportMasterNodeActionTests"); + } + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + transport = new CapturingTransport(); + clusterService = createClusterService(threadPool); + transportService = transport.createTransportService( + clusterService.getSettings(), + threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> clusterService.localNode(), + null, + Collections.emptySet() + ); + transportService.start(); + transportService.acceptIncomingRequests(); + localNode = new DiscoveryNode( + "local_node", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Version.CURRENT + ); + remoteNode = new DiscoveryNode( + "remote_node", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Version.CURRENT + ); + allNodes = new DiscoveryNode[] { localNode, remoteNode }; + } + + @After + public void tearDown() throws Exception { + super.tearDown(); + clusterService.close(); + transportService.close(); + } + + @AfterClass + public static void afterClass() { + ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + threadPool = null; + } + + void assertListenerThrows(String msg, ActionFuture listener, Class klass) throws InterruptedException { + try { + listener.get(); + fail(msg); + } catch (ExecutionException ex) { + assertThat(ex.getCause(), instanceOf(klass)); + } + } + + public static class Request extends MasterNodeRequest { + Request() {} + + Request(StreamInput in) throws IOException { + super(in); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + } + + class Response extends ActionResponse { + private long identity = randomLong(); + + Response() {} + + Response(StreamInput in) throws IOException { + super(in); + identity = in.readLong(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Response response = (Response) o; + return identity == response.identity; + } + + @Override + public int hashCode() { + return Objects.hash(identity); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeLong(identity); + } + } + + class Action extends TransportMasterNodeAction { + Action(String actionName, TransportService transportService, ClusterService clusterService, ThreadPool threadPool) { + super( + actionName, + transportService, + clusterService, + threadPool, + new ActionFilters(new HashSet<>()), + Request::new, + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)) + ); + } + + @Override + protected void doExecute(Task task, final Request request, ActionListener listener) { + // remove unneeded threading by wrapping listener with SAME to prevent super.doExecute from wrapping it with LISTENER + super.doExecute(task, request, new ThreadedActionListener<>(logger, threadPool, ThreadPool.Names.SAME, listener, false)); + } + + @Override + protected String executor() { + // very lightweight operation in memory, no need to fork to a thread + return ThreadPool.Names.SAME; + } + + @Override + protected Response read(StreamInput in) throws IOException { + return new Response(in); + } + + @Override + protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { + listener.onResponse(new Response()); // default implementation, overridden in specific tests + } + + @Override + protected ClusterBlockException checkBlock(Request request, ClusterState state) { + return null; // default implementation, overridden in specific tests + } + } + + public void testLocalOperationWithoutBlocks() throws ExecutionException, InterruptedException { + final boolean masterOperationFailure = randomBoolean(); + + Request request = new Request(); + PlainActionFuture listener = new PlainActionFuture<>(); + + final Exception exception = new Exception(); + final Response response = new Response(); + + setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); + + new Action("internal:testAction", transportService, clusterService, threadPool) { + @Override + protected void masterOperation(Task task, Request request, ClusterState state, ActionListener listener) { + if (masterOperationFailure) { + listener.onFailure(exception); + } else { + listener.onResponse(response); + } + } + }.execute(request, listener); + assertTrue(listener.isDone()); + + if (masterOperationFailure) { + try { + listener.get(); + fail("Expected exception but returned proper result"); + } catch (ExecutionException ex) { + assertThat(ex.getCause(), equalTo(exception)); + } + } else { + assertThat(listener.get(), equalTo(response)); + } + } + + public void testLocalOperationWithBlocks() throws ExecutionException, InterruptedException { + final boolean retryableBlock = randomBoolean(); + final boolean unblockBeforeTimeout = randomBoolean(); + + Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(unblockBeforeTimeout ? 60 : 0)); + PlainActionFuture listener = new PlainActionFuture<>(); + + ClusterBlock block = new ClusterBlock(1, "", retryableBlock, true, false, randomFrom(RestStatus.values()), ClusterBlockLevel.ALL); + ClusterState stateWithBlock = ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) + .blocks(ClusterBlocks.builder().addGlobalBlock(block)) + .build(); + setState(clusterService, stateWithBlock); + + new Action("internal:testAction", transportService, clusterService, threadPool) { + @Override + protected ClusterBlockException checkBlock(Request request, ClusterState state) { + Set blocks = state.blocks().global(); + return blocks.isEmpty() ? null : new ClusterBlockException(blocks); + } + }.execute(request, listener); + + if (retryableBlock && unblockBeforeTimeout) { + assertFalse(listener.isDone()); + setState( + clusterService, + ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) + .blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK) + .build() + ); + assertTrue(listener.isDone()); + listener.get(); + return; + } + + assertTrue(listener.isDone()); + if (retryableBlock) { + try { + listener.get(); + fail("Expected exception but returned proper result"); + } catch (ExecutionException ex) { + assertThat(ex.getCause(), instanceOf(MasterNotDiscoveredException.class)); + assertThat(ex.getCause().getCause(), instanceOf(ClusterBlockException.class)); + } + } else { + assertListenerThrows("ClusterBlockException should be thrown", listener, ClusterBlockException.class); + } + } + + public void testCheckBlockThrowsException() throws InterruptedException { + boolean throwExceptionOnRetry = randomBoolean(); + Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(60)); + PlainActionFuture listener = new PlainActionFuture<>(); + + ClusterBlock block = new ClusterBlock(1, "", true, true, false, randomFrom(RestStatus.values()), ClusterBlockLevel.ALL); + ClusterState stateWithBlock = ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) + .blocks(ClusterBlocks.builder().addGlobalBlock(block)) + .build(); + setState(clusterService, stateWithBlock); + + new Action("internal:testAction", transportService, clusterService, threadPool) { + @Override + protected ClusterBlockException checkBlock(Request request, ClusterState state) { + Set blocks = state.blocks().global(); + if (throwExceptionOnRetry == false || blocks.isEmpty()) { + throw new RuntimeException("checkBlock has thrown exception"); + } + return new ClusterBlockException(blocks); + + } + }.execute(request, listener); + + if (throwExceptionOnRetry == false) { + assertListenerThrows("checkBlock has thrown exception", listener, RuntimeException.class); + } else { + assertFalse(listener.isDone()); + setState( + clusterService, + ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) + .blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK) + .build() + ); + assertListenerThrows("checkBlock has thrown exception", listener, RuntimeException.class); + } + } + + public void testForceLocalOperation() throws ExecutionException, InterruptedException { + Request request = new Request(); + PlainActionFuture listener = new PlainActionFuture<>(); + + setState(clusterService, ClusterStateCreationUtils.state(localNode, randomFrom(localNode, remoteNode, null), allNodes)); + + new Action("internal:testAction", transportService, clusterService, threadPool) { + @Override + protected boolean localExecute(Request request) { + return true; + } + }.execute(request, listener); + + assertTrue(listener.isDone()); + listener.get(); + } + + public void testMasterNotAvailable() throws ExecutionException, InterruptedException { + Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(0)); + setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); + PlainActionFuture listener = new PlainActionFuture<>(); + new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); + assertTrue(listener.isDone()); + assertListenerThrows("MasterNotDiscoveredException should be thrown", listener, MasterNotDiscoveredException.class); + } + + public void testMasterBecomesAvailable() throws ExecutionException, InterruptedException { + Request request = new Request(); + setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); + PlainActionFuture listener = new PlainActionFuture<>(); + new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); + assertFalse(listener.isDone()); + setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); + assertTrue(listener.isDone()); + listener.get(); + } + + public void testDelegateToMaster() throws ExecutionException, InterruptedException { + Request request = new Request(); + setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); + + PlainActionFuture listener = new PlainActionFuture<>(); + new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); + + assertThat(transport.capturedRequests().length, equalTo(1)); + CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; + assertTrue(capturedRequest.node.isMasterNode()); + assertThat(capturedRequest.request, equalTo(request)); + assertThat(capturedRequest.action, equalTo("internal:testAction")); + + Response response = new Response(); + transport.handleResponse(capturedRequest.requestId, response); + assertTrue(listener.isDone()); + assertThat(listener.get(), equalTo(response)); + } + + public void testDelegateToFailingMaster() throws ExecutionException, InterruptedException { + boolean failsWithConnectTransportException = randomBoolean(); + boolean rejoinSameMaster = failsWithConnectTransportException && randomBoolean(); + Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(failsWithConnectTransportException ? 60 : 0)); + DiscoveryNode masterNode = this.remoteNode; + setState( + clusterService, + // use a random base version so it can go down when simulating a restart. + ClusterState.builder(ClusterStateCreationUtils.state(localNode, masterNode, allNodes)).version(randomIntBetween(0, 10)) + ); + + PlainActionFuture listener = new PlainActionFuture<>(); + new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); + + CapturingTransport.CapturedRequest[] capturedRequests = transport.getCapturedRequestsAndClear(); + assertThat(capturedRequests.length, equalTo(1)); + CapturingTransport.CapturedRequest capturedRequest = capturedRequests[0]; + assertTrue(capturedRequest.node.isMasterNode()); + assertThat(capturedRequest.request, equalTo(request)); + assertThat(capturedRequest.action, equalTo("internal:testAction")); + + if (rejoinSameMaster) { + transport.handleRemoteError( + capturedRequest.requestId, + randomBoolean() ? new ConnectTransportException(masterNode, "Fake error") : new NodeClosedException(masterNode) + ); + assertFalse(listener.isDone()); + if (randomBoolean()) { + // simulate master node removal + final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); + nodesBuilder.masterNodeId(null); + setState(clusterService, ClusterState.builder(clusterService.state()).nodes(nodesBuilder)); + } + if (randomBoolean()) { + // reset the same state to increment a version simulating a join of an existing node + // simulating use being disconnected + final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); + nodesBuilder.masterNodeId(masterNode.getId()); + setState(clusterService, ClusterState.builder(clusterService.state()).nodes(nodesBuilder)); + } else { + // simulate master restart followed by a state recovery - this will reset the cluster state version + final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); + nodesBuilder.remove(masterNode); + masterNode = new DiscoveryNode(masterNode.getId(), masterNode.getAddress(), masterNode.getVersion()); + nodesBuilder.add(masterNode); + nodesBuilder.masterNodeId(masterNode.getId()); + final ClusterState.Builder builder = ClusterState.builder(clusterService.state()).nodes(nodesBuilder); + setState(clusterService, builder.version(0)); + } + assertFalse(listener.isDone()); + capturedRequests = transport.getCapturedRequestsAndClear(); + assertThat(capturedRequests.length, equalTo(1)); + capturedRequest = capturedRequests[0]; + assertTrue(capturedRequest.node.isMasterNode()); + assertThat(capturedRequest.request, equalTo(request)); + assertThat(capturedRequest.action, equalTo("internal:testAction")); + } else if (failsWithConnectTransportException) { + transport.handleRemoteError(capturedRequest.requestId, new ConnectTransportException(masterNode, "Fake error")); + assertFalse(listener.isDone()); + setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); + assertTrue(listener.isDone()); + listener.get(); + } else { + OpenSearchException t = new OpenSearchException("test"); + t.addHeader("header", "is here"); + transport.handleRemoteError(capturedRequest.requestId, t); + assertTrue(listener.isDone()); + try { + listener.get(); + fail("Expected exception but returned proper result"); + } catch (ExecutionException ex) { + final Throwable cause = ex.getCause().getCause(); + assertThat(cause, instanceOf(OpenSearchException.class)); + final OpenSearchException es = (OpenSearchException) cause; + assertThat(es.getMessage(), equalTo(t.getMessage())); + assertThat(es.getHeader("header"), equalTo(t.getHeader("header"))); + } + } + } + + public void testMasterFailoverAfterStepDown() throws ExecutionException, InterruptedException { + Request request = new Request().masterNodeTimeout(TimeValue.timeValueHours(1)); + PlainActionFuture listener = new PlainActionFuture<>(); + + final Response response = new Response(); + + setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); + + new Action("internal:testAction", transportService, clusterService, threadPool) { + @Override + protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { + // The other node has become master, simulate failures of this node while publishing cluster state through ZenDiscovery + setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); + Exception failure = randomBoolean() + ? new FailedToCommitClusterStateException("Fake error") + : new NotMasterException("Fake error"); + listener.onFailure(failure); + } + }.execute(request, listener); + + assertThat(transport.capturedRequests().length, equalTo(1)); + CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; + assertTrue(capturedRequest.node.isMasterNode()); + assertThat(capturedRequest.request, equalTo(request)); + assertThat(capturedRequest.action, equalTo("internal:testAction")); + + transport.handleResponse(capturedRequest.requestId, response); + assertTrue(listener.isDone()); + assertThat(listener.get(), equalTo(response)); + } +} diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java index a8ad356e947b5..60dcbc15b96c8 100644 --- a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java @@ -121,14 +121,14 @@ public void setUp() throws Exception { "local_node", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), Version.CURRENT ); remoteNode = new DiscoveryNode( "remote_node", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), Version.CURRENT ); allNodes = new DiscoveryNode[] { localNode, remoteNode }; From 1a6e3f3f75d9596abf7bcbd4e474f4c382e11504 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 7 Apr 2022 14:48:25 -0700 Subject: [PATCH 06/13] Add tests for master role in ClusterChangedEventTests Signed-off-by: Tianli Feng --- .../cluster/ClusterChangedEventTests.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java index 49d4a8baf0a1c..7b0ed160b3fb3 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java @@ -314,6 +314,36 @@ public void testChangedCustomMetadataSet() { assertTrue(changedCustomMetadataTypeSet.contains(customMetadata1.getWriteableName())); } + // Validate the above test case testLocalNodeIsMaster() passes when the deprecated 'master' role is assigned to the local node. + // TODO: Remove the test after removing MASTER_ROLE. + public void testLocalNodeIsMasterWithDeprecatedMasterRole() { + final DiscoveryNodes.Builder builderLocalIsMaster = DiscoveryNodes.builder(); + final DiscoveryNode node0 = newNode("node_0", Set.of(DiscoveryNodeRole.MASTER_ROLE)); + final DiscoveryNode node1 = newNode("node_1", Set.of(DiscoveryNodeRole.DATA_ROLE)); + builderLocalIsMaster.add(node0).add(node1).masterNodeId(node0.getId()).localNodeId(node0.getId()); + + final DiscoveryNodes.Builder builderLocalNotMaster = DiscoveryNodes.builder(); + builderLocalNotMaster.add(node0).add(node1).masterNodeId(node0.getId()).localNodeId(node1.getId()); + + ClusterState previousState = createSimpleClusterState(); + final Metadata metadata = createMetadata(initialIndices); + ClusterState newState = ClusterState.builder(TEST_CLUSTER_NAME) + .nodes(builderLocalIsMaster.build()) + .metadata(metadata) + .routingTable(createRoutingTable(1, metadata)) + .build(); + ClusterChangedEvent event = new ClusterChangedEvent("_na_", newState, previousState); + assertTrue("local node should be master", event.localNodeMaster()); + + newState = ClusterState.builder(TEST_CLUSTER_NAME) + .nodes(builderLocalNotMaster.build()) + .metadata(metadata) + .routingTable(createRoutingTable(1, metadata)) + .build(); + event = new ClusterChangedEvent("_na_", newState, previousState); + assertFalse("local node should not be master", event.localNodeMaster()); + } + private static class CustomMetadata2 extends TestCustomMetadata { protected CustomMetadata2(String data) { super(data); From f9eefb73672f5dae513eb586fe35f3e4a5de766a Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 7 Apr 2022 17:56:06 -0700 Subject: [PATCH 07/13] Add tests for master role in ClusterBootsteapServiceTests Signed-off-by: Tianli Feng --- ...BootstrapServiceDeprecatedMasterTests.java | 674 ++++++++++++++++++ ...erBootstrapServiceRenamedSettingTests.java | 2 +- .../ClusterBootstrapServiceTests.java | 27 - 3 files changed, 675 insertions(+), 28 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java new file mode 100644 index 0000000000000..0bc675206e049 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java @@ -0,0 +1,674 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/* + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.cluster.coordination; + +import org.junit.Before; +import org.opensearch.OpenSearchException; +import org.opensearch.Version; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodeRole; +import org.opensearch.common.settings.Settings; +import org.opensearch.discovery.DiscoveryModule; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.transport.MockTransport; +import org.opensearch.transport.TransportRequest; +import org.opensearch.transport.TransportService; + +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; +import static java.util.Collections.singletonList; +import static org.opensearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; +import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; +import static org.opensearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; +import static org.opensearch.common.settings.Settings.builder; +import static org.opensearch.node.Node.NODE_NAME_SETTING; +import static org.opensearch.test.NodeRoles.nonMasterNode; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; + +public class ClusterBootstrapServiceDeprecatedMasterTests extends OpenSearchTestCase { + + private DiscoveryNode localNode, otherNode1, otherNode2; + private DeterministicTaskQueue deterministicTaskQueue; + private TransportService transportService; + private static final String CLUSTER_SETTING_DEPRECATED_MESSAGE = + "[cluster.initial_master_nodes] setting was deprecated in OpenSearch and will be removed in a future release! " + + "See the breaking changes documentation for the next major version."; + + @Before + public void createServices() { + localNode = newDiscoveryNode("local"); + otherNode1 = newDiscoveryNode("other1"); + otherNode2 = newDiscoveryNode("other2"); + + deterministicTaskQueue = new DeterministicTaskQueue(builder().put(NODE_NAME_SETTING.getKey(), "node").build(), random()); + + final MockTransport transport = new MockTransport() { + @Override + protected void onSendRequest(long requestId, String action, TransportRequest request, DiscoveryNode node) { + throw new AssertionError("unexpected " + action); + } + }; + + transportService = transport.createTransportService( + Settings.EMPTY, + deterministicTaskQueue.getThreadPool(), + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + boundTransportAddress -> localNode, + null, + Collections.emptySet() + ); + } + + private DiscoveryNode newDiscoveryNode(String nodeName) { + return new DiscoveryNode( + nodeName, + randomAlphaOfLength(10), + buildNewFakeTransportAddress(), + emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + } + + public void testBootstrapsAutomaticallyWithDefaultConfiguration() { + final Settings.Builder settings = Settings.builder(); + final long timeout; + if (randomBoolean()) { + timeout = UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(Settings.EMPTY).millis(); + } else { + timeout = randomLongBetween(1, 10000); + settings.put(UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.getKey(), timeout + "ms"); + } + + final AtomicReference>> discoveredNodesSupplier = new AtomicReference<>( + () -> { throw new AssertionError("should not be called yet"); } + ); + + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + settings.build(), + transportService, + () -> discoveredNodesSupplier.get().get(), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat( + vc.getNodeIds(), + equalTo(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toSet())) + ); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(timeout)); + } + ); + + deterministicTaskQueue.scheduleAt( + timeout - 1, + () -> discoveredNodesSupplier.set(() -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toSet())) + ); + + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + deterministicTaskQueue.runAllTasksInTimeOrder(); + assertTrue(bootstrapped.get()); + } + + // Validate the deprecated setting is still valid during the cluster bootstrap. + public void testDoesNothingByDefaultIfMasterNodesConfigured() { + testDoesNothingWithSettings(builder().putList(INITIAL_MASTER_NODES_SETTING.getKey())); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + } + + private void testDoesNothingWithSettings(Settings.Builder builder) { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + builder.build(), + transportService, + () -> { throw new AssertionError("should not be called"); }, + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + deterministicTaskQueue.runAllTasks(); + } + + public void testThrowsExceptionOnDuplicates() { + final IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, () -> { + new ClusterBootstrapService( + builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), "duplicate-requirement", "duplicate-requirement").build(), + transportService, + Collections::emptyList, + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + }); + + assertThat(illegalArgumentException.getMessage(), containsString(INITIAL_MASTER_NODES_SETTING.getKey())); + assertThat(illegalArgumentException.getMessage(), containsString("duplicate-requirement")); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + } + + public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), containsInAnyOrder(localNode.getId(), otherNode1.getId(), otherNode2.getId())); + assertThat(vc.getNodeIds(), not(hasItem(containsString("placeholder")))); + } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testBootstrapsOnDiscoveryOfTwoOfThreeRequiredNodes() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> singletonList(otherNode1), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), hasSize(3)); + assertThat(vc.getNodeIds(), hasItem(localNode.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); + assertThat(vc.getNodeIds(), hasItem(allOf(startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX), containsString(otherNode2.getName())))); + assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + assertFalse(vc.hasQuorum(singletonList(localNode.getId()))); + assertFalse(vc.hasQuorum(singletonList(otherNode1.getId()))); + } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testBootstrapsOnDiscoveryOfThreeOfFiveRequiredNodes() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList( + INITIAL_MASTER_NODES_SETTING.getKey(), + localNode.getName(), + otherNode1.getName(), + otherNode2.getName(), + "missing-node-1", + "missing-node-2" + ) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), hasSize(5)); + assertThat(vc.getNodeIds(), hasItem(localNode.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); + assertThat(vc.getNodeIds(), hasItem(otherNode2.getId())); + + final List placeholders = vc.getNodeIds() + .stream() + .filter(ClusterBootstrapService::isBootstrapPlaceholder) + .collect(Collectors.toList()); + assertThat(placeholders.size(), equalTo(2)); + assertNotEquals(placeholders.get(0), placeholders.get(1)); + assertThat(placeholders, hasItem(containsString("missing-node-1"))); + assertThat(placeholders, hasItem(containsString("missing-node-2"))); + + assertTrue( + vc.hasQuorum(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toList())) + ); + assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); + } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testDoesNotBootstrapIfNoNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + Collections::emptyList, + () -> true, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList( + INITIAL_MASTER_NODES_SETTING.getKey(), + localNode.getName(), + otherNode1.getName(), + otherNode2.getName(), + "not-a-node-1", + "not-a-node-2" + ) + .build(), + transportService, + () -> Stream.of(otherNode1).collect(Collectors.toList()), + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapIfThreeOfSixNodesDiscovered() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList( + INITIAL_MASTER_NODES_SETTING.getKey(), + localNode.getName(), + otherNode1.getName(), + otherNode2.getName(), + "not-a-node-1", + "not-a-node-2", + "not-a-node-3" + ) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapIfAlreadyBootstrapped() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), + () -> true, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapsOnNonMasterNode() { + localNode = new DiscoveryNode( + "local", + randomAlphaOfLength(10), + buildNewFakeTransportAddress(), + emptyMap(), + emptySet(), + Version.CURRENT + ); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapsIfLocalNodeNotInInitialClusterManagerNodes() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getName(), otherNode2.getName()).build(), + transportService, + () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testDoesNotBootstrapsIfNotConfigured() { + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey()).build(), + transportService, + () -> Stream.of(localNode, otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + transportService.start(); + clusterBootstrapService.scheduleUnconfiguredBootstrap(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testRetriesBootstrappingOnException() { + final AtomicLong bootstrappingAttempts = new AtomicLong(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), + () -> false, + vc -> { + bootstrappingAttempts.incrementAndGet(); + if (bootstrappingAttempts.get() < 5L) { + throw new OpenSearchException("test"); + } + } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertThat(bootstrappingAttempts.get(), greaterThanOrEqualTo(5L)); + assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); + } + + public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { + AtomicReference> discoveredNodes = new AtomicReference<>( + Stream.of(otherNode1, otherNode2).collect(Collectors.toList()) + ); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), + transportService, + discoveredNodes::get, + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + + discoveredNodes.set(emptyList()); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { + AtomicReference> discoveredNodes = new AtomicReference<>( + Stream.of(otherNode1, otherNode2).collect(Collectors.toList()) + ); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) + .build(), + transportService, + discoveredNodes::get, + () -> false, + vc -> { throw new AssertionError("should not be called"); } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + + discoveredNodes.set( + Stream.of( + new DiscoveryNode( + otherNode1.getName(), + randomAlphaOfLength(10), + buildNewFakeTransportAddress(), + emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ), + new DiscoveryNode( + "yet-another-node", + randomAlphaOfLength(10), + otherNode1.getAddress(), + emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ) + ).collect(Collectors.toList()) + ); + + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + } + + public void testMatchesOnNodeName() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), + transportService, + Collections::emptyList, + () -> false, + vc -> assertTrue(bootstrapped.compareAndSet(false, true)) + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testMatchesOnNodeAddress() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), + transportService, + Collections::emptyList, + () -> false, + vc -> assertTrue(bootstrapped.compareAndSet(false, true)) + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testMatchesOnNodeHostAddress() { + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), + transportService, + Collections::emptyList, + () -> false, + vc -> assertTrue(bootstrapped.compareAndSet(false, true)) + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testDoesNotIncludeExtraNodes() { + final DiscoveryNode extraNode = newDiscoveryNode("extra-node"); + final AtomicBoolean bootstrapped = new AtomicBoolean(); + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + Settings.builder() + .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) + .build(), + transportService, + () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); + } + ); + assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + } + + public void testBootstrapsAutomaticallyWithSingleNodeDiscovery() { + final Settings.Builder settings = Settings.builder() + .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) + .put(NODE_NAME_SETTING.getKey(), localNode.getName()); + final AtomicBoolean bootstrapped = new AtomicBoolean(); + + ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( + settings.build(), + transportService, + () -> emptyList(), + () -> false, + vc -> { + assertTrue(bootstrapped.compareAndSet(false, true)); + assertThat(vc.getNodeIds(), hasSize(1)); + assertThat(vc.getNodeIds(), hasItem(localNode.getId())); + assertTrue(vc.hasQuorum(singletonList(localNode.getId()))); + } + ); + + transportService.start(); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertTrue(bootstrapped.get()); + + bootstrapped.set(false); + clusterBootstrapService.onFoundPeersUpdated(); + deterministicTaskQueue.runAllTasks(); + assertFalse(bootstrapped.get()); // should only bootstrap once + } + + public void testFailBootstrapWithBothSingleNodeDiscoveryAndInitialMasterNodes() { + final Settings.Builder settings = Settings.builder() + .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) + .put(NODE_NAME_SETTING.getKey(), localNode.getName()) + .put(INITIAL_MASTER_NODES_SETTING.getKey(), "test"); + + assertThat( + expectThrows( + IllegalArgumentException.class, + () -> new ClusterBootstrapService(settings.build(), transportService, () -> emptyList(), () -> false, vc -> fail()) + ).getMessage(), + containsString( + "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] is not allowed when [discovery.type] is set " + "to [single-node]" + ) + ); + } + + public void testFailBootstrapNonMasterEligibleNodeWithSingleNodeDiscovery() { + final Settings.Builder settings = Settings.builder() + .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) + .put(NODE_NAME_SETTING.getKey(), localNode.getName()) + .put(nonMasterNode()); + + assertThat( + expectThrows( + IllegalArgumentException.class, + () -> new ClusterBootstrapService(settings.build(), transportService, () -> emptyList(), () -> false, vc -> fail()) + ).getMessage(), + containsString("node with [discovery.type] set to [single-node] must be cluster-manager-eligible") + ); + } +} diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceRenamedSettingTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceRenamedSettingTests.java index 09fb6a5ea256d..f95e50a22cf5a 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceRenamedSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceRenamedSettingTests.java @@ -25,7 +25,7 @@ public class ClusterBootstrapServiceRenamedSettingTests extends OpenSearchTestCa /** * Validate the both settings are known and supported. */ - public void testReindexSettingsExist() { + public void testClusterBootstrapServiceSettingsExist() { Set> settings = ClusterSettings.BUILT_IN_CLUSTER_SETTINGS; assertTrue( "Both 'cluster.initial_cluster_manager_nodes' and its predecessor should be supported built-in settings.", diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java index 3e4148cef61cd..812bf9425968a 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -58,7 +58,6 @@ import static java.util.Collections.singletonList; import static org.opensearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING; -import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.opensearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; import static org.opensearch.common.settings.Settings.builder; import static org.opensearch.discovery.DiscoveryModule.DISCOVERY_SEED_PROVIDERS_SETTING; @@ -170,15 +169,6 @@ public void testDoesNothingByDefaultIfClusterManagerNodesConfigured() { testDoesNothingWithSettings(builder().putList(INITIAL_CLUSTER_MANAGER_NODES_SETTING.getKey())); } - // Validate the deprecated setting is still valid during the cluster bootstrap. - public void testDoesNothingByDefaultIfMasterNodesConfigured() { - testDoesNothingWithSettings(builder().putList(INITIAL_MASTER_NODES_SETTING.getKey())); - assertWarnings( - "[cluster.initial_master_nodes] setting was deprecated in OpenSearch and will be removed in a future release! " - + "See the breaking changes documentation for the next major version." - ); - } - public void testDoesNothingByDefaultOnMasterIneligibleNodes() { localNode = new DiscoveryNode( "local", @@ -663,23 +653,6 @@ public void testBootstrapsAutomaticallyWithSingleNodeDiscovery() { assertFalse(bootstrapped.get()); // should only bootstrap once } - public void testFailBootstrapWithBothSingleNodeDiscoveryAndInitialMasterNodes() { - final Settings.Builder settings = Settings.builder() - .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) - .put(NODE_NAME_SETTING.getKey(), localNode.getName()) - .put(INITIAL_MASTER_NODES_SETTING.getKey(), "test"); - - assertThat( - expectThrows( - IllegalArgumentException.class, - () -> new ClusterBootstrapService(settings.build(), transportService, () -> emptyList(), () -> false, vc -> fail()) - ).getMessage(), - containsString( - "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] is not allowed when [discovery.type] is set " + "to [single-node]" - ) - ); - } - /** * Validate the correct setting name of cluster.initial_cluster_manager_nodes is shown in the exception, * when discovery type is single-node. From e5d730af28e83a88351348e232158606419ca274 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 8 Apr 2022 16:28:19 -0700 Subject: [PATCH 08/13] Add tests for master role in TransportMasterNodeAction Signed-off-by: Tianli Feng --- ...ransportClusterManagerNodeActionTests.java | 528 ------------------ .../TransportMasterNodeActionTests.java | 43 +- 2 files changed, 41 insertions(+), 530 deletions(-) delete mode 100644 server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java deleted file mode 100644 index 5e393d1335fea..0000000000000 --- a/server/src/test/java/org/opensearch/action/support/master/TransportClusterManagerNodeActionTests.java +++ /dev/null @@ -1,528 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.action.support.master; - -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.opensearch.OpenSearchException; -import org.opensearch.Version; -import org.opensearch.action.ActionFuture; -import org.opensearch.action.ActionListener; -import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.action.ActionResponse; -import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.PlainActionFuture; -import org.opensearch.action.support.ThreadedActionListener; -import org.opensearch.action.support.replication.ClusterStateCreationUtils; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.NotMasterException; -import org.opensearch.cluster.block.ClusterBlock; -import org.opensearch.cluster.block.ClusterBlockException; -import org.opensearch.cluster.block.ClusterBlockLevel; -import org.opensearch.cluster.block.ClusterBlocks; -import org.opensearch.cluster.coordination.FailedToCommitClusterStateException; -import org.opensearch.cluster.metadata.IndexNameExpressionResolver; -import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.node.DiscoveryNodeRole; -import org.opensearch.cluster.node.DiscoveryNodes; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.discovery.MasterNotDiscoveredException; -import org.opensearch.node.NodeClosedException; -import org.opensearch.rest.RestStatus; -import org.opensearch.tasks.Task; -import org.opensearch.test.OpenSearchTestCase; -import org.opensearch.test.transport.CapturingTransport; -import org.opensearch.threadpool.TestThreadPool; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.ConnectTransportException; -import org.opensearch.transport.TransportService; - -import java.io.IOException; -import java.util.Collections; -import java.util.HashSet; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; - -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.opensearch.test.ClusterServiceUtils.createClusterService; -import static org.opensearch.test.ClusterServiceUtils.setState; - -public class TransportClusterManagerNodeActionTests extends OpenSearchTestCase { - private static ThreadPool threadPool; - - private ClusterService clusterService; - private TransportService transportService; - private CapturingTransport transport; - private DiscoveryNode localNode; - private DiscoveryNode remoteNode; - private DiscoveryNode[] allNodes; - - @BeforeClass - public static void beforeClass() { - threadPool = new TestThreadPool("TransportMasterNodeActionTests"); - } - - @Override - @Before - public void setUp() throws Exception { - super.setUp(); - transport = new CapturingTransport(); - clusterService = createClusterService(threadPool); - transportService = transport.createTransportService( - clusterService.getSettings(), - threadPool, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> clusterService.localNode(), - null, - Collections.emptySet() - ); - transportService.start(); - transportService.acceptIncomingRequests(); - localNode = new DiscoveryNode( - "local_node", - buildNewFakeTransportAddress(), - Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), - Version.CURRENT - ); - remoteNode = new DiscoveryNode( - "remote_node", - buildNewFakeTransportAddress(), - Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), - Version.CURRENT - ); - allNodes = new DiscoveryNode[] { localNode, remoteNode }; - } - - @After - public void tearDown() throws Exception { - super.tearDown(); - clusterService.close(); - transportService.close(); - } - - @AfterClass - public static void afterClass() { - ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); - threadPool = null; - } - - void assertListenerThrows(String msg, ActionFuture listener, Class klass) throws InterruptedException { - try { - listener.get(); - fail(msg); - } catch (ExecutionException ex) { - assertThat(ex.getCause(), instanceOf(klass)); - } - } - - public static class Request extends MasterNodeRequest { - Request() {} - - Request(StreamInput in) throws IOException { - super(in); - } - - @Override - public ActionRequestValidationException validate() { - return null; - } - } - - class Response extends ActionResponse { - private long identity = randomLong(); - - Response() {} - - Response(StreamInput in) throws IOException { - super(in); - identity = in.readLong(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Response response = (Response) o; - return identity == response.identity; - } - - @Override - public int hashCode() { - return Objects.hash(identity); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeLong(identity); - } - } - - class Action extends TransportMasterNodeAction { - Action(String actionName, TransportService transportService, ClusterService clusterService, ThreadPool threadPool) { - super( - actionName, - transportService, - clusterService, - threadPool, - new ActionFilters(new HashSet<>()), - Request::new, - new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)) - ); - } - - @Override - protected void doExecute(Task task, final Request request, ActionListener listener) { - // remove unneeded threading by wrapping listener with SAME to prevent super.doExecute from wrapping it with LISTENER - super.doExecute(task, request, new ThreadedActionListener<>(logger, threadPool, ThreadPool.Names.SAME, listener, false)); - } - - @Override - protected String executor() { - // very lightweight operation in memory, no need to fork to a thread - return ThreadPool.Names.SAME; - } - - @Override - protected Response read(StreamInput in) throws IOException { - return new Response(in); - } - - @Override - protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { - listener.onResponse(new Response()); // default implementation, overridden in specific tests - } - - @Override - protected ClusterBlockException checkBlock(Request request, ClusterState state) { - return null; // default implementation, overridden in specific tests - } - } - - public void testLocalOperationWithoutBlocks() throws ExecutionException, InterruptedException { - final boolean masterOperationFailure = randomBoolean(); - - Request request = new Request(); - PlainActionFuture listener = new PlainActionFuture<>(); - - final Exception exception = new Exception(); - final Response response = new Response(); - - setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - - new Action("internal:testAction", transportService, clusterService, threadPool) { - @Override - protected void masterOperation(Task task, Request request, ClusterState state, ActionListener listener) { - if (masterOperationFailure) { - listener.onFailure(exception); - } else { - listener.onResponse(response); - } - } - }.execute(request, listener); - assertTrue(listener.isDone()); - - if (masterOperationFailure) { - try { - listener.get(); - fail("Expected exception but returned proper result"); - } catch (ExecutionException ex) { - assertThat(ex.getCause(), equalTo(exception)); - } - } else { - assertThat(listener.get(), equalTo(response)); - } - } - - public void testLocalOperationWithBlocks() throws ExecutionException, InterruptedException { - final boolean retryableBlock = randomBoolean(); - final boolean unblockBeforeTimeout = randomBoolean(); - - Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(unblockBeforeTimeout ? 60 : 0)); - PlainActionFuture listener = new PlainActionFuture<>(); - - ClusterBlock block = new ClusterBlock(1, "", retryableBlock, true, false, randomFrom(RestStatus.values()), ClusterBlockLevel.ALL); - ClusterState stateWithBlock = ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) - .blocks(ClusterBlocks.builder().addGlobalBlock(block)) - .build(); - setState(clusterService, stateWithBlock); - - new Action("internal:testAction", transportService, clusterService, threadPool) { - @Override - protected ClusterBlockException checkBlock(Request request, ClusterState state) { - Set blocks = state.blocks().global(); - return blocks.isEmpty() ? null : new ClusterBlockException(blocks); - } - }.execute(request, listener); - - if (retryableBlock && unblockBeforeTimeout) { - assertFalse(listener.isDone()); - setState( - clusterService, - ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) - .blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK) - .build() - ); - assertTrue(listener.isDone()); - listener.get(); - return; - } - - assertTrue(listener.isDone()); - if (retryableBlock) { - try { - listener.get(); - fail("Expected exception but returned proper result"); - } catch (ExecutionException ex) { - assertThat(ex.getCause(), instanceOf(MasterNotDiscoveredException.class)); - assertThat(ex.getCause().getCause(), instanceOf(ClusterBlockException.class)); - } - } else { - assertListenerThrows("ClusterBlockException should be thrown", listener, ClusterBlockException.class); - } - } - - public void testCheckBlockThrowsException() throws InterruptedException { - boolean throwExceptionOnRetry = randomBoolean(); - Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(60)); - PlainActionFuture listener = new PlainActionFuture<>(); - - ClusterBlock block = new ClusterBlock(1, "", true, true, false, randomFrom(RestStatus.values()), ClusterBlockLevel.ALL); - ClusterState stateWithBlock = ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) - .blocks(ClusterBlocks.builder().addGlobalBlock(block)) - .build(); - setState(clusterService, stateWithBlock); - - new Action("internal:testAction", transportService, clusterService, threadPool) { - @Override - protected ClusterBlockException checkBlock(Request request, ClusterState state) { - Set blocks = state.blocks().global(); - if (throwExceptionOnRetry == false || blocks.isEmpty()) { - throw new RuntimeException("checkBlock has thrown exception"); - } - return new ClusterBlockException(blocks); - - } - }.execute(request, listener); - - if (throwExceptionOnRetry == false) { - assertListenerThrows("checkBlock has thrown exception", listener, RuntimeException.class); - } else { - assertFalse(listener.isDone()); - setState( - clusterService, - ClusterState.builder(ClusterStateCreationUtils.state(localNode, localNode, allNodes)) - .blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK) - .build() - ); - assertListenerThrows("checkBlock has thrown exception", listener, RuntimeException.class); - } - } - - public void testForceLocalOperation() throws ExecutionException, InterruptedException { - Request request = new Request(); - PlainActionFuture listener = new PlainActionFuture<>(); - - setState(clusterService, ClusterStateCreationUtils.state(localNode, randomFrom(localNode, remoteNode, null), allNodes)); - - new Action("internal:testAction", transportService, clusterService, threadPool) { - @Override - protected boolean localExecute(Request request) { - return true; - } - }.execute(request, listener); - - assertTrue(listener.isDone()); - listener.get(); - } - - public void testMasterNotAvailable() throws ExecutionException, InterruptedException { - Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(0)); - setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); - PlainActionFuture listener = new PlainActionFuture<>(); - new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); - assertTrue(listener.isDone()); - assertListenerThrows("MasterNotDiscoveredException should be thrown", listener, MasterNotDiscoveredException.class); - } - - public void testMasterBecomesAvailable() throws ExecutionException, InterruptedException { - Request request = new Request(); - setState(clusterService, ClusterStateCreationUtils.state(localNode, null, allNodes)); - PlainActionFuture listener = new PlainActionFuture<>(); - new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); - assertFalse(listener.isDone()); - setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - assertTrue(listener.isDone()); - listener.get(); - } - - public void testDelegateToMaster() throws ExecutionException, InterruptedException { - Request request = new Request(); - setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); - - PlainActionFuture listener = new PlainActionFuture<>(); - new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); - - assertThat(transport.capturedRequests().length, equalTo(1)); - CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; - assertTrue(capturedRequest.node.isMasterNode()); - assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("internal:testAction")); - - Response response = new Response(); - transport.handleResponse(capturedRequest.requestId, response); - assertTrue(listener.isDone()); - assertThat(listener.get(), equalTo(response)); - } - - public void testDelegateToFailingMaster() throws ExecutionException, InterruptedException { - boolean failsWithConnectTransportException = randomBoolean(); - boolean rejoinSameMaster = failsWithConnectTransportException && randomBoolean(); - Request request = new Request().masterNodeTimeout(TimeValue.timeValueSeconds(failsWithConnectTransportException ? 60 : 0)); - DiscoveryNode masterNode = this.remoteNode; - setState( - clusterService, - // use a random base version so it can go down when simulating a restart. - ClusterState.builder(ClusterStateCreationUtils.state(localNode, masterNode, allNodes)).version(randomIntBetween(0, 10)) - ); - - PlainActionFuture listener = new PlainActionFuture<>(); - new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); - - CapturingTransport.CapturedRequest[] capturedRequests = transport.getCapturedRequestsAndClear(); - assertThat(capturedRequests.length, equalTo(1)); - CapturingTransport.CapturedRequest capturedRequest = capturedRequests[0]; - assertTrue(capturedRequest.node.isMasterNode()); - assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("internal:testAction")); - - if (rejoinSameMaster) { - transport.handleRemoteError( - capturedRequest.requestId, - randomBoolean() ? new ConnectTransportException(masterNode, "Fake error") : new NodeClosedException(masterNode) - ); - assertFalse(listener.isDone()); - if (randomBoolean()) { - // simulate master node removal - final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); - nodesBuilder.masterNodeId(null); - setState(clusterService, ClusterState.builder(clusterService.state()).nodes(nodesBuilder)); - } - if (randomBoolean()) { - // reset the same state to increment a version simulating a join of an existing node - // simulating use being disconnected - final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); - nodesBuilder.masterNodeId(masterNode.getId()); - setState(clusterService, ClusterState.builder(clusterService.state()).nodes(nodesBuilder)); - } else { - // simulate master restart followed by a state recovery - this will reset the cluster state version - final DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(clusterService.state().nodes()); - nodesBuilder.remove(masterNode); - masterNode = new DiscoveryNode(masterNode.getId(), masterNode.getAddress(), masterNode.getVersion()); - nodesBuilder.add(masterNode); - nodesBuilder.masterNodeId(masterNode.getId()); - final ClusterState.Builder builder = ClusterState.builder(clusterService.state()).nodes(nodesBuilder); - setState(clusterService, builder.version(0)); - } - assertFalse(listener.isDone()); - capturedRequests = transport.getCapturedRequestsAndClear(); - assertThat(capturedRequests.length, equalTo(1)); - capturedRequest = capturedRequests[0]; - assertTrue(capturedRequest.node.isMasterNode()); - assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("internal:testAction")); - } else if (failsWithConnectTransportException) { - transport.handleRemoteError(capturedRequest.requestId, new ConnectTransportException(masterNode, "Fake error")); - assertFalse(listener.isDone()); - setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - assertTrue(listener.isDone()); - listener.get(); - } else { - OpenSearchException t = new OpenSearchException("test"); - t.addHeader("header", "is here"); - transport.handleRemoteError(capturedRequest.requestId, t); - assertTrue(listener.isDone()); - try { - listener.get(); - fail("Expected exception but returned proper result"); - } catch (ExecutionException ex) { - final Throwable cause = ex.getCause().getCause(); - assertThat(cause, instanceOf(OpenSearchException.class)); - final OpenSearchException es = (OpenSearchException) cause; - assertThat(es.getMessage(), equalTo(t.getMessage())); - assertThat(es.getHeader("header"), equalTo(t.getHeader("header"))); - } - } - } - - public void testMasterFailoverAfterStepDown() throws ExecutionException, InterruptedException { - Request request = new Request().masterNodeTimeout(TimeValue.timeValueHours(1)); - PlainActionFuture listener = new PlainActionFuture<>(); - - final Response response = new Response(); - - setState(clusterService, ClusterStateCreationUtils.state(localNode, localNode, allNodes)); - - new Action("internal:testAction", transportService, clusterService, threadPool) { - @Override - protected void masterOperation(Request request, ClusterState state, ActionListener listener) throws Exception { - // The other node has become master, simulate failures of this node while publishing cluster state through ZenDiscovery - setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); - Exception failure = randomBoolean() - ? new FailedToCommitClusterStateException("Fake error") - : new NotMasterException("Fake error"); - listener.onFailure(failure); - } - }.execute(request, listener); - - assertThat(transport.capturedRequests().length, equalTo(1)); - CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; - assertTrue(capturedRequest.node.isMasterNode()); - assertThat(capturedRequest.request, equalTo(request)); - assertThat(capturedRequest.action, equalTo("internal:testAction")); - - transport.handleResponse(capturedRequest.requestId, response); - assertTrue(listener.isDone()); - assertThat(listener.get(), equalTo(response)); - } -} diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java index 60dcbc15b96c8..189238ec0cc2e 100644 --- a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java @@ -121,14 +121,14 @@ public void setUp() throws Exception { "local_node", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); remoteNode = new DiscoveryNode( "remote_node", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); allNodes = new DiscoveryNode[] { localNode, remoteNode }; @@ -525,4 +525,43 @@ protected void masterOperation(Request request, ClusterState state, ActionListen assertTrue(listener.isDone()); assertThat(listener.get(), equalTo(response)); } + + /* + * Validate TransportMasterNodeAction.testDelegateToMaster() works correctly on node with the deprecated MASTER_ROLE. + * Remove the class after removing MASTER_ROLE. + */ + public void testDelegateToMasterOnNodeWithDeprecatedMasterRole() throws ExecutionException, InterruptedException { + DiscoveryNode localNode = new DiscoveryNode( + "local_node", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + DiscoveryNode remoteNode = new DiscoveryNode( + "remote_node", + buildNewFakeTransportAddress(), + Collections.emptyMap(), + Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Version.CURRENT + ); + DiscoveryNode[] allNodes = new DiscoveryNode[] { localNode, remoteNode }; + + Request request = new Request(); + setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); + + PlainActionFuture listener = new PlainActionFuture<>(); + new Action("internal:testAction", transportService, clusterService, threadPool).execute(request, listener); + + assertThat(transport.capturedRequests().length, equalTo(1)); + CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; + assertTrue(capturedRequest.node.isMasterNode()); + assertThat(capturedRequest.request, equalTo(request)); + assertThat(capturedRequest.action, equalTo("internal:testAction")); + + Response response = new Response(); + transport.handleResponse(capturedRequest.requestId, response); + assertTrue(listener.isDone()); + assertThat(listener.get(), equalTo(response)); + } } From 9bc46f45f6f6de431f5d635d4dc2de00fedda21f Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 8 Apr 2022 17:31:18 -0700 Subject: [PATCH 09/13] Add tests for master role in ClusterBootstrapService Signed-off-by: Tianli Feng --- .../admin/cluster/stats/ClusterStatsIT.java | 1 + ...AddVotingConfigExclusionsRequestTests.java | 8 +- .../shrink/TransportResizeActionTests.java | 1 + .../TransportMultiSearchActionTests.java | 1 + .../TransportMasterNodeActionTests.java | 6 +- ...BootstrapServiceDeprecatedMasterTests.java | 286 +----------------- 6 files changed, 16 insertions(+), 287 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java index 461981c87a103..a807062131f7f 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java @@ -146,6 +146,7 @@ public void testNodeCounts() { } // Validate assigning value "master" to setting "node.roles" can get correct count in Node Stats response after MASTER_ROLE deprecated. + // TODO: Remove the test after removing MASTER_ROLE. public void testNodeCountsWithDeprecatedMasterRole() { int total = 1; Settings settings = Settings.builder() diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java index 7c66379f0a16c..8f9dbe032698a 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java @@ -450,10 +450,9 @@ public void testResolveAndCheckMaximum() { assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } - /* - * As of 2.0, MASTER_ROLE is deprecated to promote inclusive language. - * Validate node with MASTER_ROLE can be resolved by resolveVotingConfigExclusions() like before. - */ + // As of 2.0, MASTER_ROLE is deprecated to promote inclusive language. + // Validate node with MASTER_ROLE can be resolved by resolveVotingConfigExclusions() like before. + // TODO: Remove the test after removing MASTER_ROLE. public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { final DiscoveryNode localNode = new DiscoveryNode( "local", @@ -473,6 +472,7 @@ public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } + // TODO: Remove the test after removing MASTER_ROLE. public void testResolveByNodeIdWithDeprecatedMasterRole() { final DiscoveryNode node = new DiscoveryNode( "nodeName", diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index 8f86d61891278..f97986184b9d5 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -277,6 +277,7 @@ public void testShrinkIndexSettings() { } // Validate the Action works correctly on a node with deprecated 'master' role + // TODO: Remove the test after removing MASTER_ROLE. public void testPassNumRoutingShardsOnNodeWithDeprecatedMasterRole() { final Set roles = Set.of(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE); diff --git a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java index 6cbac74948150..a93837eb01a8e 100644 --- a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java @@ -287,6 +287,7 @@ public void testDefaultMaxConcurrentSearches() { } // Validate the Action works correctly on a node with deprecated 'master' role + // TODO: Remove the test after removing MASTER_ROLE. public void testDefaultMaxConcurrentSearchesOnNodeWithDeprecatedMasterRole() { DiscoveryNodes.Builder builder = DiscoveryNodes.builder(); builder.add( diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java index 189238ec0cc2e..84e6945b70862 100644 --- a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java @@ -526,10 +526,8 @@ protected void masterOperation(Request request, ClusterState state, ActionListen assertThat(listener.get(), equalTo(response)); } - /* - * Validate TransportMasterNodeAction.testDelegateToMaster() works correctly on node with the deprecated MASTER_ROLE. - * Remove the class after removing MASTER_ROLE. - */ + // Validate TransportMasterNodeAction.testDelegateToMaster() works correctly on node with the deprecated MASTER_ROLE. + // TODO: Remove the test after removing MASTER_ROLE. public void testDelegateToMasterOnNodeWithDeprecatedMasterRole() throws ExecutionException, InterruptedException { DiscoveryNode localNode = new DiscoveryNode( "local_node", diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java index 0bc675206e049..f86f1988fa26a 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java @@ -32,7 +32,6 @@ package org.opensearch.cluster.coordination; import org.junit.Before; -import org.opensearch.OpenSearchException; import org.opensearch.Version; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodeRole; @@ -44,9 +43,7 @@ import org.opensearch.transport.TransportService; import java.util.Collections; -import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -56,13 +53,11 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static java.util.Collections.singletonList; -import static org.opensearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.opensearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; import static org.opensearch.common.settings.Settings.builder; import static org.opensearch.node.Node.NODE_NAME_SETTING; import static org.opensearch.test.NodeRoles.nonMasterNode; -import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -70,8 +65,13 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.startsWith; +/* + * As of 2.0, MASTER_ROLE and setting 'cluster.initial_master_nodes' is deprecated to promote inclusive language. + * This class is a partial copy of ClusterBootstrapServiceTests + * to validate ClusterBootstrapService works correctly with the deprecated node role and cluster setting. + * Remove the class after the deprecated node role and cluster setting is removed. + */ public class ClusterBootstrapServiceDeprecatedMasterTests extends OpenSearchTestCase { private DiscoveryNode localNode, otherNode1, otherNode2; @@ -222,110 +222,6 @@ public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { assertFalse(bootstrapped.get()); // should only bootstrap once } - public void testBootstrapsOnDiscoveryOfTwoOfThreeRequiredNodes() { - final AtomicBoolean bootstrapped = new AtomicBoolean(); - - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) - .build(), - transportService, - () -> singletonList(otherNode1), - () -> false, - vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), hasSize(3)); - assertThat(vc.getNodeIds(), hasItem(localNode.getId())); - assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); - assertThat(vc.getNodeIds(), hasItem(allOf(startsWith(BOOTSTRAP_PLACEHOLDER_PREFIX), containsString(otherNode2.getName())))); - assertTrue(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); - assertFalse(vc.hasQuorum(singletonList(localNode.getId()))); - assertFalse(vc.hasQuorum(singletonList(otherNode1.getId()))); - } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - - bootstrapped.set(false); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertFalse(bootstrapped.get()); // should only bootstrap once - } - - public void testBootstrapsOnDiscoveryOfThreeOfFiveRequiredNodes() { - final AtomicBoolean bootstrapped = new AtomicBoolean(); - - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList( - INITIAL_MASTER_NODES_SETTING.getKey(), - localNode.getName(), - otherNode1.getName(), - otherNode2.getName(), - "missing-node-1", - "missing-node-2" - ) - .build(), - transportService, - () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), - () -> false, - vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), hasSize(5)); - assertThat(vc.getNodeIds(), hasItem(localNode.getId())); - assertThat(vc.getNodeIds(), hasItem(otherNode1.getId())); - assertThat(vc.getNodeIds(), hasItem(otherNode2.getId())); - - final List placeholders = vc.getNodeIds() - .stream() - .filter(ClusterBootstrapService::isBootstrapPlaceholder) - .collect(Collectors.toList()); - assertThat(placeholders.size(), equalTo(2)); - assertNotEquals(placeholders.get(0), placeholders.get(1)); - assertThat(placeholders, hasItem(containsString("missing-node-1"))); - assertThat(placeholders, hasItem(containsString("missing-node-2"))); - - assertTrue( - vc.hasQuorum(Stream.of(localNode, otherNode1, otherNode2).map(DiscoveryNode::getId).collect(Collectors.toList())) - ); - assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); - assertFalse(vc.hasQuorum(Stream.of(localNode, otherNode1).map(DiscoveryNode::getId).collect(Collectors.toList()))); - } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - - bootstrapped.set(false); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertFalse(bootstrapped.get()); // should only bootstrap once - } - - public void testDoesNotBootstrapIfNoNodesDiscovered() { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) - .build(), - transportService, - Collections::emptyList, - () -> true, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder() @@ -350,31 +246,6 @@ public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { deterministicTaskQueue.runAllTasks(); } - public void testDoesNotBootstrapIfThreeOfSixNodesDiscovered() { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList( - INITIAL_MASTER_NODES_SETTING.getKey(), - localNode.getName(), - otherNode1.getName(), - otherNode2.getName(), - "not-a-node-1", - "not-a-node-2", - "not-a-node-3" - ) - .build(), - transportService, - () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), - () -> false, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - public void testDoesNotBootstrapIfAlreadyBootstrapped() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder() @@ -416,7 +287,7 @@ public void testDoesNotBootstrapsOnNonMasterNode() { deterministicTaskQueue.runAllTasks(); } - public void testDoesNotBootstrapsIfLocalNodeNotInInitialClusterManagerNodes() { + public void testDoesNotBootstrapsIfLocalNodeNotInInitialMasterNodes() { ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getName(), otherNode2.getName()).build(), transportService, @@ -445,31 +316,6 @@ public void testDoesNotBootstrapsIfNotConfigured() { deterministicTaskQueue.runAllTasks(); } - public void testRetriesBootstrappingOnException() { - final AtomicLong bootstrappingAttempts = new AtomicLong(); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) - .build(), - transportService, - () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), - () -> false, - vc -> { - bootstrappingAttempts.incrementAndGet(); - if (bootstrappingAttempts.get() < 5L) { - throw new OpenSearchException("test"); - } - } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertThat(bootstrappingAttempts.get(), greaterThanOrEqualTo(5L)); - assertThat(deterministicTaskQueue.getCurrentTimeMillis(), greaterThanOrEqualTo(40000L)); - } - public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { AtomicReference> discoveredNodes = new AtomicReference<>( Stream.of(otherNode1, otherNode2).collect(Collectors.toList()) @@ -492,124 +338,6 @@ public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { deterministicTaskQueue.runAllTasks(); } - public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { - AtomicReference> discoveredNodes = new AtomicReference<>( - Stream.of(otherNode1, otherNode2).collect(Collectors.toList()) - ); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), otherNode1.getAddress().toString(), otherNode1.getName()) - .build(), - transportService, - discoveredNodes::get, - () -> false, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - - discoveredNodes.set( - Stream.of( - new DiscoveryNode( - otherNode1.getName(), - randomAlphaOfLength(10), - buildNewFakeTransportAddress(), - emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT - ), - new DiscoveryNode( - "yet-another-node", - randomAlphaOfLength(10), - otherNode1.getAddress(), - emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT - ) - ).collect(Collectors.toList()) - ); - - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - - public void testMatchesOnNodeName() { - final AtomicBoolean bootstrapped = new AtomicBoolean(); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName()).build(), - transportService, - Collections::emptyList, - () -> false, - vc -> assertTrue(bootstrapped.compareAndSet(false, true)) - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - } - - public void testMatchesOnNodeAddress() { - final AtomicBoolean bootstrapped = new AtomicBoolean(); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().toString()).build(), - transportService, - Collections::emptyList, - () -> false, - vc -> assertTrue(bootstrapped.compareAndSet(false, true)) - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - } - - public void testMatchesOnNodeHostAddress() { - final AtomicBoolean bootstrapped = new AtomicBoolean(); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, - Collections::emptyList, - () -> false, - vc -> assertTrue(bootstrapped.compareAndSet(false, true)) - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - } - - public void testDoesNotIncludeExtraNodes() { - final DiscoveryNode extraNode = newDiscoveryNode("extra-node"); - final AtomicBoolean bootstrapped = new AtomicBoolean(); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) - .build(), - transportService, - () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), - () -> false, - vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId()))); - } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - } - public void testBootstrapsAutomaticallyWithSingleNodeDiscovery() { final Settings.Builder settings = Settings.builder() .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) From eb546080d5d054077b575c1236741200327727fc Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 8 Apr 2022 21:34:38 -0700 Subject: [PATCH 10/13] Add tests for master role in NodeJoinTests Signed-off-by: Tianli Feng --- .../cluster/coordination/NodeJoinTests.java | 23 +++++++++++++++++++ .../cluster/node/DiscoveryNodeTests.java | 3 +++ 2 files changed, 26 insertions(+) diff --git a/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java b/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java index 3b309908a1df0..e99642ff649e2 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java @@ -742,6 +742,29 @@ public void testConcurrentJoining() { } } + // Validate the deprecated MASTER_ROLE can join another node and elected as leader. + // TODO: Remove the test after removing MASTER_ROLE. + public void testJoinElectedLeaderWithDeprecatedMasterRole() { + final Set roles = singleton(DiscoveryNodeRole.MASTER_ROLE); + DiscoveryNode node0 = new DiscoveryNode("master0", "0", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); + DiscoveryNode node1 = new DiscoveryNode("master1", "1", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); + long initialTerm = 1; + long initialVersion = 1; + setupFakeMasterServiceAndCoordinator( + initialTerm, + initialState(node0, initialTerm, initialVersion, VotingConfiguration.of(node0)), + () -> new StatusInfo(HEALTHY, "healthy-info") + ); + assertFalse(isLocalNodeElectedMaster()); + long newTerm = 2; + joinNodeAndRun(new JoinRequest(node0, newTerm, Optional.of(new Join(node0, node0, newTerm, initialTerm, initialVersion)))); + assertTrue(isLocalNodeElectedMaster()); + assertFalse(clusterStateHasNode(node1)); + joinNodeAndRun(new JoinRequest(node1, newTerm, Optional.of(new Join(node1, node0, newTerm, initialTerm, initialVersion)))); + assertTrue(isLocalNodeElectedMaster()); + assertTrue(clusterStateHasNode(node1)); + } + private boolean isLocalNodeElectedMaster() { return MasterServiceTests.discoveryState(masterService).nodes().isLocalNodeElectedMaster(); } diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java index 933d9d5f825e9..403a60556c072 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -176,6 +176,9 @@ public void testDiscoveryNodeIsRemoteClusterClientUnset() { // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. public void testSetAdditionalRolesCanAddDeprecatedMasterRole() { + // Validate MASTER_ROLE is not in DiscoveryNodeRole.BUILT_IN_ROLES + assertFalse(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); assertTrue(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); } From c64878fb0a474b2d2b39c0d56748b77d86123e60 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sun, 10 Apr 2022 23:58:03 -0700 Subject: [PATCH 11/13] Add more test for deprecated setting initial master nodes Signed-off-by: Tianli Feng --- ...BootstrapServiceDeprecatedMasterTests.java | 97 +------------------ .../ClusterFormationFailureHelperTests.java | 34 +++++++ 2 files changed, 38 insertions(+), 93 deletions(-) diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java index f86f1988fa26a..14cecae05650b 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java @@ -222,47 +222,6 @@ public void testBootstrapsOnDiscoveryOfAllRequiredNodes() { assertFalse(bootstrapped.get()); // should only bootstrap once } - public void testDoesNotBootstrapIfTwoOfFiveNodesDiscovered() { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList( - INITIAL_MASTER_NODES_SETTING.getKey(), - localNode.getName(), - otherNode1.getName(), - otherNode2.getName(), - "not-a-node-1", - "not-a-node-2" - ) - .build(), - transportService, - () -> Stream.of(otherNode1).collect(Collectors.toList()), - () -> false, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - - public void testDoesNotBootstrapIfAlreadyBootstrapped() { - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder() - .putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()) - .build(), - transportService, - () -> Stream.of(otherNode1, otherNode2).collect(Collectors.toList()), - () -> true, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - public void testDoesNotBootstrapsOnNonMasterNode() { localNode = new DiscoveryNode( "local", @@ -316,58 +275,10 @@ public void testDoesNotBootstrapsIfNotConfigured() { deterministicTaskQueue.runAllTasks(); } - public void testCancelsBootstrapIfRequirementMatchesMultipleNodes() { - AtomicReference> discoveredNodes = new AtomicReference<>( - Stream.of(otherNode1, otherNode2).collect(Collectors.toList()) - ); - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getAddress().getAddress()).build(), - transportService, - discoveredNodes::get, - () -> false, - vc -> { throw new AssertionError("should not be called"); } - ); - assertWarnings(CLUSTER_SETTING_DEPRECATED_MESSAGE); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - - discoveredNodes.set(emptyList()); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - } - - public void testBootstrapsAutomaticallyWithSingleNodeDiscovery() { - final Settings.Builder settings = Settings.builder() - .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) - .put(NODE_NAME_SETTING.getKey(), localNode.getName()); - final AtomicBoolean bootstrapped = new AtomicBoolean(); - - ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService( - settings.build(), - transportService, - () -> emptyList(), - () -> false, - vc -> { - assertTrue(bootstrapped.compareAndSet(false, true)); - assertThat(vc.getNodeIds(), hasSize(1)); - assertThat(vc.getNodeIds(), hasItem(localNode.getId())); - assertTrue(vc.hasQuorum(singletonList(localNode.getId()))); - } - ); - - transportService.start(); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertTrue(bootstrapped.get()); - - bootstrapped.set(false); - clusterBootstrapService.onFoundPeersUpdated(); - deterministicTaskQueue.runAllTasks(); - assertFalse(bootstrapped.get()); // should only bootstrap once - } - + /** + * Validate the correct deprecated setting name of cluster.initial_master_nodes is shown in the exception, + * when discovery type is single-node. + */ public void testFailBootstrapWithBothSingleNodeDiscoveryAndInitialMasterNodes() { final Settings.Builder settings = Settings.builder() .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), DiscoveryModule.SINGLE_NODE_DISCOVERY_TYPE) diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java index 391d7b0e56332..420e2bbcf2f23 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java @@ -57,6 +57,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.cluster.coordination.ClusterBootstrapService.BOOTSTRAP_PLACEHOLDER_PREFIX; import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_CLUSTER_MANAGER_NODES_SETTING; +import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.opensearch.monitor.StatusInfo.Status.HEALTHY; import static org.opensearch.monitor.StatusInfo.Status.UNHEALTHY; import static org.opensearch.node.Node.NODE_NAME_SETTING; @@ -400,6 +401,39 @@ public void testDescriptionBeforeBootstrapping() { ); } + // Validate the value of the deprecated 'master' setting can be obtained during cluster formation failure. + // TODO: Remove the test after removing MASTER_ROLE. + public void testDescriptionBeforeBootstrappingWithDeprecatedMasterNodesSetting() { + final DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); + final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) + .version(7L) + .metadata(Metadata.builder().coordinationMetadata(CoordinationMetadata.builder().term(4L).build())) + .nodes(DiscoveryNodes.builder().add(localNode).localNodeId(localNode.getId())) + .build(); + assertThat( + new ClusterFormationState( + Settings.builder().putList(INITIAL_MASTER_NODES_SETTING.getKey(), "other").build(), + clusterState, + emptyList(), + emptyList(), + 4L, + electionStrategy, + new StatusInfo(HEALTHY, "healthy-info") + ).getDescription(), + is( + "cluster-manager not discovered yet, this node has not previously joined a bootstrapped cluster, and " + + "this node must discover cluster-manager-eligible nodes [other] to bootstrap a cluster: have discovered []; " + + "discovery will continue using [] from hosts providers and [" + + localNode + + "] from last-known cluster state; node term 4, last-accepted version 7 in term 4" + ) + ); + assertWarnings( + "[cluster.initial_master_nodes] setting was deprecated in OpenSearch and will be removed in a future release! " + + "See the breaking changes documentation for the next major version." + ); + } + private static VotingConfiguration config(String[] nodeIds) { return new VotingConfiguration(Arrays.stream(nodeIds).collect(Collectors.toSet())); } From 76a20eb532a259eaf755bae660842ed1e762a700 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Mon, 11 Apr 2022 00:17:19 -0700 Subject: [PATCH 12/13] Remove some unnecessary TransportActionTests Signed-off-by: Tianli Feng --- .../shrink/TransportResizeActionTests.java | 36 ------------------- .../TransportMultiSearchActionTests.java | 27 -------------- ...BootstrapServiceDeprecatedMasterTests.java | 2 -- 3 files changed, 65 deletions(-) diff --git a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java index f97986184b9d5..e4b79ac54f8fd 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/shrink/TransportResizeActionTests.java @@ -276,42 +276,6 @@ public void testShrinkIndexSettings() { assertEquals(request.waitForActiveShards(), activeShardCount); } - // Validate the Action works correctly on a node with deprecated 'master' role - // TODO: Remove the test after removing MASTER_ROLE. - public void testPassNumRoutingShardsOnNodeWithDeprecatedMasterRole() { - final Set roles = Set.of(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE); - - ClusterState clusterState = ClusterState.builder( - createClusterState("source", 1, 0, Settings.builder().put("index.blocks.write", true).build()) - ) - .nodes( - DiscoveryNodes.builder().add(new DiscoveryNode("node1", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT)) - ) - .build(); - AllocationService service = new AllocationService( - new AllocationDeciders(Collections.singleton(new MaxRetryAllocationDecider())), - new TestGatewayAllocator(), - new BalancedShardsAllocator(Settings.EMPTY), - EmptyClusterInfoService.INSTANCE, - EmptySnapshotsInfoService.INSTANCE - ); - - RoutingTable routingTable = service.reroute(clusterState, "reroute").routingTable(); - clusterState = ClusterState.builder(clusterState).routingTable(routingTable).build(); - // now we start the shard - routingTable = OpenSearchAllocationTestCase.startInitializingShardsAndReroute(service, clusterState, "source").routingTable(); - clusterState = ClusterState.builder(clusterState).routingTable(routingTable).build(); - - ResizeRequest resizeRequest = new ResizeRequest("target", "source"); - resizeRequest.setResizeType(ResizeType.SPLIT); - resizeRequest.getTargetIndexRequest().settings(Settings.builder().put("index.number_of_shards", 2).build()); - TransportResizeAction.prepareCreateIndexRequest(resizeRequest, clusterState, null, "source", "target"); - - resizeRequest.getTargetIndexRequest() - .settings(Settings.builder().put("index.number_of_routing_shards", 2).put("index.number_of_shards", 2).build()); - TransportResizeAction.prepareCreateIndexRequest(resizeRequest, clusterState, null, "source", "target"); - } - private DiscoveryNode newNode(String nodeId) { final Set roles = Collections.unmodifiableSet( new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) diff --git a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java index a93837eb01a8e..09ab2438bd106 100644 --- a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java @@ -286,31 +286,4 @@ public void testDefaultMaxConcurrentSearches() { assertThat(result, equalTo(1)); } - // Validate the Action works correctly on a node with deprecated 'master' role - // TODO: Remove the test after removing MASTER_ROLE. - public void testDefaultMaxConcurrentSearchesOnNodeWithDeprecatedMasterRole() { - DiscoveryNodes.Builder builder = DiscoveryNodes.builder(); - builder.add( - new DiscoveryNode( - "master", - buildNewFakeTransportAddress(), - Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), - Version.CURRENT - ) - ); - builder.add( - new DiscoveryNode( - "data", - buildNewFakeTransportAddress(), - Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.DATA_ROLE), - Version.CURRENT - ) - ); - - ClusterState state = ClusterState.builder(new ClusterName("_name")).nodes(builder).build(); - int result = TransportMultiSearchAction.defaultMaxConcurrentSearches(10, state); - assertThat(result, equalTo(10)); - } } diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java index 14cecae05650b..e4fd8064098dd 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java @@ -52,7 +52,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; -import static java.util.Collections.singletonList; import static org.opensearch.cluster.coordination.ClusterBootstrapService.INITIAL_MASTER_NODES_SETTING; import static org.opensearch.cluster.coordination.ClusterBootstrapService.UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING; import static org.opensearch.common.settings.Settings.builder; @@ -63,7 +62,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; /* From 572225323cb5bf992b48fb1d48d3bbb8591af0e9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Tue, 26 Apr 2022 14:22:54 -0700 Subject: [PATCH 13/13] Remove unnecessary TODOs and change an assertWarning() to allowedWarning() Signed-off-by: Tianli Feng --- .../action/admin/cluster/stats/ClusterStatsIT.java | 1 - .../configuration/AddVotingConfigExclusionsRequestTests.java | 5 ++--- .../support/master/TransportMasterNodeActionTests.java | 1 - .../org/opensearch/cluster/ClusterChangedEventTests.java | 1 - .../coordination/ClusterFormationFailureHelperTests.java | 1 - .../org/opensearch/cluster/coordination/NodeJoinTests.java | 1 - .../java/org/opensearch/cluster/node/DiscoveryNodeTests.java | 5 ++++- 7 files changed, 6 insertions(+), 9 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java index a807062131f7f..461981c87a103 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java @@ -146,7 +146,6 @@ public void testNodeCounts() { } // Validate assigning value "master" to setting "node.roles" can get correct count in Node Stats response after MASTER_ROLE deprecated. - // TODO: Remove the test after removing MASTER_ROLE. public void testNodeCountsWithDeprecatedMasterRole() { int total = 1; Settings settings = Settings.builder() diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java index 8f9dbe032698a..54b51944999d4 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java @@ -452,7 +452,7 @@ public void testResolveAndCheckMaximum() { // As of 2.0, MASTER_ROLE is deprecated to promote inclusive language. // Validate node with MASTER_ROLE can be resolved by resolveVotingConfigExclusions() like before. - // TODO: Remove the test after removing MASTER_ROLE. + // The following 3 tests assign nodes by description, id and name respectively. public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { final DiscoveryNode localNode = new DiscoveryNode( "local", @@ -469,10 +469,9 @@ public void testResolveByNodeDescriptionWithDeprecatedMasterRole() { .build(); assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState), contains(localNodeExclusion)); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); + allowedWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } - // TODO: Remove the test after removing MASTER_ROLE. public void testResolveByNodeIdWithDeprecatedMasterRole() { final DiscoveryNode node = new DiscoveryNode( "nodeName", diff --git a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java index 84e6945b70862..1dd44f3186657 100644 --- a/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/master/TransportMasterNodeActionTests.java @@ -527,7 +527,6 @@ protected void masterOperation(Request request, ClusterState state, ActionListen } // Validate TransportMasterNodeAction.testDelegateToMaster() works correctly on node with the deprecated MASTER_ROLE. - // TODO: Remove the test after removing MASTER_ROLE. public void testDelegateToMasterOnNodeWithDeprecatedMasterRole() throws ExecutionException, InterruptedException { DiscoveryNode localNode = new DiscoveryNode( "local_node", diff --git a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java index 7b0ed160b3fb3..e0a12fc1d312b 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java @@ -315,7 +315,6 @@ public void testChangedCustomMetadataSet() { } // Validate the above test case testLocalNodeIsMaster() passes when the deprecated 'master' role is assigned to the local node. - // TODO: Remove the test after removing MASTER_ROLE. public void testLocalNodeIsMasterWithDeprecatedMasterRole() { final DiscoveryNodes.Builder builderLocalIsMaster = DiscoveryNodes.builder(); final DiscoveryNode node0 = newNode("node_0", Set.of(DiscoveryNodeRole.MASTER_ROLE)); diff --git a/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java b/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java index 420e2bbcf2f23..392403151cdb5 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java @@ -402,7 +402,6 @@ public void testDescriptionBeforeBootstrapping() { } // Validate the value of the deprecated 'master' setting can be obtained during cluster formation failure. - // TODO: Remove the test after removing MASTER_ROLE. public void testDescriptionBeforeBootstrappingWithDeprecatedMasterNodesSetting() { final DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) diff --git a/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java b/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java index e99642ff649e2..11476431024ba 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java @@ -743,7 +743,6 @@ public void testConcurrentJoining() { } // Validate the deprecated MASTER_ROLE can join another node and elected as leader. - // TODO: Remove the test after removing MASTER_ROLE. public void testJoinElectedLeaderWithDeprecatedMasterRole() { final Set roles = singleton(DiscoveryNodeRole.MASTER_ROLE); DiscoveryNode node0 = new DiscoveryNode("master0", "0", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java index 403a60556c072..3a058a282be9c 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -174,7 +174,10 @@ public void testDiscoveryNodeIsRemoteClusterClientUnset() { runTestDiscoveryNodeIsRemoteClusterClient(nonRemoteClusterClientNode(), false); } - // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. + // Added in 2.0 temporarily, validate the MASTER_ROLE is in the list of known roles. + // MASTER_ROLE was removed from BUILT_IN_ROLES and is imported by setAdditionalRoles(), + // as a workaround for making the new CLUSTER_MANAGER_ROLE has got the same abbreviation 'm'. + // The test validate this behavior. public void testSetAdditionalRolesCanAddDeprecatedMasterRole() { // Validate MASTER_ROLE is not in DiscoveryNodeRole.BUILT_IN_ROLES assertFalse(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName()));