From 00c0bf2dd9da5b67dbe331be502814de2d399de9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Tue, 17 May 2022 10:37:52 -0700 Subject: [PATCH] [TEST] Add back necessary tests for deprecated settings that are replaced during applying inclusive naming (#2825) Signed-off-by: Tianli Feng --- .../admin/cluster/stats/ClusterStatsIT.java | 22 ++ ...AddVotingConfigExclusionsRequestTests.java | 58 ++++ .../TransportMasterNodeActionTests.java | 36 ++ .../cluster/ClusterChangedEventTests.java | 29 ++ ...BootstrapServiceDeprecatedMasterTests.java | 311 ++++++++++++++++++ ...erBootstrapServiceRenamedSettingTests.java | 2 +- .../ClusterBootstrapServiceTests.java | 27 -- .../ClusterFormationFailureHelperTests.java | 33 ++ .../cluster/coordination/NodeJoinTests.java | 22 ++ .../cluster/node/DiscoveryNodeTests.java | 8 +- 10 files changed, 519 insertions(+), 29 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java 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 48e9f49ece6..a44cf05a4bd 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.opensearch.test.OpenSearchIntegTestCase.Scope; 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,27 @@ public class ClusterStatsIT extends OpenSearchIntegTestCase { } } + // 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(); + 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) { 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 a92e4e4a6c5..54b51944999 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,64 @@ public class AddVotingConfigExclusionsRequestTests extends OpenSearchTestCase { 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. + // The following 3 tests assign nodes by description, id and name respectively. + 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)); + allowedWarnings(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, 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 a8ad356e947..1dd44f31866 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 @@ -525,4 +525,40 @@ public class TransportMasterNodeActionTests extends OpenSearchTestCase { assertTrue(listener.isDone()); assertThat(listener.get(), equalTo(response)); } + + // Validate TransportMasterNodeAction.testDelegateToMaster() works correctly on node with the deprecated 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)); + } } diff --git a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java index 49d4a8baf0a..e0a12fc1d31 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java @@ -314,6 +314,35 @@ public class ClusterChangedEventTests extends OpenSearchTestCase { assertTrue(changedCustomMetadataTypeSet.contains(customMetadata1.getWriteableName())); } + // Validate the above test case testLocalNodeIsMaster() passes when the deprecated 'master' role is assigned to the local node. + 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); 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 00000000000..e4fd8064098 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceDeprecatedMasterTests.java @@ -0,0 +1,311 @@ +/* + * 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.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.concurrent.atomic.AtomicBoolean; +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 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.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.not; + +/* + * 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; + 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 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 testDoesNotBootstrapsIfLocalNodeNotInInitialMasterNodes() { + 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(); + } + + /** + * 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) + .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 09fb6a5ea25..f95e50a22cf 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 3e4148cef61..812bf942596 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.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_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 class ClusterBootstrapServiceTests extends OpenSearchTestCase { 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 class ClusterBootstrapServiceTests extends OpenSearchTestCase { 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. 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 c3be1a726d9..4fb96145732 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.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_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,38 @@ public class ClusterFormationFailureHelperTests extends OpenSearchTestCase { ); } + // Validate the value of the deprecated 'master' setting can be obtained during cluster formation failure. + 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())); } 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 f00361160f2..43c9c77f193 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,28 @@ public class NodeJoinTests extends OpenSearchTestCase { } } + // Validate the deprecated MASTER_ROLE can join another node and elected as leader. + 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 933d9d5f825..3a058a282be 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -174,8 +174,14 @@ public class DiscoveryNodeTests extends OpenSearchTestCase { 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())); + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); assertTrue(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); }