Update generic errors with the new http package codes

All of these errors are now part of the standard HTTP method. Formalize
those into our error types and remove duplication and unclear
separation.

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
 	}
 

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)
 	}
 }

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:
+	case http.StatusGatewayTimeout:
-		reason = metav1.StatusReasonServerTimeout
-		message = "the server cannot complete the requested operation at this time, try again later"
-	case StatusTooManyRequests:
 		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
 }

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

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)
 	}

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

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)
 }

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
 	}

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()