From 00fb2b72dc253c5697e5bc2410830b66879d6bc3 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Tue, 14 Jun 2022 22:41:06 -0700 Subject: [PATCH] Support dynamic node role (#3436) (#3585) --- .../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 445267d772827..16cf08e3cf920 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); + } }