From e0f770643ca505292698a7bd7e1f8d699994b7db Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 18 Mar 2022 15:09:37 -0700 Subject: [PATCH] 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 --- .../rest-api-spec/api/cluster.reroute.json | 1 + .../rest-api-spec/api/cluster.state.json | 1 + .../test/cluster.reroute/10_basic.yml | 15 +++++++++++++++ .../cluster.reroute/20_response_filtering.yml | 13 +++++++++++++ .../test/cluster.state/10_basic.yml | 15 +++++++++++++++ .../test/cluster.state/20_filtering.yml | 16 ++++++++++++++++ .../org/opensearch/cluster/ClusterState.java | 11 +++++++++++ .../cluster/RestClusterRerouteAction.java | 15 +++++++++++++++ .../admin/cluster/RestClusterStateAction.java | 19 ++++++++++++++++++- .../reroute/ClusterRerouteResponseTests.java | 5 +++-- .../opensearch/cluster/ClusterStateTests.java | 4 ++++ 11 files changed, 112 insertions(+), 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 index b0e8054fc9e..bcf27041106 100644 --- 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 index 017705082d1..c17e5b073e3 100644 --- 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 index 771f647a952..1b45e19c24a 100644 --- 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/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/20_response_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/20_response_filtering.yml index 437b78e6119..c05d1db5114 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.reroute/20_response_filtering.yml +++ b/rest-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 index b443e322f80..fa973454cfb 100644 --- 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 index 88da42ee876..3d20f1d0f7e 100644 --- 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 index 6e17be690dd..459c0b9502a 100644 --- 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 class ClusterState implements ToXContentFragment, Diffable 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 class ClusterState implements ToXContentFragment, Diffable 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 index 1d1feff36f2..f519da109ba 100644 --- 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.ClusterState; 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 class RestClusterRerouteAction extends BaseRestHandler { 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 routes() { return singletonList(new Route(POST, "/_cluster/reroute")); @@ -104,6 +111,14 @@ public class RestClusterRerouteAction extends BaseRestHandler { 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 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 index 5056e29d47d..32aa055c183 100644 --- 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.client.node.NodeClient; 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 class RestClusterStateAction extends BaseRestHandler { 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 class RestClusterStateAction extends BaseRestHandler { if (request.hasParam("metric")) { EnumSet 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/server/src/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java index f0384f24987..7a7bc188769 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/reroute/ClusterRerouteResponseTests.java @@ -94,6 +94,7 @@ public class ClusterRerouteResponseTests extends OpenSearchTestCase { + clusterState.stateUUID() + "\",\n" + " \"master_node\" : \"node0\",\n" + + " \"cluster_manager_node\" : \"node0\",\n" + " \"blocks\" : { },\n" + " \"nodes\" : {\n" + " \"node0\" : {\n" @@ -173,7 +174,7 @@ public class ClusterRerouteResponseTests extends OpenSearchTestCase { XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); Map 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 class ClusterRerouteResponseTests extends OpenSearchTestCase { + " \"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 index 4cc3108d6bf..7cbab104cd0 100644 --- 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 class ClusterStateTests extends OpenSearchTestCase { + " \"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 class ClusterStateTests extends OpenSearchTestCase { + " \"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 class ClusterStateTests extends OpenSearchTestCase { + " \"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 class ClusterStateTests extends OpenSearchTestCase { + " \"version\" : 0,\n" + " \"state_uuid\" : \"stateUUID\",\n" + " \"master_node\" : null,\n" + + " \"cluster_manager_node\" : null,\n" + " \"blocks\" : { },\n" + " \"nodes\" : { },\n" + " \"metadata\" : {\n"