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 <qiwan@redhat.com>

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 {

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 {
+func retryCopyImage(ctx context.Context, policyContext *signature.PolicyContext, dest, src, registry types.ImageReference, copyOptions *cp.Options, maxRetries int) ([]byte, error) {
-	err = errors.Cause(err)
+	var (
-	type unwrapper interface {
+		manifestBytes []byte
-		Unwrap() error
+		err           error
-	}
+		lastErr       error
-	if unwrapper, ok := err.(unwrapper); ok {
+	)
-		err = unwrapper.Unwrap()
+	err = retry.RetryIfNecessary(ctx, func() error {
-		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)
 		manifestBytes, err = cp.Image(ctx, policyContext, dest, src, copyOptions)
-		if err == nil {
+		if registry != nil && registry.Transport().Name() != docker.Transport.Name() {
-			break
+			lastErr = err
+			return nil
 		}
+		return err
+	}, &retry.RetryOptions{MaxRetry: maxRetries})
+	if lastErr != nil {
+		err = lastErr
 	}
 	return manifestBytes, err
 }

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
 	}

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

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