Add retry on image push 5xx errors

Aaron Lehmann · Aaron Lehmann · commit d3b96f4e31c4 · 2021-03-26T18:13:45.000-07:00
Some registries can be flaky and return intermittent 5xx errors. This change allows those errors to be retried, similarly to network-level errors. Note that this needs the upstream containerd fix containerd/containerd#5276 to work reliably. This was tested with a registry that was modified to return 504 on every other manifest PUT. Without the change, exports to the registry fail every other attempt. With the change and the related containerd change, exports to the registry always succeed. Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
diff --git a/util/contentutil/copy.go b/util/contentutil/copy.go
@@ -65,7 +65,7 @@ func CopyChain(ctx context.Context, ingester content.Ingester, provider content.
 	handlers := []images.Handler{
 		images.ChildrenHandler(provider),
 		filterHandler,
-		retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), nil),
+		retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), func(_ []byte) {}),
 	}
 
 	if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {
diff --git a/util/imageutil/config.go b/util/imageutil/config.go
@@ -101,7 +101,7 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co
 	children := childrenConfigHandler(cache, platform)
 
 	handlers := []images.Handler{
-		retryhandler.New(remotes.FetchHandler(cache, fetcher), nil),
+		retryhandler.New(remotes.FetchHandler(cache, fetcher), func(_ []byte) {}),
 		children,
 	}
 	if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {
diff --git a/util/progress/logs/logs.go b/util/progress/logs/logs.go
@@ -133,6 +133,7 @@ func (sw *streamWriter) Close() error {
 func LoggerFromContext(ctx context.Context) func([]byte) {
 	return func(dt []byte) {
 		pw, _, _ := progress.FromContext(ctx)
+		defer pw.Close()
 		pw.Write(identity.NewID(), client.VertexLog{
 			Stream: stderr,
 			Data:   []byte(dt),
diff --git a/util/resolver/retryhandler/retry.go b/util/resolver/retryhandler/retry.go
@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/containerd/containerd/images"
+	remoteserrors "github.com/containerd/containerd/remotes/errors"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
@@ -48,6 +49,14 @@ func New(f images.HandlerFunc, logger func([]byte)) images.HandlerFunc {
 }
 
 func retryError(err error) bool {
+	// Retry on 5xx errors
+	var errUnexpectedStatus remoteserrors.ErrUnexpectedStatus
+	if errors.As(err, &errUnexpectedStatus) &&
+		errUnexpectedStatus.StatusCode >= 500 &&
+		errUnexpectedStatus.StatusCode <= 599 {
+		return true
+	}
+
 	if errors.Is(err, io.EOF) || errors.Is(err, syscall.ECONNRESET) || errors.Is(err, syscall.EPIPE) {
 		return true
 	}

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ func CopyChain(ctx context.Context, ingester content.Ingester, provider content.`
`65`	`65`	`handlers := []images.Handler{`
`66`	`66`	`images.ChildrenHandler(provider),`
`67`	`67`	`filterHandler,`
`68`		`- retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), nil),`
	`68`	`+ retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), func(_ []byte) {}),`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co`
`101`	`101`	`children := childrenConfigHandler(cache, platform)`
`102`	`102`
`103`	`103`	`handlers := []images.Handler{`
`104`		`- retryhandler.New(remotes.FetchHandler(cache, fetcher), nil),`
	`104`	`+ retryhandler.New(remotes.FetchHandler(cache, fetcher), func(_ []byte) {}),`
`105`	`105`	`children,`
`106`	`106`	`}`
`107`	`107`	`if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {`