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: #2877

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* fix cat nodes rest API spec

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* fix mixed cluster IT failure

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* add DynamicRole

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* change generator method name

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* fix failed docker test

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* transform role name to lower case to avoid confusion

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* transform the node role abbreviation to lower case

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* fix checkstyle

Signed-off-by: Yaliang Wu <ylwu@amazon.com>

* add test for case-insensitive role name change

Signed-off-by: Yaliang Wu <ylwu@amazon.com>
(cherry picked from commit e9c5ce3)

distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml

@@ -24,17 +24,17 @@
 
   - 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:
         v: true
 
   - 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:

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,17 +32,17 @@
 
   - 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:
           v: true
 
   - 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:

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<String, DiscoveryNodeRole> rolesToMap(final Stream<DiscoveryN
     private static Map<String, DiscoveryNodeRole> 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<DiscoveryNodeRole> getPossibleRoles() {

server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java

@@ -58,7 +58,6 @@ public abstract class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole>
     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,13 +94,19 @@ 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}.
      */
     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<Boolean> 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<Boolean> 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.

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

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

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

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

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<DiscoveryNodeRole> 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<DiscoveryNodeRole> nodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings);
+        assertEquals(1, nodeRoles.size());
+        assertEquals(testRole, nodeRoles.get(0).roleName());
+        assertEquals(testRole, nodeRoles.get(0).roleNameAbbreviation());
+    }
 }