Add 'cluster_manager_node' into ClusterState Metric as an alternative…

… to 'master_node' (#2415) * Add `cluster_manager_node` into `ClusterState Metric`, as an alternative to `master_node`. So that the request parameter "metric" in `Cluster reroute` and `Cluster state` API accept the new value `cluster_manager_node` * Deprecate the enum value of `Metric: MASTER_NODE("master_node")` * Add YAML REST tests for the new parameter value for "cluster state" and "cluster reroute" API Signed-off-by: Tianli Feng <[email protected]>
opensearch-project · Mar 18, 2022 · e0f7706 · e0f7706
1 parent ea31483
commit e0f7706
Show file tree

Hide file tree

Showing 11 changed files with 112 additions and 3 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.reroute.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.reroute.json
@@ -37,6 +37,7 @@
           "nodes",
           "routing_table",
           "master_node",
+          "cluster_manager_node",
           "version"
         ],
         "description":"Limit the information returned to the specified metrics. Defaults to all but metadata"

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.state.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.state.json
@@ -55,6 +55,7 @@
                 "routing_table",
                 "routing_nodes",
                 "master_node",
+                "cluster_manager_node",
                 "version"
               ],
               "description":"Limit the information returned to the specified metrics"

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/10_basic.yml
@@ -2,3 +2,18 @@
 "Basic sanity check":
   - do:
       cluster.reroute: {}
+
+---
+"Cluster reroute returns cluster_manager_node":
+  - skip:
+      version: " - 1.4.99"
+      reason: "The metric cluster_manager_node is added to cluster state in version 2.0.0"
+
+  - do:
+      cluster.reroute: {}
+
+  - set:
+      state.cluster_manager_node: node_id
+
+  - match: {state.master_node: $node_id}
+  - match: {state.cluster_manager_node: $node_id}
diff --git a/...-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/20_response_filtering.yml b/...-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/20_response_filtering.yml
@@ -3,6 +3,7 @@
   - do:
       cluster.reroute: {}
   - is_false: state.metadata
+
 ---
 "return metadata if requested":
   - do:
@@ -12,3 +13,15 @@
   - is_true: state.metadata
   - is_false: state.nodes
 
+---
+"Filter the cluster reroute by cluster_manager_node only should work":
+  - skip:
+      version: " - 1.4.99"
+      reason: "The metric cluster_manager_node is added to cluster state in version 2.0.0"
+
+  - do:
+      cluster.reroute:
+        metric: [ cluster_manager_node ]
+
+  - is_true: state.cluster_manager_node
+  - is_false: state.master_node
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/10_basic.yml
@@ -17,3 +17,18 @@
 
   - is_true: cluster_uuid
   - is_true: master_node
+
+---
+"Get cluster state returns cluster_manager_node":
+  - skip:
+      version: " - 1.4.99"
+      reason: "The metric cluster_manager_node is added to cluster state in version 2.0.0"
+
+  - do:
+      cluster.state: {}
+
+  - set:
+      cluster_manager_node: node_id
+
+  - match: {master_node: $node_id}
+  - match: {cluster_manager_node: $node_id}
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/20_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/20_filtering.yml
@@ -158,6 +158,7 @@ setup:
   - skip:
       version:  " - 6.3.99"
       reason:   "cluster state including cluster_uuid at the top level is new in v6.4.0 and higher"
+      features: allowed_warnings
 
   # Get the current cluster_uuid
   - do:
@@ -167,6 +168,8 @@ setup:
   - do:
       cluster.state:
         metric: [ master_node, version ]
+      allowed_warnings:
+        - 'Deprecated value [master_node] used for parameter [metric]. To promote inclusive language, please use [cluster_manager_node] instead. It will be unsupported in a future major version.'
 
   - match: { cluster_uuid: $cluster_uuid }
   - is_true: master_node
@@ -180,3 +183,16 @@ setup:
 
   - match: { cluster_uuid: $cluster_uuid }
   - is_true: routing_table
