[7.16] Revert "Deprecate returning 408 for a server timeout on _cluster/health (#78180)"

client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java

@@ -338,11 +338,6 @@ public void testClusterHealthNotFoundIndex() throws IOException {
         assertThat(response.status(), equalTo(RestStatus.REQUEST_TIMEOUT));
         assertThat(response.getStatus(), equalTo(ClusterHealthStatus.RED));
         assertNoIndices(response);
-        assertWarnings(
-            "The HTTP status code for a cluster health timeout will be changed from 408 to 200 in a "
-                + "future version. Set the [return_200_for_cluster_health_timeout] query parameter to [true] to suppress this message and "
-                + "opt in to the future behaviour now."
-        );
     }
 
     public void testRemoteInfo() throws Exception {

docs/reference/cluster/health.asciidoc

@@ -97,11 +97,6 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=timeoutparms]
     provided or better, i.e. `green` > `yellow` > `red`. By default, will not
     wait for any status.
 
-`return_200_for_cluster_health_timeout`::
-   (Optional, Boolean) A boolean value which controls whether to return HTTP 200
-   status code instead of HTTP 408 in case of a cluster health timeout from
-   the server side. Defaults to false.
-
 [[cluster-health-api-response-body]]
 ==== {api-response-body-title}
 

docs/reference/migration/migrate_7_16.asciidoc

@@ -91,7 +91,7 @@ logging>>.
 ====
 *Details* +
 In SAML, Identity Providers (IdPs) can either be explicitly configured to
-release a `NameID` with a specific format, or configured to attempt to conform 
+release a `NameID` with a specific format, or configured to attempt to conform
 with the requirements of a Service Provider (SP). The SP declares its
 requirements in the `NameIDPolicy` element of a SAML Authentication Request.
 In {es}, the `nameid_format` SAML realm setting controls the `NameIDPolicy`
@@ -106,9 +106,9 @@ IdP. If you want to retain the previous behavior, set `nameid_format` to
 
 *Impact* +
 If you currently don't configure `nameid_format` explicitly, it's possible
-that your IdP will reject authentication requests from {es} because the requests 
+that your IdP will reject authentication requests from {es} because the requests
 do not specify a `NameID` format (and your IdP is configured to expect one).
