Skip to content

Commit

Permalink
Add 'cluster_manager_node' into ClusterState Metric as an alternative…
Browse files Browse the repository at this point in the history
… 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 <ftianli@amazon.com>
  • Loading branch information
Tianli Feng authored Mar 18, 2022
1 parent ea31483 commit e0f7706
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"routing_table",
"routing_nodes",
"master_node",
"cluster_manager_node",
"version"
],
"description":"Limit the information returned to the specified metrics"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- do:
cluster.reroute: {}
- is_false: state.metadata

---
"return metadata if requested":
- do:
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
11 changes: 11 additions & 0 deletions server/src/main/java/org/opensearch/cluster/ClusterState.java
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit e0f7706

Please sign in to comment.