Add a new node role 'cluster_manager' as the alternative for 'master'…

… role and deprecate 'master' role (#2424) Add a new node role `cluster_manager`, as the replacement for `master` role, that used in setting `node.roles: [ master ]`. They have got the same functionality, but can NOT be assigned to `node.roles` together. * Add `CLUSTER_MANAGER_ROLE` in `DiscoveryNodeRole` class, and deprecate `MASTER_ROLE` * Remove `MASTER_ROLE` from the `DiscoveryNodeRole.BUILT_IN_ROLES`, and temporarily load the role by the existing method `DiscoveryNode.setAdditionalRoles` * Add a method `validateRole()` in `DiscoveryNodeRole` class, it's used to validate a specific role is compatible with the other roles to be assigned to a node together. * Add deprecation message when assigning `master` role in setting `node.roles` * Replace most `MASTER_ROLE` with `CLUSTER_MANAGER_ROLE` in current unit and integration tests * Add new unit and integration tests to validate `CLUSTER_MANAGER_ROLE` and `MASTER_ROLE` can be treated as the same. More explanation: * New node will have "cluster_manager", "data", and "ingest" roles by default. Which means the default value of setting "node.roles" will be `["cluster_manager","data","ingest"]`, instead of `["master","data","ingest"]` * "cluster_manager" role will be treated as "master" role in the OpenSearch node of previous versions. * "cluster_manager” role and "master" role have got the same abbreviation name: "m" Signed-off-by: Tianli Feng <[email protected]>
opensearch-project · Mar 18, 2022 · ea31483 · ea31483
1 parent 641350b
commit ea31483
Show file tree

Hide file tree

Showing 52 changed files with 356 additions and 105 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
@@ -111,7 +111,7 @@ public static DiscoveryNode newNode(String nodeId, Map<String, String> 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/client/rest/src/main/java/org/opensearch/client/Node.java b/client/rest/src/main/java/org/opensearch/client/Node.java
@@ -210,21 +210,21 @@ public Roles(final Set<String> roles) {
         }
 
         /**
-         * Teturns whether or not the node <strong>could</strong> be elected master.
+         * Returns whether or not the node <strong>could</strong> be elected master.
          */
         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 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/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
@@ -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"
+      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
@@ -29,3 +39,21 @@ setup:
   - match: { nodes.$node_id.roles.2: "master" }
   - match: { nodes.$node_id.roles.3: "remote_cluster_client" }
 
+---
+"node_info role test":
+  - 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: "remote_cluster_client" }
diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
@@ -85,6 +85,7 @@ public void testNodeCounts() {
         Map<String, Integer> 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);
@@ -106,7 +107,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);
@@ -129,6 +130,7 @@ public void testNodeCounts() {
             }
             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
@@ -95,7 +95,7 @@ public Settings onNodeStopped(String nodeName) {
             })
         );
         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/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<NodeInfo> 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.isClusterManager()) {
+                            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/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);
     }
 
     /**
@@ -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);
@@ -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) {
+                if (role.isClusterManager()) {
                     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);
                 }
             }
@@ -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);
     }
 
     /**
@@ -591,15 +591,19 @@ public static void setAdditionalRoles(final Set<DiscoveryNodeRole> additionalRol
             + "], roles by name abbreviation ["
             + roleNameAbbreviationToPossibleRoles
             + "]";
-        roleMap = 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<String, DiscoveryNodeRole> roleNameToPossibleRolesWithMaster = new HashMap<>(roleNameToPossibleRoles);
+        roleNameToPossibleRolesWithMaster.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.MASTER_ROLE);
+        roleMap = Collections.unmodifiableMap(roleNameToPossibleRolesWithMaster);
     }
 
     public static Set<String> getPossibleRoleNames() {
         return roleMap.keySet();
     }
 
     /**
-     * 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
@@ -34,13 +34,16 @@
 
 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;
 import org.opensearch.transport.RemoteClusterService;
 
 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;
@@ -50,6 +53,10 @@
  */
 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;
 
     /**
@@ -129,6 +136,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<DiscoveryNodeRole> roles) {};
+
     @Override
     public final boolean equals(Object o) {
         if (this == o) return true;
@@ -193,15 +207,60 @@ public Setting<Boolean> 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<Boolean> 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);
+        }
+
+        @Override
+        public void validateRole(List<DiscoveryNodeRole> roles) {
+            deprecationLogger.deprecate("node_role_master", MASTER_ROLE_DEPRECATION_MESSAGE);
+        }
+
+    };
+
+    /**
+     * Represents the role for a cluster-manager-eligible node.
+     */
+    public static final DiscoveryNodeRole CLUSTER_MANAGER_ROLE = new DiscoveryNodeRole("cluster_manager", "m") {
+
         @Override
         public Setting<Boolean> 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);
         }
 
+        @Override
+        public DiscoveryNodeRole getCompatibilityRole(Version nodeVersion) {
+            if (nodeVersion.onOrAfter(Version.V_2_0_0)) {
+                return this;
+            } else {
+                return DiscoveryNodeRole.MASTER_ROLE;
+            }
+        }
+
+        @Override
+        public void validateRole(List<DiscoveryNodeRole> roles) {
+            if (roles.contains(DiscoveryNodeRole.MASTER_ROLE)) {
+                throw new IllegalArgumentException(
+                    String.format(
+                        Locale.ROOT,
+                        "The two roles [%s, %s] can not be assigned together to a node. %s",
+                        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") {
@@ -223,7 +282,7 @@ public Setting<Boolean> legacySetting() {
      * The built-in node roles.
      */
     public static SortedSet<DiscoveryNodeRole> 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, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE))
     );
 
     /**
@@ -262,4 +321,13 @@ public Setting<Boolean> 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
@@ -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);
@@ -419,7 +419,7 @@ public String[] resolveNodes(String... nodes) {
                             } else {
                                 resolvedNodesIds.removeAll(dataNodes.keys());
                             }
-                        } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName)) {
+                        } else if (roleNameIsClusterManager(matchAttrName)) {
                             if (Booleans.parseBoolean(matchAttrValue, true)) {
                                 resolvedNodesIds.addAll(masterNodes.keys());
                             } else {
@@ -797,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);
+    }
 }