From 344998cc4bac259722646cff41ff902edf4b4ad6 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 18:45:16 +0000 Subject: [PATCH 01/51] Add a new role cluster_manager as alternative to existing master role Signed-off-by: Tianli Feng --- .../main/java/org/opensearch/client/Node.java | 17 +++++++++++++---- .../cluster/node/DiscoveryNode.java | 4 ++-- .../cluster/node/DiscoveryNodeRole.java | 19 +++++++++++++++++-- .../main/java/org/opensearch/node/Node.java | 14 +++++++++++++- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/client/rest/src/main/java/org/opensearch/client/Node.java b/client/rest/src/main/java/org/opensearch/client/Node.java index 41c2926a7d54d..cf304015ed54d 100644 --- a/client/rest/src/main/java/org/opensearch/client/Node.java +++ b/client/rest/src/main/java/org/opensearch/client/Node.java @@ -210,21 +210,30 @@ public Roles(final Set roles) { } /** - * Teturns whether or not the node could be elected master. + * Returns whether or not the node could be elected master. + * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #isClusterManagerEligible()} */ + @Deprecated public boolean isMasterEligible() { - return roles.contains("master"); + return roles.contains("master") || roles.contains("cluster_manager"); } /** - * Teturns whether or not the node stores data. + * Returns whether or not the node could be elected cluster manager. + */ + public boolean isClusterManagerEligible() { + return roles.contains("master") || roles.contains("cluster_manager"); + } + + /** + * Returns whether or not the node stores data. */ public boolean isData() { return roles.contains("data"); } /** - * Teturns whether or not the node runs ingest pipelines. + * Returns whether or not the node runs ingest pipelines. */ public boolean isIngest() { return roles.contains("ingest"); diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 9fa49ec93d307..b7887bc61ef99 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -343,7 +343,7 @@ public DiscoveryNode(StreamInput in) throws IOException { final LegacyRole legacyRole = in.readEnum(LegacyRole.class); switch (legacyRole) { case MASTER: - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); break; case DATA: roles.add(DiscoveryNodeRole.DATA_ROLE); @@ -599,7 +599,7 @@ public static Set getPossibleRoleNames() { } /** - * Enum that holds all the possible roles that that a node can fulfill in a cluster. + * Enum that holds all the possible roles that a node can fulfill in a cluster. * Each role has its name and a corresponding abbreviation used by cat apis. */ private enum LegacyRole { diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index fb1cfb24166c4..254932a1e9f2f 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -193,13 +193,28 @@ public Setting legacySetting() { /** * Represents the role for a master-eligible node. + * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #CLUSTER_MANAGER_ROLE} */ + @Deprecated public static final DiscoveryNodeRole MASTER_ROLE = new DiscoveryNodeRole("master", "m") { @Override public Setting legacySetting() { // copy the setting here so we can mark it private in org.opensearch.node.Node - return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope); + return Setting.boolSetting("node.cluster_manager", true, Property.Deprecated, Property.NodeScope); + } + + }; + + /** + * Represents the role for a cluster_manager-eligible node. + */ + public static final DiscoveryNodeRole CLUSTER_MANAGER_ROLE = new DiscoveryNodeRole("cluster_manager", "m") { + + @Override + public Setting legacySetting() { + // copy the setting here so we can mark it private in org.opensearch.node.Node + return Setting.boolSetting("node.cluster_manager", true, Property.Deprecated, Property.NodeScope); } }; @@ -223,7 +238,7 @@ public Setting legacySetting() { * The built-in node roles. */ public static SortedSet BUILT_IN_ROLES = Collections.unmodifiableSortedSet( - new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)) + new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)) ); /** diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 060f5b23fb504..eeaf370d13292 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -231,12 +231,23 @@ public class Node implements Closeable { Property.Deprecated, Property.NodeScope ); + + /** + * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #NODE_CLUSTER_MANAGER_SETTING} + */ + @Deprecated private static final Setting NODE_MASTER_SETTING = Setting.boolSetting( "node.master", true, Property.Deprecated, Property.NodeScope ); + private static final Setting NODE_CLUSTER_MANAGER_SETTING = Setting.boolSetting( + "node.cluster_manager", + true, + Property.Deprecated, + Property.NodeScope + ); private static final Setting NODE_INGEST_SETTING = Setting.boolSetting( "node.ingest", true, @@ -444,10 +455,11 @@ protected Node( resourcesToClose.add(() -> HeaderWarning.removeThreadContext(threadPool.getThreadContext())); final List> additionalSettings = new ArrayList<>(); - // register the node.data, node.ingest, node.master, node.remote_cluster_client settings here so we can mark them private + // register the node.data, node.ingest, node.cluster_manager, node.remote_cluster_client settings here so we can mark them private additionalSettings.add(NODE_DATA_SETTING); additionalSettings.add(NODE_INGEST_SETTING); additionalSettings.add(NODE_MASTER_SETTING); + additionalSettings.add(NODE_CLUSTER_MANAGER_SETTING); additionalSettings.add(NODE_REMOTE_CLUSTER_CLIENT); additionalSettings.addAll(pluginsService.getPluginSettings()); final List additionalSettingsFilter = new ArrayList<>(pluginsService.getPluginSettingsFilter()); From 0396a4ad16f7205c2b8dde6f723d52cb8256ebe9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 16:08:42 -0800 Subject: [PATCH 02/51] Add CLUSTER_MANAGER_ROLE in server code and replace MASTER_ROLE in tests Signed-off-by: Tianli Feng --- .../routing/allocation/Allocators.java | 2 +- .../admin/cluster/stats/ClusterStatsIT.java | 6 ++-- .../org/opensearch/env/NodeEnvironmentIT.java | 2 +- .../env/NodeRepurposeCommandIT.java | 2 +- .../cluster/node/DiscoveryNode.java | 6 ++-- .../cluster/node/DiscoveryNodeRole.java | 4 +-- .../cluster/node/DiscoveryNodes.java | 2 +- .../org/opensearch/env/NodeEnvironment.java | 2 +- .../main/java/org/opensearch/node/Node.java | 14 +-------- .../repositories/RepositoriesService.java | 2 +- ...AddVotingConfigExclusionsRequestTests.java | 30 +++++++++---------- ...tAddVotingConfigExclusionsActionTests.java | 2 +- .../shrink/TransportResizeActionTests.java | 2 +- .../TransportMultiSearchActionTests.java | 2 +- .../TransportMasterNodeActionTests.java | 4 +-- .../cluster/ClusterChangedEventTests.java | 6 ++-- .../ClusterBootstrapServiceTests.java | 6 ++-- .../ClusterFormationFailureHelperTests.java | 4 +-- .../coordination/CoordinatorTests.java | 2 +- .../cluster/coordination/NodeJoinTests.java | 4 +-- .../health/ClusterHealthAllocationTests.java | 2 +- .../metadata/AutoExpandReplicasTests.java | 6 ++-- .../MetadataCreateIndexServiceTests.java | 2 +- .../metadata/TemplateUpgradeServiceTests.java | 2 +- .../node/DiscoveryNodeRoleSettingTests.java | 2 +- .../cluster/node/DiscoveryNodesTests.java | 2 +- .../DelayedAllocationServiceTests.java | 2 +- .../routing/OperationRoutingTests.java | 2 +- .../allocation/AllocationCommandsTests.java | 6 ++-- .../allocation/FailedNodeRoutingTests.java | 2 +- .../decider/DiskThresholdDeciderTests.java | 4 +-- ...storeInProgressAllocationDeciderTests.java | 2 +- .../opensearch/env/NodeEnvironmentTests.java | 2 +- .../env/NodeRepurposeCommandTests.java | 2 +- .../gateway/ClusterStateUpdatersTests.java | 4 +-- .../GatewayMetaStatePersistedStateTests.java | 2 +- ...ClusterStateServiceRandomUpdatesTests.java | 2 +- .../PersistentTasksClusterServiceTests.java | 2 +- .../snapshots/SnapshotResiliencyTests.java | 2 +- .../transport/ConnectionProfileTests.java | 2 +- .../SniffConnectionStrategyTests.java | 8 ++--- .../cluster/OpenSearchAllocationTestCase.java | 2 +- .../CoordinationStateTestCluster.java | 6 ++-- .../opensearch/test/InternalTestCluster.java | 10 +++---- .../java/org/opensearch/test/NodeRoles.java | 8 +++-- .../test/test/InternalTestClusterTests.java | 12 ++++---- 46 files changed, 96 insertions(+), 106 deletions(-) diff --git a/benchmarks/src/main/java/org/opensearch/benchmark/routing/allocation/Allocators.java b/benchmarks/src/main/java/org/opensearch/benchmark/routing/allocation/Allocators.java index 7c00fa5abb53f..d700b9dab2cf3 100644 --- a/benchmarks/src/main/java/org/opensearch/benchmark/routing/allocation/Allocators.java +++ b/benchmarks/src/main/java/org/opensearch/benchmark/routing/allocation/Allocators.java @@ -111,7 +111,7 @@ public static DiscoveryNode newNode(String nodeId, Map attribute nodeId, new TransportAddress(TransportAddress.META_ADDRESS, portGenerator.incrementAndGet()), attributes, - Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE), + Sets.newHashSet(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE), Version.CURRENT ); } 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 19d1728a1fecd..eea5d458898bf 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 @@ -86,7 +86,7 @@ public void testNodeCounts() { internalCluster().startNode(); Map expectedCounts = new HashMap<>(); expectedCounts.put(DiscoveryNodeRole.DATA_ROLE.roleName(), 1); - expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); + expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), 1); expectedCounts.put(ClusterStatsNodes.Counts.COORDINATING_ONLY, 0); @@ -108,7 +108,7 @@ public void testNodeCounts() { roles.add(DiscoveryNodeRole.INGEST_ROLE); } if (isMasterNode) { - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } if (isRemoteClusterClientNode) { roles.add(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE); @@ -130,7 +130,7 @@ public void testNodeCounts() { incrementCountForRole(DiscoveryNodeRole.INGEST_ROLE.roleName(), expectedCounts); } if (isMasterNode) { - incrementCountForRole(DiscoveryNodeRole.MASTER_ROLE.roleName(), expectedCounts); + incrementCountForRole(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), expectedCounts); } if (isRemoteClusterClientNode) { incrementCountForRole(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), expectedCounts); diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java index 0bebcce27f975..aa91179c9b7ee 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java @@ -88,7 +88,7 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { public Settings onNodeStopped(String nodeName) { return NodeRoles.removeRoles( Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) ) ); } diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java index 2547333490f23..80acc98293bd6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java @@ -75,7 +75,7 @@ public void testRepurpose() throws Exception { final Settings dataNodeDataPathSettings = internalCluster().dataPathSettings(dataNode); final Settings noMasterNoDataSettings = NodeRoles.removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); final Settings noMasterNoDataSettingsForMasterNode = Settings.builder() diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index b7887bc61ef99..e2965a8fe3026 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -94,7 +94,7 @@ public static boolean hasRole(final Settings settings, final DiscoveryNodeRole r } public static boolean isMasterNode(Settings settings) { - return hasRole(settings, DiscoveryNodeRole.MASTER_ROLE); + return hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) || hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } /** @@ -390,7 +390,7 @@ public void writeTo(StreamOutput out) throws IOException { .collect(Collectors.toList()); out.writeVInt(rolesToWrite.size()); for (final DiscoveryNodeRole role : rolesToWrite) { - if (role == DiscoveryNodeRole.MASTER_ROLE) { + if (role == DiscoveryNodeRole.MASTER_ROLE || role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { out.writeEnum(LegacyRole.MASTER); } else if (role == DiscoveryNodeRole.DATA_ROLE) { out.writeEnum(LegacyRole.DATA); @@ -456,7 +456,7 @@ public boolean isDataNode() { * Can this node become master or not. */ public boolean isMasterNode() { - return roles.contains(DiscoveryNodeRole.MASTER_ROLE); + return roles.contains(DiscoveryNodeRole.MASTER_ROLE) || roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } /** diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 254932a1e9f2f..7029454297440 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -201,7 +201,7 @@ public Setting legacySetting() { @Override public Setting legacySetting() { // copy the setting here so we can mark it private in org.opensearch.node.Node - return Setting.boolSetting("node.cluster_manager", true, Property.Deprecated, Property.NodeScope); + return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope); } }; @@ -214,7 +214,7 @@ public Setting legacySetting() { @Override public Setting legacySetting() { // copy the setting here so we can mark it private in org.opensearch.node.Node - return Setting.boolSetting("node.cluster_manager", true, Property.Deprecated, Property.NodeScope); + return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope); } }; diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java index d6e2f129f0ed8..d031667e6ba74 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java @@ -419,7 +419,7 @@ public String[] resolveNodes(String... nodes) { } else { resolvedNodesIds.removeAll(dataNodes.keys()); } - } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName)) { + } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName) || DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName().equals(matchAttrName)) { if (Booleans.parseBoolean(matchAttrValue, true)) { resolvedNodesIds.addAll(masterNodes.keys()); } else { diff --git a/server/src/main/java/org/opensearch/env/NodeEnvironment.java b/server/src/main/java/org/opensearch/env/NodeEnvironment.java index 06109e7fcdc5b..555ca990b736d 100644 --- a/server/src/main/java/org/opensearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/opensearch/env/NodeEnvironment.java @@ -1138,7 +1138,7 @@ private void ensureNoIndexMetadata(final NodePath[] nodePaths) throws IOExceptio Locale.ROOT, "node does not have the %s and %s roles but has index metadata: %s. Use 'opensearch-node repurpose' tool to clean up", DiscoveryNodeRole.DATA_ROLE.roleName(), - DiscoveryNodeRole.MASTER_ROLE.roleName(), + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), indexMetadataPaths ); throw new IllegalStateException(message); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index eeaf370d13292..060f5b23fb504 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -231,23 +231,12 @@ public class Node implements Closeable { Property.Deprecated, Property.NodeScope ); - - /** - * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #NODE_CLUSTER_MANAGER_SETTING} - */ - @Deprecated private static final Setting NODE_MASTER_SETTING = Setting.boolSetting( "node.master", true, Property.Deprecated, Property.NodeScope ); - private static final Setting NODE_CLUSTER_MANAGER_SETTING = Setting.boolSetting( - "node.cluster_manager", - true, - Property.Deprecated, - Property.NodeScope - ); private static final Setting NODE_INGEST_SETTING = Setting.boolSetting( "node.ingest", true, @@ -455,11 +444,10 @@ protected Node( resourcesToClose.add(() -> HeaderWarning.removeThreadContext(threadPool.getThreadContext())); final List> additionalSettings = new ArrayList<>(); - // register the node.data, node.ingest, node.cluster_manager, node.remote_cluster_client settings here so we can mark them private + // register the node.data, node.ingest, node.master, node.remote_cluster_client settings here so we can mark them private additionalSettings.add(NODE_DATA_SETTING); additionalSettings.add(NODE_INGEST_SETTING); additionalSettings.add(NODE_MASTER_SETTING); - additionalSettings.add(NODE_CLUSTER_MANAGER_SETTING); additionalSettings.add(NODE_REMOTE_CLUSTER_CLIENT); additionalSettings.addAll(pluginsService.getPluginSettings()); final List additionalSettingsFilter = new ArrayList<>(pluginsService.getPluginSettingsFilter()); diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index ec5cb02c32379..faee4a0808baf 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -347,7 +347,7 @@ protected void doRun() { } static boolean isDedicatedVotingOnlyNode(Set roles) { - return roles.contains(DiscoveryNodeRole.MASTER_ROLE) + return (roles.contains(DiscoveryNodeRole.MASTER_ROLE) || roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) && roles.contains(DiscoveryNodeRole.DATA_ROLE) == false && roles.stream().anyMatch(role -> role.roleName().equals("voting_only")); } 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 a1aa38017a808..8da65ba13b9cb 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 @@ -113,7 +113,7 @@ public void testResolve() { "local", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode); @@ -122,7 +122,7 @@ public void testResolve() { "other1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion otherNode1Exclusion = new VotingConfigExclusion(otherNode1); @@ -131,7 +131,7 @@ public void testResolve() { "other2", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion otherNode2Exclusion = new VotingConfigExclusion(otherNode2); @@ -238,7 +238,7 @@ public void testResolveByNodeIds() { "nodeId1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion node1Exclusion = new VotingConfigExclusion(node1); @@ -248,7 +248,7 @@ public void testResolveByNodeIds() { "nodeId2", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2); @@ -258,7 +258,7 @@ public void testResolveByNodeIds() { "nodeId3", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); @@ -298,7 +298,7 @@ public void testResolveByNodeNames() { "nodeId1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion node1Exclusion = new VotingConfigExclusion(node1); @@ -308,7 +308,7 @@ public void testResolveByNodeNames() { "nodeId2", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2); @@ -318,7 +318,7 @@ public void testResolveByNodeNames() { "nodeId3", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); @@ -348,7 +348,7 @@ public void testResolveRemoveExistingVotingConfigExclusions() { "nodeId1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); @@ -357,7 +357,7 @@ public void testResolveRemoveExistingVotingConfigExclusions() { "nodeId2", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2); @@ -367,7 +367,7 @@ public void testResolveRemoveExistingVotingConfigExclusions() { "nodeId3", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); @@ -399,7 +399,7 @@ public void testResolveAndCheckMaximum() { "local", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode); @@ -408,7 +408,7 @@ public void testResolveAndCheckMaximum() { "other1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion otherNode1Exclusion = new VotingConfigExclusion(otherNode1); @@ -417,7 +417,7 @@ public void testResolveAndCheckMaximum() { "other2", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final VotingConfigExclusion otherNode2Exclusion = new VotingConfigExclusion(otherNode2); diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java index b4540a66c9264..a570db040a805 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java @@ -119,7 +119,7 @@ private static DiscoveryNode makeDiscoveryNode(String name) { name, buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); } 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 37cdda6c365b8..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 @@ -278,7 +278,7 @@ public void testShrinkIndexSettings() { private DiscoveryNode newNode(String nodeId) { final Set roles = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) ); return new DiscoveryNode(nodeId, buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); } 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 05f9ef8baed29..09ab2438bd106 100644 --- a/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/TransportMultiSearchActionTests.java @@ -263,7 +263,7 @@ public void testDefaultMaxConcurrentSearches() { "master", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ) ); 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..a8ad356e947b5 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 }; diff --git a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java index 4f68358b15b3d..49d4a8baf0a1c 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterChangedEventTests.java @@ -447,17 +447,17 @@ private static DiscoveryNodes createDiscoveryNodes(final int numNodes, final boo if (i == 0) { // the master node builder.masterNodeId(nodeId); - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } else if (i == 1) { // the alternate master node - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } else if (i == 2) { // we need at least one data node roles.add(DiscoveryNodeRole.DATA_ROLE); } else { // remaining nodes can be anything (except for master) if (randomBoolean()) { - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } if (randomBoolean()) { roles.add(DiscoveryNodeRole.DATA_ROLE); 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 a2fe39ef4531e..e94c5b8d22b88 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterBootstrapServiceTests.java @@ -111,7 +111,7 @@ private DiscoveryNode newDiscoveryNode(String nodeName) { randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); } @@ -521,7 +521,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { randomAlphaOfLength(10), buildNewFakeTransportAddress(), emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ), new DiscoveryNode( @@ -529,7 +529,7 @@ public void testCancelsBootstrapIfNodeMatchesMultipleRequirements() { randomAlphaOfLength(10), otherNode1.getAddress(), emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ) ).collect(Collectors.toList()) 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 c53c6fb8f4a18..464f4d0873fed 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/ClusterFormationFailureHelperTests.java @@ -287,7 +287,7 @@ public void testDescriptionOnUnhealthyNodes() { "local", buildNewFakeTransportAddress(), emptyMap(), - org.opensearch.common.collect.Set.of(DiscoveryNodeRole.MASTER_ROLE), + org.opensearch.common.collect.Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); clusterState = ClusterState.builder(ClusterName.DEFAULT) @@ -825,7 +825,7 @@ public void testDescriptionAfterBootstrapping() { emptyMap(), new HashSet<>( randomSubsetOf(DiscoveryNodeRole.BUILT_IN_ROLES).stream() - .filter(r -> r != DiscoveryNodeRole.MASTER_ROLE) + .filter(r -> r != DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) .collect(Collectors.toList()) ), Version.CURRENT diff --git a/server/src/test/java/org/opensearch/cluster/coordination/CoordinatorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/CoordinatorTests.java index 8164047178b1f..fa274c60aa6d9 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/CoordinatorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/CoordinatorTests.java @@ -1789,7 +1789,7 @@ private ClusterState buildNewClusterStateWithVotingConfigExclusion( "resolvableNodeId", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ) ) 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 bccd94e2c40bf..3b309908a1df0 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/NodeJoinTests.java @@ -248,7 +248,7 @@ protected DiscoveryNode newNode(int i) { protected DiscoveryNode newNode(int i, boolean master) { final Set roles; if (master) { - roles = singleton(DiscoveryNodeRole.MASTER_ROLE); + roles = singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } else { roles = Collections.emptySet(); } @@ -492,7 +492,7 @@ public void testJoinUpdateVotingConfigExclusion() throws Exception { "newNodeId", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); long newTerm = initialTerm + randomLongBetween(1, 10); diff --git a/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java b/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java index 2e4bba0ff1c7b..0f3db1517b5a9 100644 --- a/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java +++ b/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java @@ -87,7 +87,7 @@ public void testClusterHealth() { private ClusterState addNode(ClusterState clusterState, String nodeName, boolean isMaster) { DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.getNodes()); - nodeBuilder.add(newNode(nodeName, Collections.singleton(isMaster ? DiscoveryNodeRole.MASTER_ROLE : DiscoveryNodeRole.DATA_ROLE))); + nodeBuilder.add(newNode(nodeName, Collections.singleton(isMaster ? DiscoveryNodeRole.CLUSTER_MANAGER_ROLE : DiscoveryNodeRole.DATA_ROLE))); return ClusterState.builder(clusterState).nodes(nodeBuilder).build(); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java index 13bc108591f7f..db9122b8d8d27 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java @@ -146,7 +146,7 @@ public void testAutoExpandWhenNodeLeavesAndPossiblyRejoins() throws InterruptedE try { List allNodes = new ArrayList<>(); - DiscoveryNode localNode = createNode(DiscoveryNodeRole.MASTER_ROLE); // local node is the master + DiscoveryNode localNode = createNode(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); // local node is the master allNodes.add(localNode); int numDataNodes = randomIntBetween(3, 5); List dataNodes = new ArrayList<>(numDataNodes); @@ -245,7 +245,7 @@ public void testOnlyAutoExpandAllocationFilteringAfterAllNodesUpgraded() { List allNodes = new ArrayList<>(); DiscoveryNode oldNode = createNode( VersionUtils.randomVersionBetween(random(), LegacyESVersion.V_7_0_0, LegacyESVersion.V_7_5_1), - DiscoveryNodeRole.MASTER_ROLE, + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE ); // local node is the master allNodes.add(oldNode); @@ -266,7 +266,7 @@ public void testOnlyAutoExpandAllocationFilteringAfterAllNodesUpgraded() { state = cluster.reroute(state, new ClusterRerouteRequest()); } - DiscoveryNode newNode = createNode(LegacyESVersion.V_7_6_0, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE); // local + DiscoveryNode newNode = createNode(LegacyESVersion.V_7_6_0, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE); // local // node // is // the diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 5caa9eb212e15..f4b26c7159445 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -583,7 +583,7 @@ private void runPrepareResizeIndexSettingsTest( private DiscoveryNode newNode(String nodeId) { final Set roles = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) ); return new DiscoveryNode(nodeId, buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java index e4692fe7d21f7..1e52fa380793e 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/TemplateUpgradeServiceTests.java @@ -261,7 +261,7 @@ public void testUpdateTemplates() { } private static final Set MASTER_DATA_ROLES = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) ); @SuppressWarnings("unchecked") diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 006968a2f2ac2..3357db3e5bb3a 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -56,7 +56,7 @@ public void testIsIngestNode() { } public void testIsMasterNode() { - runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.MASTER_ROLE); + runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } public void testIsRemoteClusterClient() { diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java index 69cecade5b745..639c5c0388a91 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java @@ -361,7 +361,7 @@ Set matchingNodeIds(DiscoveryNodes nodes) { return Collections.singleton(nodes.getMasterNodeId()); } }, - MASTER_ELIGIBLE(DiscoveryNodeRole.MASTER_ROLE.roleName() + ":true") { + MASTER_ELIGIBLE(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + ":true") { @Override Set matchingNodeIds(DiscoveryNodes nodes) { Set ids = new HashSet<>(); diff --git a/server/src/test/java/org/opensearch/cluster/routing/DelayedAllocationServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/DelayedAllocationServiceTests.java index 1f79d52d84d74..1f45c5c1dfce4 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/DelayedAllocationServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/DelayedAllocationServiceTests.java @@ -262,7 +262,7 @@ public void testDelayedUnassignedScheduleRerouteAfterDelayedReroute() throws Exc .routingTable(RoutingTable.builder().addAsNew(metadata.index("short_delay")).addAsNew(metadata.index("long_delay")).build()) .nodes( DiscoveryNodes.builder() - .add(newNode("node0", singleton(DiscoveryNodeRole.MASTER_ROLE))) + .add(newNode("node0", singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) .localNodeId("node0") .masterNodeId("node0") .add(newNode("node1")) diff --git a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java index 7a5e24a7eeca2..d42c3e80c60c9 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/OperationRoutingTests.java @@ -778,7 +778,7 @@ private DiscoveryNode[] setupNodes() { "master", buildNewFakeTransportAddress(), Collections.emptyMap(), - Collections.singleton(DiscoveryNodeRole.MASTER_ROLE), + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); allNodes[i] = master; diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java index 28599ea0b9baa..34d5f4b93beed 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java @@ -188,7 +188,7 @@ public void testAllocateCommand() { .add(newNode("node1")) .add(newNode("node2")) .add(newNode("node3")) - .add(newNode("node4", singleton(DiscoveryNodeRole.MASTER_ROLE))) + .add(newNode("node4", singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ) .build(); clusterState = allocation.reroute(clusterState, "reroute"); @@ -748,7 +748,7 @@ public void testMoveShardToNonDataNode() { "test2", buildNewFakeTransportAddress(), emptyMap(), - new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), + new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), Version.CURRENT ); @@ -817,7 +817,7 @@ public void testMoveShardFromNonDataNode() { "test2", buildNewFakeTransportAddress(), emptyMap(), - new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), + new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), Version.CURRENT ); diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedNodeRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedNodeRoutingTests.java index f871d74d3d1a5..f60497b4108b7 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedNodeRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedNodeRoutingTests.java @@ -230,7 +230,7 @@ private static Version getNodeVersion(ShardRouting shardRouting, ClusterState st public ClusterState randomInitialClusterState() { List allNodes = new ArrayList<>(); - DiscoveryNode localNode = createNode(DiscoveryNodeRole.MASTER_ROLE); // local node is the master + DiscoveryNode localNode = createNode(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); // local node is the master allNodes.add(localNode); // at least two nodes that have the data role so that we can allocate shards allNodes.add(createNode(DiscoveryNodeRole.DATA_ROLE)); diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java index 038dd2713bbf9..a2fcf14638d45 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java @@ -1078,7 +1078,7 @@ public void testForSingleDataNode() { "node1", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); DiscoveryNode discoveryNode2 = new DiscoveryNode( @@ -1227,7 +1227,7 @@ public void testWatermarksEnabledForSingleDataNode() { "master", buildNewFakeTransportAddress(), emptyMap(), - singleton(DiscoveryNodeRole.MASTER_ROLE), + singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); DiscoveryNode dataNode = new DiscoveryNode( diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/RestoreInProgressAllocationDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/RestoreInProgressAllocationDeciderTests.java index 8917b9793e621..5b05cb3afd83e 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/RestoreInProgressAllocationDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/decider/RestoreInProgressAllocationDeciderTests.java @@ -199,7 +199,7 @@ private ClusterState createInitialClusterState() { RoutingTable routingTable = RoutingTable.builder().addAsNew(metadata.index("test")).build(); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder() - .add(newNode("master", Collections.singleton(DiscoveryNodeRole.MASTER_ROLE))) + .add(newNode("master", Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) .localNodeId("master") .masterNodeId("master") .build(); diff --git a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java index 011b5c8ea0e4e..e21ea27e12aff 100644 --- a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java @@ -530,7 +530,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException { .put( NodeRoles.removeRoles( settings, - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ) ) .build(); diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index 45ec6d8ebf75a..9897ad1a3650b 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -105,7 +105,7 @@ public void createNodePaths() throws IOException { dataNoMasterSettings = nonMasterNode(dataMasterSettings); noDataNoMasterSettings = removeRoles( dataMasterSettings, - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); noDataMasterSettings = masterNode(nonDataNode(dataMasterSettings)); diff --git a/server/src/test/java/org/opensearch/gateway/ClusterStateUpdatersTests.java b/server/src/test/java/org/opensearch/gateway/ClusterStateUpdatersTests.java index f6a8f1515002d..1f63dff04b3df 100644 --- a/server/src/test/java/org/opensearch/gateway/ClusterStateUpdatersTests.java +++ b/server/src/test/java/org/opensearch/gateway/ClusterStateUpdatersTests.java @@ -304,7 +304,7 @@ public void testSetLocalNode() { "node1", buildNewFakeTransportAddress(), Collections.emptyMap(), - Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE), + Sets.newHashSet(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); @@ -354,7 +354,7 @@ public void testHideStateIfNotRecovered() { "node1", buildNewFakeTransportAddress(), Collections.emptyMap(), - Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE), + Sets.newHashSet(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); final ClusterState updatedState = Function.identity() diff --git a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java index 23977877773f0..07207d595f204 100644 --- a/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java +++ b/server/src/test/java/org/opensearch/gateway/GatewayMetaStatePersistedStateTests.java @@ -97,7 +97,7 @@ public void setUp() throws Exception { "node1", buildNewFakeTransportAddress(), Collections.emptyMap(), - Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE), + Sets.newHashSet(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), Version.CURRENT ); clusterName = new ClusterName(randomAlphaOfLength(10)); diff --git a/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java b/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java index 5574e3c911213..7789054cfdc16 100644 --- a/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java +++ b/server/src/test/java/org/opensearch/indices/cluster/IndicesClusterStateServiceRandomUpdatesTests.java @@ -327,7 +327,7 @@ public ClusterState randomInitialClusterState( Supplier indicesServiceSupplier ) { List allNodes = new ArrayList<>(); - DiscoveryNode localNode = createNode(DiscoveryNodeRole.MASTER_ROLE); // local node is the master + DiscoveryNode localNode = createNode(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); // local node is the master allNodes.add(localNode); // at least two nodes that have the data role so that we can allocate shards allNodes.add(createNode(DiscoveryNodeRole.DATA_ROLE)); diff --git a/server/src/test/java/org/opensearch/persistent/PersistentTasksClusterServiceTests.java b/server/src/test/java/org/opensearch/persistent/PersistentTasksClusterServiceTests.java index de9dbdc2d5793..bb8f0405ecf7e 100644 --- a/server/src/test/java/org/opensearch/persistent/PersistentTasksClusterServiceTests.java +++ b/server/src/test/java/org/opensearch/persistent/PersistentTasksClusterServiceTests.java @@ -886,7 +886,7 @@ private String addTask(PersistentTasksCustomMetadata.Builder tasks, String param private DiscoveryNode newNode(String nodeId) { final Set roles = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) ); return new DiscoveryNode(nodeId, buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT); } diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index 74da44af28e1d..8eea5f6ab34eb 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -1500,7 +1500,7 @@ public TestClusterNode nodeById(final String nodeId) { } private TestClusterNode newMasterNode(String nodeName) throws IOException { - return newNode(nodeName, DiscoveryNodeRole.MASTER_ROLE); + return newNode(nodeName, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } private TestClusterNode newDataNode(String nodeName) throws IOException { diff --git a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java index d6dc56204f5da..be7e6ae58448c 100644 --- a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java +++ b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java @@ -258,7 +258,7 @@ public void testDefaultConnectionProfile() { profile = ConnectionProfile.buildDefaultConnectionProfile( removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ) ); assertEquals(10, profile.getNumConnections()); diff --git a/server/src/test/java/org/opensearch/transport/SniffConnectionStrategyTests.java b/server/src/test/java/org/opensearch/transport/SniffConnectionStrategyTests.java index 7e456fa86754f..1714f154036a5 100644 --- a/server/src/test/java/org/opensearch/transport/SniffConnectionStrategyTests.java +++ b/server/src/test/java/org/opensearch/transport/SniffConnectionStrategyTests.java @@ -747,7 +747,7 @@ public void testGetNodePredicateNodeRoles() { "id", address, Collections.emptyMap(), - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)), + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)), Version.CURRENT ); assertTrue(nodePredicate.test(dataMaster)); @@ -757,7 +757,7 @@ public void testGetNodePredicateNodeRoles() { "id", address, Collections.emptyMap(), - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE)), + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)), Version.CURRENT ); assertFalse(nodePredicate.test(dedicatedMaster)); @@ -777,7 +777,7 @@ public void testGetNodePredicateNodeRoles() { "id", address, Collections.emptyMap(), - new HashSet<>(Arrays.asList(DiscoveryNodeRole.INGEST_ROLE, DiscoveryNodeRole.MASTER_ROLE)), + new HashSet<>(Arrays.asList(DiscoveryNodeRole.INGEST_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)), Version.CURRENT ); assertTrue(nodePredicate.test(masterIngest)); @@ -855,7 +855,7 @@ public void testGetNodePredicatesCombination() { TransportAddress address = new TransportAddress(TransportAddress.META_ADDRESS, 0); Settings settings = Settings.builder().put("cluster.remote.node.attr", "gateway").build(); Predicate nodePredicate = SniffConnectionStrategy.getNodePredicate(settings); - Set dedicatedMasterRoles = new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE)); + Set dedicatedMasterRoles = new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); Set allRoles = DiscoveryNodeRole.BUILT_IN_ROLES; { DiscoveryNode node = new DiscoveryNode( diff --git a/test/framework/src/main/java/org/opensearch/cluster/OpenSearchAllocationTestCase.java b/test/framework/src/main/java/org/opensearch/cluster/OpenSearchAllocationTestCase.java index e806548cee088..e42dd42b18da5 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/OpenSearchAllocationTestCase.java +++ b/test/framework/src/main/java/org/opensearch/cluster/OpenSearchAllocationTestCase.java @@ -149,7 +149,7 @@ public static AllocationDeciders randomAllocationDeciders(Settings settings, Clu } protected static Set MASTER_DATA_ROLES = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE)) ); protected static DiscoveryNode newNode(String nodeId) { diff --git a/test/framework/src/main/java/org/opensearch/cluster/coordination/CoordinationStateTestCluster.java b/test/framework/src/main/java/org/opensearch/cluster/coordination/CoordinationStateTestCluster.java index 32ef47c4366a4..70430b0a2954c 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/coordination/CoordinationStateTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/cluster/coordination/CoordinationStateTestCluster.java @@ -164,10 +164,10 @@ void reboot() { final Set roles = new HashSet<>(localNode.getRoles()); if (randomBoolean()) { - if (roles.contains(DiscoveryNodeRole.MASTER_ROLE)) { - roles.remove(DiscoveryNodeRole.MASTER_ROLE); + if (roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + roles.remove(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } else { - roles.add(DiscoveryNodeRole.MASTER_ROLE); + roles.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } } diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 5ae441ed651b1..3128d113d473a 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -787,13 +787,13 @@ private static String getRoleSuffix(Settings settings) { String suffix = ""; // only add the suffixes if roles are explicitly defined if (settings.hasValue("nodes.roles")) { - if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE)) { - suffix = suffix + DiscoveryNodeRole.MASTER_ROLE.roleNameAbbreviation(); + if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) || DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + suffix = suffix + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleNameAbbreviation(); } if (DiscoveryNode.isDataNode(settings)) { suffix = suffix + DiscoveryNodeRole.DATA_ROLE.roleNameAbbreviation(); } - if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) == false && DiscoveryNode.isDataNode(settings) == false) { + if ((DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) == false && DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false) && DiscoveryNode.isDataNode(settings) == false) { suffix = suffix + "c"; } } @@ -1143,7 +1143,7 @@ private synchronized void reset(boolean wipeData) throws IOException { for (int i = numSharedDedicatedMasterNodes; i < numSharedDedicatedMasterNodes + numSharedDataNodes; i++) { final Settings nodeSettings = getNodeSettings(i, sharedNodesSeeds[i], Settings.EMPTY, defaultMinMasterNodes); if (numSharedDedicatedMasterNodes > 0) { - settings.add(removeRoles(nodeSettings, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE))); + settings.add(removeRoles(nodeSettings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE))))); } else { // if we don't have dedicated master nodes, keep things default settings.add(nodeSettings); @@ -2147,7 +2147,7 @@ public List startMasterOnlyNodes(int numNodes) { } public List startMasterOnlyNodes(int numNodes, Settings settings) { - return startNodes(numNodes, Settings.builder().put(onlyRole(settings, DiscoveryNodeRole.MASTER_ROLE)).build()); + return startNodes(numNodes, Settings.builder().put(onlyRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)).build()); } public List startDataOnlyNodes(int numNodes) { diff --git a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java index ed5f505e02304..72ad0c070831f 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java @@ -36,7 +36,9 @@ import org.opensearch.common.settings.Settings; import org.opensearch.node.NodeRoleSettings; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -166,7 +168,7 @@ public static Settings masterNode() { } public static Settings masterNode(final Settings settings) { - return addRoles(settings, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE)); + return addRoles(settings, Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); } public static Settings masterOnlyNode() { @@ -174,7 +176,7 @@ public static Settings masterOnlyNode() { } public static Settings masterOnlyNode(final Settings settings) { - return onlyRole(settings, DiscoveryNodeRole.MASTER_ROLE); + return onlyRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } public static Settings nonMasterNode() { @@ -182,7 +184,7 @@ public static Settings nonMasterNode() { } public static Settings nonMasterNode(final Settings settings) { - return removeRoles(settings, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE)); + return removeRoles(settings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)))); } public static Settings remoteClusterClientNode() { diff --git a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java index 7e38061392312..72523f898cedd 100644 --- a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java +++ b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java @@ -400,15 +400,15 @@ public Path nodeConfigPath(int nodeOrdinal) { cluster.beforeTest(random()); List roles = new ArrayList<>(); for (int i = 0; i < numNodes; i++) { - final DiscoveryNodeRole role = i == numNodes - 1 && roles.contains(DiscoveryNodeRole.MASTER_ROLE) == false - ? DiscoveryNodeRole.MASTER_ROLE + final DiscoveryNodeRole role = i == numNodes - 1 && roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false + ? DiscoveryNodeRole.CLUSTER_MANAGER_ROLE : // last node and still no master - randomFrom(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.INGEST_ROLE); + randomFrom(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.INGEST_ROLE); roles.add(role); } cluster.setBootstrapMasterNodeIndex( - randomIntBetween(0, (int) roles.stream().filter(role -> role.equals(DiscoveryNodeRole.MASTER_ROLE)).count() - 1) + randomIntBetween(0, (int) roles.stream().filter(role -> role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)).count() - 1) ); try { @@ -416,7 +416,7 @@ public Path nodeConfigPath(int nodeOrdinal) { for (int i = 0; i < numNodes; i++) { final DiscoveryNodeRole role = roles.get(i); final String node; - if (role == DiscoveryNodeRole.MASTER_ROLE) { + if (role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { node = cluster.startMasterOnlyNode(); } else if (role == DiscoveryNodeRole.DATA_ROLE) { node = cluster.startDataOnlyNode(); @@ -438,7 +438,7 @@ public Path nodeConfigPath(int nodeOrdinal) { DiscoveryNode node = cluster.getInstance(ClusterService.class, name).localNode(); List paths = Arrays.stream(getNodePaths(cluster, name)).map(Path::toString).collect(Collectors.toList()); if (node.isMasterNode()) { - result.computeIfAbsent(DiscoveryNodeRole.MASTER_ROLE, k -> new HashSet<>()).addAll(paths); + result.computeIfAbsent(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, k -> new HashSet<>()).addAll(paths); } else if (node.isDataNode()) { result.computeIfAbsent(DiscoveryNodeRole.DATA_ROLE, k -> new HashSet<>()).addAll(paths); } else { From 77520c80087dfd2519c6fac9657c9b8a6451a321 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 16:12:40 -0800 Subject: [PATCH 03/51] adjust format by spotlessApply Signed-off-by: Tianli Feng --- .../cluster/node/DiscoveryNodes.java | 67 ++++++++++--------- .../health/ClusterHealthAllocationTests.java | 4 +- .../metadata/AutoExpandReplicasTests.java | 14 ++-- .../allocation/AllocationCommandsTests.java | 8 ++- .../opensearch/env/NodeEnvironmentTests.java | 4 +- .../transport/ConnectionProfileTests.java | 4 +- .../opensearch/test/InternalTestCluster.java | 16 ++++- .../java/org/opensearch/test/NodeRoles.java | 5 +- 8 files changed, 75 insertions(+), 47 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java index d031667e6ba74..95f118ee58658 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java @@ -419,46 +419,47 @@ public String[] resolveNodes(String... nodes) { } else { resolvedNodesIds.removeAll(dataNodes.keys()); } - } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName) || DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(masterNodes.keys()); - } else { - resolvedNodesIds.removeAll(masterNodes.keys()); - } - } else if (DiscoveryNodeRole.INGEST_ROLE.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(ingestNodes.keys()); - } else { - resolvedNodesIds.removeAll(ingestNodes.keys()); - } - } else if (DiscoveryNode.COORDINATING_ONLY.equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(getCoordinatingOnlyNodes().keys()); + } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName) + || DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName().equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(masterNodes.keys()); + } else { + resolvedNodesIds.removeAll(masterNodes.keys()); + } + } else if (DiscoveryNodeRole.INGEST_ROLE.roleName().equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(ingestNodes.keys()); + } else { + resolvedNodesIds.removeAll(ingestNodes.keys()); + } + } else if (DiscoveryNode.COORDINATING_ONLY.equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(getCoordinatingOnlyNodes().keys()); + } else { + resolvedNodesIds.removeAll(getCoordinatingOnlyNodes().keys()); + } } else { - resolvedNodesIds.removeAll(getCoordinatingOnlyNodes().keys()); - } - } else { - for (DiscoveryNode node : this) { - for (DiscoveryNodeRole role : Sets.difference(node.getRoles(), DiscoveryNodeRole.BUILT_IN_ROLES)) { - if (role.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.add(node.getId()); - } else { - resolvedNodesIds.remove(node.getId()); + for (DiscoveryNode node : this) { + for (DiscoveryNodeRole role : Sets.difference(node.getRoles(), DiscoveryNodeRole.BUILT_IN_ROLES)) { + if (role.roleName().equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.add(node.getId()); + } else { + resolvedNodesIds.remove(node.getId()); + } } } } - } - for (DiscoveryNode node : this) { - for (Map.Entry entry : node.getAttributes().entrySet()) { - String attrName = entry.getKey(); - String attrValue = entry.getValue(); - if (Regex.simpleMatch(matchAttrName, attrName) && Regex.simpleMatch(matchAttrValue, attrValue)) { - resolvedNodesIds.add(node.getId()); + for (DiscoveryNode node : this) { + for (Map.Entry entry : node.getAttributes().entrySet()) { + String attrName = entry.getKey(); + String attrValue = entry.getValue(); + if (Regex.simpleMatch(matchAttrName, attrName) && Regex.simpleMatch(matchAttrValue, attrValue)) { + resolvedNodesIds.add(node.getId()); + } } } } - } } } } diff --git a/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java b/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java index 0f3db1517b5a9..2f05297146f8e 100644 --- a/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java +++ b/server/src/test/java/org/opensearch/cluster/health/ClusterHealthAllocationTests.java @@ -87,7 +87,9 @@ public void testClusterHealth() { private ClusterState addNode(ClusterState clusterState, String nodeName, boolean isMaster) { DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.getNodes()); - nodeBuilder.add(newNode(nodeName, Collections.singleton(isMaster ? DiscoveryNodeRole.CLUSTER_MANAGER_ROLE : DiscoveryNodeRole.DATA_ROLE))); + nodeBuilder.add( + newNode(nodeName, Collections.singleton(isMaster ? DiscoveryNodeRole.CLUSTER_MANAGER_ROLE : DiscoveryNodeRole.DATA_ROLE)) + ); return ClusterState.builder(clusterState).nodes(nodeBuilder).build(); } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java index db9122b8d8d27..3265653c046eb 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java @@ -266,11 +266,15 @@ public void testOnlyAutoExpandAllocationFilteringAfterAllNodesUpgraded() { state = cluster.reroute(state, new ClusterRerouteRequest()); } - DiscoveryNode newNode = createNode(LegacyESVersion.V_7_6_0, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE); // local - // node - // is - // the - // master + DiscoveryNode newNode = createNode( + LegacyESVersion.V_7_6_0, + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, + DiscoveryNodeRole.DATA_ROLE + ); // local + // node + // is + // the + // master state = cluster.addNodes(state, Collections.singletonList(newNode)); diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java index 34d5f4b93beed..cda0f1afaf62c 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java @@ -748,7 +748,9 @@ public void testMoveShardToNonDataNode() { "test2", buildNewFakeTransportAddress(), emptyMap(), - new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), + new HashSet<>( + randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE))) + ), Version.CURRENT ); @@ -817,7 +819,9 @@ public void testMoveShardFromNonDataNode() { "test2", buildNewFakeTransportAddress(), emptyMap(), - new HashSet<>(randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE)))), + new HashSet<>( + randomSubsetOf(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.INGEST_ROLE))) + ), Version.CURRENT ); diff --git a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java index e21ea27e12aff..532cce709097a 100644 --- a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java @@ -530,7 +530,9 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException { .put( NodeRoles.removeRoles( settings, - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + ) ) ) .build(); diff --git a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java index be7e6ae58448c..28cf20e20753e 100644 --- a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java +++ b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java @@ -258,7 +258,9 @@ public void testDefaultConnectionProfile() { profile = ConnectionProfile.buildDefaultConnectionProfile( removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + ) ) ); assertEquals(10, profile.getNumConnections()); diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 3128d113d473a..f9f6422a49069 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -787,13 +787,16 @@ private static String getRoleSuffix(Settings settings) { String suffix = ""; // only add the suffixes if roles are explicitly defined if (settings.hasValue("nodes.roles")) { - if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) || DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) + || DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { suffix = suffix + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleNameAbbreviation(); } if (DiscoveryNode.isDataNode(settings)) { suffix = suffix + DiscoveryNodeRole.DATA_ROLE.roleNameAbbreviation(); } - if ((DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) == false && DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false) && DiscoveryNode.isDataNode(settings) == false) { + if ((DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) == false + && DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false) + && DiscoveryNode.isDataNode(settings) == false) { suffix = suffix + "c"; } } @@ -1143,7 +1146,14 @@ private synchronized void reset(boolean wipeData) throws IOException { for (int i = numSharedDedicatedMasterNodes; i < numSharedDedicatedMasterNodes + numSharedDataNodes; i++) { final Settings nodeSettings = getNodeSettings(i, sharedNodesSeeds[i], Settings.EMPTY, defaultMinMasterNodes); if (numSharedDedicatedMasterNodes > 0) { - settings.add(removeRoles(nodeSettings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE))))); + settings.add( + removeRoles( + nodeSettings, + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)) + ) + ) + ); } else { // if we don't have dedicated master nodes, keep things default settings.add(nodeSettings); diff --git a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java index 72ad0c070831f..cf7d7467c1f40 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java @@ -184,7 +184,10 @@ public static Settings nonMasterNode() { } public static Settings nonMasterNode(final Settings settings) { - return removeRoles(settings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)))); + return removeRoles( + settings, + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE))) + ); } public static Settings remoteClusterClientNode() { From 085f07fa5a7bb8f8dac09c9c09e24f11de2f03b5 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 17:20:38 -0800 Subject: [PATCH 04/51] add hack in setAdditionalRoles() to allow CLUSTER_MANAGER_ROLE having same abbr name with MASTER_ROLE Signed-off-by: Tianli Feng --- .../opensearch/cluster/node/DiscoveryNode.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index e2965a8fe3026..024074a6d1c48 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -577,9 +577,13 @@ public static Set getPossibleRoles() { public static void setAdditionalRoles(final Set additionalRoles) { assert additionalRoles.stream().allMatch(r -> r.legacySetting() == null || r.legacySetting().isDeprecated()) : additionalRoles; - final Map roleNameToPossibleRoles = rolesToMap( - Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream()) + // TODO: Remove 'new HashMap<>()' and only keep 'rolesToMap(...)' after removing the MASTER_ROLE. + // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. + final Map roleNameToPossibleRoles = new HashMap<>(rolesToMap( + Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) ); + // TODO: Remove this line after removing the MASTER_ROLE. + roleNameToPossibleRoles.remove(DiscoveryNodeRole.MASTER_ROLE.roleName()); // collect the abbreviation names into a map to ensure that there are not any duplicate abbreviations final Map roleNameAbbreviationToPossibleRoles = Collections.unmodifiableMap( roleNameToPossibleRoles.values() @@ -591,7 +595,11 @@ public static void setAdditionalRoles(final Set additionalRol + "], roles by name abbreviation [" + roleNameAbbreviationToPossibleRoles + "]"; - roleMap = roleNameToPossibleRoles; + // TODO: Remove this line after removing the MASTER_ROLE. + roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); + // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing the MASTER_ROLE, + // because the map roleNameToPossibleRoles is supposed to be immutable. + roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); } public static Set getPossibleRoleNames() { @@ -618,5 +626,4 @@ public String roleName() { } } - } From d9aec7398bc43def798e9dab423e2274fe9c21c4 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 17:30:33 -0800 Subject: [PATCH 05/51] adjust format by spotlessApply Signed-off-by: Tianli Feng --- .../org/opensearch/cluster/node/DiscoveryNode.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 024074a6d1c48..8e0763e046363 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -578,9 +578,9 @@ public static Set getPossibleRoles() { public static void setAdditionalRoles(final Set additionalRoles) { assert additionalRoles.stream().allMatch(r -> r.legacySetting() == null || r.legacySetting().isDeprecated()) : additionalRoles; // TODO: Remove 'new HashMap<>()' and only keep 'rolesToMap(...)' after removing the MASTER_ROLE. - // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. - final Map roleNameToPossibleRoles = new HashMap<>(rolesToMap( - Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) + // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. + final Map roleNameToPossibleRoles = new HashMap<>( + rolesToMap(Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) ); // TODO: Remove this line after removing the MASTER_ROLE. roleNameToPossibleRoles.remove(DiscoveryNodeRole.MASTER_ROLE.roleName()); @@ -597,8 +597,8 @@ public static void setAdditionalRoles(final Set additionalRol + "]"; // TODO: Remove this line after removing the MASTER_ROLE. roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); - // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing the MASTER_ROLE, - // because the map roleNameToPossibleRoles is supposed to be immutable. + // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing the MASTER_ROLE, + // because the map roleNameToPossibleRoles is supposed to be immutable. roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); } From 02f8f862ac07d8e0dd92ab877652733dd5e72938 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 17:49:55 -0800 Subject: [PATCH 06/51] Fix DiscoveryNodeRoleSettingTests Signed-off-by: Tianli Feng --- .../node/DiscoveryNodeRoleSettingTests.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 3357db3e5bb3a..2ead54186a67e 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -36,7 +36,9 @@ import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.function.Predicate; import static org.opensearch.test.NodeRoles.onlyRole; @@ -83,7 +85,20 @@ private void runRoleTest(final Predicate predicate, final DiscoveryNod assertTrue(predicate.test(onlyRole(role))); assertNoDeprecationWarnings(); - assertFalse(predicate.test(removeRoles(Collections.singleton(role)))); + // TODO: Remove the statement in 'if' after removing the MASTER_ROLE. + if (role == DiscoveryNodeRole.MASTER_ROLE || role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { + assertFalse( + predicate.test( + removeRoles( + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)) + ) + ) + ) + ); + } else { + assertFalse(predicate.test(removeRoles(Collections.singleton(role)))); + } assertNoDeprecationWarnings(); final Settings settings = Settings.builder().put(onlyRole(role)).put(role.legacySetting().getKey(), randomBoolean()).build(); From 573380fc0f36d59395b2be777e56320055e3fe09 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 17:53:26 -0800 Subject: [PATCH 07/51] Change todo expression Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNode.java | 8 ++++---- .../cluster/node/DiscoveryNodeRoleSettingTests.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 8e0763e046363..e0ea14a2419e8 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -577,12 +577,12 @@ public static Set getPossibleRoles() { public static void setAdditionalRoles(final Set additionalRoles) { assert additionalRoles.stream().allMatch(r -> r.legacySetting() == null || r.legacySetting().isDeprecated()) : additionalRoles; - // TODO: Remove 'new HashMap<>()' and only keep 'rolesToMap(...)' after removing the MASTER_ROLE. + // TODO: Remove 'new HashMap<>()' and only keep 'rolesToMap(...)' after removing DiscoveryNodeRole.MASTER_ROLE // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. final Map roleNameToPossibleRoles = new HashMap<>( rolesToMap(Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) ); - // TODO: Remove this line after removing the MASTER_ROLE. + // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE roleNameToPossibleRoles.remove(DiscoveryNodeRole.MASTER_ROLE.roleName()); // collect the abbreviation names into a map to ensure that there are not any duplicate abbreviations final Map roleNameAbbreviationToPossibleRoles = Collections.unmodifiableMap( @@ -595,9 +595,9 @@ public static void setAdditionalRoles(final Set additionalRol + "], roles by name abbreviation [" + roleNameAbbreviationToPossibleRoles + "]"; - // TODO: Remove this line after removing the MASTER_ROLE. + // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); - // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing the MASTER_ROLE, + // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing DiscoveryNodeRole.MASTER_ROLE // because the map roleNameToPossibleRoles is supposed to be immutable. roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); } diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 2ead54186a67e..817e103c3a6dc 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -85,7 +85,7 @@ private void runRoleTest(final Predicate predicate, final DiscoveryNod assertTrue(predicate.test(onlyRole(role))); assertNoDeprecationWarnings(); - // TODO: Remove the statement in 'if' after removing the MASTER_ROLE. + // TODO: Remove the statement in 'if' after removing DiscoveryNodeRole.MASTER_ROLE if (role == DiscoveryNodeRole.MASTER_ROLE || role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { assertFalse( predicate.test( From bd395af5962a937407231f330f009e9fa715e95c Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 10 Mar 2022 01:55:09 +0000 Subject: [PATCH 08/51] adjust format by spotlessApply Signed-off-by: Tianli Feng --- .../main/java/org/opensearch/cluster/node/DiscoveryNode.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index e0ea14a2419e8..4ad80bee2dce4 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -597,7 +597,8 @@ public static void setAdditionalRoles(final Set additionalRol + "]"; // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); - // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing DiscoveryNodeRole.MASTER_ROLE + // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing + // DiscoveryNodeRole.MASTER_ROLE // because the map roleNameToPossibleRoles is supposed to be immutable. roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); } From 83460f41c7dfc2bdf2af53e8eb03b020fed81fc3 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 10 Mar 2022 02:36:08 +0000 Subject: [PATCH 09/51] remove MASTER_ROLE from built-in roles Signed-off-by: Tianli Feng --- .../main/java/org/opensearch/cluster/node/DiscoveryNode.java | 5 +---- .../java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 4ad80bee2dce4..68ce0526d2d34 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -582,8 +582,6 @@ public static void setAdditionalRoles(final Set additionalRol final Map roleNameToPossibleRoles = new HashMap<>( rolesToMap(Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) ); - // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE - roleNameToPossibleRoles.remove(DiscoveryNodeRole.MASTER_ROLE.roleName()); // collect the abbreviation names into a map to ensure that there are not any duplicate abbreviations final Map roleNameAbbreviationToPossibleRoles = Collections.unmodifiableMap( roleNameToPossibleRoles.values() @@ -597,8 +595,7 @@ public static void setAdditionalRoles(final Set additionalRol + "]"; // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); - // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing - // DiscoveryNodeRole.MASTER_ROLE + // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing MASTER_ROLE // because the map roleNameToPossibleRoles is supposed to be immutable. roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); } diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 7029454297440..1e2ab2418d9b0 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -238,7 +238,7 @@ public Setting legacySetting() { * The built-in node roles. */ public static SortedSet BUILT_IN_ROLES = Collections.unmodifiableSortedSet( - new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)) + new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)) ); /** From 0940b41e8720decbc5c68fd45b8156c2c0cf2dc1 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 10 Mar 2022 04:33:54 +0000 Subject: [PATCH 10/51] fix unit tests Signed-off-by: Tianli Feng --- .../java/org/opensearch/env/NodeEnvironmentIT.java | 4 ++-- .../java/org/opensearch/env/NodeRepurposeCommandIT.java | 2 +- .../test/java/org/opensearch/env/NodeEnvironmentTests.java | 6 ++---- .../java/org/opensearch/env/NodeRepurposeCommandTests.java | 5 ++++- .../org/opensearch/transport/ConnectionProfileTests.java | 4 +--- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java index aa91179c9b7ee..3b3e2e8418bf4 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java @@ -88,14 +88,14 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { public Settings onNodeStopped(String nodeName) { return NodeRoles.removeRoles( Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) ) ); } }) ); if (writeDanglingIndices) { - assertThat(ex.getMessage(), startsWith("node does not have the data and master roles but has index metadata")); + assertThat(ex.getMessage(), startsWith("node does not have the data and cluster_manager roles but has index metadata")); } else { assertThat(ex.getMessage(), startsWith("node does not have the data role but has shard data")); } diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java index 80acc98293bd6..4b902f688d780 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java @@ -75,7 +75,7 @@ public void testRepurpose() throws Exception { final Settings dataNodeDataPathSettings = internalCluster().dataPathSettings(dataNode); final Settings noMasterNoDataSettings = NodeRoles.removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); final Settings noMasterNoDataSettingsForMasterNode = Settings.builder() diff --git a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java index 532cce709097a..c2e1a38b90d84 100644 --- a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java @@ -530,9 +530,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException { .put( NodeRoles.removeRoles( settings, - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) - ) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ) ) .build(); @@ -604,7 +602,7 @@ private void verifyFailsOnMetadata(Settings settings, Path indexPath) { ); assertThat(ex.getMessage(), containsString(indexPath.resolve(MetadataStateFormat.STATE_DIR_NAME).toAbsolutePath().toString())); - assertThat(ex.getMessage(), startsWith("node does not have the data and master roles but has index metadata")); + assertThat(ex.getMessage(), startsWith("node does not have the data and cluster_manager roles but has index metadata")); } /** Converts an array of Strings to an array of Paths, adding an additional child if specified */ diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index 9897ad1a3650b..b3a34259c0aea 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -105,9 +105,10 @@ public void createNodePaths() throws IOException { dataNoMasterSettings = nonMasterNode(dataMasterSettings); noDataNoMasterSettings = removeRoles( dataMasterSettings, + //Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); - + logger.info("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); noDataMasterSettings = masterNode(nonDataNode(dataMasterSettings)); } @@ -119,6 +120,8 @@ public void testEarlyExitNoCleanup() throws Exception { } public void testNothingToCleanup() throws Exception { + logger.info("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); + System.out.println("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); verifyNoQuestions(noDataNoMasterSettings, containsString(NO_DATA_TO_CLEAN_UP_FOUND)); verifyNoQuestions(noDataMasterSettings, containsString(NO_SHARD_DATA_TO_CLEAN_UP_FOUND)); diff --git a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java index 28cf20e20753e..5fed2c6abae89 100644 --- a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java +++ b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java @@ -258,9 +258,7 @@ public void testDefaultConnectionProfile() { profile = ConnectionProfile.buildDefaultConnectionProfile( removeRoles( - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) - ) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ) ); assertEquals(10, profile.getNumConnections()); From 41806f0396a2eba6bc1099e65e3684d1c2d819f2 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 21:12:28 -0800 Subject: [PATCH 11/51] Fix unit tests Signed-off-by: Tianli Feng --- .../org/opensearch/env/NodeEnvironmentIT.java | 2 +- .../opensearch/env/NodeRepurposeCommandIT.java | 2 +- .../org/opensearch/node/NodeRoleSettings.java | 2 ++ .../node/DiscoveryNodeRoleSettingTests.java | 17 +---------------- .../opensearch/env/NodeEnvironmentTests.java | 4 +++- .../env/NodeRepurposeCommandTests.java | 7 +++---- .../transport/ConnectionProfileTests.java | 4 +++- 7 files changed, 14 insertions(+), 24 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java index 3b3e2e8418bf4..89adc15e41cd6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java @@ -88,7 +88,7 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { public Settings onNodeStopped(String nodeName) { return NodeRoles.removeRoles( Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) ) ); } diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java index 4b902f688d780..80acc98293bd6 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java @@ -75,7 +75,7 @@ public void testRepurpose() throws Exception { final Settings dataNodeDataPathSettings = internalCluster().dataPathSettings(dataNode); final Settings noMasterNoDataSettings = NodeRoles.removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); final Settings noMasterNoDataSettingsForMasterNode = Settings.builder() diff --git a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java index 1e29f22b140f9..cdb0e1ad71389 100644 --- a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java +++ b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java @@ -49,6 +49,8 @@ public class NodeRoleSettings { settings -> DiscoveryNode.getPossibleRoles() .stream() .filter(role -> role.isEnabledByDefault(settings)) + // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE. As of 2.0, MASTER_ROLE isn't needed to be assigned. + .filter(role -> !role.equals(DiscoveryNodeRole.MASTER_ROLE)) .map(DiscoveryNodeRole::roleName) .collect(Collectors.toList()), roles -> {}, diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 817e103c3a6dc..3357db3e5bb3a 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -36,9 +36,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.function.Predicate; import static org.opensearch.test.NodeRoles.onlyRole; @@ -85,20 +83,7 @@ private void runRoleTest(final Predicate predicate, final DiscoveryNod assertTrue(predicate.test(onlyRole(role))); assertNoDeprecationWarnings(); - // TODO: Remove the statement in 'if' after removing DiscoveryNodeRole.MASTER_ROLE - if (role == DiscoveryNodeRole.MASTER_ROLE || role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { - assertFalse( - predicate.test( - removeRoles( - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)) - ) - ) - ) - ); - } else { - assertFalse(predicate.test(removeRoles(Collections.singleton(role)))); - } + assertFalse(predicate.test(removeRoles(Collections.singleton(role)))); assertNoDeprecationWarnings(); final Settings settings = Settings.builder().put(onlyRole(role)).put(role.legacySetting().getKey(), randomBoolean()).build(); diff --git a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java index c2e1a38b90d84..c59f5e3607e10 100644 --- a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java @@ -530,7 +530,9 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException { .put( NodeRoles.removeRoles( settings, - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + ) ) ) .build(); diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index b3a34259c0aea..a1ead721c16e4 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -105,10 +105,9 @@ public void createNodePaths() throws IOException { dataNoMasterSettings = nonMasterNode(dataMasterSettings); noDataNoMasterSettings = removeRoles( dataMasterSettings, - //Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); - logger.info("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); + logger.info("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); noDataMasterSettings = masterNode(nonDataNode(dataMasterSettings)); } @@ -120,8 +119,8 @@ public void testEarlyExitNoCleanup() throws Exception { } public void testNothingToCleanup() throws Exception { - logger.info("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); - System.out.println("### noDataNoMasterSettings: "+ noDataNoMasterSettings.get("node.roles")); + logger.info("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); + System.out.println("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); verifyNoQuestions(noDataNoMasterSettings, containsString(NO_DATA_TO_CLEAN_UP_FOUND)); verifyNoQuestions(noDataMasterSettings, containsString(NO_SHARD_DATA_TO_CLEAN_UP_FOUND)); diff --git a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java index 5fed2c6abae89..28cf20e20753e 100644 --- a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java +++ b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java @@ -258,7 +258,9 @@ public void testDefaultConnectionProfile() { profile = ConnectionProfile.buildDefaultConnectionProfile( removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + ) ) ); assertEquals(10, profile.getNumConnections()); From 11c4ff590c072e213171b0789822e6a063e8abae Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 21:25:56 -0800 Subject: [PATCH 12/51] Adjust format Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNode.java | 1 + .../cluster/metadata/AutoExpandReplicasTests.java | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 68ce0526d2d34..5b23378d881ef 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -624,4 +624,5 @@ public String roleName() { } } + } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java index 3265653c046eb..d6c62e4bb0903 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/AutoExpandReplicasTests.java @@ -270,11 +270,7 @@ public void testOnlyAutoExpandAllocationFilteringAfterAllNodesUpgraded() { LegacyESVersion.V_7_6_0, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE - ); // local - // node - // is - // the - // master + ); // local node is the cluster_manager state = cluster.addNodes(state, Collections.singletonList(newNode)); From 8cc54e3cde9479bb33b38d56d744e67c55a845a9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 21:51:56 -0800 Subject: [PATCH 13/51] Remove temprorary log message Signed-off-by: Tianli Feng --- .../test/java/org/opensearch/env/NodeRepurposeCommandTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index a1ead721c16e4..f00d9af6014b8 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -119,8 +119,6 @@ public void testEarlyExitNoCleanup() throws Exception { } public void testNothingToCleanup() throws Exception { - logger.info("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); - System.out.println("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); verifyNoQuestions(noDataNoMasterSettings, containsString(NO_DATA_TO_CLEAN_UP_FOUND)); verifyNoQuestions(noDataMasterSettings, containsString(NO_SHARD_DATA_TO_CLEAN_UP_FOUND)); From f06766e9bbd367cdb99f07811affa6ddd50f561e Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 21:56:17 -0800 Subject: [PATCH 14/51] Remove temprorary log message Signed-off-by: Tianli Feng --- .../test/java/org/opensearch/env/NodeRepurposeCommandTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index f00d9af6014b8..f2a8a7d58ff24 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -107,7 +107,6 @@ public void createNodePaths() throws IOException { dataMasterSettings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); - logger.info("### noDataNoMasterSettings: " + noDataNoMasterSettings.get("node.roles")); noDataMasterSettings = masterNode(nonDataNode(dataMasterSettings)); } From 70c8f4a27720cca98d7b580bf81e6d7aaccdbe0a Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 22:51:58 -0800 Subject: [PATCH 15/51] Remove duplicate m in node.role of cat nodes API Signed-off-by: Tianli Feng --- .../org/opensearch/rest/action/cat/RestNodesAction.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index bce9b2d6b7e9d..a1b0f55826714 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -419,7 +419,13 @@ Table buildTable( if (node.getRoles().isEmpty()) { roles = "-"; } else { - roles = node.getRoles().stream().map(DiscoveryNodeRole::roleNameAbbreviation).sorted().collect(Collectors.joining()); + // TODO: Remove '.distinct()' after removing DiscoveryNodeRole.MASTER_ROLE, because the role abbr name should be unique. + roles = node.getRoles() + .stream() + .map(DiscoveryNodeRole::roleNameAbbreviation) + .sorted() + .distinct() + .collect(Collectors.joining()); } table.addCell(roles); table.addCell(masterId == null ? "x" : masterId.equals(node.getId()) ? "*" : "-"); From e8e2e309a3d1c58598ba7b805741cb0ccf4cb73f Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 9 Mar 2022 23:24:52 -0800 Subject: [PATCH 16/51] Add yaml rest test for node role cluster_manager Signed-off-by: Tianli Feng --- .../test/cluster.stats/10_basic.yml | 19 +++++++++++++++++ .../test/nodes.info/10_basic.yml | 21 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.stats/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.stats/10_basic.yml index a0432fa7aa558..6e149b297d843 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.stats/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.stats/10_basic.yml @@ -91,3 +91,22 @@ cluster.stats: {} - is_true: nodes.packaging_types + +--- +"get cluster stats nodes count with both master and cluster_manager": + - skip: + version: " - 1.4.99" + reason: "node role cluster_manager is added in 2.0.0" + + - do: + cluster.stats: {} + + - set: + nodes.count.cluster_manager: cluster_manager_count + + - gte: { nodes.count.total: 1} + - match: { nodes.count.cluster_manager: $cluster_manager_count } + - match: { nodes.count.master: $cluster_manager_count } + - gte: { nodes.count.data: 1} + - gte: { nodes.count.ingest: 0} + - gte: { nodes.count.coordinating_only: 0} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index 2a4bbfe0a20ba..4af8663a05bf0 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -14,7 +14,7 @@ setup: --- "node_info role test": - skip: - version: " - 7.7.99" + version: " - 7.7.99 , 2.0.0 - " reason: "node roles were not sorted before 7.8.0" - do: @@ -28,4 +28,23 @@ setup: - match: { nodes.$node_id.roles.1: "ingest" } - match: { nodes.$node_id.roles.2: "master" } - match: { nodes.$node_id.roles.3: "remote_cluster_client" } + +--- +"node_info role test with both master and cluster_manager": + - skip: + version: " - 1.4.99" + reason: "node role cluster_manager is added in 2.0.0" + + - do: + nodes.info: {} + - set: + nodes._arbitrary_key_: node_id + + - is_true: nodes.$node_id.roles + # the roles output is sorted + - match: { nodes.$node_id.roles.0: "cluster_manager" } + - match: { nodes.$node_id.roles.1: "data" } + - match: { nodes.$node_id.roles.2: "ingest" } + - match: { nodes.$node_id.roles.3: "master" } + - match: { nodes.$node_id.roles.4: "remote_cluster_client" } From 46098cd4b2c6cb73e53b7fe67d14c7923372af28 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 10 Mar 2022 14:14:26 -0800 Subject: [PATCH 17/51] Allow _cluster_manager as a value of node filter Signed-off-by: Tianli Feng --- .../main/java/org/opensearch/cluster/node/DiscoveryNodes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java index 95f118ee58658..e2ee3d8551994 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java @@ -370,7 +370,7 @@ public DiscoveryNode resolveNode(String node) { * Works by tracking the current set of nodes and applying each node specification in sequence. The set starts out empty and each node * specification may either add or remove nodes. For instance: * - * - _local, _master and _all respectively add to the subset the local node, the currently-elected master, and all the nodes + * - _local, _cluster_manager (_master) and _all respectively add to the subset the local node, the currently-elected cluster_manager, and all the nodes * - node IDs, names, hostnames and IP addresses all add to the subset any nodes which match * - a wildcard-based pattern of the form "attr*:value*" adds to the subset all nodes with a matching attribute with a matching value * - role:true adds to the subset all nodes with a matching role @@ -393,7 +393,7 @@ public String[] resolveNodes(String... nodes) { if (localNodeId != null) { resolvedNodesIds.add(localNodeId); } - } else if (nodeId.equals("_master")) { + } else if (nodeId.equals("_master") || nodeId.equals("_cluster_manager")) { String masterNodeId = getMasterNodeId(); if (masterNodeId != null) { resolvedNodesIds.add(masterNodeId); From 73b46523f49294ed127fba5a1d25ccf703346a91 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 17:43:42 -0800 Subject: [PATCH 18/51] Add compatitabilityRole to cluster_manager role Signed-off-by: Tianli Feng --- .../opensearch/cluster/node/DiscoveryNodeRole.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 1e2ab2418d9b0..e6ee1573a3b3d 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -207,7 +207,7 @@ public Setting legacySetting() { }; /** - * Represents the role for a cluster_manager-eligible node. + * Represents the role for a cluster-manager-eligible node. */ public static final DiscoveryNodeRole CLUSTER_MANAGER_ROLE = new DiscoveryNodeRole("cluster_manager", "m") { @@ -217,6 +217,15 @@ public Setting legacySetting() { return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope); } + @Override + public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { + if (nodeVersion.onOrAfter(Version.V_2_0_0)) { + return this; + } else { + return DiscoveryNodeRole.MASTER_ROLE; + } + } + }; public static final DiscoveryNodeRole REMOTE_CLUSTER_CLIENT_ROLE = new DiscoveryNodeRole("remote_cluster_client", "r") { From d1acef0226c8f3926f51e7657e04679fd24cc6aa Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 20:31:00 -0800 Subject: [PATCH 19/51] Revert adding isClusterManagerEligible() Signed-off-by: Tianli Feng --- .../src/main/java/org/opensearch/client/Node.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/client/rest/src/main/java/org/opensearch/client/Node.java b/client/rest/src/main/java/org/opensearch/client/Node.java index cf304015ed54d..1ededeb6d11dd 100644 --- a/client/rest/src/main/java/org/opensearch/client/Node.java +++ b/client/rest/src/main/java/org/opensearch/client/Node.java @@ -209,22 +209,10 @@ public Roles(final Set roles) { this.roles = new TreeSet<>(roles); } - /** - * Returns whether or not the node could be elected master. - * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #isClusterManagerEligible()} - */ - @Deprecated public boolean isMasterEligible() { return roles.contains("master") || roles.contains("cluster_manager"); } - /** - * Returns whether or not the node could be elected cluster manager. - */ - public boolean isClusterManagerEligible() { - return roles.contains("master") || roles.contains("cluster_manager"); - } - /** * Returns whether or not the node stores data. */ From 812aa39ec46cba29c2a8bc9f09cfcd3d2a0037f2 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 20:47:53 -0800 Subject: [PATCH 20/51] Remove master role from the default value of node.roles Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 2 +- server/src/main/java/org/opensearch/node/NodeRoleSettings.java | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index e6ee1573a3b3d..a74e10562f32f 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -201,7 +201,7 @@ public Setting legacySetting() { @Override public Setting legacySetting() { // copy the setting here so we can mark it private in org.opensearch.node.Node - return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope); + return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope); } }; diff --git a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java index cdb0e1ad71389..1e29f22b140f9 100644 --- a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java +++ b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java @@ -49,8 +49,6 @@ public class NodeRoleSettings { settings -> DiscoveryNode.getPossibleRoles() .stream() .filter(role -> role.isEnabledByDefault(settings)) - // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE. As of 2.0, MASTER_ROLE isn't needed to be assigned. - .filter(role -> !role.equals(DiscoveryNodeRole.MASTER_ROLE)) .map(DiscoveryNodeRole::roleName) .collect(Collectors.toList()), roles -> {}, From d15e44f49cf4158692103a0dec925fd755771a37 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 21:01:55 -0800 Subject: [PATCH 21/51] Revert a mistake of removing javadoc comment Signed-off-by: Tianli Feng --- client/rest/src/main/java/org/opensearch/client/Node.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/rest/src/main/java/org/opensearch/client/Node.java b/client/rest/src/main/java/org/opensearch/client/Node.java index 1ededeb6d11dd..c982ae8eb931f 100644 --- a/client/rest/src/main/java/org/opensearch/client/Node.java +++ b/client/rest/src/main/java/org/opensearch/client/Node.java @@ -209,6 +209,9 @@ public Roles(final Set roles) { this.roles = new TreeSet<>(roles); } + /** + * Returns whether or not the node could be elected master. + */ public boolean isMasterEligible() { return roles.contains("master") || roles.contains("cluster_manager"); } From 0189e86c65e2680b1f9bcf18227c1596d2ccca71 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 21:06:44 -0800 Subject: [PATCH 22/51] Add a comment of setting a default value of a temp setting to false Signed-off-by: Tianli Feng --- .../main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index a74e10562f32f..7d71078e7abfe 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -201,6 +201,7 @@ public Setting legacySetting() { @Override public Setting legacySetting() { // copy the setting here so we can mark it private in org.opensearch.node.Node + // As of 2.0, set the default value to 'false', so that MASTER_ROLE isn't added as a default value of NODE_ROLES_SETTING return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope); } From 14ef7c5f9c8d3a83b512a7578a0f0c78d2930253 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 11 Mar 2022 22:25:54 -0800 Subject: [PATCH 23/51] Fix cluster stats nodes count Signed-off-by: Tianli Feng --- .../resources/rest-api-spec/test/nodes.info/10_basic.yml | 5 ++--- .../action/admin/cluster/stats/ClusterStatsNodes.java | 9 ++++++++- .../cluster/node/DiscoveryNodeRoleSettingTests.java | 8 ++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index 4af8663a05bf0..bca5d1e2dc44a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -30,7 +30,7 @@ setup: - match: { nodes.$node_id.roles.3: "remote_cluster_client" } --- -"node_info role test with both master and cluster_manager": +"node_info role test with cluster_manager": - skip: version: " - 1.4.99" reason: "node role cluster_manager is added in 2.0.0" @@ -45,6 +45,5 @@ setup: - match: { nodes.$node_id.roles.0: "cluster_manager" } - match: { nodes.$node_id.roles.1: "data" } - match: { nodes.$node_id.roles.2: "ingest" } - - match: { nodes.$node_id.roles.3: "master" } - - match: { nodes.$node_id.roles.4: "remote_cluster_client" } + - match: { nodes.$node_id.roles.3: "remote_cluster_client" } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java index 0ae7ebd17aaa5..a1782dc7a0c6f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java @@ -223,7 +223,14 @@ private Counts(final List nodeInfos) { roles.merge(COORDINATING_ONLY, 1, Integer::sum); } else { for (DiscoveryNodeRole role : nodeInfo.getNode().getRoles()) { - roles.merge(role.roleName(), 1, Integer::sum); + // TODO: Remove the 'if' condition and only keep the statement in 'else' after removing MASTER_ROLE. + // As of 2.0, CLUSTER_MANAGER_ROLE is added, and it should be taken as MASTER_ROLE + if (role.equals(DiscoveryNodeRole.MASTER_ROLE) || role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + roles.merge(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1, Integer::sum); + roles.merge(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1, Integer::sum); + } else { + roles.merge(role.roleName(), 1, Integer::sum); + } } } } diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 3357db3e5bb3a..e2ad816cb0a01 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -55,7 +55,15 @@ public void testIsIngestNode() { runRoleTest(DiscoveryNode::isIngestNode, DiscoveryNodeRole.INGEST_ROLE); } + /** + * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #testIsClusterManagerNode()}. + */ + @Deprecated public void testIsMasterNode() { + runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.MASTER_ROLE); + } + + public void testIsClusterManagerNode() { runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE); } From 06fdf74f9ed245850b40be597b48c1e5a4f41ca6 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 00:06:59 -0800 Subject: [PATCH 24/51] Fix DiscoveryNodeRoleSettingTests Signed-off-by: Tianli Feng --- .../admin/cluster/stats/ClusterStatsIT.java | 2 ++ .../opensearch/cluster/node/DiscoveryNode.java | 16 +++++++--------- .../cluster/stats/ClusterStatsNodesTests.java | 8 +------- .../node/DiscoveryNodeRoleSettingTests.java | 3 +++ .../main/java/org/opensearch/test/NodeRoles.java | 12 ++++++++---- 5 files changed, 21 insertions(+), 20 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 eea5d458898bf..477e71fb84c10 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 @@ -86,6 +86,7 @@ public void testNodeCounts() { internalCluster().startNode(); Map expectedCounts = new HashMap<>(); expectedCounts.put(DiscoveryNodeRole.DATA_ROLE.roleName(), 1); + expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); // TODO: Remove along with MASTER_ROLE expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), 1); @@ -131,6 +132,7 @@ public void testNodeCounts() { } if (isMasterNode) { incrementCountForRole(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), expectedCounts); + incrementCountForRole(DiscoveryNodeRole.MASTER_ROLE.roleName(), expectedCounts); // TODO: Remove along with MASTER_ROLE } if (isRemoteClusterClientNode) { incrementCountForRole(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), expectedCounts); diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 5b23378d881ef..5b7643e4f88b8 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -577,10 +577,8 @@ public static Set getPossibleRoles() { public static void setAdditionalRoles(final Set additionalRoles) { assert additionalRoles.stream().allMatch(r -> r.legacySetting() == null || r.legacySetting().isDeprecated()) : additionalRoles; - // TODO: Remove 'new HashMap<>()' and only keep 'rolesToMap(...)' after removing DiscoveryNodeRole.MASTER_ROLE - // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. - final Map roleNameToPossibleRoles = new HashMap<>( - rolesToMap(Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream())) + final Map roleNameToPossibleRoles = rolesToMap( + Stream.concat(DiscoveryNodeRole.BUILT_IN_ROLES.stream(), additionalRoles.stream()) ); // collect the abbreviation names into a map to ensure that there are not any duplicate abbreviations final Map roleNameAbbreviationToPossibleRoles = Collections.unmodifiableMap( @@ -593,11 +591,11 @@ public static void setAdditionalRoles(final Set additionalRol + "], roles by name abbreviation [" + roleNameAbbreviationToPossibleRoles + "]"; - // TODO: Remove this line after removing DiscoveryNodeRole.MASTER_ROLE - roleNameToPossibleRoles.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); - // TODO: Remove 'Collections.unmodifiableMap()', and only keep 'roleNameToPossibleRoles', after removing MASTER_ROLE - // because the map roleNameToPossibleRoles is supposed to be immutable. - roleMap = Collections.unmodifiableMap(roleNameToPossibleRoles); + // TODO: Remove the Map 'roleNameToPossibleRolesWithMaster' and let 'roleMap = roleNameToPossibleRoles', after removing MASTER_ROLE. + // It's used to allow CLUSTER_MANAGER_ROLE that introduced in 2.0, having the same abbreviation name with MASTER_ROLE. + final Map roleNameToPossibleRolesWithMaster = new HashMap<>(roleNameToPossibleRoles); + roleNameToPossibleRolesWithMaster.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE); + roleMap = Collections.unmodifiableMap(roleNameToPossibleRolesWithMaster); } public static Set getPossibleRoleNames() { diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java index 9c0090a15d7a5..24fc3bb993930 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java @@ -41,13 +41,7 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.test.OpenSearchTestCase; -import java.util.Arrays; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.SortedMap; -import java.util.TreeMap; +import java.util.*; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index e2ad816cb0a01..9d01df698e55d 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -37,6 +37,7 @@ import org.opensearch.test.OpenSearchTestCase; import java.util.Collections; +import java.util.HashSet; import java.util.function.Predicate; import static org.opensearch.test.NodeRoles.onlyRole; @@ -60,6 +61,8 @@ public void testIsIngestNode() { */ @Deprecated public void testIsMasterNode() { + // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES. + DiscoveryNode.setAdditionalRoles(new HashSet<>()); runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.MASTER_ROLE); } diff --git a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java index cf7d7467c1f40..d8acb9c1f025d 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java @@ -82,6 +82,13 @@ public static Settings removeRoles(final Settings settings, final Set roles.contains(r) == false) + // TODO: Remove the below filter after removing MASTER_ROLE. + // It's used to remove both CLUSTER_MANAGER_ROLE and MASTER_ROLE, when requested to remove either. + .filter( + roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) || roles.contains(DiscoveryNodeRole.MASTER_ROLE) + ? r -> (!r.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) && !r.equals(DiscoveryNodeRole.MASTER_ROLE)) + : r -> true + ) .map(DiscoveryNodeRole::roleName) .collect(Collectors.toList()) ) @@ -184,10 +191,7 @@ public static Settings nonMasterNode() { } public static Settings nonMasterNode(final Settings settings) { - return removeRoles( - settings, - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE))) - ); + return removeRoles(settings, Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); } public static Settings remoteClusterClientNode() { From 10247a21997aa1e5f9a5237564c69d39b1afe185 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 00:13:00 -0800 Subject: [PATCH 25/51] adjust format Signed-off-by: Tianli Feng --- .../action/admin/cluster/stats/ClusterStatsIT.java | 2 +- .../admin/cluster/stats/ClusterStatsNodesTests.java | 8 +++++++- .../java/org/opensearch/env/NodeEnvironmentTests.java | 4 +--- .../org/opensearch/env/NodeRepurposeCommandTests.java | 1 + .../java/org/opensearch/test/InternalTestCluster.java | 9 +-------- .../src/main/java/org/opensearch/test/NodeRoles.java | 2 -- 6 files changed, 11 insertions(+), 15 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 477e71fb84c10..484e9acedc841 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 @@ -131,8 +131,8 @@ public void testNodeCounts() { incrementCountForRole(DiscoveryNodeRole.INGEST_ROLE.roleName(), expectedCounts); } if (isMasterNode) { - incrementCountForRole(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), expectedCounts); incrementCountForRole(DiscoveryNodeRole.MASTER_ROLE.roleName(), expectedCounts); // TODO: Remove along with MASTER_ROLE + incrementCountForRole(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), expectedCounts); } if (isRemoteClusterClientNode) { incrementCountForRole(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), expectedCounts); diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java index 24fc3bb993930..9c0090a15d7a5 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodesTests.java @@ -41,7 +41,13 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.test.OpenSearchTestCase; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; diff --git a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java index c59f5e3607e10..a6a25252e9852 100644 --- a/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java @@ -530,9 +530,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException { .put( NodeRoles.removeRoles( settings, - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) - ) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) ) ) .build(); diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java index f2a8a7d58ff24..9897ad1a3650b 100644 --- a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java +++ b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java @@ -107,6 +107,7 @@ public void createNodePaths() throws IOException { dataMasterSettings, Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) ); + noDataMasterSettings = masterNode(nonDataNode(dataMasterSettings)); } diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index f9f6422a49069..e79d8ddd90a08 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -1146,14 +1146,7 @@ private synchronized void reset(boolean wipeData) throws IOException { for (int i = numSharedDedicatedMasterNodes; i < numSharedDedicatedMasterNodes + numSharedDataNodes; i++) { final Settings nodeSettings = getNodeSettings(i, sharedNodesSeeds[i], Settings.EMPTY, defaultMinMasterNodes); if (numSharedDedicatedMasterNodes > 0) { - settings.add( - removeRoles( - nodeSettings, - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)) - ) - ) - ); + settings.add(removeRoles(nodeSettings, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE))); } else { // if we don't have dedicated master nodes, keep things default settings.add(nodeSettings); diff --git a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java index d8acb9c1f025d..728b4fd069fc6 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java @@ -36,9 +36,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.node.NodeRoleSettings; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; From 8817b73a80bf2d1d900157663b40d00858873316 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 00:15:58 -0800 Subject: [PATCH 26/51] Reduce code change Signed-off-by: Tianli Feng --- .../java/org/opensearch/env/NodeEnvironmentIT.java | 2 +- .../java/org/opensearch/env/NodeRepurposeCommandIT.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java index 89adc15e41cd6..83c103bd82738 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeEnvironmentIT.java @@ -88,7 +88,7 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { public Settings onNodeStopped(String nodeName) { return NodeRoles.removeRoles( Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)) ) ); } diff --git a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java index 80acc98293bd6..2547333490f23 100644 --- a/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/env/NodeRepurposeCommandIT.java @@ -75,7 +75,7 @@ public void testRepurpose() throws Exception { final Settings dataNodeDataPathSettings = internalCluster().dataPathSettings(dataNode); final Settings noMasterNoDataSettings = NodeRoles.removeRoles( - Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE))) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) ); final Settings noMasterNoDataSettingsForMasterNode = Settings.builder() From 010fd2944414e6df60ea53b4a54a184c8faf6f4a Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 14:55:03 -0800 Subject: [PATCH 27/51] Skip run node_info role test during during mixedClusterTest, after a node got upgraded Signed-off-by: Tianli Feng --- .../main/resources/rest-api-spec/test/nodes.info/10_basic.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index bca5d1e2dc44a..c5aa20333003c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -34,9 +34,13 @@ setup: - skip: version: " - 1.4.99" reason: "node role cluster_manager is added in 2.0.0" + features: node_selector - do: nodes.info: {} + node_selector: + version: "2.0.0 - " # It's used to skip this test when a node got upgraded from a lower version during mixedClusterTest. + - set: nodes._arbitrary_key_: node_id From eda18dbaac5c84fca57900fde8545b34aba509c3 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 16:10:10 -0800 Subject: [PATCH 28/51] Skip run node_info role test during during mixedClusterTest, after a node got upgraded Signed-off-by: Tianli Feng --- .../resources/rest-api-spec/test/nodes.info/10_basic.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index c5aa20333003c..7305bb56cd93a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -14,7 +14,7 @@ setup: --- "node_info role test": - skip: - version: " - 7.7.99 , 2.0.0 - " + version: " - 7.7.99 , 2.0.0 -" reason: "node roles were not sorted before 7.8.0" - do: @@ -32,14 +32,13 @@ setup: --- "node_info role test with cluster_manager": - skip: - version: " - 1.4.99" reason: "node role cluster_manager is added in 2.0.0" features: node_selector - do: nodes.info: {} node_selector: - version: "2.0.0 - " # It's used to skip this test when a node got upgraded from a lower version during mixedClusterTest. + version: "2.0.0 - " # Run the test when a node got upgraded from <2.0 to 2.x version during mixedClusterTest. - set: nodes._arbitrary_key_: node_id From 8f9ab2bc5712959819b0588ec8833f9e9350e6bc Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 17:20:34 -0800 Subject: [PATCH 29/51] Add unit tests in DiscoveryNodeTests Signed-off-by: Tianli Feng --- .../cluster/node/DiscoveryNodeRole.java | 2 +- .../cluster/node/DiscoveryNodeTests.java | 34 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 7d71078e7abfe..f663f8e12c832 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -220,7 +220,7 @@ public Setting legacySetting() { @Override public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { - if (nodeVersion.onOrAfter(Version.V_2_0_0)) { + if (nodeVersion.onOrAfter(Version.CURRENT)) { return this; } else { return DiscoveryNodeRole.MASTER_ROLE; 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 39a3381bede26..36cd3864bbeaf 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -39,12 +39,11 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.transport.TransportAddress; +import org.opensearch.node.NodeRoleSettings; import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import static java.util.Collections.emptyMap; @@ -174,6 +173,35 @@ public void testDiscoveryNodeIsRemoteClusterClientUnset() { runTestDiscoveryNodeIsRemoteClusterClient(nonRemoteClusterClientNode(), false); } + /* + * @deprecated The test is added in 2.0, along with CLUSTER_MANAGER_ROLE, and it will be removed along with MASTER_ROLE. + */ + @Deprecated + public void testSetAdditionalRolesCanAddDeprecatedMasterRole() { + DiscoveryNode.setAdditionalRoles(new HashSet<>()); + assertTrue(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); + } + + /* + * @deprecated The test is added in 2.0, along with CLUSTER_MANAGER_ROLE, and it will be removed along with MASTER_ROLE. + */ + @Deprecated + public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { + Set setWithClusterManagerRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); + Set setWithMasterRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE)); + Set setWithClusterManagerAndMasterRole = new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)); + List> possibleClusterManagerRoleSetList = new ArrayList<>(Arrays.asList(setWithMasterRole, setWithClusterManagerRole, setWithClusterManagerAndMasterRole)); + + // Test method isMasterNode() + DiscoveryNode clusterManagerNode = new DiscoveryNode("name", "id", buildNewFakeTransportAddress(), emptyMap(), randomFrom(possibleClusterManagerRoleSetList), Version.CURRENT); + assertTrue(clusterManagerNode.isMasterNode()); + + // Test method isMasterNode(Settings) + List clusterManagerRoleNameList = randomFrom(possibleClusterManagerRoleSetList).stream().map(DiscoveryNodeRole::roleName).collect(Collectors.toList()); + Settings settingWithMasterRole = Settings.builder().putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), clusterManagerRoleNameList).build(); + assertTrue(DiscoveryNode.isMasterNode(settingWithMasterRole)); + } + private void runTestDiscoveryNodeIsRemoteClusterClient(final Settings settings, final boolean expected) { final DiscoveryNode node = DiscoveryNode.createLocal(settings, new TransportAddress(TransportAddress.META_ADDRESS, 9200), "node"); assertThat(node.isRemoteClusterClient(), equalTo(expected)); From 22567a4911e36029e94d67404237e6aa2bbdf104 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 17:44:26 -0800 Subject: [PATCH 30/51] Revert adding clustermanager role into isDedicatedVotingOnlyNode Signed-off-by: Tianli Feng --- .../opensearch/repositories/RepositoriesService.java | 3 ++- .../cluster/node/DiscoveryNodeRoleSettingTests.java | 5 +---- .../opensearch/cluster/node/DiscoveryNodeTests.java | 10 ++-------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index faee4a0808baf..847053adf041c 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -346,8 +346,9 @@ protected void doRun() { }); } + // Note: "voting_only" role has not been implemented yet. static boolean isDedicatedVotingOnlyNode(Set roles) { - return (roles.contains(DiscoveryNodeRole.MASTER_ROLE) || roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) + return roles.contains(DiscoveryNodeRole.MASTER_ROLE) && roles.contains(DiscoveryNodeRole.DATA_ROLE) == false && roles.stream().anyMatch(role -> role.roleName().equals("voting_only")); } diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 9d01df698e55d..2581c88516934 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -56,10 +56,7 @@ public void testIsIngestNode() { runRoleTest(DiscoveryNode::isIngestNode, DiscoveryNodeRole.INGEST_ROLE); } - /** - * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #testIsClusterManagerNode()}. - */ - @Deprecated + // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. public void testIsMasterNode() { // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES. DiscoveryNode.setAdditionalRoles(new HashSet<>()); 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 36cd3864bbeaf..1c1c930ccc892 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -173,19 +173,13 @@ public void testDiscoveryNodeIsRemoteClusterClientUnset() { runTestDiscoveryNodeIsRemoteClusterClient(nonRemoteClusterClientNode(), false); } - /* - * @deprecated The test is added in 2.0, along with CLUSTER_MANAGER_ROLE, and it will be removed along with MASTER_ROLE. - */ - @Deprecated + // 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() { DiscoveryNode.setAdditionalRoles(new HashSet<>()); assertTrue(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); } - /* - * @deprecated The test is added in 2.0, along with CLUSTER_MANAGER_ROLE, and it will be removed along with MASTER_ROLE. - */ - @Deprecated + // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { Set setWithClusterManagerRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); Set setWithMasterRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE)); From 4e0b27c792df80c3890d4e6c161c5ec08133b166 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 18:19:47 -0800 Subject: [PATCH 31/51] Modify the test to assign master and clusetr_manager role in InternalTestClusterTests Signed-off-by: Tianli Feng --- .../cluster/node/DiscoveryNodeTests.java | 25 +++++++++++++++---- .../test/test/InternalTestClusterTests.java | 16 ++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) 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 1c1c930ccc892..9d2a490344103 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -183,16 +183,31 @@ public void testSetAdditionalRolesCanAddDeprecatedMasterRole() { public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { Set setWithClusterManagerRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); Set setWithMasterRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE)); - Set setWithClusterManagerAndMasterRole = new HashSet<>(Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE)); - List> possibleClusterManagerRoleSetList = new ArrayList<>(Arrays.asList(setWithMasterRole, setWithClusterManagerRole, setWithClusterManagerAndMasterRole)); + Set setWithClusterManagerAndMasterRole = new HashSet<>( + Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE) + ); + List> possibleClusterManagerRoleSetList = new ArrayList<>( + Arrays.asList(setWithMasterRole, setWithClusterManagerRole, setWithClusterManagerAndMasterRole) + ); // Test method isMasterNode() - DiscoveryNode clusterManagerNode = new DiscoveryNode("name", "id", buildNewFakeTransportAddress(), emptyMap(), randomFrom(possibleClusterManagerRoleSetList), Version.CURRENT); + DiscoveryNode clusterManagerNode = new DiscoveryNode( + "name", + "id", + buildNewFakeTransportAddress(), + emptyMap(), + randomFrom(possibleClusterManagerRoleSetList), + Version.CURRENT + ); assertTrue(clusterManagerNode.isMasterNode()); // Test method isMasterNode(Settings) - List clusterManagerRoleNameList = randomFrom(possibleClusterManagerRoleSetList).stream().map(DiscoveryNodeRole::roleName).collect(Collectors.toList()); - Settings settingWithMasterRole = Settings.builder().putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), clusterManagerRoleNameList).build(); + List clusterManagerRoleNameList = randomFrom(possibleClusterManagerRoleSetList).stream() + .map(DiscoveryNodeRole::roleName) + .collect(Collectors.toList()); + Settings settingWithMasterRole = Settings.builder() + .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), clusterManagerRoleNameList) + .build(); assertTrue(DiscoveryNode.isMasterNode(settingWithMasterRole)); } diff --git a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java index 72523f898cedd..6764a986b7b46 100644 --- a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java +++ b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java @@ -398,17 +398,19 @@ public Path nodeConfigPath(int nodeOrdinal) { Function.identity() ); cluster.beforeTest(random()); + // TODO: Remove this line, and replace 'clusterManagerRole' with CLUSTER_MANAGER_ROLE, after MASTER_ROLE is removed. + // It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. + DiscoveryNodeRole clusterManagerRole = randomFrom(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE); List roles = new ArrayList<>(); for (int i = 0; i < numNodes; i++) { - final DiscoveryNodeRole role = i == numNodes - 1 && roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false - ? DiscoveryNodeRole.CLUSTER_MANAGER_ROLE - : // last node and still no master - randomFrom(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.INGEST_ROLE); + final DiscoveryNodeRole role = i == numNodes - 1 && roles.contains(clusterManagerRole) == false + ? clusterManagerRole // last node and still no master + : randomFrom(clusterManagerRole, DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.INGEST_ROLE); roles.add(role); } cluster.setBootstrapMasterNodeIndex( - randomIntBetween(0, (int) roles.stream().filter(role -> role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)).count() - 1) + randomIntBetween(0, (int) roles.stream().filter(role -> role.equals(clusterManagerRole)).count() - 1) ); try { @@ -416,7 +418,7 @@ public Path nodeConfigPath(int nodeOrdinal) { for (int i = 0; i < numNodes; i++) { final DiscoveryNodeRole role = roles.get(i); final String node; - if (role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { + if (role == clusterManagerRole) { node = cluster.startMasterOnlyNode(); } else if (role == DiscoveryNodeRole.DATA_ROLE) { node = cluster.startDataOnlyNode(); @@ -438,7 +440,7 @@ public Path nodeConfigPath(int nodeOrdinal) { DiscoveryNode node = cluster.getInstance(ClusterService.class, name).localNode(); List paths = Arrays.stream(getNodePaths(cluster, name)).map(Path::toString).collect(Collectors.toList()); if (node.isMasterNode()) { - result.computeIfAbsent(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, k -> new HashSet<>()).addAll(paths); + result.computeIfAbsent(clusterManagerRole, k -> new HashSet<>()).addAll(paths); } else if (node.isDataNode()) { result.computeIfAbsent(DiscoveryNodeRole.DATA_ROLE, k -> new HashSet<>()).addAll(paths); } else { From 6d08a78fdbb1f16a68ba707b000f930cc9fee5f7 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 18:22:17 -0800 Subject: [PATCH 32/51] modify the comment Signed-off-by: Tianli Feng --- .../java/org/opensearch/test/test/InternalTestClusterTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java index 6764a986b7b46..e07f04d11b8fb 100644 --- a/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java +++ b/test/framework/src/test/java/org/opensearch/test/test/InternalTestClusterTests.java @@ -399,7 +399,7 @@ public Path nodeConfigPath(int nodeOrdinal) { ); cluster.beforeTest(random()); // TODO: Remove this line, and replace 'clusterManagerRole' with CLUSTER_MANAGER_ROLE, after MASTER_ROLE is removed. - // It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. + // It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE, aims to test the 2 roles have the same effect. DiscoveryNodeRole clusterManagerRole = randomFrom(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE); List roles = new ArrayList<>(); for (int i = 0; i < numNodes; i++) { From 8ca3a9f6f29432137ede4c92eaac28f85fe34100 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 18:45:30 -0800 Subject: [PATCH 33/51] Add cluster_manager role to be tested by node selector in 2 uni tests Signed-off-by: Tianli Feng --- .../support/nodes/TransportNodesActionTests.java | 2 ++ .../opensearch/cluster/node/DiscoveryNodesTests.java | 11 ++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java index a8755df6b5daa..53ebceb25510a 100644 --- a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java @@ -172,7 +172,9 @@ private List mockList(Supplier supplier, int size) { private enum NodeSelector { LOCAL("_local"), ELECTED_MASTER("_master"), + // TODO: Remove this element after removing DiscoveryNodeRole.MASTER_ROLE MASTER_ELIGIBLE("master:true"), + CLUSTER_MANAGER_ELIGIBLE("cluster_manager:true"), DATA("data:true"), CUSTOM_ATTRIBUTE("attr:value"); diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java index 639c5c0388a91..aff9e1cfe7a8c 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodesTests.java @@ -361,7 +361,16 @@ Set matchingNodeIds(DiscoveryNodes nodes) { return Collections.singleton(nodes.getMasterNodeId()); } }, - MASTER_ELIGIBLE(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + ":true") { + // TODO: Remove this element after removing DiscoveryNodeRole.MASTER_ROLE + MASTER_ELIGIBLE(DiscoveryNodeRole.MASTER_ROLE.roleName() + ":true") { + @Override + Set matchingNodeIds(DiscoveryNodes nodes) { + Set ids = new HashSet<>(); + nodes.getMasterNodes().keysIt().forEachRemaining(ids::add); + return ids; + } + }, + CLUSTER_MANAGER_ELIGIBLE(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + ":true") { @Override Set matchingNodeIds(DiscoveryNodes nodes) { Set ids = new HashSet<>(); From 4c0e5f4078d4a3f02a909cf9f88f4e84132f5f85 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 18:46:36 -0800 Subject: [PATCH 34/51] Revert import util.* Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNodeTests.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 9d2a490344103..84f4d8b8f301e 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -43,7 +43,9 @@ import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; -import java.util.*; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import java.util.stream.Collectors; import static java.util.Collections.emptyMap; From 4fa904f626a39012581543f9ff396b80d8c0a69d Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 19:03:08 -0800 Subject: [PATCH 35/51] Fix newly added test in DiscoveryNodeTests Signed-off-by: Tianli Feng --- .../opensearch/cluster/node/DiscoveryNodeTests.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 84f4d8b8f301e..92838e284ca62 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -43,8 +43,10 @@ import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -183,14 +185,9 @@ public void testSetAdditionalRolesCanAddDeprecatedMasterRole() { // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { - Set setWithClusterManagerRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)); - Set setWithMasterRole = new HashSet<>(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE)); - Set setWithClusterManagerAndMasterRole = new HashSet<>( + Set possibleClusterManagerRoleSet = new HashSet<>( Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE) ); - List> possibleClusterManagerRoleSetList = new ArrayList<>( - Arrays.asList(setWithMasterRole, setWithClusterManagerRole, setWithClusterManagerAndMasterRole) - ); // Test method isMasterNode() DiscoveryNode clusterManagerNode = new DiscoveryNode( @@ -198,13 +195,13 @@ public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { "id", buildNewFakeTransportAddress(), emptyMap(), - randomFrom(possibleClusterManagerRoleSetList), + new HashSet<>(randomSubsetOf(randomIntBetween(1, 2), possibleClusterManagerRoleSet)), Version.CURRENT ); assertTrue(clusterManagerNode.isMasterNode()); // Test method isMasterNode(Settings) - List clusterManagerRoleNameList = randomFrom(possibleClusterManagerRoleSetList).stream() + List clusterManagerRoleNameList = randomSubsetOf(randomIntBetween(1, 2), possibleClusterManagerRoleSet).stream() .map(DiscoveryNodeRole::roleName) .collect(Collectors.toList()); Settings settingWithMasterRole = Settings.builder() From d1ee0c6e74941e190b37139e5fdf7d09fc9618ba Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 19:27:44 -0800 Subject: [PATCH 36/51] Fix nodes.info yaml rest test during mixedclustertest Signed-off-by: Tianli Feng --- .../resources/rest-api-spec/test/nodes.info/10_basic.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index 7305bb56cd93a..a062e82972665 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -14,7 +14,7 @@ setup: --- "node_info role test": - skip: - version: " - 7.7.99 , 2.0.0 -" + version: " - 7.7.99 , 2.0.0 - " reason: "node roles were not sorted before 7.8.0" - do: @@ -32,13 +32,14 @@ setup: --- "node_info role test with cluster_manager": - skip: + version: " - 1.4.99" reason: "node role cluster_manager is added in 2.0.0" features: node_selector - do: nodes.info: {} node_selector: - version: "2.0.0 - " # Run the test when a node got upgraded from <2.0 to 2.x version during mixedClusterTest. + version: "2.0.0 - " # Only send request to nodes in 2.x versions, especially during mixedClusterTest. - set: nodes._arbitrary_key_: node_id From a4b7b7245bdb3fdf50e6ed4b4d8ff732ceea89b9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Sat, 12 Mar 2022 19:37:05 -0800 Subject: [PATCH 37/51] Reduce code change in removeRoles() Signed-off-by: Tianli Feng --- .../java/org/opensearch/transport/ConnectionProfileTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java index 28cf20e20753e..d6dc56204f5da 100644 --- a/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java +++ b/server/src/test/java/org/opensearch/transport/ConnectionProfileTests.java @@ -258,9 +258,7 @@ public void testDefaultConnectionProfile() { profile = ConnectionProfile.buildDefaultConnectionProfile( removeRoles( - Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) - ) + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE))) ) ); assertEquals(10, profile.getNumConnections()); From 7a2a6b99374d0b2cc6eefc72ef4087e0004a395a Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 16 Mar 2022 20:34:42 -0700 Subject: [PATCH 38/51] Change == to equals() Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNode.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 5b7643e4f88b8..2e175ee744740 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -390,11 +390,11 @@ public void writeTo(StreamOutput out) throws IOException { .collect(Collectors.toList()); out.writeVInt(rolesToWrite.size()); for (final DiscoveryNodeRole role : rolesToWrite) { - if (role == DiscoveryNodeRole.MASTER_ROLE || role == DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) { + if (role.equals(DiscoveryNodeRole.MASTER_ROLE) || role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { out.writeEnum(LegacyRole.MASTER); - } else if (role == DiscoveryNodeRole.DATA_ROLE) { + } else if (role.equals(DiscoveryNodeRole.DATA_ROLE)) { out.writeEnum(LegacyRole.DATA); - } else if (role == DiscoveryNodeRole.INGEST_ROLE) { + } else if (role.equals(DiscoveryNodeRole.INGEST_ROLE)) { out.writeEnum(LegacyRole.INGEST); } } From 83a3c733170c32b1f939083f14f26c41993e4f16 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 16 Mar 2022 20:35:36 -0700 Subject: [PATCH 39/51] Change Version.Current to Version.v2_0_0, since current is a moving target Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index f663f8e12c832..7d71078e7abfe 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -220,7 +220,7 @@ public Setting legacySetting() { @Override public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { - if (nodeVersion.onOrAfter(Version.CURRENT)) { + if (nodeVersion.onOrAfter(Version.V_2_0_0)) { return this; } else { return DiscoveryNodeRole.MASTER_ROLE; From e4404d7d27f82b0402378804ee7d5e644208e0c3 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 16 Mar 2022 20:36:28 -0700 Subject: [PATCH 40/51] Remove an unnecessary TODO Signed-off-by: Tianli Feng --- .../opensearch/action/admin/cluster/stats/ClusterStatsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 33ad667c75302..01329285947c7 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 @@ -84,7 +84,7 @@ public void testNodeCounts() { internalCluster().startNode(); Map expectedCounts = new HashMap<>(); expectedCounts.put(DiscoveryNodeRole.DATA_ROLE.roleName(), 1); - expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); // TODO: Remove along with MASTER_ROLE + expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), 1); expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), 1); From b5837b794376ab7b5bd115bee1f011381ca2ffc9 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 16 Mar 2022 20:41:14 -0700 Subject: [PATCH 41/51] Change MASTER_ROLE to CLUSTER_MANAGER_ROLE directly in a method about voting_only node Signed-off-by: Tianli Feng --- .../java/org/opensearch/repositories/RepositoriesService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java index 847053adf041c..e7c5804f458a0 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java @@ -346,9 +346,9 @@ protected void doRun() { }); } - // Note: "voting_only" role has not been implemented yet. + // Note: "voting_only" role has not been implemented yet, so the method always returns false. static boolean isDedicatedVotingOnlyNode(Set roles) { - return roles.contains(DiscoveryNodeRole.MASTER_ROLE) + return roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) && roles.contains(DiscoveryNodeRole.DATA_ROLE) == false && roles.stream().anyMatch(role -> role.roleName().equals("voting_only")); } From ebb1c454609a58627f58cd1294545da7a7fcf306 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Wed, 16 Mar 2022 22:31:30 -0700 Subject: [PATCH 42/51] Fix yaml rest test by adding proper node selector Signed-off-by: Tianli Feng --- .../test/nodes.info/10_basic.yml | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml index a062e82972665..c779cb80d13b3 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml @@ -12,14 +12,24 @@ setup: - is_true: cluster_name --- -"node_info role test": +"node_info role test - before 2.0.0": - skip: version: " - 7.7.99 , 2.0.0 - " reason: "node roles were not sorted before 7.8.0" + features: node_selector - do: nodes.info: {} + node_selector: + # Only send request to nodes in <2.0 versions, especially during ':qa:mixed-cluster:v1.x.x#mixedClusterTest'. + # Because YAML REST test takes the minimum OpenSearch version in the cluster to apply the filter in 'skip' section, + # see OpenSearchClientYamlSuiteTestCase#initAndResetContext() for detail. + # During 'mixedClusterTest', the cluster can be mixed with nodes in 1.x and 2.x versions, + # so node_selector is required, and only filtering version in 'skip' is not enough. + version: "1.0.0 - 1.4.99" + - set: + # Note: It will only stash the first node_id in the api response. nodes._arbitrary_key_: node_id - is_true: nodes.$node_id.roles @@ -28,18 +38,15 @@ setup: - match: { nodes.$node_id.roles.1: "ingest" } - match: { nodes.$node_id.roles.2: "master" } - match: { nodes.$node_id.roles.3: "remote_cluster_client" } - + --- -"node_info role test with cluster_manager": +"node_info role test": - skip: version: " - 1.4.99" reason: "node role cluster_manager is added in 2.0.0" - features: node_selector - do: nodes.info: {} - node_selector: - version: "2.0.0 - " # Only send request to nodes in 2.x versions, especially during mixedClusterTest. - set: nodes._arbitrary_key_: node_id @@ -50,4 +57,3 @@ setup: - match: { nodes.$node_id.roles.1: "data" } - match: { nodes.$node_id.roles.2: "ingest" } - match: { nodes.$node_id.roles.3: "remote_cluster_client" } - From 0dadd6850b7c1200daec5c53a02291acc9a15541 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 00:29:33 -0700 Subject: [PATCH 43/51] Add validateRole() to aviod cluster_manager and master role exist together Signed-off-by: Tianli Feng --- .../cluster/node/DiscoveryNodeRole.java | 35 +++++++++++++++++++ .../org/opensearch/node/NodeRoleSettings.java | 6 +++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 7d71078e7abfe..b627dc4a833c0 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -41,6 +41,8 @@ import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.SortedSet; import java.util.TreeSet; @@ -129,6 +131,13 @@ public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { return this; } + /** + * Validate the role is compatible with the other roles in the list, when assigning the list of roles to a node. + * An {@link IllegalArgumentException} is expected to be thrown, if the role can't coexist with the other roles. + * @param roles A {@link List} of {@link DiscoveryNodeRole} that a node is going to have. + */ + public void validateRole(List roles) {}; + @Override public final boolean equals(Object o) { if (this == o) return true; @@ -205,6 +214,19 @@ public Setting legacySetting() { return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope); } + @Override + public void validateRole(List roles) { + if (roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "To promote inclusive language, [%s] role is deprecated, and is replaced by [%s] role. The two roles can not be assigned together to a node.", + DiscoveryNodeRole.MASTER_ROLE.roleName(), + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + ) + ); + } + } }; /** @@ -227,6 +249,19 @@ public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { } } + @Override + public void validateRole(List roles) { + if (roles.contains(DiscoveryNodeRole.MASTER_ROLE)) { + throw new IllegalArgumentException( + String.format( + Locale.ROOT, + "To promote inclusive language, [%s] role is deprecated, and is replaced by [%s] role. The two roles can not be assigned together to a node.", + DiscoveryNodeRole.MASTER_ROLE.roleName(), + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + ) + ); + } + } }; public static final DiscoveryNodeRole REMOTE_CLUSTER_CLIENT_ROLE = new DiscoveryNodeRole("remote_cluster_client", "r") { diff --git a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java index 1e29f22b140f9..d690cfa4caa69 100644 --- a/server/src/main/java/org/opensearch/node/NodeRoleSettings.java +++ b/server/src/main/java/org/opensearch/node/NodeRoleSettings.java @@ -51,7 +51,11 @@ public class NodeRoleSettings { .filter(role -> role.isEnabledByDefault(settings)) .map(DiscoveryNodeRole::roleName) .collect(Collectors.toList()), - roles -> {}, + roles -> { + for (DiscoveryNodeRole role : roles) { + role.validateRole(roles); + } + }, Property.NodeScope ); From 6845cddb14f274c0696bbdd9c82b5c56938a984c Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 00:35:23 -0700 Subject: [PATCH 44/51] Remove an unnecessary TODO Signed-off-by: Tianli Feng --- .../opensearch/action/admin/cluster/stats/ClusterStatsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 01329285947c7..5b48268dfa89f 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 @@ -129,7 +129,7 @@ public void testNodeCounts() { incrementCountForRole(DiscoveryNodeRole.INGEST_ROLE.roleName(), expectedCounts); } if (isMasterNode) { - incrementCountForRole(DiscoveryNodeRole.MASTER_ROLE.roleName(), expectedCounts); // TODO: Remove along with MASTER_ROLE + incrementCountForRole(DiscoveryNodeRole.MASTER_ROLE.roleName(), expectedCounts); incrementCountForRole(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), expectedCounts); } if (isRemoteClusterClientNode) { From f818ea1c6c53a3d885847ad93e14c4d385a9014e Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 00:56:26 -0700 Subject: [PATCH 45/51] Remove validateRole() in MASTER_ROLE Signed-off-by: Tianli Feng --- .../opensearch/cluster/node/DiscoveryNodeRole.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index b627dc4a833c0..2ad444bebc82d 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -214,19 +214,6 @@ public Setting legacySetting() { return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope); } - @Override - public void validateRole(List roles) { - if (roles.contains(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { - throw new IllegalArgumentException( - String.format( - Locale.ROOT, - "To promote inclusive language, [%s] role is deprecated, and is replaced by [%s] role. The two roles can not be assigned together to a node.", - DiscoveryNodeRole.MASTER_ROLE.roleName(), - DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() - ) - ); - } - } }; /** From 5847ae0c5580512456d862fbbcf7dc850c06a127 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 02:34:27 -0700 Subject: [PATCH 46/51] Add unit test for the role setting validator Signed-off-by: Tianli Feng --- .../node/DiscoveryNodeRoleSettingTests.java | 6 +-- .../cluster/node/DiscoveryNodeTests.java | 32 +------------ .../node/NodeRoleSettingsTests.java | 45 +++++++++++++++++++ 3 files changed, 48 insertions(+), 35 deletions(-) create mode 100644 server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java index 2581c88516934..55c518c62973e 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleSettingTests.java @@ -37,7 +37,6 @@ import org.opensearch.test.OpenSearchTestCase; import java.util.Collections; -import java.util.HashSet; import java.util.function.Predicate; import static org.opensearch.test.NodeRoles.onlyRole; @@ -56,10 +55,9 @@ public void testIsIngestNode() { runRoleTest(DiscoveryNode::isIngestNode, DiscoveryNodeRole.INGEST_ROLE); } - // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. public void testIsMasterNode() { - // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES. - DiscoveryNode.setAdditionalRoles(new HashSet<>()); + // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES in 2.0. + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); runRoleTest(DiscoveryNode::isMasterNode, DiscoveryNodeRole.MASTER_ROLE); } 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 92838e284ca62..933d9d5f825e9 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -39,14 +39,11 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.transport.TransportAddress; -import org.opensearch.node.NodeRoleSettings; import org.opensearch.test.OpenSearchTestCase; import java.net.InetAddress; -import java.util.Arrays; import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -179,37 +176,10 @@ 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() { - DiscoveryNode.setAdditionalRoles(new HashSet<>()); + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); assertTrue(DiscoveryNode.getPossibleRoleNames().contains(DiscoveryNodeRole.MASTER_ROLE.roleName())); } - // TODO: Remove the test along with MASTER_ROLE. It is added in 2.0, along with the introduction of CLUSTER_MANAGER_ROLE. - public void testClusterManagerNodeAndMasterNodeCanBeIdentified() { - Set possibleClusterManagerRoleSet = new HashSet<>( - Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.MASTER_ROLE) - ); - - // Test method isMasterNode() - DiscoveryNode clusterManagerNode = new DiscoveryNode( - "name", - "id", - buildNewFakeTransportAddress(), - emptyMap(), - new HashSet<>(randomSubsetOf(randomIntBetween(1, 2), possibleClusterManagerRoleSet)), - Version.CURRENT - ); - assertTrue(clusterManagerNode.isMasterNode()); - - // Test method isMasterNode(Settings) - List clusterManagerRoleNameList = randomSubsetOf(randomIntBetween(1, 2), possibleClusterManagerRoleSet).stream() - .map(DiscoveryNodeRole::roleName) - .collect(Collectors.toList()); - Settings settingWithMasterRole = Settings.builder() - .putList(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), clusterManagerRoleNameList) - .build(); - assertTrue(DiscoveryNode.isMasterNode(settingWithMasterRole)); - } - private void runTestDiscoveryNodeIsRemoteClusterClient(final Settings settings, final boolean expected) { final DiscoveryNode node = DiscoveryNode.createLocal(settings, new TransportAddress(TransportAddress.META_ADDRESS, 9200), "node"); assertThat(node.isRemoteClusterClient(), equalTo(expected)); diff --git a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java new file mode 100644 index 0000000000000..dc4fc16370419 --- /dev/null +++ b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java @@ -0,0 +1,45 @@ +/* + * 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. + */ + +package org.opensearch.node; + +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodeRole; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; + +public class NodeRoleSettingsTests extends OpenSearchTestCase { + + /** + * Validate cluster_manager role and master role can not coexist in a node. + * Remove the test after removing MASTER_ROLE. + */ + public void testClusterManagerAndMasterRoleCanNotCoexist() { + // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES in 2.0. + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); + Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "cluster_manager, master").build(); + Exception exception = expectThrows(IllegalArgumentException.class, () -> NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings)); + assertThat(exception.getMessage(), containsString("The two roles can not be assigned together to a node")); + } + + /** + * Validate cluster_manager role and data role can coexist in a node. The test is added along with validateRole(). + */ + public void testClusterManagerAndDataNodeRoles() { + Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "cluster_manager, data").build(); + List actualNodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings); + List expectedNodeRoles = Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE); + assertEquals(expectedNodeRoles, actualNodeRoles); + } +} From 14124bdf1c9d277d09adf42909f4e9995e0f5ef8 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 02:55:10 -0700 Subject: [PATCH 47/51] Modify error message for both roles assigned Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 5 ++++- .../test/java/org/opensearch/node/NodeRoleSettingsTests.java | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 2ad444bebc82d..40bb35a1e0e36 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -242,7 +242,10 @@ public void validateRole(List roles) { throw new IllegalArgumentException( String.format( Locale.ROOT, - "To promote inclusive language, [%s] role is deprecated, and is replaced by [%s] role. The two roles can not be assigned together to a node.", + "The two roles [%s, %s] can not be assigned together to a node. " + + "To promote inclusive language, [%s] role is deprecated, and replaced by [%s] role.", + DiscoveryNodeRole.MASTER_ROLE.roleName(), + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() ) diff --git a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java index dc4fc16370419..2aaedd1f34ac1 100644 --- a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java +++ b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java @@ -30,7 +30,7 @@ public void testClusterManagerAndMasterRoleCanNotCoexist() { DiscoveryNode.setAdditionalRoles(Collections.emptySet()); Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "cluster_manager, master").build(); Exception exception = expectThrows(IllegalArgumentException.class, () -> NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings)); - assertThat(exception.getMessage(), containsString("The two roles can not be assigned together to a node")); + assertThat(exception.getMessage(), containsString("[master, cluster_manager] can not be assigned together to a node")); } /** From a2661a83180507967e63f964e6b521b127bf0bff Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 15:07:59 -0700 Subject: [PATCH 48/51] Reduce || for comparing with cluster_manager and master role by warpping in methods Signed-off-by: Tianli Feng --- .../cluster/stats/ClusterStatsNodes.java | 2 +- .../cluster/node/DiscoveryNode.java | 2 +- .../cluster/node/DiscoveryNodeRole.java | 9 +++ .../cluster/node/DiscoveryNodes.java | 80 +++++++++++-------- .../cluster/node/DiscoveryNodeRoleTests.java | 10 +++ .../opensearch/test/InternalTestCluster.java | 7 +- .../java/org/opensearch/test/NodeRoles.java | 2 +- 7 files changed, 70 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java index a1782dc7a0c6f..fbca94780f827 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsNodes.java @@ -225,7 +225,7 @@ private Counts(final List nodeInfos) { for (DiscoveryNodeRole role : nodeInfo.getNode().getRoles()) { // TODO: Remove the 'if' condition and only keep the statement in 'else' after removing MASTER_ROLE. // As of 2.0, CLUSTER_MANAGER_ROLE is added, and it should be taken as MASTER_ROLE - if (role.equals(DiscoveryNodeRole.MASTER_ROLE) || role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + if (role.isClusterManager()) { roles.merge(DiscoveryNodeRole.MASTER_ROLE.roleName(), 1, Integer::sum); roles.merge(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), 1, Integer::sum); } else { diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 2e175ee744740..6bd943c5e1d0d 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -390,7 +390,7 @@ public void writeTo(StreamOutput out) throws IOException { .collect(Collectors.toList()); out.writeVInt(rolesToWrite.size()); for (final DiscoveryNodeRole role : rolesToWrite) { - if (role.equals(DiscoveryNodeRole.MASTER_ROLE) || role.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + if (role.isClusterManager()) { out.writeEnum(LegacyRole.MASTER); } else if (role.equals(DiscoveryNodeRole.DATA_ROLE)) { out.writeEnum(LegacyRole.DATA); diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 40bb35a1e0e36..aecf68f289541 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -312,4 +312,13 @@ public Setting legacySetting() { } + /** + * Check if the role is {@link #CLUSTER_MANAGER_ROLE} or {@link #MASTER_ROLE}. + * @deprecated As of 2.0, because promoting inclusive language. MASTER_ROLE is deprecated. + * @return true if the node role is{@link #CLUSTER_MANAGER_ROLE} or {@link #MASTER_ROLE} + */ + @Deprecated + public boolean isClusterManager() { + return this.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) || this.equals(DiscoveryNodeRole.MASTER_ROLE); + } } diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java index e2ee3d8551994..1097f3bc245ac 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java @@ -419,47 +419,46 @@ public String[] resolveNodes(String... nodes) { } else { resolvedNodesIds.removeAll(dataNodes.keys()); } - } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName) - || DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(masterNodes.keys()); - } else { - resolvedNodesIds.removeAll(masterNodes.keys()); - } - } else if (DiscoveryNodeRole.INGEST_ROLE.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(ingestNodes.keys()); - } else { - resolvedNodesIds.removeAll(ingestNodes.keys()); - } - } else if (DiscoveryNode.COORDINATING_ONLY.equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.addAll(getCoordinatingOnlyNodes().keys()); - } else { - resolvedNodesIds.removeAll(getCoordinatingOnlyNodes().keys()); - } + } else if (roleNameIsClusterManager(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(masterNodes.keys()); + } else { + resolvedNodesIds.removeAll(masterNodes.keys()); + } + } else if (DiscoveryNodeRole.INGEST_ROLE.roleName().equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(ingestNodes.keys()); + } else { + resolvedNodesIds.removeAll(ingestNodes.keys()); + } + } else if (DiscoveryNode.COORDINATING_ONLY.equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.addAll(getCoordinatingOnlyNodes().keys()); } else { - for (DiscoveryNode node : this) { - for (DiscoveryNodeRole role : Sets.difference(node.getRoles(), DiscoveryNodeRole.BUILT_IN_ROLES)) { - if (role.roleName().equals(matchAttrName)) { - if (Booleans.parseBoolean(matchAttrValue, true)) { - resolvedNodesIds.add(node.getId()); - } else { - resolvedNodesIds.remove(node.getId()); - } + resolvedNodesIds.removeAll(getCoordinatingOnlyNodes().keys()); + } + } else { + for (DiscoveryNode node : this) { + for (DiscoveryNodeRole role : Sets.difference(node.getRoles(), DiscoveryNodeRole.BUILT_IN_ROLES)) { + if (role.roleName().equals(matchAttrName)) { + if (Booleans.parseBoolean(matchAttrValue, true)) { + resolvedNodesIds.add(node.getId()); + } else { + resolvedNodesIds.remove(node.getId()); } } } - for (DiscoveryNode node : this) { - for (Map.Entry entry : node.getAttributes().entrySet()) { - String attrName = entry.getKey(); - String attrValue = entry.getValue(); - if (Regex.simpleMatch(matchAttrName, attrName) && Regex.simpleMatch(matchAttrValue, attrValue)) { - resolvedNodesIds.add(node.getId()); - } + } + for (DiscoveryNode node : this) { + for (Map.Entry entry : node.getAttributes().entrySet()) { + String attrName = entry.getKey(); + String attrValue = entry.getValue(); + if (Regex.simpleMatch(matchAttrName, attrName) && Regex.simpleMatch(matchAttrValue, attrValue)) { + resolvedNodesIds.add(node.getId()); } } } + } } } } @@ -798,4 +797,17 @@ public boolean isLocalNodeElectedMaster() { return masterNodeId != null && masterNodeId.equals(localNodeId); } } + + /** + * Check if the given name of the node role is 'cluster_manger' or 'master'. + * The method is added for {@link #resolveNodes} to keep the code clear, when support the both above roles. + * @deprecated As of 2.0, because promoting inclusive language. MASTER_ROLE is deprecated. + * @param matchAttrName a given String for a name of the node role. + * @return true if the given roleName is 'cluster_manger' or 'master' + */ + @Deprecated + private boolean roleNameIsClusterManager(String matchAttrName) { + return DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName) + || DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName().equals(matchAttrName); + } } diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java index 76f9843e386b8..d1acec2832b7c 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java @@ -128,4 +128,14 @@ public void testUnknownRoleIsDistinctFromKnownRoles() { assertNotEquals(buildInRole.toString(), unknownDataRole.toString()); } } + + /** + * Validate the method can identify the role as cluster-manager. + * Remove along with MASTER_ROLE. + */ + public void testIsClusterManager() { + assertTrue(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.isClusterManager()); + assertTrue(DiscoveryNodeRole.MASTER_ROLE.isClusterManager()); + assertFalse(randomFrom(DiscoveryNodeRole.DATA_ROLE.isClusterManager(), DiscoveryNodeRole.INGEST_ROLE.isClusterManager())); + } } diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 5dfe75db01c7b..504053e9006a6 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -787,16 +787,13 @@ private static String getRoleSuffix(Settings settings) { String suffix = ""; // only add the suffixes if roles are explicitly defined if (settings.hasValue("nodes.roles")) { - if (DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) - || DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE)) { + if (DiscoveryNode.isMasterNode(settings)) { suffix = suffix + DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleNameAbbreviation(); } if (DiscoveryNode.isDataNode(settings)) { suffix = suffix + DiscoveryNodeRole.DATA_ROLE.roleNameAbbreviation(); } - if ((DiscoveryNode.hasRole(settings, DiscoveryNodeRole.MASTER_ROLE) == false - && DiscoveryNode.hasRole(settings, DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) == false) - && DiscoveryNode.isDataNode(settings) == false) { + if (!DiscoveryNode.isMasterNode(settings) && !DiscoveryNode.isDataNode(settings)) { suffix = suffix + "c"; } } diff --git a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java index 728b4fd069fc6..64fd6b22e9805 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeRoles.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeRoles.java @@ -84,7 +84,7 @@ public static Settings removeRoles(final Settings settings, final Set (!r.equals(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE) && !r.equals(DiscoveryNodeRole.MASTER_ROLE)) + ? r -> !r.isClusterManager() : r -> true ) .map(DiscoveryNodeRole::roleName) From f59a27b4cd821b561a30eb16024f2eb5ed453628 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 16:06:41 -0700 Subject: [PATCH 49/51] Add deprecation message for setting master role Signed-off-by: Tianli Feng --- .../cluster/node/DiscoveryNodeRole.java | 17 ++++++++++++---- .../node/NodeRoleSettingsTests.java | 20 +++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index aecf68f289541..89234ee44ce37 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -34,6 +34,7 @@ import org.opensearch.LegacyESVersion; import org.opensearch.Version; +import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; @@ -52,6 +53,10 @@ */ public abstract class DiscoveryNodeRole implements Comparable { + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DiscoveryNodeRole.class); + public static final String MASTER_ROLE_DEPRECATION_MESSAGE = + "[master] role is deprecated. To promote inclusive language, please use [cluster_manager] role instead."; + private final String roleName; /** @@ -214,6 +219,11 @@ public Setting legacySetting() { return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope); } + @Override + public void validateRole(List roles) { + deprecationLogger.deprecate("node_role_master", MASTER_ROLE_DEPRECATION_MESSAGE); + } + }; /** @@ -242,16 +252,15 @@ public void validateRole(List roles) { throw new IllegalArgumentException( String.format( Locale.ROOT, - "The two roles [%s, %s] can not be assigned together to a node. " - + "To promote inclusive language, [%s] role is deprecated, and replaced by [%s] role.", + "The two roles [%s, %s] can not be assigned together to a node. %s", DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), - DiscoveryNodeRole.MASTER_ROLE.roleName(), - DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName() + MASTER_ROLE_DEPRECATION_MESSAGE ) ); } } + }; public static final DiscoveryNodeRole REMOTE_CLUSTER_CLIENT_ROLE = new DiscoveryNodeRole("remote_cluster_client", "r") { diff --git a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java index 2aaedd1f34ac1..c875fec1979d1 100644 --- a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java +++ b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java @@ -15,7 +15,6 @@ import java.util.Arrays; import java.util.Collections; -import java.util.List; import static org.hamcrest.Matchers.containsString; @@ -38,8 +37,21 @@ public void testClusterManagerAndMasterRoleCanNotCoexist() { */ public void testClusterManagerAndDataNodeRoles() { Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "cluster_manager, data").build(); - List actualNodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings); - List expectedNodeRoles = Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE); - assertEquals(expectedNodeRoles, actualNodeRoles); + assertEquals( + Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE), + NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings) + ); + } + + /** + * Validate setting master role will result a deprecation message. + * Remove the test after removing MASTER_ROLE. + */ + public void testMasterRoleDeprecationMessage() { + // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES in 2.0. + DiscoveryNode.setAdditionalRoles(Collections.emptySet()); + Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "master").build(); + assertEquals(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE), NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings)); + assertWarnings(DiscoveryNodeRole.MASTER_ROLE_DEPRECATION_MESSAGE); } } From 90f143afca0b28eb9d15d3b210bcc8339816230a Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 16:18:59 -0700 Subject: [PATCH 50/51] Modify the deprecation message for master role Signed-off-by: Tianli Feng --- .../java/org/opensearch/cluster/node/DiscoveryNodeRole.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index 89234ee44ce37..de6be74133da2 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -55,7 +55,7 @@ public abstract class DiscoveryNodeRole implements Comparable private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DiscoveryNodeRole.class); public static final String MASTER_ROLE_DEPRECATION_MESSAGE = - "[master] role is deprecated. To promote inclusive language, please use [cluster_manager] role instead."; + "Assigning [master] role in setting [node.roles] is deprecated. To promote inclusive language, please use [cluster_manager] role instead."; private final String roleName; From 87e50486bdc150409b4a867adf62ee240d0839b5 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 18 Mar 2022 03:03:19 +0000 Subject: [PATCH 51/51] Revert code about abbr name in RestNodesAction Signed-off-by: Tianli Feng --- .../org/opensearch/cluster/node/DiscoveryNodeRole.java | 2 +- .../org/opensearch/rest/action/cat/RestNodesAction.java | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java index de6be74133da2..cff1a77f4cdb7 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -139,7 +139,7 @@ public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) { /** * Validate the role is compatible with the other roles in the list, when assigning the list of roles to a node. * An {@link IllegalArgumentException} is expected to be thrown, if the role can't coexist with the other roles. - * @param roles A {@link List} of {@link DiscoveryNodeRole} that a node is going to have. + * @param roles a {@link List} of {@link DiscoveryNodeRole} that a node is going to have */ public void validateRole(List roles) {}; diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index a1b0f55826714..bce9b2d6b7e9d 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -419,13 +419,7 @@ Table buildTable( if (node.getRoles().isEmpty()) { roles = "-"; } else { - // TODO: Remove '.distinct()' after removing DiscoveryNodeRole.MASTER_ROLE, because the role abbr name should be unique. - roles = node.getRoles() - .stream() - .map(DiscoveryNodeRole::roleNameAbbreviation) - .sorted() - .distinct() - .collect(Collectors.joining()); + roles = node.getRoles().stream().map(DiscoveryNodeRole::roleNameAbbreviation).sorted().collect(Collectors.joining()); } table.addCell(roles); table.addCell(masterId == null ? "x" : masterId.equals(node.getId()) ? "*" : "-");