diff --git a/pkg/kubelet/server/server.go b/pkg/kubelet/server/server.go index e728daaf784..74a5036828a 100644 --- a/pkg/kubelet/server/server.go +++ b/pkg/kubelet/server/server.go @@ -39,7 +39,6 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" "k8s.io/api/core/v1" - apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -484,7 +483,7 @@ func (s *Server) getContainerLogs(request *restful.Request, response *restful.Re } logOptions.TypeMeta = metav1.TypeMeta{} if errs := validation.ValidatePodLogOptions(logOptions); len(errs) > 0 { - response.WriteError(apierrs.StatusUnprocessableEntity, fmt.Errorf(`{"message": "Invalid request."}`)) + response.WriteError(http.StatusUnprocessableEntity, fmt.Errorf(`{"message": "Invalid request."}`)) return } diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index 015947ddac4..b3b5ce9b46e 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -39,7 +39,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/api/core/v1" - apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/httpstream" @@ -1065,7 +1064,7 @@ func TestContainerLogsWithInvalidTail(t *testing.T) { t.Errorf("Got error GETing: %v", err) } defer resp.Body.Close() - if resp.StatusCode != apierrs.StatusUnprocessableEntity { + if resp.StatusCode != http.StatusUnprocessableEntity { t.Errorf("Unexpected non-error reading container logs: %#v", resp) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go index 560c889b9c1..6ede64718af 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go @@ -28,16 +28,6 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) -// HTTP Status codes not in the golang http package. -const ( - StatusUnprocessableEntity = 422 - StatusTooManyRequests = 429 - // StatusServerTimeout is an indication that a transient server error has - // occurred and the client *should* retry, with an optional Retry-After - // header to specify the back off window. - StatusServerTimeout = 504 -) - // StatusError is an error intended for consumption by a REST API server; it can also be // reconstructed by clients from a REST response. Public to allow easy type switches. type StatusError struct { @@ -189,7 +179,7 @@ func NewInvalid(qualifiedKind schema.GroupKind, name string, errs field.ErrorLis } return &StatusError{metav1.Status{ Status: metav1.StatusFailure, - Code: StatusUnprocessableEntity, // RFC 4918: StatusUnprocessableEntity + Code: http.StatusUnprocessableEntity, Reason: metav1.StatusReasonInvalid, Details: &metav1.StatusDetails{ Group: qualifiedKind.Group, @@ -211,6 +201,21 @@ func NewBadRequest(reason string) *StatusError { }} } +// NewTooManyRequests creates an error that indicates that the client must try again later because +// the specified endpoint is not accepting requests. More specific details should be provided +// if client should know why the failure was limited4. +func NewTooManyRequests(message string, retryAfterSeconds int) *StatusError { + return &StatusError{metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusTooManyRequests, + Reason: metav1.StatusReasonTooManyRequests, + Message: message, + Details: &metav1.StatusDetails{ + RetryAfterSeconds: int32(retryAfterSeconds), + }, + }} +} + // NewServiceUnavailable creates an error that indicates that the requested service is unavailable. func NewServiceUnavailable(reason string) *StatusError { return &StatusError{metav1.Status{ @@ -276,7 +281,7 @@ func NewInternalError(err error) *StatusError { func NewTimeoutError(message string, retryAfterSeconds int) *StatusError { return &StatusError{metav1.Status{ Status: metav1.StatusFailure, - Code: StatusServerTimeout, + Code: http.StatusGatewayTimeout, Reason: metav1.StatusReasonTimeout, Message: fmt.Sprintf("Timeout: %s", message), Details: &metav1.StatusDetails{ @@ -313,14 +318,14 @@ func NewGenericServerResponse(code int, verb string, qualifiedResource schema.Gr case http.StatusMethodNotAllowed: reason = metav1.StatusReasonMethodNotAllowed message = "the server does not allow this method on the requested resource" - case StatusUnprocessableEntity: + case http.StatusUnprocessableEntity: reason = metav1.StatusReasonInvalid message = "the server rejected our request due to an error in our request" - case StatusServerTimeout: - reason = metav1.StatusReasonServerTimeout - message = "the server cannot complete the requested operation at this time, try again later" - case StatusTooManyRequests: + case http.StatusGatewayTimeout: reason = metav1.StatusReasonTimeout + message = "the server was unable to return a response in the time allotted, but may still be processing the request" + case http.StatusTooManyRequests: + reason = metav1.StatusReasonTooManyRequests message = "the server has received too many requests and has asked us to try again later" default: if code >= 500 { @@ -423,11 +428,13 @@ func IsInternalError(err error) bool { // IsTooManyRequests determines if err is an error which indicates that there are too many requests // that the server cannot handle. -// TODO: update IsTooManyRequests() when the TooManyRequests(429) error returned from the API server has a non-empty Reason field func IsTooManyRequests(err error) bool { + if reasonForError(err) == metav1.StatusReasonTooManyRequests { + return true + } switch t := err.(type) { case APIStatus: - return t.Status().Code == StatusTooManyRequests + return t.Status().Code == http.StatusTooManyRequests } return false } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 14c7255c92b..8c1709e28a5 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -586,6 +586,15 @@ const ( // Status code 504 StatusReasonTimeout StatusReason = "Timeout" + // StatusReasonTooManyRequests means the server experienced too many requests within a + // given window and that the client must wait to perform the action again. A client may + // always retry the request that led to this error, although the client should wait at least + // the number of seconds specified by the retryAfterSeconds field. + // Details (optional): + // "retryAfterSeconds" int32 - the number of seconds before the operation should be retried + // Status code 429 + StatusReasonTooManyRequests StatusReason = "TooManyRequests" + // StatusReasonBadRequest means that the request itself was invalid, because the request // doesn't make any sense, for example deleting a read-only object. This is different than // StatusReasonInvalid above which indicates that the API call could possibly succeed, but the diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index b28dbc1a1a1..01a4fb9ac07 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -3834,7 +3834,7 @@ func TestCreateTimeout(t *testing.T) { if err != nil { t.Errorf("unexpected error: %v", err) } - itemOut := expectApiStatus(t, "POST", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/foo?timeout=4ms", data, apierrs.StatusServerTimeout) + itemOut := expectApiStatus(t, "POST", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/foo?timeout=4ms", data, http.StatusGatewayTimeout) if itemOut.Status != metav1.StatusFailure || itemOut.Reason != metav1.StatusReasonTimeout { t.Errorf("Unexpected status %#v", itemOut) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index b8f6a038689..1e49cfa7d74 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -509,7 +509,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag // http.StatusUnsupportedMediaType, http.StatusNotAcceptable, // http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, // http.StatusRequestTimeout, http.StatusConflict, http.StatusPreconditionFailed, - // 422 (StatusUnprocessableEntity), http.StatusInternalServerError, + // http.StatusUnprocessableEntity, http.StatusInternalServerError, // http.StatusServiceUnavailable // and api error codes // Note that if we specify a versioned Status object here, we may need to diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/maxinflight.go b/staging/src/k8s.io/apiserver/pkg/server/filters/maxinflight.go index 3e159c21f28..883a27dc355 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/maxinflight.go +++ b/staging/src/k8s.io/apiserver/pkg/server/filters/maxinflight.go @@ -123,5 +123,5 @@ func WithMaxInFlightLimit( func tooManyRequests(req *http.Request, w http.ResponseWriter) { // Return a 429 status indicating "Too Many Requests" w.Header().Set("Retry-After", retryAfter) - http.Error(w, "Too many requests, please try again later.", errors.StatusTooManyRequests) + http.Error(w, "Too many requests, please try again later.", http.StatusTooManyRequests) } diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index 2d676cec06f..68ac4752682 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/staging/src/k8s.io/client-go/rest/request.go @@ -889,7 +889,7 @@ func isTextResponse(resp *http.Response) bool { func checkWait(resp *http.Response) (int, bool) { switch r := resp.StatusCode; { // any 500 error code and 429 can trigger a wait - case r == errors.StatusTooManyRequests, r >= 500: + case r == http.StatusTooManyRequests, r >= 500: default: return 0, false } diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index fa016dfe1ce..0691fc7260c 100755 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/staging/src/k8s.io/client-go/rest/request_test.go @@ -1130,7 +1130,7 @@ func TestCheckRetryClosesBody(t *testing.T) { return } w.Header().Set("Retry-After", "1") - http.Error(w, "Too many requests, please try again later.", apierrors.StatusTooManyRequests) + http.Error(w, "Too many requests, please try again later.", http.StatusTooManyRequests) })) defer testServer.Close() @@ -1204,7 +1204,7 @@ func TestCheckRetryHandles429And5xx(t *testing.T) { return } w.Header().Set("Retry-After", "0") - w.WriteHeader([]int{apierrors.StatusTooManyRequests, 500, 501, 504}[count]) + w.WriteHeader([]int{http.StatusTooManyRequests, 500, 501, 504}[count]) count++ })) defer testServer.Close() @@ -1234,7 +1234,7 @@ func BenchmarkCheckRetryClosesBody(b *testing.B) { return } w.Header().Set("Retry-After", "0") - w.WriteHeader(apierrors.StatusTooManyRequests) + w.WriteHeader(http.StatusTooManyRequests) })) defer testServer.Close()