From 26d1a7d454d9e50e6e9420c8ce11d2bed996de9d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 12 Sep 2019 10:56:30 +0200 Subject: [PATCH] docker: handle http 429 status codes Consolidate checking the http-status codes to allow for a more uniform error handling. Also treat code 429 (too many requests) as a known error instead of an invalid status code. When hitting 429, perform an exponential back off starting a 2 seconds for at most 5 iterations. If the http.Response set the `Retry-Header` then use the provided value or date to compute the delay until the next attempt. Note that the maximum delay is 60 seconds. Signed-off-by: Valentin Rothberg --- docker/docker_client.go | 105 +++++++++++++++++++++++++++--------- docker/docker_image.go | 6 +-- docker/docker_image_dest.go | 2 +- docker/docker_image_src.go | 5 +- docker/errors.go | 32 +++++++++++ 5 files changed, 118 insertions(+), 32 deletions(-) create mode 100644 docker/errors.go diff --git a/docker/docker_client.go b/docker/docker_client.go index 48427f3d37..fef26e26b8 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -21,7 +21,7 @@ import ( "github.com/containers/image/pkg/sysregistriesv2" "github.com/containers/image/pkg/tlsclientconfig" "github.com/containers/image/types" - "github.com/docker/distribution/registry/client" + clientLib "github.com/docker/distribution/registry/client" "github.com/docker/go-connections/tlsconfig" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" @@ -47,14 +47,7 @@ const ( extensionSignatureTypeAtomic = "atomic" // extensionSignature.Type ) -var ( - // ErrV1NotSupported is returned when we're trying to talk to a - // docker V1 registry. - ErrV1NotSupported = errors.New("can't talk to a V1 docker registry") - // ErrUnauthorizedForCredentials is returned when the status code returned is 401 - ErrUnauthorizedForCredentials = errors.New("unable to retrieve auth token: invalid username/password") - systemPerHostCertDirPaths = [2]string{"/etc/containers/certs.d", "/etc/docker/certs.d"} -) +var systemPerHostCertDirPaths = [2]string{"/etc/containers/certs.d", "/etc/docker/certs.d"} // extensionSignature and extensionSignatureList come from github.com/openshift/origin/pkg/dockerregistry/server/signaturedispatcher.go: // signature represents a Docker image signature. @@ -284,14 +277,7 @@ func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password } defer resp.Body.Close() - switch resp.StatusCode { - case http.StatusOK: - return nil - case http.StatusUnauthorized: - return ErrUnauthorizedForCredentials - default: - return errors.Errorf("error occured with status code %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) - } + return httpResponseToError(resp) } // SearchResult holds the information of each matching image @@ -364,8 +350,8 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima logrus.Debugf("error getting search results from v1 endpoint %q: %v", registry, err) } else { defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - logrus.Debugf("error getting search results from v1 endpoint %q, status code %d (%s)", registry, resp.StatusCode, http.StatusText(resp.StatusCode)) + if resp.StatusCode == http.StatusOK { + logrus.Debugf("error getting search results from v1 endpoint %q: %v", registry, httpResponseToError(resp)) } else { if err := json.NewDecoder(resp.Body).Decode(v1Res); err != nil { return nil, err @@ -381,8 +367,8 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima logrus.Debugf("error getting search results from v2 endpoint %q: %v", registry, err) } else { defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - logrus.Errorf("error getting search results from v2 endpoint %q, status code %d (%s)", registry, resp.StatusCode, http.StatusText(resp.StatusCode)) + if resp.StatusCode == http.StatusOK { + logrus.Errorf("error getting search results from v2 endpoint %q: %v", registry, httpResponseToError(resp)) } else { if err := json.NewDecoder(resp.Body).Decode(v2Res); err != nil { return nil, err @@ -417,8 +403,77 @@ func (c *dockerClient) makeRequest(ctx context.Context, method, path string, hea // makeRequestToResolvedURL creates and executes a http.Request with the specified parameters, adding authentication and TLS options for the Docker client. // streamLen, if not -1, specifies the length of the data expected on stream. // makeRequest should generally be preferred. +// In case of an http 429 status code in the response, it performs an exponential back off starting at 2 seconds for at most 5 iterations. +// If the `Retry-After` header is set in the response, the specified value or date is +// If the stream is non-nil, no back off will be performed. // TODO(runcom): too many arguments here, use a struct func (c *dockerClient) makeRequestToResolvedURL(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { + var ( + res *http.Response + err error + delay int64 + ) + delay = 2 + const numIterations = 5 + const maxDelay = 60 + + // math.Min() only supports float64, so have an anonymous func to avoid + // casting. + min := func(a int64, b int64) int64 { + if a < b { + return a + } + return b + } + + nextDelay := func(r *http.Response, delay int64) int64 { + after := res.Header.Get("Retry-After") + if after == "" { + return min(delay, maxDelay) + } + logrus.Debugf("detected 'Retry-After' header %q", after, delay) + // First check if we have a numerical value. + if num, err := strconv.ParseInt(after, 10, 64); err == nil { + return min(num, maxDelay) + } + // Secondly check if we have an http date. + // If the delta between the date and now is positive, use it. + // Otherwise, fall back to using the default exponential back off. + if t, err := http.ParseTime(after); err == nil { + delta := int64(t.Sub(time.Now()).Seconds()) + if delta > 0 { + return min(delta, maxDelay) + } + logrus.Debugf("negative date: falling back to using %d seconds", after, delay) + return min(delay, maxDelay) + } + // If the header contains bogus, fall back to using the default + // exponential back off. + logrus.Debugf("invalid format: falling back to using %d seconds", after, delay) + return min(delay, maxDelay) + } + + for i := 0; i < numIterations; i++ { + res, err = c.makeRequestToResolvedURLOnce(ctx, method, url, headers, stream, streamLen, auth, extraScope) + if stream == nil && res != nil && res.StatusCode == http.StatusTooManyRequests { + if i < numIterations-1 { + delay = nextDelay(res, delay) // compute next delay - does NOT exceed maxDelay + logrus.Debugf("too many request to %s: sleeping for %d seconds before next attempt", url, delay) + time.Sleep(time.Duration(delay) * time.Second) + delay = delay * 2 // exponential back off + } + continue + } + break + } + return res, err +} + +// makeRequestToResolvedURLOnce creates and executes a http.Request with the specified parameters, adding authentication and TLS options for the Docker client. +// streamLen, if not -1, specifies the length of the data expected on stream. +// makeRequest should generally be preferred. +// Note that no exponential back off is performed when receiving an http 429 status code. +func (c *dockerClient) makeRequestToResolvedURLOnce(ctx context.Context, method, url string, headers map[string][]string, stream io.Reader, streamLen int64, auth sendAuth, extraScope *authScope) (*http.Response, error) { req, err := http.NewRequest(method, url, stream) if err != nil { return nil, err @@ -569,7 +624,7 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { defer resp.Body.Close() logrus.Debugf("Ping %s status %d", url, resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { - return errors.Errorf("error pinging registry %s, response code %d (%s)", c.registry, resp.StatusCode, http.StatusText(resp.StatusCode)) + return httpResponseToError(resp) } c.challenges = parseAuthHeader(resp.Header) c.scheme = scheme @@ -581,7 +636,7 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { err = ping("http") } if err != nil { - err = errors.Wrap(err, "pinging docker registry returned") + err = errors.Wrapf(err, "error pinging docker registry %s", c.registry) if c.sys != nil && c.sys.DockerDisableV1Ping { return err } @@ -627,9 +682,11 @@ func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerRe return nil, err } defer res.Body.Close() + if res.StatusCode != http.StatusOK { - return nil, errors.Wrapf(client.HandleErrorResponse(res), "Error downloading signatures for %s in %s", manifestDigest, ref.ref.Name()) + return nil, errors.Wrapf(clientLib.HandleErrorResponse(res), "Error downloading signatures for %s in %s", manifestDigest, ref.ref.Name()) } + body, err := ioutil.ReadAll(res.Body) if err != nil { return nil, err diff --git a/docker/docker_image.go b/docker/docker_image.go index 744667f549..118cb87f78 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "net/http" "net/url" "strings" @@ -71,9 +70,8 @@ func GetRepositoryTags(ctx context.Context, sys *types.SystemContext, ref types. return nil, err } defer res.Body.Close() - if res.StatusCode != http.StatusOK { - // print url also - return nil, errors.Errorf("Invalid status code returned when fetching tags list %d (%s)", res.StatusCode, http.StatusText(res.StatusCode)) + if err := httpResponseToError(res); err != nil { + return nil, err } var tagsHolder struct { diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index c116cbec32..8bb9ee803f 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -19,7 +19,7 @@ import ( "github.com/containers/image/pkg/blobinfocache/none" "github.com/containers/image/types" "github.com/docker/distribution/registry/api/errcode" - "github.com/docker/distribution/registry/api/v2" + v2 "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client" "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 6951f31e93..99b1c16b4a 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -232,9 +232,8 @@ func (s *dockerImageSource) GetBlob(ctx context.Context, info types.BlobInfo, ca if err != nil { return nil, 0, err } - if res.StatusCode != http.StatusOK { - // print url also - return nil, 0, errors.Errorf("Invalid status code returned when fetching blob %d (%s)", res.StatusCode, http.StatusText(res.StatusCode)) + if err := httpResponseToError(res); err != nil { + return nil, 0, err } cache.RecordKnownLocation(s.ref.Transport(), bicTransportScope(s.ref), info.Digest, newBICLocationReference(s.ref)) return res.Body, getBlobSize(res), nil diff --git a/docker/errors.go b/docker/errors.go new file mode 100644 index 0000000000..df182a422e --- /dev/null +++ b/docker/errors.go @@ -0,0 +1,32 @@ +package docker + +import ( + "net/http" + + "github.com/pkg/errors" +) + +var ( + // ErrV1NotSupported is returned when we're trying to talk to a + // docker V1 registry. + ErrV1NotSupported = errors.New("can't talk to a V1 docker registry") + // ErrUnauthorizedForCredentials is returned when the status code returned is 401 + ErrUnauthorizedForCredentials = errors.New("unable to retrieve auth token: invalid username/password") + // ErrTooManyRequests is returned when the status code returned is 429 + ErrTooManyRequests = errors.New("too many request to registry") +) + +// httpResponseToError translates the https.Response into an error. It returns +// nil if the response is not considered an error. +func httpResponseToError(res *http.Response) error { + switch res.StatusCode { + case http.StatusOK: + return nil + case http.StatusTooManyRequests: + return ErrTooManyRequests + case http.StatusUnauthorized: + return ErrUnauthorizedForCredentials + default: + return errors.Errorf("invalid status code from registry %d (%s)", res.StatusCode, http.StatusText(res.StatusCode)) + } +}