Merge pull request #35376 from KingEmet/master
Permit a broader range of errors from mirror endpoints when determining whether to fall back
diff --git a/distribution/errors.go b/distribution/errors.go
index dd6ff0a..355e9da 100644
--- a/distribution/errors.go
+++ b/distribution/errors.go
@@ -126,21 +126,25 @@
// 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()))
}
diff --git a/distribution/errors_test.go b/distribution/errors_test.go
new file mode 100644
index 0000000..aa9ef4f
--- /dev/null
+++ b/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")
+ }
+}
diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go
index c8d784c..35ff529 100644
--- a/distribution/pull_v2.go
+++ b/distribution/pull_v2.go
@@ -74,7 +74,7 @@
if _, ok := err.(fallbackError); ok {
return err
}
- if continueOnError(err) {
+ if continueOnError(err, p.endpoint.Mirror) {
return fallbackError{
err: err,
confirmedV2: p.confirmedV2,
diff --git a/distribution/push_v2.go b/distribution/push_v2.go
index 7ffce5b..2aecc18 100644
--- a/distribution/push_v2.go
+++ b/distribution/push_v2.go
@@ -67,7 +67,7 @@
}
if err = p.pushV2Repository(ctx); err != nil {
- if continueOnError(err) {
+ if continueOnError(err, p.endpoint.Mirror) {
return fallbackError{
err: err,
confirmedV2: p.pushState.confirmedV2,
