From 74f6669b4983a9295dc0549ad15e44d70a18cc8f Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 26 Jul 2017 21:47:47 -0400 Subject: [PATCH] Timeout filter returns 504 and an inconsistent error body Our rules are that code of the error must match code of the response. We were also not setting the correct reason. This updates the timeout filter to be consistent with other clients, without changing the error code (504 is correct). The new message properly indicates the request may still be running, which the old message did not do. --- .../k8s.io/apiserver/pkg/server/filters/timeout.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go b/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go index e3e21a5f03..1e13f247f0 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go +++ b/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go @@ -27,7 +27,6 @@ import ( "time" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/endpoints/metrics" apirequest "k8s.io/apiserver/pkg/endpoints/request" ) @@ -64,9 +63,13 @@ func WithTimeoutForNonLongRunningRequests(handler http.Handler, requestContextMa if requestInfo.Namespace != "" { scope = "namespace" } - metrics.MonitorRequest(req, strings.ToUpper(requestInfo.Verb), requestInfo.Resource, requestInfo.Subresource, "", scope, http.StatusInternalServerError, 0, now) + if requestInfo.IsResourceRequest { + metrics.MonitorRequest(req, strings.ToUpper(requestInfo.Verb), requestInfo.Resource, requestInfo.Subresource, "", scope, http.StatusGatewayTimeout, 0, now) + } else { + metrics.MonitorRequest(req, strings.ToUpper(requestInfo.Verb), "", requestInfo.Path, "", scope, http.StatusGatewayTimeout, 0, now) + } } - return time.After(globalTimeout), metricFn, apierrors.NewServerTimeout(schema.GroupResource{Group: requestInfo.APIGroup, Resource: requestInfo.Resource}, requestInfo.Verb, 0) + return time.After(globalTimeout), metricFn, apierrors.NewTimeoutError(fmt.Sprintf("request did not complete within %s", globalTimeout), 0) } return WithTimeout(handler, timeoutFunc) } @@ -74,7 +77,7 @@ func WithTimeoutForNonLongRunningRequests(handler http.Handler, requestContextMa // WithTimeout returns an http.Handler that runs h with a timeout // determined by timeoutFunc. The new http.Handler calls h.ServeHTTP to handle // each request, but if a call runs for longer than its time limit, the -// handler responds with a 503 Service Unavailable error and the message +// handler responds with a 504 Gateway Timeout error and the message // provided. (If msg is empty, a suitable default message will be sent.) After // the handler times out, writes by h to its http.ResponseWriter will return // http.ErrHandlerTimeout. If timeoutFunc returns a nil timeout channel, no