[TEST] Add back necessary tests for deprecated settings that are replaced during applying inclusive naming (#2825)

Signed-off-by: Tianli Feng <ftianli@amazon.com>

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<String, Integer> 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<String, Integer> counts) {
         Integer count = counts.get(role);
         if (count == null) {

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,

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<Response> 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));
+    }
 }

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);

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<Supplier<Iterable<DiscoveryNode>>> 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")
+        );
+    }
+}

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<Setting<?>> settings = ClusterSettings.BUILT_IN_CLUSTER_SETTINGS;
         assertTrue(
             "Both 'cluster.initial_cluster_manager_nodes' and its predecessor should be supported built-in settings.",

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.

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()));
     }

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<DiscoveryNodeRole> 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();
     }

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()));
     }