-This mismatch can result in a broken SAML configuration. If you're unsure whether 
+This mismatch can result in a broken SAML configuration. If you're unsure whether
 your IdP is explicitly configured to use a certain `NameID` format and you want to retain current behavior
 , try setting `nameid_format` to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient` explicitly.
 ====
@@ -290,7 +290,7 @@ The `estimated_heap_memory_usage_bytes` property in the
 {ref}/put-trained-models.html[create trained models API] is deprecated in 7.16.
 
 *Impact* +
-Use `model_size_bytes` instead. 
+Use `model_size_bytes` instead.
 ====
 
 [discrete]
@@ -324,14 +324,14 @@ guide].
 *Details* +
 Directly accessing system indices is deprecated. In {es} 8.0, you must add the
 `allow_restricted_indices` permission set to `true` on a user role to access
-system indices. Refer to 
+system indices. Refer to
 {ref}/defining-roles.html#roles-indices-priv[indices privileges] for
 information on adding this permission to an index privilege.
 
 *Impact* +
-Accessing system indices directly results in warnings in the header of API 
+Accessing system indices directly results in warnings in the header of API
 responses and in the deprecation logs. Use {kib} or the associated feature's
-{es} APIs to manage the data that you want to access. While it's still possible 
+{es} APIs to manage the data that you want to access. While it's still possible
 to access system indices in {es} 7.16, they are reserved only for internal use
 by Elastic products and should not be accessed directly.
 ====
@@ -340,27 +340,6 @@ by Elastic products and should not be accessed directly.
 [[breaking_716_cluster_deprecations]]
 ==== Cluster deprecations
 
-[[deprecate-cluster-health-408]]
-.Distinguishing cluster health timeout status by HTTP response code is now deprecated.
-[%collapsible]
-====
-*Details* +
-The {ref}/cluster-health.html[cluster health API] includes options for waiting
-for certain health conditions to be satisfied. If the requested conditions are
-not satisfied within a timeout then {es} will send back a normal response
-including the field `"timed_out": true`. In earlier versions it would also use
-the HTTP response code `408 Request timeout` if the request timed out, and `200
-OK` otherwise. The `408 Request timeout` response code is not appropriate for
-this situation and its use is deprecated. Future versions will use the response
-code `200 OK` for both cases.
-
-*Impact* +
-Update your application to read the `"timed_out"` field of the response instead
-of the HTTP response code to determine whether the request timed out or not. To
-avoid deprecation warnings and opt into the future behaviour, include the query
-parameter `?return_200_for_cluster_health_timeout` in your request.
-====
-
 [[script-context-cache-deprecated]]
 .The script context cache is deprecated.
 [%collapsible]

rest-api-spec/src/main/resources/rest-api-spec/api/cluster.health.json

@@ -102,10 +102,6 @@
           "red"
         ],
         "description":"Wait until cluster is in a specific state"
-      },
-      "return_200_for_cluster_health_timeout":{
-        "type":"boolean",
-        "description":"Whether to return HTTP 200 instead of 408 in case of a cluster health timeout from the server side"
       }
     }
   }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.health/20_request_timeout.yml

@@ -35,28 +35,3 @@
   - match:     { initializing_shards:     0 }
   - match:     { unassigned_shards:       0 }
   - gte:       { number_of_pending_tasks: 0 }
-
----
-"cluster health request timeout with 200 response code":
-  - skip:
-      version: " - 7.15.99"
-      reason: "return_200_for_cluster_health_timeout was added in 7.16"
-      features: [ "allowed_warnings" ]
-  - do:
-      allowed_warnings:
-        - 'the [return_200_for_cluster_health_timeout] parameter is deprecated and will be removed in a future release.'
-      cluster.health:
-        timeout: 1ms
-        wait_for_active_shards: 5
-        return_200_for_cluster_health_timeout: true
-
-  - is_true:   cluster_name
-  - is_true:   timed_out
-  - gte:       { number_of_nodes:         1 }
-  - gte:       { number_of_data_nodes:    1 }
-  - match:     { active_primary_shards:   0 }
-  - match:     { active_shards:           0 }
-  - match:     { relocating_shards:       0 }
-  - match:     { initializing_shards:     0 }
-  - match:     { unassigned_shards:       0 }
-  - gte:       { number_of_pending_tasks: 0 }

server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequest.java

@@ -35,8 +35,6 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
     private ActiveShardCount waitForActiveShards = ActiveShardCount.NONE;
     private String waitForNodes = "";
     private Priority waitForEvents = null;
-    private boolean return200ForClusterHealthTimeout;
-
     /**
      * Only used by the high-level REST Client. Controls the details level of the health information returned.
      * The default value is 'cluster'.
@@ -70,9 +68,6 @@ public ClusterHealthRequest(StreamInput in) throws IOException {
         } else {
             indicesOptions = IndicesOptions.lenientExpandOpen();
         }
-        if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
-            return200ForClusterHealthTimeout = in.readBoolean();
-        }
     }
 
     @Override
@@ -105,11 +100,6 @@ public void writeTo(StreamOutput out) throws IOException {
         if (out.getVersion().onOrAfter(Version.V_7_2_0)) {
             indicesOptions.writeIndicesOptions(out);
         }
-        if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
-            out.writeBoolean(return200ForClusterHealthTimeout);
-        } else if (return200ForClusterHealthTimeout) {
-            throw new IllegalArgumentException("Can't fix response code in a cluster involving nodes with version " + out.getVersion());
-        }
     }
 
     @Override
@@ -253,18 +243,6 @@ public Priority waitForEvents() {
         return this.waitForEvents;
     }
 
-    public boolean doesReturn200ForClusterHealthTimeout() {
-        return return200ForClusterHealthTimeout;
-    }
-
-    /**
-     * Sets whether to return HTTP 200 status code instead of HTTP 408 in case of a
-     * cluster health timeout from the server side.
-     */
-    public void return200ForClusterHealthTimeout(boolean return200ForClusterHealthTimeout) {
-        this.return200ForClusterHealthTimeout = return200ForClusterHealthTimeout;
-    }
-
     /**
      * Set the level of detail for the health information to be returned.
      * Only used by the high-level REST Client.

server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java

@@ -8,7 +8,6 @@
 
 package org.elasticsearch.action.admin.cluster.health;
 
-import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
@@ -17,12 +16,9 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.logging.DeprecationCategory;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.StatusToXContentObject;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.rest.RestStatus;
-import org.elasticsearch.rest.action.search.RestSearchAction;
 import org.elasticsearch.xcontent.ConstructingObjectParser;
 import org.elasticsearch.xcontent.ObjectParser;
 import org.elasticsearch.xcontent.ParseField;
@@ -59,7 +55,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
     private static final String INITIALIZING_SHARDS = "initializing_shards";
     private static final String UNASSIGNED_SHARDS = "unassigned_shards";
     private static final String INDICES = "indices";
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestSearchAction.class);
 
     private static final ConstructingObjectParser<ClusterHealthResponse, Void> PARSER = new ConstructingObjectParser<>(
         "cluster_health_response",
@@ -124,12 +119,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
         XContentParser parser,
         Void context,
         String index) -> ClusterIndexHealth.innerFromXContent(parser, index);
-    static final String ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY = "return_200_for_cluster_health_timeout";
-    static final String CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG = "The HTTP status code for a cluster health timeout "
-        + "will be changed from 408 to 200 in a future version. Set the ["
-        + ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY
-        + "] "
-        + "query parameter to [true] to suppress this message and opt in to the future behaviour now.";
 
     static {
         // ClusterStateHealth fields
@@ -162,7 +151,8 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
     private boolean timedOut = false;
     private ClusterStateHealth clusterStateHealth;
     private ClusterHealthStatus clusterHealthStatus;
-    private boolean return200ForClusterHealthTimeout;
+
+    public ClusterHealthResponse() {}
 
     public ClusterHealthResponse(StreamInput in) throws IOException {
         super(in);
@@ -174,19 +164,11 @@ public ClusterHealthResponse(StreamInput in) throws IOException {
         numberOfInFlightFetch = in.readInt();
         delayedUnassignedShards = in.readInt();
         taskMaxWaitingTime = in.readTimeValue();
-        if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
-            return200ForClusterHealthTimeout = in.readBoolean();
-        }
     }
 
     /** needed for plugins BWC */
-    public ClusterHealthResponse(
-        String clusterName,
-        String[] concreteIndices,
-        ClusterState clusterState,
-        boolean return200ForServerTimeout
-    ) {
-        this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0), return200ForServerTimeout);
+    public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState) {
+        this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0));
     }
 
     public ClusterHealthResponse(
@@ -196,8 +178,7 @@ public ClusterHealthResponse(
         int numberOfPendingTasks,
         int numberOfInFlightFetch,
         int delayedUnassignedShards,
-        TimeValue taskMaxWaitingTime,
-        boolean return200ForServerTimeout
+        TimeValue taskMaxWaitingTime
     ) {
         this.clusterName = clusterName;
         this.numberOfPendingTasks = numberOfPendingTasks;
@@ -206,7 +187,6 @@ public ClusterHealthResponse(
         this.taskMaxWaitingTime = taskMaxWaitingTime;
         this.clusterStateHealth = new ClusterStateHealth(clusterState, concreteIndices);
         this.clusterHealthStatus = clusterStateHealth.getStatus();
-        this.return200ForClusterHealthTimeout = return200ForServerTimeout;
     }
 
     /**
@@ -345,11 +325,6 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeInt(numberOfInFlightFetch);
         out.writeInt(delayedUnassignedShards);
         out.writeTimeValue(taskMaxWaitingTime);
-        if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
-            out.writeBoolean(return200ForClusterHealthTimeout);
-        } else if (return200ForClusterHealthTimeout) {
-            throw new IllegalArgumentException("Can't fix response code in a cluster involving nodes with version " + out.getVersion());
-        }
     }
 
     @Override
@@ -359,19 +334,7 @@ public String toString() {
 
     @Override
     public RestStatus status() {
-        if (isTimedOut() == false) {
-            return RestStatus.OK;
-        }
-        if (return200ForClusterHealthTimeout) {
-            return RestStatus.OK;
-        } else {
-            deprecationLogger.critical(
-                DeprecationCategory.API,
-                "cluster_health_request_timeout",
-                CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG
-            );
-            return RestStatus.REQUEST_TIMEOUT;
-        }
+        return isTimedOut() ? RestStatus.REQUEST_TIMEOUT : RestStatus.OK;
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java

@@ -418,8 +418,7 @@ private ClusterHealthResponse clusterHealth(
                 numberOfPendingTasks,
                 numberOfInFlightFetch,
                 UnassignedInfo.getNumberOfDelayedUnassigned(clusterState),
-                pendingTaskTimeInQueue,
-                request.doesReturn200ForClusterHealthTimeout()
+                pendingTaskTimeInQueue
             );
             response.setStatus(ClusterHealthStatus.RED);
             return response;
@@ -432,8 +431,7 @@ private ClusterHealthResponse clusterHealth(
             numberOfPendingTasks,
             numberOfInFlightFetch,
             UnassignedInfo.getNumberOfDelayedUnassigned(clusterState),
-            pendingTaskTimeInQueue,
-            request.doesReturn200ForClusterHealthTimeout()
+            pendingTaskTimeInQueue
         );
     }
 }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterHealthAction.java

@@ -83,9 +83,6 @@ public static ClusterHealthRequest fromRequest(final RestRequest request) {
         if (request.param("wait_for_events") != null) {
             clusterHealthRequest.waitForEvents(Priority.valueOf(request.param("wait_for_events").toUpperCase(Locale.ROOT)));
         }
-        clusterHealthRequest.return200ForClusterHealthTimeout(
-            request.paramAsBoolean("return_200_for_cluster_health_timeout", clusterHealthRequest.doesReturn200ForClusterHealthTimeout())
-        );
         return clusterHealthRequest;
     }
 

server/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequestTests.java

@@ -42,7 +42,6 @@ public void testSerialize() throws Exception {
         assertThat(cloneRequest.waitForEvents(), equalTo(originalRequest.waitForEvents()));
         assertIndicesEquals(cloneRequest.indices(), originalRequest.indices());
         assertThat(cloneRequest.indicesOptions(), equalTo(originalRequest.indicesOptions()));
-        assertThat(cloneRequest.doesReturn200ForClusterHealthTimeout(), equalTo(originalRequest.doesReturn200ForClusterHealthTimeout()));
     }
 
     public void testRequestReturnsHiddenIndicesByDefault() {

server/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponsesTests.java

@@ -15,7 +15,6 @@
 import org.elasticsearch.cluster.health.ClusterIndexHealthTests;
 import org.elasticsearch.cluster.health.ClusterStateHealth;
 import org.elasticsearch.cluster.metadata.Metadata;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -44,26 +43,17 @@ public class ClusterHealthResponsesTests extends AbstractSerializingTestCase<Clu
     private final ClusterHealthRequest.Level level = randomFrom(ClusterHealthRequest.Level.values());
 
     public void testIsTimeout() {
-        ClusterHealthResponse res = new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, false);
+        ClusterHealthResponse res = new ClusterHealthResponse();
         for (int i = 0; i < 5; i++) {
             res.setTimedOut(randomBoolean());
             if (res.isTimedOut()) {
                 assertEquals(RestStatus.REQUEST_TIMEOUT, res.status());
-                assertWarnings(ClusterHealthResponse.CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG);
             } else {
                 assertEquals(RestStatus.OK, res.status());
             }
         }
     }
 
-    public void testTimeoutReturns200IfOptedIn() {
-        ClusterHealthResponse res = new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, true);
-        for (int i = 0; i < 5; i++) {
-            res.setTimedOut(randomBoolean());
-            assertEquals(RestStatus.OK, res.status());
-        }
-    }
-
     public void testClusterHealth() throws IOException {
         ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
         int pendingTasks = randomIntBetween(0, 200);
@@ -77,8 +67,7 @@ public void testClusterHealth() throws IOException {
             pendingTasks,
             inFlight,
             delayedUnassigned,
-            pendingTaskInQueueTime,
-            false
+            pendingTaskInQueueTime
         );
         clusterHealth = maybeSerialize(clusterHealth);
         assertClusterHealth(clusterHealth);