+
+---
+"Filter the cluster state by cluster_manager_node only should work":
+  - skip:
+      version: " - 1.4.99"
+      reason: "The metric cluster_manager_node is added to cluster state in version 2.0.0"
+
+  - do:
+      cluster.state:
+        metric: [ cluster_manager_node ]
+
+  - is_true: cluster_manager_node
+  - is_false: master_node
diff --git a/server/src/main/java/org/opensearch/cluster/ClusterState.java b/server/src/main/java/org/opensearch/cluster/ClusterState.java
@@ -402,7 +402,13 @@ public boolean supersedes(ClusterState other) {
 
     public enum Metric {
         VERSION("version"),
+
+        /**
+         * @deprecated As of 2.0, because promoting inclusive language, replaced by {@link #CLUSTER_MANAGER_NODE}
+         */
+        @Deprecated
         MASTER_NODE("master_node"),
+        CLUSTER_MANAGER_NODE("cluster_manager_node"),
         BLOCKS("blocks"),
         NODES("nodes"),
         METADATA("metadata"),
@@ -467,6 +473,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             builder.field("master_node", nodes().getMasterNodeId());
         }
 
+        // Value of the field is identical with the above, and aims to replace the above field.
+        if (metrics.contains(Metric.CLUSTER_MANAGER_NODE)) {
+            builder.field("cluster_manager_node", nodes().getMasterNodeId());
+        }
+
         if (metrics.contains(Metric.BLOCKS)) {
             builder.startObject("blocks");
 

diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterRerouteAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterRerouteAction.java
@@ -39,6 +39,7 @@
 import org.opensearch.cluster.routing.allocation.command.AllocationCommands;
 import org.opensearch.common.ParseField;
 import org.opensearch.common.Strings;
+import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.SettingsFilter;
 import org.opensearch.common.xcontent.ObjectParser;
@@ -78,6 +79,12 @@ public RestClusterRerouteAction(SettingsFilter settingsFilter) {
         this.settingsFilter = settingsFilter;
     }
 
+    // TODO: Remove the DeprecationLogger after removing MASTER_ROLE.
+    // It's used to log deprecation when request parameter 'metric' contains 'master_node'.
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterRerouteAction.class);
+    private static final String DEPRECATED_MESSAGE_MASTER_NODE =
+        "Deprecated value [master_node] used for parameter [metric]. To promote inclusive language, please use [cluster_manager_node] instead. It will be unsupported in a future major version.";
+
     @Override
     public List<Route> routes() {
         return singletonList(new Route(POST, "/_cluster/reroute"));
@@ -104,6 +111,14 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         final String metric = request.param("metric");
         if (metric == null) {
             request.params().put("metric", DEFAULT_METRICS);
+        } else {
+            // TODO: Remove the statements in 'else' after removing MASTER_ROLE.
+            EnumSet<ClusterState.Metric> metrics = ClusterState.Metric.parseString(request.param("metric"), true);
+            // Because "_all" value will add all Metric into metrics set, for prevent deprecation message shown in that case,
+            // add the check of validating metrics set doesn't contain all enum elements.
+            if (!metrics.equals(EnumSet.allOf(ClusterState.Metric.class)) && metrics.contains(ClusterState.Metric.MASTER_NODE)) {
+                deprecationLogger.deprecate("cluster_reroute_metric_parameter_master_node_value", DEPRECATED_MESSAGE_MASTER_NODE);
+            }
         }
         return channel -> client.admin().cluster().reroute(clusterRerouteRequest, new RestToXContentListener<>(channel));
     }

diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStateAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStateAction.java
@@ -40,6 +40,7 @@
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.common.Strings;
+import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.SettingsFilter;
 import org.opensearch.common.xcontent.ToXContent;
@@ -71,6 +72,12 @@ public RestClusterStateAction(SettingsFilter settingsFilter) {
         this.settingsFilter = settingsFilter;
     }
 
+    // TODO: Remove the DeprecationLogger after removing MASTER_ROLE.
+    // It's used to log deprecation when request parameter 'metric' contains 'master_node'.
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterStateAction.class);
+    private static final String DEPRECATED_MESSAGE_MASTER_NODE =
+        "Deprecated value [master_node] used for parameter [metric]. To promote inclusive language, please use [cluster_manager_node] instead. It will be unsupported in a future major version.";
+
     @Override
     public String getName() {
         return "cluster_state_action";
@@ -112,7 +119,17 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         if (request.hasParam("metric")) {
             EnumSet<ClusterState.Metric> metrics = ClusterState.Metric.parseString(request.param("metric"), true);
             // do not ask for what we do not need.
-            clusterStateRequest.nodes(metrics.contains(ClusterState.Metric.NODES) || metrics.contains(ClusterState.Metric.MASTER_NODE));
+            clusterStateRequest.nodes(
+                metrics.contains(ClusterState.Metric.NODES)
+                    || metrics.contains(ClusterState.Metric.MASTER_NODE)
+                    || metrics.contains(ClusterState.Metric.CLUSTER_MANAGER_NODE)
+            );
+            // TODO: Remove the DeprecationLogger after removing MASTER_ROLE.
+            // Because "_all" value will add all Metric into metrics set, for prevent deprecation message shown in that case,
+            // add the check of validating metrics set doesn't contain all enum elements.
+            if (!metrics.equals(EnumSet.allOf(ClusterState.Metric.class)) && metrics.contains(ClusterState.Metric.MASTER_NODE)) {
+                deprecationLogger.deprecate("cluster_state_metric_parameter_master_node_value", DEPRECATED_MESSAGE_MASTER_NODE);
+            }
             /*
              * there is no distinction in Java api between routing_table and routing_nodes, it's the same info set over the wire, one single
              * flag to ask for it

diff --git a/...rc/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java b/...rc/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java
@@ -94,6 +94,7 @@ public void testToXContent() throws IOException {
                     + clusterState.stateUUID()
                     + "\",\n"
                     + "    \"master_node\" : \"node0\",\n"
+                    + "    \"cluster_manager_node\" : \"node0\",\n"
                     + "    \"blocks\" : { },\n"
                     + "    \"nodes\" : {\n"
                     + "      \"node0\" : {\n"
@@ -173,7 +174,7 @@ public void testToXContent() throws IOException {
             XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint();
             Map<String, String> params = new HashMap<>();
             params.put("explain", "true");
-            params.put("metric", "version,master_node");
+            params.put("metric", "version,cluster_manager_node");
             clusterRerouteResponse.toXContent(builder, new ToXContent.MapParams(params));
             assertEquals(
                 "{\n"
@@ -184,7 +185,7 @@ public void testToXContent() throws IOException {
                     + "    \"state_uuid\" : \""
                     + clusterState.stateUUID()
                     + "\",\n"
-                    + "    \"master_node\" : \"node0\"\n"
+                    + "    \"cluster_manager_node\" : \"node0\"\n"
                     + "  },\n"
                     + "  \"explanations\" : [\n"
                     + "    {\n"

diff --git a/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java b/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java
@@ -147,6 +147,7 @@ public void testToXContent() throws IOException {
                 + "  \"version\" : 0,\n"
                 + "  \"state_uuid\" : \"stateUUID\",\n"
                 + "  \"master_node\" : \"masterNodeId\",\n"
+                + "  \"cluster_manager_node\" : \"masterNodeId\",\n"
                 + "  \"blocks\" : {\n"
                 + "    \"global\" : {\n"
                 + "      \"1\" : {\n"
@@ -352,6 +353,7 @@ public void testToXContent_FlatSettingTrue_ReduceMappingFalse() throws IOExcepti
                 + "  \"version\" : 0,\n"
                 + "  \"state_uuid\" : \"stateUUID\",\n"
                 + "  \"master_node\" : \"masterNodeId\",\n"
+                + "  \"cluster_manager_node\" : \"masterNodeId\",\n"
                 + "  \"blocks\" : {\n"
                 + "    \"global\" : {\n"
                 + "      \"1\" : {\n"
@@ -550,6 +552,7 @@ public void testToXContent_FlatSettingFalse_ReduceMappingTrue() throws IOExcepti
                 + "  \"version\" : 0,\n"
                 + "  \"state_uuid\" : \"stateUUID\",\n"
                 + "  \"master_node\" : \"masterNodeId\",\n"
+                + "  \"cluster_manager_node\" : \"masterNodeId\",\n"
                 + "  \"blocks\" : {\n"
                 + "    \"global\" : {\n"
                 + "      \"1\" : {\n"
@@ -772,6 +775,7 @@ public void testToXContentSameTypeName() throws IOException {
                 + "  \"version\" : 0,\n"
                 + "  \"state_uuid\" : \"stateUUID\",\n"
                 + "  \"master_node\" : null,\n"
+                + "  \"cluster_manager_node\" : null,\n"
                 + "  \"blocks\" : { },\n"
                 + "  \"nodes\" : { },\n"
                 + "  \"metadata\" : {\n"