From cea59d97c91bb7cf8841dad7bfc64be71a4fad43 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Thu, 13 Aug 2020 15:19:38 -0400 Subject: [PATCH] Replace retry functions with common/pkg/retry Use retry pacakge from containers/common and change the retryDelay to exponential backoff from there. Signed-off-by: Qi Wang --- commit.go | 4 +- common.go | 80 ++++------------- pull.go | 2 +- .../containers/common/pkg/retry/retry.go | 87 +++++++++++++++++++ vendor/modules.txt | 1 + 5 files changed, 106 insertions(+), 68 deletions(-) create mode 100644 vendor/github.com/containers/common/pkg/retry/retry.go diff --git a/commit.go b/commit.go index 6c3febd5d..24063f940 100644 --- a/commit.go +++ b/commit.go @@ -344,7 +344,7 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options } var manifestBytes []byte - if manifestBytes, err = retryCopyImage(ctx, policyContext, maybeCachedDest, maybeCachedSrc, dest, "push", getCopyOptions(b.store, options.ReportWriter, nil, systemContext, "", false, options.SignBy, options.OciEncryptLayers, options.OciEncryptConfig, nil), options.MaxRetries, options.RetryDelay); err != nil { + if manifestBytes, err = retryCopyImage(ctx, policyContext, maybeCachedDest, maybeCachedSrc, dest, getCopyOptions(b.store, options.ReportWriter, nil, systemContext, "", false, options.SignBy, options.OciEncryptLayers, options.OciEncryptConfig, nil), options.MaxRetries); err != nil { return imgID, nil, "", errors.Wrapf(err, "error copying layers and metadata for container %q", b.ContainerID) } // If we've got more names to attach, and we know how to do that for @@ -476,7 +476,7 @@ func Push(ctx context.Context, image string, dest types.ImageReference, options systemContext.DirForceCompress = true } var manifestBytes []byte - if manifestBytes, err = retryCopyImage(ctx, policyContext, dest, maybeCachedSrc, dest, "push", getCopyOptions(options.Store, options.ReportWriter, nil, systemContext, options.ManifestType, options.RemoveSignatures, options.SignBy, options.OciEncryptLayers, options.OciEncryptConfig, nil), options.MaxRetries, options.RetryDelay); err != nil { + if manifestBytes, err = retryCopyImage(ctx, policyContext, dest, maybeCachedSrc, dest, getCopyOptions(options.Store, options.ReportWriter, nil, systemContext, options.ManifestType, options.RemoveSignatures, options.SignBy, options.OciEncryptLayers, options.OciEncryptConfig, nil), options.MaxRetries); err != nil { return nil, "", errors.Wrapf(err, "error copying layers and metadata from %q to %q", transports.ImageName(maybeCachedSrc), transports.ImageName(dest)) } if options.ReportWriter != nil { diff --git a/common.go b/common.go index b43cfffc9..d83e5417e 100644 --- a/common.go +++ b/common.go @@ -3,13 +3,10 @@ package buildah import ( "context" "io" - "net" - "net/url" "os" "path/filepath" - "syscall" - "time" + "github.com/containers/common/pkg/retry" cp "github.com/containers/image/v5/copy" "github.com/containers/image/v5/docker" "github.com/containers/image/v5/signature" @@ -17,11 +14,6 @@ import ( encconfig "github.com/containers/ocicrypt/config" "github.com/containers/storage" "github.com/containers/storage/pkg/unshare" - "github.com/docker/distribution/registry/api/errcode" - errcodev2 "github.com/docker/distribution/registry/api/v2" - multierror "github.com/hashicorp/go-multierror" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) const ( @@ -76,64 +68,22 @@ func getSystemContext(store storage.Store, defaults *types.SystemContext, signat return sc } -func isRetryable(err error) bool { - err = errors.Cause(err) - type unwrapper interface { - Unwrap() error - } - if unwrapper, ok := err.(unwrapper); ok { - err = unwrapper.Unwrap() - return isRetryable(err) - } - if registryError, ok := err.(errcode.Error); ok { - switch registryError.Code { - case errcode.ErrorCodeUnauthorized, errcodev2.ErrorCodeNameUnknown, errcodev2.ErrorCodeManifestUnknown: - return false - } - return true - } - if op, ok := err.(*net.OpError); ok { - return isRetryable(op.Err) - } - if url, ok := err.(*url.Error); ok { - return isRetryable(url.Err) - } - if errno, ok := err.(syscall.Errno); ok { - if errno == syscall.ECONNREFUSED { - return false - } - } - if errs, ok := err.(errcode.Errors); ok { - // if this error is a group of errors, process them all in turn - for i := range errs { - if !isRetryable(errs[i]) { - return false - } - } - } - if errs, ok := err.(*multierror.Error); ok { - // if this error is a group of errors, process them all in turn - for i := range errs.Errors { - if !isRetryable(errs.Errors[i]) { - return false - } - } - } - return true -} - -func retryCopyImage(ctx context.Context, policyContext *signature.PolicyContext, dest, src, registry types.ImageReference, action string, copyOptions *cp.Options, maxRetries int, retryDelay time.Duration) ([]byte, error) { - manifestBytes, err := cp.Image(ctx, policyContext, dest, src, copyOptions) - for retries := 0; err != nil && isRetryable(err) && registry != nil && registry.Transport().Name() == docker.Transport.Name() && retries < maxRetries; retries++ { - if retryDelay == 0 { - retryDelay = 5 * time.Second - } - logrus.Infof("Warning: %s failed, retrying in %s ... (%d/%d)", action, retryDelay, retries+1, maxRetries) - time.Sleep(retryDelay) +func retryCopyImage(ctx context.Context, policyContext *signature.PolicyContext, dest, src, registry types.ImageReference, copyOptions *cp.Options, maxRetries int) ([]byte, error) { + var ( + manifestBytes []byte + err error + lastErr error + ) + err = retry.RetryIfNecessary(ctx, func() error { manifestBytes, err = cp.Image(ctx, policyContext, dest, src, copyOptions) - if err == nil { - break + if registry != nil && registry.Transport().Name() != docker.Transport.Name() { + lastErr = err + return nil } + return err + }, &retry.RetryOptions{MaxRetry: maxRetries}) + if lastErr != nil { + err = lastErr } return manifestBytes, err } diff --git a/pull.go b/pull.go index f8d4bdeb6..a57bf5914 100644 --- a/pull.go +++ b/pull.go @@ -280,7 +280,7 @@ func pullImage(ctx context.Context, store storage.Store, srcRef types.ImageRefer }() logrus.Debugf("copying %q to %q", transports.ImageName(srcRef), destName) - if _, err := retryCopyImage(ctx, policyContext, maybeCachedDestRef, srcRef, srcRef, "pull", getCopyOptions(store, options.ReportWriter, sc, nil, "", options.RemoveSignatures, "", nil, nil, options.OciDecryptConfig), options.MaxRetries, options.RetryDelay); err != nil { + if _, err := retryCopyImage(ctx, policyContext, maybeCachedDestRef, srcRef, srcRef, getCopyOptions(store, options.ReportWriter, sc, nil, "", options.RemoveSignatures, "", nil, nil, options.OciDecryptConfig), options.MaxRetries); err != nil { logrus.Debugf("error copying src image [%q] to dest image [%q] err: %v", transports.ImageName(srcRef), destName, err) return nil, err } diff --git a/vendor/github.com/containers/common/pkg/retry/retry.go b/vendor/github.com/containers/common/pkg/retry/retry.go new file mode 100644 index 000000000..c20f900d8 --- /dev/null +++ b/vendor/github.com/containers/common/pkg/retry/retry.go @@ -0,0 +1,87 @@ +package retry + +import ( + "context" + "math" + "net" + "net/url" + "syscall" + "time" + + "github.com/docker/distribution/registry/api/errcode" + errcodev2 "github.com/docker/distribution/registry/api/v2" + "github.com/hashicorp/go-multierror" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// RetryOptions defines the option to retry +type RetryOptions struct { + MaxRetry int // The number of times to possibly retry +} + +// RetryIfNecessary retries the operation in exponential backoff with the retryOptions +func RetryIfNecessary(ctx context.Context, operation func() error, retryOptions *RetryOptions) error { + err := operation() + for attempt := 0; err != nil && isRetryable(err) && attempt < retryOptions.MaxRetry; attempt++ { + delay := time.Duration(int(math.Pow(2, float64(attempt)))) * time.Second + logrus.Infof("Warning: failed, retrying in %s ... (%d/%d)", delay, attempt+1, retryOptions.MaxRetry) + select { + case <-time.After(delay): + break + case <-ctx.Done(): + return err + } + err = operation() + } + return err +} + +func isRetryable(err error) bool { + err = errors.Cause(err) + + if err == context.Canceled || err == context.DeadlineExceeded { + return false + } + + type unwrapper interface { + Unwrap() error + } + + switch e := err.(type) { + + case errcode.Error: + switch e.Code { + case errcode.ErrorCodeUnauthorized, errcodev2.ErrorCodeNameUnknown, errcodev2.ErrorCodeManifestUnknown: + return false + } + return true + case *net.OpError: + return isRetryable(e.Err) + case *url.Error: + return isRetryable(e.Err) + case syscall.Errno: + return e != syscall.ECONNREFUSED + case errcode.Errors: + // if this error is a group of errors, process them all in turn + for i := range e { + if !isRetryable(e[i]) { + return false + } + } + return true + case *multierror.Error: + // if this error is a group of errors, process them all in turn + for i := range e.Errors { + if !isRetryable(e.Errors[i]) { + return false + } + } + return true + case unwrapper: + err = e.Unwrap() + return isRetryable(err) + } + + return false +} diff --git a/vendor/modules.txt b/vendor/modules.txt index ae81839ef..4e7b46c06 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -60,6 +60,7 @@ github.com/containers/common/pkg/auth github.com/containers/common/pkg/capabilities github.com/containers/common/pkg/cgroupv2 github.com/containers/common/pkg/config +github.com/containers/common/pkg/retry github.com/containers/common/version # github.com/containers/image/v5 v5.5.1 github.com/containers/image/v5/copy