Remove cluster state from /_cluster/reroute response (#114231)

DaveCTurner · web-flow · commit 07c3acf1c0ae · 2024-10-08T07:59:57.000+01:00
Including the cluster state in responses to the `POST _cluster/state` API was deprecated in #90399 (v8.6.0) requiring callers to pass `?metric=none` to avoid the deprecation warning. This commit adjusts the behaviour as promised in v9 so that this API never returns the cluster state, and deprecates the `?metric` parameter itself. Closes #88978
diff --git a/docs/changelog/114231.yaml b/docs/changelog/114231.yaml
@@ -0,0 +1,17 @@
+pr: 114231
+summary: Remove cluster state from `/_cluster/reroute` response
+area: Allocation
+type: breaking
+issues:
+ - 88978
+breaking:
+  title: Remove cluster state from `/_cluster/reroute` response
+  area: REST API
+  details: >-
+    The `POST /_cluster/reroute` API no longer returns the cluster state in its
+    response. The `?metric` query parameter to this API now has no effect and
+    its use will be forbidden in a future version.
+  impact: >-
+    Cease usage of the `?metric` query parameter when calling the
+    `POST /_cluster/reroute` API.
+  notable: false
diff --git a/docs/reference/cluster/reroute.asciidoc b/docs/reference/cluster/reroute.asciidoc
@@ -10,7 +10,7 @@ Changes the allocation of shards in a cluster.
 [[cluster-reroute-api-request]]
 ==== {api-request-title}
 
-`POST /_cluster/reroute?metric=none`
+`POST /_cluster/reroute`
 
 [[cluster-reroute-api-prereqs]]
 ==== {api-prereq-title}
@@ -193,7 +193,7 @@ This is a short example of a simple reroute API call:
 
 [source,console]
 --------------------------------------------------
-POST /_cluster/reroute?metric=none
+POST /_cluster/reroute
 {
   "commands": [
     {
diff --git a/docs/reference/commands/shard-tool.asciidoc b/docs/reference/commands/shard-tool.asciidoc
@@ -95,7 +95,7 @@ Changing allocation id V8QXk-QXSZinZMT-NvEq4w to tjm9Ve6uTBewVFAlfUMWjA
 
 You should run the following command to allocate this shard:
 
-POST /_cluster/reroute?metric=none
+POST /_cluster/reroute
 {
   "commands" : [
     {
diff --git a/docs/reference/troubleshooting/common-issues/red-yellow-cluster-status.asciidoc b/docs/reference/troubleshooting/common-issues/red-yellow-cluster-status.asciidoc
@@ -2,12 +2,12 @@
 === Red or yellow cluster health status
 
 A red or yellow cluster health status indicates one or more shards are not assigned to
-a node. 
+a node.
 
 * **Red health status**: The cluster has some unassigned primary shards, which
-means that some operations such as searches and indexing may fail. 
-* **Yellow health status**: The cluster has no unassigned primary shards but some 
-unassigned replica shards. This increases your risk of data loss and can degrade 
+means that some operations such as searches and indexing may fail.
+* **Yellow health status**: The cluster has no unassigned primary shards but some
+unassigned replica shards. This increases your risk of data loss and can degrade
 cluster performance.
 
 When your cluster has a red or yellow health status, it will continue to process
@@ -16,8 +16,8 @@ cleanup activities until the cluster returns to green health status. For instanc
 some <<index-lifecycle-management,{ilm-init}>> actions require the index on which they
 operate to have a green health status.
 
-In many cases, your cluster will recover to green health status automatically. 
-If the cluster doesn't automatically recover, then you must <<fix-red-yellow-cluster-status,manually address>> 
+In many cases, your cluster will recover to green health status automatically.
+If the cluster doesn't automatically recover, then you must <<fix-red-yellow-cluster-status,manually address>>
 the remaining problems so management and cleanup activities can proceed.
 
 [discrete]
@@ -107,7 +107,7 @@ asynchronously in the background.
 
 [source,console]
 ----
-POST _cluster/reroute?metric=none
+POST _cluster/reroute
 ----
 
 [discrete]
@@ -231,10 +231,10 @@ unassigned. See <<high-jvm-memory-pressure>>.
 
 If a node containing a primary shard is lost, {es} can typically replace it
 using a replica on another node. If you can't recover the node and replicas
-don't exist or are irrecoverable, <<cluster-allocation-explain,Allocation 
-Explain>> will report `no_valid_shard_copy` and you'll need to do one of the following: 
+don't exist or are irrecoverable, <<cluster-allocation-explain,Allocation
+Explain>> will report `no_valid_shard_copy` and you'll need to do one of the following:
 
-* restore the missing data from <<snapshot-restore,snapshot>> 
+* restore the missing data from <<snapshot-restore,snapshot>>
 * index the missing data from its original data source
 * accept data loss on the index-level by running <<indices-delete-index,Delete Index>>
 * accept data loss on the shard-level by executing <<cluster-reroute,Cluster Reroute>> allocate_stale_primary or allocate_empty_primary command with `accept_data_loss: true`
@@ -246,7 +246,7 @@ resulting in data loss.
 +
 [source,console]
 ----
-POST _cluster/reroute?metric=none
+POST _cluster/reroute
 {
   "commands": [
     {
diff --git a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SnapshotBasedRecoveryIT.java b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SnapshotBasedRecoveryIT.java
@@ -203,7 +203,7 @@ private void cancelShard(String indexName, int shard, String nodeName) throws IO
             }
             builder.endObject();
 
-            Request request = new Request(HttpPost.METHOD_NAME, "/_cluster/reroute?pretty&metric=none");
+            Request request = new Request(HttpPost.METHOD_NAME, "/_cluster/reroute?pretty");
             request.setJsonEntity(Strings.toString(builder));
             Response response = client().performRequest(request);
             logger.info("--> Relocated primary to an older version {}", EntityUtils.toString(response.getEntity()));
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/10_basic.yml
@@ -1,5 +1,8 @@
 ---
 "Basic sanity check":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
+
   - do:
-      cluster.reroute:
-        metric: none
+        cluster.reroute: {}
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/11_explain.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/11_explain.yml
@@ -13,19 +13,25 @@ setup:
 
 ---
 "Explain API with empty command list":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
 
   - do:
       cluster.reroute:
         explain: true
         dry_run: true
-        metric: none
         body:
           commands: []
 
   - match: {explanations: []}
 
 ---
 "Explain API for non-existent node & shard":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
+
   - skip:
       features: [arbitrary_key]
 
@@ -39,7 +45,6 @@ setup:
       cluster.reroute:
         explain: true
         dry_run: true
-        metric: none
         body:
           commands:
             - cancel:
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/20_deprecated_response_filtering.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.reroute/20_deprecated_response_filtering.yml
@@ -1,21 +1,45 @@
 ---
-"Do not return metadata by default and produce deprecation warning":
+"Do not return metadata by default and emit no warning":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
+
+  - do:
+      cluster.reroute: {}
+  - is_false: state
+
+---
+"Do not return metadata with ?metric=none and produce deprecation warning":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
+
   - skip:
       features: [ "allowed_warnings" ]
+
   - do:
-      cluster.reroute: {}
+      cluster.reroute:
+        metric: none
       allowed_warnings:
-        - "The [state] field in the response to the reroute API is deprecated and will be removed in a future version. Specify ?metric=none to adopt the future behaviour."
-  - is_false: state.metadata
+        - >-
+          the [?metric] query parameter to the [POST /_cluster/reroute] API has no effect;
+          its use will be forbidden in a future version
+  - is_false: state
+
 ---
-"If requested return metadata and produce deprecation warning":
+"Do not return metadata with ?metric=metadata and produce deprecation warning":
+  - requires:
+      cluster_features: ["cluster.reroute.ignores_metric_param"]
+      reason: requires this feature
+
   - skip:
       features: [ "allowed_warnings" ]
+
   - do:
       cluster.reroute:
         metric: metadata
       allowed_warnings:
-        - "The [state] field in the response to the reroute API is deprecated and will be removed in a future version. Specify ?metric=none to adopt the future behaviour."
-  - is_true: state.metadata
-  - is_false: state.nodes
-
+        - >-
+          the [?metric] query parameter to the [POST /_cluster/reroute] API has no effect;
+          its use will be forbidden in a future version
+  - is_false: state
diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java
@@ -429,6 +429,7 @@
             org.elasticsearch.indices.IndicesFeatures,
             org.elasticsearch.repositories.RepositoriesFeatures,
             org.elasticsearch.action.admin.cluster.allocation.AllocationStatsFeatures,
+            org.elasticsearch.rest.action.admin.cluster.ClusterRerouteFeatures,
             org.elasticsearch.index.mapper.MapperFeatures,
             org.elasticsearch.ingest.IngestGeoIpFeatures,
             org.elasticsearch.search.SearchFeatures,
diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java
@@ -22,7 +22,6 @@
 import org.elasticsearch.common.xcontent.ChunkedToXContentObject;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.core.UpdateForV10;
-import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.rest.action.search.RestSearchAction;
 import org.elasticsearch.xcontent.ToXContent;
 
@@ -45,7 +44,6 @@ public class ClusterRerouteResponse extends ActionResponse implements IsAcknowle
     /**
      * To be removed when REST compatibility with {@link org.elasticsearch.Version#V_8_6_0} / {@link RestApiVersion#V_8} no longer needed
      */
-    @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION)    // to remove from the v9 API only
     @UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION)  // to remove entirely
     private final ClusterState state;
     private final RoutingExplanations explanations;
diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/MaxRetryAllocationDecider.java
@@ -35,7 +35,7 @@ public class MaxRetryAllocationDecider extends AllocationDecider {
         Setting.Property.NotCopyableOnResize
     );
 
-    private static final String RETRY_FAILED_API = "POST /_cluster/reroute?retry_failed&metric=none";
+    private static final String RETRY_FAILED_API = "POST /_cluster/reroute?retry_failed";
 
     public static final String NAME = "max_retry";
 
diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/ClusterRerouteFeatures.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/ClusterRerouteFeatures.java
@@ -0,0 +1,24 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.rest.action.admin.cluster;
+
+import org.elasticsearch.features.FeatureSpecification;
+import org.elasticsearch.features.NodeFeature;
+
+import java.util.Set;
+
+public class ClusterRerouteFeatures implements FeatureSpecification {
+    public static final NodeFeature CLUSTER_REROUTE_IGNORES_METRIC_PARAM = new NodeFeature("cluster.reroute.ignores_metric_param");
+
+    @Override
+    public Set<NodeFeature> getFeatures() {
+        return Set.of(CLUSTER_REROUTE_IGNORES_METRIC_PARAM);
+    }
+}
diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java
@@ -15,8 +15,11 @@
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.routing.allocation.command.AllocationCommands;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsFilter;
+import org.elasticsearch.core.UpdateForV10;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.Scope;
@@ -39,6 +42,8 @@
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterRerouteAction extends BaseRestHandler {
 
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterRerouteAction.class);
+
     private static final Set<String> RESPONSE_PARAMS = addToCopy(Settings.FORMAT_PARAMS, "metric");
 
     private static final ObjectParser<ClusterRerouteRequest, Void> PARSER = new ObjectParser<>("cluster_reroute");
@@ -51,7 +56,8 @@ public class RestClusterRerouteAction extends BaseRestHandler {
         PARSER.declareBoolean(ClusterRerouteRequest::dryRun, new ParseField("dry_run"));
     }
 
-    private static final String DEFAULT_METRICS = Strings.arrayToCommaDelimitedString(
+    @UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION) // no longer used, so can be removed
+    private static final String V8_DEFAULT_METRICS = Strings.arrayToCommaDelimitedString(
         EnumSet.complementOf(EnumSet.of(ClusterState.Metric.METADATA)).toArray()
     );
 
@@ -76,18 +82,36 @@ public boolean allowSystemIndexAccessByDefault() {
         return true;
     }
 
+    @UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION)
+    // actually UpdateForV11 because V10 still supports the V9 API including this deprecation message
+    private static final String METRIC_DEPRECATION_MESSAGE = """
+        the [?metric] query parameter to the [POST /_cluster/reroute] API has no effect; its use will be forbidden in a future version""";
+
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
         ClusterRerouteRequest clusterRerouteRequest = createRequest(request);
         settingsFilter.addFilterSettingParams(request);
         if (clusterRerouteRequest.explain()) {
             request.params().put("explain", Boolean.TRUE.toString());
         }
-        // by default, return everything but metadata
-        final String metric = request.param("metric");
-        if (metric == null) {
-            request.params().put("metric", DEFAULT_METRICS);
+
+        switch (request.getRestApiVersion()) {
+            case V_9 -> {
+                // always avoid returning the cluster state by forcing `?metric=none`; emit a warning if `?metric` is even present
+                if (request.hasParam("metric")) {
+                    deprecationLogger.critical(DeprecationCategory.API, "cluster-reroute-metric-param", METRIC_DEPRECATION_MESSAGE);
+                }
+                request.params().put("metric", "none");
+            }
+            case V_8, V_7 -> {
+                // by default, return everything but metadata
+                final String metric = request.param("metric");
+                if (metric == null) {
+                    request.params().put("metric", V8_DEFAULT_METRICS);
+                }
+            }
         }
+
         return channel -> client.execute(
             TransportClusterRerouteAction.TYPE,
             clusterRerouteRequest,
diff --git a/server/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification b/server/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification
@@ -16,6 +16,7 @@ org.elasticsearch.rest.RestFeatures
 org.elasticsearch.indices.IndicesFeatures
 org.elasticsearch.repositories.RepositoriesFeatures
 org.elasticsearch.action.admin.cluster.allocation.AllocationStatsFeatures
+org.elasticsearch.rest.action.admin.cluster.ClusterRerouteFeatures
 org.elasticsearch.index.mapper.MapperFeatures
 org.elasticsearch.ingest.IngestGeoIpFeatures
 org.elasticsearch.search.SearchFeatures
diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MaxRetryAllocationDeciderTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/MaxRetryAllocationDeciderTests.java
@@ -181,7 +181,7 @@ public void testFailedAllocation() {
                 decision.getExplanation(),
                 allOf(
                     containsString("shard has exceeded the maximum number of retries"),
-                    containsString("POST /_cluster/reroute?retry_failed&metric=none")
+                    containsString("POST /_cluster/reroute?retry_failed")
                 )
             );
         }
@@ -280,7 +280,7 @@ public void testFailedRelocation() {
                 decision.getExplanation(),
                 allOf(
                     containsString("shard has exceeded the maximum number of retries"),
-                    containsString("POST /_cluster/reroute?retry_failed&metric=none")
+                    containsString("POST /_cluster/reroute?retry_failed")
                 )
             );
         });
diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
@@ -982,7 +982,7 @@ public void testSearchableSnapshotTotalShardsPerNode() throws Exception {
      * notification that partial-index is now GREEN.
      */
     private void triggerStateChange() throws IOException {
-        Request rerouteRequest = new Request("POST", "/_cluster/reroute?metric=none");
+        Request rerouteRequest = new Request("POST", "/_cluster/reroute");
         client().performRequest(rerouteRequest);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ Changes the allocation of shards in a cluster.`
`10`	`10`	`[[cluster-reroute-api-request]]`
`11`	`11`	`==== {api-request-title}`
`12`	`12`
`13`		-`POST /_cluster/reroute?metric=none`
	`13`	+`POST /_cluster/reroute`
`14`	`14`
`15`	`15`	`[[cluster-reroute-api-prereqs]]`
`16`	`16`	`==== {api-prereq-title}`
`@@ -193,7 +193,7 @@ This is a short example of a simple reroute API call:`
`193`	`193`
`194`	`194`	`[source,console]`
`195`	`195`	`--------------------------------------------------`
`196`		`-POST /_cluster/reroute?metric=none`
	`196`	`+POST /_cluster/reroute`
`197`	`197`	`{`
`198`	`198`	`"commands": [`
`199`	`199`	`{`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ Changing allocation id V8QXk-QXSZinZMT-NvEq4w to tjm9Ve6uTBewVFAlfUMWjA`
`95`	`95`
`96`	`96`	`You should run the following command to allocate this shard:`
`97`	`97`
`98`		`-POST /_cluster/reroute?metric=none`
	`98`	`+POST /_cluster/reroute`
`99`	`99`	`{`
`100`	`100`	`"commands" : [`
`101`	`101`	`{`
Original file line number	Diff line number	Diff line change
`@@ -203,7 +203,7 @@ private void cancelShard(String indexName, int shard, String nodeName) throws IO`
`203`	`203`	`}`
`204`	`204`	`builder.endObject();`
`205`	`205`
`206`		`- Request request = new Request(HttpPost.METHOD_NAME, "/_cluster/reroute?pretty&metric=none");`
	`206`	`+ Request request = new Request(HttpPost.METHOD_NAME, "/_cluster/reroute?pretty");`
`207`	`207`	`request.setJsonEntity(Strings.toString(builder));`
`208`	`208`	`Response response = client().performRequest(request);`
`209`	`209`	`logger.info("--> Relocated primary to an older version {}", EntityUtils.toString(response.getEntity()));`