Merge pull request #35376 from KingEmet/master

Permit a broader range of errors from mirror endpoints when determining whether to fall back

distribution/errors.go

@@ -126,21 +126,25 @@ func TranslatePullError(err error, ref reference.Named) error {
 
 // continueOnError returns true if we should fallback to the next endpoint
 // as a result of this error.
-func continueOnError(err error) bool {
+func continueOnError(err error, mirrorEndpoint bool) bool {
 	switch v := err.(type) {
 	case errcode.Errors:
 		if len(v) == 0 {
 			return true
 		}
-		return continueOnError(v[0])
+		return continueOnError(v[0], mirrorEndpoint)
 	case ErrNoSupport:
-		return continueOnError(v.Err)
+		return continueOnError(v.Err, mirrorEndpoint)
 	case errcode.Error:
-		return shouldV2Fallback(v)
+		return mirrorEndpoint || shouldV2Fallback(v)
 	case *client.UnexpectedHTTPResponseError:
 		return true
 	case ImageConfigPullError:
-		return false
+		// ImageConfigPullError only happens with v2 images, v1 fallback is
+		// unnecessary.
+		// Failures from a mirror endpoint should result in fallback to the
+		// canonical repo.
+		return mirrorEndpoint
 	case error:
 		return !strings.Contains(err.Error(), strings.ToLower(syscall.ESRCH.Error()))
 	}

distribution/errors_test.go

@@ -0,0 +1,85 @@
+package distribution
+
+import (
+	"errors"
+	"strings"
+	"syscall"
+	"testing"
+
+	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/docker/distribution/registry/api/v2"
+	"github.com/docker/distribution/registry/client"
+)
+
+var alwaysContinue = []error{
+	&client.UnexpectedHTTPResponseError{},
+
+	// Some errcode.Errors that don't disprove the existence of a V1 image
+	errcode.Error{Code: errcode.ErrorCodeUnauthorized},
+	errcode.Error{Code: v2.ErrorCodeManifestUnknown},
+	errcode.Error{Code: v2.ErrorCodeNameUnknown},
+
+	errors.New("some totally unexpected error"),
+}
+
+var continueFromMirrorEndpoint = []error{
+	ImageConfigPullError{},
+
+	// Some other errcode.Error that doesn't indicate we should search for a V1 image.
+	errcode.Error{Code: errcode.ErrorCodeTooManyRequests},
+}
+
+var neverContinue = []error{
+	errors.New(strings.ToLower(syscall.ESRCH.Error())), // No such process
+}
+
+func TestContinueOnError_NonMirrorEndpoint(t *testing.T) {
+	for _, err := range alwaysContinue {
+		if !continueOnError(err, false) {
+			t.Errorf("Should continue from non-mirror endpoint: %T: '%s'", err, err.Error())
+		}
+	}
+
+	for _, err := range continueFromMirrorEndpoint {
+		if continueOnError(err, false) {
+			t.Errorf("Should only continue from mirror endpoint: %T: '%s'", err, err.Error())
+		}
+	}
+}
+
+func TestContinueOnError_MirrorEndpoint(t *testing.T) {
+	errs := []error{}
+	errs = append(errs, alwaysContinue...)
+	errs = append(errs, continueFromMirrorEndpoint...)
+	for _, err := range errs {
+		if !continueOnError(err, true) {
+			t.Errorf("Should continue from mirror endpoint: %T: '%s'", err, err.Error())
+		}
+	}
+}
+
+func TestContinueOnError_NeverContinue(t *testing.T) {
+	for _, isMirrorEndpoint := range []bool{true, false} {
+		for _, err := range neverContinue {
+			if continueOnError(err, isMirrorEndpoint) {
+				t.Errorf("Should never continue: %T: '%s'", err, err.Error())
+			}
+		}
+	}
+}
+
+func TestContinueOnError_UnnestsErrors(t *testing.T) {
+	// ContinueOnError should evaluate nested errcode.Errors.
+
+	// Assumes that v2.ErrorCodeNameUnknown is a continueable error code.
+	err := errcode.Errors{errcode.Error{Code: v2.ErrorCodeNameUnknown}}
+	if !continueOnError(err, false) {
+		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
+	}
+
+	// Assumes that errcode.ErrorCodeTooManyRequests is not a V1-fallback indication
+	err = errcode.Errors{errcode.Error{Code: errcode.ErrorCodeTooManyRequests}}
+	if continueOnError(err, false) {
+		t.Fatal("ContinueOnError should unnest, base return value on errcode.Errors")
+	}
+}

distribution/pull_v2.go

@@ -74,7 +74,7 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named, platform strin
 		if _, ok := err.(fallbackError); ok {
 			return err
 		}
-		if continueOnError(err) {
+		if continueOnError(err, p.endpoint.Mirror) {
 			return fallbackError{
 				err:         err,
 				confirmedV2: p.confirmedV2,

distribution/push_v2.go

@@ -67,7 +67,7 @@ func (p *v2Pusher) Push(ctx context.Context) (err error) {
 	}
 
 	if err = p.pushV2Repository(ctx); err != nil {
-		if continueOnError(err) {
+		if continueOnError(err, p.endpoint.Mirror) {
 			return fallbackError{
 				err:         err,
 				confirmedV2: p.pushState.confirmedV2,