From e9c5ce355c67860dae856e7745a472cab9004c38 Mon Sep 17 00:00:00 2001 From: Yaliang Wu Date: Tue, 14 Jun 2022 22:48:48 +0000 Subject: [PATCH] Support dynamic node role (#3436) * Support unknown node role Currently OpenSearch only supports several built-in nodes like data node role. If specify unknown node role, OpenSearch node will fail to start. This limit how to extend OpenSearch to support some extension function. For example, user may prefer to run ML tasks on some dedicated node which doesn't serve as any built-in node roles. So the ML tasks won't impact OpenSearch core function. This PR removed the limitation and user can specify any node role and OpenSearch will start node correctly with that unknown role. This opens the door for plugin developer to run specific tasks on dedicated nodes. Issue: https://github.com/opensearch-project/OpenSearch/issues/2877 Signed-off-by: Yaliang Wu * fix cat nodes rest API spec Signed-off-by: Yaliang Wu * fix mixed cluster IT failure Signed-off-by: Yaliang Wu * add DynamicRole Signed-off-by: Yaliang Wu * change generator method name Signed-off-by: Yaliang Wu * fix failed docker test Signed-off-by: Yaliang Wu * transform role name to lower case to avoid confusion Signed-off-by: Yaliang Wu * transform the node role abbreviation to lower case Signed-off-by: Yaliang Wu * fix checkstyle Signed-off-by: Yaliang Wu * add test for case-insensitive role name change Signed-off-by: Yaliang Wu --- .../resources/rest-api-spec/test/11_nodes.yml | 8 +- .../rest-api-spec/test/cat.nodes/10_basic.yml | 12 +-- .../cluster/node/DiscoveryNode.java | 15 +++- .../cluster/node/DiscoveryNodeRole.java | 50 ++++++++++-- .../rest/action/cat/RestNodesAction.java | 14 +++- .../node/DiscoveryNodeRoleGenerator.java | 16 ++++ .../cluster/node/DiscoveryNodeRoleTests.java | 23 +++++- .../cluster/node/DiscoveryNodeTests.java | 11 +++ .../node/NodeRoleSettingsTests.java | 20 +++++ .../rest/action/cat/RestNodesActionTests.java | 76 ++++++++++++++++--- 10 files changed, 210 insertions(+), 35 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleGenerator.java diff --git a/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml b/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml index a6b78645087f4..1c10166e96eeb 100644 --- a/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml +++ b/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml @@ -24,8 +24,8 @@ - match: $body: | - / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role cluster_manager name - ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role node.roles cluster_manager name + ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) (\s+ (-|\w+(,\w+)*+))? \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: @@ -33,8 +33,8 @@ - match: $body: | - /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ cluster_manager \s+ name \n - ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ node\.roles \s+ cluster_manager \s+ name \n + ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) (\s+ (-|\w+(,\w+)*+ ))? \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml index f04c674d420ee..525d705de88f1 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml @@ -10,9 +10,9 @@ v: true 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, + # 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, + # 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" @@ -32,8 +32,8 @@ - match: $body: | - / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role cluster_manager name - ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role node.roles cluster_manager name + ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) (\s+ (-|\w+(,\w+)*+))? \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: @@ -41,8 +41,8 @@ - match: $body: | - /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ cluster_manager \s+ name \n - ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role (\s+ node\.roles)? \s+ cluster_manager \s+ name \n + ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) (\s+ (-|\w+(,\w+)*+ ))? \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: 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 38e4cb6d8791a..0d55624a35998 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -52,6 +52,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -328,7 +329,11 @@ public DiscoveryNode(StreamInput in) throws IOException { } final DiscoveryNodeRole role = roleMap.get(roleName); if (role == null) { - roles.add(new DiscoveryNodeRole.UnknownRole(roleName, roleNameAbbreviation, canContainData)); + if (in.getVersion().onOrAfter(Version.V_2_1_0)) { + roles.add(new DiscoveryNodeRole.DynamicRole(roleName, roleNameAbbreviation, canContainData)); + } else { + roles.add(new DiscoveryNodeRole.UnknownRole(roleName, roleNameAbbreviation, canContainData)); + } } else { assert roleName.equals(role.roleName()) : "role name [" + roleName + "] does not match role [" + role.roleName() + "]"; assert roleNameAbbreviation.equals(role.roleNameAbbreviation()) : "role name abbreviation [" @@ -567,10 +572,12 @@ private static Map rolesToMap(final Stream roleMap = rolesToMap(DiscoveryNodeRole.BUILT_IN_ROLES.stream()); public static DiscoveryNodeRole getRoleFromRoleName(final String roleName) { - if (roleMap.containsKey(roleName) == false) { - throw new IllegalArgumentException("unknown role [" + roleName + "]"); + // As we are supporting dynamic role, should make role name case-insensitive to avoid confusion of role name like "Data"/"DATA" + String lowerCasedRoleName = Objects.requireNonNull(roleName).toLowerCase(Locale.ROOT); + if (roleMap.containsKey(lowerCasedRoleName)) { + return roleMap.get(lowerCasedRoleName); } - return roleMap.get(roleName); + return new DiscoveryNodeRole.DynamicRole(lowerCasedRoleName, lowerCasedRoleName, false); } public static Set getPossibleRoles() { 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 26ace4b9d80c1..5685667c05b1a 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java @@ -58,7 +58,6 @@ public abstract class DiscoveryNodeRole implements Comparable private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DiscoveryNodeRole.class); public static final String MASTER_ROLE_DEPRECATION_MESSAGE = "Assigning [master] role in setting [node.roles] is deprecated. To promote inclusive language, please use [cluster_manager] role instead."; - private final String roleName; /** @@ -95,6 +94,8 @@ public final boolean canContainData() { private final boolean isKnownRole; + private final boolean isDynamicRole; + /** * Whether this role is known by this node, or is an {@link DiscoveryNodeRole.UnknownRole}. */ @@ -102,6 +103,10 @@ public final boolean isKnownRole() { return isKnownRole; } + public final boolean isDynamicRole() { + return isDynamicRole; + } + public boolean isEnabledByDefault(final Settings settings) { return legacySetting() != null && legacySetting().get(settings); } @@ -111,18 +116,21 @@ protected DiscoveryNodeRole(final String roleName, final String roleNameAbbrevia } protected DiscoveryNodeRole(final String roleName, final String roleNameAbbreviation, final boolean canContainData) { - this(true, roleName, roleNameAbbreviation, canContainData); + this(true, false, roleName, roleNameAbbreviation, canContainData); } private DiscoveryNodeRole( final boolean isKnownRole, + final boolean isDynamicRole, final String roleName, final String roleNameAbbreviation, final boolean canContainData ) { this.isKnownRole = isKnownRole; - this.roleName = Objects.requireNonNull(roleName); - this.roleNameAbbreviation = Objects.requireNonNull(roleNameAbbreviation); + this.isDynamicRole = isDynamicRole; + // As we are supporting dynamic role, should make role name case-insensitive to avoid confusion of role name like "Data"/"DATA" + this.roleName = Objects.requireNonNull(roleName).toLowerCase(Locale.ROOT); + this.roleNameAbbreviation = Objects.requireNonNull(roleNameAbbreviation).toLowerCase(Locale.ROOT); this.canContainData = canContainData; } @@ -153,12 +161,13 @@ public final boolean equals(Object o) { return roleName.equals(that.roleName) && roleNameAbbreviation.equals(that.roleNameAbbreviation) && canContainData == that.canContainData - && isKnownRole == that.isKnownRole; + && isKnownRole == that.isKnownRole + && isDynamicRole == that.isDynamicRole; } @Override public final int hashCode() { - return Objects.hash(isKnownRole, roleName(), roleNameAbbreviation(), canContainData()); + return Objects.hash(isKnownRole, isDynamicRole, roleName(), roleNameAbbreviation(), canContainData()); } @Override @@ -178,6 +187,7 @@ public final String toString() { + ", canContainData=" + canContainData + (isKnownRole ? "" : ", isKnownRole=false") + + (isDynamicRole ? "" : ", isDynamicRole=false") + '}'; } @@ -311,7 +321,7 @@ static class UnknownRole extends DiscoveryNodeRole { * @param canContainData whether or not nodes with the role can contain data */ UnknownRole(final String roleName, final String roleNameAbbreviation, final boolean canContainData) { - super(false, roleName, roleNameAbbreviation, canContainData); + super(false, false, roleName, roleNameAbbreviation, canContainData); } @Override @@ -323,6 +333,32 @@ public Setting legacySetting() { } + /** + * Represents a dynamic role. This can occur if a custom role that not in {@link DiscoveryNodeRole#BUILT_IN_ROLES} added for a node. + * Some plugin can support extension function with dynamic roles. For example, ML plugin may run machine learning tasks on nodes + * with "ml" dynamic role. + */ + static class DynamicRole extends DiscoveryNodeRole { + + /** + * Construct a dynamic role with the specified role name and role name abbreviation. + * + * @param roleName the role name + * @param roleNameAbbreviation the role name abbreviation + * @param canContainData whether or not nodes with the role can contain data + */ + DynamicRole(final String roleName, final String roleNameAbbreviation, final boolean canContainData) { + super(false, true, roleName, roleNameAbbreviation, canContainData); + } + + @Override + public Setting legacySetting() { + // return null as dynamic role has no legacy setting + return null; + } + + } + /** * 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. 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 661a53e3d37b8..aaa0413dc4c5f 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 @@ -195,10 +195,12 @@ protected Table getTableWithHeader(final RestRequest request) { table.addCell("load_5m", "alias:l;text-align:right;desc:5m load avg"); table.addCell("load_15m", "alias:l;text-align:right;desc:15m load avg"); table.addCell("uptime", "default:false;alias:u;text-align:right;desc:node uptime"); + // TODO: Deprecate "node.role", use "node.roles" which shows full node role names table.addCell( "node.role", "alias:r,role,nodeRole;desc:m:master eligible node, d:data node, i:ingest node, -:coordinating node only" ); + table.addCell("node.roles", "alias:rs,all roles;desc: -:coordinating node only"); // TODO: Remove the header alias 'master', after removing MASTER_ROLE. It's added for compatibility when using parameter 'h=master'. table.addCell("cluster_manager", "alias:cm,m,master;desc:*:current cluster manager"); table.addCell("name", "alias:n;desc:node name"); @@ -423,12 +425,22 @@ Table buildTable( table.addCell(jvmStats == null ? null : jvmStats.getUptime()); final String roles; + final String allRoles; if (node.getRoles().isEmpty()) { roles = "-"; + allRoles = "-"; } else { - roles = node.getRoles().stream().map(DiscoveryNodeRole::roleNameAbbreviation).sorted().collect(Collectors.joining()); + List knownNodeRoles = node.getRoles() + .stream() + .filter(DiscoveryNodeRole::isKnownRole) + .collect(Collectors.toList()); + roles = knownNodeRoles.size() > 0 + ? knownNodeRoles.stream().map(DiscoveryNodeRole::roleNameAbbreviation).sorted().collect(Collectors.joining()) + : "-"; + allRoles = node.getRoles().stream().map(DiscoveryNodeRole::roleName).sorted().collect(Collectors.joining(",")); } table.addCell(roles); + table.addCell(allRoles); table.addCell(clusterManagerId == null ? "x" : clusterManagerId.equals(node.getId()) ? "*" : "-"); table.addCell(node.getName()); diff --git a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleGenerator.java b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleGenerator.java new file mode 100644 index 0000000000000..c1aa9390fec94 --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleGenerator.java @@ -0,0 +1,16 @@ +/* + * 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.cluster.node; + +public class DiscoveryNodeRoleGenerator { + + public static DiscoveryNodeRole createDynamicRole(String roleName) { + return new DiscoveryNodeRole.DynamicRole(roleName, roleName, false); + } +} 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 d1acec2832b7c..f906a0f937d28 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeRoleTests.java @@ -38,6 +38,7 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.Locale; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasToString; @@ -117,7 +118,7 @@ public void testDiscoveryNodeRoleEqualsHashCode() { } - public void testUnknownRoleIsDistinctFromKnownRoles() { + public void testUnknownRoleIsDistinctFromKnownOrDynamicRoles() { for (DiscoveryNodeRole buildInRole : DiscoveryNodeRole.BUILT_IN_ROLES) { final DiscoveryNodeRole.UnknownRole unknownDataRole = new DiscoveryNodeRole.UnknownRole( buildInRole.roleName(), @@ -126,6 +127,15 @@ public void testUnknownRoleIsDistinctFromKnownRoles() { ); assertNotEquals(buildInRole, unknownDataRole); assertNotEquals(buildInRole.toString(), unknownDataRole.toString()); + final DiscoveryNodeRole.DynamicRole dynamicRole = new DiscoveryNodeRole.DynamicRole( + buildInRole.roleName(), + buildInRole.roleNameAbbreviation(), + buildInRole.canContainData() + ); + assertNotEquals(buildInRole, dynamicRole); + assertNotEquals(buildInRole.toString(), dynamicRole.toString()); + assertNotEquals(unknownDataRole, dynamicRole); + assertNotEquals(unknownDataRole.toString(), dynamicRole.toString()); } } @@ -138,4 +148,15 @@ public void testIsClusterManager() { assertTrue(DiscoveryNodeRole.MASTER_ROLE.isClusterManager()); assertFalse(randomFrom(DiscoveryNodeRole.DATA_ROLE.isClusterManager(), DiscoveryNodeRole.INGEST_ROLE.isClusterManager())); } + + public void testRoleNameIsCaseInsensitive() { + String roleName = "TestRole"; + String roleNameAbbreviation = "T"; + DiscoveryNodeRole unknownRole = new DiscoveryNodeRole.UnknownRole(roleName, roleNameAbbreviation, false); + assertEquals(roleName.toLowerCase(Locale.ROOT), unknownRole.roleName()); + assertEquals(roleNameAbbreviation.toLowerCase(Locale.ROOT), unknownRole.roleNameAbbreviation()); + DiscoveryNodeRole dynamicRole = new DiscoveryNodeRole.DynamicRole(roleName, roleNameAbbreviation, false); + assertEquals(roleName.toLowerCase(Locale.ROOT), dynamicRole.roleName()); + assertEquals(roleNameAbbreviation.toLowerCase(Locale.ROOT), dynamicRole.roleNameAbbreviation()); + } } 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 1b7f698ae1f5c..abd1cae1ed97d 100644 --- a/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java +++ b/server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java @@ -44,6 +44,7 @@ import java.net.InetAddress; import java.util.Collections; import java.util.HashSet; +import java.util.Locale; import java.util.Set; import java.util.stream.Collectors; @@ -193,4 +194,14 @@ private void runTestDiscoveryNodeIsRemoteClusterClient(final Settings settings, } } + public void testGetRoleFromRoleNameIsCaseInsensitive() { + String dataRoleName = "DATA"; + DiscoveryNodeRole dataNodeRole = DiscoveryNode.getRoleFromRoleName(dataRoleName); + assertEquals(DiscoveryNodeRole.DATA_ROLE, dataNodeRole); + + String dynamicRoleName = "TestRole"; + DiscoveryNodeRole dynamicNodeRole = DiscoveryNode.getRoleFromRoleName(dynamicRoleName); + assertEquals(dynamicRoleName.toLowerCase(Locale.ROOT), dynamicNodeRole.roleName()); + assertEquals(dynamicRoleName.toLowerCase(Locale.ROOT), dynamicNodeRole.roleNameAbbreviation()); + } } diff --git a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java index c875fec1979d1..3248b97b8b71f 100644 --- a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java +++ b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java @@ -15,6 +15,7 @@ import java.util.Arrays; import java.util.Collections; +import java.util.List; import static org.hamcrest.Matchers.containsString; @@ -54,4 +55,23 @@ public void testMasterRoleDeprecationMessage() { assertEquals(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE), NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings)); assertWarnings(DiscoveryNodeRole.MASTER_ROLE_DEPRECATION_MESSAGE); } + + public void testUnknownNodeRoleAndBuiltInRoleCanCoexist() { + String testRole = "test_role"; + Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "data, " + testRole).build(); + List nodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings); + assertEquals(2, nodeRoles.size()); + assertEquals(DiscoveryNodeRole.DATA_ROLE, nodeRoles.get(0)); + assertEquals(testRole, nodeRoles.get(1).roleName()); + assertEquals(testRole, nodeRoles.get(1).roleNameAbbreviation()); + } + + public void testUnknownNodeRoleOnly() { + String testRole = "test_role"; + Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), testRole).build(); + List nodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings); + assertEquals(1, nodeRoles.size()); + assertEquals(testRole, nodeRoles.get(0).roleName()); + assertEquals(testRole, nodeRoles.get(0).roleNameAbbreviation()); + } } diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestNodesActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestNodesActionTests.java index 593ad2907797e..6485ddd3bbc94 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestNodesActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestNodesActionTests.java @@ -40,7 +40,10 @@ import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.cluster.node.DiscoveryNodeRole; +import org.opensearch.cluster.node.DiscoveryNodeRoleGenerator; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.common.Table; import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestRequest; @@ -48,6 +51,11 @@ import org.junit.Before; import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; @@ -64,18 +72,15 @@ public void setUpAction() { } public void testBuildTableDoesNotThrowGivenNullNodeInfoAndStats() { - ClusterName clusterName = new ClusterName("cluster-1"); - DiscoveryNodes.Builder builder = DiscoveryNodes.builder(); - builder.add(new DiscoveryNode("node-1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT)); - DiscoveryNodes discoveryNodes = builder.build(); - ClusterState clusterState = mock(ClusterState.class); - when(clusterState.nodes()).thenReturn(discoveryNodes); - - ClusterStateResponse clusterStateResponse = new ClusterStateResponse(clusterName, clusterState, false); - NodesInfoResponse nodesInfoResponse = new NodesInfoResponse(clusterName, Collections.emptyList(), Collections.emptyList()); - NodesStatsResponse nodesStatsResponse = new NodesStatsResponse(clusterName, Collections.emptyList(), Collections.emptyList()); - - action.buildTable(false, new FakeRestRequest(), clusterStateResponse, nodesInfoResponse, nodesStatsResponse); + testBuildTableWithRoles(emptySet(), (table) -> { + Map> nodeInfoMap = table.getAsMap(); + List cells = nodeInfoMap.get("node.role"); + assertEquals(1, cells.size()); + assertEquals("-", cells.get(0).value); + cells = nodeInfoMap.get("node.roles"); + assertEquals(1, cells.size()); + assertEquals("-", cells.get(0).value); + }); } public void testCatNodesWithLocalDeprecationWarning() { @@ -89,4 +94,51 @@ public void testCatNodesWithLocalDeprecationWarning() { terminate(threadPool); } + + public void testBuildTableWithDynamicRoleOnly() { + Set roles = new HashSet<>(); + String roleName = "test_role"; + DiscoveryNodeRole testRole = DiscoveryNodeRoleGenerator.createDynamicRole(roleName); + roles.add(testRole); + + testBuildTableWithRoles(roles, (table) -> { + Map> nodeInfoMap = table.getAsMap(); + List cells = nodeInfoMap.get("node.roles"); + assertEquals(1, cells.size()); + assertEquals(roleName, cells.get(0).value); + }); + } + + public void testBuildTableWithBothBuiltInAndDynamicRoles() { + Set roles = new HashSet<>(); + roles.add(DiscoveryNodeRole.DATA_ROLE); + String roleName = "test_role"; + DiscoveryNodeRole testRole = DiscoveryNodeRoleGenerator.createDynamicRole(roleName); + roles.add(testRole); + + testBuildTableWithRoles(roles, (table) -> { + Map> nodeInfoMap = table.getAsMap(); + List cells = nodeInfoMap.get("node.roles"); + assertEquals(1, cells.size()); + assertEquals("data,test_role", cells.get(0).value); + }); + } + + private void testBuildTableWithRoles(Set roles, Consumer verificationFunction) { + ClusterName clusterName = new ClusterName("cluster-1"); + DiscoveryNodes.Builder builder = DiscoveryNodes.builder(); + + builder.add(new DiscoveryNode("node-1", buildNewFakeTransportAddress(), emptyMap(), roles, Version.CURRENT)); + DiscoveryNodes discoveryNodes = builder.build(); + ClusterState clusterState = mock(ClusterState.class); + when(clusterState.nodes()).thenReturn(discoveryNodes); + + ClusterStateResponse clusterStateResponse = new ClusterStateResponse(clusterName, clusterState, false); + NodesInfoResponse nodesInfoResponse = new NodesInfoResponse(clusterName, Collections.emptyList(), Collections.emptyList()); + NodesStatsResponse nodesStatsResponse = new NodesStatsResponse(clusterName, Collections.emptyList(), Collections.emptyList()); + + Table table = action.buildTable(false, new FakeRestRequest(), clusterStateResponse, nodesInfoResponse, nodesStatsResponse); + + verificationFunction.accept(table); + } }