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 6ede64718af..91975be85b3 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go @@ -462,13 +462,20 @@ func IsUnexpectedObjectError(err error) bool { } // SuggestsClientDelay returns true if this error suggests a client delay as well as the -// suggested seconds to wait, or false if the error does not imply a wait. +// suggested seconds to wait, or false if the error does not imply a wait. It does not +// address whether the error *should* be retried, since some errors (like a 3xx) may +// request delay without retry. func SuggestsClientDelay(err error) (int, bool) { switch t := err.(type) { case APIStatus: if t.Status().Details != nil { switch t.Status().Reason { - case metav1.StatusReasonServerTimeout, metav1.StatusReasonTimeout: + // this StatusReason explicitly requests the caller to delay the action + case metav1.StatusReasonServerTimeout: + return int(t.Status().Details.RetryAfterSeconds), true + } + // If the client requests that we retry after a certain number of seconds + if t.Status().Details.RetryAfterSeconds > 0 { return int(t.Status().Details.RetryAfterSeconds), true } } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go index ab14e7af9a1..afc26fd31c4 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go @@ -83,15 +83,34 @@ func TestErrorNew(t *testing.T) { if !IsServerTimeout(NewServerTimeout(resource("tests"), "reason", 0)) { t.Errorf("expected to be %s", metav1.StatusReasonServerTimeout) } - if time, ok := SuggestsClientDelay(NewServerTimeout(resource("tests"), "doing something", 10)); time != 10 || !ok { - t.Errorf("expected to be %s", metav1.StatusReasonServerTimeout) - } - if time, ok := SuggestsClientDelay(NewTimeoutError("test reason", 10)); time != 10 || !ok { - t.Errorf("expected to be %s", metav1.StatusReasonTimeout) - } if !IsMethodNotSupported(NewMethodNotSupported(resource("foos"), "delete")) { t.Errorf("expected to be %s", metav1.StatusReasonMethodNotAllowed) } + + if time, ok := SuggestsClientDelay(NewServerTimeout(resource("tests"), "doing something", 10)); time != 10 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewServerTimeout(resource("tests"), "doing something", 0)); time != 0 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewTimeoutError("test reason", 10)); time != 10 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewTooManyRequests("doing something", 10)); time != 10 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewTooManyRequests("doing something", 1)); time != 1 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewGenericServerResponse(429, "get", resource("tests"), "test", "doing something", 10, true)); time != 10 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewGenericServerResponse(500, "get", resource("tests"), "test", "doing something", 10, true)); time != 10 || !ok { + t.Errorf("unexpected %d", time) + } + if time, ok := SuggestsClientDelay(NewGenericServerResponse(429, "get", resource("tests"), "test", "doing something", 0, true)); time != 0 || ok { + t.Errorf("unexpected %d", time) + } } func TestNewInvalid(t *testing.T) { 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 8c1709e28a5..2e709b64986 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 @@ -481,7 +481,9 @@ type StatusDetails struct { // failure. Not all StatusReasons may provide detailed causes. // +optional Causes []StatusCause `json:"causes,omitempty" protobuf:"bytes,4,rep,name=causes"` - // If specified, the time in seconds before the operation should be retried. + // If specified, the time in seconds before the operation should be retried. Some errors may indicate + // the client must take an alternate action - for those errors this field may indicate how long to wait + // before taking the alternate action. // +optional RetryAfterSeconds int32 `json:"retryAfterSeconds,omitempty" protobuf:"varint,5,opt,name=retryAfterSeconds"` } diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook.go index 4f03b50e74b..b1a897a193a 100755 --- a/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook.go +++ b/staging/src/k8s.io/apiserver/pkg/util/webhook/webhook.go @@ -90,6 +90,11 @@ func WithExponentialBackoff(initialBackoff time.Duration, webhookFn func() error var err error wait.ExponentialBackoff(backoff, func() (bool, error) { err = webhookFn() + // these errors indicate a need to retry an authentication check + if apierrors.IsServerTimeout(err) || apierrors.IsTimeout(err) || apierrors.IsTooManyRequests(err) { + return false, nil + } + // if the error sends the Retry-After header, we respect it as an explicit confirmation we should retry. if _, shouldRetry := apierrors.SuggestsClientDelay(err); shouldRetry { return false, nil }